Skip to content
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

[screen-orientation][ios] Fix addOrientationChangeListener for iPadOS #23656

Merged
merged 5 commits into from
Jul 25, 2023

Conversation

behenate
Copy link
Member

@behenate behenate commented Jul 20, 2023

Why

addOrientationChangeListener would not work on iPadOS due to the traitCollectionDidChange not being called (both vertical and horizontal trait collections are the same on iPads).

closes #23387 ENG-9260

How

Refactored the ViewControllers to use viewWillTransition(to size: ... instead of traitCollectionDidChange(to traitCollection: UITraitCollection) as the source of the orientation events. viewWillTransition is correctly called on iPads and phones.
The difference in timing of the event is now marginally different. I measured less than 1ms of a difference between the old and new version (new version is always called later than the old one), so users shouldn't notice any side-effects.
This also allows us to remove some code that was necessary in cases when the traitCollectionDidChange wasn't called by the view controller.

Test Plan

Tested in Expo Go and bare expo on iOS 16, iPadOS 16 and iPadOS 15. Used a real iPad for iOS 16 tests and simulators for the rest.

@behenate behenate requested a review from tsapeta July 20, 2023 13:56
@behenate behenate self-assigned this Jul 20, 2023
@behenate behenate requested a review from lukmccall as a code owner July 20, 2023 13:56
@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Jul 20, 2023
@behenate behenate marked this pull request as draft July 20, 2023 14:00
@behenate behenate marked this pull request as ready for review July 20, 2023 14:10
Co-authored-by: Expo Bot <[email protected]>
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Jul 20, 2023
@watadarkstar
Copy link

watadarkstar commented Jul 24, 2023

@behenate @brentvatne Can we get a beta npm build for this? We are willing to test this PR for you guys as its blocking my client.

@tsapeta tsapeta merged commit 24857ee into main Jul 25, 2023
@tsapeta tsapeta deleted the @behenate/fix-23397-fix-orientation-listener-on-ipad branch July 25, 2023 09:01
@tsapeta tsapeta added the published Changes from the PR have been published to npm label Jul 25, 2023
@tsapeta
Copy link
Member

tsapeta commented Jul 25, 2023

@watadarkstar I've just published [email protected] – please let me know how it works 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[screen-orientation] addOrientationChangeListener not working for iPadOS
5 participants