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

feat: QR hardware signing in new designs #13261

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,18 @@ const styleSheet = (params: { theme: Theme }) => {
backgroundColor: theme.colors.background.alternative,
paddingHorizontal: 16,
paddingVertical: 24,
minHeight: '70%',
borderTopLeftRadius: 20,
borderTopRightRadius: 20,
paddingBottom: Device.isIphoneX() ? 20 : 0,
justifyContent: 'space-between',
},
scrollableSection: {
flex: 1,
},
scrollWrapper: {
minHeight: '68%',
maxHeight: '68%',
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason for the fixed height? I was able to remove this in this PR and it seems to work. This way no excess space for confirmations with less content.

I do have a PR that is open that applies the BottomSheet and scroll container here. With this, the header and footer are also fixed whilst having scrollable content. We will need to resolve the conflict in one of our PRs

#13268

Copy link
Contributor Author

Choose a reason for hiding this comment

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

min height helpe confirmation page not look too short if there is less content

yep we will need to resolve conflicts

},
});
};

Expand Down
15 changes: 12 additions & 3 deletions app/components/Views/confirmations/Confirm/Confirm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,17 @@ 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('should render correct information for personal sign', async () => {
it('should render correct information for personal sign', () => {
const { getAllByRole, getByText } = renderWithProvider(<Confirm />, {
state: personalSignatureConfirmationState,
});
Expand All @@ -48,7 +57,7 @@ describe('Confirm', () => {
expect(getAllByRole('button')).toHaveLength(2);
});

it('should render correct information for typed sign v1', async () => {
it('should render correct information for typed sign v1', () => {
const { getAllByRole, getAllByText, getByText, queryByText } =
renderWithProvider(<Confirm />, {
state: typedSignV1ConfirmationState,
Expand All @@ -62,7 +71,7 @@ describe('Confirm', () => {
expect(queryByText('This is a deceptive request')).toBeNull();
});

it('should render blockaid banner if confirmation has blockaid error response', async () => {
it('should render blockaid banner if confirmation has blockaid error response', () => {
const { getByText } = renderWithProvider(<Confirm />, {
state: {
...typedSignV1ConfirmationState,
Expand Down
33 changes: 21 additions & 12 deletions app/components/Views/confirmations/Confirm/Confirm.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import React from 'react';
import { View } from 'react-native';
import { ScrollView, View } from 'react-native';

import { useStyles } from '../../../../component-library/hooks';
import BottomModal from '../components/UI/BottomModal';
import AccountNetworkInfo from '../components/Confirm/AccountNetworkInfo';
import Footer from '../components/Confirm/Footer';
import Info from '../components/Confirm/Info';
import { QRHardwareContextProvider } from '../context/QRHardwareContext/QRHardwareContext';
import SignatureBlockaidBanner from '../components/Confirm/SignatureBlockaidBanner';
import Title from '../components/Confirm/Title';
import useApprovalRequest from '../hooks/useApprovalRequest';
Expand All @@ -22,18 +23,26 @@ const Confirm = () => {
}

return (
<BottomModal canCloseOnBackdropClick={false}>
<View style={styles.container} testID={approvalRequest?.type}>
<View>
<Title />
{/* TODO: component SignatureBlockaidBanner to be removed once we implement alert system in mobile */}
<SignatureBlockaidBanner />
<AccountNetworkInfo />
<Info />
<QRHardwareContextProvider>
<BottomModal canCloseOnBackdropClick={false}>
<View style={styles.container} testID={approvalRequest?.type}>
<View>
<Title />
<View style={styles.scrollWrapper}>
<ScrollView>
<View style={styles.scrollableSection}>
Copy link
Contributor

Choose a reason for hiding this comment

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

ScrollView accepts style, contentContainerStyle, and other style props that can be used to avoid adding extra View components around or within ScrollView

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea, I could remove inner view, but not outer one.

{/* TODO: component SignatureBlockaidBanner to be removed once we implement alert system in mobile */}
<SignatureBlockaidBanner />
<AccountNetworkInfo />
<Info />
</View>
</ScrollView>
</View>
</View>
<Footer />
</View>
<Footer />
</View>
</BottomModal>
</BottomModal>
</QRHardwareContextProvider>
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import React from 'react';
import { fireEvent } from '@testing-library/react-native';

import { ConfirmationFooterSelectorIDs } from '../../../../../../../e2e/selectors/Confirmation/ConfirmationView.selectors';
import renderWithProvider from '../../../../../../util/test/renderWithProvider';
import { personalSignatureConfirmationState } from '../../../../../../util/test/confirm-data-helpers';
// eslint-disable-next-line import/no-namespace
import * as QRHardwareHook from '../../../context/QRHardwareContext/QRHardwareContext';
import Footer from './index';
import { fireEvent } from '@testing-library/react-native';

const mockConfirmSpy = jest.fn();
const mockRejectSpy = jest.fn();
Expand All @@ -15,7 +18,7 @@ jest.mock('../../../hooks/useConfirmActions', () => ({
}));

describe('Footer', () => {
it('should render correctly', async () => {
it('should render correctly', () => {
const { getByText, getAllByRole } = renderWithProvider(<Footer />, {
state: personalSignatureConfirmationState,
});
Expand All @@ -24,19 +27,45 @@ describe('Footer', () => {
expect(getAllByRole('button')).toHaveLength(2);
});

it('should call onConfirm when confirm button is clicked', async () => {
it('should call onConfirm when confirm button is clicked', () => {
const { getByText } = renderWithProvider(<Footer />, {
state: personalSignatureConfirmationState,
});
fireEvent.press(getByText('Confirm'));
expect(mockConfirmSpy).toHaveBeenCalledTimes(1);
});

it('should call onReject when reject button is clicked', async () => {
it('should call onReject when reject button is clicked', () => {
const { getByText } = renderWithProvider(<Footer />, {
state: personalSignatureConfirmationState,
});
fireEvent.press(getByText('Reject'));
expect(mockRejectSpy).toHaveBeenCalledTimes(1);
});

it('render confirm button text "Get Signature" if QR signing is in progress', () => {
jest
.spyOn(QRHardwareHook, 'useQRHardwareContext')
.mockReturnValue({
isQRSigningInProgress: true,
} as unknown as QRHardwareHook.QRHardwareContextType);
const { getByText } = renderWithProvider(<Footer />, {
state: personalSignatureConfirmationState,
});
expect(getByText('Get Signature')).toBeDefined();
});

it('confirm button is disabled if `needsCameraPermission` is true', () => {
jest
.spyOn(QRHardwareHook, 'useQRHardwareContext')
.mockReturnValue({
needsCameraPermission: true,
} as unknown as QRHardwareHook.QRHardwareContextType);
const { getByTestId } = renderWithProvider(<Footer />, {
state: personalSignatureConfirmationState,
});
expect(
getByTestId(ConfirmationFooterSelectorIDs.CONFIRM_BUTTON).props.disabled,
).toBe(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@ import Button, {
ButtonWidthTypes,
} from '../../../../../../component-library/components/Buttons/Button';
import { useStyles } from '../../../../../../component-library/hooks';
import { ResultType } from '../../../constants/signatures';
import { useConfirmActions } from '../../../hooks/useConfirmActions';
import { useSecurityAlertResponse } from '../../../hooks/useSecurityAlertResponse';
import { useQRHardwareContext } from '../../../context/QRHardwareContext/QRHardwareContext';
import { ResultType } from '../../BlockaidBanner/BlockaidBanner.types';
import styleSheet from './Footer.styles';

const Footer = () => {
const { onConfirm, onReject } = useConfirmActions();
const { isQRSigningInProgress, needsCameraPermission } =
useQRHardwareContext();
const { securityAlertResponse } = useSecurityAlertResponse();

const { styles } = useStyles(styleSheet, {});
Expand All @@ -34,13 +37,18 @@ const Footer = () => {
<View style={styles.buttonDivider} />
<Button
onPress={onConfirm}
label={strings('confirm.confirm')}
label={
isQRSigningInProgress
? strings('confirm.qr_get_sign')
: strings('confirm.confirm')
}
style={styles.footerButton}
size={ButtonSize.Lg}
testID={ConfirmationFooterSelectorIDs.CONFIRM_BUTTON}
variant={ButtonVariants.Primary}
width={ButtonWidthTypes.Full}
isDanger={securityAlertResponse?.result_type === ResultType.Malicious}
disabled={needsCameraPermission}
/>
</View>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,32 @@ import React from 'react';

import renderWithProvider from '../../../../../../util/test/renderWithProvider';
import { personalSignatureConfirmationState } from '../../../../../../util/test/confirm-data-helpers';
// eslint-disable-next-line import/no-namespace
import * as QRHardwareHook from '../../../context/QRHardwareContext/QRHardwareContext';
import Info from './Info';
import { Text } from 'react-native';

const MockText = Text;
jest.mock('./QRInfo', () => () => <MockText>QR Scanning Component</MockText>);

describe('Info', () => {
it('should render correctly for personal sign', async () => {
it('renders correctly for personal sign', () => {
const { getByText } = renderWithProvider(<Info />, {
state: personalSignatureConfirmationState,
});
expect(getByText('Message')).toBeDefined();
expect(getByText('Example `personal_sign` message')).toBeDefined();
});

it('renders QRInfo if user is signing using QR hardware', () => {
jest
.spyOn(QRHardwareHook, 'useQRHardwareContext')
.mockReturnValue({
isSigningQRObject: true,
} as unknown as QRHardwareHook.QRHardwareContextType);
const { getByText } = renderWithProvider(<Info />, {
state: personalSignatureConfirmationState,
});
expect(getByText('QR Scanning Component')).toBeDefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import { TransactionType } from '@metamask/transaction-controller';
import React from 'react';

import useApprovalRequest from '../../../hooks/useApprovalRequest';
import { useQRHardwareContext } from '../../../context/QRHardwareContext/QRHardwareContext';
import PersonalSign from './PersonalSign';
import QRInfo from './QRInfo';
import TypedSignV1 from './TypedSignV1';
import TypedSignV3V4 from './TypedSignV3V4';

Expand All @@ -16,11 +18,16 @@ const ConfirmationInfoComponentMap = {

const Info = () => {
const { approvalRequest } = useApprovalRequest();
const { isSigningQRObject } = useQRHardwareContext();

if (!approvalRequest?.type) {
return null;
}

if (isSigningQRObject) {
return <QRInfo />;
}

const { requestData } = approvalRequest ?? {
requestData: {},
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { StyleSheet } from 'react-native';

import { Theme } from '../../../../../../../util/theme/models';
import { fontStyles } from '../../../../../../../styles/common';

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

return StyleSheet.create({
container: {
width: '100%',
flexDirection: 'column',
alignItems: 'center',
backgroundColor: theme.colors.background.default,
},
title: {
flexDirection: 'row',
justifyContent: 'center',
marginTop: 20,
marginBottom: 20,
},
titleText: {
...fontStyles.normal,
fontSize: 14,
color: theme.colors.text.default,
},
errorText: {
...fontStyles.normal,
fontSize: 12,
color: theme.colors.error.default,
},
alert: {
marginHorizontal: 16,
marginTop: 12,
},
});
};

export default styleSheet;
Loading
Loading