Skip to content

Commit

Permalink
fix: Revert "feat: Changing title for permit requests (#28537)" (#28734)
Browse files Browse the repository at this point in the history
## **Description**

Revert changes in the PR:
#28537

## **Related issues**
Ref: MetaMask/MetaMask-planning#3633

## **Manual testing steps**

1. Enable permit signature decoding locally
2. Go to test dapp
3. Check title and description of permit pages


## **Screenshots/Recordings**
TODO

## **Pre-merge author checklist**

- [X] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [X] I've completed the PR template to the best of my ability
- [X] I’ve included tests if applicable
- [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [X] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.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.
  • Loading branch information
jpuri authored Nov 26, 2024
1 parent 815c445 commit d24389c
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 10 deletions.
4 changes: 2 additions & 2 deletions test/e2e/tests/confirmations/signatures/nft-permit.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,9 @@ async function assertInfoValues(driver: Driver) {
text: '0x581c3...45947',
});

const title = driver.findElement({ text: 'Signature request' });
const title = driver.findElement({ text: 'Withdrawal request' });
const description = driver.findElement({
text: 'Review request details before you confirm.',
text: 'This site wants permission to withdraw your NFTs',
});
const primaryType = driver.findElement({ text: 'Permit' });
const spender = driver.findElement({
Expand Down
4 changes: 2 additions & 2 deletions test/integration/confirmations/signatures/permit.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,9 @@ describe('Permit Confirmation', () => {
});

await waitFor(() => {
expect(screen.getByText('Signature request')).toBeInTheDocument();
expect(screen.getByText('Spending cap request')).toBeInTheDocument();
expect(
screen.getByText('Review request details before you confirm.'),
screen.getByText('This site wants permission to spend your tokens.'),
).toBeInTheDocument();
});
});
Expand Down
35 changes: 35 additions & 0 deletions ui/pages/confirmations/components/confirm/title/title.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,13 @@ import {
getMockPersonalSignConfirmStateForRequest,
getMockSetApprovalForAllConfirmState,
getMockTypedSignConfirmState,
getMockTypedSignConfirmStateForRequest,
} from '../../../../../../test/data/confirmations/helper';
import { unapprovedPersonalSignMsg } from '../../../../../../test/data/confirmations/personal_sign';
import {
permitNFTSignatureMsg,
permitSignatureMsg,
} from '../../../../../../test/data/confirmations/typed_sign';
import { renderWithConfirmContextProvider } from '../../../../../../test/lib/confirmations/render-helpers';
import { tEn } from '../../../../../../test/lib/i18n-helpers';
import {
Expand Down Expand Up @@ -54,6 +59,36 @@ describe('ConfirmTitle', () => {
).toBeInTheDocument();
});

it('should render the title and description for a permit signature', () => {
const mockStore = configureMockStore([])(
getMockTypedSignConfirmStateForRequest(permitSignatureMsg),
);
const { getByText } = renderWithConfirmContextProvider(
<ConfirmTitle />,
mockStore,
);

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

it('should render the title and description for a NFT permit signature', () => {
const mockStore = configureMockStore([])(
getMockTypedSignConfirmStateForRequest(permitNFTSignatureMsg),
);
const { getByText } = renderWithConfirmContextProvider(
<ConfirmTitle />,
mockStore,
);

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

it('should render the title and description for typed signature', () => {
const mockStore = configureMockStore([])(getMockTypedSignConfirmState());
const { getByText } = renderWithConfirmContextProvider(
Expand Down
22 changes: 22 additions & 0 deletions ui/pages/confirmations/components/confirm/title/title.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
} from '@metamask/transaction-controller';
import React, { memo, useMemo } from 'react';

import { TokenStandard } from '../../../../../../shared/constants/transaction';
import GeneralAlert from '../../../../../components/app/alert-system/general-alert/general-alert';
import { Box, Text } from '../../../../../components/component-library';
import {
Expand All @@ -13,6 +14,7 @@ import {
} from '../../../../../helpers/constants/design-system';
import useAlerts from '../../../../../hooks/useAlerts';
import { useI18nContext } from '../../../../../hooks/useI18nContext';
import { TypedSignSignaturePrimaryTypes } from '../../../constants';
import { useConfirmContext } from '../../../context/confirm';
import { Confirmation, SignatureRequestType } from '../../../types/confirm';
import { isSIWESignatureRequest } from '../../../utils';
Expand Down Expand Up @@ -59,6 +61,8 @@ const getTitle = (
customSpendingCap?: string,
isRevokeSetApprovalForAll?: boolean,
pending?: boolean,
primaryType?: keyof typeof TypedSignSignaturePrimaryTypes,
tokenStandard?: string,
) => {
if (pending) {
return '';
Expand All @@ -75,6 +79,12 @@ const getTitle = (
}
return t('confirmTitleSignature');
case TransactionType.signTypedData:
if (primaryType === TypedSignSignaturePrimaryTypes.PERMIT) {
if (tokenStandard === TokenStandard.ERC721) {
return t('setApprovalForAllRedesignedTitle');
}
return t('confirmTitlePermitTokens');
}
return t('confirmTitleSignature');
case TransactionType.tokenMethodApprove:
if (isNFT) {
Expand Down Expand Up @@ -103,6 +113,8 @@ const getDescription = (
customSpendingCap?: string,
isRevokeSetApprovalForAll?: boolean,
pending?: boolean,
primaryType?: keyof typeof TypedSignSignaturePrimaryTypes,
tokenStandard?: string,
) => {
if (pending) {
return '';
Expand All @@ -119,6 +131,12 @@ const getDescription = (
}
return t('confirmTitleDescSign');
case TransactionType.signTypedData:
if (primaryType === TypedSignSignaturePrimaryTypes.PERMIT) {
if (tokenStandard === TokenStandard.ERC721) {
return t('confirmTitleDescApproveTransaction');
}
return t('confirmTitleDescPermitSignature');
}
return t('confirmTitleDescSign');
case TransactionType.tokenMethodApprove:
if (isNFT) {
Expand Down Expand Up @@ -177,6 +195,8 @@ const ConfirmTitle: React.FC = memo(() => {
customSpendingCap,
isRevokeSetApprovalForAll,
spendingCapPending || revokePending,
primaryType,
tokenStandard,
),
[
currentConfirmation,
Expand All @@ -199,6 +219,8 @@ const ConfirmTitle: React.FC = memo(() => {
customSpendingCap,
isRevokeSetApprovalForAll,
spendingCapPending || revokePending,
primaryType,
tokenStandard,
),
[
currentConfirmation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,12 +457,12 @@ exports[`Confirm should match snapshot for signature - typed sign - V4 - PermitB
<h2
class="mm-box mm-text mm-text--heading-lg mm-text--text-align-center mm-box--padding-top-4 mm-box--padding-bottom-4 mm-box--color-text-default"
>
Signature request
Spending cap request
</h2>
<p
class="mm-box mm-text mm-text--body-md mm-text--text-align-center mm-box--padding-bottom-4 mm-box--color-text-alternative"
>
Review request details before you confirm.
This site wants permission to spend your tokens.
</p>
<div
class="mm-box mm-box--margin-bottom-4 mm-box--padding-2 mm-box--background-color-background-default mm-box--rounded-md"
Expand Down Expand Up @@ -1111,12 +1111,12 @@ exports[`Confirm should match snapshot for signature - typed sign - V4 - PermitS
<h2
class="mm-box mm-text mm-text--heading-lg mm-text--text-align-center mm-box--padding-top-4 mm-box--padding-bottom-4 mm-box--color-text-default"
>
Signature request
Spending cap request
</h2>
<p
class="mm-box mm-text mm-text--body-md mm-text--text-align-center mm-box--padding-bottom-4 mm-box--color-text-alternative"
>
Review request details before you confirm.
This site wants permission to spend your tokens.
</p>
<div
class="mm-box mm-box--margin-bottom-4 mm-box--padding-2 mm-box--background-color-background-default mm-box--rounded-md"
Expand Down Expand Up @@ -2794,12 +2794,12 @@ exports[`Confirm should match snapshot for signature - typed sign - permit 1`] =
<h2
class="mm-box mm-text mm-text--heading-lg mm-text--text-align-center mm-box--padding-top-4 mm-box--padding-bottom-4 mm-box--color-text-default"
>
Signature request
Spending cap request
</h2>
<p
class="mm-box mm-text mm-text--body-md mm-text--text-align-center mm-box--padding-bottom-4 mm-box--color-text-alternative"
>
Review request details before you confirm.
This site wants permission to spend your tokens.
</p>
<div
class="mm-box mm-box--margin-bottom-4 mm-box--padding-2 mm-box--background-color-background-default mm-box--rounded-md"
Expand Down

0 comments on commit d24389c

Please sign in to comment.