[Collapsible] animate collapsible max-height property

[artygus]

WHY are these changes introduced?

Fixes #1133

I believe the issue is caused by collapsing margins. Height of element with overflow: hidden includes margins of children, while margins of children of overflow: visible element spills through the parent and are not included in parent's height.

WHAT is this pull request doing?

• changes collapsible animation property from height to max-height

[ghost]

👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines.

[artygus]

another solution might be adding same overflow properties to height container

[danrosenthal]

Hey @artygus, thanks for the contribution. Would you be able to add some tests for this change?

[artygus]

@danrosenthal sure, it it'll help to get it merged

[danrosenthal]

sure, it it'll help to get it merged

It'll definitely be a good start! And sorry for the delay on getting you review.

[danrosenthal]

@ry5n, since you created the issue this addresses, would you be able to give this a tophat?

You can tophat it locally by running:

$ git fetch origin pull/1980/head:tophat-1980 && git checkout tophat-1980

[danrosenthal]

My 🎩 looks pretty good. Frame-rate is mostly around 60fps, and the jank appears to be significantly reduced. This'll likely not be perfectly smooth until we reimplement it using a CSS property that creates its own layer and uses hardware acceleration (a property like transform).

[ry5n]

I may not be able to take a look today but I’ll do my best. If you feel satisfied this is better please go ahead and merge.

[sarahill]

This looks good to me. Much smoother! Definitely an improvement. @danrosenthal or @kaelig can you give this another once over so we can get this in?

[artygus]

I'm stuck with tests, couldn't find a good example on how to test animations. Any references to good examples would certainly help 🙂

[danrosenthal]

Looking deeper at the changes, I think this one is going to be tricky (impossible) to test. Go ahead and rebase your PR to fix the UNRELEASED conflict, and @sarahill or someone else on the team can merge it for you.

[artygus]

cool, rebased!

[ghost]

🎉 Thanks for your contribution to Polaris React!