Refactor navigation destinations #1219
Conversation
|
|
||
| /// A NavigationStack that knows how to present views to display Nostr entities like `Events` and `Authors`. | ||
| /// Take care not to nest these. | ||
| struct NostrNavigationStack<Content: View>: View { |
There was a problem hiding this comment.
I'm not very convinced about this name. It doesn't look precise to me. The fact that it also adds URL and ReplyToNavigationDestination makes me think that it is more Nos related than Nostr.
There was a problem hiding this comment.
I agree with your other comment; I like NosNavigationStack for the name here. But I'm open to NostrNavigationStack if @mplorentz has a convincing reason.
There was a problem hiding this comment.
I was already on the fence between NosNavigationStack and NostrNavigationStack. Updated!
There was a problem hiding this comment.
Ah, I just remembered what pushed me towards NostrNavigationStack was that I'm creating a NostrNavigationDestination enum in my next PR, and NosNavigationDestination sounds like it's every navigation destination in the app, which it isn't. But we can address that in the next PR.
There was a problem hiding this comment.
We are losing .navigationDestination(for: EditProfileDestination.self) in all cases, but it seems safe to remove. We shouldn't be opening the EditProfile directly from these cases. I tested it to be double-sure and opening the Edit Profile from the banner that appears when you create an account still works.
I just left a comment about NostrNavigationStack. I think it should be NosNavigationStack because it includes more than NostrEntities, but I'm open to leave as it is if we don't agree. We can ask @joshuatbrown to give advice about this.
Issues covered
This is the beginning of the solution to problem 2 in #703 (comment)
Description
This is purely a refactoring PR. The app should work exactly the same after this is merged. Each tab in the app and the side menu all had duplicated code for handling
navigationDestination()s. This refactors them into a shared view. In the next PR I'll change how the destinations work.How to test
Verify that each tab can still push a detail view for each type of content - Event, Author, URL, etc. Pay special attention to the Profile tab and side menu - these had some duplicated navigation destinations that I removed.