-
Notifications
You must be signed in to change notification settings - Fork 734
Fix/dialog animation #891
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/dialog animation #891
Conversation
I believe we had a bug with that, I cannot remember ATM what it was (I think it was on iOS), I hope our e2e will find it (or it doesn't happen :) ). Edit: this was before I saw the PR, I'm guessing that not using the Modal's fade can solve that. |
if (!dialogVisibility) { | ||
animateFading(0); | ||
} | ||
}, [dialogVisibility]); |
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.
The hooks plugin says you are missing animateFading
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.
fixed the warnings..
But I gotta say, the plugin is not that smart.
It just blindly check what's being used and ask to add it as a dependency, which sometimes is not really necessary.
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.
I know, and it has bugs sometimes (see AutoLockScrollView, it cannot recognize that not all the props
need to be checked), but it did find these which are bugs, I think, so it's worth using IMHO.
WOAUILIB-972
The purpose of this fix is to improve the dialog entrance/exist animation.
On the way I fixed small stuff I found
PanResponderView
I'm now usingView
's internalanimation
prop instead of the Container ternary.DialogDismissibleView
I removed unnecessary state update (isAnimating
doesn't affect render)In the
Dialog
componentanimationType
to "none" so we can have better control over the background fading animationOverlayFadingBackground
, with a single purpose - the handle the background fading transition. (Technically, now the background fade animation and the dialog entrance/exit animation happen together.)