Update Frame to allow use of React.Fragment #57
Merged
Conversation
This file contains hidden or 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
nkzawa
reviewed
Apr 17, 2019
readme.md
Outdated
|
|
||
| - Use `import { Keyframes } from 'react-keyframes'` or `require('react-keyframes').Keyframes`. | ||
| - The `component` prop specifies what component `Keyframes` renders as. | ||
| - The `component` prop specifies what component `Keyframes` renders as. Passing "Fragment" will render the component inside a `<React.Fragment />`. If no component is passed, the default is a "span" tag. |
There was a problem hiding this comment.
It seems we can pass a React component to React.createElement so that <Keyframes component={React.Fragment}> just work. Maybe we should make it clear on README.
There was a problem hiding this comment.
I wonder if we should support a "Fragment" string too. What do you think?
There was a problem hiding this comment.
Ah! I didn't realize that. I updated the docs.
Also, I don't think we need to? I removed it from my code because doing <Keyframes component={React.Fragment}> seems to be sufficient. What do you think?
nkzawa
approved these changes
May 10, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Intro
Hi there!
First, thank you for creating this 🙏 I was looking for an easy way to animate using React components.
Problem
I wanted to be able to use <React.Fragment /> as the rendered component so that the components inside of
Framecould be rendered without a wrapper essentially.My hope is to use this to animate parts of an SVG component in React.
Solution
Inside
frame.jscheck ifcomponent === 'Fragment', if so, render inside<React.Fragment />I tested this in a Codesandbox and it seems to be working: https://codesandbox.io/s/5309430n5x
Notes
This is my first OSS PR that is not related to docs so please be kind in any feedback you have 😄 Happy to make changes based on any feedback! Not sure if this is the best way to approach the problem but thought I'd submit the PR and listen to any feedback you have.
PS...I didn't write any tests but we probably should. I wasn't sure exactly what to test. If you have advice there, also open to it.