Skip to content

Commit a5d99f5

Browse files
authored
fix: Scope landscape mode to perps chart cp-7.61.0 (#23947)
## **Description** This PR introduces a service to manage the horizontal orientation of the mobile application programatically instead of being global. ## **Changelog** CHANGELOG entry: horizontal orientation service ## **Related issues** Fixes: #23903 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** Uploading Screen Recording 2025-12-11 at 2.46.30 PM.mov… ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Centralizes orientation control, locking app to portrait and enabling landscape solely for the perps fullscreen chart, with updated modal logic and tests. > > - **Core**: > - **ScreenOrientation**: Add `ScreenOrientationService` (`lockToPortrait`, `allowLandscape`, `isLockedToPortrait`) with exports and constants; introduce `useScreenOrientation` hook. > - **App Init**: Lock orientation to portrait on startup in `components/Views/Root/index.tsx`. > - **Perps**: > - **Fullscreen Modal**: Refactor `PerpsChartFullscreenModal` to use `useScreenOrientation({ allowLandscape: isVisible })`; simplify close/error handlers (orientation handled by hook). > - **Tests**: > - Add unit tests for `ScreenOrientationService` and `useScreenOrientation`. > - Update modal tests to reflect hook-managed orientation and ensure `onClose` behavior; mock `Logger`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5b75bee. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent ff510cc commit a5d99f5

File tree

9 files changed

+377
-88
lines changed

9 files changed

+377
-88
lines changed

app/components/UI/Perps/components/PerpsChartFullscreenModal/PerpsChartFullscreenModal.test.tsx

Lines changed: 14 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
} from '../../../../../../e2e/selectors/Perps/Perps.selectors';
1717

1818
jest.mock('expo-screen-orientation');
19+
jest.mock('../../../../../util/Logger');
1920

2021
const mockLockAsync = lockAsync as jest.MockedFunction<typeof lockAsync>;
2122
const mockUnlockAsync = unlockAsync as jest.MockedFunction<typeof unlockAsync>;
@@ -254,7 +255,7 @@ describe('PerpsChartFullscreenModal', () => {
254255
});
255256
});
256257

257-
it('locks orientation when close button is pressed', async () => {
258+
it('calls onClose when close button is pressed (orientation locked via visibility change)', async () => {
258259
const { getByTestId } = render(
259260
<PerpsChartFullscreenModal {...defaultProps} isVisible />,
260261
);
@@ -265,8 +266,10 @@ describe('PerpsChartFullscreenModal', () => {
265266
);
266267
});
267268

269+
// onClose is called, which triggers isVisible to become false,
270+
// which causes the hook to lock orientation
268271
await waitFor(() => {
269-
expect(mockLockAsync).toHaveBeenCalledWith(OrientationLock.PORTRAIT_UP);
272+
expect(mockOnClose).toHaveBeenCalled();
270273
});
271274
});
272275

@@ -275,6 +278,11 @@ describe('PerpsChartFullscreenModal', () => {
275278
<PerpsChartFullscreenModal {...defaultProps} isVisible />,
276279
);
277280

281+
// Wait for the initial effect to complete before unmounting
282+
await waitFor(() => {
283+
expect(mockUnlockAsync).toHaveBeenCalled();
284+
});
285+
278286
unmount();
279287

280288
await waitFor(() => {
@@ -369,7 +377,7 @@ describe('PerpsChartFullscreenModal', () => {
369377
});
370378
});
371379

372-
it('locks orientation before calling onClose', async () => {
380+
it('calls onClose immediately when pressed (orientation managed by hook)', async () => {
373381
const { getByTestId } = render(
374382
<PerpsChartFullscreenModal {...defaultProps} isVisible />,
375383
);
@@ -380,8 +388,9 @@ describe('PerpsChartFullscreenModal', () => {
380388
);
381389
});
382390

391+
// onClose is called immediately; orientation locking happens
392+
// via the useScreenOrientation hook when isVisible changes
383393
await waitFor(() => {
384-
expect(mockLockAsync).toHaveBeenCalledWith(OrientationLock.PORTRAIT_UP);
385394
expect(mockOnClose).toHaveBeenCalled();
386395
});
387396
});
@@ -458,40 +467,11 @@ describe('PerpsChartFullscreenModal', () => {
458467
mockOnError();
459468
});
460469

470+
// onClose is called on error; orientation is restored via hook when isVisible changes
461471
await waitFor(() => {
462-
expect(mockLockAsync).toHaveBeenCalledWith(OrientationLock.PORTRAIT_UP);
463472
expect(mockOnClose).toHaveBeenCalled();
464473
});
465474
});
466-
467-
it('restores orientation lock when chart error occurs', async () => {
468-
// eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires
469-
const ComponentErrorBoundary = require('../../../ComponentErrorBoundary');
470-
const mockOnError = jest.fn();
471-
472-
ComponentErrorBoundary.mockImplementationOnce(
473-
({
474-
onError,
475-
children,
476-
}: {
477-
onError: () => void;
478-
children: React.ReactNode;
479-
}) => {
480-
mockOnError.mockImplementation(onError);
481-
return children;
482-
},
483-
);
484-
485-
render(<PerpsChartFullscreenModal {...defaultProps} isVisible />);
486-
487-
await act(async () => {
488-
mockOnError();
489-
});
490-
491-
await waitFor(() => {
492-
expect(mockLockAsync).toHaveBeenCalledWith(OrientationLock.PORTRAIT_UP);
493-
});
494-
});
495475
});
496476

497477
describe('Edge Cases', () => {

app/components/UI/Perps/components/PerpsChartFullscreenModal/PerpsChartFullscreenModal.tsx

Lines changed: 12 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,6 @@ import React, { useCallback, useEffect, useState, useRef } from 'react';
22
import { View, Dimensions } from 'react-native';
33
import Modal from 'react-native-modal';
44
import { useSafeAreaInsets } from 'react-native-safe-area-context';
5-
import {
6-
lockAsync,
7-
unlockAsync,
8-
OrientationLock,
9-
} from 'expo-screen-orientation';
105
import { useStyles } from '../../../../../component-library/hooks';
116
import ButtonIcon, {
127
ButtonIconSizes,
@@ -26,6 +21,7 @@ import PerpsCandlestickChartIntervalSelector from '../PerpsCandlestickChartInter
2621
import { styleSheet } from './PerpsChartFullscreenModal.styles';
2722
import PerpsOHLCVBar from '../PerpsOHLCVBar';
2823
import ComponentErrorBoundary from '../../../ComponentErrorBoundary';
24+
import { useScreenOrientation } from '../../../../../core/ScreenOrientation';
2925

3026
export interface PerpsChartFullscreenModalProps {
3127
isVisible: boolean;
@@ -60,34 +56,9 @@ const PerpsChartFullscreenModal: React.FC<PerpsChartFullscreenModalProps> = ({
6056
// Track OHLCV bar height to subtract from chart height
6157
const [ohlcvHeight, setOhlcvHeight] = useState<number>(0);
6258

63-
// Auto-follow device orientation when modal is open
64-
useEffect(() => {
65-
const handleOrientationChange = async () => {
66-
try {
67-
if (isVisible) {
68-
// Unlock orientation to follow device
69-
await unlockAsync();
70-
} else {
71-
// Lock back to portrait when closing
72-
await lockAsync(OrientationLock.PORTRAIT_UP);
73-
}
74-
} catch (error) {
75-
// Silent error handling - orientation lock failures are non-critical
76-
}
77-
};
78-
79-
handleOrientationChange();
80-
81-
// Cleanup only if component unmounts while modal is visible
82-
// No need to lock again on visibility change as it's handled above
83-
return () => {
84-
if (isVisible) {
85-
lockAsync(OrientationLock.PORTRAIT_UP).catch(() => {
86-
// Silent error handling
87-
});
88-
}
89-
};
90-
}, [isVisible]);
59+
// Allow landscape orientation when modal is visible
60+
// Automatically locks back to portrait when modal closes or unmounts
61+
useScreenOrientation({ allowLandscape: isVisible });
9162

9263
// Reset OHLCV height when OHLCV bar disappears
9364
useEffect(() => {
@@ -116,29 +87,16 @@ const PerpsChartFullscreenModal: React.FC<PerpsChartFullscreenModalProps> = ({
11687
}
11788
}, [candleData, selectedInterval, visibleCandleCount]);
11889

119-
const handleClose = useCallback(async () => {
120-
try {
121-
// Lock orientation back to portrait before closing
122-
await lockAsync(OrientationLock.PORTRAIT_UP);
123-
} catch (error) {
124-
// Silent error handling - orientation lock failures are non-critical
125-
} finally {
126-
// Always call onClose even if orientation lock fails
127-
onClose();
128-
}
90+
// Close handler - orientation is automatically restored by the hook
91+
// when isVisible becomes false
92+
const handleClose = useCallback(() => {
93+
onClose();
12994
}, [onClose]);
13095

131-
// Handle chart errors by restoring orientation and closing modal
132-
const handleChartError = useCallback(async () => {
133-
try {
134-
// Restore orientation lock on error to prevent getting stuck
135-
await lockAsync(OrientationLock.PORTRAIT_UP);
136-
} catch (error) {
137-
// Silent error handling - orientation lock failures are non-critical
138-
} finally {
139-
// Close modal even if orientation lock fails
140-
onClose();
141-
}
96+
// Handle chart errors by closing modal
97+
// Orientation is automatically restored by the hook
98+
const handleChartError = useCallback(() => {
99+
onClose();
142100
}, [onClose]);
143101

144102
return (

app/components/Views/Root/index.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import NavigationProvider from '../../Nav/NavigationProvider';
1515
import ControllersGate from '../../Nav/ControllersGate';
1616
import { isTest } from '../../../util/test/utils';
1717
import { FeatureFlagOverrideProvider } from '../../../contexts/FeatureFlagOverrideContext';
18+
import { ScreenOrientationService } from '../../../core/ScreenOrientation';
1819
///: BEGIN:ONLY_INCLUDE_IF(preinstalled-snaps,external-snaps)
1920
import { SnapsExecutionWebView } from '../../../lib/snaps';
2021
///: END:ONLY_INCLUDE_IF
@@ -50,6 +51,8 @@ const Root = ({ foxCode }: RootProps) => {
5051
SecureKeychain.init(foxCode);
5152
// Init EntryScriptWeb3 asynchronously on the background
5253
EntryScriptWeb3.init();
54+
// Lock screen orientation to portrait on app start
55+
ScreenOrientationService.lockToPortrait();
5356
// Wait for store to be initialized in Detox tests
5457
if (isTest) {
5558
waitForStore();
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import { ScreenOrientationService } from './ScreenOrientationService';
2+
import {
3+
lockAsync,
4+
unlockAsync,
5+
OrientationLock,
6+
} from 'expo-screen-orientation';
7+
8+
jest.mock('expo-screen-orientation');
9+
jest.mock('../../util/Logger');
10+
11+
const mockLockAsync = lockAsync as jest.MockedFunction<typeof lockAsync>;
12+
const mockUnlockAsync = unlockAsync as jest.MockedFunction<typeof unlockAsync>;
13+
14+
describe('ScreenOrientationService', () => {
15+
beforeEach(() => {
16+
jest.clearAllMocks();
17+
mockLockAsync.mockResolvedValue(undefined);
18+
mockUnlockAsync.mockResolvedValue(undefined);
19+
});
20+
21+
describe('lockToPortrait', () => {
22+
it('calls lockAsync with PORTRAIT_UP', async () => {
23+
await ScreenOrientationService.lockToPortrait();
24+
25+
expect(mockLockAsync).toHaveBeenCalledWith(OrientationLock.PORTRAIT_UP);
26+
});
27+
28+
it('sets isLocked to true after successful lock', async () => {
29+
await ScreenOrientationService.lockToPortrait();
30+
31+
expect(ScreenOrientationService.isLockedToPortrait()).toBe(true);
32+
});
33+
34+
it('handles lock errors gracefully', async () => {
35+
mockLockAsync.mockRejectedValueOnce(new Error('Lock failed'));
36+
37+
await expect(
38+
ScreenOrientationService.lockToPortrait(),
39+
).resolves.not.toThrow();
40+
});
41+
});
42+
43+
describe('allowLandscape', () => {
44+
it('calls unlockAsync', async () => {
45+
await ScreenOrientationService.allowLandscape();
46+
47+
expect(mockUnlockAsync).toHaveBeenCalled();
48+
});
49+
50+
it('sets isLocked to false after successful unlock', async () => {
51+
await ScreenOrientationService.lockToPortrait();
52+
await ScreenOrientationService.allowLandscape();
53+
54+
expect(ScreenOrientationService.isLockedToPortrait()).toBe(false);
55+
});
56+
57+
it('handles unlock errors gracefully', async () => {
58+
mockUnlockAsync.mockRejectedValueOnce(new Error('Unlock failed'));
59+
60+
await expect(
61+
ScreenOrientationService.allowLandscape(),
62+
).resolves.not.toThrow();
63+
});
64+
});
65+
66+
describe('isLockedToPortrait', () => {
67+
it('returns false initially', async () => {
68+
// Reset state by allowing landscape first
69+
await ScreenOrientationService.allowLandscape();
70+
71+
expect(ScreenOrientationService.isLockedToPortrait()).toBe(false);
72+
});
73+
74+
it('returns true after locking to portrait', async () => {
75+
await ScreenOrientationService.lockToPortrait();
76+
77+
expect(ScreenOrientationService.isLockedToPortrait()).toBe(true);
78+
});
79+
80+
it('returns false after allowing landscape', async () => {
81+
await ScreenOrientationService.lockToPortrait();
82+
await ScreenOrientationService.allowLandscape();
83+
84+
expect(ScreenOrientationService.isLockedToPortrait()).toBe(false);
85+
});
86+
});
87+
});
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import {
2+
lockAsync,
3+
unlockAsync,
4+
OrientationLock,
5+
} from 'expo-screen-orientation';
6+
import Logger from '../../util/Logger';
7+
8+
/**
9+
* ScreenOrientationService provides centralized control over screen orientation.
10+
*
11+
* By default, the app is locked to portrait mode. Specific screens can opt-in
12+
* to allow landscape orientation using the provided methods.
13+
*
14+
* @example
15+
* // Lock app to portrait on startup
16+
* await ScreenOrientationService.lockToPortrait();
17+
*
18+
* // Allow landscape for a specific screen
19+
* await ScreenOrientationService.allowLandscape();
20+
*
21+
* // Lock back to portrait when leaving the screen
22+
* await ScreenOrientationService.lockToPortrait();
23+
*/
24+
export class ScreenOrientationService {
25+
private static isLocked = false;
26+
27+
/**
28+
* Locks the screen orientation to portrait mode.
29+
* This should be called on app startup and when leaving screens that allow landscape.
30+
*/
31+
static async lockToPortrait(): Promise<void> {
32+
try {
33+
await lockAsync(OrientationLock.PORTRAIT_UP);
34+
this.isLocked = true;
35+
} catch (error) {
36+
// Silent error handling - orientation lock failures are non-critical
37+
Logger.log('ScreenOrientationService: Failed to lock to portrait', error);
38+
}
39+
}
40+
41+
/**
42+
* Unlocks the screen orientation to allow landscape mode.
43+
* The device orientation will follow the physical device position.
44+
*/
45+
static async allowLandscape(): Promise<void> {
46+
try {
47+
await unlockAsync();
48+
this.isLocked = false;
49+
} catch (error) {
50+
// Silent error handling - orientation unlock failures are non-critical
51+
Logger.log('ScreenOrientationService: Failed to allow landscape', error);
52+
}
53+
}
54+
55+
/**
56+
* Returns whether the orientation is currently locked to portrait.
57+
*/
58+
static isLockedToPortrait(): boolean {
59+
return this.isLocked;
60+
}
61+
}
62+
63+
export default ScreenOrientationService;
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
/**
2+
* Screen identifiers that are allowed to display in landscape orientation.
3+
* Add screen identifiers here to enable landscape support for specific screens.
4+
*/
5+
export const LANDSCAPE_ALLOWED_SCREENS = {
6+
PERPS_CHART_FULLSCREEN: 'PerpsChartFullscreenModal',
7+
} as const;
8+
9+
export type LandscapeAllowedScreen =
10+
(typeof LANDSCAPE_ALLOWED_SCREENS)[keyof typeof LANDSCAPE_ALLOWED_SCREENS];
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
export { ScreenOrientationService } from './ScreenOrientationService';
2+
export {
3+
LANDSCAPE_ALLOWED_SCREENS,
4+
type LandscapeAllowedScreen,
5+
} from './constants';
6+
export { useScreenOrientation } from './useScreenOrientation';

0 commit comments

Comments
 (0)