slovnicki / beamer

A routing package built on top of Router and Navigator's pages API, supporting arbitrary nested navigation, guards and more.
MIT License
588 stars 129 forks source link

Improve handling of page keys that use URI parameters #414

Open slovnicki opened 2 years ago

slovnicki commented 2 years ago

Issue #399 explains and demonstrates the problem.

Reasons why it happens and possible solutions are given in https://github.com/slovnicki/beamer/issues/399#issuecomment-985172364, but do not seem ideal.

jpangburn commented 2 years ago

I'll restate the problem for clarity. You have a home screen, then a list screen that can optionally be filtered by a URL parameter, then a view result screen- let's call these screens Home, Books, and Detail. Here are the steps to create the problem:

  1. On Home, you enter a query to filter books and the app takes you to the Books screen with a ?title=xyz query parameter.
  2. In the location builder for Books, you use that title query parameter to build the Home page and Books page
  3. You click on a book to go to Detail screen
  4. In the location builder for Detail, you don't have access to the title query parameter to recreate the Books page so it just ends up with the key 'books' instead of 'books-$titleQuery'
  5. When you hit the app bar back arrow, it pops the navigation stack- but instead of finding [Home, Books(key=books)], the stack recalls the query parameter so you find [Home, Books(key=books-xyz)]

This mismatch of expected and actual navigation stack entries (forgive me if I'm using terminology incorrectly, I'm new here) leads to a weird animation.

I don't know the solution, but agree the current solution is not ideal. It requires the user to do extra work passing around state for pages with query parameters so child pages can build the location stack properly.

Perhaps the fault lies in the way the beamToNamed parameter is constructed:

onTap: () => context.beamToNamed(
  '/books/${book['id']}',
  data: {'title': titleQuery},
),

When you're beaming to a child location of your current location, maybe it's wrong to fully reconstruct the pages stack from the URL using a location builder here. URLs were not meant to preserve navigation state as browers' back button means 'go back chronologically' not 'navigate up the hierarchy' like our app bar arrow button does. Maybe it should be constructed more like:

onTap: () => context.beamToChild(
  '${book['id']}'
),

(I'm making up this 'beamToChild' method)

Why should the user have to rebuild the full pages stack of their current location to go to a child location? It's fine with this simple books example, but suppose you were a couple levels deep already with perhaps a couple queries above you. This is not unlikely, suppose you have filtered by manufacturer, then by product- you could have something conceptually like /manufacturers?name=cat/products?type=earthmoving (which is only even legal if you're using URL fragments like '#/blah/blah'). At this point you could expect to go to a detail screen. Why build all that back up again in the beamToNamed call when you really just want to say "go to this specific child and keep my parent stack the same".

I guess the downside of a 'beamToChild' method is that the location's buildPages method would become more complex because it would have to know if it was only adding a child on to an existing parent stack vs. building the whole stack.