Fix LockViewController dismissal when presented as a popup #647
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
agirault
changed the title
closable option behavior when Lock is presented as a popupclosable option behavior when Lock is presented as a popup
On iPad, with a presentation style of .formSheet, the LockViewController could be dismissed by simply tapping outside the bounds of the modal, without regard for the `closable` [1] option. Setting `isModalInPresentation` [2] based on `closable` allows controlling whether or not UIKit should prevent the interactive dismissal of the popup. [1] https://auth0.com/docs/libraries/lock-swift/lock-swift-configuration-options#closable [2] https://developer.apple.com/documentation/uikit/uiviewcontroller/3229894-ismodalinpresentation
Call onCancel when the Lock view controller modal is dismissed by the user (by tapping outside its bounds on iPad). This is not called when it's dismissed programmatically: https://developer.apple.com/documentation/uikit/uiadaptivepresentationcontrollerdelegate/3229889-presentationcontrollerdiddismiss
agirault
changed the title
closable option behavior when Lock is presented as a popup
agirault
changed the title
Widcket
added
the
needs investigation
|
FYI @Widcket I'm still interested in pursuing this but had some issues running the tests locally. I should be able to spend a bit more time investigating early next year, though I'd appreciate any insight if you found the time to review. Ty! |
|
I found out the issue, the delegate needs to be set on Screen.Recording.2021-04-08.at.21.57.19.mov@cocojoe please review, as I made and pushed that change. |
agirault
commented
Apr 9, 2021
| @@ -138,6 +137,7 @@ public class LockViewController: UIViewController { | |||
| center.addObserver(self, selector: #selector(keyboardWasShown), name: UIResponder.responderKeyboardWillShowNotification, object: nil) | |||
| center.addObserver(self, selector: #selector(keyboardWasHidden), name: UIResponder.responderKeyboardWillHideNotification, object: nil) | |||
|
|
|||
| self.presentationController?.delegate = self | |||
There was a problem hiding this comment.
I don't get why it had to be in viewDidLoad instead of init: could you explain?
cocojoe
approved these changes
Apr 13, 2021
Widcket
added
CH: Fixed
bug
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issues
On iPad, the
LockViewControlleris presented as a.formSheet. As such, it can be dismissed by simply tapping outside the bounds of the modal. In this scenario, there are two issues:closableoption has no effect on iPad #658: TheclosableLock option has no effect if set tofalse(default): the modal will be dismissed either wayonCancelcallback not triggered when dismissing LockViewController on iPad #659: The developer gets no callbacks (onCancel) after that dismissalChanges
isModalInPresentationbased onclosableallows controlling whether or not UIKit should prevent the interactive dismissal of the modalpresentationControllerDidDismissallows us to trigger the cancel callback when the user manually dismissed the modalTesting
On iPad:
closableoption set to false (or not set, since that's the default), check that the lock modal does not get dismissed when tapping outside its bounds (used to fail before this change)closableoption set to true, check that the lock modal gets dismissed when tapping outside its bounds and that theonCancelcallback is triggered.Checklist