Skip to content

Commit a13b9af

Browse files
refactor: Replace redesign confirmation BottomModal with BottomSheet (#13268)
<!-- 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** Updates: - Replaces BottomModal with BottomSheet in redesign Confirm page - Remove min height to prevent redesign Confirm page from showing excess space - Allow BottomSheet styles (rounded corners, footer bottom styles, and device padding detection padding) rather than custom BottomModal styles - Update BlockaidBanner to remove surrounding margin to allow for better reusability - Replace BlockaidBanner instances margin overrides with surrounding container margins - Update BottomSheet to pass style to BottomSheetDialog sheet cc: @brianacnguyen - Note: I think we don't need the additional background colors for header and footer since the parent, BottomSheet animated view sets the background color. Once I removed them, it triggered many snapshot tests and code owner reviews from 5 additional teams. Instead of making this change, we will continue to update the bottomsheet, footer, and header when applicable Fixes: - Fix TransactionReview BlockaidBanner negative padding → turn to positive padding to allow space between next element - Fix Confirm.test console.errors ## **Related issues** Fixes: #13267 Todo follow-up - Related Issue: #12656 ## **Manual testing steps** Test BottomSheets and BlockaidBanner alerts work as expected. Test redesign confirmation page works as expected ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### Before using BottomModal and no scroll behavior https://github.com/user-attachments/assets/0d4c5453-5824-47fd-8aa5-7405cf19de53 ### After using BottomSheet with scroll behavior <img width="320" src="https://github.com/user-attachments/assets/a908f81f-1a28-4e8c-966a-a390430dc7d9"> https://github.com/user-attachments/assets/8f8e2344-7706-49a4-b773-a5226b7826d4 ### Before 70% min height <img width="320" src="https://github.com/user-attachments/assets/94509d26-5e20-414d-942f-56938620f90a"> ### After remove 70% min height <img width="320" src="https://github.com/user-attachments/assets/641022c5-6f23-47a5-9f00-4e9358a96d3b"> ### **Before Transaction Review BlockaidBanner negative padding** <img width="320" src="https://github.com/user-attachments/assets/c057c4e7-c7a7-42f2-8251-b4f2c075a802"> ### **After Transaction Review BlockaidBanner positive padding ** <img width="320" src="https://github.com/user-attachments/assets/de1ecadb-c7ef-49e1-8ae2-6ce1b5d16997"> ### Before Confirm.test console errors ![CleanShot 2025-02-09 at 01 35 46@2x](https://github.com/user-attachments/assets/2653a073-b0de-4c2d-bcea-dfbc11d38246) ![CleanShot 2025-02-09 at 01 35 54@2x](https://github.com/user-attachments/assets/f9ce410c-42c5-4738-b99f-f871456eb82f) ### After Confirm.test no console.errors ![CleanShot 2025-02-09 at 01 49 47@2x](https://github.com/user-attachments/assets/d3bf6580-553d-4b14-96ab-3f529d17db67) ## **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 dcf67aa commit a13b9af

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: {
@@ -31,14 +67,10 @@ jest.mock('../../../../core/Engine', () => ({
3167
},
3268
controllerMessenger: {
3369
subscribe: jest.fn(),
70+
unsubscribe: jest.fn(),
3471
},
3572
}));
3673

37-
jest.mock('../../../../util/address', () => ({
38-
...jest.requireActual('../../../../util/address'),
39-
getAddressAccountType: (str: string) => str,
40-
}));
41-
4274
jest.mock('react-native-gzip', () => ({
4375
deflate: (str: string) => str,
4476
}));
@@ -61,10 +93,22 @@ describe('Confirm', () => {
6193
expect(getByTestId('modal-confirmation-container')).toBeDefined();
6294
});
6395

64-
it('renders correct information for personal sign', async () => {
65-
const { getAllByRole, getByText } = renderWithProvider(<Confirm />, {
66-
state: personalSignatureConfirmationState,
96+
it('renders a flat confirmation for specified type(s): staking deposit', () => {
97+
const { getByTestId } = renderWithProvider(<Confirm />, {
98+
state: stakingDepositConfirmationState,
6799
});
100+
expect(getByTestId('flat-confirmation-container')).toBeDefined();
101+
});
102+
103+
it('renders correct information for personal sign', () => {
104+
const { getAllByRole, getByText } = renderWithProvider(
105+
<SafeAreaProvider>
106+
<Confirm />
107+
</SafeAreaProvider>,
108+
{
109+
state: personalSignatureConfirmationState,
110+
},
111+
);
68112
expect(getByText('Signature request')).toBeDefined();
69113
expect(
70114
getByText('Review request details before you confirm.'),
@@ -76,11 +120,16 @@ describe('Confirm', () => {
76120
expect(getAllByRole('button')).toHaveLength(2);
77121
});
78122

79-
it('renders correct information for typed sign v1', async () => {
123+
it('renders correct information for typed sign v1', () => {
80124
const { getAllByRole, getAllByText, getByText, queryByText } =
81-
renderWithProvider(<Confirm />, {
82-
state: typedSignV1ConfirmationState,
83-
});
125+
renderWithProvider(
126+
<SafeAreaProvider>
127+
<Confirm />
128+
</SafeAreaProvider>,
129+
{
130+
state: typedSignV1ConfirmationState,
131+
},
132+
);
84133
expect(getByText('Signature request')).toBeDefined();
85134
expect(getByText('Request from')).toBeDefined();
86135
expect(getByText('metamask.github.io')).toBeDefined();
@@ -102,18 +151,21 @@ describe('Confirm', () => {
102151
expect(getByText('Advanced details')).toBeDefined();
103152
});
104153

105-
it('renders blockaid banner if confirmation has blockaid error response', async () => {
154+
it('renders a blockaid banner if the confirmation has blockaid error response', async () => {
106155
const { getByText } = renderWithProvider(<Confirm />, {
107156
state: {
108157
...typedSignV1ConfirmationState,
109158
signatureRequest: { securityAlertResponse },
110159
},
111160
});
161+
162+
await act(async () => undefined);
163+
112164
expect(getByText('Signature request')).toBeDefined();
113165
expect(getByText('This is a deceptive request')).toBeDefined();
114166
});
115167

116-
it('returns null if re-design is not enabled for confirmation', () => {
168+
it('returns null if confirmation redesign is not enabled', () => {
117169
jest
118170
.spyOn(ConfirmationRedesignEnabled, 'useConfirmationRedesignEnabled')
119171
.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)