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

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 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
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,
stylesDialogSheet,
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}
stylesDialogSheet={stylesDialogSheet}
>
{children}
</BottomSheetDialog>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@ const styleSheet = (params: {
}) => {
const { vars, theme } = params;
const { colors, shadows } = theme;
const { maxSheetHeight, screenBottomPadding, isFullscreen } = vars;
const {
isFullscreen,
maxSheetHeight,
screenBottomPadding,
stylesDialogSheet,
} = vars;

return StyleSheet.create({
base: Object.assign({
Expand All @@ -30,7 +35,7 @@ const styleSheet = (params: {
left: 0,
right: 0,
} as ViewStyle) as ViewStyle,
sheet: {
sheet: Object.assign({
backgroundColor: colors.background.default,
borderTopLeftRadius: 8,
borderTopRightRadius: 8,
Expand All @@ -41,7 +46,9 @@ const styleSheet = (params: {
borderColor: colors.border.muted,
...(isFullscreen && { height: maxSheetHeight }),
...shadows.size.lg,
},
},
stylesDialogSheet,
) 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,
stylesDialogSheet = {},
...props
},
ref,
Expand All @@ -71,6 +72,7 @@ const BottomSheetDialog = forwardRef<
const { styles } = useStyles(styleSheet, {
maxSheetHeight,
screenBottomPadding,
stylesDialogSheet,
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 @@ -27,6 +27,10 @@ export interface BottomSheetDialogProps extends ViewProps {
* Optional callback that gets triggered when sheet is opened.
*/
onOpen?: (hasPendingAction?: boolean) => void;
/**
* Optional sheet styles
*/
stylesDialogSheet?: StyleProp<ViewStyle>;
}

export interface BottomSheetDialogRef {
Expand All @@ -40,5 +44,6 @@ export interface BottomSheetDialogRef {
export interface BottomSheetDialogStyleSheetVars {
maxSheetHeight: number;
screenBottomPadding: number;
stylesDialogSheet: StyleProp<ViewStyle>;
isFullscreen: boolean;
}
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,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixes missing padding between blockaid banner alert and account header

"marginHorizontal": 16,
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,15 @@ exports[`Approve should render transaction approval 1`] = `
}
}
>
<View
style={
{
"marginBottom": 10,
"marginHorizontal": 10,
"marginTop": 20,
}
}
/>
<Text
accessibilityRole="text"
style={
Expand Down
27 changes: 6 additions & 21 deletions app/components/Views/confirmations/Confirm/Confirm.styles.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import { StyleSheet } from 'react-native';

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

const styleSheet = (params: {
theme: Theme;
vars: { isFlatConfirmation: boolean };
theme: Theme;
}) => {
const {
theme,
vars: { isFlatConfirmation },
} = params;

return StyleSheet.create({
bottomSheetDialogSheet: {
backgroundColor: theme.colors.background.alternative,
},
flatContainer: {
position: 'absolute',
top: 0,
Expand All @@ -24,25 +25,9 @@ const styleSheet = (params: {
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,
maxHeight: '90%',
},
scrollableSection: {
padding: 4,
},
scrollable: {
minHeight: '100%',
},
scrollWrapper: {
minHeight: isFlatConfirmation ? '100%' : '75%',
maxHeight: isFlatConfirmation ? '100%' : '75%',
margin: 0,
height: isFlatConfirmation ? '100%' : undefined,
},
});
};
Expand Down
116 changes: 81 additions & 35 deletions app/components/Views/confirmations/Confirm/Confirm.test.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,53 @@
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 {
personalSignatureConfirmationState,
securityAlertResponse,
typedSignV1ConfirmationState,
stakingDepositConfirmationState,
typedSignV1ConfirmationState,
} from '../../../../util/test/confirm-data-helpers';
// eslint-disable-next-line import/no-namespace
import * as ConfirmationRedesignEnabled from '../hooks/useConfirmationRedesignEnabled';

import Confirm from './index';

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 @@ -24,46 +60,43 @@ 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,
}));

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

describe('Confirm', () => {
it('renders flat confirmation', async () => {
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 modal confirmation', async () => {
const { getByTestId } = renderWithProvider(<Confirm />, {
state: typedSignV1ConfirmationState,
});
it('renders a modal confirmation', () => {
const { getByTestId } = renderWithProvider(
<SafeAreaProvider>
<Confirm />
</SafeAreaProvider>,
{
state: typedSignV1ConfirmationState,
},
);
expect(getByTestId('modal-confirmation-container')).toBeDefined();
});

it('renders correct information for personal sign', async () => {
const { getAllByRole, getByText } = renderWithProvider(<Confirm />, {
state: personalSignatureConfirmationState,
});
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 @@ -75,11 +108,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 @@ -89,18 +127,26 @@ describe('Confirm', () => {
expect(queryByText('This is a deceptive request')).toBeNull();
});

it('renders blockaid banner if confirmation has blockaid error response', async () => {
const { getByText } = renderWithProvider(<Confirm />, {
state: {
...typedSignV1ConfirmationState,
signatureRequest: { securityAlertResponse },
it('renders a blockaid banner if the confirmation has blockaid error response', async () => {
const { getByText } = renderWithProvider(
<SafeAreaProvider>
<Confirm />
</SafeAreaProvider>,
{
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
Loading
Loading