Opened 11 years ago

Closed 9 years ago

Last modified 9 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 9 years ago.
test_AccordionContainerPadding.html (16.8 KB) - added by jfcunat 9 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 11 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 11 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 11 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 9 years ago by jfcunat

Changed 9 years ago by jfcunat

comment:4 Changed 9 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 9 years ago by bill

Milestone: 1.21.5

comment:6 Changed 9 years ago by jfcunat

proposed patch would solve #9141 as well

comment:7 Changed 9 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 9 years ago by bill

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

comment:9 Changed 9 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 9 years ago by bill

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

comment:11 Changed 9 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 9 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.