feat: Improve Image Viewer toolbar auto-hide

[tinsukE]

Cancel the auto-hide behavior in case the toolbar menu items are interacted with.

Improves #505, #507

[nikclayton]

As well as the menu comments, how do you feel about a slightly larger refactoring?

The existing mechanism for sharing the toolbar visibility isn't great.

I think the right way to do it (based on https://developer.android.com/guide/fragments/communicate#viewmodel) is roughly:

1. Create a ViewMediaActivityViewModel, that contains the state of the toolbar's visibility (as a MutableStateFlow internally, externally presented as a StateFlow)
2. Have a method in the viewmodel that the activity can call to set a new state for the toolbar's visibility. In the viewmodel this sets a new value in to the state flow.
3. Have ViewMediaFragment collect this state flow, and react to changes in the toolbar's visibility as appropriate.

I appreciate that's more work. If you can take that on, great. If not, I'm happy to build on this PR and do that implementation.

[tinsukE]

@nikclayton I took a stab at the proposed ViewModel refactoring: tinsukE#2

I could keep the PRs separate or add that commit to this one.

[nikclayton]

I really like the viewmodel refactoring in the second PR (I left a couple of comments there), so I think merging that in to this one would be good, as it cleans up some code that's otherwise a little confusing.

In hindsight, understanding the whole class hierarchy could've given me a better insight on how to make the changes.

It doesn't help that the existing code is a mix of different styles and approaches. I'm slowly cleaning it up to make things more consistent and hopefully easier to understand, but it's taking quite a bit of time.

And sometimes the only way to see what's wrong with a particular approach is to write the code and see how it can be cleaned up. I've lost count of the number of times I've started refactoring something to go in one direction, and then realise (because the refactoring made me understand the code better) that there was a cleaner way to do it.

[tinsukE]

@nikclayton I addressed the comments on the refactor PR and merged it here.

It now seems good to merge to me!

[nikclayton]

@nikclayton I addressed the comments on the refactor PR and merged it here.

It now seems good to merge to me!

Yep. I made a couple of small changes in df5d66e (it didn't seem worth the effort of going through another round of review) that I discovered after running the code locally. I filed https://issuetracker.google.com/issues/329322653 for the surprising onPrepareMenu behaviour too.

Assuming this goes cleanly through CI I'll merge it shortly. Thanks for your work on this

[tinsukE]

Alright, it looks good!

I see the clarity brought by the rename and flag value inversion, but why make the MenuProvider change?

[nikclayton]

Menus are added elsewhere using MenuProvider. If you don't do that here then they're ignored (e.g., the "Report error" menu was missing).