Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send in $destination into Adapter #8

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

esilverberg
Copy link
Member

So it knows when the destination has been cleared, and can clear its own state.

…as been cleared, and can clear its own state.
@caiobzen
Copy link

caiobzen commented Jan 7, 2023

I like this idea but I also have another suggestion for dealing with navigations that may be useful:

1. Start by creating protocols for navigation actions

(This could be a file named Navigations or something like that, just as you did for the Destinations enum

import DomainModels

public protocol DetailsNavigation {
    func navigateToCountryDetails(country: Country)
}

public protocol AboutThisAppNavigation {
    func navigateToAboutThisApp()
}

2. Make the app's navigator conform to the protocols

extension TravelAdvisoriesNavHost: DetailsNavigation, AboutThisAppNavigation {

    public func navigateToCountryDetails(country: Country) {
        self.destination = Destinations.details(regionCode: country.regionCode)
    }

    public func navigateToAboutThisApp() {
        self.destination = Destinations.aboutThisApp
    }
}

3. Update CountryListAdapter init to receive an entity that conforms to these protocols

(It's worth creating a type alias to improve readability when there are multiple protocols)

// Adding new navigations would be as flexible as adding more protocols to this list
public typealias CountryListNavigation = DetailsNavigation & AboutThisAppNavigation

public struct CountryListAdapter: View {

    private let navigator: CountryListNavigation

    public init(navigator: CountryListNavigation) {
        self.navigator = navigator
    }

4. Pass in TravelAdvisoriesNavHost as the CountryListAdapter navigator

    @ViewBuilder func buildBaseView() -> some View {
        CountryListAdapter(navigator: self)
    }

5. Invoke the navigations from within CountryListAdapter

    public var body: some View {
        CountryListPage(listUiState: viewModel.state, onItemTapped: { country in
            viewModel.onCountrySelected(country: country)
        }, onButtonTapped: {
            viewModel.onButtonTapped()
        }, onFailOtherTapped: {
            viewModel.onFailOtherTapped()
        })
        .onReceive(viewModel.$navigationDestination, perform: { country in
            guard let country = country else { return }

            navigator.navigateToCountryDetails(country: country) // <----- Here

            viewModel.navigationDestination = nil
        })
        .pss_notify(item: $viewModel.error, alertBuilder: {
            $0.asFloatingAlert(viewModel: viewModel) {
                navigator.navigateToAboutThisApp()       // <----- And here
            }
        })
    }

Pros

  • Easy to append new navigations for an existing screen
  • It's possible to reuse existing code (for example: one could navigate from another screen to "About this app" screen by reusing the AboutThisAppNavigation protocol
  • All the screens that will handle navigation events doesn't have to know the existence of Destination. Indeed, this enum could live strictly within TravelAdvisoriesNavHost

Cons

  • The class responsible for dealing with the navigation tends to grow depending on the amount of user interactions
  • Since TravelAdvisoriesNavHost is a View, it gets weird writing tests to it. (Maybe moving all this routing logic to a separate entity may be an alternative 🤷)

Context

This is how we are currently handling navigations at Farfetch, and so far the app is scaling pretty well.
There's a short video about it here: https://www.youtube.com/watch?v=Y3hQ6Z_wW9k (It's the pilot episode of FARFETCH MOBCAST, so please bear with us and our acting skills 😂 )

Sorry for being nosy and also for the long comment but I found it interesting that this routing strategy seems to fit well on this app 😃


import Foundation

public enum Destinations {
Copy link

Choose a reason for hiding this comment

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

Maybe naming this enum singular (instead of plural) would eliminate any confusion when expecting parameters of this type:

// 👍 
func navigateTo(destination: Destination) { ... }

// ❓ 
func navigateTo(destination: Destinations) { ... } // it feels like we are going to navigate to multiple places

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants