Skip to content

Conversation

@chrismcv
Copy link
Contributor

@chrismcv chrismcv commented Dec 4, 2015

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.

Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Member

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.

@oliviertassinari
Copy link
Member

@chrismcv That's my last feedback, nice work 👍

@chrismcv chrismcv force-pushed the pop-anim branch 2 times, most recently from cfeb50d to 5f73c33 Compare December 4, 2015 23:32
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

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

Labels

component: Popover The React component.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants