-
Notifications
You must be signed in to change notification settings - Fork 30
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
iOS 17 state restoration crash fix #767
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
viewIsAppearing
works great and it's great that it is backwards compatible. One small note, it is called every time the view appears, and although in this case the actions would be noops after the first call, in some cases we may still need to guard with a Bool property, to truly run specific appearance logic only once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In MasterContainerViewController
since the setup of the bottom container happens only for iPad form, which is the best optimization, now in iPhone form bottomYConstraint
& bottomContainerBottomConstraint
may be nil, so they should become optional as well.
Fixed in 6878aab, also there was another issue with keyboard apperance. When tag filter was in custom position and keyboard appeared, it just covered the filter if it was just too low. Fixed in 88f888f |
The app crashed on iOS 17 during state restoration due to changes in #762. It crashed because
zotero-ios/Zotero/Scenes/Main/Views/MainViewController.swift
Line 96 in 27853fa
zotero-ios/Zotero/Scenes/Master/MasterContainerViewController.swift
Line 104 in 27853fa
bottomController
tried to add itself intobottomContainer
, which didn't exist yet.I fixed it by creating
bottomController
and not adding it as subview immediately, but I moved adding it as subview toviewDidLoad
function together with other UI components.I also noticed something I didn't notice during code review of #762 and that is that the distinction between
MasterCoordinator
andMasterTopCoordinator
was not needed anymore, because theMasterContainerViewController
took that role and connects "top" and "bottom" controllers together. TheMasterCoordinator
was almost empty anyway at this point, so I refactored a little and removed it.Last fix is in
zotero-ios/Zotero/Scenes/Master/MasterContainerViewController.swift
Line 320 in 27853fa
viewDidAppear
which is called too late. I moved it toviewIsAppearing
which seems to have better effect.