Opened 14 years ago

Closed 12 years ago

Last modified 12 years ago

#7389 closed defect (fixed)

dijit.layout.AccordionContainer: doesn't account for padding in child panes

Reported by: Nathan Toone Owned by: bill
Priority: high Milestone: 1.6
Component: Dijit Version: 1.1.1
Keywords: Cc:
Blocked By: Blocking:


If you set padding on an AccordionPane?'s content node, the AccordionContainer? doesn't resize it correctly.

It should use dojo.marginBox instead of style.height.

Attachments (2)

patch_AccordionContainer.txt (3.1 KB) - added by jfcunat 13 years ago.
test_AccordionContainerPadding.html (16.8 KB) - added by jfcunat 13 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 14 years ago by Nathan Toone

Resolution: fixed
Status: newclosed

(In [14735]) Fixes #7389 - use dojo.marginBox instead of style.height for sizing the accordion panes

comment:2 Changed 14 years ago by Nathan Toone

Resolution: fixed
Status: closedreopened

Can't just use dmb since we use animateProperty to show/hide the accordion. Will resubmit with test case and more robust fix.

comment:3 Changed 14 years ago by Nathan Toone

Resolution: fixed
Status: reopenedclosed

(In [14736]) Fixes #7389 - add in a padding accordionpane test case and make it work.

Changed 13 years ago by jfcunat

Changed 13 years ago by jfcunat

comment:4 Changed 13 years ago by jfcunat

Resolution: fixed
Status: closedreopened

It seems that padding inside ContentPane? children of AccordionContainer? causes bad animation. Look at the the last accordion button : it is moved down during the animation. Bigger is the padding inside contentpane bigger is the delta...

See attached test page (long duration is used to better show the problem). extremePadding css class is used in the second case to illustrate the problem. It is a modified version of test_AccordionContainer.html where extremePadding class was not well defined (old dijitAccordionBody not existing anymore) and with duration=2000 to better see the problem

Proposed attached patch : animation on height but also on paddingTop and paddingBottom for oldWidget and newWidget. Padding is animated from original value to 0 for oldWidget and from 0 to original value for newWidget.

After applying patch, animation is better on FF3, safari and IE (a little flickering on IE is noticeable though)

comment:5 Changed 13 years ago by bill

Milestone: 1.21.5

comment:6 Changed 13 years ago by jfcunat

proposed patch would solve #9141 as well

comment:7 Changed 12 years ago by Nathan Toone

Milestone: 1.5future
Owner: Nathan Toone deleted

Moving my tickets to future, as I am not currently working on them.

comment:8 Changed 12 years ago by bill

Milestone: future1.6
Owner: set to bill
Status: reopenednew

comment:9 Changed 12 years ago by bill

Resolution: fixed
Status: newclosed

(In [22677]) Fix AccordionContainer rendering problems, by properly accounting for margin/border/padding and by dealing w/the fact that during the animation, on claro, need to account for two blue borders wrapping around the two "active" children. See comments in code for more details.

This fixes the problem where the animation doesn't work well when there's lots of padding on the child ContentPane, and also the problem where the blue trim underneath a child ContentPane (claro only) would disappear during the animation.

Also, added correct handling for race condition when another pane is selected or the accordion is destroyed while an animation is in progress.

IE6 and IE7 have overflow problems with this new design (the overflow: hidden isn't taking effect), but rather than fix the problem I just disabled the animation for those browsers.

Fixes #4017, #7389, #9141 !strict.

Also removed some workaround code for #11415, and from [21426] (#10527), that seems to no longer be needed.

comment:10 Changed 12 years ago by bill

(In [22695]) Update tests to match code changes in [22677], refs #7389.

comment:11 Changed 12 years ago by bill

(In [22701]) Don't leave height: 1px setting on <div> after animation because it will cause problems when said pane is shown w/out the animation. Still need to check in test case: on accordion with one pane, add new pane, select new, and then destroy new pane, causing accordion to return to first pane sans animation.) Refs #7389 !strict.

comment:12 Changed 12 years ago by bill

(In [22702]) Tests that AccordionContainer animation doesn't leave residual height settings that interfere in the future, when other panes are selected [without] animation. Refs #7389 !strict.

Note: See TracTickets for help on using tickets.