Removed Unsafe functions and Legacy Context #169
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It meant adding crumb to state instead of to instance otherwise can't access in static function. I think that's the idea, put things in state not on instance. Manually edited enzyme adapter so that consumer and provider types recognized (jquense/enzyme@b3a6eb1). Updated tests to use NavigationHandler instead of context
Not needed now don't ever listen to navigation changes
Was only there so it could work with all version of React
When react typings updated this should be automatic from generic state type
The prevState didn't get rest = true even after set to true in clearScene. Worked around by only updating state if scene's changed. Weirdly, it seems to get rest = true after this change!?
This had me stumped for ages. I'm pretty sure React will have to add a non-static prop update function before componentDidUpdate for raf animation. Moving it into didUpdate meant had to rerun the getItems to check if it needs the raf added?!
This reverts commit c8fdb55.
I had so much trouble with this becuase of a bug with the new context api. With this change the raf runs forever, even after the animation completes. Thought that meant couldn't use this approach but really just a bug with context api. See facebook/react#12335. Need to retest this when next alpha comes out and check the raf stops running
Means that Links still render outside of a Provider/NavigationHandler. Can test a component without provider context, links will have null href's. Caused a lot of problems for the test setup because first time needed navigation from navigation-react (Link utility references it for type checking but these disappear once compiled). Could've built the navigation package and stuck it in node_modules first but this would've slowed down the tests a lot. Instead used paths ts config option to point at physical file
Resolved modules as local source files in typescript config
The Context needs the StateNavigator
Allows components to render outside of NavigationMotion
When navigating back from zoom details to list, the shared element motion tick now fires before the navigation motion tick. That's because the shared el tick is registered in componentDidMount but the navigation tick is registered in componentDidUpdate (they used to both be registered in componentWillMount). So when the shared el tick fires the progress is still set to 1 but it should be 0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ready for React 17 when unsafe componentWill functions and legacy context will be dropped (more painful than it should've been because React alphas were buggy)