-
Notifications
You must be signed in to change notification settings - Fork 262
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
Fixing webviews not being cleared properly #718
Conversation
9b48ad9
to
ce02055
Compare
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.
LGTM nit comment
} | ||
navController.presentationController.delegate = self; |
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.
can you add a comment to explain this? for the future 😬
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.
This looks like a standard delegate setup, I am ok without having a comment in code
https://developer.apple.com/documentation/uikit/uipresentationcontroller/1618329-delegate
if (!viewControllerForPresentation.view.superview) | ||
[mainWindow addSubview:[viewControllerForPresentation view]]; | ||
if (!viewControllerForPresentation.view.superview) { | ||
[mainWindow addSubview:[viewControllerForPresentation view]]; |
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.
is this well formatted?
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.
ah yes, there is an extra space here
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.
Fixed this one!
} | ||
navController.presentationController.delegate = self; |
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.
This looks like a standard delegate setup, I am ok without having a comment in code
https://developer.apple.com/documentation/uikit/uipresentationcontroller/1618329-delegate
if (!viewControllerForPresentation.view.superview) | ||
[mainWindow addSubview:[viewControllerForPresentation view]]; | ||
if (!viewControllerForPresentation.view.superview) { | ||
[mainWindow addSubview:[viewControllerForPresentation view]]; |
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.
ah yes, there is an extra space here
ce02055
to
ccd0251
Compare
Webviews were not being cleared properly if multiple were queued in a row. #606
In
OneSignalWebView's
showInApp
method we were attempting to reusenavController
andviewControllerForPresentation
when opening webviews in the App. However thepresentationController delegate
lifecycle methods were not being properly called for the second webivew after removing theviewControllerForPresentation.view
from its superview.This was because upon clearing the
navController
its presentationController delegate would get reset, and we only set the delegate if thenavController
was nil. The fix is to always set thenavController.presentationController.delegate = self
This change is