added `category='structural'` to SequenceWidget #233

Merged
merged 1 commit into from Dec 13, 2016

Projects

None yet

3 participants

@tisdall
Contributor
tisdall commented Jul 25, 2014

According to the docstring on Widget:

However, if the default
mapping widget encounters a child widget with the category of
structural during rendering (the default mapping and
sequence widgets are in this category), it omits the title.

The default for a SequenceWidget should be "structural", but
it's missing.

This deals with issue #232

@tisdall tisdall added `category='structural'` to SequenceWidget
According to the docstring on Widget:
However, if the default
mapping widget encounters a child widget with the category of
``structural`` during rendering (the default mapping and
sequence widgets are in this category), it omits the title.

The default for a SequenceWidget should be "structural", but
it's missing.
91331d2
@tisdall
Contributor
tisdall commented Jul 28, 2014

The change to SequenceWidget was made in dadddbb and is no longer valid since we're using bootstrap panels to title the sequences.

@tisdall
Contributor
tisdall commented Dec 12, 2014

@mcdonc - can I get your blessing on this one since this is essentially reverting your change in dadddbb ?

@tisdall
Contributor
tisdall commented Dec 12, 2016

@miohtama - Can you please review this? This seems like a pretty straightforward issue.

@miohtama miohtama merged commit 0784612 into Pylons:master Dec 13, 2016
@miohtama
Contributor

Merged. Can you also update CHANGELOG?

@tseaver
Member
tseaver commented Dec 16, 2016

@tisdall, @miohtama This PR seems to have broken the functional tests (they were passing on master before its merge).

@miohtama miohtama added a commit that referenced this pull request Dec 16, 2016
@miohtama miohtama Revert "Merge pull request #233 from tisdall/structural_sequence"
This reverts commit 0784612, reversing
changes made to 8f24b30.
717df75
@miohtama
Contributor

Looks like this PR precedes the time before Deform had proper functional test infrastructure in place. @tisdall can you open a new PR and make sure all functional tests pass?

@tisdall
Contributor
tisdall commented Dec 16, 2016

I'll see if I can find the time. However, it may be quicker for someone who's already familiar with the functional tests to modify it to match the changes. Essentially the three failed tests are looking for the duplicate titles that are now removed (If I'm reading the messages in Travis correctly).

@miohtama
Contributor

Added a sprintable issue on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment