-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fully disable animation style if enableAnimations is false. #16893
Fully disable animation style if enableAnimations is false. #16893
Conversation
let animationStyle = useMovingAnimation( wrapper, isSelected || isPartOfMultiSelection, enableAnimation, animateOnChange ); | ||
|
||
// Dismiss animation style if animations are disabled. | ||
animationStyle = enableAnimation ? animationStyle : {}; |
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.
Why don't we do that in the hook itself?
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.
Moved within 029c0a8.
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 👍 Thanks for the update.
Description
useMovingAnimation
acceptsenableAnimation
as param, however, the animation styling is applied eventually to the block wrapper even ifenableAnimation
is set tofalse
. WithenableAnimation
set tofalse
,transform
is set toundefined
, alsotransformOrigin
is always set.This can potentially override existing
transform
coming fromwrapperProps.style
via a filter for example.How has this been tested?
Functional testing making sure that the block animations remain as they are by default.
Also tested that when setting
enableAnimations
tofalse
the animation related styles are not applied.Checklist: