Navigate to conversation on tapping notification #362
Conversation
| // TODO(nav): Better interact with existing nav stack on notif open | ||
| navigator.push(MaterialWidgetRoute( | ||
| page: PerAccountStoreWidget(accountId: account.id, | ||
| child: MessageListPage(narrow: narrow)))); |
There was a problem hiding this comment.
Could add a TODO(#82) for opening the message list to the specific message the notification was about:
|
Oh hmm, one thing I just noticed: is this meant to work when the app is not open at all, i.e., where tapping a notification is meant to cause a cold start of the app? I don't think this case is yet covered on Android. When I tap a notification when the app is completely closed, it does open the app, but just to the "Choose account" screen. |
Hmmm good catch. I think that is an important case for #123 — probably the most common case. I'll take a look at handling it. |
|
Thanks for the review! Updated — PTAL. I acted on the small comment above and then also added three commits at the end to handle the launch-from-notification case. |
|
(As discussed at #352, on my device I still don't actually get notifications while the app is completely stopped. So wherever those symptoms apply, this still won't deliver a large swath of the value of getting a notification and opening it. That issue is tracked at #342. But I can still manually test the issue you raised: open the app, cause a notification, then kill the app, then open the notification. The app was running when the notification appeared, but not when it was opened, so the opening of the notification is what launches the app.) |
We'll add a test soon where we want to test a navigation interaction against the actual ZulipApp widget. This will be helpful for that.
Still only on Android; iOS support for notifications in general is coming next. This also only applies when the app is already running (either in the foreground or the background) when the notification is opened. The case where the app isn't already running and is launched from the notification will be handled in an upcoming commit. Fixes-partly: zulip#123
As a bonus, this also means that if you manage to open a notification in the brief time between the app launching and the GlobalStore data getting loaded from local disk, we'll properly wait for that data and then navigate to the conversation in that case too. Fixes: zulip#123
|
Thanks! Merged. FWIW in my testing just now, on the office Android device, I also saw long delays in a notification appearing if the app was completely closed when the message was sent. The notifications did eventually arrive, but not before I opened the app again. (In fact, they didn't even arrive at the time I reopened the app—it took a surprisingly long time after that, maybe over a minute.) So I managed to test the launch-from-notification case this way too:
|
Still only on Android; iOS support for notifications in general is coming next.
Fixes: #123