Skip to content

Commit 1d58bc0

Browse files
authored
refactor: remove ScreenComponent any casts for RootModalFlow security/misc (#28185)
## **Description** Migrate 5 RootModalFlow security/misc components from prop-based route access to `useRoute()` hook (PR 9 of 13). **Components:** SRPQuiz, OriginSpamModal, ChangeInSimulationModal, LearnMoreBottomSheet, ReturnToAppNotification ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** N/A — pure refactoring. All existing tests pass. ## **Screenshots/Recordings** ### **Before** N/A ### **After** N/A ## **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. Made with [Cursor](https://cursor.com) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk refactor limited to navigation param plumbing and test updates; behavior should be unchanged aside from stricter typing and slightly different test rendering expectations. > > **Overview** > Refactors several RootModalFlow security/misc screens (`SRPQuiz`, `OriginSpamModal`, `ChangeInSimulationModal`, `ReturnToAppNotification`) to read navigation params via `useRoute()` instead of prop-injected `route`, and removes the `as ScreenComponent` casts for these routes in `App.tsx`. > > Makes `LearnMoreBottomSheet`’s `onClose` prop optional and updates affected unit tests to mock `useRoute()` rather than passing `route` props; `ReturnToAppNotification` tests were expanded from a snapshot to detailed assertions around toast display, delays (`wait`), and one-time execution. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit fc3d334. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent b2f5730 commit 1d58bc0

11 files changed

Lines changed: 431 additions & 148 deletions

File tree

app/components/Nav/App/App.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ const RootModalFlow = (props: RootModalFlowProps) => (
571571
}
572572
<Stack.Screen
573573
name={Routes.MODAL.SRP_REVEAL_QUIZ}
574-
component={SRPQuiz as ScreenComponent}
574+
component={SRPQuiz}
575575
initialParams={{ ...props.route.params }}
576576
/>
577577
<Stack.Screen
@@ -607,11 +607,11 @@ const RootModalFlow = (props: RootModalFlowProps) => (
607607
/>
608608
<Stack.Screen
609609
name={Routes.SHEET.ORIGIN_SPAM_MODAL}
610-
component={OriginSpamModal as ScreenComponent}
610+
component={OriginSpamModal}
611611
/>
612612
<Stack.Screen
613613
name={Routes.SHEET.CHANGE_IN_SIMULATION_MODAL}
614-
component={ChangeInSimulationModal as ScreenComponent}
614+
component={ChangeInSimulationModal}
615615
/>
616616
<Stack.Screen name={Routes.SHEET.TOOLTIP_MODAL} component={TooltipModal} />
617617
<Stack.Screen
@@ -625,7 +625,7 @@ const RootModalFlow = (props: RootModalFlowProps) => (
625625
/>
626626
<Stack.Screen
627627
name={Routes.MODAL.MULTICHAIN_ACCOUNTS_LEARN_MORE}
628-
component={LearnMoreBottomSheet as ScreenComponent}
628+
component={LearnMoreBottomSheet}
629629
options={{ headerShown: false }}
630630
/>
631631
<Stack.Screen
@@ -634,7 +634,7 @@ const RootModalFlow = (props: RootModalFlowProps) => (
634634
/>
635635
<Stack.Screen
636636
name={Routes.SDK.RETURN_TO_DAPP_NOTIFICATION}
637-
component={ReturnToAppNotification as ScreenComponent}
637+
component={ReturnToAppNotification}
638638
initialParams={{ ...props.route.params }}
639639
/>
640640
<Stack.Screen

app/components/Views/ChangeInSimulationModal/ChangeInSimulationModal.test.tsx

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ import ChangeInSimulationModal, {
1111
} from './ChangeInSimulationModal';
1212
import { RootState } from '../../../reducers';
1313

14+
let mockRouteParams: { onProceed: jest.Mock; onReject: jest.Mock } = {
15+
onProceed: jest.fn(),
16+
onReject: jest.fn(),
17+
};
18+
1419
jest.mock('react-redux', () => ({
1520
...jest.requireActual('react-redux'),
1621
useDispatch: () => jest.fn(),
@@ -22,12 +27,12 @@ jest.mock(
2227
({ children }: { children: React.ReactElement }) => <>{children}</>,
2328
);
2429

25-
const NAVIGATION_PARAMS_MOCK = {
26-
params: {
27-
onProceed: jest.fn(),
28-
onReject: jest.fn(),
29-
},
30-
};
30+
jest.mock('@react-navigation/native', () => ({
31+
...jest.requireActual('@react-navigation/native'),
32+
useRoute: () => ({
33+
params: mockRouteParams,
34+
}),
35+
}));
3136

3237
const mockInitialState: DeepPartial<RootState> = {
3338
engine: {
@@ -40,29 +45,26 @@ const mockInitialState: DeepPartial<RootState> = {
4045
describe('ChangeInSimulationModal', () => {
4146
beforeEach(() => {
4247
jest.clearAllMocks();
48+
mockRouteParams = {
49+
onProceed: jest.fn(),
50+
onReject: jest.fn(),
51+
};
4352
});
4453

4554
it('render matches snapshot', () => {
46-
const { toJSON } = renderWithProvider(
47-
<ChangeInSimulationModal route={NAVIGATION_PARAMS_MOCK} />,
48-
{
49-
state: mockInitialState,
50-
},
51-
);
55+
const { toJSON } = renderWithProvider(<ChangeInSimulationModal />, {
56+
state: mockInitialState,
57+
});
5258
expect(toJSON()).toMatchSnapshot();
5359
});
5460

5561
it('calls onProceed and onReject callbacks', () => {
5662
const mockOnReject = jest.fn();
5763
const mockOnProceed = jest.fn();
58-
const wrapper = renderWithProvider(
59-
<ChangeInSimulationModal
60-
route={{ params: { onProceed: mockOnProceed, onReject: mockOnReject } }}
61-
/>,
62-
{
63-
state: mockInitialState,
64-
},
65-
);
64+
mockRouteParams = { onProceed: mockOnProceed, onReject: mockOnReject };
65+
const wrapper = renderWithProvider(<ChangeInSimulationModal />, {
66+
state: mockInitialState,
67+
});
6668
fireEvent.press(wrapper.getByTestId(PROCEED_BUTTON_TEST_ID));
6769
expect(mockOnProceed).toHaveBeenCalledTimes(1);
6870

app/components/Views/ChangeInSimulationModal/ChangeInSimulationModal.tsx

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import React, { useCallback, useRef } from 'react';
2+
import { RouteProp, useRoute } from '@react-navigation/native';
23
import { StyleSheet, View } from 'react-native';
34
import { strings } from '../../../../locales/i18n';
45
import BottomSheet, {
@@ -38,11 +39,16 @@ const createStyles = () =>
3839
},
3940
});
4041

41-
const ChangeInSimulationModal = ({
42-
route,
43-
}: {
44-
route: { params: { onProceed: () => void; onReject: () => void } };
45-
}) => {
42+
interface ChangeInSimulationModalRouteParams {
43+
onProceed: () => void;
44+
onReject: () => void;
45+
}
46+
47+
const ChangeInSimulationModal = () => {
48+
const route =
49+
useRoute<
50+
RouteProp<{ params: ChangeInSimulationModalRouteParams }, 'params'>
51+
>();
4652
const styles = createStyles();
4753
const sheetRef = useRef<BottomSheetRef>(null);
4854
const { onProceed, onReject } = route.params;

app/components/Views/MultichainAccounts/IntroModal/LearnMoreBottomSheet.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { setMultichainAccountsIntroModalSeen } from '../../../../actions/user';
2525
import { LEARN_MORE_BOTTOM_SHEET_TEST_IDS } from './LearnMoreBottomSheet.testIds';
2626

2727
interface LearnMoreBottomSheetProps {
28-
onClose: () => void;
28+
onClose?: () => void;
2929
}
3030

3131
const LearnMoreBottomSheet: React.FC<LearnMoreBottomSheetProps> = ({

app/components/Views/OriginSpamModal/OriginSpamModal.test.tsx

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,17 @@ import OriginSpamModal, {
1212
} from './OriginSpamModal';
1313
import { RootState } from '../../../reducers';
1414

15+
const SCAM_ORIGIN_MOCK = 'scam.origin';
16+
17+
let mockRouteParams: { origin: string } = { origin: SCAM_ORIGIN_MOCK };
18+
19+
jest.mock('@react-navigation/native', () => ({
20+
...jest.requireActual('@react-navigation/native'),
21+
useRoute: () => ({
22+
params: mockRouteParams,
23+
}),
24+
}));
25+
1526
jest.mock('react-redux', () => ({
1627
...jest.requireActual('react-redux'),
1728
useDispatch: () => jest.fn(),
@@ -28,13 +39,6 @@ jest.mock(
2839
({ children }: { children: React.ReactElement }) => <>{children}</>,
2940
);
3041

31-
const SCAM_ORIGIN_MOCK = 'scam.origin';
32-
const NAVIGATION_PARAMS_MOCK = {
33-
params: {
34-
origin: SCAM_ORIGIN_MOCK,
35-
},
36-
};
37-
3842
const mockInitialState: DeepPartial<RootState> = {
3943
engine: {
4044
backgroundState: {
@@ -48,38 +52,30 @@ describe('OriginSpamModal', () => {
4852

4953
beforeEach(() => {
5054
jest.clearAllMocks();
55+
mockRouteParams = { origin: SCAM_ORIGIN_MOCK };
5156
});
5257

5358
describe('renders', () => {
5459
it('spam modal content by default', () => {
55-
const { toJSON } = renderWithProvider(
56-
<OriginSpamModal route={NAVIGATION_PARAMS_MOCK} />,
57-
{
58-
state: mockInitialState,
59-
},
60-
);
60+
const { toJSON } = renderWithProvider(<OriginSpamModal />, {
61+
state: mockInitialState,
62+
});
6163
expect(toJSON()).toMatchSnapshot();
6264
});
6365

6466
it('SiteBlockedContent if user opt in to block dapp', () => {
65-
const wrapper = renderWithProvider(
66-
<OriginSpamModal route={NAVIGATION_PARAMS_MOCK} />,
67-
{
68-
state: mockInitialState,
69-
},
70-
);
67+
const wrapper = renderWithProvider(<OriginSpamModal />, {
68+
state: mockInitialState,
69+
});
7170
fireEvent.press(wrapper.getByTestId(BLOCK_BUTTON_TEST_ID));
7271
expect(wrapper.toJSON()).toMatchSnapshot();
7372
});
7473
});
7574

7675
it('reset dapp spam state on clicking continue button', () => {
77-
const wrapper = renderWithProvider(
78-
<OriginSpamModal route={NAVIGATION_PARAMS_MOCK} />,
79-
{
80-
state: mockInitialState,
81-
},
82-
);
76+
const wrapper = renderWithProvider(<OriginSpamModal />, {
77+
state: mockInitialState,
78+
});
8379
fireEvent.press(wrapper.getByTestId(CONTINUE_BUTTON_TEST_ID));
8480

8581
expect(mockResetOriginSpamState).toHaveBeenCalledTimes(1);

app/components/Views/OriginSpamModal/OriginSpamModal.tsx

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import React, { useCallback, useMemo, useRef, useState } from 'react';
2+
import { RouteProp, useRoute } from '@react-navigation/native';
23
import { ImageSourcePropType, StyleSheet, View } from 'react-native';
34
import { useDispatch } from 'react-redux';
45
import { getUrlObj, prefixUrlWithProtocol } from '../../../util/browser';
@@ -160,11 +161,13 @@ const SiteBlockedContent = ({ onCloseModal }: { onCloseModal: () => void }) => {
160161
);
161162
};
162163

163-
const OriginSpamModal = ({
164-
route,
165-
}: {
166-
route: { params: { origin: string } };
167-
}) => {
164+
interface OriginSpamModalRouteParams {
165+
origin: string;
166+
}
167+
168+
const OriginSpamModal = () => {
169+
const route =
170+
useRoute<RouteProp<{ params: OriginSpamModalRouteParams }, 'params'>>();
168171
const dispatch = useDispatch();
169172
const { origin } = route.params;
170173
const styles = createStyles();

app/components/Views/Quiz/SRPQuiz/SRPQuiz.test.tsx

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,13 @@ import {
99
SrpSecurityQuestionOneSelectorsIDs,
1010
SrpSecurityQuestionTwoSelectorsIDs,
1111
} from './SrpQuizModal.testIds';
12-
import SRPQuiz, { SRPQuizProps } from './SRPQuiz';
12+
import SRPQuiz from './SRPQuiz';
1313
import Routes from '../../../../constants/navigation/Routes';
1414
import { strings } from '../../../../../locales/i18n';
1515
import { Linking } from 'react-native';
1616

17+
let mockRouteParams: { keyringId?: string } = {};
18+
1719
const mockNavigate = jest.fn();
1820

1921
const initialMetrics: Metrics = {
@@ -25,17 +27,21 @@ jest.mock('@react-navigation/native', () => ({
2527
useNavigation: () => ({
2628
navigate: mockNavigate,
2729
}),
30+
useRoute: () => ({
31+
params: mockRouteParams,
32+
}),
2833
}));
2934

3035
jest.mock('react-native/Libraries/Linking/Linking', () => ({
3136
openURL: jest.fn(),
3237
}));
3338

3439
const renderSRPQuiz = (
35-
props: SRPQuizProps,
40+
routeParams: { keyringId?: string } = {},
3641
completeQuiz: boolean = true,
3742
hasVault: boolean = false,
3843
) => {
44+
mockRouteParams = routeParams;
3945
const mockStore = configureMockStore();
4046
const initialState = {
4147
engine: {
@@ -52,7 +58,7 @@ const renderSRPQuiz = (
5258
<SafeAreaProvider initialMetrics={initialMetrics}>
5359
<Provider store={store}>
5460
<ThemeContext.Provider value={mockTheme}>
55-
<SRPQuiz {...props} />
61+
<SRPQuiz />
5662
</ThemeContext.Provider>
5763
</Provider>
5864
</SafeAreaProvider>,
@@ -89,15 +95,14 @@ const renderSRPQuiz = (
8995
};
9096

9197
describe('SRPQuiz', () => {
98+
beforeEach(() => {
99+
mockRouteParams = {};
100+
});
101+
92102
it('passes the keyringId to the SRPQuiz', async () => {
93103
const keyringId = '123';
94-
const props = {
95-
route: {
96-
params: { keyringId },
97-
},
98-
};
99104

100-
renderSRPQuiz(props, true);
105+
renderSRPQuiz({ keyringId }, true);
101106

102107
await waitFor(() => {
103108
expect(mockNavigate).toHaveBeenCalledWith(
@@ -113,12 +118,7 @@ describe('SRPQuiz', () => {
113118

114119
it('should navigate to the learn more article of social login when the learn more button is pressed', async () => {
115120
const keyringId = '123';
116-
const props = {
117-
route: {
118-
params: { keyringId },
119-
},
120-
};
121-
const { getByText } = renderSRPQuiz(props, true, true);
121+
const { getByText } = renderSRPQuiz({ keyringId }, true, true);
122122

123123
const learnMoreButton = getByText(strings('srp_security_quiz.learn_more'));
124124
fireEvent.press(learnMoreButton);

app/components/Views/Quiz/SRPQuiz/SRPQuiz.tsx

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* eslint-disable @typescript-eslint/no-var-requires, @typescript-eslint/no-require-imports, import-x/no-commonjs */
22
import React, { useState, useCallback, useEffect, useRef } from 'react';
33
import { View, Linking, AppState } from 'react-native';
4-
import { useNavigation } from '@react-navigation/native';
4+
import { RouteProp, useNavigation, useRoute } from '@react-navigation/native';
55
import BottomSheet, {
66
BottomSheetRef,
77
} from '../../../../component-library/components/BottomSheets/BottomSheet';
@@ -34,22 +34,15 @@ import { useSelector } from 'react-redux';
3434

3535
const introductionImg = require('../../../../images/reveal_srp.png');
3636

37-
export interface SRPQuizProps {
38-
route: {
39-
params: {
40-
keyringId?: string;
41-
};
42-
};
37+
interface SRPQuizRouteParams {
38+
keyringId?: string;
4339
}
4440

45-
const SRPQuiz = (props: SRPQuizProps) => {
46-
// It has be destructured like this because of prettier
47-
// shifting the fence to the ending curly brace.
41+
const SRPQuiz = () => {
42+
const route = useRoute<RouteProp<{ params: SRPQuizRouteParams }, 'params'>>();
4843
const {
49-
route: {
50-
params: { keyringId },
51-
},
52-
} = props;
44+
params: { keyringId },
45+
} = route;
5346
const modalRef = useRef<BottomSheetRef>(null);
5447
const [stage, setStage] = useState<QuizStage>(QuizStage.introduction);
5548
const { styles, theme } = useStyles(stylesheet, {});

0 commit comments

Comments
 (0)