Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#3788 closed bug (fixed)

accordion no longer supports dl

Reported by: wichert Owned by:
Priority: critical Milestone: 1.7
Component: ui.accordion Version: 1.6rc4
Keywords: Cc:
Blocked by: Blocking:

Description

In 1.6b it was possible to use a dl list for the accordion if you passed in a headers: "dt" option. This no longer works with rc4.

This is very inconvenient: a dl is the most natural (and semantically correct often) markup style you can use for an accordion and much simpler to write.

Attachments (5)

accordion-animation.patch (2.2 KB) - added by Scott González 11 years ago.
dl.html.patch (2.1 KB) - added by Jörn Zaefferer 11 years ago.
Visual test for dl-accordion
accordion-animation2.patch (2.6 KB) - added by Jörn Zaefferer 11 years ago.
Removed code for groups
accordion-dl-animations.patch (9.0 KB) - added by Jörn Zaefferer 11 years ago.
complete patch with modified demos/accordion/default.html, tests/visual/dl.html and accordion patch for fixing container height for autoHeight accordions
accordion-jump-improvement.patch (3.1 KB) - added by Scott González 11 years ago.
patch to be applied with Jörn's latest patch - improves the jump and removes the container height setting

Download all attachments as: .zip

Change History (15)

comment:1 Changed 11 years ago by Scott González

Milestone: TBD1.6
Priority: majorcritical

comment:2 Changed 11 years ago by Jörn Zaefferer

This is a regression caused by the workaround to fix animations. In lack of a better solution for the jumpy animations we may need to accept this and defer the fix to a later release.

comment:3 Changed 11 years ago by Jörn Zaefferer

Milestone: 1.61.next

comment:4 Changed 11 years ago by Scott González

Milestone: 1.next1.6

We need to try to find a different workaround to remove the regression. We should have a working prototype of how to do this without the extra div by looking at animations in jQuery core, since they animate padding and margin now.

comment:5 Changed 11 years ago by Scott González

Owner: set to scott.gonzalez
Status: newaccepted

Changed 11 years ago by Scott González

Attachment: accordion-animation.patch added

comment:6 Changed 11 years ago by Scott González

I've attached a patch to remove the content wrapper div and animate the padding and margin of the content element. This should make it possible to use dl's again. This will also require the CSS to be updated (available in /tests/static/accordion/default_nowrappers.html). There's a 1px shift during animation that I can't track down. I'm not sure if the destroy method will also need to be updated. The demos may need to be updated as well.

Changed 11 years ago by Jörn Zaefferer

Attachment: dl.html.patch added

Visual test for dl-accordion

Changed 11 years ago by Jörn Zaefferer

Attachment: accordion-animation2.patch added

Removed code for groups

Changed 11 years ago by Jörn Zaefferer

complete patch with modified demos/accordion/default.html, tests/visual/dl.html and accordion patch for fixing container height for autoHeight accordions

Changed 11 years ago by Scott González

patch to be applied with Jörn's latest patch - improves the jump and removes the container height setting

comment:7 Changed 11 years ago by Scott González

The jump improvement patch makes the animation completely smooth in Firefox. There is still a jump in other browsers, and I have to specifically exclude border animations in IE.

comment:8 Changed 11 years ago by Scott González

Owner: scott.gonzalez deleted
Status: acceptedassigned

comment:9 Changed 11 years ago by Scott González

Resolution: fixed
Status: assignedclosed

Fixed in r1878.

comment:10 Changed 10 years ago by etiger13

http://dev.jqueryui.com/changeset/1878/trunk/ui/ui.accordion.js

Why was the ui-accordion-group class functionality removed? Doesn't look like it was/would cause a problem. This is on line 36 and 106. It was in the code for awhile and if anyone created styles based on that update, it breaks. I propose those two lines be added back in since it shouldn't affect anything but leaving them out will affect some sites.

Note: See TracTickets for help on using tickets.