Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Replace redesign confirmation BottomModal with BottomSheet #13268

Merged
merged 46 commits into from
Feb 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
04a974b
refactor: rm background color form bottom sheet footer and header
digiwand Jan 29, 2025
a7c0698
refactor: use BottomSheet for redesign Confirm
digiwand Jan 30, 2025
f1defbf
refactor: replace BlockaidBanner style prop uses
digiwand Jan 30, 2025
44837f7
test: update snapshots following BottomSheet updates
digiwand Jan 30, 2025
3605a39
test:fix: redesign Confirm mock SafeAreaProvider
digiwand Jan 30, 2025
f9d3150
Revert "refactor: rm background color form bottom sheet footer and he…
digiwand Jan 30, 2025
60757f0
refactor: BottomSheet replace styleAnimatedView -> isBackgroundAltern…
digiwand Jan 30, 2025
50594e4
test:fix: restore BottomSheet header and footer background
digiwand Jan 30, 2025
3c4a247
Merge branch 'main' into refactor-signature-use-bottom-sheet
digiwand Feb 7, 2025
02bbf25
fix: merge conflicts
digiwand Feb 8, 2025
58ec799
test: rm async and update phrases Confirm.test
digiwand Feb 8, 2025
a7ea0fe
test: rm unused getAddressAccountType mock Confirm.test
digiwand Feb 8, 2025
52761af
test: rm unnecessary beforeEach clearAllMocks
digiwand Feb 8, 2025
40d16ad
test: Confirm.test add more mocks to remove console.errors
digiwand Feb 8, 2025
07e7f1f
test: Confirm.test rm missing act warning
digiwand Feb 8, 2025
9479133
Merge branch 'main' into refactor-signature-use-bottom-sheet
digiwand Feb 11, 2025
8cd072e
refactor: revert isBackgroundAlternative -> stylesDialogSheet Confir…
digiwand Feb 12, 2025
2605bca
test:fix: bypass lint empty function
digiwand Feb 12, 2025
42a4559
fix: Confirm merge conflicts padding and height
digiwand Feb 13, 2025
e76e66a
fix: ConfirmFooter backgroundColor
digiwand Feb 14, 2025
b103da4
Merge branch 'main' into refactor-signature-use-bottom-sheet
digiwand Feb 14, 2025
b7e45e8
fix: confirm Footer import
digiwand Feb 17, 2025
ef8ebc5
Merge branch 'main' into refactor-signature-use-bottom-sheet
digiwand Feb 17, 2025
1c8616d
Merge branch 'main' into refactor-signature-use-bottom-sheet
digiwand Feb 18, 2025
e05ec52
fix: merge conflicts rm Confirm scrollView styles
digiwand Feb 18, 2025
52fb72e
fix: confirm scroll centering
digiwand Feb 18, 2025
7634cb5
fix: confirm scroll icon color
digiwand Feb 18, 2025
257462d
fix: confirm scroll performance and log warnings
digiwand Feb 18, 2025
29d0159
fix: snapshots after removal of View + margin
digiwand Feb 18, 2025
e7dd4ae
Merge branch 'main' into refactor-signature-use-bottom-sheet
digiwand Feb 19, 2025
1598bba
fix merge
digiwand Feb 19, 2025
8b18399
Merge branch 'main' into refactor-signature-use-bottom-sheet
digiwand Feb 20, 2025
35aa603
merge commit fixes
digiwand Feb 20, 2025
8e9532f
Updated BottomSheetDialog style
brianacnguyen Feb 20, 2025
06d7de3
Merge branch 'main' into refactor-signature-use-bottom-sheet
digiwand Feb 21, 2025
b4ba3c3
style: fix indent
digiwand Feb 21, 2025
acbdf1d
fix: confirm footer styles
digiwand Feb 21, 2025
580cf91
fix: Footer import LedgerContext
digiwand Feb 21, 2025
41da02c
fix: bottom to top animation
digiwand Feb 25, 2025
a61de1c
Merge branch 'main' into refactor-signature-use-bottom-sheet
digiwand Feb 26, 2025
8a7c8ff
Removed unnecessary changes
brianacnguyen Feb 26, 2025
1daf7b0
Merge branch 'main' into refactor-signature-use-bottom-sheet
digiwand Feb 27, 2025
5ea61bf
fix: reintroduce onClose onReject Confirm just in case
digiwand Feb 27, 2025
9ec06d6
fix: missing scroll paddingHorizontal
digiwand Feb 27, 2025
4034da6
refactor: <View> -> <>. TouchableWithoutFeedback needs children elem
digiwand Feb 27, 2025
a7c539a
fix: rm flatContainer paddingHorizontal
digiwand Feb 28, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const BottomSheet = forwardRef<BottomSheetRef, BottomSheetProps>(
children,
onClose,
onOpen,
style,
isInteractable = true,
shouldNavigateBack = true,
isFullscreen = false,
Expand Down Expand Up @@ -107,6 +108,7 @@ const BottomSheet = forwardRef<BottomSheetRef, BottomSheetProps>(
onOpen={onOpenCB}
ref={bottomSheetDialogRef}
isFullscreen={isFullscreen}
style={style}
>
{children}
</BottomSheetDialog>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Third party dependencies.
// eslint-disable-next-line @typescript-eslint/no-shadow
import { StyleSheet, ViewStyle } from 'react-native';

// External dependencies.
Expand All @@ -21,7 +22,7 @@ const styleSheet = (params: {
}) => {
const { vars, theme } = params;
const { colors, shadows } = theme;
const { maxSheetHeight, screenBottomPadding, isFullscreen } = vars;
const { isFullscreen, maxSheetHeight, screenBottomPadding, style } = vars;

return StyleSheet.create({
base: Object.assign({
Expand All @@ -30,18 +31,21 @@ const styleSheet = (params: {
left: 0,
right: 0,
} as ViewStyle) as ViewStyle,
sheet: {
backgroundColor: colors.background.default,
borderTopLeftRadius: 8,
borderTopRightRadius: 8,
maxHeight: maxSheetHeight,
overflow: 'hidden',
paddingBottom: screenBottomPadding,
borderWidth: 1,
borderColor: colors.border.muted,
...(isFullscreen && { height: maxSheetHeight }),
...shadows.size.lg,
},
sheet: Object.assign(
{
backgroundColor: colors.background.default,
borderTopLeftRadius: 8,
borderTopRightRadius: 8,
maxHeight: maxSheetHeight,
overflow: 'hidden',
paddingBottom: screenBottomPadding,
borderWidth: 1,
borderColor: colors.border.muted,
...(isFullscreen && { height: maxSheetHeight }),
...shadows.size.lg,
},
style,
) as ViewStyle,
notchWrapper: {
alignSelf: 'stretch',
padding: 4,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ const BottomSheetDialog = forwardRef<
isInteractable = true,
onClose,
onOpen,
style,
...props
},
ref,
Expand All @@ -71,6 +72,7 @@ const BottomSheetDialog = forwardRef<
const { styles } = useStyles(styleSheet, {
maxSheetHeight,
screenBottomPadding,
style,
isFullscreen,
});
// X and Y values start on top left of the DIALOG
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Third party dependencies.
import { ViewProps } from 'react-native';
import { StyleProp, ViewProps, ViewStyle } from 'react-native';

/**
* BottomSheetDialog component props.
Expand Down Expand Up @@ -40,5 +40,6 @@ export interface BottomSheetDialogRef {
export interface BottomSheetDialogStyleSheetVars {
maxSheetHeight: number;
screenBottomPadding: number;
style: StyleProp<ViewStyle>;
isFullscreen: boolean;
}
2 changes: 0 additions & 2 deletions app/components/Nav/App/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1039,12 +1039,10 @@ const App = (props) => {
<Stack.Screen
name={Routes.CONFIRM_FLAT_PAGE}
component={ConfirmRequest}
options={{ animationEnabled: true }}
/>
<Stack.Screen
name={Routes.CONFIRM_MODAL}
component={ConfirmDappRequest}
options={{ animationEnabled: true }}
/>
</Stack.Navigator>
</NavigationContainer>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ exports[`Approval render matches snapshot 1`] = `
<View
style={
{
"marginBottom": -8,
"marginBottom": 8,
"marginHorizontal": 16,
}
}
Expand Down
24 changes: 7 additions & 17 deletions app/components/Views/confirmations/Confirm/Confirm.styles.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { StyleSheet } from 'react-native';

import Device from '../../../../util/device';
import { Theme } from '../../../../util/theme/models';

const styleSheet = (params: { theme: Theme }) => {
const styleSheet = (params: {
theme: Theme;
}) => {
const { theme } = params;

return StyleSheet.create({
bottomSheetDialogSheet: {
backgroundColor: theme.colors.background.alternative,
},
flatContainer: {
position: 'absolute',
top: 0,
Expand All @@ -16,22 +19,9 @@ const styleSheet = (params: { theme: Theme }) => {
zIndex: 9999,
backgroundColor: theme.colors.background.alternative,
justifyContent: 'space-between',
paddingHorizontal: 16,
},
modalContainer: {
backgroundColor: theme.colors.background.alternative,
scrollView: {
paddingHorizontal: 16,
paddingVertical: 24,
borderTopLeftRadius: 20,
borderTopRightRadius: 20,
paddingBottom: Device.isIphoneX() ? 20 : 0,
height: '85%',
},
scrollableSection: {
padding: 4,
},
scrollable: {
height: '75%',
},
});
};
Expand Down
80 changes: 66 additions & 14 deletions app/components/Views/confirmations/Confirm/Confirm.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import React from 'react';
import { SafeAreaProvider } from 'react-native-safe-area-context';
import { act } from '@testing-library/react-native';

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

import { Confirm } from './Confirm';

jest.mock('@react-navigation/native', () => ({
...jest.requireActual('@react-navigation/native'),
useNavigation: () => ({
addListener: jest.fn(),
dispatch: jest.fn(),
goBack: jest.fn(),
navigate: jest.fn(),
removeListener: jest.fn(),
}),
}));

jest.mock('react-native/Libraries/Linking/Linking', () => ({
addEventListener: jest.fn(),
removeEventListener: jest.fn(),
openURL: jest.fn(),
canOpenURL: jest.fn(),
getInitialURL: jest.fn(),
}));

jest.mock('react-native-safe-area-context', () => {
const inset = { top: 0, right: 0, bottom: 0, left: 0 };
const frame = { width: 0, height: 0, x: 0, y: 0 };

return {
...jest.requireActual('react-native-safe-area-context'),
SafeAreaProvider: jest.fn().mockImplementation(({ children }) => children),
SafeAreaConsumer: jest
.fn()
.mockImplementation(({ children }) => children(inset)),
useSafeAreaInsets: jest.fn().mockImplementation(() => inset),
useSafeAreaFrame: jest.fn().mockImplementation(() => frame),
};
});

jest.mock('../../../../core/Engine', () => ({
getTotalFiatAccountBalance: () => ({ tokenFiat: 10 }),
context: {
Expand All @@ -31,14 +67,10 @@ jest.mock('../../../../core/Engine', () => ({
},
controllerMessenger: {
subscribe: jest.fn(),
unsubscribe: jest.fn(),
},
}));

jest.mock('../../../../util/address', () => ({
...jest.requireActual('../../../../util/address'),
getAddressAccountType: (str: string) => str,
}));

jest.mock('react-native-gzip', () => ({
deflate: (str: string) => str,
}));
Expand All @@ -61,10 +93,22 @@ describe('Confirm', () => {
expect(getByTestId('modal-confirmation-container')).toBeDefined();
});

it('renders correct information for personal sign', async () => {
const { getAllByRole, getByText } = renderWithProvider(<Confirm />, {
state: personalSignatureConfirmationState,
it('renders a flat confirmation for specified type(s): staking deposit', () => {
const { getByTestId } = renderWithProvider(<Confirm />, {
state: stakingDepositConfirmationState,
});
expect(getByTestId('flat-confirmation-container')).toBeDefined();
});

it('renders correct information for personal sign', () => {
const { getAllByRole, getByText } = renderWithProvider(
<SafeAreaProvider>
<Confirm />
</SafeAreaProvider>,
{
state: personalSignatureConfirmationState,
},
);
expect(getByText('Signature request')).toBeDefined();
expect(
getByText('Review request details before you confirm.'),
Expand All @@ -76,11 +120,16 @@ describe('Confirm', () => {
expect(getAllByRole('button')).toHaveLength(2);
});

it('renders correct information for typed sign v1', async () => {
it('renders correct information for typed sign v1', () => {
const { getAllByRole, getAllByText, getByText, queryByText } =
renderWithProvider(<Confirm />, {
state: typedSignV1ConfirmationState,
});
renderWithProvider(
<SafeAreaProvider>
<Confirm />
</SafeAreaProvider>,
{
state: typedSignV1ConfirmationState,
},
);
expect(getByText('Signature request')).toBeDefined();
expect(getByText('Request from')).toBeDefined();
expect(getByText('metamask.github.io')).toBeDefined();
Expand All @@ -102,18 +151,21 @@ describe('Confirm', () => {
expect(getByText('Advanced details')).toBeDefined();
});

it('renders blockaid banner if confirmation has blockaid error response', async () => {
it('renders a blockaid banner if the confirmation has blockaid error response', async () => {
const { getByText } = renderWithProvider(<Confirm />, {
state: {
...typedSignV1ConfirmationState,
signatureRequest: { securityAlertResponse },
},
});

await act(async () => undefined);

expect(getByText('Signature request')).toBeDefined();
expect(getByText('This is a deceptive request')).toBeDefined();
});

it('returns null if re-design is not enabled for confirmation', () => {
it('returns null if confirmation redesign is not enabled', () => {
jest
.spyOn(ConfirmationRedesignEnabled, 'useConfirmationRedesignEnabled')
.mockReturnValue({ isRedesignedEnabled: false });
Expand Down
31 changes: 18 additions & 13 deletions app/components/Views/confirmations/Confirm/Confirm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,36 @@ import {
View,
} from 'react-native';

import BottomSheet from '../../../../component-library/components/BottomSheets/BottomSheet';
import { useStyles } from '../../../../component-library/hooks';
import BottomModal from '../components/UI/BottomModal';
import Footer from '../components/Confirm/Footer';
import { Footer } from '../components/Confirm/Footer';
import Info from '../components/Confirm/Info';
import { LedgerContextProvider } from '../context/LedgerContext';
import { QRHardwareContextProvider } from '../context/QRHardwareContext/QRHardwareContext';
import SignatureBlockaidBanner from '../components/Confirm/SignatureBlockaidBanner';
import Title from '../components/Confirm/Title';
import useApprovalRequest from '../hooks/useApprovalRequest';
import { useConfirmActions } from '../hooks/useConfirmActions';
import { useConfirmationRedesignEnabled } from '../hooks/useConfirmationRedesignEnabled';
import { useFlatConfirmation } from '../hooks/useFlatConfirmation';
import useApprovalRequest from '../hooks/useApprovalRequest';
import { useConfirmActions } from '../hooks/useConfirmActions';
import styleSheet from './Confirm.styles';

const ConfirmWrapped = ({
styles,
testID,
}: {
styles: StyleSheet.NamedStyles<Record<string, unknown>>;
testID?: string;
}) => (
<QRHardwareContextProvider>
<LedgerContextProvider>
<Title />
<ScrollView style={styles.scrollable}>
<TouchableWithoutFeedback>
<View style={styles.scrollableSection}>
<ScrollView style={styles.scrollView}>
<TouchableWithoutFeedback testID={testID}>
<>
<SignatureBlockaidBanner />
<Info />
</View>
</>
</TouchableWithoutFeedback>
</ScrollView>
<Footer />
Expand Down Expand Up @@ -62,10 +64,13 @@ export const Confirm = () => {
}

return (
<BottomModal onClose={onReject} testID="modal-confirmation-container">
<View style={styles.modalContainer} testID={approvalRequest?.type}>
<ConfirmWrapped styles={styles} />
</View>
</BottomModal>
<BottomSheet
isInteractable={false}
onClose={onReject}
style={styles.bottomSheetDialogSheet}
testID="modal-confirmation-container"
>
<ConfirmWrapped testID={approvalRequest?.type} styles={styles} />
</BottomSheet>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ const createStyles = (colors: any) =>
textDecorationLine: 'underline',
...fontStyles.bold,
},
blockaidBanner: {
blockaidBannerContainer: {
marginBottom: 10,
marginTop: 20,
marginHorizontal: 10,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ const createStyles = (colors: any) =>
justifyContent: 'center',
alignItems: 'center',
},
blockaidWarning: {
blockaidBannerContainer: {
marginBottom: 10,
marginTop: 20,
marginHorizontal: 10,
Expand Down
Loading
Loading