fix: long token names breaking ui #589
Conversation
WalkthroughThe pull request introduces a consistent text truncation mechanism across multiple Vue components by implementing the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
💼 Build Files |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/extension/src/ui/action/views/swap/components/swap-token-select/index.vue (1)
7-13: Consider centralizing the truncation length in a single constant.This logic effectively prevents layout disruption and improves readability. However, the hardcoded truncation length (25) might be more maintainable and consistent if declared as a constant or extracted to a shared utility. This helps ensure uniform updates across multiple components if the truncation length changes.
packages/extension/src/ui/action/views/assets-select-list/components/assets-select-list-item.vue (1)
8-14: Avoid duplication by reusing the truncation approach across components.Similar to the other component, this logic works as intended, but storing the truncation length in a shared constant or helper function can enhance maintainability and consistency in the codebase. Consider extracting this logic to promote DRY (Don’t Repeat Yourself) principles.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/extension/src/ui/action/views/assets-select-list/components/assets-select-list-item.vue(1 hunks)packages/extension/src/ui/action/views/swap/components/swap-token-select/index.vue(1 hunks)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/extension/src/ui/action/utils/filters.ts (1)
21-25: Consider handling multibyte characters.This logic works for simple strings, but multi-byte characters might be split incorrectly. If your UI must handle emojis or non-Latin scripts, you may want to use methods that account for Unicode grapheme clusters, such as using
Intl.Segmenter.packages/extension/src/providers/common/ui/send-transaction/send-nft-select.vue (1)
12-12: Apply same logic for shorter collection names.For clarity, consider whether shorter collection names (less than 50 characters) should be displayed without the ellipsis. The current filter logic is correct, but be mindful of the user experience.
packages/extension/src/providers/common/ui/send-transaction/nft-select-list/components/nft-select-list-item.vue (1)
8-8: Consider documenting the truncation length choices.While the truncation lengths (25 for name, 50 for collection name) seem appropriate, it would be helpful to document why these specific lengths were chosen, perhaps as a comment or in component documentation.
Add a comment above the template section explaining the truncation lengths:
<template> + <!-- + NFT item display with truncated text: + - Name: 25 chars (compact display for main identifier) + - Collection: 50 chars (allows for longer collection names) + --> <a class="nft-select-list__token" @click="$emit('selectNft', item)">Also applies to: 11-11
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/extension/src/providers/common/ui/send-transaction/nft-select-list/components/nft-select-list-item.vue(1 hunks)packages/extension/src/providers/common/ui/send-transaction/send-nft-select.vue(1 hunks)packages/extension/src/ui/action/types/filters.ts(2 hunks)packages/extension/src/ui/action/utils/filters.ts(1 hunks)packages/extension/src/ui/action/views/assets-select-list/components/assets-select-list-item.vue(1 hunks)packages/extension/src/ui/action/views/network-activity/components/network-activity-transaction.vue(1 hunks)packages/extension/src/ui/action/views/swap/components/swap-token-select/index.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/extension/src/ui/action/views/assets-select-list/components/assets-select-list-item.vue
- packages/extension/src/ui/action/views/swap/components/swap-token-select/index.vue
🔇 Additional comments (5)
packages/extension/src/ui/action/types/filters.ts (2)
7-7: Great addition of thetruncatemethod to$filters.Exporting
truncatewithin$filterskeeps the code consistent with other utilities likeformatDurationandreplaceWithEllipsis.
16-16: Ensure full coverage in unit tests.If there are tests for
$filters, consider adding test cases fortruncateto verify various input lengths, including zero-length, longer strings, non-ASCII characters, etc.packages/extension/src/providers/common/ui/send-transaction/send-nft-select.vue (1)
9-9: Consistent usage of$filters.truncate.This approach aligns well with the rest of the codebase. Ensure you verify that all references to
item.nameare truncated the same way for UI consistency.packages/extension/src/ui/action/views/network-activity/components/network-activity-transaction.vue (1)
114-114: Truncation for the token symbol is properly handled.Truncating the symbol avoids potential UI overflow. Confirm any swap providers or contract addresses that might produce extremely long tokens are properly truncated as well.
packages/extension/src/providers/common/ui/send-transaction/nft-select-list/components/nft-select-list-item.vue (1)
8-8: Implementation looks good, verify filter registration.The use of
$filters.truncateis a good approach for consistent text truncation across components. The different truncation lengths (25 for name, 50 for collection name) make sense given their UI context.Let's verify the filter registration and consistent usage across components:
Also applies to: 11-11
Summary by CodeRabbit