ekazaev / route-composer

Protocol oriented, Cocoa UI abstractions based library that helps to handle view controllers composition, navigation and deep linking tasks in the iOS application. Can be used as the universal replacement for the Coordinator pattern.
MIT License
896 stars 63 forks source link

UINavigationController push animations breaking #37

Closed benronicus closed 5 years ago

benronicus commented 5 years ago

After a couple of routings have been successfully performed, animations stop working for all navigation controller pushes. I can't tell what causes it because it seems to begin randomly, but thereafter it's an app-wide problem until I restart the app. Basic app structure is a tab bar controller with split view controllers in some of the tabs. I'm on iOS 13 beta and Xcode 11.

ekazaev commented 5 years ago

@benronicus I would love to help you but I need more details about this issue. UI tests are passing on iOS 13 beta and it works in our project as well. Are you on the latest version 2.2.0? Does it happen on iOS 12? Do you have custom transitions? Can you provide a test project, that would be ideal.

ekazaev commented 5 years ago

@benronicus I created a branch where in example app i tried to reproduce the configuration you described. Please see CitiesConfiguration.

You can try to run deep-links from terminal

xcrun simctl openurl booted dll://city?city=1
xcrun simctl openurl booted dll://city?city=3
xcrun simctl openurl booted dll://product?product=3
xcrun simctl openurl booted dll://product?product=5
xcrun simctl openurl booted dll://product?product=1
xcrun simctl openurl booted dll://product?product=4
xcrun simctl openurl booted dll://city?city=1
xcrun simctl openurl booted dll://city?city=2

I could not reproduce the issue described neither on iOS 12 nor iOS 13. Can you please play around with it and provide me steps to reproduce your issue.

Also, in your app please check the logs. The issue you describing reminds me the behaviour after unbalanced calls to begin/end appearance transitions warning. Probably you will find something similar in your logs.

NB: I see that Apple changed the behaviour of UISplitViewController in iOS 13, and it doesn't split its chidren view controllers during the transition from extended layout to compact the way it happens in iOS 12. You probably shoud handle it in your UISplitViewControllerDelegate.

benronicus commented 5 years ago

Great, I'll look into this over the weekend in depth. Thanks for the quick feedback!

benronicus commented 5 years ago

Do you have a link to Apple's API change to UISplitViewController?

ekazaev commented 5 years ago

@benronicus It is not an API change. It is a behaviour change. I see that in ios 12 it keeps 2 navigation controllers. and IOS 13 it removes it during a transition from a compact-width size class to a regular-width size class. But i did not really use UISplitViewController to give advices. Just noticed this difference in the behaviour.

ekazaev commented 5 years ago

@benronicus The code in the branch in iOS 12: https://www.dropbox.com/s/2gok2lq8u81he4b/SplitIOS12.mov?dl=0

The same code on iOS 13: https://www.dropbox.com/s/xuxb24zrbjxvny3/SplitIOS13.mov?dl=0

So event there is no change in the API of the UISplitViewController there are the changes in the behaviour. But i managed to fix it by changing the delegate implementation

ekazaev commented 5 years ago

@benronicus So, first i would recommend you to check that you do not have warnings like unbalanced calls to begin/end appearance transitions or similar. It can be the result of your implementation of UISplitViewControllerDelegate. Second, checkout the branch and try to reproduce the issue you are having using deeplinks from the terminal. Probably you will have to update the UISplitViewControllerDelegate there to make the behaviour of the UISplitViewController the same as in your project. Unfortunately, i wont be able to help if i don't see the issue. Please, keep me updated.

ekazaev commented 5 years ago

@benronicus Do you have any update?

benronicus commented 5 years ago

Yes, please close this issue as not a bug. Found it and it had nothing to do with RouteComposer. Sorry for the goose chase!

ekazaev commented 5 years ago

@benronicus Cool. Thank you