Skip to content

fix: wrap long asset names #31657

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.

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 2 commits into from
Apr 7, 2025
Merged

Conversation

bergarces
Copy link
Contributor

@bergarces bergarces commented Apr 7, 2025

Description

Open in GitHub Codespaces

Very long asset titles overflow is not working, and it is hiding the token value.

Related issues

Fixes:

Manual testing steps

Option 1:

  1. Open an account with a token with a very long name (like the one in the description)
  2. Token name should not push token value out of bounds, but cut the overflow with three dots

Option 2:

  1. Open an account with any token and expand view
  2. Open chrome dev tools (F12) so that they open on the right hand side
  3. Make chrome dev tool window large enough so that the token name cuts the overflow with three dots

Screenshots/Recordings

Before

image

After

image

Pre-merge author checklist

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.

Copy link
Contributor

github-actions bot commented Apr 7, 2025

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the team-mmi PRs from the MMI team label Apr 7, 2025
@bergarces bergarces added team-assets and removed team-mmi PRs from the MMI team labels Apr 7, 2025
@@ -27,7 +27,7 @@ export const TokenCellTitle = React.memo(
<Tooltip
position="bottom"
html={token.title}
tooltipInnerClassName="multichain-token-list-item__tooltip"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The class multichain-token-list-item__tooltip does not seem to exist.
Also, tooltipInnerClassName does not look like a property of Tooltip.

Copy link
Contributor

Choose a reason for hiding this comment

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

nice find!

white-space: nowrap;
overflow: hidden;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as mm-text--ellipsis.

@sahar-fehri
Copy link
Contributor

Nice fix! On the UI it looks like if we shorten it a bit more it would look better, the dots seem to be too close to the value

@metamaskbot
Copy link
Collaborator

Builds ready [e7f4699]
UI Startup Metrics (1251 ± 67 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1251111014876712881382
load10959571289651156995
domContentLoaded10889451277651158991
domInteractive17136571630
firstPaint7791381286421214963
backgroundConnect117284910
firstReactRender20157172034
getState13539879
initialActions001001
loadScripts834692101464867953
setupStore8527378
WebpackHomeuiStartup21981776271717723012562
load17171281216115617801995
domContentLoaded17111276215315417751980
domInteractive181366131455
firstPaint163664186627293
backgroundConnect3210256273555
firstReactRender185553791085773
getState1844244569
initialActions317145
loadScripts17021269213115117671955
setupStore25740650328
FirefoxBrowserifyHomeuiStartup14071205190015214651780
load12581068174014613261596
domContentLoaded12581068174014613241596
domInteractive10241191269096
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect26176992746
firstReactRender24195652433
getState9445789
initialActions001001
loadScripts12341048172314512991570
setupStore7440567
WebpackHomeuiStartup15491341193114316311836
load13361162173813314171592
domContentLoaded13361161173713314171592
domInteractive9937228289097
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect25184662641
firstReactRender36285364049
getState1442863578
initialActions001011
loadScripts13141143171713213951570
setupStore9538688
Bundle size diffs
  • background: 0 Bytes (0%)
  • ui: -14 Bytes (0%)
  • common: 0 Bytes (0%)

@bergarces bergarces added this pull request to the merge queue Apr 7, 2025
Merged via the queue into main with commit 39dffea Apr 7, 2025
164 checks passed
@bergarces bergarces deleted the fix/MMASSETS-697-wrap-long-asset-names branch April 7, 2025 15:32
@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2025
@metamaskbot metamaskbot added the release-12.17.0 Issue or pull request that will be included in release 12.17.0 label Apr 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.17.0 Issue or pull request that will be included in release 12.17.0 team-assets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants