Search and Top Navigation
#4695 closed bug (fixed)
Opened July 18, 2009 05:07PM UTC
Closed July 12, 2010 02:51PM UTC
Accordion issue (and fix) with fillSpace:true
Reported by: | jzaefferer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | 1.8.2 |
Component: | ui.accordion | Version: | 1.7.2 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
There is a logical bug in accordion's .resize() method, when fillSpace is true. It calculates the maximum padding of all content panels, and then subtracts it from maxHeight to obtain the new panel size. This is wrong. I, for example, need to use different paddings in different content divs, and this approach produces wrong results in this case. The correct thing to do is to subtract each panel's padding individually. I was able to fix this (in 1.7.2) by replacing this: var maxPadding = 0; this.headers.next().each(function() { maxPadding = Math.max(maxPadding, $(this).innerHeight() - $ (this).height()); }).height(Math.max(0, maxHeight - maxPadding)) with this: this.headers.next().each(function() { var padding = $(this).innerHeight() - $(this).height(); $(this).height(Math.max(0, maxHeight - padding)); }) Please integrate this fix in the dev version, and in 1.7.3.
Attachments (0)
Change History (5)
Changed July 18, 2009 05:08PM UTC by comment:1
description: | {{{There is a logical bug in accordion's .resize() method, when fillSpace \ is true. It calculates the maximum padding of all content panels, and \ then subtracts it from maxHeight to obtain the new panel size. This is \ wrong. I, for example, need to use different paddings in different \ content divs, and this approach produces wrong results in this case. \ The correct thing to do is to subtract each panel's padding \ individually. \ \ I was able to fix this (in 1.7.2) by replacing this: \ \ var maxPadding = 0; \ this.headers.next().each(function() { \ maxPadding = Math.max(maxPadding, $(this).innerHeight() - $ \ (this).height()); \ }).height(Math.max(0, maxHeight - maxPadding)) \ \ with this: \ \ this.headers.next().each(function() { \ var padding = $(this).innerHeight() - $(this).height(); \ $(this).height(Math.max(0, maxHeight - padding)); \ }) \ \ Please integrate this fix in the dev version, and in 1.7.3.}}} → {{{ \ There is a logical bug in accordion's .resize() method, when fillSpace \ is true. It calculates the maximum padding of all content panels, and \ then subtracts it from maxHeight to obtain the new panel size. This is \ wrong. I, for example, need to use different paddings in different \ content divs, and this approach produces wrong results in this case. \ The correct thing to do is to subtract each panel's padding \ individually. \ \ I was able to fix this (in 1.7.2) by replacing this: \ \ var maxPadding = 0; \ this.headers.next().each(function() { \ maxPadding = Math.max(maxPadding, $(this).innerHeight() - $ \ (this).height()); \ }).height(Math.max(0, maxHeight - maxPadding)) \ \ with this: \ \ this.headers.next().each(function() { \ var padding = $(this).innerHeight() - $(this).height(); \ $(this).height(Math.max(0, maxHeight - padding)); \ }) \ \ Please integrate this fix in the dev version, and in 1.7.3. \ }}} |
---|
Changed August 11, 2009 06:58PM UTC by comment:2
resolution: | → fixed |
---|---|
status: | new → closed |
Fixed in r3047.
Changed August 11, 2009 07:14PM UTC by comment:3
resolution: | fixed |
---|---|
status: | closed → reopened |
Animations still jumpy for fillSpace.
Changed August 11, 2009 07:26PM UTC by comment:4
The visual testcase passes when replacing the em-padding with px. Animating an element to 15% of 3em doesn't work that well.
Changed July 12, 2010 02:51PM UTC by comment:5
milestone: | 1.9 → 1.8.2 |
---|---|
resolution: | → fixed |
status: | reopened → closed |
Closing as this is working with pixels. Handling ems is a separate issue that we already know about.