Skip to content

fix: NFT Grid tooltip #31625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Apr 15, 2025
Merged

fix: NFT Grid tooltip #31625

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
56f43e1
chore: Init tooltip
gambinish Apr 4, 2025
353f8ff
fix: Add tooltip to truncated NFT grid item titles
gambinish Apr 4, 2025
ec0c2c8
fix: Lint
gambinish Apr 4, 2025
1630cb2
fix: Update snapshot
gambinish Apr 4, 2025
1eb2b30
chore: Update snapshots
gambinish Apr 4, 2025
a464167
fix: Empty whitespace
gambinish Apr 4, 2025
93facbd
chore: Update snapshots
gambinish Apr 4, 2025
0c55317
chore: Update snapshots
gambinish Apr 4, 2025
1155f70
Merge branch 'main' into fix/nft-grid-tooltip
gambinish Apr 7, 2025
b7a8af2
chore: Merge main
gambinish Apr 11, 2025
94b833f
chore: Update snapshots
gambinish Apr 11, 2025
4e2610b
Merge branch 'main' into fix/nft-grid-tooltip
gambinish Apr 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions ui/components/app/assets/nfts/nft-details/__snapshots__/nft-details.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,6 @@ exports[`NFT Details should match minimal props and state snapshot 1`] = `
</div>
</div>
</button>
<p
class="mm-box mm-text mm-text--body-sm mm-text--ellipsis mm-box--color-text-default"
/>
<p
class="mm-box mm-text mm-text--body-sm mm-text--ellipsis mm-box--color-text-alternative"
/>
</div>
</div>
</div>
Expand Down
6 changes: 0 additions & 6 deletions ui/components/app/assets/nfts/nft-details/__snapshots__/nft-full-image.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,6 @@ exports[`NFT full image should match snapshot 1`] = `
</div>
</div>
</button>
<p
class="mm-box mm-text mm-text--body-sm mm-text--ellipsis mm-box--color-text-default"
/>
<p
class="mm-box mm-text mm-text--body-sm mm-text--ellipsis mm-box--color-text-alternative"
/>
</div>
</div>
</div>
Expand Down
51 changes: 41 additions & 10 deletions ui/components/multichain/nft-item/nft-item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ import {
getTestNetworkBackgroundColor,
} from '../../../selectors';
import { NFT } from '../asset-picker-amount/asset-picker-modal/types';
import Tooltip from '../../ui/tooltip/tooltip';
import { ENVIRONMENT_TYPE_FULLSCREEN } from '../../../../shared/constants/app';
// eslint-disable-next-line import/no-restricted-paths
import { getEnvironmentType } from '../../../../app/scripts/lib/util';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm this rule is meant to prevent the UI from requesting stuff from the background/scripts.
Is there another getEnvironmentType or utility we can use here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, this is the only utility. We're using it in a few places already. Do you know of another one we can use instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aw damn. No I don't know of another one we can use.

Thats okay then 😄

Copy link
Contributor

@Prithpal-Sooriya Prithpal-Sooriya Apr 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the implementation, I'm not sure why this can only be used in the background.

metamask-extension/app/scripts/lib/util.ts

Lines 55 to 56 in a4179bc

const getEnvironmentType = (url = window.location.href) =>
getEnvironmentTypeMemo(url);

I think it can be moved to the shared folder in the future.


type NftItemProps = {
nft?: NFT;
Expand Down Expand Up @@ -103,6 +107,15 @@ export const NftItem = ({
/>
);

const environmentType = getEnvironmentType();
const maxStrLen = environmentType === ENVIRONMENT_TYPE_FULLSCREEN ? 40 : 20;

const isLongName = nft?.name && nft.name.length > maxStrLen;
const isLongCollection =
nft?.collection?.name &&
typeof nft?.collection?.name === 'string' &&
nft.collection.name.length > maxStrLen;

return (
<Box className="nft-item__card">
<Box
Expand Down Expand Up @@ -135,16 +148,34 @@ export const NftItem = ({
{nftImageComponentToRender}
</BadgeWrapper>
</Box>
<Text variant={TextVariant.bodySm} color={TextColor.textDefault} ellipsis>
{nft?.name}
</Text>
<Text
variant={TextVariant.bodySm}
color={TextColor.textAlternative}
ellipsis
>
{nft?.collection?.name}
</Text>
{nft && (
<Tooltip
position="bottom"
html={
<>
<span>{nft?.name}</span>
<br />
<span>{nft.collection?.name}</span>
</>
}
disabled={!isLongName && !isLongCollection}
>
<Text
variant={TextVariant.bodySm}
color={TextColor.textDefault}
ellipsis
>
{nft?.name}
</Text>
<Text
variant={TextVariant.bodySm}
color={TextColor.textAlternative}
ellipsis
>
{nft?.collection?.name}
</Text>
</Tooltip>
)}
</Box>
);
};
6 changes: 0 additions & 6 deletions ...components/confirm/info/nft-token-transfer/__snapshots__/nft-token-transfer.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@ exports[`NFTTokenTransferInfo renders correctly 1`] = `
</div>
</div>
</button>
<p
class="mm-box mm-text mm-text--body-sm mm-text--ellipsis mm-box--color-text-default"
/>
<p
class="mm-box mm-text mm-text--body-sm mm-text--ellipsis mm-box--color-text-alternative"
/>
</div>
</div>
<h2
Expand Down
6 changes: 0 additions & 6 deletions ...ponents/confirm/info/shared/nft-send-heading/__snapshots__/nft-send-heading.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@ exports[`<NFTSendHeading /> renders component 1`] = `
</div>
</div>
</button>
<p
class="mm-box mm-text mm-text--body-sm mm-text--ellipsis mm-box--color-text-default"
/>
<p
class="mm-box mm-text mm-text--body-sm mm-text--ellipsis mm-box--color-text-alternative"
/>
</div>
</div>
<h2
Expand Down
Loading