-
Notifications
You must be signed in to change notification settings - Fork 172
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
fix: custom token spacing, and overflow #559
Conversation
WalkthroughThe changes in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant API
User->>UI: Input token symbol
UI->>UI: Check symbol length
alt Length > 16
UI->>UI: Wrap symbol in <tooltip>
else Length <= 16
UI->>UI: Display symbol in <span>
end
UI->>API: Retrieve token data
API-->>UI: Return token data
UI-->>User: Display token information
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/extension/src/ui/action/views/network-assets/components/custom-evm-token.vue (1)
50-57
: LGTM! Consider extracting the magic numbers.The conditional rendering logic for token symbols is well-implemented. The tooltip usage provides good UX for long symbols.
Consider extracting the magic numbers (16 and 12) into named constants at the component level for better maintainability and consistency with other similar checks in the file:
<script setup lang="ts"> + const MAX_SYMBOL_LENGTH = 16; + const TRUNCATED_SYMBOL_LENGTH = 12; ... </script>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
packages/extension/src/ui/action/views/network-assets/components/custom-evm-token.vue
(3 hunks)
🔇 Additional comments (2)
packages/extension/src/ui/action/views/network-assets/components/custom-evm-token.vue (2)
432-436
: LGTM! CSS changes improve alignment and consistency.
The addition of display: flex
improves the alignment of token information, and the explicit font size ensures consistent typography.
289-289
: Verify content visibility after height reduction.
The height has been significantly reduced from 568px to 385px. While this might improve the UI compactness, we should ensure all content remains visible in different states (with/without warnings, errors, etc.).
✅ Verification successful
Let me gather more information about the component's structure and content to verify if the height reduction impacts content visibility.
Let me check more of the template content to see all the sections and their conditional rendering:
Based on the template content analysis, I can now make a determination about the height reduction's impact. The component has a clear structure with:
- Header section
- Contract input section
- Conditional token info section (with image and balance)
- Conditional warning section
- Conditional error section
- Button section
The height reduction from 568px to 385px appears safe because:
- The layout is vertically compact
- Sections are conditionally rendered (only one of token info + warning OR error message is shown at a time)
- No scrollable content areas are present
- No content overflow styles are defined
Height reduction appears safe - no content visibility issues.
The component's structure ensures that all content remains visible as sections are mutually exclusive and the layout is optimized for the reduced height.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other height-related styles or potential overflow issues
rg -g '*.vue' -g '*.less' "overflow|height|min-height" packages/extension/src/ui/action/
Length of output: 72868
Script:
#!/bin/bash
# Check the template structure and content of the custom-evm-token component
rg -A 20 "<template" packages/extension/src/ui/action/views/network-assets/components/custom-evm-token.vue
Length of output: 806
Script:
#!/bin/bash
# Get the full template content to analyze all sections
rg -A 100 "<template" packages/extension/src/ui/action/views/network-assets/components/custom-evm-token.vue
Length of output: 3639
Summary by CodeRabbit