#3788 closed bug (fixed)

Opened January 07, 2009 06:39PM UTC

Closed January 30, 2009 02:35AM UTC

Last modified February 10, 2010 06:51AM UTC

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 scottgonzalez January 29, 2009 04:20AM UTC.
  • accordion-animation2.patch (2.6 KB) - added by jzaefferer January 29, 2009 08:26PM UTC.

    Removed code for groups

  • accordion-dl-animations.patch (9.0 KB) - added by jzaefferer January 29, 2009 08:54PM UTC.

    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 scottgonzalez January 29, 2009 09:58PM UTC.

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

  • dl.html.patch (2.1 KB) - added by jzaefferer January 29, 2009 08:26PM UTC.

    Visual test for dl-accordion

Change History (10)

Changed January 13, 2009 01:21AM UTC by scottgonzalez comment:1

milestone: TBD → 1.6
priority: major → critical

Changed January 19, 2009 05:10PM UTC by jzaefferer comment:2

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.

Changed January 21, 2009 01:38PM UTC by jzaefferer comment:3

milestone: 1.6 → 1.next

Changed January 21, 2009 07:54PM UTC by scottgonzalez comment:4

milestone: 1.next → 1.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.

Changed January 29, 2009 03:44AM UTC by scottgonzalez comment:5

owner: → scott.gonzalez
status: new → accepted

Changed January 29, 2009 04:25AM UTC by scottgonzalez comment:6

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 January 29, 2009 09:59PM UTC by scottgonzalez comment:7

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.

Changed January 29, 2009 10:00PM UTC by scottgonzalez comment:8

owner: scott.gonzalez
status: accepted → assigned

Changed January 30, 2009 02:35AM UTC by scottgonzalez comment:9

resolution: → fixed
status: assigned → closed

Fixed in r1878.

Changed November 11, 2009 11:03PM UTC by etiger13 comment:10

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.