Skip to content

Commit d76192c

Browse files
authored
fix: Fix UI issue related to SafeAreaView top inset recalculation cp-7.73.0 (#28622)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> We're experiencing an issue where the SafeAreaView top padding gets recalculated upon component mount, which results in bad UI. This PR reduces the blast radius of the changes by consolidating all SafeAreaView imports to `react-native-safe-area-context`. We then create a shim for the module, which handles applying top insets via style instead. Changes also includes: - Consolidate safe area context mocks in test setup file - Remove unused safe area mocks within test files - Update snapshots ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: ## **Related issues** Fixes: ## **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** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I've included tests if applicable - [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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] > **Medium Risk** > Touches SafeAreaView usage across many screens, so incorrect inset/edge handling could cause widespread layout regressions on iOS/Android despite being mostly mechanical changes. > > **Overview** > Standardizes `SafeAreaView` usage by switching many components from `react-native` to `react-native-safe-area-context` (including adding explicit `edges` in a few places) to avoid incorrect top-inset recalculation on mount. > > Refactors navigation/header configuration for `DeFiProtocolPositionDetails` by moving `setOptions` logic out of the component and into `MainNavigator` options via `getDeFiProtocolPositionDetailsNavbarOptions` (now explicitly sets `headerShown: true`). > > Cleans up tests by removing per-file safe-area mocks in favor of the centralized jest mock in `testSetupView.js`, and updates snapshots/expectations accordingly. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit f88a0cf. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 894a971 commit d76192c

File tree

216 files changed

+946
-2321
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

216 files changed

+946
-2321
lines changed

app/component-library/components-temp/HeaderRoot/HeaderRoot.test.tsx

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@ import { IconName } from '@metamask/design-system-react-native';
99
// Internal dependencies.
1010
import HeaderRoot from './HeaderRoot';
1111

12-
jest.mock('react-native-safe-area-context', () => ({
13-
useSafeAreaInsets: () => ({ top: 44, bottom: 34, left: 0, right: 0 }),
14-
}));
15-
1612
const CONTAINER_TEST_ID = 'header-root-container';
1713
const LEFT_CHILDREN_TEST_ID = 'header-root-left-children';
1814
const END_ACCESSORY_TEST_ID = 'header-root-end-accessory';
@@ -250,7 +246,7 @@ describe('HeaderRoot', () => {
250246
expect(container.props.style).toEqual(
251247
expect.arrayContaining([
252248
expect.anything(),
253-
expect.objectContaining({ marginTop: 44 }),
249+
expect.objectContaining({ marginTop: expect.any(Number) }),
254250
]),
255251
);
256252
});

app/component-library/components-temp/HeaderStandardAnimated/HeaderStandardAnimated.test.tsx

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,6 @@ jest.mock('react-native-reanimated', () => {
1818
return Reanimated;
1919
});
2020

21-
jest.mock('react-native-safe-area-context', () => ({
22-
useSafeAreaInsets: () => ({ top: 44, bottom: 34, left: 0, right: 0 }),
23-
}));
24-
2521
const CONTAINER_TEST_ID = 'header-standard-animated-container';
2622
const TITLE_TEST_ID = 'header-standard-animated-title';
2723
const SUBTITLE_TEST_ID = 'header-standard-animated-subtitle';

app/component-library/components-temp/MultichainAccounts/MultichainAddWalletActions/MultichainAddWalletActions.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Third party dependencies.
22
import React, { Fragment, useCallback, useMemo } from 'react';
3-
import { SafeAreaView } from 'react-native';
3+
import { SafeAreaView } from 'react-native-safe-area-context';
44
import { useNavigation } from '@react-navigation/native';
55

66
// External dependencies.
@@ -85,7 +85,7 @@ const MultichainAddWalletActions = ({
8585
);
8686

8787
return (
88-
<SafeAreaView>
88+
<SafeAreaView edges={['bottom']}>
8989
<Fragment>
9090
{actionConfigs.map(
9191
(config) =>

app/component-library/components/BottomSheets/BottomSheet/foundation/BottomSheetDialog/BottomSheetDialog.test.tsx

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,6 @@ jest.mock('react-native', () => {
2121
};
2222
});
2323

24-
jest.mock('react-native-safe-area-context', () => {
25-
// using disting digits for mock rects to make sure they are not mixed up
26-
const inset = { top: 1, right: 2, bottom: 3, left: 4 };
27-
const frame = { width: 5, height: 6, x: 7, y: 8 };
28-
return {
29-
SafeAreaProvider: jest.fn().mockImplementation(({ children }) => children),
30-
SafeAreaConsumer: jest
31-
.fn()
32-
.mockImplementation(({ children }) => children(inset)),
33-
useSafeAreaInsets: jest.fn().mockImplementation(() => inset),
34-
useSafeAreaFrame: jest.fn().mockImplementation(() => frame),
35-
};
36-
});
37-
3824
jest.mock('@react-navigation/native', () => {
3925
const actualNav = jest.requireActual('@react-navigation/native');
4026
return {

app/component-library/components/Toast/Toast.test.tsx

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,6 @@ jest.mock('react-native-reanimated', () => ({
2525
}));
2626

2727
// Mock safe area context
28-
jest.mock('react-native-safe-area-context', () => ({
29-
useSafeAreaInsets: () => ({ bottom: 0, top: 0, left: 0, right: 0 }),
30-
}));
31-
3228
describe('Toast', () => {
3329
let toastRef: React.RefObject<ToastRef>;
3430

app/components/Base/InfoModal.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import React from 'react';
2-
import { StyleSheet, View, TouchableOpacity, SafeAreaView } from 'react-native';
2+
import { StyleSheet, View, TouchableOpacity } from 'react-native';
3+
import { SafeAreaView } from 'react-native-safe-area-context';
34
import Modal from 'react-native-modal';
45
import IonicIcon from 'react-native-vector-icons/Ionicons';
56
import Text from './Text';

app/components/Nav/App/App.test.tsx

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -453,21 +453,6 @@ describe('App', () => {
453453
};
454454

455455
beforeAll(() => {
456-
jest.mock('react-native-safe-area-context', () => {
457-
const inset = { top: 0, right: 0, bottom: 0, left: 0 };
458-
const frame = { width: 0, height: 0, x: 0, y: 0 };
459-
return {
460-
SafeAreaProvider: jest
461-
.fn()
462-
.mockImplementation(({ children }) => children),
463-
SafeAreaConsumer: jest
464-
.fn()
465-
.mockImplementation(({ children }) => children(inset)),
466-
useSafeAreaInsets: jest.fn().mockImplementation(() => inset),
467-
useSafeAreaFrame: jest.fn().mockImplementation(() => frame),
468-
};
469-
});
470-
471456
// Mock the storage item to simulate existing user and bypass onboarding
472457
jest.spyOn(StorageWrapper, 'getItem').mockImplementation(async (key) => {
473458
if (key === EXISTING_USER) {

app/components/Nav/Main/MainNavigator.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ import CampaignTourStepView from '../../UI/Rewards/Views/CampaignTourStepView';
142142
import { selectRewardsSubscriptionId } from '../../../selectors/rewards';
143143
import SitesFullView from '../../Views/SitesFullView/SitesFullView';
144144
import { TokenDetails } from '../../UI/TokenDetails/Views/TokenDetails';
145+
import { getDeFiProtocolPositionDetailsNavbarOptions } from '../../UI/Navbar';
145146

146147
const Stack = createStackNavigator();
147148
const Tab = createBottomTabNavigator();
@@ -1310,7 +1311,10 @@ const MainNavigator = () => {
13101311
<Stack.Screen
13111312
name="DeFiProtocolPositionDetails"
13121313
component={DeFiProtocolPositionDetails}
1313-
options={{ headerShown: true, ...slideFromRightAnimation }}
1314+
options={({ navigation }) => ({
1315+
...slideFromRightAnimation,
1316+
...getDeFiProtocolPositionDetailsNavbarOptions(navigation),
1317+
})}
13141318
/>
13151319
{
13161320
///: BEGIN:ONLY_INCLUDE_IF(sample-feature)

app/components/Nav/Main/__snapshots__/MainNavigator.test.tsx.snap

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -387,13 +387,7 @@ exports[`MainNavigator Tab Bar Visibility hides tab bar when browser is active 1
387387
<Screen
388388
component={[Function]}
389389
name="DeFiProtocolPositionDetails"
390-
options={
391-
{
392-
"animationEnabled": true,
393-
"cardStyleInterpolator": [Function],
394-
"headerShown": true,
395-
}
396-
}
390+
options={[Function]}
397391
/>
398392
<Screen
399393
component={[Function]}
@@ -816,13 +810,7 @@ exports[`MainNavigator Tab Bar Visibility shows tab bar when not in browser 1`]
816810
<Screen
817811
component={[Function]}
818812
name="DeFiProtocolPositionDetails"
819-
options={
820-
{
821-
"animationEnabled": true,
822-
"cardStyleInterpolator": [Function],
823-
"headerShown": true,
824-
}
825-
}
813+
options={[Function]}
826814
/>
827815
<Screen
828816
component={[Function]}
@@ -1245,13 +1233,7 @@ exports[`MainNavigator matches rendered snapshot 1`] = `
12451233
<Screen
12461234
component={[Function]}
12471235
name="DeFiProtocolPositionDetails"
1248-
options={
1249-
{
1250-
"animationEnabled": true,
1251-
"cardStyleInterpolator": [Function],
1252-
"headerShown": true,
1253-
}
1254-
}
1236+
options={[Function]}
12551237
/>
12561238
<Screen
12571239
component={[Function]}

app/components/UI/AccountNetworkIndicator/__snapshots__/AccountNetworkIndicator.test.tsx.snap

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ exports[`AccountNetworkIndicator should render correctly 1`] = `
88
}
99
}
1010
>
11-
<RNCSafeAreaProvider
12-
onInsetsChange={[Function]}
11+
<View
1312
style={
1413
[
1514
{
@@ -440,6 +439,6 @@ exports[`AccountNetworkIndicator should render correctly 1`] = `
440439
</RNSScreen>
441440
</RNSScreenContainer>
442441
</View>
443-
</RNCSafeAreaProvider>
442+
</View>
444443
</View>
445444
`;

0 commit comments

Comments
 (0)