Accordion: fixes #9264

[JacquesPerrault]

Using .show method and outerHeight property to force height calculation when
heightStyle='auto'

[scottgonzalez]

Using .outerHeight() here makes the panels too large since we're setting the value via .height(). The two methods have to match; since both methods account for box-sizing, there shouldn't be a need to change.

You can see the problem in the default demo; there's extra spacing below the content of panel 3 (the tallest content). Probably the easiest way to see the difference is to just visually compare it with http://view.jqueryui.com/master/demos/accordion/default.html.

[scottgonzalez]

We should make sure that there's a unit test that fails prior to the fix and passes after the fix. We already have unit tests for heightStyle, but perhaps we need to limit the width in the test fixture so the bug occurs.

[JacquesPerrault]

@scottgonzalez thanks, good points. I'll work on a test case and bear this in mind going forward. Should I close this for now?

[scottgonzalez]

You can leave it open and just push more commits to the branch. Just add a comment when you're done because there's no notification when new commits are added.

[JacquesPerrault]

Still working on the test case, and box-sizing "snap" issue,

[JacquesPerrault]

This addresses the "snap" issue referenced in the ticket and works for all valid values of heightStyle. However, I haven't been able to generate an automated test case, since
fx.now = Math.round( total - toHide.outerHeight() - adjust);
seems to be calculating correctly. That would seem to indicate the problem is with the tween processing but I haven't been able to lock down precisely where - I can only confirm that omitting the tween processing solves the problem.

[scottgonzalez]

If we're excluding two options, we should just explicitly check for the one case we do want to handle. However, I have a feeling this causes jittery animations by excluding "auto".

[scottgonzalez]

I've confirmed that this breaks the "auto" animations.

[scottgonzalez]

@JacquesPerrault How's this going? Do you need some help?

[JacquesPerrault]

Scott, thanks for checking in. Some help would be appreciated, I've been hesitant to bug you in IRC about it. You said that you "confirmed that this breaks the 'auto' animations", but I have been unable to reproduce that breakage using either the bug's jsfiddle example or by updating the "default" accordion demo. I've been testing on PC using chrome 36, firefox 31, and IE 10.

[scottgonzalez]

I'll add this to my todo list. To see the broken animations, just open the default demo (unchanged) and look at the description text or the Section 4 header as you toggle sections 1, 2, and 3. They shouldn't move at all, but they're bouncing around because of subpixel errors in the animation.

[JacquesPerrault]

The irregular animation occurs when the element has a style of box-sizing: 'border-box'.
I'd bet this is because padding and border values negate adjust. Adding a condition such as } else if ( that.options.heightStyle == "auto" && toHide.css( "box-sizing" ) == "border-box") {...} and removing the adjust parameter seems to do the trick.

[scottgonzalez]

This should only be checked once and cached. See

jquery-ui/ui/effect.js

Lines 641 to 648 in 8eca7b8

	jQuery.fx.step[ hook ] = function( fx ) {
	if ( !fx.colorInit ) {
	fx.start = color( fx.elem, hook );
	fx.end = color( fx.end );
	fx.colorInit = true;
	}
	jQuery.cssHooks[ hook ].set( fx.elem, fx.start.transition( fx.end, fx.pos ) );
	};

for an example.

[scottgonzalez]

I'd bet this is because padding and border values negate adjust.

Correct; this is what I said in the meeting last month. You still need to handle padding-box as well.

[mikesherov]

Anyone using padding-box deserves to be punished.

[scottgonzalez]

border-box and padding-box need different adjustments.

[JacquesPerrault]

I separated padding-box, but only Firefox recognizes it, and it still causes text below the accordion to jump. I know that I need to exclude padding, but I can't seem to get it right.

[scottgonzalez]

I think I might be on board with @mikesherov now; let's just handle border-box and content-box.

[scottgonzalez]

@JacquesPerrault Are you still working on this?

[JacquesPerrault]

There's a race condition that I was trying to address with my last commit, but it didn't do the trick. The whole point is to force the recompute of outerHeight so that the section collapses smoothly.