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

[dev-client] support landscape orientation #18509

Merged
merged 5 commits into from
Aug 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/expo-dev-launcher/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

### 🎉 New features

- Add landscape orienation support. ([#18509](https://github.com/expo/expo/pull/18509)) by [@ajsmth](https://github.com/ajsmth)

### 🐛 Bug fixes

### 💡 Others
Expand Down
3 changes: 0 additions & 3 deletions packages/expo-dev-launcher/ios/EXDevLauncherController.m
Original file line number Diff line number Diff line change
Expand Up @@ -565,9 +565,6 @@ - (void)_initAppWithUrl:(NSURL *)appUrl bundleUrl:(NSURL *)bundleUrl manifest:(E

[self _ensureUserInterfaceStyleIsInSyncWithTraitEnv:self.window.rootViewController];

[[UIDevice currentDevice] setValue:@(orientation) forKey:@"orientation"];
[UIViewController attemptRotationToDeviceOrientation];
Comment on lines -568 to -569
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm curious about why this is no longer needed -- with this gone, what sets the orientation when it is supposed to be restricted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could be wrong but I believe this is set at the Info.plist / OS level. I'll double check but i'm pretty sure these lines don't do anything

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(those plist values are set during prebuild / config plugin step)

Copy link
Contributor

@esamelson esamelson Aug 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, I see, that makes sense. I do think there's some value from having a dev client (specifically) be able to respond dynamically to changing app.json values -- like, it would be nice if you could modify this app.json value and then see the effect right away in your dev client instead of having to make a new build. But if that is much trickier now then it probably isn't a huge deal for this particular field.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll see if the app.json modifications work w/o rebuild - if so i don't see any harm in including it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like this only works on initial launch (i.e it will rotate to the configured orientation on launch) but the orientation is not locked unless its set in the build settings. it makes more sense to me to omit these lines


if (backgroundColor) {
self.window.rootViewController.view.backgroundColor = backgroundColor;
self.window.backgroundColor = backgroundColor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import UIKit
@objc
public class EXDevLauncherManifestHelper: NSObject {
private static func defaultOrientationForOrientationMask(_ orientationMask: UIInterfaceOrientationMask) -> UIInterfaceOrientation {
if orientationMask.contains(.portrait) {
if orientationMask.contains(.all) {
return UIInterfaceOrientation.unknown
} else if orientationMask.contains(.portrait) {
return UIInterfaceOrientation.portrait
} else if orientationMask.contains(.landscapeLeft) {
return UIInterfaceOrientation.landscapeLeft
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,12 @@ public class ExpoDevLauncherReactDelegateHandler: ExpoReactDelegateHandler, RCTB
let rootView = RCTRootView(bridge: bridge!, moduleName: self.rootViewModuleName!, initialProperties: self.rootViewInitialProperties)
rootView.backgroundColor = self.deferredRootView?.backgroundColor ?? UIColor.white
let window = getWindow()
window.rootViewController = self.reactDelegate?.createRootViewController()
window.rootViewController!.view = rootView

// NOTE: this order of assignment seems to actually have an effect on behaviour
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keith-kurak this appears to have been the root issue - pretty underwhelming!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WOW, I could have looked at that for years, what a find!

// direct assignment of window.rootViewController.view = rootView does not work
let rootViewController = self.reactDelegate?.createRootViewController()
rootViewController!.view = rootView
window.rootViewController = rootViewController
window.makeKeyAndVisible()

// it is purposeful that we don't clean up saved properties here, because we may initialize
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ class EXDevLauncherManifestHelperTests: XCTestCase {
func testExportManifestOrientation() {
XCTAssertEqual(UIInterfaceOrientation.portrait, EXDevLauncherManifestHelper.exportManifestOrientation("portrait"))
XCTAssertEqual(UIInterfaceOrientation.landscapeLeft, EXDevLauncherManifestHelper.exportManifestOrientation("landscape"))
XCTAssertEqual(UIInterfaceOrientation.portrait, EXDevLauncherManifestHelper.exportManifestOrientation("default"))
XCTAssertEqual(UIInterfaceOrientation.portrait, EXDevLauncherManifestHelper.exportManifestOrientation("unsupported-value"))
XCTAssertEqual(UIInterfaceOrientation.unknown, EXDevLauncherManifestHelper.exportManifestOrientation("default"))
XCTAssertEqual(UIInterfaceOrientation.unknown, EXDevLauncherManifestHelper.exportManifestOrientation("unsupported-value"))
}

func testExportManifestUserInterfaceStyle() {
Expand Down
2 changes: 2 additions & 0 deletions packages/expo-dev-menu/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

### 🎉 New features

- Add landscape orienation support. ([#18509](https://github.com/expo/expo/pull/18509)) by [@ajsmth](https://github.com/ajsmth)

### 🐛 Bug fixes

### 💡 Others
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,18 +80,6 @@ class DevMenuActivity : ReactActivity() {
}
}

override fun onCreate(savedInstanceState: Bundle?) {
// Due to a bug in API 26, we can't set the orientation in translucent activity.
// See https://stackoverflow.com/questions/48072438/java-lang-illegalstateexception-only-fullscreen-opaque-activities-can-request-o
requestedOrientation = if (Build.VERSION.SDK_INT == Build.VERSION_CODES.O) {
ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED
} else {
ActivityInfo.SCREEN_ORIENTATION_PORTRAIT
}

super.onCreate(savedInstanceState)
}

override fun onKeyUp(keyCode: Int, event: KeyEvent): Boolean {
return if (keyCode == KeyEvent.KEYCODE_MENU || DevMenuManager.onKeyEvent(keyCode, event)) {
DevMenuManager.closeMenu()
Expand Down
17 changes: 11 additions & 6 deletions packages/expo-dev-menu/app/components/BottomSheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ type Props = {
innerGestureHandlerRefs: [React.RefObject<PanGestureHandler>, React.RefObject<TapGestureHandler>];

animationEnabled?: boolean;

screenHeight: number;
};

type State = {
Expand All @@ -77,8 +79,6 @@ type State = {
heightOfContent: Animated.Value<number>;
};

const { height: screenHeight } = Dimensions.get('window');

const P = <T extends any>(android: T, ios: T): T => (Platform.OS === 'ios' ? ios : android);

const magic = {
Expand Down Expand Up @@ -532,7 +532,10 @@ export class BottomSheet extends React.Component<Props, State> {
this.state.heightOfContent.setValue(height - this.state.initSnap);
};

static renumber = (str: string) => (Number(str.split('%')[0]) * screenHeight) / 100;
static renumber = (str: string, screenHeight: number) => {
const result = (Number(str.split('%')[0]) * screenHeight) / 100;
return result;
};

static getDerivedStateFromProps(props: Props, state: State | undefined): State {
let snapPoints;
Expand All @@ -551,7 +554,7 @@ export class BottomSheet extends React.Component<Props, State> {
if (typeof s === 'number') {
return { val: s, ind: i };
} else if (typeof s === 'string') {
return { val: BottomSheet.renumber(s), ind: i };
return { val: BottomSheet.renumber(s, props.screenHeight), ind: i };
}

throw new Error(`Invalid type for value ${s}: ${typeof s}`);
Expand Down Expand Up @@ -690,8 +693,10 @@ const styles = StyleSheet.create({
},
container: {
overflow: 'hidden',
borderTopLeftRadius: 10,
borderTopRightRadius: 10,
borderRadius: 10,
maxWidth: 525,
width: '100%',
alignSelf: 'center',
},
fullscreenView: {
width: '100%',
Expand Down
1 change: 1 addition & 0 deletions packages/expo-dev-menu/app/components/Main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ export function Main({ registeredCallbacks = [] }: MainProps) {
</Row>
</Button.ScaleOnPressContainer>
</View>
<Spacer.Vertical size="large" />
</View>
);
}
Expand Down
5 changes: 4 additions & 1 deletion packages/expo-dev-menu/app/hooks/useBottomSheet.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { View, StyleSheet, Pressable } from 'react-native';
import { View, StyleSheet, Pressable, useWindowDimensions } from 'react-native';
import Animated from 'react-native-reanimated';

import { BottomSheet } from '../components/BottomSheet';
Expand Down Expand Up @@ -55,6 +55,8 @@ export function BottomSheetProvider({ children }: BottomSheetProviderProps) {
outputRange: [0, 0.5],
});

const { height: screenHeight } = useWindowDimensions();

return (
<BottomSheetContext.Provider value={{ collapse }}>
<View style={styles.container}>
Expand All @@ -69,6 +71,7 @@ export function BottomSheetProvider({ children }: BottomSheetProviderProps) {
/>
</Pressable>
<BottomSheet
screenHeight={screenHeight}
ref={bottomSheetRef}
callbackNode={callbackNode.current}
snapPoints={snapPoints}>
Expand Down
14 changes: 1 addition & 13 deletions packages/expo-dev-menu/ios/DevMenuViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,9 @@ class DevMenuViewController: UIViewController {
reactRootView?.becomeFirstResponder()
}

override var shouldAutorotate: Bool {
get {
return true
}
}

override var supportedInterfaceOrientations: UIInterfaceOrientationMask {
get {
return UIInterfaceOrientationMask.portrait
}
}

override var preferredInterfaceOrientationForPresentation: UIInterfaceOrientation {
get {
return UIInterfaceOrientation.portrait
return UIInterfaceOrientationMask.all
}
}

Expand Down