-
Notifications
You must be signed in to change notification settings - Fork 132
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.
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
Fix panel transition abruptly snapping to the end #2200
Fix panel transition abruptly snapping to the end #2200
Conversation
…nelTransitionSnap
…nelTransitionSnap
Thanks for the PR @yucheng11122017! While the changes made seems to have solved the original issue, it seems introduces a new problem - the panels are snapping back when the panel body is minimized: MarkBind.-.User.Guide_.Presentational.Components.-.Google.Chrome.2023-03-15.13-34-32.mp4This is different from the original behavior where the closing transition would be smooth. Might want to look into this! As a side note, I've realized that the original bug of the abrupt snapping does not occur on FireFox Ver. 110.0.1 (on Windows 10), but does occur on Google Chrome Ver. 111.0.5563.65 (also on Windows 10), while the new issue of snapping closing transition occurs on both. (I don't know if this is useful information, just something I noticed!) |
Oh I see, I didn't realize this was now the intended behavior (though to be honest, having the closing transition seems more natural). Regardless, I suppose that means all's working as intended 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Tested on Chrome, Firefox and Safari. Interestingly, I didn't really notice the original issue on Firefox and Safari. Good job!
…nelTransitionSnap
if (Number.isNaN(bottomSwitchBottomMargin)) { | ||
return this.$refs.panel.scrollHeight; | ||
} | ||
return this.$refs.panel.scrollHeight + bottomSwitchBottomMargin; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would creating a <div>
wrapper with bottom padding instead work?
What is the purpose of this pull request?
Fixes #2185.
Current problem:
Overview of changes:
Added collapse button's bottom margin to the panel's maxHeight when panel is transitioning.
Anything you'd like to highlight/discuss:
An alternative I thought of would be to scale up the maxHeight (eg. the maxHeight = 1.5 * scrollHeight)
Pros would be that if other components that are added in the future to the panels also have a bottom margin, there would be a need to keep including those elements in the calculation
Cons are the transition might not be as consistent since the ratio of the actual height (including the bottom margin) to calculated maxHeight would defer. And if a really large bottom margin is used, the problem would occur again
Testing instructions:
Proposed commit message: (wrap lines at 72 characters)
Fix panel transition abruptly snapping to the end
Checklist: ☑️