Popover animation flexibility #2367
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| const { | ||
| let { | ||
| animation, | ||
| style, |
There was a problem hiding this comment.
I think that we should add children and ...other.
|
|
||
| return ( | ||
| <Paper | ||
| style={this.mergeAndPrefix(styles.base, style, openStyles.base)} |
There was a problem hiding this comment.
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 👍 |
chrismcv
force-pushed
the
pop-anim
branch
2 times, most recently
from
cfeb50d
to
5f73c33
Compare
| import DefaultRawTheme from '../styles/raw-themes/light-raw-theme'; | ||
| import ThemeManager from '../styles/theme-manager'; | ||
| import Extend from '../utils/extend'; | ||
| import PopoverAnimation from './popover-default-animation'; |
There was a problem hiding this comment.
Name doesn't match here too.
oliviertassinari
added a commit
that referenced
this pull request
Dec 4, 2015
[Popover] Add animation flexibility
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.