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.

Sign up for GitHub

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

Jump to bottom

feat: add Svg component #1091

Merged
merged 11 commits into from Oct 17, 2022
Merged

feat: add Svg component #1091

merged 11 commits into from Oct 17, 2022

Conversation

RodrigoHamuy
Copy link
Contributor

@RodrigoHamuy RodrigoHamuy commented Oct 11, 2022 •
edited

Why

So is easy to render SVG stuff.

What

Added an Svg r3f component that can render SVG data in three.

Is a wrapper around the svg loader demo.

Storybook PR link

Checklist

  • Documentation updated
  • Storybook entry added
  • Ready to be merged

@vercel
Copy link

vercel bot commented Oct 11, 2022 •
edited

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
drei ✅ Ready (Inspect) Visit Preview Oct 16, 2022 at 3:17PM (UTC)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 11, 2022 •
edited

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 31186b1:

Sandbox Source
white-http-y08efd Configuration
Ground reflections and video textures Configuration
arc-x-pmndrs-colors Configuration

CodyJasonBennett
CodyJasonBennett reviewed Oct 12, 2022
View reviewed changes
src/core/Svg.tsx Outdated Show resolved Hide resolved
joshuaellis
joshuaellis requested changes Oct 13, 2022
View reviewed changes
src/core/Svg.tsx Show resolved Hide resolved
@drcmda
Copy link
Member

drcmda commented Oct 16, 2022

seems usefull, is it done?

@RodrigoHamuy
Copy link
Contributor Author

RodrigoHamuy commented Oct 16, 2022 •
edited

seems usefull, is it done?

@drcmda it is ready to go imho. Not sure if I should resolve the change requests or the change request authors are in charge of deciding that.

The only thing I could say to maybe add if you want is ability to pass the material type (Phong/Standard, etc) to use but I'm not sure how to do that and keep a nice JSX style. I guess if that's wanted we can always do another PR?

The other thing is that not all SVG types load, but that's an issue with three-stdlib not using the latest improvements that you can find in the three example folder. If that's upgraded, it will still require no changes on this component, so is not a blocker.

@joshuaellis joshuaellis merged commit 7b7d486 into pmndrs:master Oct 17, 2022
@joshuaellis
Copy link
Member

The only thing I could say to maybe add if you want is ability to pass the material type (Phong/Standard, etc) to use but I'm not sure how to do that and keep a nice JSX style. I guess if that's wanted we can always do another PR?

lets keep it simple for the interest of tracking, upgrades can be made in other PRs. 👍🏼 great work!

@github-actions
Copy link

🎉 This PR is included in version 9.36.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

@CodyJasonBennett CodyJasonBennett CodyJasonBennett left review comments

@joshuaellis joshuaellis joshuaellis approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@RodrigoHamuy @drcmda @joshuaellis @CodyJasonBennett