-
Notifications
You must be signed in to change notification settings - Fork 739
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
Adds up navigation in spaces #6073
Conversation
Unit Test Results124 files + 2 124 suites +2 2m 14s ⏱️ -6s Results for commit 90ab67e. ± Comparison against base commit 3674ae7. This pull request removes 1 and adds 16 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
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.
Code seems good to me, my first impression was that the new navigation seemed a bit weird to me, I didn't realize you were going up into the hierarchy of the spaces by opening the drawer. But in real usage, we would see more rooms in the rooms list by navigating up, so I guess it makes sense
Failed checkk seems unrelated |
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.
LGTM
Matrix SDKIntegration Tests Results:
|
Type of change
Content
Being in a space or subspace now allows to click an up button on the toolbar or the device back button to navigate up through the spaces
Motivation and context
Closes #3958
I added a new vector icon for the back button instead of using the default inbuilt toolbar one as given we're using a custom toolbar and it very dynamically changes based on space state it's easier to manage it this way. On the topic of using the existing icon, we do have a back icon but the arrow is much longer and stands out as not being the android native icon.
Screenshots / GIFs
backnav.mp4
Tests
Tested devices
Checklist