davdroman / swiftui-navigation-transitions

Pure SwiftUI Navigation transitions ✨
MIT License
838 stars 37 forks source link

UI freezing on iOS 17 device #100

Closed muzix closed 1 year ago

muzix commented 1 year ago

Hi @davdroman, thanks so much for making this library. Recently, I bumped into a strange issue which is 100% reproducible on iOS 17 device (not simulator)

Reproducible setup within Demo app

Reproduce steps From PageOne, go to PageTwo then using pan gesture to go back to PageOne, then tap Show me! button again. The UI will hang forever

You can check out my branch for the demo: https://github.com/muzix/swiftui-navigation-transitions/tree/muzix/freezing-issue-ios-17

Investigating I think this is a SwiftUI bug. But I found that adding setNavigationBarHidden(false, animated: true|false) into NavigationTransitionDelegate somehow fixed this issue.

func navigationController(_ navigationController: UINavigationController, willShow viewController: UIViewController, animated: Bool) {
        navigationController.setNavigationBarHidden(false, animated: true)
        initialAreAnimationsEnabled = UIView.areAnimationsEnabled
    UIView.setAnimationsEnabled(transition.animation != nil)
    baseDelegate?.navigationController?(navigationController, willShow: viewController, animated: animated)
}

But it has another side effect which make navigation bar blinking when transitioning between screens with hidden nav bar.

davdroman commented 1 year ago

Thank you for the comprehensive report. I'll look into this as soon as I can.

davdroman commented 1 year ago

Sorry for the delay. Very weird indeed, most definitely a SwiftUI bug that's hidden by the fact that one can't swipe back when nav bars are hidden in vanilla SwiftUI.

Your solution does (somehow? 🤯) work, so I'm going to apply it selectively on iOS 17 and write some regression UI tests in order to have our bases covered for when iOS 18 comes around. I'm currently quite busy so this will take me until sometime next week, but feel free to do it yourself and PR it if you feel like it. My current working branch is https://github.com/davdroman/swiftui-navigation-transitions/tree/fix/freezing-ios-17.

muzix commented 1 year ago

Hi @davdroman Thanks for your time. I found that my one-line fixed the issue at first but then it introduced another problem.

navigationController.setNavigationBarHidden(false, animated: true)

The problem is in this context, we don't know if the target view controller needs a hidden navigation bar or not. So if we go back and forth between screen A (not hidden) and screen B (hidden bar), or even more screens involved, the navigation bar visibility will be messed up. Even freezing happens when the visibility doesn't match what we declared in SwiftUI view.

In my project, I workaround this issue by not patching the library, but using a delegate instead

.navigationTransition(.default, interactivity: panNavigationState.isPanEnabled ? .pan : .disabled)
        .introspectNavigationController { navigationController in
            if #available(iOS 17, *) {
                navigationController.delegate = NavigationControllerBaseDelegate.shared
            }
        }
class NavigationControllerBaseDelegate: NSObject, UINavigationControllerDelegate {
    static let shared = NavigationControllerBaseDelegate()

    func navigationController(_ navigationController: UINavigationController, willShow viewController: UIViewController, animated: Bool) {
        navigationController.setNavigationBarHidden(viewController.isNavBarHidden, animated: true)
    }
}

isNavBarHidden is the property I injected into the underlying UIViewController of the SwiftUI view, where I set the navigation bar visibility

.navigationBarHidden(true)
.introspectViewController { viewController in
      viewController.isNavBarHidden = true
}

Quite a lot of weird code but the solution is working fine so far in my project. So I think maybe better not to introduce the one-line fix into the library (and we also rarely have the navigation bar visibility varying between screens? 🫠)

davdroman commented 1 year ago

So I think maybe better not to introduce the one-line fix into the library (and we also rarely have the navigation bar visibility varying between screens? 🫠)

I concur. If anyone else raises this issue, now we have somewhere to direct them for answers.

I guess I'll be closing this now. Thank you for your research 🤝