firtoz / react-three-renderer

Render into a three.js canvas using React.
https://toxicfork.github.com/react-three-renderer-example/
MIT License
1.49k stars 155 forks source link

🌁 Add FogExp2 prop support to scene #144

Closed epilande closed 7 years ago

epilande commented 7 years ago

Fix console error when using FogExp2 within scene. Closes #143.

epilande commented 7 years ago

Not exactly sure how to fix the breaking test..

Since i'm using PropTypes.oneOfType, it shows the react error instead of the custom one from propTypeInstanceOf 😕

image

toxicFork commented 7 years ago

Thanks for the PR :) I'll look at what's up with the test :)

toxicFork commented 7 years ago

Since i'm using PropTypes.oneOfType, it shows the react error instead of the custom one from propTypeInstanceOf 😕

Yes, that's slightly unfortunate but I'm hoping the users can look at documentation etc to find out what's the right value to pass (hopefully? :D)

In worst case we can wrap oneOfType like instanceOf but that's a dangerous path in these time-constrained lands

epilande commented 7 years ago

What would you suggest? We update the tests or wrap oneOfType like instanceOf?

BTW awesome lib. thanks!

toxicFork commented 7 years ago

I recommend to update the test to match the message, for now :)

And thank you for contributing :D

On Fri, 27 Jan 2017, 00:44 Emmanuel Pilande, notifications@github.com wrote:

What would you suggest? We update the tests or wrap oneOfType like instanceOf?

BTW awesome lib. thanks!

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/toxicFork/react-three-renderer/pull/144#issuecomment-275559389, or mute the thread https://github.com/notifications/unsubscribe-auth/AA0iLTl2YEneNUNoomrSMeJ0XAgBOJ8Sks5rWT3egaJpZM4LsFDH .