-
-
Notifications
You must be signed in to change notification settings - Fork 79
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
Refactor App Routing & Navigation API's #785
Conversation
eb8949d
to
228d1d9
Compare
…ough a Router to improve how deep link URLs are handled
…t at the app root instead
On the messages tab I am not seeing the messaging pane updating properly in catalyst when selecting a contact for a DM, otherwise everything seems to match what is working currently, it is working on a phone. |
fixed in latest |
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.
All of the nav and notifications seem to work as good as they did before.
What Changed
This change is a substantial refactor that changes how app routing and navigation state is handled across the app. I've introduced a new type,
Router
, that is the single source of truth for where the user is in the app. Navigation APIs in Messages, Nodes, and Settings were refactored to utilize the newRouter
type.How is this tested?
I've added a unit test target to the app that validates several deep link URL's map to the appropriate routes, and done manual validation across two nodes & two client devices to send notifications back & forth.
Next Steps?
We still need to implement routing on the Map views to handle Waypoint notifications. We could also improve the map focus for map routing, but these can be done in a follow-up change.