Mijick / NavigationView

Navigation made simple (SwiftUI)
MIT License
238 stars 9 forks source link

pop(to:_) does not work #51

Open Nathan1258 opened 1 month ago

Nathan1258 commented 1 month ago

For the given code:

struct View1: NavigatableView {
        var body: some View {
            VStack {
                Text("View1")
                Button(action: {
                    View2().push(with: .horizontalSlide)
                }) {
                    Text("Go to second view")
                }
            }
        }
    }

    struct View2: NavigatableView {
        var body: some View {
            VStack {
                Text("View2")
                Button(action: {
                    View3().push(with: .horizontalSlide)
                }) {
                    Text("Go to third view")
                }
            }
        }
    }

    struct View3: NavigatableView {
        var body: some View {
            Button(action: {
                pop(to: View1.self)
            }){
                Text("Go to View 1")
            }
        }
    }

pop(to:_) does not work. Sometimes it does not pop any view, sometimes it pops to the previous view as if I ran pop(). Steps to reproduce are above. Below is a demo of the code above:

https://github.com/user-attachments/assets/9284e8c9-5a4b-4b48-92a9-b5255379ee02

FulcrumOne commented 1 month ago

Hey @Nathan1258,

Hah, I'm super confused because I've just tested your code and it seems to work: https://github.com/user-attachments/assets/b81f621c-d53a-4d51-8700-066eea74c561

Could you show me your @main struct please?

Nathan1258 commented 1 month ago

Hey @Nathan1258,

Hah, I'm super confused because I've just tested your code and it seems to work: https://github.com/user-attachments/assets/b81f621c-d53a-4d51-8700-066eea74c561

Could you show me your @main struct please?

Hey, @FulcrumOne, I implement the library:

.implementNavigationView(config: navigationConfig)

with this config:

private extension App {
    var navigationConfig: NavigationGlobalConfig {
        var config = NavigationGlobalConfig()
        config.backgroundColour = .black
        config.backGesturePosition = .edge
        return config
    }
}

I've just realised I was testing on a device on iOS18 - maybe this is the issue? I will be able to test on a iOS17 device later and will update! It seems that setAsRoot() is also a bit buggy upon testing but not sure yet - again, will update/make a new issue if it is purely iOS18

FulcrumOne commented 1 month ago

@Nathan1258,

You read my mind, because I was going to ask if it's on iOS beta by any chance 😅 At the end of next month, I will be checking and fixing all the libraries for iOS 18, as the same situation affects PopupView, for example.

Nathan1258 commented 1 month ago

@FulcrumOne

Right, now I'm even more confused. I've just tried my partners iPhone (running iOS 17.5.1) and I get the same behaviour, and I've just created a new project and tried the above code so it's my app. I literally have no idea what it could be as I was troubleshooting for 3 hours yesterday.

This is my @main with the objects and unnecessary thing omitted:

import SwiftUI
import MijickPopupView
import MijickNavigationView

@main
struct MyApp: App {

    @UIApplicationDelegateAdaptor(AppDelegate.self) var delegate

    // StateObjects are herer

    init() {
        // StateObject initalisation here
    }

    @Environment(\.scenePhase) var scenePhase
    @State private var lastOpenDate = Date()

    var body: some Scene {
        WindowGroup {
            Splash()
                .implementNavigationView(config: navigationConfig)
                // Environment Objects injections here
                .implementPopupView()
                .onChange(of: scenePhase) {
                    if scenePhase == .active {
                        handleAppBecameActive()
                    } else if scenePhase == .inactive {
                        handleAppBecameInactive()
                    } else if scenePhase == .background {
                        handleAppEnteredBackground()
                    }
                }
        }
    }
}

private func handleAppBecameActive() {
    let defaults = UserDefaults.standard
    if let savedDate = defaults.object(forKey: "lastOpenDate") as? Date {
        let calendar = Calendar.current
        if !calendar.isDateInToday(savedDate) {
            // MARK: The app was last opened on a different day, reload data
            reloadAppData()
        }
    }
}

private func handleAppBecameInactive() {}

private func handleAppEnteredBackground() {
    let defaults = UserDefaults.standard
    defaults.set(Date(), forKey: "lastOpenDate")
}

private func reloadAppData() {
    NotificationCenter.default.post(name: Notification.Name("reloadAppData"), object: nil)
}

private extension MyApp {
    var navigationConfig: NavigationGlobalConfig {
        var config = NavigationGlobalConfig()
        config.backgroundColour = .black
        config.backGesturePosition = .edge
        return config
    }
}
Nathan1258 commented 1 month ago

Think I've found the issue.

Given this:

var body: some Scene {
        WindowGroup {
            MainView()
                .implementNavigationView(config: navigationConfig)
                .implementPopupView()
        }
    }
struct MainTest: NavigatableView{
    var body: some View{
        View1()
    }
}

struct View1: NavigatableView {
    var body: some View {
        VStack {
            Text("View1")
            Button(action: {
                View2().push(with: .horizontalSlide)
            }) {
                Text("Go to second view")
            }
        }
    }
}

struct View2: NavigatableView {
    var body: some View {
        VStack {
            Text("View2")
            Button(action: {
                View3().push(with: .horizontalSlide)
            }) {
                Text("Go to third view")
            }
        }
    }
}

struct View3: NavigatableView {
    var body: some View {
        Button(action: {
            pop(to: View1.self)
        }){
            Text("Go to View 1")
        }
    }
}

The pop(to:_) does not work when View1() is wrapped around a root view. However if I go straight to View1() from my @main:

var body: some Scene {
        WindowGroup {
            View1()
                .implementNavigationView(config: navigationConfig)
                .implementPopupView()
        }
    }

the functionality works.

You're probably wondering why I would do the first version of the code. In my actual app, the first view in my @main is Splash.view which conditionally renders either a login view, loading view or the main app (depending if the app is loading/logging in the user or if the user isn't logged in yet). The MainView in this example acts like my Splash view. For example:

var body: some Scene {
        WindowGroup {
            MainTest()
                .implementNavigationView(config: navigationConfig)
                .implementPopupView()
        }
    }
struct MainTest: NavigatableView{
        var body: some View{
            if user.loggedIn && auth.isLoaded{
                View1()
            }else{
                View2()
            }
        }
    }

struct Login: NavigatableView {
    var body: some View {
        <...>
    }
}

struct Main: NavigatableView {
    var body: some View {
        <...>
    }
}
FulcrumOne commented 1 month ago

@Nathan1258, Oh, I see. To be more specific, this doesn't work because View1 wasn't pushed onto the stack (only MainTest, View2 and View3 are on the stack). There are several possible solutions to this problem:

  1. You can maintain your structure, i.e. have an app state wrapper containing View1, View2 and View3 according to the application state. The problem here will be the need to do extra work when changing the app's state.
  2. You can play with the .setAsNewRoot modifier, but there are some "visual" limitations; let's say that the user clicks on "logout" button - logically the animation that comes afterwards should start from the left edge of the screen and go towards right, but this will be impossible to achieve.

So here we have the difficult art of compromises and I think I would choose the first path, because it seems to be a cleaner solution in the long run.