[expo-cli][eject] Fix generated orientation in AndroidManifest #2431
Conversation
barthap
changed the title
| @@ -16,7 +24,7 @@ export async function setAndroidOrientation(config: ExpoConfig, manifestDocument | |||
| const mainActivity = manifestDocument.manifest.application[0].activity.filter( | |||
| (e: any) => e['$']['android:name'] === '.MainActivity' | |||
| ); | |||
| mainActivity[0]['$'][SCREEN_ORIENTATION_ATTRIBUTE] = orientation; | |||
| mainActivity[0]['$'][SCREEN_ORIENTATION_ATTRIBUTE] = prepareOrientation(orientation); | |||
There was a problem hiding this comment.
| mainActivity[0]['$'][SCREEN_ORIENTATION_ATTRIBUTE] = prepareOrientation(orientation); | |
| mainActivity[0]['$'][SCREEN_ORIENTATION_ATTRIBUTE] = orientation !== 'default' ? orientation : 'unspecified'; |
nit pick: prepareOrientation might be a bit misleading and implementation is simple enough to just put inline
There was a problem hiding this comment.
You're right - assuming that Expo accepts only 3 values: default, portrait and landscape, and 2 of them are valid manifest values, I can simplify that 😉 I extracted it to external function to make it easier to handle possible future extensions.
| @@ -7,6 +7,14 @@ export function getOrientation(config: ExpoConfig) { | |||
| return typeof config.orientation === 'string' ? config.orientation : null; | |||
| } | |||
|
|
|||
| export function prepareOrientation(orientation: string) { | |||
| if (orientation === 'default') { | |||
There was a problem hiding this comment.
maybe it's a stupid question, but why you are handling 'default' value differently than missing one?
There was a problem hiding this comment.
Just because the missing one is already handled when called this function
bbarthec
changed the title
|
@barthap could you please update the changelog? https://github.com/expo/expo-cli/blob/master/CHANGELOG.md |
|
|
Fixes #2429
Tested by just ejecting project and checking if orientation in manifest is correct. Also wrote 2 tests and ran
jest.