Skip to content

Commit dec5421

Browse files
authored
fix: asset picker touch ups (#23264)
## **Description** Fixes some things with the asset picker. 1. Too tall. Vertical padding was increased in [4a930f9](4a930f9#diff-0c6f888c60276323462869ba5dbaf02088f5f06b37c2adb424f38caa8462608cR131-R13) to support the NFT case, but this made native + erc20 too tall. before: <img width="345" alt="Screenshot 2024-02-29 at 11 39 32 PM" src="https://github.com/MetaMask/metamask-extension/assets/3500406/bea36359-bc83-4cbe-ab17-898b3afcb68e"> after: <img width="345" alt="Screenshot 2024-02-29 at 11 44 57 PM" src="https://github.com/MetaMask/metamask-extension/assets/3500406/28028be7-f2fb-4758-a2fe-5208e99fa2a5"> [original figma design](https://www.figma.com/file/LDlI7yjKBNIY60MtQv9QS0/Amon-Hen%2FV1?node-id=2385%3A169151&mode=dev): <img width="317" alt="Screenshot 2024-02-29 at 11 32 53 PM" src="https://github.com/MetaMask/metamask-extension/assets/3500406/5f20c6e0-08da-49bb-8346-5335286ee89e"> 2. Token id not always right aligned during NFT case 3. Images missing from pill in NFT and some ERC20 cases 4. ERC721 balance had regressed - showing blank instead of # of NFTs you own. before: <img width="344" alt="Screenshot 2024-03-05 at 12 14 20 PM" src="https://github.com/MetaMask/metamask-extension/assets/3500406/b120518a-b875-4718-857c-d2a6f9615ce6"> after: <img width="347" alt="Screenshot 2024-02-29 at 11 53 51 PM" src="https://github.com/MetaMask/metamask-extension/assets/3500406/2180ab8f-5260-43cf-8250-592e5ce11c77"> [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23264?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** - Send an NFT in the new asset flow, verify the pill has the NFT's image ## **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 Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've clearly explained what problem this PR is solving and how it is solved. - [ ] I've linked related issues - [ ] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [ ] 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. - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] 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.
1 parent a43c645 commit dec5421

File tree

3 files changed

+39
-25
lines changed

3 files changed

+39
-25
lines changed

shared/constants/tokens.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,12 @@ export const LISTED_CONTRACT_ADDRESSES = Object.keys(contractMap).map(
1818
* @property {number} [decimals] - The number of decimals of the selected
1919
* 'ERC20' asset.
2020
* @property {number} [tokenId] - The id of the selected 'NFT' asset.
21+
* @property {string} [image] - A URL to the image of the NFT or ERC20 icon.
2122
* @property {TokenStandardStrings} [standard] - The standard of the selected
2223
* asset.
2324
* @property {boolean} [isERC721] - True when the asset is a ERC721 token.
2425
*/
26+
2527
export const STATIC_MAINNET_TOKEN_LIST = Object.keys(contractMap).reduce(
2628
(acc, base) => {
2729
const { logo, ...tokenMetadata } = contractMap[base];

ui/components/multichain/asset-picker-amount/asset-picker-amount.tsx

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,14 @@ const renderCurrencyInput = (asset: Asset, amount: Amount) => {
8282
) {
8383
return (
8484
<>
85-
<Box marginLeft={'auto'}>
85+
<Box
86+
marginLeft={'auto'}
87+
textAlign={TextAlign.End}
88+
paddingTop={2}
89+
paddingBottom={2}
90+
>
8691
<Text variant={TextVariant.bodySm}>{t('tokenId')}</Text>
87-
<Text
88-
variant={TextVariant.bodySm}
89-
fontWeight={FontWeight.Bold}
90-
marginLeft={10}
91-
>
92+
<Text variant={TextVariant.bodySm} fontWeight={FontWeight.Bold}>
9293
{asset?.details?.tokenId}
9394
</Text>
9495
</Box>
@@ -100,13 +101,7 @@ const renderCurrencyInput = (asset: Asset, amount: Amount) => {
100101
asset.details?.standard === TokenStandard.ERC1155
101102
) {
102103
return (
103-
<Box
104-
marginLeft={'auto'}
105-
textAlign={TextAlign.End}
106-
paddingTop={2}
107-
paddingBottom={2}
108-
width={BlockSize.Max}
109-
>
104+
<Box marginLeft={'auto'} textAlign={TextAlign.End} width={BlockSize.Max}>
110105
<Text variant={TextVariant.bodyMd} ellipsis>
111106
{t('amount')}
112107
</Text>
@@ -163,7 +158,10 @@ export const AssetPickerAmount = () => {
163158
/>
164159
);
165160
}
166-
if (asset.details?.standard === TokenStandard.ERC20) {
161+
if (
162+
asset.details?.standard === TokenStandard.ERC20 ||
163+
asset.details?.standard === TokenStandard.ERC721
164+
) {
167165
return (
168166
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
169167
// @ts-ignore: Details should be defined for token assets
@@ -214,13 +212,13 @@ export const AssetPickerAmount = () => {
214212
paddingRight={4}
215213
borderRadius={BorderRadius.LG}
216214
borderColor={
217-
amount.error ? BorderColor.errorDefault : BorderColor.primaryDefault
215+
error ? BorderColor.errorDefault : BorderColor.primaryDefault
218216
}
219217
borderStyle={BorderStyle.solid}
220218
borderWidth={2}
221219
marginTop={2}
222-
paddingTop={3}
223-
paddingBottom={3}
220+
paddingTop={1}
221+
paddingBottom={1}
224222
>
225223
<AssetPicker asset={asset} />
226224
{renderCurrencyInput(asset, amount)}
@@ -260,8 +258,10 @@ export const AssetPickerAmount = () => {
260258
alignItems={AlignItems.flexEnd}
261259
>
262260
<Text
263-
variant={TextVariant.bodyMd}
261+
variant={TextVariant.bodySm}
262+
color={TextColor.textAlternative}
264263
width={BlockSize.ThreeFourths}
264+
textAlign={TextAlign.End}
265265
ellipsis
266266
>
267267
ID: {`#${asset.details?.tokenId}`}

ui/components/multichain/asset-picker-amount/asset-picker/asset-picker.tsx

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,35 @@ import {
1818
TextVariant,
1919
} from '../../../../helpers/constants/design-system';
2020
import { AssetType } from '../../../../../shared/constants/transaction';
21-
import { getNativeCurrencyImage, getTokenList } from '../../../../selectors';
21+
import {
22+
getIpfsGateway,
23+
getNativeCurrencyImage,
24+
getTokenList,
25+
} from '../../../../selectors';
2226
import { getNativeCurrency } from '../../../../ducks/metamask/metamask';
2327
import { AssetPickerModal } from '../asset-picker-modal/asset-picker-modal';
28+
import { getAssetImageURL } from '../../../../helpers/utils/util';
2429

2530
// A component that lets the user pick from a list of assets.
2631
export default function AssetPicker({ asset }: { asset: Asset }) {
2732
const nativeCurrency = useSelector(getNativeCurrency);
2833
const nativeCurrencyImage = useSelector(getNativeCurrencyImage);
2934
const tokenList = useSelector(getTokenList);
35+
const ipfsGateway = useSelector(getIpfsGateway);
3036
const [showAssetPickerModal, setShowAssetPickerModal] = useState(false);
3137

32-
const image =
33-
asset.type === AssetType.native
34-
? nativeCurrencyImage
35-
: // eslint-disable-next-line @typescript-eslint/ban-ts-comment
36-
// @ts-ignore: type 'string' can't be used to index type '{}'
37-
tokenList?.[asset.details?.address?.toLowerCase()]?.iconUrl;
38+
let image: string | undefined;
39+
if (asset.type === AssetType.native) {
40+
image = nativeCurrencyImage;
41+
} else if (asset.type === AssetType.token) {
42+
image =
43+
asset.details?.image ??
44+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
45+
// @ts-ignore: type 'string' can't be used to index type '{}'
46+
tokenList?.[asset.details?.address?.toLowerCase()]?.iconUrl;
47+
} else if (asset.type === AssetType.NFT) {
48+
image = getAssetImageURL(asset.details?.image, ipfsGateway);
49+
}
3850

3951
// TODO: Handle long symbols in the UI
4052
const symbol =

0 commit comments

Comments
 (0)