-
-
Notifications
You must be signed in to change notification settings - Fork 32.7k
Popover animation flexibility #2367
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
Conversation
src/popover/popover.jsx
Outdated
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 think that we should add children and ...other.
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.
done
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.
Could you use prepareStyles instead?
mergeAndPrefix is adding the prefixes, however Paper is already doing it. No need to do it twice. Plus, it's deprecated, we should update our code base to use prepareStyles, that is also handling the RTL with the prefixes.
|
@chrismcv That's my last feedback, nice work 👍 |
cfeb50d to
5f73c33
Compare
src/popover/popover.jsx
Outdated
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.
Name doesn't match here too.
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.
done
[Popover] Add animation flexibility
This PR allows popover to be passed an animation component that is responsible for animating its open and close.
@oliviertassinari I tried react-motion, but struggled to get it to work as I wanted, so I've just extracted the animation css parts.
The other weirdness is that the outer element needs to provide the border zDepth because the box-shadow on paper is effectively an overflow.