Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Update Frame to allow use of React.Fragment #57

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

Merged
nkzawa merged 5 commits into vercel:master from jsjoeio:master
May 10, 2019
Merged

Update Frame to allow use of React.Fragment #57

nkzawa merged 5 commits into vercel:master from jsjoeio:master
May 10, 2019

Conversation

@jsjoeio
Copy link
Contributor

@jsjoeio jsjoeio commented Mar 17, 2019
edited
Loading

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 Frame could be rendered without a wrapper essentially.

My hope is to use this to animate parts of an SVG component in React.

Solution

Inside frame.js check if component === '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.

biw reacted with thumbs up emoji
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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should support a "Fragment" string too. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 nkzawa merged commit bc9625b into vercel:master May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@nkzawa nkzawa nkzawa approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /