Fix Notices showing anywhere but the bottom on iPad devices #11345
Conversation
This regression happened in 4cac38f. The `UntouchableWindow` rotation is fixed but this causes a bug in `MaskView` which will be fixed in a later commit.
The window.isHidden is enough to keep the auto-resize on iPads.
| @@ -70,6 +70,14 @@ class NoticePresenter: NSObject { | |||
|
|
|||
| super.init() | |||
|
|
|||
| // Keep the window visible but hide it on the next run loop. If we hide it immediately, | |||
| // the window is not automatically resized when the device is rotated. This issue | |||
| // only happens on iPad devices. | |||
There was a problem hiding this comment.
Can we change this comment to mention that the problem appears to be only in the simulator?
That way we can revisit it in the future if this code gets refactored.
| // Update the `frame` on the next run loop. When this Notification event handler is | ||
| // called, the `self.parentView` still has the frame from the previous orientation. | ||
| // Running this routine after the current run loop ensures we have the correct frame. | ||
| DispatchQueue.main.async { [weak self] in |
There was a problem hiding this comment.
This is just a suggestion but, what if we wrapped the DispatchQueue.main.async call in a method with a name that explained what was going on, such as
waitALoopSoRotationWorksOnSimulator() {
self.frame = MaskView.calculateFrame(parent: self.parentView, untouchableVC: self.untouchableVC)
}There was a problem hiding this comment.
I'm not sure about this. 😬I think it's already well contained in this method. Perhaps I can add a comment in line 73 to re-evaluate this method if the iPad simulator issue ever gets fixed.
There was a problem hiding this comment.
What made me think of it is that we're calling DispatchQueue.main.async twice, with the only hint that they're related being the comments. I like good comments, but I fear that they're fragile, ya know?
Anyway, still up to you 👍
We tested on both iOS 11 and 12 simulators. We tested on an iPad device with iOS 12 and this bug does not happen.
|
@nheagy Thank you for the thorough testing and review. I updated just the comment for now. 🙂 Please let me know if the comment is good enough. |
!
Fixes #11325.
The bug seems to consistently happen when you launch the app, show a
Notice, and then rotate the device.The Problem
Based on my tests, the
UntouchableViewController.viewnever gets automatically resized when the device is rotated. The first orientation that the app is launched in will determine the size of theview.This only happens on iPad devices. For iPhone, the
viewseems to be rotated correctly.The Solution
Based on @nheagy's advice, this is a regression. We hunted for the cause and found that this started happening in 4cac38f. It looks like the auto-resizing gets disabled when the
windowis not visible after it is initialized. This PR restores that. It will still be hidden but on the next UI loop.The
MaskViewgot affected with the fix so theMaskViewframeupdate had to be fixed as well.Testing
To reproduce the bug:
Please also play around with showing the Notice while on portrait.