Skip to content

Conversation

@gnprice
Copy link
Member

@gnprice gnprice commented Dec 30, 2025

Fixes #1852.

This required a bit of setup in order to get our hands on the right information about the current navigation state.

There were a few product choices to make about the details of the behavior. Quoting from the last commit:

    if (…) {
      // The current page is already a MessageListPage at the desired narrow.
      // Instead of pushing another copy of it, stay there; see #1852.

      // Do dismiss any non-page routes, like dialogs and bottom sheets, though.
      // That way we're presenting the page directly, like the user asked for
      // by opening the notification.
      navigator.popUntil((route) => route is PageRoute);

      // TODO(#1565): Scroll to the specific message if nearby; else jump there,
      //   or push a new page anchored there.
      return;
    }

Selected commit messages

783e1d1 nav: Track the whole navigation stack

Kind of silly that Navigator upstream doesn't provide this directly
in its API. But here it is.

bc396ff nav: Add currentPageRoute getter on NavigationStack

1345d0d nav: Add WidgetRoute.pageElement

5d0703b msglist: Add currentNarrow and stateOfRoute static methods

9e33d49 notif: Dedupe msglist page on opening notif

Fixes #1852.

@gnprice gnprice added the maintainer review PR ready for review by Zulip maintainers label Jan 6, 2026
Copy link
Member

@rajveermalviya rajveermalviya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gnprice! LGTM and tests great. Please go ahead and merge.

(This'll probably create conflicts for #2043, I'll rebase that PR once this is merged.)

These already do something a bit more complex than opening the
notification's conversation (they might pop a number of routes
belonging to a different account, and push a new HomePage);
and more wrinkles will be coming in the future.
Kind of silly that Navigator upstream doesn't provide this directly
in its API.  But here it is.
The behavior should be unchanged here, because the navigation observer
_UpdateLastVisitedAccount was already keeping the global store's
record of the current account up to date.

But logically the information this bit of code wants is about the
current navigation stack in the app's navigator.  So now that we
maintain an accessible record of that stack, let's inspect that
directly.
This method will soon need to be more hands-on with the details of the
route, in order to decide when an equivalent route is already in view,
for zulip#1852.
@gnprice gnprice merged commit d54bdab into zulip:main Jan 6, 2026
1 check passed
@gnprice gnprice deleted the pr-nav-dedup branch January 6, 2026 21:06
@gnprice
Copy link
Member Author

gnprice commented Jan 6, 2026

Thanks for the review and testing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer review PR ready for review by Zulip maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compress nav stack for duplicates on opening notification

2 participants