Calculate Correct Window Dimensions for iOS #19932
Conversation
facebook-github-bot
added
the
CLA Signed
|
Hey @rdonnelly, I opened a similar PR yesterday (#19919) to fix Dimensions when the in-call status bar is active. I didn't think about split view so this is still useful. I'll see if I can get my PR reviewed soon and then you could rebase this on top of it. |
|
@janicduplessis Sounds wonderful! I subscribed your PR, so I'll keep an eye out. |
React/Modules/RCTDeviceInfo.m
Outdated
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Wdeprecated-declarations" | ||
| [_bridge.eventDispatcher sendDeviceEventWithName:@"didUpdateDimensions" | ||
| body:RCTExportedDimensions(_bridge)]; |
There was a problem hiding this comment.
It would be good to not call RCTExportedDimensions two times, right?
There was a problem hiding this comment.
Great point, I'll update. Thanks!
|
Any updates on when this is being merged? I'm working on an app right now that's due on the App Store by Tuesday and instead of doing something terribly messy, I'd like to just update my RN version to use this. |
|
@RoyalKingMomo even if this were to be merged today, it would not make a stable release for at least another month. In your case, you can patch this change into your own fork and manually cut a release. |
This comment has been minimized.
This comment has been minimized.
|
@shergin any chance you have time to review this? Or know who might have the time/expertise on this? Any help would be greatly appreciated! 😃 |
rdonnelly
force-pushed
the
master
branch
3 times, most recently
from
5cb6b07 to
fb9fbc1
Compare
rdonnelly
requested review from
grabbou,
hramos,
janicduplessis,
Kureev and
mhorowitz
as code owners
When the app becomes active, check to see if the window size has changed. If so, send a device event with update dimensions.
|
Apologies to those that I tagged for review yesterday. I was attempting to fix the failing checks, and at one point added some commits that auto-requested reviews. |
|
Any progress with this getting reviewed? We've got a workaround running but would be nice not to have to rely on it! |
|
any news no this? |
There was a problem hiding this comment.
@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@hramos Saw you imported this PR, not sure what that means for the process, but please let me know if there’s anything I can do to help. It’s been a while since I’ve looked at this. |
|
@rdonnelly looks like this failed to build internally, we'll need to make an update to land |
|
@rickhanlonii Is that anything I can do or help with? Do I just need to pull/rebase with master? Thanks! |
|
This pull request was successfully merged by @rdonnelly in 33b55cc. When will my fix make it into a release? | Upcoming Releases |
Summary: Fixes: facebook#16152 [iOS] [Fixed] - Pass back correct dimensions for application window in Dimensions module Pull Request resolved: facebook#19932 Reviewed By: cpojer Differential Revision: D14312906 Pulled By: PeteTheHeat fbshipit-source-id: aacb729c89862267465eefc3534c48d9d4b5d746
|
Hi, not sure if the PR above is meant to be a breaking change but I've posted an Issue on React Navigation repo - https://github.com/react-navigation/react-navigation/issues/6120 that is related to this change, specifically the dims being passed back as window appear to be different to those when used on initial render. Sorry - Not sure on history of the change so can't be sure which repo needs to be changed. But wanted to let you know about downstream impact. |
Fixes: #16152
Changelog:
[iOS] [Fixed] - Pass back correct dimensions for application window in Dimensions module
Test Plan:
Before:
On iOS, when using
Dimensions.get()for either screen or window, the same value for width and height would be passed back for both screen and window. These values both represent the width and height of the whole screen regardless of the size of the window.This behavior was similar for listening to the Dimension change event. The even would fire for orientation changes, but would not fire for changes to the window size (e.g. split view app resizing).
After:
On iOS, when the Dimensions module passes back dimensions for the screen, it uses the full screen dimensions. For window, it now uses just the dimensions of the window. This goes back to Objective C and uses
RCTKeyWindow.For event handling, the code watches for the application to become active (
UIApplicationDidBecomeActiveNotification) and then recalculates the window dimensions, compares it to the previous dimensions, and sends a change event if there was a change.For testing, I threw together a project. I pointed it to a branch on my forked version of react-native.
https://github.com/rdonnelly/WindowVsScreenRN
Please feel free to let me know if anything needs re-working.