Skip to content

Commit

Permalink
fix: Add different copy for tooltip when a snap is requesting a signa…
Browse files Browse the repository at this point in the history
…ture (#27492)

## **Description**

Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change? The tooltip content incorrectly
says "site" when a snap is requesting a signature
2. What is the improvement/solution? Changing the copy to say "snap" for
when a snap is making the signature request.

## **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**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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
hmalik88 authored Nov 4, 2024
1 parent 49e5e78 commit 7a8da64
Show file tree
Hide file tree
Showing 7 changed files with 244 additions and 3 deletions.
3 changes: 3 additions & 0 deletions app/_locales/en/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
} from '../../../../../../../test/data/confirmations/helper';
import { renderWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers';
import { signatureRequestSIWE } from '../../../../../../../test/data/confirmations/personal_sign';
import * as utils from '../../../../utils';
import * as snapUtils from '../../../../../../helpers/utils/snaps';
import PersonalSignInfo from './personal-sign';

jest.mock(
Expand All @@ -21,6 +23,29 @@ jest.mock(
}),
);

jest.mock('../../../../utils', () => {
const originalUtils = jest.requireActual('../../../../utils');
return {
...originalUtils,
isSIWESignatureRequest: jest.fn().mockReturnValue(false),
};
});

jest.mock('../../../../../../../node_modules/@metamask/snaps-utils', () => {
const originalUtils = jest.requireActual(
'../../../../../../../node_modules/@metamask/snaps-utils',
);
return {
...originalUtils,
stripSnapPrefix: jest.fn().mockReturnValue('@metamask/examplesnap'),
getSnapPrefix: jest.fn().mockReturnValue('npm:'),
};
});

jest.mock('../../../../../../helpers/utils/snaps', () => ({
isSnapId: jest.fn(),
}));

describe('PersonalSignInfo', () => {
it('renders correctly for personal sign request', () => {
const state = getMockPersonalSignConfirmState();
Expand Down Expand Up @@ -62,6 +87,7 @@ describe('PersonalSignInfo', () => {
});

it('display signing in from for SIWE request', () => {
(utils.isSIWESignatureRequest as jest.Mock).mockReturnValue(true);
const state =
getMockPersonalSignConfirmStateForRequest(signatureRequestSIWE);
const mockStore = configureMockStore([])(state);
Expand All @@ -73,6 +99,7 @@ describe('PersonalSignInfo', () => {
});

it('display simulation for SIWE request if preference useTransactionSimulations is enabled', () => {
(utils.isSIWESignatureRequest as jest.Mock).mockReturnValue(true);
const state = getMockPersonalSignConfirmStateForRequest(
signatureRequestSIWE,
{
Expand All @@ -88,4 +115,73 @@ describe('PersonalSignInfo', () => {
);
expect(getByText('Estimated changes')).toBeDefined();
});

it('does not display tooltip text when isSIWE is true', async () => {
const state =
getMockPersonalSignConfirmStateForRequest(signatureRequestSIWE); // isSIWE is true

(utils.isSIWESignatureRequest as jest.Mock).mockReturnValue(true);
const mockStore = configureMockStore([])(state);
const { queryByText, getByText } = renderWithConfirmContextProvider(
<PersonalSignInfo />,
mockStore,
);

const requestFromLabel = getByText('Request from');
await requestFromLabel.dispatchEvent(
new MouseEvent('mouseenter', { bubbles: true }),
);

expect(
queryByText('This is the site asking for your signature.'),
).toBeNull();
expect(
queryByText('This is the Snap asking for your signature.'),
).toBeNull();
});

it('displays "requestFromInfoSnap" tooltip when isSIWE is false and origin is a snap', async () => {
const state =
getMockPersonalSignConfirmStateForRequest(signatureRequestSIWE);

(utils.isSIWESignatureRequest as jest.Mock).mockReturnValue(false);
(snapUtils.isSnapId as jest.Mock).mockReturnValue(true);

const mockStore = configureMockStore([])(state);
const { queryByText, getByText } = renderWithConfirmContextProvider(
<PersonalSignInfo />,
mockStore,
);

const requestFromLabel = getByText('Request from');
await requestFromLabel.dispatchEvent(
new MouseEvent('mouseenter', { bubbles: true }),
);

expect(
queryByText('This is the Snap asking for your signature.'),
).toBeDefined();
});

it('displays "requestFromInfo" tooltip when isSIWE is false and origin is not a snap', async () => {
const state =
getMockPersonalSignConfirmStateForRequest(signatureRequestSIWE);
(utils.isSIWESignatureRequest as jest.Mock).mockReturnValue(false);
(snapUtils.isSnapId as jest.Mock).mockReturnValue(true);

const mockStore = configureMockStore([])(state);
const { getByText, queryByText } = renderWithConfirmContextProvider(
<PersonalSignInfo />,
mockStore,
);

const requestFromLabel = getByText('Request from');
await requestFromLabel.dispatchEvent(
new MouseEvent('mouseenter', { bubbles: true }),
);

expect(
queryByText('This is the site asking for your signature.'),
).toBeDefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { selectUseTransactionSimulations } from '../../../../selectors/preferenc
import { isSIWESignatureRequest } from '../../../../utils';
import { ConfirmInfoAlertRow } from '../../../../../../components/app/confirm/info/row/alert-row/alert-row';
import { ConfirmInfoSection } from '../../../../../../components/app/confirm/info/row/section';
import { isSnapId } from '../../../../../../helpers/utils/snaps';
import { SIWESignInfo } from './siwe-sign';

const PersonalSignInfo: React.FC = () => {
Expand All @@ -39,6 +40,15 @@ const PersonalSignInfo: React.FC = () => {
hexToText(currentConfirmation.msgParams?.data),
);

let toolTipMessage;
if (!isSIWE) {
if (isSnapId(currentConfirmation.msgParams.origin)) {
toolTipMessage = t('requestFromInfoSnap');
} else {
toolTipMessage = t('requestFromInfo');
}
}

return (
<>
{isSIWE && useTransactionSimulations && (
Expand All @@ -56,7 +66,7 @@ const PersonalSignInfo: React.FC = () => {
alertKey={RowAlertKey.RequestFrom}
ownerId={currentConfirmation.id}
label={t('requestFrom')}
tooltip={isSIWE ? undefined : t('requestFromInfo')}
tooltip={toolTipMessage}
>
<ConfirmInfoRowUrl url={currentConfirmation.msgParams.origin} />
</ConfirmInfoAlertRow>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { TransactionType } from '@metamask/transaction-controller';
import { renderWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers';
import { getMockTypedSignConfirmStateForRequest } from '../../../../../../../test/data/confirmations/helper';
import { unapprovedTypedSignMsgV1 } from '../../../../../../../test/data/confirmations/typed_sign';
import * as snapUtils from '../../../../../../helpers/utils/snaps';
import TypedSignInfoV1 from './typed-sign-v1';

jest.mock(
Expand All @@ -16,6 +17,21 @@ jest.mock(
}),
);

jest.mock('../../../../../../../node_modules/@metamask/snaps-utils', () => {
const originalUtils = jest.requireActual(
'../../../../../../../node_modules/@metamask/snaps-utils',
);
return {
...originalUtils,
stripSnapPrefix: jest.fn().mockReturnValue('@metamask/examplesnap'),
getSnapPrefix: jest.fn().mockReturnValue('npm:'),
};
});

jest.mock('../../../../../../helpers/utils/snaps', () => ({
isSnapId: jest.fn(),
}));

describe('TypedSignInfo', () => {
it('correctly renders typed sign data request', () => {
const mockState = getMockTypedSignConfirmStateForRequest(
Expand All @@ -42,4 +58,50 @@ describe('TypedSignInfo', () => {
);
expect(container).toMatchInlineSnapshot(`<div />`);
});

it('displays "requestFromInfoSnap" tooltip when origin is a snap', async () => {
const mockState = getMockTypedSignConfirmStateForRequest({
id: '123',
type: TransactionType.signTypedData,
chainId: '0x5',
});
(snapUtils.isSnapId as jest.Mock).mockReturnValue(true);
const mockStore = configureMockStore([])(mockState);
const { queryByText } = renderWithConfirmContextProvider(
<TypedSignInfoV1 />,
mockStore,
);

const requestFromLabel = queryByText('Request from');

await requestFromLabel?.dispatchEvent(
new MouseEvent('mouseenter', { bubbles: true }),
);
expect(
queryByText('This is the Snap asking for your signature.'),
).toBeDefined();
});

it('displays "requestFromInfo" tooltip when origin is not a snap', async () => {
const mockState = getMockTypedSignConfirmStateForRequest({
id: '123',
type: TransactionType.signTypedData,
chainId: '0x5',
});
(snapUtils.isSnapId as jest.Mock).mockReturnValue(false);
const mockStore = configureMockStore([])(mockState);
const { queryByText } = renderWithConfirmContextProvider(
<TypedSignInfoV1 />,
mockStore,
);

const requestFromLabel = queryByText('Request from');

await requestFromLabel?.dispatchEvent(
new MouseEvent('mouseenter', { bubbles: true }),
);
expect(
queryByText('This is the site asking for your signature.'),
).toBeDefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
import { useConfirmContext } from '../../../../context/confirm';
import { ConfirmInfoRowTypedSignDataV1 } from '../../row/typed-sign-data-v1/typedSignDataV1';
import { ConfirmInfoSection } from '../../../../../../components/app/confirm/info/row/section';
import { isSnapId } from '../../../../../../helpers/utils/snaps';

const TypedSignV1Info: React.FC = () => {
const t = useI18nContext();
Expand All @@ -23,6 +24,9 @@ const TypedSignV1Info: React.FC = () => {
return null;
}

const toolTipMessage = isSnapId(currentConfirmation.msgParams?.origin)
? t('requestFromInfoSnap')
: t('requestFromInfo');
const chainId = currentConfirmation.chainId as string;

return (
Expand All @@ -32,7 +36,7 @@ const TypedSignV1Info: React.FC = () => {
alertKey={RowAlertKey.RequestFrom}
ownerId={currentConfirmation.id}
label={t('requestFrom')}
tooltip={t('requestFromInfo')}
tooltip={toolTipMessage}
>
<ConfirmInfoRowUrl
url={currentConfirmation.msgParams?.origin ?? ''}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
unapprovedTypedSignMsgV3,
} from '../../../../../../../test/data/confirmations/typed_sign';
import { renderWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers';
import * as snapUtils from '../../../../../../helpers/utils/snaps';
import TypedSignInfo from './typed-sign';

jest.mock(
Expand All @@ -33,6 +34,21 @@ jest.mock('../../../../../../store/actions', () => {
};
});

jest.mock('../../../../../../../node_modules/@metamask/snaps-utils', () => {
const originalUtils = jest.requireActual(
'../../../../../../../node_modules/@metamask/snaps-utils',
);
return {
...originalUtils,
stripSnapPrefix: jest.fn().mockReturnValue('@metamask/examplesnap'),
getSnapPrefix: jest.fn().mockReturnValue('npm:'),
};
});

jest.mock('../../../../../../helpers/utils/snaps', () => ({
isSnapId: jest.fn(),
}));

describe('TypedSignInfo', () => {
it('renders origin for typed sign data request', () => {
const state = getMockTypedSignConfirmState();
Expand Down Expand Up @@ -127,4 +143,50 @@ describe('TypedSignInfo', () => {
);
expect(container).toMatchSnapshot();
});

it('displays "requestFromInfoSnap" tooltip when origin is a snap', async () => {
const mockState = getMockTypedSignConfirmStateForRequest({
id: '123',
type: TransactionType.signTypedData,
chainId: '0x5',
});
(snapUtils.isSnapId as jest.Mock).mockReturnValue(true);
const mockStore = configureMockStore([])(mockState);
const { queryByText } = renderWithConfirmContextProvider(
<TypedSignInfo />,
mockStore,
);

const requestFromLabel = queryByText('Request from');

await requestFromLabel?.dispatchEvent(
new MouseEvent('mouseenter', { bubbles: true }),
);
expect(
queryByText('This is the Snap asking for your signature.'),
).toBeDefined();
});

it('displays "requestFromInfo" tooltip when origin is not a snap', async () => {
const mockState = getMockTypedSignConfirmStateForRequest({
id: '123',
type: TransactionType.signTypedData,
chainId: '0x5',
});
(snapUtils.isSnapId as jest.Mock).mockReturnValue(false);
const mockStore = configureMockStore([])(mockState);
const { queryByText } = renderWithConfirmContextProvider(
<TypedSignInfo />,
mockStore,
);

const requestFromLabel = queryByText('Request from');

await requestFromLabel?.dispatchEvent(
new MouseEvent('mouseenter', { bubbles: true }),
);
expect(
queryByText('This is the site asking for your signature.'),
).toBeDefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { fetchErc20Decimals } from '../../../../utils/token';
import { useConfirmContext } from '../../../../context/confirm';
import { selectUseTransactionSimulations } from '../../../../selectors/preferences';
import { ConfirmInfoRowTypedSignData } from '../../row/typed-sign-data/typedSignData';
import { isSnapId } from '../../../../../../helpers/utils/snaps';
import { PermitSimulation } from './permit-simulation';

const TypedSignInfo: React.FC = () => {
Expand Down Expand Up @@ -55,6 +56,9 @@ const TypedSignInfo: React.FC = () => {
})();
}, [verifyingContract]);

const toolTipMessage = isSnapId(currentConfirmation.msgParams.origin)
? t('requestFromInfoSnap')
: t('requestFromInfo');
const msgData = currentConfirmation.msgParams?.data as string;

return (
Expand All @@ -73,7 +77,7 @@ const TypedSignInfo: React.FC = () => {
alertKey={RowAlertKey.RequestFrom}
ownerId={currentConfirmation.id}
label={t('requestFrom')}
tooltip={t('requestFromInfo')}
tooltip={toolTipMessage}
>
<ConfirmInfoRowUrl url={currentConfirmation.msgParams.origin} />
</ConfirmInfoAlertRow>
Expand Down

0 comments on commit 7a8da64

Please sign in to comment.