Skip to content
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

Document adding SVGs as React components #5147

Merged
merged 2 commits into from
Oct 1, 2018

Conversation

mareksuscak
Copy link
Contributor

@mareksuscak mareksuscak commented Sep 27, 2018

@holloway
Copy link
Contributor

holloway commented Oct 1, 2018

@mareksuscak One thing to document might be how the SVG component can accept props which are applied to the <svg> such as,

 <Logo className="icon" />  or  <Logo {...props} />

These props are essential for my use of SVG components because I have the same SVG with different dimensions and classNames.

Also when Jest snapshots are made of these SVG components in CRA2 it seems to just print their filenames.. there's no <svg>...</svg> tags, just a string of "icon.svg". Is that intentional? @Timer @gaearon

This means that the snapshots don't include these props, which (of course) means the snapshots are missing particular app states.

@gaearon
Copy link
Contributor

gaearon commented Oct 1, 2018

@holloway Thanks for bringing it up. I think we discussed doing something like this and then forgot to fix it. Since 2.0 is not tagged at latest yet, I think it's acceptable to consider this a bugfix before final, and then insta-deprecate versions before the final one. We'll make a PR soon.

@gaearon gaearon merged this pull request into facebook:next Oct 1, 2018
Timer pushed a commit to Timer/create-react-app that referenced this pull request Oct 1, 2018
* Document importing SVGs as React components

* Update README.md
@Timer
Copy link
Contributor

Timer commented Oct 1, 2018

This was merged into next. Fixed via 9f7f2ab.

@gaearon
Copy link
Contributor

gaearon commented Oct 1, 2018

oops thanks

@mareksuscak mareksuscak deleted the update-svg-doc branch October 1, 2018 14:45
@mareksuscak
Copy link
Contributor Author

No worries, I was planning to update this PR later tonight but I'm glad you were able to cherry-pick the changes. Next time around, I'll make sure to allow edits from maintainers!

zmitry pushed a commit to zmitry/create-react-app that referenced this pull request Oct 2, 2018
* Document importing SVGs as React components

* Update README.md
@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants