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: Add Redesign Permit support; fix: InfoRow padding and alignment #13369

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
499bd7c
feat: redesign Permit Title
digiwand Feb 5, 2025
52fa484
style: add additional InfoSection vertical padding
digiwand Feb 5, 2025
8b47c9d
feat: create InfoRowDivider
digiwand Feb 5, 2025
eb76d78
style: fix inconsistent spacing between InfoRow
digiwand Feb 5, 2025
fa20ff0
style: fix InfoRow padding and alignment
digiwand Feb 5, 2025
2f99b44
feat: add redesign Permit support
digiwand Feb 5, 2025
2121a96
style: add padding between InfoRow label and value
digiwand Feb 5, 2025
b30458e
style: fix Message DataRow paddingBottom
digiwand Feb 5, 2025
328e6c0
style: fix extra InfoRow padding with empty label
digiwand Feb 5, 2025
135c750
feat: add Interacting with Permit row
digiwand Feb 6, 2025
dc1e9ae
fix: move tsx key TypedSignPermit
digiwand Feb 6, 2025
5cc3ed9
feat: fix and finalize Permit NFT support
digiwand Feb 6, 2025
e573974
test:fix: toBeDefined -> toBeTruthy
digiwand Feb 6, 2025
1155603
Merge branch 'main' into feat-redesign-permit
digiwand Feb 6, 2025
0d5d1f6
test:fix toBeDefined -> toBeTruthy
digiwand Feb 6, 2025
3d7e489
test: update DisplayURL test - replaces snapshot test and fixes toBeD…
digiwand Feb 6, 2025
6756c8e
Merge branch 'main' into feat-redesign-permit
digiwand Feb 7, 2025
6247124
Merge branch 'main' into feat-redesign-permit
digiwand Feb 11, 2025
674dabe
refactor: rn InfoSectionAddressAndOrigin -> InfoSectionOriginAndDetails
digiwand Feb 11, 2025
c021969
refactor: use confirm.label en.json
digiwand Feb 11, 2025
3a7586e
fix: add missing label.network en.json updated
digiwand Feb 12, 2025
6d97d98
fix: confirm.label.request_from
digiwand Feb 13, 2025
d0d9132
fix: lint error binding element has no default value
digiwand Feb 13, 2025
65419d1
Merge branch 'main' into feat-redesign-permit
digiwand Feb 13, 2025
901094f
fix: prevent leaky value w/ Boolean(label)
digiwand Feb 13, 2025
7224818
Merge branch 'main' into feat-redesign-permit
digiwand Feb 14, 2025
cc83559
fix: merge conflict extra )
digiwand Feb 14, 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 @@ -71,7 +71,7 @@ const createStyles = (depth: number) =>
},
dataRow: {
paddingHorizontal: 0,
paddingBottom: 16,
paddingBottom: 8,
},
});

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { StyleSheet } from 'react-native';

const styleSheet = () => StyleSheet.create({
dividerContainer: {
marginTop: 4,
marginBottom: 12,
},
});

export default styleSheet;
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import React from 'react';

import renderWithProvider from '../../../../../../../../util/test/renderWithProvider';
import {
typedSignV3ConfirmationState,
typedSignV4ConfirmationState,
} from '../../../../../../../../util/test/confirm-data-helpers';
import { InfoSectionAddressAndOrigin } from './InfoSectionAddressAndOrigin';

describe('InfoSectionAddressAndOrigin', () => {
it('renders origin', () => {
const { getByText } = renderWithProvider(<InfoSectionAddressAndOrigin />, {
state: typedSignV4ConfirmationState,
});

expect(getByText('Request from')).toBeTruthy();
expect(getByText('metamask.github.io')).toBeTruthy();
});

it('renders "Interacting with" if associated with a valid verifying contract', () => {
const { getByText } = renderWithProvider(<InfoSectionAddressAndOrigin />, {
state: typedSignV4ConfirmationState,
});

expect(getByText('Request from')).toBeTruthy();
});

it('renders Spender if it is a Permit', () => {
const { getByText } = renderWithProvider(<InfoSectionAddressAndOrigin />, {
state: typedSignV4ConfirmationState,
});

expect(getByText('Interacting with')).toBeTruthy();
expect(getByText('0xCcCCc...ccccC')).toBeTruthy();
});

it('does not render Spender if it is not a Permit', () => {
const { queryByText } = renderWithProvider(<InfoSectionAddressAndOrigin />, {
state: typedSignV3ConfirmationState,
});

expect(queryByText('Spender')).toBeNull();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import React from 'react';

import { ConfirmationPageSectionsSelectorIDs } from '../../../../../../../../../e2e/selectors/Confirmation/ConfirmationView.selectors';
import { strings } from '../../../../../../../../../locales/i18n';
import { useStyles } from '../../../../../../../../component-library/hooks';
import InfoRow from '../../../../UI/InfoRow';
import { InfoRowDivider } from '../../../../UI/InfoRow/Divider/Divider';
import InfoSection from '../../../../UI/InfoRow/InfoSection';
import InfoRowAddress from '../../../../UI/InfoRow/InfoValue/Address';
import DisplayURL from '../../../../UI/InfoRow/InfoValue/DisplayURL';
import { isRecognizedPermit, parseTypedDataMessageFromSignatureRequest } from '../../../../../utils/signature';
import { useSignatureRequest } from '../../../../../hooks/useSignatureRequest';
import useApprovalRequest from '../../../../../hooks/useApprovalRequest';
import { View } from 'react-native';
import styleSheet from './InfoSectionAddressAndOrigin.styles';
import { isValidAddress } from 'ethereumjs-util';

export const InfoSectionAddressAndOrigin = () => {
const { styles } = useStyles(styleSheet, {});

// signatureRequest from SignatureController does not include the origin
// so we need to use approvalRequest
const { approvalRequest } = useApprovalRequest();
const origin = approvalRequest?.origin as string;

const signatureRequest = useSignatureRequest();
const isPermit = isRecognizedPermit(signatureRequest);

const {
domain: { verifyingContract } = {},

Check failure on line 30 in app/components/Views/confirmations/components/Confirm/Info/TypedSignV3V4/InfoSectionAddressAndOrigin/InfoSectionAddressAndOrigin.tsx

View workflow job for this annotation

GitHub Actions / scripts (lint:tsc)

Initializer provides no value for this binding element and the binding element has no default value.
message: { spender } = {},

Check failure on line 31 in app/components/Views/confirmations/components/Confirm/Info/TypedSignV3V4/InfoSectionAddressAndOrigin/InfoSectionAddressAndOrigin.tsx

View workflow job for this annotation

GitHub Actions / scripts (lint:tsc)

Initializer provides no value for this binding element and the binding element has no default value.
} = parseTypedDataMessageFromSignatureRequest(signatureRequest) ?? {};

if (!signatureRequest) {
return null;
}

const chainId = signatureRequest.chainId;

return (
<InfoSection
testID={ConfirmationPageSectionsSelectorIDs.ORIGIN_INFO_SECTION}
>
{isPermit && spender && (
<>
<InfoRow label={strings('confirm.label.spender')}>
<InfoRowAddress address={spender} chainId={chainId} />
</InfoRow>
<View style={styles.dividerContainer}>
<InfoRowDivider />
</View>
Copy link
Contributor

Choose a reason for hiding this comment

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

It will be better to be able to pass styles to divider row itself rather than wrap it in another view.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's better to avoid it in this case because the dividerContainer has these styles

    dividerContainer: {
      marginTop: 4,
      marginBottom: 12,
    },

for better reusability, it is better to keep outer margins and paddings separate. I think we can reconsider this once we have more cases using InfoRowDivider

</>
)}
<InfoRow
label={strings('confirm.request_from')}
tooltip={strings('confirm.personal_sign_tooltip')}
>
<DisplayURL url={origin} />
</InfoRow>
{isValidAddress(verifyingContract) && (
<InfoRow label={strings('confirm.label.interacting_with')}>
<InfoRowAddress
address={verifyingContract}
chainId={chainId}
/>
</InfoRow>
)}
</InfoSection>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { InfoSectionAddressAndOrigin } from './InfoSectionAddressAndOrigin';
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const styleSheet = (params: { theme: Theme }) => {
},
dataRow: {
paddingHorizontal: 0,
paddingBottom: 16,
paddingBottom: 8,
},
title: {
color: theme.colors.text.default,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import { waitFor } from '@testing-library/react-native';

import renderWithProvider from '../../../../../../../../../util/test/renderWithProvider';
import { typedSignV4ConfirmationState } from '../../../../../../../../../util/test/confirm-data-helpers';
import { typedSignV4ConfirmationState, typedSignV4NFTConfirmationState } from '../../../../../../../../../util/test/confirm-data-helpers';
import PermitSimulation from './TypedSignPermit';

jest.mock('../../../../../../../../../core/Engine', () => ({
Expand All @@ -14,20 +14,37 @@ jest.mock('../../../../../../../../../core/Engine', () => ({
}));

describe('PermitSimulation', () => {
it('should render correctly for personal sign', async () => {
it('should render correctly for Permit', async () => {
const { getByText } = renderWithProvider(<PermitSimulation />, {
state: typedSignV4ConfirmationState,
});

expect(getByText('Estimated changes')).toBeDefined();
expect(getByText('Estimated changes')).toBeTruthy();
expect(
getByText(
"You're giving the spender permission to spend this many tokens from your account.",
),
).toBeDefined();
expect(getByText('Spending cap')).toBeDefined();
expect(getByText('0xCcCCc...ccccC')).toBeDefined();
).toBeTruthy();
expect(getByText('Spending cap')).toBeTruthy();
expect(getByText('0xCcCCc...ccccC')).toBeTruthy();

await waitFor(() => expect(getByText('3,000')).toBeDefined());
await waitFor(() => expect(getByText('3,000')).toBeTruthy());
});

it('should render correctly for Permit NFTs', async () => {
const { getByText } = renderWithProvider(<PermitSimulation />, {
state: typedSignV4NFTConfirmationState,
});

expect(getByText('Estimated changes')).toBeTruthy();
expect(
getByText(
"You're giving the spender permission to spend this many tokens from your account.",
),
).toBeTruthy();
expect(getByText('Withdraw')).toBeTruthy();
expect(getByText('0xC3644...1FE88')).toBeTruthy();

await waitFor(() => expect(getByText('#3606393')).toBeTruthy());
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ const PermitSimulation = () => {

const tokenDetails = extractTokenDetailsByPrimaryType(message, primaryType);

const isNFT = tokenId !== undefined;
const isNFT = tokenId !== undefined && tokenId !== '0';

const labelChangeType = isNFT
? strings('confirm.simulation.label_change_type_permit_nft')
: strings('confirm.simulation.label_change_type_permit');
Expand All @@ -87,24 +88,25 @@ const PermitSimulation = () => {

<InfoRow label={labelChangeType}>
{Array.isArray(tokenDetails) ? (
<View style={styles.permitValues}>
<>
{tokenDetails.map(
(
{ token, amount }: { token: string; amount: string },
i: number,
) => (
<PermitSimulationValueDisplay
key={`${token}-${i}`}
labelChangeType={labelChangeType}
networkClientId={networkClientId}
primaryType={primaryType}
tokenContract={safeToChecksumAddress(token)}
value={amount}
chainId={chainId}
/>
<View style={styles.permitValues} key={`${token}-${i}`}>
<PermitSimulationValueDisplay
labelChangeType={labelChangeType}
networkClientId={networkClientId}
primaryType={primaryType}
tokenContract={safeToChecksumAddress(token)}
value={amount}
chainId={chainId}
/>
</View>
),
)}
</View>
</>
) : (
<PermitSimulationValueDisplay
labelChangeType={labelChangeType}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,14 @@ const SimulationValueDisplay: React.FC<SimulationValueDisplayParams> = ({
tokenDetails as TokenDetailsERC20,
);

const isNFT = tokenId !== undefined && tokenId !== '0';

const tokenAmount =
isNumberValue(value) && !tokenId
? calcTokenAmount(value as number | string, tokenDecimals)
: null;
const isValidTokenAmount =
!isNFT &&
tokenAmount !== null &&
tokenAmount !== undefined &&
tokenAmount instanceof BigNumber;
Expand Down Expand Up @@ -163,7 +166,7 @@ const SimulationValueDisplay: React.FC<SimulationValueDisplayParams> = ({
{
<AnimatedPulse isPulsing={isPendingTokenDetails} testID="simulation-value-display-loader">
<ButtonPill
isDisabled={!!tokenId || tokenId === '0'}
isDisabled={isNFT}
onPress={handlePressTokenValue}
onPressIn={handlePressTokenValue}
onPressOut={handlePressTokenValue}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import React from 'react';
import InfoRowOrigin from '../Shared/InfoRowOrigin';
import { InfoSectionAddressAndOrigin } from './InfoSectionAddressAndOrigin';
import Message from './Message';
import TypedSignV3V4Simulation from './Simulation';

const TypedSignV3V4 = () => (
<>
<TypedSignV3V4Simulation />
<InfoRowOrigin />
<InfoSectionAddressAndOrigin />
digiwand marked this conversation as resolved.
Show resolved Hide resolved
<Message />
</>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,30 +1,54 @@
import React from 'react';

import renderWithProvider from '../../../../../../util/test/renderWithProvider';
import {
personalSignatureConfirmationState,
siweSignatureConfirmationState,
typedSignV4ConfirmationState,
typedSignV4NFTConfirmationState,
} from '../../../../../../util/test/confirm-data-helpers';
import renderWithProvider from '../../../../../../util/test/renderWithProvider';
import Title from './Title';

describe('Title', () => {
it('should render correct title and subtitle for personal sign request', async () => {
describe('Confirm Title', () => {
it('renders the title and subtitle for a permit signature', () => {
const { getByText } = renderWithProvider(<Title />, {
state: typedSignV4ConfirmationState,
});

expect(getByText('Spending cap request')).toBeTruthy();
expect(
getByText('This site wants permission to spend your tokens.'),
).toBeTruthy();
});

it('renders the title and subtitle for a permit NFT signature', () => {
const { getByText } = renderWithProvider(<Title />, {
state: typedSignV4NFTConfirmationState,
});

expect(getByText('Withdrawal request')).toBeTruthy();
expect(
getByText('This site wants permission to withdraw your NFTs.'),
).toBeTruthy();
});

it('renders correct title and subtitle for personal sign request', () => {
const { getByText } = renderWithProvider(<Title />, {
state: personalSignatureConfirmationState,
});
expect(getByText('Signature request')).toBeDefined();
expect(getByText('Signature request')).toBeTruthy();
expect(
getByText('Review request details before you confirm.'),
).toBeDefined();
).toBeTruthy();
});

it('should render correct title and subtitle for personal siwe request', async () => {
it('should render correct title and subtitle for personal siwe request', () => {
const { getByText } = renderWithProvider(<Title />, {
state: siweSignatureConfirmationState,
});
expect(getByText('Sign-in request')).toBeDefined();
expect(getByText('Sign-in request')).toBeTruthy();
expect(
getByText('A site wants you to sign in to prove you own this account.'),
).toBeDefined();
).toBeTruthy();
});
});
Loading
Loading