-
-
Notifications
You must be signed in to change notification settings - Fork 678
fix-4036 swipe back message #4432
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
base: main
Are you sure you want to change the base?
Conversation
|
@chrisbobbe please review this. |
|
Thanks, @Aman-Maheshwari! Please write a descriptive commit message that follows our commit-message style. You may also find it helpful to see examples of existing commit messages by viewing the commit history (see our git guide for how to do that; in particular, I use Greg's "secret" all the time 🙂). Also, pinging @gnprice as someone who uses Android much more than I do. FYI, as I mentioned in the issue, Android does seem to recommend using gesture-based navigation, so I think this would be a good change. You can also see previous discussion on the PR this replaces, #4398. |
1e6f4cf to
c606110
Compare
|
I have updated the commit messgae to be more descriptive @chrisbobbe . |
This is fine because we've just changed all the places where we were navigating to 'loading', to instead navigate to 'main-tabs'. This removes a `NavigationService` callsite that's been throwing quite a lot; that's zulip#4453. We still don't know why it's throwing those errors, and we haven't managed to reproduce it yet. But it means we should continue to prioritize removing `NavigationService`. See discussion at https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.23M4453.20.22Tried.20to.20use.20NavigationService.20before.20appContaine.2E.2E.2E/near/1111875. Fixes: zulip#4432
This is fine because we've just changed all the places where we were navigating to 'loading', to instead navigate to 'main-tabs'. This removes a `NavigationService` callsite that's been throwing quite a lot; that's zulip#4453. We still don't know why it's throwing those errors, and we haven't managed to reproduce it yet. But it means we should continue to prioritize removing `NavigationService`. See discussion at https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.23M4453.20.22Tried.20to.20use.20NavigationService.20before.20appContaine.2E.2E.2E/near/1111875. Fixes: zulip#4432
This is fine because we've just changed all the places where we were navigating to 'loading', to instead navigate to 'main-tabs'. This removes a `NavigationService` callsite that's been throwing quite a lot; that's zulip#4453. We still don't know why it's throwing those errors, and we haven't managed to reproduce it yet. But it means we should continue to prioritize removing `NavigationService`. See discussion at https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.23M4453.20.22Tried.20to.20use.20NavigationService.20before.20appContaine.2E.2E.2E/near/1111875. Fixes: zulip#4432
Proposed changes
Fix: issue handling go back on swipe on android
Issue(s)
Fix issue #4036 .
#4398 is closed because of merge issue.
Video
fixissue4036.1.mp4