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

[ScreenOrientation] iOS orientation lock doesn't work until backgrounded/foregrounded #11558

Closed
computerjazz opened this issue Jan 7, 2021 · 14 comments · Fixed by #14543
Closed

Comments

@computerjazz
Copy link
Contributor

🐛 Bug Report

Summary of Issue

In the bare workflow, Locking orientation doesn't work until the app has been backgrounded and foregrounded.

I traced this down to the fact that a new EXScreenOrientationModule module is instantiated every JS refresh. Since this module relies on native app foreground/background lifecycle methods to register itself, it was not getting registered until the user manually triggers an app foreground/background. Adding the following patch fixes the behavior, as it manually registers the module by calling moduleDidForeground but this does not seem semantically correct.

diff --git a/node_modules/expo-screen-orientation/ios/EXScreenOrientation/EXScreenOrientationModule.m b/node_modules/expo-screen-orientation/ios/EXScreenOrientation/EXScreenOrientationModule.m
index d4d92fc..44ce4a4 100644
--- a/node_modules/expo-screen-orientation/ios/EXScreenOrientation/EXScreenOrientationModule.m
+++ b/node_modules/expo-screen-orientation/ios/EXScreenOrientation/EXScreenOrientationModule.m
@@ -36,6 +36,11 @@ - (void)setModuleRegistry:(UMModuleRegistry *)moduleRegistry
   _eventEmitter = [moduleRegistry getModuleImplementingProtocol:@protocol(UMEventEmitterService)];
   
   _screenOrientationRegistry = [moduleRegistry getSingletonModuleForName:@"ScreenOrientationRegistry"];
+  // Workaround: every JS refresh a new module is instantiated and the old module is deallocated.
+  // onAppForegrounded is never called on the new module since the native app has not re-foregrounded.
+  // Because of this ScreenOreintationRegistry never thinks there is a "foregrounded" module.
+  // So we manually call moduleDidForeground here until this is fixed.
+  [_screenOrientationRegistry moduleDidForeground:self];
 }
 
 UM_EXPORT_METHOD_AS(lockAsync,

Is the module supposed to be a singleton?

Environment - output of expo diagnostics & the platform(s) you're targeting

Platform: iOS

  Expo CLI 4.0.17 environment info:
    System:
      OS: macOS 11.1
      Shell: 5.8 - /bin/zsh
    Binaries:
      Node: 15.4.0 - /usr/local/bin/node
      Yarn: 1.22.10 - /usr/local/bin/yarn
      npm: 7.0.15 - /usr/local/bin/npm
    Managers:
      CocoaPods: 1.10.0 - /usr/local/bin/pod
    SDKs:
      iOS SDK:
        Platforms: iOS 14.3, DriverKit 20.2, macOS 11.1, tvOS 14.3, watchOS 7.2
      Android SDK:
        API Levels: 28, 29, 30
        Build Tools: 28.0.3, 29.0.2, 30.0.3
        System Images: android-30 | Google APIs Intel x86 Atom
    IDEs:
      Android Studio: 4.1 AI-201.8743.12.41.6953283
      Xcode: 12.3/12C33 - /usr/bin/xcodebuild
    npmPackages:
      expo: ^39.0.0 => 39.0.3 
      react: 16.13.1 => 16.13.1 
      react-dom: 16.11.0 => 16.11.0 
      react-native: 0.63.3 => 0.63.3 
      react-native-web: ~0.11.7 => 0.11.7 
    npmGlobalPackages:
      expo-cli: 4.0.17
    Expo Workflow: bare

Reproducible Demo

https://github.com/computerjazz/expo-orientation-bug

Steps to Reproduce

  • Run app on iOS, make sure multiple orientations are allowed
  • Lock to a single orientation at the app root
  • Refresh JS
  • App no longer will lock

Expected Behavior vs Actual Behavior

Expected: Rotation lock continues to work after refreshing JS
Actual: App does not lock to orientation after refreshing JS

@computerjazz computerjazz added the needs validation Issue needs to be validated label Jan 7, 2021
@AdamJNavarro AdamJNavarro added iOS ScreenOrientation and removed needs validation Issue needs to be validated labels Jan 8, 2021
@GD55
Copy link

GD55 commented Mar 17, 2021

Facing the same issue did anyone found any resolution to the mentioned problem.

@iOrcohen
Copy link

iOrcohen commented Jul 1, 2021

Facing this error too after restart app using react-native-restart.

Any ideas ?

@richardwu
Copy link

richardwu commented Sep 4, 2021

Same issue! Would be interested in a fix (this seems to affect EAS builds/custom dev clients, so presumably standalone builds too).

@richardwu
Copy link

@computerjazz did you ever dig further into a more semantic fix? Any qualms if I try to upstream your temporary fix?

@Saifadin
Copy link

Saifadin commented Oct 2, 2021

@tsapeta thanks for fixing this. Any idea when this will be released?

@tsapeta
Copy link
Member

tsapeta commented Oct 2, 2021

Hi @Saifadin, the package is already published under next tag on npm and will be promoted to latest once we release SDK43 (pretty soon, we're finishing the QA). The new version will also require some changes in the unimodules setup — we're deprecating react-native-unimodules in favor of expo package, so I'd recommend waiting for our official blogpost where we will describe how to migrate it 😉

@Saifadin
Copy link

Saifadin commented Oct 6, 2021

@tsapeta Any ETA for SDK43 and the blogpost on how to migrate from unimodules? This is currently a big Blocker for our further Development.
I know how long QA can take sometimes 😣

@Saifadin
Copy link

Saifadin commented Oct 29, 2021

@tsapeta we just upgraded to SDK 43 and updated to expo-modules. But this error is still persisting and does not apply.
All settings for the bare workflow have been applied and we updated libraries.

This is the code we are using:

  useEffect(() => {
    const lockScreen = async () => {
      await ScreenOrientation.lockAsync(ScreenOrientation.OrientationLock.LANDSCAPE);
      const result = await ScreenOrientation.getOrientationAsync();
      // This logs 1, which is portrait
      console.log(result);
    };
    lockScreen();
    getPermissions();
    return () => ScreenOrientation.lockAsync(ScreenOrientation.OrientationLock.PORTRAIT_UP);
  }, []);

Edit:
I noticed a discrepancy in the expo-screen-orientation Docs number 3 which requires us to do the following in AppDelegate.h:

#import <UMCore/UMAppDelegateWrapper.h>

@interface AppDelegate : UMAppDelegateWrapper <RCTBridgeDelegate>

But in the Upgrade to expo-modules Guide it is supposed to be like this:

-#import <UMCore/UMAppDelegateWrapper.h>
+#import <Expo/Expo.h>

-@interface AppDelegate : UMAppDelegateWrapper <RCTBridgeDelegate, EXUpdatesAppControllerDelegate>
+@interface AppDelegate : EXAppDelegateWrapper <RCTBridgeDelegate>

@brycnguyen
Copy link

brycnguyen commented Oct 29, 2021

Currently facing this same issue on EAS with SDK43. The screen won't rotate to landscape right even after foregrounding/backgrounding and this is for a managed flow

@VahanLab
Copy link

VahanLab commented Sep 6, 2022

Any news on that one? Still facing the exact same issue. Works well within the Expo Go app, but fails with EAS builds

@GrahamEvans31
Copy link

GrahamEvans31 commented Jan 12, 2023

Encountering this now on EAS build with SDK 47, just on iOS landscape screen lock no longer engages. I just upgraded to EAS build from expo build and now this is an issue. Background/foreground doesn't matter

@fredrikburmester
Copy link

I have the same issue as @VahanLab

@GrahamEvans31
Copy link

GrahamEvans31 commented Jan 27, 2023

@fredrikburmester I added this to my ios in app.json:

    "UISupportedInterfaceOrientations": [
      "UIInterfaceOrientationLandscapeRight",
      "UIInterfaceOrientationLandscapeLeft",
      "UIInterfaceOrientationPortrait"
    ],
    "UISupportedInterfaceOrientations~ipad": [
      "UIInterfaceOrientationLandscapeRight",
      "UIInterfaceOrientationLandscapeLeft",
      "UIDeviceOrientationPortrait",
      "UIDeviceOrientationPortraitUpsideDown"
    ]

Then dropped the default (old) recommended config that was:

"orientation": "portrait",

Then it seemed to work fine for me, for anyone else I found this from digging for awhile through all the issues

@fredrikburmester
Copy link

fredrikburmester commented Jan 27, 2023

Thank you @GrahamEvans31 this worked! 👍🏻

To clarify for anyone who's having the same problem, this is where I put the code in my app.json file:

{
  "expo": {
    "ios": {
      "requiresFullScreen": true,
      "supportsTablet": true,
      "bundleIdentifier": "com.personalname.appname",
      "ITSAppUsesNonExemptEncryption": "NO",
      "UISupportedInterfaceOrientations": [
        "UIInterfaceOrientationLandscapeRight",
        "UIInterfaceOrientationLandscapeLeft",
        "UIInterfaceOrientationPortrait"
      ],
      "UISupportedInterfaceOrientations~ipad": [
        "UIInterfaceOrientationLandscapeRight",
        "UIInterfaceOrientationLandscapeLeft",
        "UIDeviceOrientationPortrait",
        "UIDeviceOrientationPortraitUpsideDown"
      ]
    },
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.