-
Notifications
You must be signed in to change notification settings - Fork 44
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
AnimationStackView onNavigation should return a promise for animation completion #9
Comments
Also, fwiw, if we move all the current async apis to be promised based (as we're discussing here: facebook/react-native#4971), it would fit in nicely. :) 👍 |
I'm concerned about baking this into But I totally understand the application's need for a promise to know when the nav animation is complete. My best idea is to bake it in to the action:
How does this look? This way, developers will have more flexibility because we can mix and match our own actions. |
looks good to me 👍 |
Actually, after talking with @gaearon about it, I think it does make sense for |
onNavigation() could return a promise that would allow to (optionally) track the completion of the navigation action.
For instance, if a button do a navigation push, you might want to make it display differently and disable it until the navigation is completed.
I don't think it is the responsability to the button to "check if navigation is happening and disable it itself" but if the button onPress methods allow a function that (potentially) returns a promise, we could just do:
<Button onPress={() => onNavigation(new Navigation.Action.Push('route'))} />
It's great to have a fine level of control over navigation action: on a single navigation action, user could know when the navigation has successfully moved to next screen or if it was aborted (by making the promise failing). Also the fact it's a promise make a great API to the user I think, because people might just ignore this detail and don't use it, but when you need it you can do a
then()
on the result.(for context, I asked the same question on the old navigation api: facebook/react-native#4824 )
The text was updated successfully, but these errors were encountered: