Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Setting collapse panel height to 0 disables animation #99

Open
neberhardt123 opened this issue Oct 25, 2021 · 4 comments · Fixed by #100 · May be fixed by #97
Open

Setting collapse panel height to 0 disables animation #99

neberhardt123 opened this issue Oct 25, 2021 · 4 comments · Fixed by #100 · May be fixed by #97
Assignees
@majorimi
Labels
enhancement New feature or request question Further information is requested

Comments

@neberhardt123
Copy link

When I set ContentHeight for a CollapsePanel component to 0, it disables the open and close animation for it. Is there a way to retain the animations when I set the height to auto?
@majorimi
Copy link
Owner

@neberhardt123 yes this is "known limitation". This was expressed in the Demo app. Also XML comments explains it. (see both below)

The problem is CSS not able to calculate the transition speed if Height is not a fixed value. Or something like that, however I'm not a CSS expert. I can try to look into it, but as mentioned I'm not a CSS guru... If you have any "simple" trick to solve this limitation that would be appreciated. Thanks.

image

/// <summary>
/// Determines to apply CSS animation and transition on Collapse state changes or not.
/// Note: in case of `auto` height some animation won't work.
/// </summary>
[Parameter] public bool Animate { get; set; } = true;

@majorimi majorimi self-assigned this Oct 25, 2021
@majorimi majorimi added enhancement New feature or request question Further information is requested labels Oct 25, 2021
@majorimi
Copy link
Owner

majorimi commented Oct 28, 2021 •
edited
Loading

@neberhardt123 Looked into this CSS issue and it seems there are few solutions. Unfortunately some of them does not trigger a DOM reorder (re-render) so leaves empty space on UI which is not good. So basically 1 possibility exists....

Instead of height another property max-height has to be animated. One big problem is max-height must be gather than auto calculated value. To make it work for everyone I had to introduce MaxAllowedContentHeight property which must be set be panel for your content and value is "best guess". No guaranty of auto value won't be gather. Also if you give it a huge number then animation will be super quick (time is fixed size is growing). So even animation speed won't be fixed...

But updated CollapsePanel animation when height is Auto CSS cannot calculate transition speeded. So need to animate max-height but it requires grater value then "auto" value... You can pull down branch: fix/collapse-animation and try it out locally. Test app now has this:

image

If you think it is acceptable, then I will merge PR and can release the change with v1.6.0. Let me know...

@majorimi majorimi linked a pull request Oct 28, 2021 that will close this issue
Updated CollapsePanel animation when heigh is Auto CSS cannot calcula… #100
Merged
@neberhardt123
Copy link
Author

neberhardt123 commented Oct 29, 2021 via email

@majorimi majorimi linked a pull request Oct 29, 2021 that will close this issue
Release/v1.6.0 #97
Open
@majorimi
Copy link
Owner

Yes that is a known problem of animating max-height property. When the Max value is much largen then the auto calculated value then speed of animation will be "calculate" based on that large px value. And since only auto size and its content is visible only on UI. But animation speed is for a larger value so it is collapsing "non-visible area" first. So it seems doing nothing (lagging) and then the smaller visible area will collapse quickly... For opening it is similar just reversed.

This issue is mentioned in that article. Also added XML comment to explain this. It can be mitigated by "guessing" the max-height value as close as possible to the auto calculated value... (Might not that easy to do)

Ok merged this change to RC 1.6.0. I will release new packaged in the next few weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@majorimi majorimi

Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Milestone
No milestone
2 participants
@majorimi @neberhardt123