From 266da0789c6797357d6dc9232b3ad32807a41a43 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Thu, 24 Oct 2024 10:05:49 +0200 Subject: [PATCH 1/5] fix: Fix limited visibility of decrypt message (#27622) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR aims to address UX issue where the extension displays only a limited portion of the entire message. Final designs could be find in this [Figma file](https://www.figma.com/design/aHdSNvg9ahXZMtThzz3bUo/Github-issues?node-id=645-1624&node-type=canvas&t=aesoQRhYJhITiGNv-0). And more information on: https://github.com/MetaMask/MetaMask-planning/issues/3002 It also adds scroll to bottom button to decrypt message pane. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27622?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3002 ## **Manual testing steps** 1. Go to test-dapp 2. Use Encrypt / Decrypt section 3. Get encryption key 4. Then put long message to encrypt 5. Then decrypt that message by clicking "Decrypt" 6. Confirmation will appear, click decrpyt to see message in confirmation, and see your long message with "scroll to bottom" button ## **Screenshots/Recordings** ### **Before** Screenshot 2024-10-04 at 13 55 49 Screenshot 2024-10-04 at 13 55 57 ### **After** Screenshot 2024-10-04 at 13 52 05 Screenshot 2024-10-04 at 13 52 18 Screenshot 2024-10-04 at 13 52 23 ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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. --- ...irm-decrypt-message.component.test.js.snap | 460 ++++++++------ .../confirm-decrypt-message.component.js | 592 +++++++++++------- .../confirm-decrypt-message.component.test.js | 330 +++++++--- .../confirm-decrypt-message.container.js | 76 --- .../confirm-decrypt-message.scss | 28 +- ui/pages/confirm-decrypt-message/index.js | 2 +- 6 files changed, 879 insertions(+), 609 deletions(-) delete mode 100644 ui/pages/confirm-decrypt-message/confirm-decrypt-message.container.js diff --git a/ui/pages/confirm-decrypt-message/__snapshots__/confirm-decrypt-message.component.test.js.snap b/ui/pages/confirm-decrypt-message/__snapshots__/confirm-decrypt-message.component.test.js.snap index cb4715881e27..d5e060e31d72 100644 --- a/ui/pages/confirm-decrypt-message/__snapshots__/confirm-decrypt-message.component.test.js.snap +++ b/ui/pages/confirm-decrypt-message/__snapshots__/confirm-decrypt-message.component.test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`ConfirmDecryptMessage Component should match snapshot when preference is ETH currency 1`] = ` +exports[`ConfirmDecryptMessage Component matches snapshot 1`] = `
- -
-
- - -
-
-
-
- - - - -
@@ -189,7 +112,7 @@ exports[`ConfirmDecryptMessage Component should match snapshot when preference i
- 966.987986 ABC + 966.987986 ETH
@@ -210,35 +133,40 @@ exports[`ConfirmDecryptMessage Component should match snapshot when preference i
- {"domain":{"chainId":97,"name":"Ether Mail","verifyingContract":"0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC","version":"1"},"message":{"contents":"Hello, Bob!","from":{"name":"Cow","wallets":["0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826","0xDeaDbeefdEAdbeefdEadbEEFdeadbeEFdEaDbeeF"]},"to":[{"name":"Bob","wallets":["0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB","0xB0BdaBea57B0BDABeA57b0bdABEA57b0BDabEa57","0xB0B0b0b0b0b0B000000000000000000000000000"]}]},"primaryType":"Mail","types":{"EIP712Domain":[{"name":"name","type":"string"},{"name":"version","type":"string"},{"name":"chainId","type":"uint256"},{"name":"verifyingContract","type":"address"}],"Mail":[{"name":"from","type":"Person"},{"name":"to","type":"Person[]"},{"name":"contents","type":"string"}],"Person":[{"name":"name","type":"string"},{"name":"wallets","type":"address[]"}]}} - -
-
-
+ {"domain":{"chainId":97,"name":"Ether Mail","verifyingContract":"0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC","version":"1"}} + +
+
+
-
- Decrypt message + +
+ Decrypt message +
+
-
`; -exports[`ConfirmDecryptMessage Component should match snapshot when preference is Fiat currency 1`] = ` +exports[`ConfirmDecryptMessage Component matches snapshot if no unapproved decrypt messages 1`] = `
`; + +exports[`ConfirmDecryptMessage Component shows error on decrypt inline error 1`] = `
+
+
+
+
+
+
+ Balance: +
+
+ 966.987986 ETH +
+
+
+
+
+ + T + +
+ test would like to read this message to complete your action +
+
+
+
+
+
+ + This message cannot be decrypted due to error: Decrypt inline error +
+
+
+
+ +
+ Decrypt message +
+
+
+
+
+
+
+
+
+ + +
+
+
+
+`; + +exports[`ConfirmDecryptMessage Component shows the correct message data 1`] = ` +
+
+
+
+
+ Decrypt request +
+
+
+
+
+
+
@@ -472,35 +515,66 @@ exports[`ConfirmDecryptMessage Component should match snapshot when preference i
- {"domain":{"chainId":97,"name":"Ether Mail","verifyingContract":"0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC","version":"1"},"message":{"contents":"Hello, Bob!","from":{"name":"Cow","wallets":["0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826","0xDeaDbeefdEAdbeefdEadbEEFdeadbeEFdEaDbeeF"]},"to":[{"name":"Bob","wallets":["0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB","0xB0BdaBea57B0BDABeA57b0bdABEA57b0BDabEa57","0xB0B0b0b0b0b0B000000000000000000000000000"]}]},"primaryType":"Mail","types":{"EIP712Domain":[{"name":"name","type":"string"},{"name":"version","type":"string"},{"name":"chainId","type":"uint256"},{"name":"verifyingContract","type":"address"}],"Mail":[{"name":"from","type":"Person"},{"name":"to","type":"Person[]"},{"name":"contents","type":"string"}],"Person":[{"name":"name","type":"string"},{"name":"wallets","type":"address[]"}]}} - +
+ raw message + +
+
+
+
+ +
+ Decrypt message +
+
+
-
-
- Decrypt message +
+ Copy encrypted message +
+
-
{ - copyToClipboard(this.state.rawMessage, COPY_OPTIONS); - this.context.trackEvent({ - category: MetaMetricsEventCategory.Messages, - event: 'Copy', - properties: { - action: 'Decrypt Message Copy', - legacy_event: true, - }, - }); - this.setState({ hasCopied: true }); - setTimeout(() => this.setState({ hasCopied: false }), SECOND * 3); - }; - - renderHeader = () => { - return ( -
-
- -
- {this.context.t('decryptRequest')} -
- -
-
-
+const Header = () => { + const t = useI18nContext(); + return ( +
+
+
+ {t('decryptRequest')}
- ); - }; +
+
+
+
+ ); +}; - renderAccount = () => { - const { fromAccount } = this.props; - const { t } = this.context; +const Account = ({ fromAccount, nativeCurrency }) => { + const t = useI18nContext(); - return ( + const nativeCurrencyBalance = new Numeric( + fromAccount.balance, + 16, + EtherDenomination.WEI, + ) + .toDenomination(EtherDenomination.ETH) + .round(6) + .toBase(10) + .toString(); + + return ( +
{`${t('account')}:`}
-
- ); - }; - - renderBalance = () => { - const { - conversionRate, - nativeCurrency, - currentCurrency, - fromAccount: { balance }, - } = this.props; - const { t } = this.context; - - const nativeCurrencyBalance = conversionRate - ? formatCurrency( - getValueFromWeiHex({ - value: balance, - fromCurrency: nativeCurrency, - toCurrency: currentCurrency, - conversionRate, - numberOfDecimals: 6, - toDenomination: EtherDenomination.ETH, - }), - currentCurrency, - ) - : new Numeric(balance, 16, EtherDenomination.WEI) - .toDenomination(EtherDenomination.ETH) - .round(6) - .toBase(10) - .toString(); - - return (
{`${t('balance')}:`}
- {`${nativeCurrencyBalance} ${ - conversionRate ? currentCurrency?.toUpperCase() : nativeCurrency - }`} + {`${nativeCurrencyBalance} ${nativeCurrency}`}
- ); - }; +
+ ); +}; - renderRequestIcon = () => { - const { requesterAddress } = this.props; +Account.propTypes = { + fromAccount: PropTypes.shape({ + balance: PropTypes.string.isRequired, + }).isRequired, + nativeCurrency: PropTypes.string.isRequired, +}; - return ( -
- -
- ); - }; +const VisualSection = ({ name, notice, targetSubjectMetadata }) => ( +
+
+ {targetSubjectMetadata?.iconUrl ? ( + + ) : ( + + {name.charAt(0).toUpperCase()} + + )} +
{notice}
+
+
+); - renderAccountInfo = () => { - return ( -
- {this.renderAccount()} - {this.renderRequestIcon()} - {this.renderBalance()} -
- ); - }; +VisualSection.propTypes = { + name: PropTypes.string.isRequired, + notice: PropTypes.string.isRequired, + targetSubjectMetadata: PropTypes.shape({ + iconUrl: PropTypes.string, + }), +}; + +const ScrollToBottomButton = ({ + isScrollable, + isScrolledToBottom, + hasDecrypted, + hasError, + scrollToBottom, +}) => { + const t = useI18nContext(); + + const isScrollToBottomVisible = + hasDecrypted && !hasError && isScrollable && !isScrolledToBottom; + + if (!isScrollToBottomVisible) { + return null; + } - renderBody = () => { - const { decryptMessageInline, subjectMetadata, txData } = this.props; - const { t } = this.context; + return ( + + ); +}; - const targetSubjectMetadata = subjectMetadata[txData.msgParams.origin]; - const name = targetSubjectMetadata?.name || txData.msgParams.origin; - const notice = t('decryptMessageNotice', [txData.msgParams.origin]); +ScrollToBottomButton.propTypes = { + isScrollable: PropTypes.bool.isRequired, + isScrolledToBottom: PropTypes.bool.isRequired, + hasDecrypted: PropTypes.bool.isRequired, + hasError: PropTypes.bool.isRequired, + scrollToBottom: PropTypes.func.isRequired, +}; - const { - hasCopied, - hasDecrypted, - hasError, +const MessageBody = forwardRef( + ( + { + isScrollable, + isScrolledToBottom, + onScroll, rawMessage, - errorMessage, - copyToClipboardPressed, - } = this.state; + scrollToBottom, + setRawMessage, + messageData, + }, + ref, + ) => { + const dispatch = useDispatch(); + const trackEvent = useContext(MetaMetricsContext); + const t = useI18nContext(); + + const [copyToClipboardPressed, setCopyToClipboardPressed] = useState(false); + const [hasCopied, setHasCopied] = useState(false); + const [hasDecrypted, setHasDecrypted] = useState(false); + const [hasError, setHasError] = useState(false); + const [errorMessage, setErrorMessage] = useState(''); + + const copyMessage = () => { + copyToClipboard(rawMessage, COPY_OPTIONS); + trackEvent({ + category: MetaMetricsEventCategory.Messages, + event: 'Copy', + properties: { + action: 'Decrypt Message Copy', + legacy_event: true, + }, + }); + setHasCopied(true); + setTimeout(() => setHasCopied(false), SECOND * 3); + }; + + const onDecryptMessage = async (event) => { + event.stopPropagation(event); + + const params = messageData.msgParams; + params.metamaskId = messageData.id; + + const result = await dispatch(decryptMsgInline(params)); + if (result.error) { + setHasError(true); + setErrorMessage(t('decryptInlineError', [result.error])); + } else { + setHasDecrypted(true); + setRawMessage(result.rawSig); + } + }; return ( -
- {this.renderAccountInfo()} -
-
- {targetSubjectMetadata?.iconUrl ? ( - - ) : ( - - {name.charAt(0).toUpperCase()} - - )} -
{notice}
-
-
+
-
- {!hasDecrypted && !hasError ? txData.msgParams.data : rawMessage} +
+ {!hasDecrypted && !hasError + ? messageData.msgParams.data + : rawMessage} {hasError ? errorMessage : ''}
{ - decryptMessageInline(txData, event).then((result) => { - if (result.error) { - this.setState({ - hasError: true, - errorMessage: this.context.t('decryptInlineError', [ - result.error, - ]), - }); - } else { - this.setState({ - hasDecrypted: true, - rawMessage: result.rawSig, - }); - } - }); - }} + data-testid="message-lock" + onClick={onDecryptMessage} >
@@ -236,6 +256,13 @@ export default class ConfirmDecryptMessage extends Component {
+ {hasDecrypted ? (
this.copyMessage()} - onMouseDown={() => this.setState({ copyToClipboardPressed: true })} - onMouseUp={() => this.setState({ copyToClipboardPressed: false })} + onClick={copyMessage} + onMouseDown={() => setCopyToClipboardPressed(true)} + onMouseUp={() => setCopyToClipboardPressed(false)} + data-testid="message-copy" > ); + }, +); +MessageBody.displayName = 'MessageBody'; + +MessageBody.propTypes = { + isScrollable: PropTypes.bool.isRequired, + isScrolledToBottom: PropTypes.bool.isRequired, + onScroll: PropTypes.func.isRequired, + rawMessage: PropTypes.string.isRequired, + scrollToBottom: PropTypes.func.isRequired, + setRawMessage: PropTypes.func.isRequired, + messageData: PropTypes.shape({ + msgParams: PropTypes.shape({ + data: PropTypes.string.isRequired, + from: PropTypes.string.isRequired, + origin: PropTypes.string.isRequired, + }).isRequired, + id: PropTypes.string.isRequired, + }).isRequired, +}; + +const Footer = ({ + hasScrolledToBottom, + isScrollable, + mostRecentOverviewPage, + messageData, +}) => { + const dispatch = useDispatch(); + const history = useHistory(); + const t = useI18nContext(); + const trackEvent = useContext(MetaMetricsContext); + + const onCancelClick = async (event) => { + event.stopPropagation(event); + + await dispatch(cancelDecryptMsg(messageData)); + trackEvent({ + category: MetaMetricsEventCategory.Messages, + event: 'Cancel', + properties: { + action: 'Decrypt Message Request', + legacy_event: true, + }, + }); + dispatch(clearConfirmTransaction()); + history.push(mostRecentOverviewPage); }; - renderFooter = () => { - const { - cancelDecryptMessage, - clearConfirmTransaction, - decryptMessage, - history, - mostRecentOverviewPage, - txData, - } = this.props; - const { trackEvent, t } = this.context; + const onSubmitClick = async (event) => { + event.stopPropagation(event); + const params = messageData.msgParams; + params.metamaskId = messageData.id; - return ( - { - await cancelDecryptMessage(txData, event); - trackEvent({ - category: MetaMetricsEventCategory.Messages, - event: 'Cancel', - properties: { - action: 'Decrypt Message Request', - legacy_event: true, - }, - }); - clearConfirmTransaction(); - history.push(mostRecentOverviewPage); - }} - onSubmit={async (event) => { - await decryptMessage(txData, event); - trackEvent({ - category: MetaMetricsEventCategory.Messages, - event: 'Confirm', - properties: { - action: 'Decrypt Message Request', - legacy_event: true, - }, - }); - clearConfirmTransaction(); - history.push(mostRecentOverviewPage); - }} - /> - ); + await dispatch(decryptMsg(params)); + trackEvent({ + category: MetaMetricsEventCategory.Messages, + event: 'Confirm', + properties: { + action: 'Decrypt Message Request', + legacy_event: true, + }, + }); + dispatch(clearConfirmTransaction()); + history.push(mostRecentOverviewPage); }; - render = () => { - if (!this.props.txData) { - log.warn('ConfirmDecryptMessage Page: Missing txData prop.'); - return null; - } + return ( + + ); +}; - return ( -
- {this.renderHeader()} - {this.renderBody()} - {this.renderFooter()} +Footer.propTypes = { + hasScrolledToBottom: PropTypes.bool.isRequired, + isScrollable: PropTypes.bool.isRequired, + mostRecentOverviewPage: PropTypes.string.isRequired, + messageData: PropTypes.shape({ + msgParams: PropTypes.shape({ + data: PropTypes.string.isRequired, + from: PropTypes.string.isRequired, + origin: PropTypes.string.isRequired, + }).isRequired, + id: PropTypes.string.isRequired, + }).isRequired, +}; + +const ConfirmDecryptMessage = () => { + const t = useI18nContext(); + const [rawMessage, setRawMessage] = useState(''); + const mostRecentOverviewPage = useSelector(getMostRecentOverviewPage); + const nativeCurrency = useSelector(getNativeCurrency); + + const unconfirmedTransactions = useSelector( + unconfirmedTransactionsListSelector, + ); + const messageData = cloneDeep(unconfirmedTransactions[0]); + + const fromAccount = useSelector((state) => + getTargetAccountWithSendEtherInfo(state, messageData?.msgParams?.from), + ); + + const subjectMetadata = useSelector( + (state) => state.metamask.subjectMetadata || {}, + ); + + const { + hasScrolledToBottom, + isScrollable, + isScrolledToBottom, + onScroll, + scrollToBottom, + ref, + } = useScrollRequired([rawMessage], { + offsetPxFromBottom: 0, + }); + + if (!messageData) { + log.warn('ConfirmDecryptMessage Page: Missing messageData prop.'); + return null; + } + + const targetSubjectMetadata = subjectMetadata[messageData.msgParams.origin]; + const name = targetSubjectMetadata?.name || messageData.msgParams.origin; + const notice = t('decryptMessageNotice', [messageData.msgParams.origin]); + + return ( +
+
+
+ + +
- ); - }; -} +
+
+ ); +}; + +export default ConfirmDecryptMessage; diff --git a/ui/pages/confirm-decrypt-message/confirm-decrypt-message.component.test.js b/ui/pages/confirm-decrypt-message/confirm-decrypt-message.component.test.js index e073474d79fe..7884cb22e4bb 100644 --- a/ui/pages/confirm-decrypt-message/confirm-decrypt-message.component.test.js +++ b/ui/pages/confirm-decrypt-message/confirm-decrypt-message.component.test.js @@ -1,9 +1,17 @@ import React from 'react'; import configureMockStore from 'redux-mock-store'; -import { EthAccountType } from '@metamask/keyring-api'; +import { merge } from 'lodash'; +import copyToClipboard from 'copy-to-clipboard'; import mockState from '../../../test/data/mock-state.json'; import { renderWithProvider } from '../../../test/lib/render-helpers'; -import { ETH_EOA_METHODS } from '../../../shared/constants/eth-methods'; +import { flushPromises } from '../../../test/lib/timer-helpers'; +import { + decryptMsg, + decryptMsgInline, + cancelDecryptMsg, +} from '../../store/actions'; +import { useScrollRequired } from '../../hooks/useScrollRequired'; +import { MetaMetricsContext } from '../../contexts/metametrics'; import ConfirmDecryptMessage from './confirm-decrypt-message.component'; const messageData = { @@ -13,129 +21,245 @@ const messageData = { verifyingContract: '0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC', version: '1', }, - message: { - contents: 'Hello, Bob!', - from: { - name: 'Cow', - wallets: [ - '0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826', - '0xDeaDbeefdEAdbeefdEadbEEFdeadbeEFdEaDbeeF', - ], - }, - to: [ - { - name: 'Bob', - wallets: [ - '0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB', - '0xB0BdaBea57B0BDABeA57b0bdABEA57b0BDabEa57', - '0xB0B0b0b0b0b0B000000000000000000000000000', - ], - }, - ], - }, - primaryType: 'Mail', - types: { - EIP712Domain: [ - { name: 'name', type: 'string' }, - { name: 'version', type: 'string' }, - { name: 'chainId', type: 'uint256' }, - { name: 'verifyingContract', type: 'address' }, - ], - Mail: [ - { name: 'from', type: 'Person' }, - { name: 'to', type: 'Person[]' }, - { name: 'contents', type: 'string' }, - ], - Person: [ - { name: 'name', type: 'string' }, - { name: 'wallets', type: 'address[]' }, - ], +}; +const messageIdMock = '12345'; +const messageMock = { + id: messageIdMock, + time: 123, + status: 'unapproved', + type: 'testType', + rawSig: undefined, + data: JSON.stringify(messageData), + chainId: '0x5', + msgParams: { + data: JSON.stringify(messageData), + from: '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc', + version: 'V4', + origin: 'test', }, }; -const baseProps = { - clearConfirmTransaction: () => undefined, - cancelDecryptMessage: () => undefined, - decryptMessage: () => undefined, - decryptMessageInline: () => undefined, - mostRecentOverviewPage: '/', - history: { push: '/' }, - requesterAddress: '0x123456789abcdef', - txData: { - msgParams: { - data: JSON.stringify(messageData), - version: 'V4', - origin: 'test', - }, +const mockRawSignatureMessage = 'raw message'; +const mockRawSignatureLongMessage = 'raw message '.repeat(100); +const mockUseScrollRequiredResult = { + hasScrolledToBottom: false, + isScrollable: false, + isScrolledToBottom: false, + onScroll: jest.fn(), + scrollToBottom: jest.fn(), + setHasScrolledToBottom: jest.fn(), + ref: { + current: {}, }, - subjectMetadata: { - 'peepeth.com': { - iconUrl: 'https://peepeth.com/favicon-32x32.png', - name: 'Peepeth', - }, - 'https://remix.ethereum.org': { - iconUrl: 'https://remix.ethereum.org/icon.png', - name: 'Remix - Ethereum IDE', - }, +}; + +jest.mock('../../hooks/useScrollRequired', () => ({ + useScrollRequired: jest.fn(), +})); + +jest.mock('../../store/actions', () => ({ + cancelDecryptMsg: jest.fn(), + decryptMsg: jest.fn(), + decryptMsgInline: jest.fn(), +})); + +jest.mock('copy-to-clipboard', () => ({ + __esModule: true, + default: jest.fn(), +})); + +const state = merge({}, mockState, { + history: { + mostRecentOverviewPage: '/', }, - nativeCurrency: 'ABC', - currentCurrency: 'def', - fromAccount: { - address: '0x123456789abcdef', - balance: '0x346ba7725f412cbfdb', - id: 'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3', - metadata: { - name: 'Antonio', - keyring: { - type: 'HD Key Tree', - }, - }, - options: {}, - methods: ETH_EOA_METHODS, - type: EthAccountType.Eoa, + metamask: { + unapprovedDecryptMsgs: [messageMock], }, -}; +}); describe('ConfirmDecryptMessage Component', () => { - const store = configureMockStore()(mockState); + const mockCopyToClipboard = jest.mocked(copyToClipboard); + const mockCancelDecryptMsg = jest.mocked(cancelDecryptMsg); + const mockDecryptMsgInline = jest.mocked(decryptMsgInline); + const mockDecryptMsg = jest.mocked(decryptMsg); + const mockUseScrollRequired = jest.mocked(useScrollRequired); + const mockTrackEvent = jest.fn(); - it('should match snapshot when preference is ETH currency', () => { - const { container } = renderWithProvider( - , + let store; + + beforeEach(() => { + jest.clearAllMocks(); + store = configureMockStore()(state); + + mockDecryptMsgInline.mockReturnValue({ + rawSig: mockRawSignatureMessage, + type: 'DECRYPT_MESSAGE_INLINE', + }); + mockDecryptMsg.mockReturnValue({ + type: 'DECRYPT_MESSAGE', + }); + mockCancelDecryptMsg.mockReturnValue({ + type: 'CANCEL_DECRYPT_MESSAGE', + }); + + mockUseScrollRequired.mockReturnValue(mockUseScrollRequiredResult); + }); + + const renderAndUnlockMessage = async () => { + const result = renderWithProvider( + + + , store, ); + const unlockButton = result.getByTestId('message-lock'); + unlockButton.click(); + await flushPromises(); + return result; + }; + + it('matches snapshot', () => { + const { container } = renderWithProvider(, store); expect(container).toMatchSnapshot(); - expect( - container.querySelector('.request-decrypt-message__balance-value') - .textContent, - ).toMatchInlineSnapshot(`"966.987986 ABC"`); }); - it('should match snapshot when preference is Fiat currency', () => { + it('matches snapshot if no unapproved decrypt messages', () => { + const stateWithoutUnapprovedDecryptMsgs = merge({}, mockState); + stateWithoutUnapprovedDecryptMsgs.metamask.unapprovedDecryptMsgs = []; + stateWithoutUnapprovedDecryptMsgs.metamask.transactions = []; + + const storeWithoutUnapprovedDecryptMsgs = configureMockStore()( + stateWithoutUnapprovedDecryptMsgs, + ); + const { container } = renderWithProvider( - , - store, + , + storeWithoutUnapprovedDecryptMsgs, ); + expect(container).toMatchSnapshot(); + }); + + it('shows error on decrypt inline error', async () => { + mockDecryptMsgInline.mockReturnValue({ + error: 'Decrypt inline error', + type: 'DECRYPT_MESSAGE_INLINE', + }); + + const { container } = await renderAndUnlockMessage(); expect(container).toMatchSnapshot(); - expect( - container.querySelector('.request-decrypt-message__balance-value') - .textContent, - ).toMatchInlineSnapshot(`"1520956.064158 DEF"`); }); - it('should match snapshot when there is no txData', () => { - const newProps = { - ...baseProps, - txData: null, - }; - const { container } = renderWithProvider( - , + it('decrypt button calls decrypt action and calls metric event', async () => { + const { getByText } = renderWithProvider( + + + , store, ); - expect( - container.querySelector('.request-decrypt-message__container'), - ).toBeNull(); + const confirmButton = getByText('Decrypt'); + confirmButton.click(); + await flushPromises(); + + expect(mockDecryptMsg).toHaveBeenCalled(); + expect(mockTrackEvent).toHaveBeenCalled(); + }); + + it('cancel button calls cancel action and calls metric event', async () => { + const { getByText } = renderWithProvider( + + + , + store, + ); + + const confirmButton = getByText('Cancel'); + confirmButton.click(); + await flushPromises(); + + expect(mockCancelDecryptMsg).toHaveBeenCalled(); + expect(mockTrackEvent).toHaveBeenCalled(); + }); + + it('shows the correct message data', async () => { + const { container, getByText } = await renderAndUnlockMessage(); + + expect(container).toMatchSnapshot(); + expect(getByText(mockRawSignatureMessage)).toBeInTheDocument(); + }); + + it('shows the copy button after decrypting inline and calls copy to clipboard on click', async () => { + const { getByTestId } = await renderAndUnlockMessage(); + + const copyButton = getByTestId('message-copy'); + expect(copyButton).toBeInTheDocument(); + + copyButton.click(); + expect(mockCopyToClipboard).toHaveBeenCalled(); + expect(mockTrackEvent).toHaveBeenCalled(); + }); + + describe('on long message', () => { + beforeEach(() => { + mockDecryptMsgInline.mockReturnValue({ + rawSig: mockRawSignatureLongMessage, + type: 'DECRYPT_MESSAGE_INLINE', + }); + }); + + it('shows scroll to bottom button', async () => { + mockUseScrollRequired.mockReturnValue( + merge({}, mockUseScrollRequiredResult, { isScrollable: true }), + ); + + const { getByTestId } = await renderAndUnlockMessage(); + + const scrollToBottomButton = getByTestId('scroll-to-bottom'); + expect(scrollToBottomButton).toBeInTheDocument(); + }); + + it('confirm action is disabled if scroll not finished', async () => { + mockUseScrollRequired.mockReturnValue( + merge({}, mockUseScrollRequiredResult, { isScrollable: true }), + ); + + const { getByText } = await renderAndUnlockMessage(); + + const confirmButton = getByText('Decrypt'); + expect(confirmButton).toBeDisabled(); + }); + + it('scroll to bottom button click calls scroll to bottom action', async () => { + const spyScrollToBottomAction = jest.fn(); + mockUseScrollRequired.mockReturnValue( + merge({}, mockUseScrollRequiredResult, { + isScrollable: true, + scrollToBottom: spyScrollToBottomAction, + }), + ); + + const { getByTestId } = await renderAndUnlockMessage(); + + const scrollToBottomButton = getByTestId('scroll-to-bottom'); + scrollToBottomButton.click(); + + expect(spyScrollToBottomAction).toHaveBeenCalled(); + }); + + it('enables confirm action if scrolled to bottom', async () => { + mockUseScrollRequired.mockReturnValue( + merge({}, mockUseScrollRequiredResult, { + isScrollable: true, + hasScrolledToBottom: true, + isScrolledToBottom: true, + }), + ); + + const { getByText } = await renderAndUnlockMessage(); + + const confirmButton = getByText('Decrypt'); + expect(confirmButton).not.toBeDisabled(); + }); }); }); diff --git a/ui/pages/confirm-decrypt-message/confirm-decrypt-message.container.js b/ui/pages/confirm-decrypt-message/confirm-decrypt-message.container.js deleted file mode 100644 index 88d7c8deb40b..000000000000 --- a/ui/pages/confirm-decrypt-message/confirm-decrypt-message.container.js +++ /dev/null @@ -1,76 +0,0 @@ -import { connect } from 'react-redux'; -import { compose } from 'redux'; -import { withRouter } from 'react-router-dom'; -import { cloneDeep } from 'lodash'; - -import { - goHome, - decryptMsg, - cancelDecryptMsg, - decryptMsgInline, -} from '../../store/actions'; -import { - getCurrentCurrency, - getTargetAccountWithSendEtherInfo, - unconfirmedTransactionsListSelector, -} from '../../selectors'; -import { clearConfirmTransaction } from '../../ducks/confirm-transaction/confirm-transaction.duck'; -import { getMostRecentOverviewPage } from '../../ducks/history/history'; -import { getNativeCurrency } from '../../ducks/metamask/metamask'; -import ConfirmDecryptMessage from './confirm-decrypt-message.component'; - -// ConfirmDecryptMessage component is not used in codebase, removing usage of useNativeCurrencyAsPrimaryCurrency -function mapStateToProps(state) { - const { - metamask: { subjectMetadata = {} }, - } = state; - - const unconfirmedTransactions = unconfirmedTransactionsListSelector(state); - - const txData = cloneDeep(unconfirmedTransactions[0]); - - const fromAccount = getTargetAccountWithSendEtherInfo( - state, - txData?.msgParams?.from, - ); - - return { - txData, - subjectMetadata, - fromAccount, - requester: null, - requesterAddress: null, - conversionRate: null, - mostRecentOverviewPage: getMostRecentOverviewPage(state), - nativeCurrency: getNativeCurrency(state), - currentCurrency: getCurrentCurrency(state), - }; -} - -function mapDispatchToProps(dispatch) { - return { - goHome: () => dispatch(goHome()), - clearConfirmTransaction: () => dispatch(clearConfirmTransaction()), - decryptMessage: (msgData, event) => { - const params = msgData.msgParams; - params.metamaskId = msgData.id; - event.stopPropagation(event); - return dispatch(decryptMsg(params)); - }, - cancelDecryptMessage: (msgData, event) => { - event.stopPropagation(event); - return dispatch(cancelDecryptMsg(msgData)); - }, - decryptMessageInline: (msgData, event) => { - const params = msgData.msgParams; - params.metamaskId = msgData.id; - event.stopPropagation(event); - return dispatch(decryptMsgInline(params)); - }, - }; -} - -export default compose( - withRouter, - connect(mapStateToProps, mapDispatchToProps), -)(ConfirmDecryptMessage); diff --git a/ui/pages/confirm-decrypt-message/confirm-decrypt-message.scss b/ui/pages/confirm-decrypt-message/confirm-decrypt-message.scss index b1d74b6be9cb..ac33ae6afc8c 100644 --- a/ui/pages/confirm-decrypt-message/confirm-decrypt-message.scss +++ b/ui/pages/confirm-decrypt-message/confirm-decrypt-message.scss @@ -1,4 +1,4 @@ -@use "design-system"; +@use 'design-system'; .request-decrypt-message { &__container { @@ -164,19 +164,37 @@ width: 100%; } + &__message-container { + position: relative; + + .scroll-to-bottom__button { + position: absolute; + margin-left: auto; + margin-right: auto; + left: 0; + right: 0; + bottom: 28px; + + &:hover { + background-color: var(--color-primary-default); + } + } + } + &__message { overflow-wrap: break-word; margin: 20px; overflow: hidden; - border: 1px solid var(--color-border-alternative); - padding: 5px; border-radius: 5px; position: relative; + border: 1px solid var(--color-border-muted); + padding: 13px 16px; &-text { @include design-system.H7; height: 115px; + overflow-y: scroll; } &-cover { @@ -193,6 +211,7 @@ height: 100%; width: 100%; top: 0; + left: 0; cursor: pointer; &__container { @@ -208,7 +227,6 @@ align-items: center; justify-content: center; - &__icon { color: var(--color-icon-default); display: flex; @@ -231,6 +249,7 @@ justify-content: space-evenly; margin-left: 20px; margin-right: 20px; + margin-top: 24px; display: flex; cursor: pointer; } @@ -238,6 +257,7 @@ &-copy-text { margin-right: 10px; display: inline; + color: var(--color-primary-default); } &-copy-tooltip { diff --git a/ui/pages/confirm-decrypt-message/index.js b/ui/pages/confirm-decrypt-message/index.js index c375f08a6fb5..3fbb921b8dce 100644 --- a/ui/pages/confirm-decrypt-message/index.js +++ b/ui/pages/confirm-decrypt-message/index.js @@ -1 +1 @@ -export { default } from './confirm-decrypt-message.container'; +export { default } from './confirm-decrypt-message.component'; From fa97321a9c74dce1e6b05e8c07278c956f2cd86f Mon Sep 17 00:00:00 2001 From: Matteo Scurati Date: Thu, 24 Oct 2024 10:17:30 +0200 Subject: [PATCH 2/5] feat: :sparkles: show a notification item in the settings page (#26843) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR introduces the `Notifications` option within the `settings page`. Clicking on this option will render the user to the `notifications/settings` page. Specifically: 1. If basic functionalities are not active, the option disappears from the menu 2. If notifications are not active, the user will still be redirected to the `notifications/settings page`, where they can enable them 3. If notifications are active, the user will access the `notifications/settings` page, where they can select their options [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26843?quickstart=1) ## **Related issues** N/A ## **Manual testing steps** 1. Go to the settings page. 2. Find the `Notifications` option 3. Verify that the `Notifications` option links to the `notifications/settings` page 4. Verify that the `Notifications` option is hidden if basic functionalities are off ## **Screenshots/Recordings** ### **Before** Screenshot 2024-10-23 at 16 26 12 ### **After** Screenshot 2024-10-23 at 16 41 30 ## **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. --- .../notifications-settings.tsx | 12 ++++++++++-- ui/pages/settings/settings.component.js | 19 +++++++++++++++++-- ui/pages/settings/settings.container.js | 7 ++++++- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/ui/pages/notifications-settings/notifications-settings.tsx b/ui/pages/notifications-settings/notifications-settings.tsx index 7261daeed02f..d929f048a793 100644 --- a/ui/pages/notifications-settings/notifications-settings.tsx +++ b/ui/pages/notifications-settings/notifications-settings.tsx @@ -1,6 +1,6 @@ import React, { useMemo, useState } from 'react'; import { useSelector } from 'react-redux'; -import { useHistory } from 'react-router-dom'; +import { useHistory, useLocation } from 'react-router-dom'; import type { InternalAccount } from '@metamask/keyring-api'; import { useI18nContext } from '../../hooks/useI18nContext'; import { NOTIFICATIONS_ROUTE } from '../../helpers/constants/routes'; @@ -47,6 +47,7 @@ type AccountType = InternalAccount & { export default function NotificationsSettings() { const history = useHistory(); + const location = useLocation(); const t = useI18nContext(); // Selectors @@ -74,6 +75,9 @@ export default function NotificationsSettings() { await accountSettingsProps.update(accountAddresses); }; + // Previous page + const previousPage = location.state?.fromPage; + return (
history.push(NOTIFICATIONS_ROUTE)} + onClick={() => + previousPage + ? history.push(previousPage) + : history.push(NOTIFICATIONS_ROUTE) + } /> } endAccessory={null} diff --git a/ui/pages/settings/settings.component.js b/ui/pages/settings/settings.component.js index 2f33cd78f2e1..119b01257802 100644 --- a/ui/pages/settings/settings.component.js +++ b/ui/pages/settings/settings.component.js @@ -20,6 +20,7 @@ import { ADD_NETWORK_ROUTE, ADD_POPULAR_CUSTOM_NETWORK, DEFAULT_ROUTE, + NOTIFICATIONS_SETTINGS_ROUTE, } from '../../helpers/constants/routes'; import { getSettingsRoutes } from '../../helpers/utils/settings-search'; @@ -69,6 +70,7 @@ class SettingsPage extends PureComponent { mostRecentOverviewPage: PropTypes.string.isRequired, pathnameI18nKey: PropTypes.string, toggleNetworkMenu: PropTypes.func.isRequired, + useExternalServices: PropTypes.bool, }; static contextTypes = { @@ -290,7 +292,7 @@ class SettingsPage extends PureComponent { } renderTabs() { - const { history, currentPath } = this.props; + const { history, currentPath, useExternalServices } = this.props; const { t } = this.context; const tabs = [ @@ -326,6 +328,14 @@ class SettingsPage extends PureComponent { }, ]; + if (useExternalServices) { + tabs.splice(4, 0, { + content: t('notifications'), + icon: , + key: NOTIFICATIONS_SETTINGS_ROUTE, + }); + } + if (process.env.ENABLE_SETTINGS_PAGE_DEV_OPTIONS) { tabs.splice(-1, 0, { content: t('developerOptions'), @@ -349,7 +359,12 @@ class SettingsPage extends PureComponent { } return matchPath(currentPath, { exact: true, path: key }); }} - onSelect={(key) => history.push(key)} + onSelect={(key) => + history.push({ + pathname: key, + state: { fromPage: currentPath }, + }) + } /> ); } diff --git a/ui/pages/settings/settings.container.js b/ui/pages/settings/settings.container.js index 1ee2bfb83941..58a35f37e616 100644 --- a/ui/pages/settings/settings.container.js +++ b/ui/pages/settings/settings.container.js @@ -1,7 +1,10 @@ import { compose } from 'redux'; import { connect } from 'react-redux'; import { withRouter } from 'react-router-dom'; -import { getAddressBookEntryOrAccountName } from '../../selectors'; +import { + getAddressBookEntryOrAccountName, + getUseExternalServices, +} from '../../selectors'; import { ENVIRONMENT_TYPE_POPUP } from '../../../shared/constants/app'; // TODO: Remove restricted import // eslint-disable-next-line import/no-restricted-paths @@ -96,6 +99,7 @@ const mapStateToProps = (state, ownProps) => { ? pathNameTail : '', ); + const useExternalServices = getUseExternalServices(state); return { addNewNetwork, @@ -109,6 +113,7 @@ const mapStateToProps = (state, ownProps) => { isPopup, mostRecentOverviewPage: getMostRecentOverviewPage(state), pathnameI18nKey, + useExternalServices, }; }; From b7a79d091b4e7e12f6d647ba678d616f117c32a7 Mon Sep 17 00:00:00 2001 From: seaona <54408225+seaona@users.noreply.github.com> Date: Thu, 24 Oct 2024 10:42:45 +0200 Subject: [PATCH 3/5] fix: flaky anti-pattern getText + assert 2 (#28043) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Continuing the work of removing the e2e anti-pattern of finding the element and then asserting its text. There are more occurrences, but this work is split in several PRs, for an easy review and a faster ci. Once all occurrences have been fixed, we'll be able to merge @HowardBraham 's [PR ](https://github.com/MetaMask/metamask-extension/pull/27591)for adding a lint rule which prevents introducing it again. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28043?quickstart=1) ## **Related issues** Fixes: (but doesn't yet closes) https://github.com/MetaMask/metamask-extension/issues/19870 ## **Manual testing steps** 1. Check ci ## **Screenshots/Recordings** ### **Before** ### **After** ## **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. --- .../batch-txs-per-dapp-same-network.spec.js | 25 +++++----------- .../watchAsset-switchChain-watchAsset.spec.js | 21 +++++--------- .../metamask-responsive-ui.spec.js | 21 ++++++++------ .../about-metamask-ui-validation.spec.ts | 13 +++------ test/e2e/tests/settings/address-book.spec.js | 29 ++++++------------- 5 files changed, 40 insertions(+), 69 deletions(-) diff --git a/test/e2e/tests/request-queuing/batch-txs-per-dapp-same-network.spec.js b/test/e2e/tests/request-queuing/batch-txs-per-dapp-same-network.spec.js index bd52558ec67f..c30d6a73c063 100644 --- a/test/e2e/tests/request-queuing/batch-txs-per-dapp-same-network.spec.js +++ b/test/e2e/tests/request-queuing/batch-txs-per-dapp-same-network.spec.js @@ -1,4 +1,4 @@ -const { strict: assert } = require('assert'); +const { By } = require('selenium-webdriver'); const FixtureBuilder = require('../../fixture-builder'); const { withFixtures, @@ -10,7 +10,6 @@ const { WINDOW_TITLES, defaultGanacheOptions, largeDelayMs, - switchToNotificationWindow, } = require('../../helpers'); const { PAGES } = require('../../webdriver/driver'); @@ -59,7 +58,7 @@ describe('Request Queuing for Multiple Dapps and Txs on same networks', function await driver.delay(regularDelayMs); - await switchToNotificationWindow(driver); + await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); await driver.clickElement({ text: 'Connect', @@ -98,7 +97,7 @@ describe('Request Queuing for Multiple Dapps and Txs on same networks', function await driver.delay(regularDelayMs); - await switchToNotificationWindow(driver, 4); + await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); await driver.clickElement({ text: 'Connect', @@ -134,16 +133,12 @@ describe('Request Queuing for Multiple Dapps and Txs on same networks', function await driver.clickElement('#sendButton'); await driver.clickElement('#sendButton'); - await switchToNotificationWindow(driver, 4); + await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); - let navigationElement = await driver.findElement( - '.confirm-page-container-navigation', + await driver.waitForSelector( + By.xpath("//div[normalize-space(.)='1 of 2']"), ); - let navigationText = await navigationElement.getText(); - - assert.equal(navigationText.includes('1 of 2'), true); - // Check correct network on confirm tx. await driver.findElement({ css: '[data-testid="network-display"]', @@ -162,14 +157,10 @@ describe('Request Queuing for Multiple Dapps and Txs on same networks', function await driver.delay(largeDelayMs); await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); - navigationElement = await driver.findElement( - '.confirm-page-container-navigation', + await driver.waitForSelector( + By.xpath("//div[normalize-space(.)='1 of 2']"), ); - navigationText = await navigationElement.getText(); - - assert.equal(navigationText.includes('1 of 2'), true); - // Check correct network on confirm tx. await driver.findElement({ css: '[data-testid="network-display"]', diff --git a/test/e2e/tests/request-queuing/watchAsset-switchChain-watchAsset.spec.js b/test/e2e/tests/request-queuing/watchAsset-switchChain-watchAsset.spec.js index 1c1baa17fb5a..64ac781a20e0 100644 --- a/test/e2e/tests/request-queuing/watchAsset-switchChain-watchAsset.spec.js +++ b/test/e2e/tests/request-queuing/watchAsset-switchChain-watchAsset.spec.js @@ -7,7 +7,6 @@ const { DAPP_URL, regularDelayMs, WINDOW_TITLES, - switchToNotificationWindow, defaultGanacheOptions, } = require('../../helpers'); @@ -45,19 +44,15 @@ describe('Request Queue WatchAsset -> SwitchChain -> WatchAsset', function () { // Create Token await driver.clickElement({ text: 'Create Token', tag: 'button' }); - await switchToNotificationWindow(driver); - await driver.findClickableElement({ text: 'Confirm', tag: 'button' }); + await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); await driver.clickElement({ text: 'Confirm', tag: 'button' }); // Wait for token address to populate in dapp await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); - await driver.wait(async () => { - const tokenAddressesElement = await driver.findElement( - '#tokenAddresses', - ); - const tokenAddresses = await tokenAddressesElement.getText(); - return tokenAddresses !== ''; - }, 10000); + await driver.waitForSelector({ + css: '#tokenAddresses', + text: '0x581c3C1A2A4EBDE2A0Df29B5cf4c116E42945947', + }); // Watch Asset 1st call await driver.clickElement({ @@ -65,11 +60,9 @@ describe('Request Queue WatchAsset -> SwitchChain -> WatchAsset', function () { tag: 'button', }); - await driver.waitUntilXWindowHandles(3); await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); // Switch Ethereum Chain - await driver.findClickableElement('#switchEthereumChain'); await driver.clickElement('#switchEthereumChain'); await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); @@ -83,7 +76,7 @@ describe('Request Queue WatchAsset -> SwitchChain -> WatchAsset', function () { // Wait for token to show in list of tokens to watch await driver.delay(regularDelayMs); - await switchToNotificationWindow(driver); + await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); const multipleSuggestedtokens = await driver.findElements( '.confirm-add-suggested-token__token-list-item', @@ -92,7 +85,7 @@ describe('Request Queue WatchAsset -> SwitchChain -> WatchAsset', function () { // Confirm only 1 token is present in suggested token list assert.equal(multipleSuggestedtokens.length, 1); - await switchToNotificationWindow(driver); + await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); await driver.waitUntilXWindowHandles(2); diff --git a/test/e2e/tests/responsive-ui/metamask-responsive-ui.spec.js b/test/e2e/tests/responsive-ui/metamask-responsive-ui.spec.js index 446d579630bf..958854a5252c 100644 --- a/test/e2e/tests/responsive-ui/metamask-responsive-ui.spec.js +++ b/test/e2e/tests/responsive-ui/metamask-responsive-ui.spec.js @@ -72,10 +72,10 @@ describe('MetaMask Responsive UI', function () { await driver.clickElement('[data-testid="pin-extension-done"]'); await driver.assertElementNotPresent('.loading-overlay__spinner'); // assert balance - const balance = await driver.findElement( - '[data-testid="eth-overview__primary-currency"]', - ); - assert.ok(/^0\sETH$/u.test(await balance.getText())); + await driver.waitForSelector({ + css: '[data-testid="eth-overview__primary-currency"]', + text: '0', + }); }, ); }); @@ -93,11 +93,14 @@ describe('MetaMask Responsive UI', function () { await driver.navigate(); // Import Secret Recovery Phrase - const restoreSeedLink = await driver.findClickableElement( - '.unlock-page__link', - ); - assert.equal(await restoreSeedLink.getText(), 'Forgot password?'); - await restoreSeedLink.click(); + await driver.waitForSelector({ + tag: 'span', + text: 'Localhost 8545', + }); + await driver.clickElement({ + css: '.unlock-page__link', + text: 'Forgot password?', + }); await driver.pasteIntoField( '[data-testid="import-srp__srp-word-0"]', diff --git a/test/e2e/tests/settings/about-metamask-ui-validation.spec.ts b/test/e2e/tests/settings/about-metamask-ui-validation.spec.ts index abc3c4857957..ed2702fef413 100644 --- a/test/e2e/tests/settings/about-metamask-ui-validation.spec.ts +++ b/test/e2e/tests/settings/about-metamask-ui-validation.spec.ts @@ -68,16 +68,11 @@ describe('Setting - About MetaMask : @no-mmi', function (this: Suite) { ); // verify the version number of the MetaMask - const metaMaskVersion = await driver.findElement( - selectors.metaMaskVersion, - ); - const getVersionNumber = await metaMaskVersion.getText(); const { version } = packageJson; - assert.equal( - getVersionNumber, - version, - 'Meta Mask version is incorrect in the about view section', - ); + await driver.waitForSelector({ + css: selectors.metaMaskVersion, + text: version, + }); // Validating the header text const isHeaderTextPresent = await driver.isElementPresent( diff --git a/test/e2e/tests/settings/address-book.spec.js b/test/e2e/tests/settings/address-book.spec.js index e81bd7c544aa..886dc98c174f 100644 --- a/test/e2e/tests/settings/address-book.spec.js +++ b/test/e2e/tests/settings/address-book.spec.js @@ -39,12 +39,11 @@ describe('Address Book', function () { await driver.clickElement({ css: 'button', text: 'Contacts' }); - const recipientTitle = await driver.findElement( - '.address-list-item__label', - ); + await driver.waitForSelector({ + css: '.address-list-item__label', + text: 'Test Name 1', + }); - const recipientRowTitleString = await recipientTitle.getText(); - assert.equal(recipientRowTitleString, 'Test Name 1'); await driver.clickElement('.address-list-item__label'); await driver.fill('input[placeholder="0"]', '2'); @@ -111,25 +110,15 @@ describe('Address Book', function () { await driver.clickElement('[data-testid="page-container-footer-next"]'); - const recipientUsername = await driver.findElement({ + await driver.waitForSelector({ text: 'Test Name Edit', css: '.address-list-item__label', }); - assert.equal( - await recipientUsername.getText(), - 'Test Name Edit', - 'Username is not edited correctly', - ); - - const recipientAddress = await driver.findElement( - '[data-testid="address-list-item-address"]', - ); - assert.equal( - await recipientAddress.getText(), - shortenAddress('0x74cE91B75935D6Bedc27eE002DeFa566c5946f74'), - 'Recipient address is not edited correctly', - ); + await driver.waitForSelector({ + css: '[data-testid="address-list-item-address"]', + text: shortenAddress('0x74cE91B75935D6Bedc27eE002DeFa566c5946f74'), + }); }, ); }); From 4e256177c5bef72fb49ddfa45dfd5f7cc82171ed Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 24 Oct 2024 10:22:53 +0100 Subject: [PATCH 4/5] fix: Fall back to token list for the token symbol (#28003) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28003?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/27970 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** Screenshot 2024-10-22 at 11 19 10 ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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. --- ...-image.test.ts => useTokenDetails.test.ts} | 32 +++++++++++++------ ...{use-token-image.ts => useTokenDetails.ts} | 13 ++++++-- .../info/shared/send-heading/send-heading.tsx | 13 ++++---- 3 files changed, 38 insertions(+), 20 deletions(-) rename ui/pages/confirmations/components/confirm/info/hooks/{use-token-image.test.ts => useTokenDetails.test.ts} (73%) rename ui/pages/confirmations/components/confirm/info/hooks/{use-token-image.ts => useTokenDetails.ts} (65%) diff --git a/ui/pages/confirmations/components/confirm/info/hooks/use-token-image.test.ts b/ui/pages/confirmations/components/confirm/info/hooks/useTokenDetails.test.ts similarity index 73% rename from ui/pages/confirmations/components/confirm/info/hooks/use-token-image.test.ts rename to ui/pages/confirmations/components/confirm/info/hooks/useTokenDetails.test.ts index 23e4cc3c1bda..efdf2b66ac56 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/use-token-image.test.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useTokenDetails.test.ts @@ -2,9 +2,9 @@ import { TransactionMeta } from '@metamask/transaction-controller'; import { genUnapprovedTokenTransferConfirmation } from '../../../../../../../test/data/confirmations/token-transfer'; import mockState from '../../../../../../../test/data/mock-state.json'; import { renderHookWithProvider } from '../../../../../../../test/lib/render-helpers'; -import { useTokenImage } from './use-token-image'; +import { useTokenDetails } from './useTokenDetails'; -describe('useTokenImage', () => { +describe('useTokenDetails', () => { it('returns iconUrl from selected token if it exists', () => { const transactionMeta = genUnapprovedTokenTransferConfirmation( {}, @@ -19,11 +19,14 @@ describe('useTokenImage', () => { }; const { result } = renderHookWithProvider( - () => useTokenImage(transactionMeta, TEST_SELECTED_TOKEN), + () => useTokenDetails(transactionMeta, TEST_SELECTED_TOKEN), mockState, ); - expect(result.current).toEqual({ tokenImage: 'iconUrl' }); + expect(result.current).toEqual({ + tokenImage: 'iconUrl', + tokenSymbol: 'symbol', + }); }); it('returns selected token image if no iconUrl is included', () => { @@ -39,11 +42,14 @@ describe('useTokenImage', () => { }; const { result } = renderHookWithProvider( - () => useTokenImage(transactionMeta, TEST_SELECTED_TOKEN), + () => useTokenDetails(transactionMeta, TEST_SELECTED_TOKEN), mockState, ); - expect(result.current).toEqual({ tokenImage: 'image' }); + expect(result.current).toEqual({ + tokenImage: 'image', + tokenSymbol: 'symbol', + }); }); it('returns token list icon url if no image is included in the token', () => { @@ -58,7 +64,7 @@ describe('useTokenImage', () => { }; const { result } = renderHookWithProvider( - () => useTokenImage(transactionMeta, TEST_SELECTED_TOKEN), + () => useTokenDetails(transactionMeta, TEST_SELECTED_TOKEN), { ...mockState, metamask: { @@ -72,7 +78,10 @@ describe('useTokenImage', () => { }, ); - expect(result.current).toEqual({ tokenImage: 'tokenListIconUrl' }); + expect(result.current).toEqual({ + tokenImage: 'tokenListIconUrl', + tokenSymbol: 'symbol', + }); }); it('returns undefined if no image is found', () => { @@ -87,10 +96,13 @@ describe('useTokenImage', () => { }; const { result } = renderHookWithProvider( - () => useTokenImage(transactionMeta, TEST_SELECTED_TOKEN), + () => useTokenDetails(transactionMeta, TEST_SELECTED_TOKEN), mockState, ); - expect(result.current).toEqual({ tokenImage: undefined }); + expect(result.current).toEqual({ + tokenImage: undefined, + tokenSymbol: 'symbol', + }); }); }); diff --git a/ui/pages/confirmations/components/confirm/info/hooks/use-token-image.ts b/ui/pages/confirmations/components/confirm/info/hooks/useTokenDetails.ts similarity index 65% rename from ui/pages/confirmations/components/confirm/info/hooks/use-token-image.ts rename to ui/pages/confirmations/components/confirm/info/hooks/useTokenDetails.ts index 5817d08028ab..be9578496205 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/use-token-image.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useTokenDetails.ts @@ -1,20 +1,27 @@ import { TokenListMap } from '@metamask/assets-controllers'; import { TransactionMeta } from '@metamask/transaction-controller'; import { useSelector } from 'react-redux'; +import { useI18nContext } from '../../../../../../hooks/useI18nContext'; import { getTokenList } from '../../../../../../selectors'; import { SelectedToken } from '../shared/selected-token'; -export const useTokenImage = ( +export const useTokenDetails = ( transactionMeta: TransactionMeta, selectedToken: SelectedToken, ) => { + const t = useI18nContext(); + const tokenList = useSelector(getTokenList) as TokenListMap; - // TODO: Add support for NFT images in one of the following tasks const tokenImage = selectedToken?.iconUrl || selectedToken?.image || tokenList[transactionMeta?.txParams?.to as string]?.iconUrl; - return { tokenImage }; + const tokenSymbol = + selectedToken?.symbol || + tokenList[transactionMeta?.txParams?.to as string]?.symbol || + t('unknown'); + + return { tokenImage, tokenSymbol }; }; diff --git a/ui/pages/confirmations/components/confirm/info/shared/send-heading/send-heading.tsx b/ui/pages/confirmations/components/confirm/info/shared/send-heading/send-heading.tsx index 2806c33936c0..40c571d4bc75 100644 --- a/ui/pages/confirmations/components/confirm/info/shared/send-heading/send-heading.tsx +++ b/ui/pages/confirmations/components/confirm/info/shared/send-heading/send-heading.tsx @@ -16,22 +16,23 @@ import { TextColor, TextVariant, } from '../../../../../../../helpers/constants/design-system'; -import { useI18nContext } from '../../../../../../../hooks/useI18nContext'; import { getWatchedToken } from '../../../../../../../selectors'; import { MultichainState } from '../../../../../../../selectors/multichain'; import { useConfirmContext } from '../../../../../context/confirm'; -import { useTokenImage } from '../../hooks/use-token-image'; +import { useTokenDetails } from '../../hooks/useTokenDetails'; import { useTokenValues } from '../../hooks/use-token-values'; import { ConfirmLoader } from '../confirm-loader/confirm-loader'; const SendHeading = () => { - const t = useI18nContext(); const { currentConfirmation: transactionMeta } = useConfirmContext(); const selectedToken = useSelector((state: MultichainState) => getWatchedToken(transactionMeta)(state), ); - const { tokenImage } = useTokenImage(transactionMeta, selectedToken); + const { tokenImage, tokenSymbol } = useTokenDetails( + transactionMeta, + selectedToken, + ); const { decodedTransferValue, fiatDisplayValue, pending } = useTokenValues(transactionMeta); @@ -57,9 +58,7 @@ const SendHeading = () => { variant={TextVariant.headingLg} color={TextColor.inherit} marginTop={3} - >{`${decodedTransferValue || ''} ${ - selectedToken?.symbol || t('unknown') - }`} + >{`${decodedTransferValue || ''} ${tokenSymbol}`} {fiatDisplayValue && ( {fiatDisplayValue} From 2ae92364cca791bde80072a62da7c45dd809a20a Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 24 Oct 2024 10:23:26 +0100 Subject: [PATCH 5/5] fix: Support dynamic native token name on gas component (#28048) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Uses the multinetwork ticker. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28048?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/28001 ## **Manual testing steps** See original ticket linked above. ## **Screenshots/Recordings** ### **Before** ### **After** Screenshot 2024-10-23 at 16 16 19 ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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. --- .../transactions/contract-deployment.test.tsx | 4 ++-- .../transactions/contract-interaction.test.tsx | 4 ++-- .../confirm/info/hooks/useFeeCalculations.test.ts | 4 ++-- .../confirm/info/hooks/useFeeCalculations.ts | 12 ++++++++---- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/test/integration/confirmations/transactions/contract-deployment.test.tsx b/test/integration/confirmations/transactions/contract-deployment.test.tsx index ecef04f30861..c2625e06e3e7 100644 --- a/test/integration/confirmations/transactions/contract-deployment.test.tsx +++ b/test/integration/confirmations/transactions/contract-deployment.test.tsx @@ -283,7 +283,7 @@ describe('Contract Deployment Confirmation', () => { expect(editGasFeesRow).toHaveTextContent(tEn('networkFee') as string); const firstGasField = within(editGasFeesRow).getByTestId('first-gas-field'); - expect(firstGasField).toHaveTextContent('0.0001 ETH'); + expect(firstGasField).toHaveTextContent('0.0001 SepoliaETH'); const editGasFeeNativeCurrency = within(editGasFeesRow).getByTestId('native-currency'); expect(editGasFeeNativeCurrency).toHaveTextContent('$0.47'); @@ -371,7 +371,7 @@ describe('Contract Deployment Confirmation', () => { const maxFee = screen.getByTestId('gas-fee-details-max-fee'); expect(gasFeesSection).toContainElement(maxFee); expect(maxFee).toHaveTextContent(tEn('maxFee') as string); - expect(maxFee).toHaveTextContent('0.0023 ETH'); + expect(maxFee).toHaveTextContent('0.0023 SepoliaETH'); expect(maxFee).toHaveTextContent('$7.72'); const nonceSection = screen.getByTestId('advanced-details-nonce-section'); diff --git a/test/integration/confirmations/transactions/contract-interaction.test.tsx b/test/integration/confirmations/transactions/contract-interaction.test.tsx index 1102cb21c67d..b77e48f1d660 100644 --- a/test/integration/confirmations/transactions/contract-interaction.test.tsx +++ b/test/integration/confirmations/transactions/contract-interaction.test.tsx @@ -301,7 +301,7 @@ describe('Contract Interaction Confirmation', () => { expect(editGasFeesRow).toHaveTextContent(tEn('networkFee') as string); const firstGasField = within(editGasFeesRow).getByTestId('first-gas-field'); - expect(firstGasField).toHaveTextContent('0.0001 ETH'); + expect(firstGasField).toHaveTextContent('0.0001 SepoliaETH'); const editGasFeeNativeCurrency = within(editGasFeesRow).getByTestId('native-currency'); expect(editGasFeeNativeCurrency).toHaveTextContent('$0.47'); @@ -402,7 +402,7 @@ describe('Contract Interaction Confirmation', () => { const maxFee = screen.getByTestId('gas-fee-details-max-fee'); expect(gasFeesSection).toContainElement(maxFee); expect(maxFee).toHaveTextContent(tEn('maxFee') as string); - expect(maxFee).toHaveTextContent('0.0023 ETH'); + expect(maxFee).toHaveTextContent('0.0023 SepoliaETH'); expect(maxFee).toHaveTextContent('$7.72'); const nonceSection = screen.getByTestId('advanced-details-nonce-section'); diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.test.ts b/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.test.ts index 911cdb20118c..17c8ab8dd8f6 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.test.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.test.ts @@ -22,13 +22,13 @@ describe('useFeeCalculations', () => { expect(result.current).toMatchInlineSnapshot(` { "estimatedFeeFiat": "$0.00", - "estimatedFeeNative": "0 WEI", + "estimatedFeeNative": "0 ETH", "l1FeeFiat": "", "l1FeeNative": "", "l2FeeFiat": "", "l2FeeNative": "", "maxFeeFiat": "$0.00", - "maxFeeNative": "0 WEI", + "maxFeeNative": "0 ETH", } `); }); diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.ts b/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.ts index 587d70c9c9ef..70bd2c0e3af2 100644 --- a/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.ts +++ b/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.ts @@ -8,7 +8,6 @@ import { addHexes, decGWEIToHexWEI, decimalToHex, - getEthConversionFromWeiHex, getValueFromWeiHex, multiplyHexes, } from '../../../../../../../shared/modules/conversion.utils'; @@ -17,6 +16,7 @@ import { getConversionRate } from '../../../../../../ducks/metamask/metamask'; import { useFiatFormatter } from '../../../../../../hooks/useFiatFormatter'; import { useGasFeeEstimates } from '../../../../../../hooks/useGasFeeEstimates'; import { getCurrentCurrency } from '../../../../../../selectors'; +import { getMultichainNetwork } from '../../../../../../selectors/multichain'; import { HEX_ZERO } from '../shared/constants'; import { useEIP1559TxFees } from './useEIP1559TxFees'; import { useSupportsEIP1559 } from './useSupportsEIP1559'; @@ -33,14 +33,18 @@ export function useFeeCalculations(transactionMeta: TransactionMeta) { const conversionRate = useSelector(getConversionRate); const fiatFormatter = useFiatFormatter(); + const multichainNetwork = useSelector(getMultichainNetwork); + const ticker = multichainNetwork?.network?.ticker; + const getFeesFromHex = useCallback( (hexFee: string) => { - const nativeCurrencyFee = - getEthConversionFromWeiHex({ + const nativeCurrencyFee = `${ + getValueFromWeiHex({ value: hexFee, fromCurrency: EtherDenomination.GWEI, numberOfDecimals: 4, - }) || `0 ${EtherDenomination.ETH}`; + }) || 0 + } ${ticker}`; const currentCurrencyFee = fiatFormatter( Number(