Skip to content

Commit

Permalink
chore: MMI adds note to trader support to the new Tx confirmation view (
Browse files Browse the repository at this point in the history
#27214)

## **Description**

Adds the MMI transaction "note to trader" functionality to the new
transactions redesign, the current one is also kept in order to
facilitate backwards compatibility with the current confirmation view
(to be removed later).
It also adds a few parameters that were missing for MMI like the tx
metadata.


![note_to_trader_ss](https://github.com/user-attachments/assets/7a7f123a-f0de-48f9-8646-40aabd5e27b2)


## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **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.
  • Loading branch information
zone-live authored Sep 23, 2024
1 parent 5e011f8 commit 34259a3
Show file tree
Hide file tree
Showing 23 changed files with 355 additions and 93 deletions.
6 changes: 6 additions & 0 deletions app/scripts/controllers/app-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,12 @@ export default class AppStateController extends EventEmitter {
});
}

setNoteToTraderMessage(message) {
this.store.updateState({
noteToTraderMessage: message,
});
}

///: END:ONLY_INCLUDE_IF

getSignatureSecurityAlertResponse(securityAlertId) {
Expand Down
19 changes: 19 additions & 0 deletions app/scripts/controllers/app-state.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -374,5 +374,24 @@ describe('AppStateController', () => {

updateStateSpy.mockRestore();
});

it('set the setNoteToTraderMessage with a message', () => {
appStateController = createAppStateController();
const updateStateSpy = jest.spyOn(
appStateController.store,
'updateState',
);

const mockParams = 'some message';

appStateController.setNoteToTraderMessage(mockParams);

expect(updateStateSpy).toHaveBeenCalledTimes(1);
expect(updateStateSpy).toHaveBeenCalledWith({
noteToTraderMessage: mockParams,
});

updateStateSpy.mockRestore();
});
});
});
2 changes: 2 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -3672,6 +3672,8 @@ export default class MetamaskController extends EventEmitter {
),
setCustodianDeepLink:
appStateController.setCustodianDeepLink.bind(appStateController),
setNoteToTraderMessage:
appStateController.setNoteToTraderMessage.bind(appStateController),
///: END:ONLY_INCLUDE_IF

// snaps
Expand Down
9 changes: 4 additions & 5 deletions test/e2e/playwright/mmi/specs/extension.visual.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,12 @@ test.describe('MMI extension', () => {
await accountsPopup.connectCustodian(
process.env.MMI_E2E_CUSTODIAN_NAME as string,
);

const accountNamesWithCustodian = await accountsPopup.getAccountNames();

expect(
JSON.stringify(accountNamesWithCustodian) ===
JSON.stringify(arrayWithCustodianAccounts),
).toBeTruthy();
const containsAccount = arrayWithCustodianAccounts.some((account) =>
accountNamesWithCustodian.includes(account),
);
expect(containsAccount).toBeTruthy();

await accountsPopup.selectCustodyAccount(accountFrom);
// Check remove custodian token screen (aborted before removed)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`NoteToTrader should render the Note to trader component 1`] = `
<div>
<div
class="mm-box confirm-page-container-content__data"
class="mm-box mm-box--margin-bottom-4 mm-box--padding-0 mm-box--background-color-background-default mm-box--rounded-md"
>
<div
class="mm-box mm-box--padding-4 mm-box--display-flex mm-box--flex-direction-column"
Expand All @@ -20,7 +20,7 @@ exports[`NoteToTrader should render the Note to trader component 1`] = `
<p
class="mm-box mm-text note-header__counter mm-text--body-md mm-box--color-text-default"
>
9
0
/
280
</p>
Expand All @@ -29,13 +29,13 @@ exports[`NoteToTrader should render the Note to trader component 1`] = `
class="mm-box note-field mm-box--display-flex mm-box--flex-direction-column"
>
<textarea
class="mm-box mm-text mm-textarea mm-textarea--resize-vertical mm-text--body-md mm-box--padding-2 mm-box--padding-top-1 mm-box--padding-right-4 mm-box--padding-bottom-1 mm-box--padding-left-4 mm-box--width-full mm-box--height-full mm-box--color-text-default mm-box--background-color-background-default mm-box--rounded-sm mm-box--border-color-border-default mm-box--border-width-1 box--border-style-solid"
data-testid="transaction-note"
id="transaction-note"
maxlength="280"
placeholder=""
>
some text
</textarea>
placeholder="The approver will see this note when approving the transaction at the custodian."
resize="vertical"
/>
</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import React from 'react';
import {
Display,
FlexDirection,
JustifyContent,
} from '../../../helpers/constants/design-system';
import { Label, Box, Text } from '../../component-library';

type NoteToTraderProps = {
placeholder: string;
maxLength: number;
onChange: (value: string) => void;
noteText: string;
labelText: string;
};

const TabbedNoteToTrader: React.FC<NoteToTraderProps> = ({
placeholder,
maxLength,
onChange,
noteText,
labelText,
}) => {
return (
<Box className="confirm-page-container-content__data">
<Box
display={Display.Flex}
flexDirection={FlexDirection.Column}
padding={4}
>
<Box
className="note-header"
display={Display.Flex}
justifyContent={JustifyContent.spaceBetween}
>
<Label htmlFor="transaction-note">{labelText}</Label>
<Text className="note-header__counter">
{noteText.length}/{maxLength}
</Text>
</Box>
<Box
display={Display.Flex}
flexDirection={FlexDirection.Column}
className="note-field"
>
<textarea
id="transaction-note"
data-testid="transaction-note"
onChange={({ target: { value } }) => onChange(value)}
autoFocus
maxLength={maxLength}
placeholder={placeholder}
value={noteText}
/>
</Box>
</Box>
</Box>
);
};

export default TabbedNoteToTrader;
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
import React from 'react';
import { Meta } from '@storybook/react';
import { Provider } from 'react-redux';
import NoteToTrader from '.';
import { getMockContractInteractionConfirmState } from '../../../../test/data/confirmations/helper';
import configureStore from '../../../store/store';
import { ConfirmContextProvider } from '../../../pages/confirmations/context/confirm';

const store = configureStore(getMockContractInteractionConfirmState());

export default {
title: 'Components/Institutional/NoteToTrader',
component: NoteToTrader,
decorators: [
(story: () => Meta<typeof NoteToTrader>) => (
<Provider store={store}>
<ConfirmContextProvider>{story()}</ConfirmContextProvider>
</Provider>
),
],
args: {
placeholder:
'The approver will see this note when approving the transaction at the custodian.',
Expand Down
27 changes: 17 additions & 10 deletions ui/components/institutional/note-to-trader/note-to-trader.test.tsx
Original file line number Diff line number Diff line change
@@ -1,24 +1,31 @@
import { render, fireEvent } from '@testing-library/react';
import { fireEvent } from '@testing-library/react';
import React from 'react';
import configureMockStore from 'redux-mock-store';
import thunk from 'redux-thunk';
import { renderWithConfirmContextProvider } from '../../../../test/lib/confirmations/render-helpers';
import mockState from '../../../../test/data/mock-state.json';
import NoteToTrader from './note-to-trader';

jest.mock('../../../selectors/institutional/selectors', () => ({
getIsNoteToTraderSupported: () => true,
}));

const middleware = [thunk];
const store = configureMockStore(middleware)(mockState);

describe('NoteToTrader', () => {
it('should render the Note to trader component', () => {
const props = {
placeholder: '',
maxLength: 280,
noteText: 'some text',
labelText: 'Transaction note',
onChange: jest.fn(),
};
const { getByTestId, container } = renderWithConfirmContextProvider(
<NoteToTrader />,
store,
);

const { getByTestId, container } = render(<NoteToTrader {...props} />);
const transactionNoteInput = getByTestId(
'transaction-note',
) as HTMLInputElement;

fireEvent.change(transactionNoteInput);
expect(transactionNoteInput.value).toBe('some text');
expect(transactionNoteInput.value).toBe('');
expect(transactionNoteInput).toBeDefined();
expect(container).toMatchSnapshot();
});
Expand Down
78 changes: 53 additions & 25 deletions ui/components/institutional/note-to-trader/note-to-trader.tsx
Original file line number Diff line number Diff line change
@@ -1,28 +1,54 @@
import React from 'react';
import React, { useEffect, useState } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import {
BackgroundColor,
BlockSize,
BorderRadius,
Display,
FlexDirection,
JustifyContent,
} from '../../../helpers/constants/design-system';
import { Label, Box, Text } from '../../component-library';
import { Textarea } from '../../component-library/textarea';
import { setNoteToTraderMessage } from '../../../store/institutional/institution-background';
import {
getIsNoteToTraderSupported,
State,
} from '../../../selectors/institutional/selectors';
import { toChecksumHexAddress } from '../../../../shared/modules/hexstring-utils';
import { useConfirmContext } from '../../../pages/confirmations/context/confirm';
import { getConfirmationSender } from '../../../pages/confirmations/components/confirm/utils';
import { useI18nContext } from '../../../hooks/useI18nContext';
import { isSignatureTransactionType } from '../../../pages/confirmations/utils';

type NoteToTraderProps = {
placeholder: string;
maxLength: number;
onChange: (value: string) => void;
noteText: string;
labelText: string;
};
const NoteToTrader: React.FC = () => {
const dispatch = useDispatch();
const t = useI18nContext();
const [noteText, setNoteText] = useState('');

const { currentConfirmation } = useConfirmContext();
const isSignature = isSignatureTransactionType(currentConfirmation);
const { from } = getConfirmationSender(currentConfirmation);
const fromChecksumHexAddress = toChecksumHexAddress(from || '');
const isNoteToTraderSupported = useSelector((state: State) =>
getIsNoteToTraderSupported(state, fromChecksumHexAddress),
);

const NoteToTrader: React.FC<NoteToTraderProps> = ({
placeholder,
maxLength,
onChange,
noteText,
labelText,
}) => {
return (
<Box className="confirm-page-container-content__data">
useEffect(() => {
const timer = setTimeout(() => {
dispatch(setNoteToTraderMessage(noteText));
}, 700);

return () => clearTimeout(timer);
}, [noteText]);

return isNoteToTraderSupported && !isSignature ? (
<Box
backgroundColor={BackgroundColor.backgroundDefault}
borderRadius={BorderRadius.MD}
padding={0}
marginBottom={4}
>
<Box
display={Display.Flex}
flexDirection={FlexDirection.Column}
Expand All @@ -33,29 +59,31 @@ const NoteToTrader: React.FC<NoteToTraderProps> = ({
display={Display.Flex}
justifyContent={JustifyContent.spaceBetween}
>
<Label htmlFor="transaction-note">{labelText}</Label>
<Label htmlFor="transaction-note">{t('transactionNote')}</Label>
<Text className="note-header__counter">
{noteText.length}/{maxLength}
{noteText.length}/{280}
</Text>
</Box>
<Box
display={Display.Flex}
flexDirection={FlexDirection.Column}
className="note-field"
>
<textarea
<Textarea
id="transaction-note"
data-testid="transaction-note"
onChange={({ target: { value } }) => onChange(value)}
autoFocus
maxLength={maxLength}
placeholder={placeholder}
onChange={({ target: { value } }) => setNoteText(value)}
value={noteText}
height={BlockSize.Full}
width={BlockSize.Full}
maxLength={280}
placeholder={t('notePlaceholder')}
padding={2}
/>
</Box>
</Box>
</Box>
);
) : null;
};

export default NoteToTrader;
6 changes: 4 additions & 2 deletions ui/hooks/useMMIConfirmations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ export function useMMIConfirmations() {
currentConfirmation.type === TransactionType.signTypedData) &&
Boolean(currentConfirmation?.custodyId),
mmiOnSignCallback: () => custodySignFn(currentConfirmation),
mmiOnTransactionCallback: () =>
custodyTransactionFn(currentConfirmation as TransactionMeta),
mmiOnTransactionCallback: (
transactionData: TransactionMeta,
noteToTraderMessage: string,
) => custodyTransactionFn(transactionData, noteToTraderMessage),
};
}
16 changes: 11 additions & 5 deletions ui/hooks/useMMICustodySendTransaction.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { useMMICustodySendTransaction } from './useMMICustodySendTransaction';

jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useHistory: jest.fn(),
useHistory: jest.fn().mockReturnValue({ push: jest.fn() }),
}));

jest.mock('react-redux', () => ({
Expand All @@ -23,14 +23,20 @@ jest.mock('../store/institutional/institution-background', () => ({
mmiActionsFactory: jest.fn(),
}));

jest.mock('../selectors', () => ({
getAccountType: jest.fn(),
}));

jest.mock('../store/actions', () => ({
updateAndApproveTx: jest.fn(),
}));

jest.mock('../pages/confirmations/context/confirm', () => ({
useConfirmContext: () => ({
currentConfirmation: { from: '0x123' },
}),
}));

jest.mock('../pages/confirmations/components/confirm/utils', () => ({
getConfirmationSender: () => ({ from: '0x123' }),
}));

describe('useMMICustodySendTransaction', () => {
it('handles custody account type', async () => {
const dispatch = jest.fn();
Expand Down
Loading

0 comments on commit 34259a3

Please sign in to comment.