search

zopefoundation / z3c.form

An advanced form and widget framework for Zope 3
Other
8 stars 39 forks source link

Added a consistent id on ordered selection widget. #14

Closed tdesvenain closed 10 years ago

tdesvenain commented 10 years ago

Each widget in z3c.form has an id to define the html of core widget, except ordered selection field.

tdesvenain commented 10 years ago

Hi can you review/merge this pr please ?

mgedmin commented 10 years ago

Looks good to me, but I'm not intimately familiar with z3c.form widget implementations.

Github tells me there are merge conflicts that don't allow this to be merged automatically. Would you like to rebase the pull request?

tdesvenain commented 10 years ago

of course

On Fri, Oct 18, 2013 at 3:22 PM, Marius Gedminas notifications@github.comwrote:

Looks good to me, but I'm not intimately familiar with z3c.form widget implementations.

Github tells me there are merge conflicts that don't allow this to be merged automatically. Would you like to rebase the pull request?

— Reply to this email directly or view it on GitHubhttps://github.com/zopefoundation/z3c.form/pull/14#issuecomment-26594831 .

Thomas Desvenain

Téléphone : 09 51 37 35 18

tdesvenain commented 10 years ago

is it ok now ?

On Fri, Oct 18, 2013 at 3:33 PM, thomas desvenain < thomas.desvenain@gmail.com> wrote:

of course

On Fri, Oct 18, 2013 at 3:22 PM, Marius Gedminas <notifications@github.com

wrote:

Looks good to me, but I'm not intimately familiar with z3c.form widget implementations.

Github tells me there are merge conflicts that don't allow this to be merged automatically. Would you like to rebase the pull request?

— Reply to this email directly or view it on GitHubhttps://github.com/zopefoundation/z3c.form/pull/14#issuecomment-26594831 .

Thomas Desvenain

Téléphone : 09 51 37 35 18

Thomas Desvenain

Téléphone : 09 51 37 35 18

mgedmin commented 10 years ago

Hm, travis-ci.org did not pick up this pull request. Investigating...

mgedmin commented 10 years ago

Not that I have any clue how to investigate this. Oh well, running tests manually.

mgedmin commented 10 years ago

I'm seeing two new test failures with your change. Can you please fix those?

(Good God, why don't these tests default to doctest.REPORT_NDIFF? These failures are unreadable by default.)

tdesvenain commented 10 years ago

This should be ok now

tdesvenain commented 10 years ago

I removed this. Don't have a clue why this appeared... By the way, it was redundant with upper class.

mgedmin commented 10 years ago

Thanks!