-
Notifications
You must be signed in to change notification settings - Fork 196
fix: set max char width for account dropdown in receive #11076
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
Conversation
📝 WalkthroughWalkthroughResponsive truncation was added to two AccountDropdown components (title and subtitle) using breakpoint-based max lengths and the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Account Dropdown Component
participant BP as useBreakpointValue
participant MEMO as useMemo
participant UTIL as trimWithEndEllipsis
note right of UI `#DDEBF7`: Render phase
UI->>BP: request maxLength (base:25, md:50)
UI->>MEMO: compute truncatedText using UTIL and maxLength
MEMO->>UTIL: trimWithEndEllipsis(content, maxLength)
UTIL-->>MEMO: truncated or original content
MEMO-->>UI: memoized truncatedText
UI->>UI: render truncatedText in UI
note right of UTIL `#F6F8E8`: Boundary: return original when length <= maxLength
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-08-08T14:59:40.422ZApplied to files:
🧬 Code graph analysis (2)src/components/AccountDropdown/AccountSegement.tsx (2)
src/state/slices/portfolioSlice/utils/index.test.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
🧹 Nitpick comments (2)
src/components/AccountDropdown/AccountChildOption.tsx (1)
34-35: Consider adding a tooltip to show the full account name.While the truncation fixes the overflow issue, users have no way to see the full account name when it's truncated. Adding a
titleattribute or ChakraTooltipwould improve the UX for users with long account names.Example with title attribute:
- <RawText fontWeight='bold' whiteSpace='nowrap' flex={1}> + <RawText fontWeight='bold' whiteSpace='nowrap' flex={1} title={title}> {truncatedTitle} </RawText>src/components/AccountDropdown/AccountSegement.tsx (1)
30-33: Consider adding a tooltip to show the full subtitle.Similar to the title truncation in
AccountChildOption, users cannot see the full subtitle when truncated. Adding atitleattribute would improve UX for long addresses or balances.Example with title attribute:
{subtitle && ( - <RawText fontFamily='monospace' fontWeight='bold'> + <RawText fontFamily='monospace' fontWeight='bold' title={subtitle}> {truncatedSubtitle} </RawText> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/components/AccountDropdown/AccountChildOption.tsx(2 hunks)src/components/AccountDropdown/AccountSegement.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/components/AccountDropdown/AccountChildOption.tsxsrc/components/AccountDropdown/AccountSegement.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/components/AccountDropdown/AccountChildOption.tsxsrc/components/AccountDropdown/AccountSegement.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/AccountDropdown/AccountChildOption.tsxsrc/components/AccountDropdown/AccountSegement.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/components/AccountDropdown/AccountChildOption.tsxsrc/components/AccountDropdown/AccountSegement.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/components/AccountDropdown/AccountChildOption.tsxsrc/components/AccountDropdown/AccountSegement.tsx
🧠 Learnings (10)
📓 Common learnings
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10418
File: src/plugins/walletConnectToDapps/components/header/WalletConnectToDappsHeaderButton.tsx:0-0
Timestamp: 2025-09-08T22:00:48.005Z
Learning: gomesalexandre dismissed an aria-label accessibility suggestion with "meh" in PR #10418 for WalletConnectToDappsHeaderButton.tsx, consistent with the team's pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10361
File: src/pages/Markets/components/CardWithSparkline.tsx:83-92
Timestamp: 2025-08-25T23:32:13.876Z
Learning: In shapeshift/web PR #10361, premiumjibles considered the nested button accessibility issue (ChartErrorFallback retry Button inside Card rendered as Button in CardWithSparkline.tsx) out of scope for the error boundaries feature PR, consistent with deferring minor a11y improvements to follow-up PRs.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:21-24
Timestamp: 2025-09-12T13:16:27.004Z
Learning: gomesalexandre declined to add error boundaries to WalletConnect modals in PR #10461, stating "no error boundaries in this pr ser", consistent with his preference to keep PR scope focused and defer tangential improvements to separate efforts.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10231
File: src/components/AssetSearch/components/AssetList.tsx:2-2
Timestamp: 2025-08-08T15:00:49.887Z
Learning: Project shapeshift/web: NeOMakinG prefers avoiding minor a11y/UI nitpicks (e.g., adding aria-hidden to decorative icons in empty states like src/components/AssetSearch/components/AssetList.tsx) within feature PRs; defer such suggestions to a follow-up instead of blocking the PR.
Learnt from: 0xApotheosis
Repo: shapeshift/web PR: 10691
File: src/components/AssetHeader/AssetHeader.tsx:98-106
Timestamp: 2025-09-30T05:28:39.685Z
Learning: In the ShapeShift web codebase, the "More" menu on the asset page (containing watch/unwatch, view on explorer, and mark as spam actions) should always be visible on both mobile and desktop, regardless of whether a wallet is connected or whether the wallet supports the asset's chain. Individual actions within the menu can handle their own disabled states, but the menu itself should not be gated behind wallet connection checks.
📚 Learning: 2025-08-08T14:59:40.422Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10231
File: src/pages/Explore/ExploreCategory.tsx:231-238
Timestamp: 2025-08-08T14:59:40.422Z
Learning: In src/pages/Explore/ExploreCategory.tsx, for the PageHeader filter trigger, NeOMakinG considers changing a clickable Chakra Icon to IconButton too nitpicky for this PR and prefers to keep the current Icon-based trigger; such minor a11y/UI nitpicks should be deferred to a follow-up if needed.
Applied to files:
src/components/AccountDropdown/AccountChildOption.tsxsrc/components/AccountDropdown/AccountSegement.tsx
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : BREAK DOWN large components into smaller, reusable pieces
Applied to files:
src/components/AccountDropdown/AccountChildOption.tsxsrc/components/AccountDropdown/AccountSegement.tsx
📚 Learning: 2025-08-22T12:59:01.210Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx:26-29
Timestamp: 2025-08-22T12:59:01.210Z
Learning: In src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx, NeOMakinG declined wrapping the RewardDistributionActionCard component with React.memo, saying it was "too much", suggesting that like other action center components, memoization is not beneficial for this specific use case.
Applied to files:
src/components/AccountDropdown/AccountChildOption.tsx
📚 Learning: 2025-10-07T03:44:27.350Z
Learnt from: 0xApotheosis
Repo: shapeshift/web PR: 10760
File: src/components/ManageHiddenAssets/ManageHiddenAssetsList.tsx:78-84
Timestamp: 2025-10-07T03:44:27.350Z
Learning: In the ShapeShift web codebase, the following are stable references and do not need to be included in useCallback/useMemo dependency arrays:
- `navigate` from `useBrowserRouter()` hook
- Modal control objects (like `walletDrawer`) from `useModal()` hook (including their `isOpen`, `close`, and `open` methods)
- These are backed by stable context providers
Applied to files:
src/components/AccountDropdown/AccountChildOption.tsx
📚 Learning: 2025-09-16T09:32:21.333Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10490
File: src/components/Layout/Header/NavBar/NavigationDropdown.tsx:96-103
Timestamp: 2025-09-16T09:32:21.333Z
Learning: In the shapeshift/web codebase, gomesalexandre confirms that using `item.icon && <Icon as={item.icon} boxSize={4} />` for Chakra UI MenuItem.icon prop is valid and acceptable - the boolean short-circuit pattern works fine with Chakra components and doesn't need to be changed to explicit undefined.
Applied to files:
src/components/AccountDropdown/AccountChildOption.tsx
📚 Learning: 2025-08-27T09:47:59.394Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Modal/components/DialogCloseButton.tsx:29-37
Timestamp: 2025-08-27T09:47:59.394Z
Learning: In shapeshift/web, for DialogCloseButton.tsx, the onClick handler must remain on Drawer.Close (not moved to IconButton with asChild) because stopPropagation() needs to be called at the Drawer.Close level to properly handle drawer closing behavior; moving event handling to the child IconButton would break the event flow that Drawer.Close expects.
Applied to files:
src/components/AccountDropdown/AccountChildOption.tsx
📚 Learning: 2025-07-29T15:04:28.083Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10139
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx:109-115
Timestamp: 2025-07-29T15:04:28.083Z
Learning: In src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx, the component is used under an umbrella that 100% of the time contains the quote, making the type assertion `activeTradeQuote?.steps[currentHopIndex] as TradeQuoteStep` safe. Adding conditional returns before hooks would violate React's Rules of Hooks.
Applied to files:
src/components/AccountDropdown/AccountChildOption.tsx
📚 Learning: 2025-07-29T10:27:23.424Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10128
File: .cursor/rules/react-best-practices.mdc:8-14
Timestamp: 2025-07-29T10:27:23.424Z
Learning: The ShapeShift team practices aggressive memoization in React components as documented in .cursor/rules/react-best-practices.mdc. They use useMemo for all transformations, derived values, and conditional values, and useCallback for all event handlers and functions that could be passed as props. This approach was adopted after experiencing performance issues ("had hard time") and is their current established practice, though they acknowledge it may evolve in the future.
Applied to files:
src/components/AccountDropdown/AccountChildOption.tsx
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : KEEP component files under 200 lines when possible
Applied to files:
src/components/AccountDropdown/AccountSegement.tsx
🧬 Code graph analysis (2)
src/components/AccountDropdown/AccountChildOption.tsx (2)
src/lib/utils/index.ts (1)
trimWithEndEllipsis(22-30)src/components/Text/Text.tsx (1)
RawText(15-17)
src/components/AccountDropdown/AccountSegement.tsx (2)
src/lib/utils/index.ts (1)
trimWithEndEllipsis(22-30)src/components/Text/Text.tsx (1)
RawText(15-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install and Cache
🔇 Additional comments (2)
src/components/AccountDropdown/AccountChildOption.tsx (1)
28-29: LGTM! Responsive truncation correctly implemented.The responsive character limits (25 for mobile, 50 for desktop) correctly address the mobile overflow issue. The
useMemodependencies are correct, and the implementation gracefully handles the initialundefinedfromuseBreakpointValueviatrimWithEndEllipsis.src/components/AccountDropdown/AccountSegement.tsx (1)
14-18: LGTM! Responsive subtitle truncation correctly implemented.The subtitle truncation uses the same responsive breakpoints (25/50 chars) as
AccountChildOption, ensuring consistency. TheuseMemodependencies are correct, and the implementation handlesundefinedmaxLength gracefully.
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.
3c0d447 to
f614186
Compare
|
@0xApotheosis should be sorted now |
NeOMakinG
left a comment
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.
For the sake of progression!



Description
Stop the account dropdown from getting giant on the receive modal
Had a go at solving this nicely with dynamic CSS and overflow ellipsis but too painful trying to get the width calcs to work properly. Went with a good enough solution of max char limits. 25 for mobile and 50 for desktop
Issue (if applicable)
closes #11055
Risk
Low risk, just sets character limits on the dropdown
Testing
Engineering
Operations
Screenshots (if applicable)
Summary by CodeRabbit