-
Notifications
You must be signed in to change notification settings - Fork 197
fix: limit over table overflow on large values #11077
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
📝 WalkthroughWalkthroughThe LimitOrderCard component introduces adaptive decimal precision calculation that automatically adjusts displayed decimal places based on value magnitude. Static Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
e41ee3e to
87e950c
Compare
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 (1)
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx (1)
31-40: Adaptive precision logic is well-implemented.The graduated threshold approach correctly reduces decimal places as magnitude increases, addressing the mobile overflow issue. The function properly handles edge cases using
bnOrZeroandabs().Consider these optional improvements:
- Extract to shared utility if this pattern will be reused elsewhere:
// src/lib/utils/precision.ts /** * Calculates adaptive decimal precision based on value magnitude. * Larger values receive fewer decimal places to optimize horizontal space. * * @param value - Numeric value (string or number) * @returns Decimal precision (2-6 digits) * * @example * getAdaptivePrecision(10000) // 2 * getAdaptivePrecision(1000) // 3 * getAdaptivePrecision(10) // 5 */ export const getAdaptivePrecision = (value: string | number): number => { const absValue = bnOrZero(value).abs() if (absValue.gte(10000)) return 2 if (absValue.gte(1000)) return 3 if (absValue.gte(100)) return 4 if (absValue.gte(10)) return 5 return 6 }
- Add inline JSDoc if keeping the function local to document the threshold rationale.
📜 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 (1)
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx(5 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/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.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/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.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/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.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/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx
🧠 Learnings (6)
📓 Common learnings
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: 10871
File: src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx:426-428
Timestamp: 2025-10-21T17:11:18.087Z
Learning: In src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx, within the handleInputChange function, use .toFixed() without arguments (not .toString()) when converting BigNumber amounts for input field synchronization. This avoids exponential notation in the input while preserving precision for presentational components like <Amount.Crypto /> and <Amount.Fiat /> to format appropriately.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10191
File: src/pages/Explore/Explore.tsx:56-56
Timestamp: 2025-08-05T16:39:58.598Z
Learning: In the ShapeShift web codebase, the established pattern for handling floating point numbers is to use BigNumber operations (bnOrZero, bn) for calculations and convert to strings using .toString() before passing to UI components like Amount.Fiat, Amount.Crypto, and Amount.Percent. This prevents JavaScript floating point precision issues and maintains consistency across the application.
📚 Learning: 2025-10-21T17:11:18.087Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10871
File: src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx:426-428
Timestamp: 2025-10-21T17:11:18.087Z
Learning: In src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx, within the handleInputChange function, use .toFixed() without arguments (not .toString()) when converting BigNumber amounts for input field synchronization. This avoids exponential notation in the input while preserving precision for presentational components like <Amount.Crypto /> and <Amount.Fiat /> to format appropriately.
Applied to files:
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx
📚 Learning: 2025-11-03T22:31:30.786Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10985
File: packages/swapper/src/swappers/PortalsSwapper/getPortalsTradeQuote/getPortalsTradeQuote.ts:0-0
Timestamp: 2025-11-03T22:31:30.786Z
Learning: In packages/swapper/src/swappers/PortalsSwapper, the rate and quote files intentionally use different approaches for calculating buyAmountBeforeSlippageCryptoBaseUnit: getPortalsTradeRate.tsx uses minOutputAmount / (1 - buffer) for conservative estimates, while getPortalsTradeQuote.ts uses outputAmount / (1 - buffer) for final quote display. This difference is validated by on-chain simulation testing and is intentional.
Applied to files:
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.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/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx
📚 Learning: 2025-08-05T16:39:58.598Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10191
File: src/pages/Explore/Explore.tsx:56-56
Timestamp: 2025-08-05T16:39:58.598Z
Learning: In the ShapeShift web codebase, the established pattern for handling floating point numbers is to use BigNumber operations (bnOrZero, bn) for calculations and convert to strings using .toString() before passing to UI components like Amount.Fiat, Amount.Crypto, and Amount.Percent. This prevents JavaScript floating point precision issues and maintains consistency across the application.
Applied to files:
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx
📚 Learning: 2025-10-21T17:12:47.282Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10871
File: src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx:426-428
Timestamp: 2025-10-21T17:12:47.282Z
Learning: In the ShapeShift web codebase, when converting bignumber.js BigNumber values, prefer using .toFixed() over .toString() unless there's a specific reason to use .toString(). The .toFixed() method avoids exponential notation while preserving precision, which is especially important for input field synchronization and display purposes.
Applied to files:
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx
🧬 Code graph analysis (1)
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx (2)
src/components/HoverTooltip/HoverTooltip.tsx (1)
HoverTooltip(11-39)src/components/Amount/Amount.tsx (1)
Amount(21-42)
⏰ 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: Call / Static
🔇 Additional comments (3)
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx (3)
216-231: LGTM! Precision calculations are correctly memoized.Each precision value is properly derived using
useMemowith accurate dependencies, ensuring efficient recalculation only when the underlying values change.
315-315: Adaptive precision correctly applied to all amount displays.The
maximumFractionDigitsupdates properly use the computed precision values. TheHoverTooltipwrappers preserve accessibility by allowing users to view full precision values on hover/touch.Also applies to: 323-323, 338-338, 348-348
31-40: Implementation verified—code changes are complete and correct.The adaptive precision function is properly defined and consistently applied to all four amount displays (sell amount, buy amount, limit price, execution price). Each uses
maximumFractionDigitsto reduce horizontal space for large values whileHoverTooltippreserves full precision on interaction. The logic correctly reduces decimals for values >= 10 (2-5 decimals) while maintaining 6 decimals for smaller values.To confirm the fix resolves issue #11054 (cancel button accessibility on iPhone 14), manual testing on target devices is required—verify the cancel button is fully visible without horizontal scrolling when viewing orders with large values.
0xApotheosis
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.
Works as promised!
Description
Prevent limit order table from overflowing on mobile when we have values with a high magnitude like 1002.234234.
This just adds an adaptive precision calc so it gets less precise as the magnitude grows
Issue (if applicable)
closes #11054
Risk
Low risk, will muck with precision on the limit table
Testing
Engineering
Operations
Screenshots (if applicable)
Summary by CodeRabbit