Skip to content

Commit dcf5047

Browse files
fix: cherry-pick 7.42.0 refactor: Replace redesign confirmation BottomModal with BottomSheet (#13914)
<!-- 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** Cherry-picks 1/3 BottomSheet fix: #13268 into 7.42.0 Todo cherry-picks: 2/3: #13783 3/3: #13913 ## **Related issues** Fixes: #13870 ## **Manual testing steps** Open any redesign signature in Android to test related bug fix ## **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. Co-authored-by: Brian Nguyen <[email protected]>
1 parent 4c4f499 commit dcf5047

File tree

22 files changed

+187
-135
lines changed

22 files changed

+187
-135
lines changed

app/component-library/components/BottomSheets/BottomSheet/BottomSheet.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ const BottomSheet = forwardRef<BottomSheetRef, BottomSheetProps>(
3636
children,
3737
onClose,
3838
onOpen,
39+
style,
3940
isInteractable = true,
4041
shouldNavigateBack = true,
4142
isFullscreen = false,
@@ -107,6 +108,7 @@ const BottomSheet = forwardRef<BottomSheetRef, BottomSheetProps>(
107108
onOpen={onOpenCB}
108109
ref={bottomSheetDialogRef}
109110
isFullscreen={isFullscreen}
111+
style={style}
110112
>
111113
{children}
112114
</BottomSheetDialog>

app/component-library/components/BottomSheets/BottomSheet/foundation/BottomSheetDialog/BottomSheetDialog.styles.ts

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// Third party dependencies.
2+
// eslint-disable-next-line @typescript-eslint/no-shadow
23
import { StyleSheet, ViewStyle } from 'react-native';
34

45
// External dependencies.
@@ -21,7 +22,7 @@ const styleSheet = (params: {
2122
}) => {
2223
const { vars, theme } = params;
2324
const { colors, shadows } = theme;
24-
const { maxSheetHeight, screenBottomPadding, isFullscreen } = vars;
25+
const { isFullscreen, maxSheetHeight, screenBottomPadding, style } = vars;
2526

2627
return StyleSheet.create({
2728
base: Object.assign({
@@ -30,18 +31,21 @@ const styleSheet = (params: {
3031
left: 0,
3132
right: 0,
3233
} as ViewStyle) as ViewStyle,
33-
sheet: {
34-
backgroundColor: colors.background.default,
35-
borderTopLeftRadius: 8,
36-
borderTopRightRadius: 8,
37-
maxHeight: maxSheetHeight,
38-
overflow: 'hidden',
39-
paddingBottom: screenBottomPadding,
40-
borderWidth: 1,
41-
borderColor: colors.border.muted,
42-
...(isFullscreen && { height: maxSheetHeight }),
43-
...shadows.size.lg,
44-
},
34+
sheet: Object.assign(
35+
{
36+
backgroundColor: colors.background.default,
37+
borderTopLeftRadius: 8,
38+
borderTopRightRadius: 8,
39+
maxHeight: maxSheetHeight,
40+
overflow: 'hidden',
41+
paddingBottom: screenBottomPadding,
42+
borderWidth: 1,
43+
borderColor: colors.border.muted,
44+
...(isFullscreen && { height: maxSheetHeight }),
45+
...shadows.size.lg,
46+
},
47+
style,
48+
) as ViewStyle,
4549
notchWrapper: {
4650
alignSelf: 'stretch',
4751
padding: 4,

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ const BottomSheetDialog = forwardRef<
5959
isInteractable = true,
6060
onClose,
6161
onOpen,
62+
style,
6263
...props
6364
},
6465
ref,
@@ -71,6 +72,7 @@ const BottomSheetDialog = forwardRef<
7172
const { styles } = useStyles(styleSheet, {
7273
maxSheetHeight,
7374
screenBottomPadding,
75+
style,
7476
isFullscreen,
7577
});
7678
// X and Y values start on top left of the DIALOG

app/component-library/components/BottomSheets/BottomSheet/foundation/BottomSheetDialog/BottomSheetDialog.types.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// Third party dependencies.
2-
import { ViewProps } from 'react-native';
2+
import { StyleProp, ViewProps, ViewStyle } from 'react-native';
33

44
/**
55
* BottomSheetDialog component props.
@@ -40,5 +40,6 @@ export interface BottomSheetDialogRef {
4040
export interface BottomSheetDialogStyleSheetVars {
4141
maxSheetHeight: number;
4242
screenBottomPadding: number;
43+
style: StyleProp<ViewStyle>;
4344
isFullscreen: boolean;
4445
}

app/components/Nav/App/index.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,12 +1039,10 @@ const App = (props) => {
10391039
<Stack.Screen
10401040
name={Routes.CONFIRM_FLAT_PAGE}
10411041
component={ConfirmRequest}
1042-
options={{ animationEnabled: true }}
10431042
/>
10441043
<Stack.Screen
10451044
name={Routes.CONFIRM_MODAL}
10461045
component={ConfirmDappRequest}
1047-
options={{ animationEnabled: true }}
10481046
/>
10491047
</Stack.Navigator>
10501048
</NavigationContainer>

app/components/Views/confirmations/Approval/__snapshots__/index.test.tsx.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -595,7 +595,7 @@ exports[`Approval render matches snapshot 1`] = `
595595
<View
596596
style={
597597
{
598-
"marginBottom": -8,
598+
"marginBottom": 8,
599599
"marginHorizontal": 16,
600600
}
601601
}

app/components/Views/confirmations/Confirm/Confirm.styles.ts

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
import { StyleSheet } from 'react-native';
2-
3-
import Device from '../../../../util/device';
42
import { Theme } from '../../../../util/theme/models';
53

6-
const styleSheet = (params: { theme: Theme }) => {
4+
const styleSheet = (params: {
5+
theme: Theme;
6+
}) => {
77
const { theme } = params;
88

99
return StyleSheet.create({
10+
bottomSheetDialogSheet: {
11+
backgroundColor: theme.colors.background.alternative,
12+
},
1013
flatContainer: {
1114
position: 'absolute',
1215
top: 0,
@@ -16,22 +19,9 @@ const styleSheet = (params: { theme: Theme }) => {
1619
zIndex: 9999,
1720
backgroundColor: theme.colors.background.alternative,
1821
justifyContent: 'space-between',
19-
paddingHorizontal: 16,
2022
},
21-
modalContainer: {
22-
backgroundColor: theme.colors.background.alternative,
23+
scrollView: {
2324
paddingHorizontal: 16,
24-
paddingVertical: 24,
25-
borderTopLeftRadius: 20,
26-
borderTopRightRadius: 20,
27-
paddingBottom: Device.isIphoneX() ? 20 : 0,
28-
height: '85%',
29-
},
30-
scrollableSection: {
31-
padding: 4,
32-
},
33-
scrollable: {
34-
height: '75%',
3525
},
3626
});
3727
};

app/components/Views/confirmations/Confirm/Confirm.test.tsx

Lines changed: 66 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import React from 'react';
2+
import { SafeAreaProvider } from 'react-native-safe-area-context';
3+
import { act } from '@testing-library/react-native';
24

35
import renderWithProvider from '../../../../util/test/renderWithProvider';
46
import {
@@ -12,6 +14,40 @@ import * as ConfirmationRedesignEnabled from '../hooks/useConfirmationRedesignEn
1214

1315
import { Confirm } from './Confirm';
1416

17+
jest.mock('@react-navigation/native', () => ({
18+
...jest.requireActual('@react-navigation/native'),
19+
useNavigation: () => ({
20+
addListener: jest.fn(),
21+
dispatch: jest.fn(),
22+
goBack: jest.fn(),
23+
navigate: jest.fn(),
24+
removeListener: jest.fn(),
25+
}),
26+
}));
27+
28+
jest.mock('react-native/Libraries/Linking/Linking', () => ({
29+
addEventListener: jest.fn(),
30+
removeEventListener: jest.fn(),
31+
openURL: jest.fn(),
32+
canOpenURL: jest.fn(),
33+
getInitialURL: jest.fn(),
34+
}));
35+
36+
jest.mock('react-native-safe-area-context', () => {
37+
const inset = { top: 0, right: 0, bottom: 0, left: 0 };
38+
const frame = { width: 0, height: 0, x: 0, y: 0 };
39+
40+
return {
41+
...jest.requireActual('react-native-safe-area-context'),
42+
SafeAreaProvider: jest.fn().mockImplementation(({ children }) => children),
43+
SafeAreaConsumer: jest
44+
.fn()
45+
.mockImplementation(({ children }) => children(inset)),
46+
useSafeAreaInsets: jest.fn().mockImplementation(() => inset),
47+
useSafeAreaFrame: jest.fn().mockImplementation(() => frame),
48+
};
49+
});
50+
1551
jest.mock('../../../../core/Engine', () => ({
1652
getTotalFiatAccountBalance: () => ({ tokenFiat: 10 }),
1753
context: {
@@ -42,14 +78,10 @@ jest.mock('../../../../core/Engine', () => ({
4278
},
4379
controllerMessenger: {
4480
subscribe: jest.fn(),
81+
unsubscribe: jest.fn(),
4582
},
4683
}));
4784

48-
jest.mock('../../../../util/address', () => ({
49-
...jest.requireActual('../../../../util/address'),
50-
getAddressAccountType: (str: string) => str,
51-
}));
52-
5385
jest.mock('react-native-gzip', () => ({
5486
deflate: (str: string) => str,
5587
}));
@@ -72,10 +104,22 @@ describe('Confirm', () => {
72104
expect(getByTestId('modal-confirmation-container')).toBeDefined();
73105
});
74106

75-
it('renders correct information for personal sign', async () => {
76-
const { getAllByRole, getByText } = renderWithProvider(<Confirm />, {
77-
state: personalSignatureConfirmationState,
107+
it('renders a flat confirmation for specified type(s): staking deposit', () => {
108+
const { getByTestId } = renderWithProvider(<Confirm />, {
109+
state: stakingDepositConfirmationState,
78110
});
111+
expect(getByTestId('flat-confirmation-container')).toBeDefined();
112+
});
113+
114+
it('renders correct information for personal sign', () => {
115+
const { getAllByRole, getByText } = renderWithProvider(
116+
<SafeAreaProvider>
117+
<Confirm />
118+
</SafeAreaProvider>,
119+
{
120+
state: personalSignatureConfirmationState,
121+
},
122+
);
79123
expect(getByText('Signature request')).toBeDefined();
80124
expect(
81125
getByText('Review request details before you confirm.'),
@@ -87,11 +131,16 @@ describe('Confirm', () => {
87131
expect(getAllByRole('button')).toHaveLength(2);
88132
});
89133

90-
it('renders correct information for typed sign v1', async () => {
134+
it('renders correct information for typed sign v1', () => {
91135
const { getAllByRole, getAllByText, getByText, queryByText } =
92-
renderWithProvider(<Confirm />, {
93-
state: typedSignV1ConfirmationState,
94-
});
136+
renderWithProvider(
137+
<SafeAreaProvider>
138+
<Confirm />
139+
</SafeAreaProvider>,
140+
{
141+
state: typedSignV1ConfirmationState,
142+
},
143+
);
95144
expect(getByText('Signature request')).toBeDefined();
96145
expect(getByText('Request from')).toBeDefined();
97146
expect(getByText('metamask.github.io')).toBeDefined();
@@ -113,18 +162,21 @@ describe('Confirm', () => {
113162
expect(getByText('Advanced details')).toBeDefined();
114163
});
115164

116-
it('renders blockaid banner if confirmation has blockaid error response', async () => {
165+
it('renders a blockaid banner if the confirmation has blockaid error response', async () => {
117166
const { getByText } = renderWithProvider(<Confirm />, {
118167
state: {
119168
...typedSignV1ConfirmationState,
120169
signatureRequest: { securityAlertResponse },
121170
},
122171
});
172+
173+
await act(async () => undefined);
174+
123175
expect(getByText('Signature request')).toBeDefined();
124176
expect(getByText('This is a deceptive request')).toBeDefined();
125177
});
126178

127-
it('returns null if re-design is not enabled for confirmation', () => {
179+
it('returns null if confirmation redesign is not enabled', () => {
128180
jest
129181
.spyOn(ConfirmationRedesignEnabled, 'useConfirmationRedesignEnabled')
130182
.mockReturnValue({ isRedesignedEnabled: false });

app/components/Views/confirmations/Confirm/Confirm.tsx

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,34 +6,36 @@ import {
66
View,
77
} from 'react-native';
88

9+
import BottomSheet from '../../../../component-library/components/BottomSheets/BottomSheet';
910
import { useStyles } from '../../../../component-library/hooks';
10-
import BottomModal from '../components/UI/BottomModal';
11-
import Footer from '../components/Confirm/Footer';
11+
import { Footer } from '../components/Confirm/Footer';
1212
import Info from '../components/Confirm/Info';
1313
import { LedgerContextProvider } from '../context/LedgerContext';
1414
import { QRHardwareContextProvider } from '../context/QRHardwareContext/QRHardwareContext';
1515
import SignatureBlockaidBanner from '../components/Confirm/SignatureBlockaidBanner';
1616
import Title from '../components/Confirm/Title';
17-
import useApprovalRequest from '../hooks/useApprovalRequest';
18-
import { useConfirmActions } from '../hooks/useConfirmActions';
1917
import { useConfirmationRedesignEnabled } from '../hooks/useConfirmationRedesignEnabled';
2018
import { useFlatConfirmation } from '../hooks/useFlatConfirmation';
19+
import useApprovalRequest from '../hooks/useApprovalRequest';
20+
import { useConfirmActions } from '../hooks/useConfirmActions';
2121
import styleSheet from './Confirm.styles';
2222

2323
const ConfirmWrapped = ({
2424
styles,
25+
testID,
2526
}: {
2627
styles: StyleSheet.NamedStyles<Record<string, unknown>>;
28+
testID?: string;
2729
}) => (
2830
<QRHardwareContextProvider>
2931
<LedgerContextProvider>
3032
<Title />
31-
<ScrollView style={styles.scrollable}>
32-
<TouchableWithoutFeedback>
33-
<View style={styles.scrollableSection}>
33+
<ScrollView style={styles.scrollView}>
34+
<TouchableWithoutFeedback testID={testID}>
35+
<>
3436
<SignatureBlockaidBanner />
3537
<Info />
36-
</View>
38+
</>
3739
</TouchableWithoutFeedback>
3840
</ScrollView>
3941
<Footer />
@@ -62,10 +64,13 @@ export const Confirm = () => {
6264
}
6365

6466
return (
65-
<BottomModal onClose={onReject} testID="modal-confirmation-container">
66-
<View style={styles.modalContainer} testID={approvalRequest?.type}>
67-
<ConfirmWrapped styles={styles} />
68-
</View>
69-
</BottomModal>
67+
<BottomSheet
68+
isInteractable={false}
69+
onClose={onReject}
70+
style={styles.bottomSheetDialogSheet}
71+
testID="modal-confirmation-container"
72+
>
73+
<ConfirmWrapped testID={approvalRequest?.type} styles={styles} />
74+
</BottomSheet>
7075
);
7176
};

app/components/Views/confirmations/SendFlow/Confirm/styles.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ const createStyles = (colors: any) =>
135135
textDecorationLine: 'underline',
136136
...fontStyles.bold,
137137
},
138-
blockaidBanner: {
138+
blockaidBannerContainer: {
139139
marginBottom: 10,
140140
marginTop: 20,
141141
marginHorizontal: 10,

app/components/Views/confirmations/components/ApproveTransactionReview/styles.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ const createStyles = (colors: any) =>
167167
justifyContent: 'center',
168168
alignItems: 'center',
169169
},
170-
blockaidWarning: {
170+
blockaidBannerContainer: {
171171
marginBottom: 10,
172172
marginTop: 20,
173173
marginHorizontal: 10,

0 commit comments

Comments
 (0)