austinried / subtracks

A music streaming app for Subsonic-compatible servers
GNU General Public License v3.0
703 stars 30 forks source link

[2.0] Single navigation history stack, back button to go back to the last screen #169

Open hedgepigdaniel opened 1 year ago

hedgepigdaniel commented 1 year ago

Is your feature request related to a problem? Please describe. I find it confusing that there are multiple navigation history stacks. As I navigate through the app and use the Android back button, I expect to be taken to the last screen I was on. Instead, the back button goes to various screens that I don't expect. For example:

Describe the solution you'd like Instead of having multiple stacks of navigation history inside the 4 tabs (library, browse, search, settings), I would prefer for there to be only one. This matches my mental model of what the back button does - i.e. it goes back to where I was before.

The only exceptions I expect are:

Additional context I think this could be implemented with auto_router by flattening the routes configuration, so there are no nested children.

This means that pages like ArtistPage would be independent of the tabs - i.e. not inside any tab in particular. The tab bar could behave in two ways with these routes:

If we can agree on an approach I'd be happy to make a PR.

austinried commented 1 year ago

This is pretty much what I'd like the navigation to be as well, and it would mostly match the 1.x behavior as well, I just haven't had time to work on the auto_route configuration for it. I am not sure configuring auto_route is as simple as just flattening the routes configuration however, but I would be open to a PR if you have a solution.

Couple things I would like to specify for the approach here though:

Also, this bit sounds like a bug:

For example, if I open a context menu and "View artist", when I go back it would make sense if the context menu was closed rather than open

My intention here (and what happens on my devices) is that the context menu is closed once you choose an option that navigates somewhere else.

hedgepigdaniel commented 1 year ago

I am not sure configuring auto_route is as simple as just flattening the routes configuration however

Yeah, I've done a bit more experimentation and while doing that does fix the history stack, it introduces problems with animations (the tab bar "reappears" on each transition, even though I would say it didn't go anywhere and should not be included in the animation). I'm tempted to try vrouter aswell, because it's one of the few flutter routing packages that supports nested routes in terms of components (hopefully therefore animations), but still maintains a single linear history. Will fiddle a bit more and let you know what I come up with.

Everything else (album/artist/playlist/source etc.) should still show the now playing bar and tab bar on the screen.

Makes sense - I'll try and make that work and if there's an issue with it we can discuss alternatives.

The library tabs (albums/artists/playlists/songs) should probably not be in the history stack

Makes sense. I'm reading this as "when swiping between albums/artists/etc, that does not push a new entry onto the history stack - instead it replaces the current entry".

My intention here (and what happens on my devices) is that the context menu is closed once you choose an option that navigates somewhere else

Agree - AFAICT that is how it works - I don't see a problem. I was just describing how I expect it to work as a user, to illustrate the things that "don't count" as transitions in my mind. I think it's similar to what you said about the library artists/albums/etc - except that probably those tabs maintain their state when you go back, in contrast to the modal, which probably should be gone by the time the user comes back to the screen where it was shown.

austinried commented 1 year ago

Normally I think I would say swapping out routing packages at this point is too big of a change, but to be honest auto_route has been getting on my nerves for awhile now. After I upgraded to v6 I had other animation related bugs and had to downgrade back to v5, and it seems overly complex to do stuff like observe the current route/path.

So, I would also be open to swapping that out, but I'm mostly looking for a simpler solution here. I'm hesitant about vrouter though due to the not being updated in 12 months.