feat(explorer-ui): formatAmount
#2956
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Formatter
User->>Formatter: Call formatAmount(amount, options)
Formatter->>Formatter: Validate input
Formatter->>Formatter: Check options for formatting
Formatter->>Formatter: Format amount based on options
Formatter-->>User: Return formatted amount
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
The formatAmount.ts file has been refactored to enhance functionality and flexibility, introducing a FormatOptions interface and leveraging Intl.NumberFormat for improved localization and precision.
- Introduced
FormatOptionsinterface inpackages/explorer-ui/utils/formatAmount.tsfor customizable formatting. - Replaced
numerallibrary withIntl.NumberFormatfor better localization support. - Added options like
fullAmount,standardDigits,useCompactNotation,compactDigits,minimumAmount, androundingMode. - Ensure thorough testing of edge cases and new options to avoid unexpected behavior.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Outside diff range, codebase verification and nitpick comments (2)
packages/explorer-ui/utils/formatAmount.ts (2)
29-34: Document default values for options.The default values for the options are set within the function. Consider documenting these defaults in the
FormatOptionsinterface or function documentation./** * Formats an amount based on the provided options. * * @param {string} amount - The amount to format. * @param {FormatOptions} [options] - The formatting options. * @returns {string} The formatted amount. */ export const formatAmount = ( amount: string, options?: FormatOptions ): string => { const fullAmount = options?.fullAmount ?? false const standardDigits = options?.standardDigits ?? 4 const useCompactNotation = options?.useCompactNotation ?? true const compactDigits = options?.compactDigits ?? (useCompactNotation ? 2 : 0) const minimumAmount = options?.minimumAmount ?? 0.0001
45-47: Clarify minimum amount handling.The function returns
< minimumAmountfor values below the minimum. Consider documenting this behavior.if (floatAmount < minimumAmount) { return `< ${minimumAmount}` }
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/explorer-ui/utils/formatAmount.ts (1 hunks)
Additional comments not posted (5)
packages/explorer-ui/utils/formatAmount.ts (5)
37-39: Ensure consistent zero-value return.The function returns '0.0' for zero values. Ensure consistency by returning '0.0' for empty strings as well.
57-61: Ensure fractional digits are configurable.The fractional digits for values less than 1 are hardcoded to
standardDigits. Ensure this is configurable.
63-67: Ensure significant digits are configurable.The significant digits for values less than 1000 are hardcoded to
standardDigits. Ensure this is configurable.
70-73: Ensure fractional digits are configurable.The fractional digits for values less than 1,000,000 are hardcoded to 0. Ensure this is configurable.
76-80: Ensure compact notation is configurable.The compact notation and fractional digits are configurable through
compactDigitsanduseCompactNotation. This is well-implemented.
| interface FormatOptions { | ||
| fullAmount?: boolean | ||
| standardDigits?: number | ||
| useCompactNotation?: boolean | ||
| compactDigits?: number | ||
| minimumAmount?: number | ||
| roundingMode?: string | ||
| } |
There was a problem hiding this comment.
Ensure roundingMode is validated.
The roundingMode property is a string, but its valid values are not documented or enforced. Consider using a union type or an enum to restrict possible values.
interface FormatOptions {
fullAmount?: boolean
standardDigits?: number
useCompactNotation?: boolean
compactDigits?: number
minimumAmount?: number
roundingMode?: 'ceil' | 'floor' | 'round'
}| export const formatAmount = ( | ||
| amount: string, | ||
| options?: FormatOptions | ||
| ): string => { |
There was a problem hiding this comment.
Consider validating the amount parameter.
The amount parameter is expected to be a string representing a number. Consider adding validation to ensure it is a valid numeric string before parsing.
export const formatAmount = (
amount: string,
options?: FormatOptions
): string => {
if (isNaN(parseFloat(amount))) {
throw new TypeError(`"${amount}" is not a valid numeric string`)
}| if (amount === '') { | ||
| return '' | ||
| } |
There was a problem hiding this comment.
Return '0.0' for empty string input.
Instead of returning an empty string for an empty amount, consider returning '0.0' for consistency with other zero-value returns.
if (amount === '') {
return '0.0'
}| const floatAmount = parseFloat(amount) | ||
|
|
There was a problem hiding this comment.
Avoid re-parsing the amount.
The floatAmount is parsed twice in the function. Consider parsing it once and reusing the value.
const floatAmount = parseFloat(amount)
if (!Number.isFinite(floatAmount)) {
throw new TypeError(`"${amount}" is not a finite number`)
}| } catch ({ name, message }) { | ||
| // console.error(name, message) | ||
| return amount | ||
| } |
There was a problem hiding this comment.
Remove try-catch block for finite check.
The try-catch block is unnecessary since the finite check can be done directly. Remove the try-catch block for simplicity.
if (!Number.isFinite(floatAmount)) {
throw new TypeError(`"${amount}" is not a finite number`)
}| const compactDigits = options?.compactDigits ?? (useCompactNotation ? 2 : 0) | ||
| const minimumAmount = options?.minimumAmount ?? 0.0001 | ||
|
|
||
| const locales = 'en-UK' |
There was a problem hiding this comment.
Consider making locales configurable.
The locales variable is hardcoded to 'en-UK'. Consider making it configurable through the FormatOptions interface.
const locales = options?.locales ?? 'en-UK'| if (fullAmount) { | ||
| return Intl.NumberFormat(locales).format(floatAmount) | ||
| } |
There was a problem hiding this comment.
Consider adding locale options for full amount.
When fullAmount is true, the locale should still be configurable.
if (fullAmount) {
return Intl.NumberFormat(locales).format(floatAmount)
}| const absAmount = Math.abs(floatAmount) | ||
|
|
||
| if (absAmount < 0.0001) { | ||
| return Intl.NumberFormat(locales, { | ||
| maximumSignificantDigits: 1, | ||
| }).format(floatAmount) | ||
| } |
There was a problem hiding this comment.
Ensure significant digits are configurable.
The significant digits for values less than 0.0001 are hardcoded to 1. Consider making this configurable.
if (absAmount < 0.0001) {
return Intl.NumberFormat(locales, {
maximumSignificantDigits: options?.significantDigits ?? 1,
}).format(floatAmount)
}
Deploying sanguine-fe with
|
| Latest commit: |
60a0491
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://34339dc2.sanguine-fe.pages.dev |
| Branch Preview URL: | https://explorer-format-amount.sanguine-fe.pages.dev |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2956 +/- ##
=============================================
Coverage 25.70725% 25.70725%
=============================================
Files 771 771
Lines 55603 55603
Branches 80 80
=============================================
Hits 14294 14294
Misses 39823 39823
Partials 1486 1486
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Bundle ReportChanges will increase total bundle size by 2.33MB ⬆️
|
…x has absolute zero value)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/explorer-ui/components/misc/IconAndAmount.tsx (2 hunks)
Additional comments not posted (2)
packages/explorer-ui/components/misc/IconAndAmount.tsx (2)
40-44: LGTM! But verify the correctness offormatAmountusage.The return statement correctly uses
displayAmountto enhance the user interface. Ensure that theformatAmountfunction handles all edge cases properly.
28-29: LGTM! But verify the correctness offormatAmountusage.The
displayAmountvariable correctly formats the amount for display purposes. Ensure that theformatAmountfunction handles all edge cases properly.
There was a problem hiding this comment.
PR Summary
(updates since last review)
Enhanced amount formatting and refactoring for better performance and maintainability.
- Refactor:
packages/explorer-ui/components/BridgeTransaction/BridgeTransactionTable.tsx- Reordered imports and commented out properties inIconAndAmountfor improved code organization. - Refactor:
packages/explorer-ui/components/ChainChart/index.tsx- Code formatting and minor refactoring for better readability. - Refactor:
packages/explorer-ui/components/Chart/index.tsx- Minor refactoring and improvements in code readability. - Refactor:
packages/explorer-ui/components/ContainerCard.tsx- Converted to arrow functions and updated imports for consistency. - Refactor:
packages/explorer-ui/components/CopyButtonIcon/index.tsx- Refactored to functional component with arrow functions and improved import statements.
109 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings
| @@ -35,11 +37,11 @@ export const IconAndAmount = ({ | |||
| className={`${iconSize} min-w-[1rem] min-h-[1rem] inline rounded-full`} | |||
| /> | |||
| <div | |||
| data-tooltip-content={amount} | |||
| data-tooltip-content={displayAmount} | |||
| data-tooltip-id="amount" | |||
| className="flex-1 pl-1 mr-1 text-white" | |||
There was a problem hiding this comment.
check: Ensure that the formatAmount function handles all edge cases, especially with large or small numbers.
There was a problem hiding this comment.
PR Summary
(updates since last review)
Enhanced amount formatting options and improved error handling for better user experience and performance.
- New Feature:
packages/explorer-ui/utils/formatAmount.ts- Added compact notation and rounding modes for amount formatting. - Bug Fix:
packages/explorer-ui/utils/formatAmount.ts- Improved handling of non-finite numbers and empty strings. - Refactor:
packages/explorer-ui/components/misc/IconAndAmount.tsx- IntroduceddisplayAmountvariable for clearer representation of small amounts. - Performance: Replaced external library with native locale-aware formatting in
packages/explorer-ui/utils/formatAmount.tsfor better performance.
1 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings
| @@ -25,6 +25,8 @@ export const IconAndAmount = ({ | |||
| amount = formattedValue / (dec / 10 ** 6) | |||
| } | |||
|
|
|||
| const displayAmount = amount ? formatAmount(amount) : '< 0.001' | |||
There was a problem hiding this comment.
logic: Consider caching the result of formatAmount to avoid repeated calculations.
| @@ -25,6 +25,8 @@ export const IconAndAmount = ({ | |||
| amount = formattedValue / (dec / 10 ** 6) | |||
| } | |||
|
|
|||
| const displayAmount = amount ? formatAmount(amount) : '< 0.001' | |||
There was a problem hiding this comment.
style: Ensure formatAmount handles all edge cases, especially with large or small numbers.
Summary by CodeRabbit
a7110bc: explorer-ui preview link
a609849: explorer-ui preview link
4e74e40: explorer-ui preview link