-
Notifications
You must be signed in to change notification settings - Fork 11
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
Examples and fix for #25 #28
base: master
Are you sure you want to change the base?
Conversation
Hey @she-who-codes, can you fix up the conflicts? |
Might be good to remove the example's build folder too, lots of huge files in there that just get built when you run the example anyway! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we just need to validate that the incoming property changes are the ones that get applied. See my comment on FlareComponent.js!
source/FlareComponent.js
Outdated
{ | ||
this.load(nextProps.file); | ||
this.load(prevProps.file); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this will attempt to load the previously set file. I think we want to load the incoming change (what used to be nextProps, which in this case is the this.props). Similarly below it'll try to initialze to the previous artboardName and controller.
We might want to make a test (maybe a button in your example) that changes the current file and make sure it's changing to the expected value.
You could use a second .flr file that has a cactus that's a different color or something. I'm worried that the code as it is will not change to the new file.
This is @she-who-codes's fix for the deprecated componentWillReceiveProps change from React 16.