Skip to content

Commit

Permalink
feat: overhaul AssetPickerAmount (#23192)
Browse files Browse the repository at this point in the history
## **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?
2. What is the improvement/solution?
-->

As part of the ongoing monetization initiative in default experiences,
Swap+Send was prioritized and assigned to the bridge team in an effort
to capture revenue immediately following a send transaction; these
typically go to MetaMask alternatives (i.e., Uniswap).

A blocking for this flow is an agnostic,
[design-compliant](https://www.figma.com/file/kVg6zwoyHt0fH2MGBlLrM3/Asset-Picker-(Extension)?type=design&node-id=224-149854&mode=design&t=ZUzvFEl0He30NL5M-0),
component. Therefore, this PR aims to build a robust, optimized
component to put at the heart of the Swap+Send experience.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23192?quickstart=1)

## **Related issues**


[METABRIDGE-843](https://consensyssoftware.atlassian.net/browse/METABRIDGE-843)

## **Manual testing steps**

1. Run local server with `MULTICHAIN` environment variable set to `1`
2. Go to Swap page
3. Ensure that the updated component is there (see:
[designs](https://www.figma.com/file/kVg6zwoyHt0fH2MGBlLrM3/Asset-Picker-(Extension)?type=design&node-id=224-149854&mode=design&t=BLuV7Q9MQ0QogGnR-0))
4. Ensure that toggling the swap button switches the places of the fiat
and raw token amount
- ensure that both accept input and correctly adjust their counterpart
5. Ensure that the max button only shows for tokens when the raw token
is in the input field – native token transfers will be slightly reduced
for gas
6. Ensure that the token dropdown correctly adjusts when an ERC-721 or
ERC-1155 is selected (see:
[designs](https://www.figma.com/file/kVg6zwoyHt0fH2MGBlLrM3/Asset-Picker-(Extension)?type=design&node-id=224-149854&mode=design&t=BLuV7Q9MQ0QogGnR-0))

## **Screenshots/Recordings**

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

### Token

#### **Before**

<img width="371" alt="token old"
src="https://github.com/MetaMask/metamask-extension/assets/44588480/85598e7d-d436-499c-833e-c273535db824">


#### **After**

<img width="342" alt="token new"
src="https://github.com/MetaMask/metamask-extension/assets/44588480/6a39c26e-b456-488e-88af-95c8d725b5a2">

### ERC-721

#### **Before**

<img width="371" alt="erc721 old"
src="https://github.com/MetaMask/metamask-extension/assets/44588480/7f8762de-1c19-4cdb-aa1c-f90f08d1f279">

#### **After**

<img width="342" alt="erc721 new"
src="https://github.com/MetaMask/metamask-extension/assets/44588480/2d5e768d-1864-4699-a75b-cd47922936a4">

### ERC-1155

#### **Before**

<img width="371" alt="erc1155 old"
src="https://github.com/MetaMask/metamask-extension/assets/44588480/6357dd4b-0178-440a-a4ba-79502385843e">

#### **After**

<img width="342" alt="erc1155 new"
src="https://github.com/MetaMask/metamask-extension/assets/44588480/18590bb0-abaa-4a56-9710-2325a99601ee">

### New flow recording


https://github.com/MetaMask/metamask-extension/assets/44588480/b5067372-ccec-4d34-977c-8dd8e42e8dc5


## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [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.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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
BZahory authored Mar 21, 2024
1 parent c474395 commit c6f4e61
Show file tree
Hide file tree
Showing 78 changed files with 2,402 additions and 701 deletions.
2 changes: 1 addition & 1 deletion app/images/eth_logo.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
16 changes: 14 additions & 2 deletions test/e2e/tests/transaction/send-edit.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,13 @@ describe('Editing Confirm Transaction', function () {
await driver.clickElement(
'.confirm-page-container-header__back-button',
);
await driver.fill('.unit-input__input', '2.2');

const inputAmount = await driver.findElement('.unit-input__input');

await inputAmount.press(driver.Key.BACK_SPACE);
await inputAmount.press('2');
await inputAmount.press('.');
await inputAmount.press('2');

await driver.clickElement({ text: 'Next', tag: 'button' });

Expand Down Expand Up @@ -103,7 +109,13 @@ describe('Editing Confirm Transaction', function () {
await driver.clickElement(
'.confirm-page-container-header__back-button',
);
await driver.fill('.unit-input__input', '2.2');

const inputAmount = await driver.findElement('.unit-input__input');

await inputAmount.press(driver.Key.BACK_SPACE);
await inputAmount.press('2');
await inputAmount.press('.');
await inputAmount.press('2');

await driver.clickElement({ text: 'Next', tag: 'button' });

Expand Down
10 changes: 7 additions & 3 deletions test/e2e/tests/transaction/send-eth.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ describe('Send ETH', function () {
);

const inputAmount = await driver.findElement('.unit-input__input');
await inputAmount.fill('1000');

await inputAmount.press('1');
await inputAmount.press('0');
await inputAmount.press('0');
await inputAmount.press('0');

await driver.findElement({
css: '.send-v2__error-amount',
Expand Down Expand Up @@ -113,7 +117,7 @@ describe('Send ETH', function () {
);

const inputAmount = await driver.findElement('.unit-input__input');
await inputAmount.fill('1');
await inputAmount.press('1');

const inputValue = await inputAmount.getProperty('value');
assert.equal(inputValue, '1');
Expand Down Expand Up @@ -170,7 +174,7 @@ describe('Send ETH', function () {
);

const inputAmount = await driver.findElement('.unit-input__input');
await inputAmount.fill('1');
await inputAmount.press('1');

if (!process.env.MULTICHAIN) {
// We need to wait for the text "Max Fee: 0.000xxxx ETH" before continuing
Expand Down
5 changes: 5 additions & 0 deletions types/global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,4 +244,9 @@ export declare global {
toNeverResolve(): Promise<R>;
}
}

/**
* Unions T with U; U's properties will override T's properties
*/
type OverridingUnion<T, U> = Omit<T, keyof U> & U;
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,19 @@ exports[`CurrencyInput Component rendering should render properly with a fiat va
>
<div
class="unit-input__input-container"
style="display: inherit;"
title=""
>
<input
class="unit-input__input"
data-testid="currency-input"
dir="ltr"
min="0"
placeholder="0"
step="any"
style="width: 1.5ch;"
type="number"
value="1"
value="0"
/>
<div
class="unit-input__suffix"
Expand All @@ -28,12 +32,12 @@ exports[`CurrencyInput Component rendering should render properly with a fiat va
</div>
<div
class="mm-box currency-display-component currency-input__conversion-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
title="0.00432788 ETH"
title="0 ETH"
>
<span
class="mm-box mm-text currency-display-component__text mm-text--inherit mm-text--ellipsis mm-box--color-text-default"
>
0.00432788
0
</span>
<span
class="mm-box mm-text currency-display-component__suffix mm-text--inherit mm-box--margin-inline-start-1 mm-box--color-text-default"
Expand Down Expand Up @@ -64,15 +68,19 @@ exports[`CurrencyInput Component rendering should render properly with a native
>
<div
class="unit-input__input-container"
style="display: inherit;"
title="0.004327880204275946"
>
<input
class="unit-input__input"
data-testid="currency-input"
dir="ltr"
min="0"
placeholder="0"
style="width: 10ch;"
step="any"
style="width: 20ch;"
type="number"
value="0.00432788"
value="0"
/>
<div
class="unit-input__suffix"
Expand All @@ -86,14 +94,6 @@ exports[`CurrencyInput Component rendering should render properly with a native
No conversion rate available
</div>
</div>
<button
class="currency-input__swap-component"
data-testid="currency-swap"
>
<i
class="fa fa-retweet fa-lg"
/>
</button>
</div>
</div>
`;
Expand All @@ -108,15 +108,19 @@ exports[`CurrencyInput Component rendering should render properly with an ETH va
>
<div
class="unit-input__input-container"
style="display: inherit;"
title="1"
>
<input
class="unit-input__input"
data-testid="currency-input"
dir="ltr"
min="0"
placeholder="0"
step="any"
style="width: 1.5ch;"
type="number"
value="1"
value="0"
/>
<div
class="unit-input__suffix"
Expand All @@ -126,18 +130,13 @@ exports[`CurrencyInput Component rendering should render properly with an ETH va
</div>
<div
class="mm-box currency-display-component currency-input__conversion-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
title="$231.06 USD"
title="$231.06"
>
<span
class="mm-box mm-text currency-display-component__text mm-text--inherit mm-text--ellipsis mm-box--color-text-default"
>
$231.06
</span>
<span
class="mm-box mm-text currency-display-component__suffix mm-text--inherit mm-box--margin-inline-start-1 mm-box--color-text-default"
>
USD
</span>
</div>
</div>
<button
Expand All @@ -162,12 +161,16 @@ exports[`CurrencyInput Component rendering should render properly without a suff
>
<div
class="unit-input__input-container"
style="display: inherit;"
title=""
>
<input
class="unit-input__input"
data-testid="currency-input"
dir="ltr"
min="0"
placeholder="0"
step="any"
style="width: 1.5ch;"
type="number"
value="0"
Expand All @@ -180,18 +183,13 @@ exports[`CurrencyInput Component rendering should render properly without a suff
</div>
<div
class="mm-box currency-display-component currency-input__conversion-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
title="$0.00 USD"
title="$0.00"
>
<span
class="mm-box mm-text currency-display-component__text mm-text--inherit mm-text--ellipsis mm-box--color-text-default"
>
$0.00
</span>
<span
class="mm-box mm-text currency-display-component__suffix mm-text--inherit mm-box--margin-inline-start-1 mm-box--color-text-default"
>
USD
</span>
</div>
</div>
<button
Expand Down
Loading

0 comments on commit c6f4e61

Please sign in to comment.