Skip to content
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

android: When opening a notification, "back" should not leave the account's context #1210

Open
chrisbobbe opened this issue Dec 26, 2024 · 4 comments · May be fixed by #1373
Open

android: When opening a notification, "back" should not leave the account's context #1210

chrisbobbe opened this issue Dec 26, 2024 · 4 comments · May be fixed by #1373
Labels
a-Android Issues specific to Android, or requiring Android-specific work a-notifications beta feedback Things beta users have specifically asked for
Milestone

Comments

@chrisbobbe
Copy link
Collaborator

chrisbobbe commented Dec 26, 2024

From beta feedback:

Steps to reproduce:

  • have at least two servers configured in the app
  • use the app on one server, then switch away from it
  • receive a notification for the other server
  • tap the notification to open zulip
  • hit the back button

Expected behavior: the back button should go to rhe next level up on the server that the notification came from. (For instance, if browsing a topic it should go up to the stream, or similar.)

Observed behavior: the back button goes to a view of the other server.


If the app is showing UI for Account A and you open a notification for Account B, we push a message-list route for Account B onto the nav stack that's supposed to belong to Account A. This means the "back" button will take you out of Account B's context, which can be confusing.

When we navigate on opening a notification, we should switch to a new nav stack for the notification's account (if different) before pushing the message-list route.

(The iOS counterpart is covered by #1147, for navigating at all when a notification is tapped.)

Implementation

Probably we want to call HomePage.navigate

  /// Navigate to [HomePage], ensuring that its route is at the root level.
  static void navigate(BuildContext context, {required int accountId}) {
    final navigator = Navigator.of(context);
    navigator.popUntil((route) => route.isFirst);
    unawaited(navigator.pushReplacement(
      HomePage.buildRoute(accountId: accountId)));
  }

—and then push the result of MessageListPage.buildRoute after that.

@chrisbobbe chrisbobbe added a-Android Issues specific to Android, or requiring Android-specific work a-notifications beta feedback Things beta users have specifically asked for labels Dec 26, 2024
@chrisbobbe chrisbobbe added this to the M5: Launch milestone Dec 26, 2024
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Dec 30, 2024
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Dec 30, 2024
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Dec 30, 2024
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Jan 2, 2025
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Jan 27, 2025
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Jan 27, 2025
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Jan 27, 2025
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Jan 28, 2025
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Feb 7, 2025
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Feb 7, 2025
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Feb 7, 2025
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Feb 7, 2025
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Feb 7, 2025
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Feb 10, 2025
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Feb 11, 2025
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses zulip#1210 for background notifications.
@lakshya1goel
Copy link
Contributor

lakshya1goel commented Feb 12, 2025

Hi, I have started working on this, waiting for related CZO Discussion will raise a PR shortly. Thanks!

github-actions bot pushed a commit that referenced this issue Feb 12, 2025
…ground

Previously, when two accounts (Account-1 and Account-2) were
logged in, the app always defaulted to showing the home page
of Account-1 on launch. If the app was closed and the user
opened a notification from Account-2, the navigation stack
would be:
  HomePage(Account-1) -> MessageListPage(Account-2)

This commit fixes that behaviour, now when a notification is
opened while the app is closed, the home page will correspond
to the account associated with the notification's conversation.

This addresses #1210 for background notifications.
@gnprice gnprice marked this as a duplicate of #1343 Feb 15, 2025
@gnprice
Copy link
Member

gnprice commented Feb 15, 2025

To update the status of this issue: #1240 fixed one case of this. The description says it applies when "opened from background", but thinking again about the code I think it actually applies to a situation I would term differently:

  • That PR applies when the app was not running, and starts running because the user opened a notification.
  • If the app was already running but in the background, I believe that PR doesn't apply. That would instead go through didPushRouteInformation, which wasn't affected by the PR.
  • The third possible case is when the app was already running and was in the foreground. I believe that also goes through didPushRouteInformation.

So I believe the current state is that this issue is resolved for the case where the app wasn't running, but remains open for the case when the app was already running, either in the background or the foreground. @rajveermalviya does that sound right to you?

@rajveermalviya
Copy link
Collaborator

Correct, #1240 only addresses the issue when the app was not running, and the user tapped the notification.

I agree that the title of the main (4b2f51e) commit of that PR may've been misleading (though the description tries to explain it correctly), I guess the terminology I was trying to match was FCM's use of "foreground" and "background" for received notifications. But I see it doesn't really make much sense for opening notifications. Sorry for the confusion.

As for the other cases, there was some discussion in the CZO thread linked in the description regarding some potential UX challenges.

lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Feb 22, 2025
lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Feb 22, 2025
lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Feb 22, 2025
@lakshya1goel
Copy link
Contributor

Opened PR #1373 for fixing this issue, PTAL. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-Android Issues specific to Android, or requiring Android-specific work a-notifications beta feedback Things beta users have specifically asked for
Projects
Status: No status
4 participants