feat(PE-7854): New buy name checkout #731
Conversation
WalkthroughThe changes introduce new dependencies, UI components, custom hooks, and styling updates. A new checkout route has been added, along with components for registration checkout, tooltips, domain checkout display, and payment options. New hooks manage token balances, cost details, and countdown functionality, while an older balance hook is removed. Additionally, minor adjustments to component rendering and CSS styling are implemented, and the Tailwind color palette is extended. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant R as Router
participant C as Checkout Component
participant DCC as DomainCheckoutCard
participant POF as PaymentOptionsForm
participant HB as Balance Hooks
U->>R: Navigate to /register/:name/checkout
R->>C: Render Checkout component
C->>DCC: Display domain checkout details
C->>POF: Render payment options form
POF->>HB: Fetch token balance data
HB-->>POF: Return balance information
U->>C: Trigger payment process
C-->>U: Process payment and update status
Poem
✨ Finishing Touches
🪧 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (12)
src/App.tsx (1)
318-329: Added new checkout route for domain registrationThe new route properly follows the application's established routing pattern and correctly implements the Suspense fallback with PageLoader. This route enables the new domain-specific checkout flow.
Consider adding breadcrumb navigation support to this route, similar to other routes in the application, to maintain navigation consistency.
<Route path="register/:name/checkout" element={ <Suspense fallback={ <PageLoader loading={true} message={'Loading, please wait'} /> } > <Checkout /> </Suspense> } + handle={{ + crumbs: (data: string) => [ + { name: 'Register', route: '/' }, + { name: data, route: `/register/${data}` }, + { name: 'Checkout', route: `/register/${data}/checkout` }, + ], + }} />src/components/Tooltips/ANTDetailsTip.tsx (2)
1-8: Consider adding component documentationAdding JSDoc comments would help explain the purpose of this component to other developers.
+/** + * A tooltip component that displays ANT (Arweave Name Token) details including process ID, target ID, and owner. + * Validates and formats Arweave transaction IDs, providing clickable links to explore them. + */ function ANTDetailsTip({
30-71: Consider extracting repeated patterns into a reusable componentThe tooltip contains three nearly identical sections with only minor differences. Consider extracting this pattern into a reusable component to reduce duplication.
+// Add to the top of the file +function DetailRow({ label, id, type }: { label: string; id?: string; type?: ArweaveIdTypes }) { + return ( + <span className="flex gap-2 text-sm text-grey py-4 border-b-[1px] border-dark-grey whitespace-nowrap gap-2"> + {label}:{' '} + {isArweaveTransactionID(id) ? ( + <ArweaveID + id={new ArweaveTransactionID(id)} + shouldLink={true} + characterCount={16} + type={type} + /> + ) : ( + type === ArweaveIdTypes.ADDRESS ? formatForMaxCharCount(id ?? 'N/A', 16) : 'N/A' + )} + </span> + ); +} // Then in the component -<div className="flex flex-col p-2"> - <span className="flex gap-2 text-sm text-grey py-4 border-b-[1px] border-dark-grey whitespace-nowrap gap-2"> - Process ID:{' '} - {isArweaveTransactionID(antId) ? ( - <ArweaveID - id={new ArweaveTransactionID(antId)} - shouldLink={true} - characterCount={16} - type={ArweaveIdTypes.CONTRACT} - /> - ) : ( - 'N/A' - )} - </span> - - <span className="flex gap-2 text-sm text-grey py-4 border-b-[1px] border-dark-grey whitespace-nowrap"> - Target ID:{' '} - {isArweaveTransactionID(targetId) ? ( - <ArweaveID - id={new ArweaveTransactionID(targetId)} - shouldLink={true} - characterCount={16} - type={ArweaveIdTypes.TRANSACTION} - /> - ) : ( - 'N/A' - )} - </span> - <span className="flex gap-2 text-sm text-grey py-4 border-b-[1px] border-dark-grey whitespace-nowrap"> - Owner:{' '} - {isArweaveTransactionID(owner) ? ( - <ArweaveID - id={new ArweaveTransactionID(owner)} - shouldLink={true} - characterCount={16} - type={ArweaveIdTypes.ADDRESS} - /> - ) : ( - formatForMaxCharCount(owner ?? 'N/A', 16) - )} - </span> +<div className="flex flex-col p-2"> + <DetailRow label="Process ID" id={antId} type={ArweaveIdTypes.CONTRACT} /> + <DetailRow label="Target ID" id={targetId} type={ArweaveIdTypes.TRANSACTION} /> + <DetailRow label="Owner" id={owner} type={ArweaveIdTypes.ADDRESS} /> </div>src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (3)
40-64: Ensure proper calculation of token balancesThe balance calculations use
useMemocorrectly, butselectedCryptoBalanceshould also directly depend on the individual balance variables for better consistency.const selectedCryptoBalance = useMemo(() => { if (selectedCrypto === '$ARIO') { - return allArIOBalance; + return fundingSource === 'balance' + ? liquidArIOBalance + : fundingSource === 'stakes' + ? stakedAndVaultedArIOBalance + : allArIOBalance; } return 0; -}, [selectedCrypto, allArIOBalance]); +}, [selectedCrypto, allArIOBalance, liquidArIOBalance, stakedAndVaultedArIOBalance, fundingSource]);
110-111: Enhance "Coming Soon" messagingThe "Coming Soon!" messages for credit card and credits payment methods could provide more context to users about when these features might be available or offer an alternative payment method.
- <span className="text-grey flex m-auto">Coming Soon!</span> + <div className="flex flex-col items-center justify-center gap-2 m-auto text-center"> + <span className="text-grey">Coming Soon!</span> + <span className="text-xs text-grey/70">This payment method will be available in a future update.</span> + <span className="text-xs text-grey/70">Please use the Crypto option for now.</span> + </div>Also applies to: 204-205
37-39: Add loading states for balance fetchingThe component could benefit from showing loading states when fetching balances to provide better user feedback.
- const { data: liquidBalance } = useArIOLiquidBalance(); - const { data: stakedAndVaultedBalance } = useArIOStakedAndVaultedBalance(); + const { data: liquidBalance, isLoading: isLiquidLoading } = useArIOLiquidBalance(); + const { + data: stakedAndVaultedBalance, + isLoading: isStakedLoading + } = useArIOStakedAndVaultedBalance(); + const isBalanceLoading = isLiquidLoading || isStakedLoading; // Then in the balance display section: <span className="text-white text-2xl font-bold"> - {formatARIOWithCommas(selectedCryptoBalance)} {selectedCrypto} + {isBalanceLoading + ? <span className="text-grey animate-pulse">Loading...</span> + : `${formatARIOWithCommas(selectedCryptoBalance)} ${selectedCrypto}` + } </span>src/hooks/useCountdown.ts (2)
15-57: Add protection against invalid timestampsThe hook should handle cases where the provided timestamp is invalid.
useEffect(() => { + // Return early if timestamp is invalid + if (!endTimestamp || isNaN(endTimestamp) || endTimestamp <= 0) { + setCountdownString(null); + return; + } const interval = setInterval(() => {
36-53: Extract formatting logic into a separate functionThe countdown formatting logic is complex and would benefit from being extracted into a separate function for better readability and testability.
+ // Add this function outside the hook or inside but before useEffect + function formatCountdown(duration: Duration): string { + const years = duration.years(); + const months = duration.months(); + const weeks = Math.floor(duration.days() / 7); + const days = duration.days() % 7; + const hours = duration.hours(); + const minutes = duration.minutes(); + const seconds = duration.seconds(); + + // Format based on the largest unit present + if (years > 0) { + return `${years}y ${months}m`; + } else if (months > 0) { + return `${months}m ${weeks}w`; + } else if (weeks > 0) { + return `${weeks}w ${days}d`; + } else if (days > 0) { + return `${days}d ${hours}h`; + } else if (hours > 0) { + return `${hours}:${minutes < 10 ? `0${minutes}` : minutes}`; + } else { + return `${minutes}:${seconds < 10 ? `0${seconds}` : seconds}`; + } + } // Then in the useEffect const duration = dayjs.duration(diff); - const years = duration.years(); - const months = duration.months(); - const weeks = Math.floor(duration.days() / 7); - const days = duration.days() % 7; - const hours = duration.hours(); - const minutes = duration.minutes(); - const seconds = duration.seconds(); - - // Format based on the largest unit present - if (years > 0) { - setCountdownString(`${years}y ${months}m`); - } else if (months > 0) { - setCountdownString(`${months}m ${weeks}w`); - } else if (weeks > 0) { - setCountdownString(`${weeks}w ${days}d`); - } else if (days > 0) { - setCountdownString(`${days}d ${hours}h`); - } else if (hours > 0) { - setCountdownString( - `${hours}:${minutes < 10 ? `0${minutes}` : minutes}`, - ); - } else { - setCountdownString( - `${minutes}:${seconds < 10 ? `0${seconds}` : seconds}`, - ); - } + setCountdownString(formatCountdown(duration));src/components/cards/DomainCheckoutCard.tsx (2)
41-44: Consider extracting these color references into the Tailwind config or constants.
Centralizing color definitions (e.g.,#[FEC35F]and#[EEA5D2]) helps maintain consistency across the app and makes it simpler to update branding later.
1-160: Add a basic unit test to validate the domain, fee, and countdown rendering logic.
This new component is key to the checkout flow. A short test suite can ensure future refactors don’t break expected behavior. You can use React Testing Library to confirm that fees and countdown strings display as intended.Do you want me to generate an initial test sample? I can open a new issue if you’d like to track this.
src/hooks/useArIOBalance.tsx (1)
38-53: Consider adding a maximum iteration limit or partial results strategy for large delegations.
The while loop can potentially run numerous times if the user’s delegations are very large. Adding a limit or loading progressively can improve responsiveness and reduce heavy calls in extreme cases.src/components/pages/Register/Checkout.tsx (1)
69-73: Simplify the boolean expression in the ternary.
Using a direct boolean cast is more concise and clearer:- return costDetail?.fundingPlan?.shortfall ? true : false; + return !!costDetail?.fundingPlan?.shortfall;🧰 Tools
🪛 Biome (1.9.4)
[error] 70-70: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
assets/images/logo/arns-logo.svgis excluded by!**/*.svgsrc/components/icons/ArIOTokenIcon.svgis excluded by!**/*.svgsrc/components/icons/ArNSLogo.svgis excluded by!**/*.svgsrc/components/icons/TurboIcon.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
package.json(2 hunks)src/App.tsx(2 hunks)src/components/Tooltips/ANTDetailsTip.tsx(1 hunks)src/components/cards/DomainCheckoutCard.tsx(1 hunks)src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx(1 hunks)src/components/icons/index.ts(4 hunks)src/components/inputs/Counter/Counter.tsx(1 hunks)src/components/layout/progress/StepProgressBar/styles.css(3 hunks)src/components/pages/Register/Checkout.tsx(1 hunks)src/components/pages/Register/Register.tsx(1 hunks)src/hooks/useArIOBalance.tsx(1 hunks)src/hooks/useBalance.tsx(0 hunks)src/hooks/useCostDetails.tsx(2 hunks)src/hooks/useCountdown.ts(1 hunks)src/utils/constants.ts(1 hunks)tailwind.config.mjs(1 hunks)
💤 Files with no reviewable changes (1)
- src/hooks/useBalance.tsx
🧰 Additional context used
🧬 Code Definitions (3)
src/App.tsx (1)
src/utils/routes.tsx (1)
Route(15-23)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (1)
src/components/icons/index.ts (3)
ArIOTokenIcon(74-74)TurboIcon(133-133)CircleCheck(93-93)
src/components/pages/Register/Checkout.tsx (1)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (1)
PaymentMethod(13-13)
🪛 Biome (1.9.4)
src/components/pages/Register/Checkout.tsx
[error] 70-70: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build_and_test / lint_test_build
🔇 Additional comments (18)
src/utils/constants.ts (1)
96-96: LGTM! New constant for default ANT logo.The addition of the
DEFAULT_ANT_LOGOconstant follows the project's naming convention and will likely be used in the new domain checkout UI components.tailwind.config.mjs (1)
52-52: LGTM! Adding transparent color to Tailwind config.Adding the transparent color explicitly to the configuration makes it more discoverable and ensures consistency in the project's color system.
package.json (2)
45-45: LGTM! Adding dayjs dependency.Adding dayjs is appropriate for the new countdown functionality required in the checkout feature.
55-55:❓ Verification inconclusive
Verify if base
radix-uipackage is needed.The project already uses specific Radix UI component packages (e.g.,
@radix-ui/react-radio-group). Adding the baseradix-uipackage is unusual - typically you would add specific component packages as needed.
🏁 Script executed:
#!/bin/bash # Check if radix-ui is imported anywhere in the codebase rg -i "from ['\"]radix-ui['\"]" || echo "No direct imports of radix-ui found" # Check which specific Radix UI packages are already imported rg -i "from ['\"]@radix-ui/react-" | sort | uniqLength of output: 458
Action: Confirm Necessity of the Base
radix-uiDependency
Observation:
- The file
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsximports{ Tabs }from the baseradix-uipackage.- Other components consistently import specific Radix UI packages (e.g.,
@radix-ui/react-radio-group,@radix-ui/react-select,@radix-ui/react-switch).Suggestion:
- Verify if importing
Tabsdirectly fromradix-uiis intentional.- If a dedicated package (such as
@radix-ui/react-tabs) exists, consider refactoring the import inPaymentOptionsForm.tsxfor consistency and clearer dependency management.- Otherwise, if the base package is required for additional functionality, please confirm its necessity and ensure that its usage aligns with the current design approach.
src/hooks/useCostDetails.tsx (1)
10-10: LGTM! Extracted stale time to a named constant.Good refactoring to improve code maintainability by extracting the hardcoded stale time value into a named constant. This makes the code more readable and easier to modify in the future.
Also applies to: 25-25
src/components/layout/progress/StepProgressBar/styles.css (3)
39-39: Improved step descriptions layout withwhite-space: nowrapAdding
white-space: nowrapensures the description text in completed steps stays on a single line, preventing awkward text wrapping that could disrupt the layout of the progress bar in the new checkout flow.
65-65: Consistent text wrapping behavior for active stepGood addition of
white-space: nowrapto maintain layout consistency with the current step's description, matching the styling applied to completed steps.
91-91: Completed the consistent styling across all step statesThis completes the consistent application of
white-space: nowrapto waiting steps, ensuring uniform text layout behavior across all three states of the progress bar (completed, active, and waiting).src/components/pages/Register/Register.tsx (1)
189-189:❓ Verification inconclusive
Updated navigation path to new checkout flow
The navigation path has been correctly updated to point to the new domain-specific checkout page instead of the generic transaction review page, aligning with the new checkout feature being implemented.
🏁 Script executed:
#!/bin/bash # Verify that there are no other places still using the old transaction review path for domain registration rg -A 1 -B 1 "navigate\(\`/transaction/review\`" --type ts | grep -v "//.*navigate"Length of output: 83
Navigation Path Update Verified and Approved
The navigation in
src/components/pages/Register/Register.tsx(line 189) is now correctly updated to the new domain-specific checkout page (/register/${domain}/checkout). An initial repository-wide search for references to the deprecated/transaction/reviewpath returned no results. However, since the original verification output was ambiguous, please perform a manual review or run an additional check to confirm that no lingering references to the old transaction review path exist.
- Updated navigation path correctly targets the new checkout flow.
- No instances of the old
/transaction/reviewpath were detected, but manual re-verification is advised.src/components/inputs/Counter/Counter.tsx (1)
102-106: Enhanced value and unit display withwhitespace-nowrapThe change wraps the value and its unit (like "years") in a span with the
whitespace-nowrapclass, ensuring they always appear together on the same line. This improves visual consistency in the checkout process when displaying registration periods.src/App.tsx (1)
15-15: Added import for new Checkout componentAdded the necessary import for the Checkout component to support the new checkout route.
src/components/icons/index.ts (1)
6-7: New icons look good!The newly added icons (ArIOTokenIcon, ArNSLogo, TurboIcon) and TurboJson asset are properly imported and exported. These additions align well with the checkout feature being implemented.
Also applies to: 58-58, 67-67, 70-70, 74-74, 133-134
src/components/Tooltips/ANTDetailsTip.tsx (1)
9-76: LGTM: Well-implemented tooltip component for ANT detailsThe component correctly displays ANT details with proper validation using
isArweaveTransactionIDand fallbacks to "N/A" when values are missing or invalid.src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (2)
1-36: Well-structured payment form implementationThe PaymentOptionsForm component is well-structured with clear types, component organization, and state management for different payment methods and funding sources.
65-209: Clean UI implementation with appropriate user optionsThe UI implementation provides a clear interface for users to select payment methods and funding sources with appropriate visual feedback.
src/hooks/useCountdown.ts (1)
7-60: Well-implemented countdown hook with clean formatting logicThe useCountdown hook provides a nice abstraction for creating countdown timers with appropriate cleanup and format handling.
src/components/cards/DomainCheckoutCard.tsx (1)
57-63: Validate or sanitize domain input to avoid malformed URLs in the link.
Ifdomainmight be user-provided or derived from external inputs, ensure it’s validated or sanitized to prevent user-experience issues or potential vulnerabilities (e.g., accidental or malicious injection of extra path segments).Would you like help creating a basic validation utility?
src/components/pages/Register/Checkout.tsx (1)
48-51: Clarify or implement the 'card' and 'credits' payment methods.
Currently, the code defaults to'crypto', and non-crypto methods are marked as always insufficient. Consider adding a placeholder flow or separate logic for'card'and'credits'if they’ll be supported soon.Would you like me to open a follow-up PR or create an issue to track this?
|
Visit the preview URL for this PR (updated for commit caac677): https://arns-portal--pr731-pe-7854-payments-exdfgkfn.web.app (expires Thu, 17 Apr 2025 18:38:50 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 1f59769442cb89cf2a92042a342e51fe94047b94 |
…bs for credits and card
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/components/cards/DomainCheckoutCard.tsx (8)
34-37: Consider adding error handling for domain info fetching.The component fetches domain information but doesn't handle potential loading or error states from the
useDomainInfohook. This could lead to undefined property access later in the component.- const { data: domainInfo } = useDomainInfo({ + const { data: domainInfo, isLoading, error } = useDomainInfo({ antId: isArweaveTransactionID(antId) ? antId : undefined, }); + // Display loading state or error message if needed + if (isLoading) { + // Consider adding a loading indicator + } + + if (error) { + // Consider handling error state + }
46-52: Ensure fallback handling for AntLogoIcon.Good use of conditional rendering for the logo based on whether
antIdis a valid Arweave transaction ID. However, there's no explicit handling ifdomainInfois undefined or null when accessing the nested property.<AntLogoIcon id={ isArweaveTransactionID(antId) - ? domainInfo?.logo + ? domainInfo?.logo || DEFAULT_ANT_LOGO : DEFAULT_ANT_LOGO } />
54-55: Remove unnecessary whitespace elements.There are several instances of
{' '}in the JSX which add extra whitespace. These can often be removed without affecting the layout, or handled with CSS margin/padding instead.<div className="flex items-center w-fit "> - {' '} <Link<span className="text-white max-w-[180px] truncate"> - {' '} {domain} </span>- </div>{' '} + </div>Also applies to: 77-78, 81-81
90-91: Tailwind class optimization opportunity.The
size-fullclass (which sets both width and height to 100%) is used alongsideflex-col, but may be redundant since the parent already controls the sizing.<div - className={`flex flex-col border-y-[1px] border-foreground gap-4 mt-8 py-6 size-full z-10`} + className={`flex flex-col border-y-[1px] border-foreground gap-4 mt-8 py-6 w-full z-10`} >
98-105: Consider adding empty state handling for order summary.The component maps over
orderSummaryentries but doesn't handle the case where it might be empty. Consider adding a fallback message when there are no items to display.- {Object.entries(orderSummary).map(([key, value], i) => ( + {Object.entries(orderSummary).length > 0 ? Object.entries(orderSummary).map(([key, value], i) => ( <div className="flex flex-row w-full text-sm" key={i}> <span className="flex w-1/3 whitespace-nowrap text-grey"> {key} </span> <span className="flex w-1/2 whitespace-nowrap">{value}</span> </div> - ))} + )) : ( + <div className="text-grey">No order details available</div> + )}
108-108: Remove unnecessary space comment.There's an unnecessary
{' '}after the closing div that can be removed.- </div>{' '} + </div>
142-152: Fix cursor pointer styling in refresh button.The class "pointer" is used, but in Tailwind CSS the correct class is "cursor-pointer".
<button onClick={refresh} - className="flex gap-2 items-center justify-center text-center text-grey pointer" + className="flex gap-2 items-center justify-center text-center text-grey cursor-pointer" >
41-156: Consider adding accessibility improvements.The component could benefit from additional accessibility attributes. Consider adding appropriate ARIA labels to interactive elements and ensuring all UI elements have sufficient contrast.
For the refresh button, add an aria-label:
<button onClick={refresh} + aria-label="Refresh quote" className="flex gap-2 items-center justify-center text-center text-grey pointer" >For the quote status, consider adding aria-live for screen readers:
- <span> + <span aria-live="polite"> {quoteEndTimestamp < 0 ? ( <span className="text-error">Error fetching quote</span> ) : countdownString ? (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/cards/DomainCheckoutCard.tsx(1 hunks)src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx(1 hunks)src/components/pages/Register/Register.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/pages/Register/Register.tsx
- src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build_and_test / lint_test_build
🔇 Additional comments (5)
src/components/cards/DomainCheckoutCard.tsx (5)
1-14: Good organization of imports and clean dependency structure.The imports are well-organized and logical, separating custom hooks, utilities, constants, and UI components. Good practice to use absolute imports with
@srcprefix for project files.
15-23: Well-defined prop interface with appropriate optionality.The
DomainCheckoutCardPropstype is clearly defined with proper JSDoc comments. You've correctly marked optional props with the?operator.
39-39: Verify countdown behavior when timestamp expires.The
useCountdownhook is used to display time remaining, but there's no clear handling for what happens after the countdown reaches zero. Ensure the UI updates appropriately when the quote expires.
111-123: Good implementation of fees display section.The component cleanly iterates through the fees object and displays each fee with proper styling and layout. The use of
key={i}is acceptable here since the data structure is stable.
130-140: Well-handled conditional rendering for quote status.Good implementation of conditional rendering for different quote states (error, countdown, updating). The use of appropriate visual cues (error text color, animation for loading) helps with user experience.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (7)
57-62: Consider handling all CryptoOptions valuesThe type
CryptoOptionsincludes$dARIOand$tARIO, but the balance calculation only returns a value for$ARIO.Since you've defined multiple crypto options in the type, consider handling all cases or adding a comment explaining why only ARIO is supported currently.
const selectedCryptoBalance = useMemo(() => { if (selectedCrypto === formattedARIOTicker) { return allArIOBalance; } + // Other tokens not supported yet, returning 0 + // TODO: Add support for other tokens when available return 0; }, [selectedCrypto, allArIOBalance, formattedARIOTicker]);
82-93: Consider extracting repeated tooltip patternThe tooltips for disabled payment methods follow the same pattern. Consider extracting this to a helper component for better maintainability.
+ const DisabledPaymentOption = ({ icon, label, message = "Coming Soon!" }) => ( + <Tooltip + message={message} + icon={ + <div className="flex gap-3 items-center"> + {icon} + {label} + </div> + } + /> + ); <Tabs.Trigger value="card" disabled={true} className="flex gap-3 p-3 data-[state=active]:bg-foreground rounded border border-[#222224] data-[state=active]:border-grey text-white items-center flex-1 whitespace-nowrap transition-all duration-300 disabled:opacity-50" > - <Tooltip - message="Coming Soon!" - icon={ - <div className="flex gap-3 items-center"> - <CreditCard className="size-5 text-grey" /> - Credit Card - </div> - } - /> + <DisabledPaymentOption + icon={<CreditCard className="size-5 text-grey" />} + label="Credit Card" + /> </Tabs.Trigger>Also applies to: 103-118
82-82: Avoid hardcoded color valuesReplace hardcoded colors like '#222224' with Tailwind color variables for better maintainability.
For example:
- className="flex gap-3 p-3 data-[state=active]:bg-foreground rounded border border-[#222224] data-[state=active]:border-grey text-white items-center flex-1 whitespace-nowrap transition-all duration-300 disabled:opacity-50" + className="flex gap-3 p-3 data-[state=active]:bg-foreground rounded border border-dark-grey data-[state=active]:border-grey text-white items-center flex-1 whitespace-nowrap transition-all duration-300 disabled:opacity-50"This makes it easier to maintain consistent colors across the application.
Also applies to: 96-96, 104-104, 122-122, 128-128, 139-139, 169-169, 183-183, 198-198, 216-216
122-122: Extract common tab content stylesThe
classNamefor the tab contents are nearly identical with minor differences. Consider extracting these to a common style variable.+ const tabContentBaseClass = + "flex flex-col data-[state=active]:border border-dark-grey rounded h-full data-[state=inactive]:size-0 data-[state=inactive]:opacity-0 data-[state=active]:min-h-[405px]"; <Tabs.Content value="card" - className={`flex flex-col data-[state=active]:p-4 data-[state=active]:border border-dark-grey rounded h-full data-[state=inactive]:size-0 data-[state=inactive]:opacity-0 data-[state=active]:min-h-[405px]`} + className={`${tabContentBaseClass} data-[state=active]:p-4`} >Also applies to: 128-128, 216-216
34-49: Add error handling for balance fetchingThe component doesn't visually handle potential errors when fetching balances.
Consider adding error handling to show a user-friendly message when balance fetching fails:
- const { data: liquidBalance } = useArIOLiquidBalance(); - const { data: stakedAndVaultedBalance } = useArIOStakedAndVaultedBalance(); + const { data: liquidBalance, error: liquidBalanceError } = useArIOLiquidBalance(); + const { data: stakedAndVaultedBalance, error: stakedBalanceError } = useArIOStakedAndVaultedBalance(); // Later in the render function: + {(liquidBalanceError || stakedBalanceError) && ( + <div className="text-error text-xs mt-1"> + Error loading balances. Please try again. + </div> + )}
148-150: Fix token symbol duplicationThere's a potential duplication of the $ symbol since
selectedCryptoalready includes it.<span className="text-grey text-xs"> Your total {selectedCrypto} balance: </span> <span className="text-white text-2xl font-bold"> - {formatARIOWithCommas(selectedCryptoBalance)} {selectedCrypto} + {formatARIOWithCommas(selectedCryptoBalance)} ARIO </span>Similarly, check lines 179, 195, and 207 where "ARIO" is displayed directly, which is more consistent.
178-179: Ensure consistent spacing in formatted textThe spacing in the formatted text for balances is inconsistent. There are cases where parentheses are adjacent to text without proper spacing.
- <span className="font-bold">Liquid Balance</span> ({formatARIOWithCommas(liquidArIOBalance)} ARIO) + <span className="font-bold">Liquid Balance</span> ({formatARIOWithCommas(liquidArIOBalance)} ARIO) - <span className="font-bold">Liquid + Staked Balances</span>{' '}({formatARIOWithCommas(allArIOBalance)} ARIO) + <span className="font-bold">Liquid + Staked Balances</span> ({formatARIOWithCommas(allArIOBalance)} ARIO) - </span>{' '}<span className="font-bold">Staked Balances</span> ({formatARIOWithCommas(stakedAndVaultedArIOBalance)} ARIO) + </span> <span className="font-bold">Staked Balances</span> ({formatARIOWithCommas(stakedAndVaultedArIOBalance)} ARIO)Be consistent with how whitespace is handled in these lines. The explicit
{' '}is unnecessary if spacing can be added directly.Also applies to: 191-195, 206-207
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (1)
src/components/icons/index.ts (3)
ArIOTokenIcon(74-74)TurboIcon(133-133)CircleCheck(93-93)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy-to-firebase
🔇 Additional comments (6)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (6)
29-33: Using ticker from contract is goodUsing
arioTickerfrom global state to create crypto options is the right approach.
37-52: Balance calculations look correctThe balance calculations are well-structured and correctly memoized, with proper handling of undefined values.
1-28: Props and imports look goodThe component has well-structured props with appropriate defaults and clear type definitions. The imports are properly organized.
131-143: SelectDropdown implementation is correctThe dropdown implementation for crypto options is well done, with proper value management and styling.
68-119: Tab navigation is well implementedThe payment method tabs are properly set up with clear visual indication of which tab is active and appropriate disabling of future features.
12-12:✅ Verification successful
Verify Radix UI import path
The import
Tabsfrom 'radix-ui' seems unusual. Typically, Radix UI components are imported from scoped packages like '@radix-ui/react-tabs'.
🏁 Script executed:
#!/bin/bash # Check for Radix UI package in package.json grep -r "\"radix-ui\":" package.json # Check if there are other imports from radix-ui in the codebase grep -r "from 'radix-ui'" --include="*.tsx" --include="*.ts" src/Length of output: 232
Radix UI Import Path Verified as Correct
After checking the package.json and the codebase, we confirmed that the project intentionally depends on the "radix-ui" package (version "^1.1.3") and consistently imports from it. No changes are needed regarding the current import path.
atticusofsparta
force-pushed
the
PE-7854-payments
branch
from
2cdd28d to
81cb85e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (5)
32-34: Consider future-proofing the crypto dropdown optionsCurrently, the dropdown only offers one option (
$ARIO). While this matches current requirements, the component structure suggests you intend to support multiple tokens in the future. You might want to add a TODO comment for clarity.
58-63: Consider adding error states for balance retrieval failuresWhile the code handles undefined balances by defaulting to 0, it might be helpful to show loading states or error messages when balance retrieval fails.
const selectedCryptoBalance = useMemo(() => { if (selectedCrypto === formattedARIOTicker) { return allArIOBalance; } return 0; -}, [selectedCrypto, allArIOBalance, formattedARIOTicker]); +}, [selectedCrypto, allArIOBalance, formattedARIOTicker]); + +// Add error handling for balance retrieval failures +const isLoadingBalance = liquidBalance === undefined || stakedAndVaultedBalance === undefined;
132-144: Consider improving accessibility for crypto dropdownThe dropdown is currently marked with
pointer-events-none, which makes it appear disabled but might cause accessibility issues. Since this is the only available option, consider either:
- Making it actually functional but with just one option
- Displaying the selection as plain text without a dropdown UI
className={{ trigger: - 'flex w-full gap-2 p-3 rounded-md bg-transparent border border-[#222224] items-center pointer-events-none', + 'flex w-full gap-2 p-3 rounded-md bg-transparent border border-[#222224] items-center', icon: 'text-grey size-5', }} options={cryptoDropdownOptions}
180-180: Use consistent token naming in UI displayThere's an inconsistency in how the token is displayed. Some places use "ARIO" and others use the variable
selectedCryptowhich includes the "$" prefix. Consider using a consistent approach throughout.-<span className="font-bold">Liquid Balance</span> ({formatARIOWithCommas(liquidArIOBalance)} ARIO) +<span className="font-bold">Liquid Balance</span> ({formatARIOWithCommas(liquidArIOBalance)} {selectedCrypto.replace('$', '')})Similar changes should be applied to lines 195-196 and 208-209.
Also applies to: 195-196, 208-209
1-227: Consider adding validation for insufficient balance scenariosThe component displays balances effectively but doesn't appear to have validation for scenarios where the selected funding source has insufficient balance for the purchase. Consider adding this validation to prevent failed transactions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (1)
src/components/icons/index.ts (3)
ArIOTokenIcon(74-74)TurboIcon(133-133)CircleCheck(93-93)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy-to-firebase
🔇 Additional comments (4)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (4)
30-31: Use the ticker directly from the contract without manual formattingThe code formats the ticker with a dollar sign prefix before using it. According to a previous review comment, you should use the ticker directly from the contract.
38-53: Good use of useMemo for balance calculationsThe balance calculations are appropriately wrapped in useMemo hooks, which prevents unnecessary recalculations during renders.
82-94: Great user experience with tooltips for unavailable optionsUsing tooltips with "Coming Soon!" messages for disabled payment methods provides good UX by setting clear expectations for future features.
155-213: Well-implemented funding source selectionThe funding source selection is well-implemented with clear visual feedback using check icons and informative balance displays. The component correctly updates the parent component through callbacks when selections change.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (4)
1-1: Consider adding explicit type definitions for imports.
When using inline imports, it can be helpful to explicitly type them (e.g.,import { FundFrom } from '@ar.io/sdk';) to ensure the integration with external libraries is fully typed.
32-34: Handle future expansions of crypto dropdown options.
UsinguseMemois efficient here, but if additional tokens end up being introduced, ensure they dynamically refresh the dropdown if new tokens are fetched from an API or global state.
82-83: Double-check disabled state for payment triggers.
Disabling the "card" payment method is an effective placeholder. However, consider conditionally enabling it or completely hiding the tab if development is far off to avoid user confusion.
104-106: Confirm the user flow for crypto vs. credits.
Similar to the "card" method, the "credits" option is also disabled. If these are on a near-term roadmap, it might be clearer to display a short contextual message in plain text instead of a disabled button.src/components/Tooltips/ANTDetailsTip.tsx (5)
9-21: Props structure looks good, but needs validation for edge cases.The component has a clear prop structure with appropriate types, making it easy to understand what information should be provided. However, since all props are optional, the component could render with no meaningful information if no props are provided.
Consider adding validation or default content for cases when no valid IDs are provided:
function ANTDetailsTip({ antId, targetId, owner, icon = ( <InfoCircleOutlined className="text-grey" width={'14px'} height={'14px'} /> ), }: { antId?: string; targetId?: string; owner?: string; icon?: ReactNode; }) { + const hasValidData = antId || targetId || owner; + if (!hasValidData) { + return null; // Or some fallback UI + } return ( // ...
13-15: Icon dimensions should use number values instead of strings.The
widthandheightprops should be provided as numbers rather than strings to avoid potential warnings from React.icon = ( - <InfoCircleOutlined className="text-grey" width={'14px'} height={'14px'} /> + <InfoCircleOutlined className="text-grey" style={{ fontSize: '14px' }} /> ),
23-28: Consider extracting tooltip styling to dedicated class.The tooltip configuration uses multiple inline style objects which could be better managed through a dedicated CSS class or Tailwind utility classes.
<Tooltip - tooltipOverrides={{ - overlayClassName: 'flex w-fit', - overlayStyle: { width: 'fit-content' }, - overlayInnerStyle: { width: 'fit-content' }, - }} + tooltipOverrides={{ + overlayClassName: 'flex w-fit ant-details-tooltip', + }} message={Then define the class in your CSS/SCSS file:
.ant-details-tooltip { width: fit-content !important; } .ant-details-tooltip .ant-tooltip-inner { width: fit-content !important; }
31-43: Inconsistent handling of invalid IDs.The component handles invalid values differently across the three sections. Process ID and Target ID show 'N/A' when invalid, but Owner uses formatting.
For better consistency:
{isArweaveTransactionID(owner) ? ( <ArweaveID id={new ArweaveTransactionID(owner)} shouldLink={true} characterCount={16} type={ArweaveIdTypes.ADDRESS} /> ) : ( - formatForMaxCharCount(owner ?? 'N/A', 16) + owner ? formatForMaxCharCount(owner, 16) : 'N/A' )}Additionally, consider extracting the repeated pattern into a reusable function to reduce duplication:
const renderArweaveId = (id?: string, type: ArweaveIdTypes) => { if (isArweaveTransactionID(id)) { return ( <ArweaveID id={new ArweaveTransactionID(id)} shouldLink={true} characterCount={16} type={type} /> ); } return id ? formatForMaxCharCount(id, 16) : 'N/A'; };Also applies to: 45-57, 58-70
30-71: Extract repeated layout patterns to improve readability.The component has three similar sections with identical styling and only minor differences in content. This pattern could be extracted to reduce duplication.
Consider creating a helper component for the repeated layout:
type DetailRowProps = { label: string; value?: string; idType?: ArweaveIdTypes; }; const DetailRow = ({ label, value, idType }: DetailRowProps) => ( <span className="flex gap-2 text-sm text-grey py-4 border-b-[1px] border-dark-grey whitespace-nowrap"> {label}:{' '} {isArweaveTransactionID(value) && idType ? ( <ArweaveID id={new ArweaveTransactionID(value)} shouldLink={true} characterCount={16} type={idType} /> ) : ( value ? formatForMaxCharCount(value, 16) : 'N/A' )} </span> ); // Then in the component: <div className="flex flex-col p-2 pt-0"> <DetailRow label="Process ID" value={antId} idType={ArweaveIdTypes.CONTRACT} /> <DetailRow label="Target ID" value={targetId} idType={ArweaveIdTypes.TRANSACTION} /> <DetailRow label="Owner" value={owner} idType={ArweaveIdTypes.ADDRESS} /> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Tooltips/ANTDetailsTip.tsx(1 hunks)src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (1)
src/components/icons/index.ts (3)
ArIOTokenIcon(74-74)TurboIcon(133-133)CircleCheck(93-93)
🔇 Additional comments (3)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (2)
15-17: Validate additional token options.
Currently,ARIOCryptoOptionsincludes$dARIOand$tARIO, but you only display$ARIOin the dropdown. Consider validating or handling the other options more explicitly in case you expand support for them in the future.
185-225: Ensure comprehensive test coverage for balance logic.
You correctly handle the user’s choice between liquid, staked, or any combination. However, test coverage should confirm the correct displayed balance and selected funding source under different states (e.g., zero balance, partial balance, high balance).src/components/Tooltips/ANTDetailsTip.tsx (1)
1-78: Component looks good overall, with well-defined functionality.This tooltip component is well-structured and provides a clean way to display ANT details. The use of conditional rendering based on ID validity is a good approach, and the component is nicely modular.
The component demonstrates good practices by:
- Using TypeScript for type safety
- Providing default values for optional props
- Using proper validation before rendering ID components
- Creating a reusable UI element for displaying ANT metadata
| </div> | ||
| {selectedCrypto === formattedARIOTicker && ( | ||
| <div className="flex flex-col gap-2 size-full items-start"> | ||
| <span className="text-grey text-xs"> | ||
| Select balance method: | ||
| </span> | ||
| <Tabs.Root | ||
| defaultValue={fundingSource} | ||
| value={fundingSource} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid nested Tabs if possible.
You have nested Tabs for crypto balances. While it works, the nested approach can sometimes lead to confusion. Consider using a single set of components or disclaimers within the primary crypto tab (e.g., a single radio group for choosing between liquid/stakes).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/pages/Register/Checkout.tsx (2)
69-74: Simplify handling ofisInsufficientBalance.There is an unnecessary ternary operator when checking for
costDetail?.fundingPlan?.shortfall. Also, returningtruefor every non-crypto payment method effectively blocks those methods. Verify whether you intend to support'card'or'credits'in the future.Consider the following patch to remove the redundant ternary operator:
- return costDetail?.fundingPlan?.shortfall ? true : false; + return !!costDetail?.fundingPlan?.shortfall;🧰 Tools
🪛 Biome (1.9.4)
[error] 71-71: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
163-164: Address the wallet connectivity TODO.The comment indicates the need to check wallet connectivity. This step is crucial to prevent errors in dispatching transactions.
Would you like me to propose a check to confirm wallet connectivity before proceeding?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/pages/Register/Checkout.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/components/pages/Register/Checkout.tsx (1)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (1)
PaymentMethod(15-15)
🪛 Biome (1.9.4)
src/components/pages/Register/Checkout.tsx
[error] 71-71: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy-to-firebase
🔇 Additional comments (2)
src/components/pages/Register/Checkout.tsx (2)
112-115: Clarify handling of non-crypto payment methods.Currently, the code sets the fees to
'Invalid payment method selected'into the returned object if the payment method is not'crypto'. If you plan to integrate'card'or'credits'later (per thePaymentMethodtype definition), prompt users or handle these methods appropriately.
190-273: Overall code quality is solid.Other than the suggestions and questions above, the component effectively handles the checkout workflow, integrates well with your existing states/hooks, and includes appropriate error handling.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
src/components/cards/DomainCheckoutCard.tsx (9)
9-12: Consider reordering and grouping imports.The imports could be more organized to improve readability. Consider grouping them by type (external libraries, internal components, hooks, utilities) and using alphabetical ordering within each group.
import { useCountdown } from '@src/hooks/useCountdown'; import useDomainInfo from '@src/hooks/useDomainInfo'; import { useWalletState } from '@src/state'; import { isArweaveTransactionID } from '@src/utils'; import { DEFAULT_ANT_LOGO } from '@src/utils/constants'; import { RotateCw } from 'lucide-react'; import { ReactNode } from 'react'; -import ANTDetailsTip from '../Tooltips/ANTDetailsTip'; -import { Tooltip } from '../data-display'; -import { AntLogoIcon } from '../data-display/AntLogoIcon'; -import { ArNSLogo } from '../icons'; +// Internal components +import { Tooltip } from '../data-display'; +import { AntLogoIcon } from '../data-display/AntLogoIcon'; +import ANTDetailsTip from '../Tooltips/ANTDetailsTip'; +import { ArNSLogo } from '../icons';
51-51: Consider adding descriptive alt text or aria-label for the AntLogoIcon.For better accessibility, consider adding descriptive alt text or aria-label to the logo icon.
<AntLogoIcon id={ isArweaveTransactionID(antId) ? domainInfo?.logo : DEFAULT_ANT_LOGO } + aria-label={`ANT logo for ${domain}`} />
54-73: Unnecessary whitespace and non-semantic span elements.There are unnecessary whitespace string literals and non-semantic span elements that could be cleaned up for better code quality.
<div className="flex items-center w-fit "> - {' '} <span className="text-link-normal">ar://</span> <Tooltip tooltipOverrides={{ overlayClassName: 'w-fit', overlayInnerStyle: { whiteSpace: 'nowrap', width: 'fit-content', padding: '0.625rem', border: '1px solid var(--text-faded)', }, }} message={domain} icon={ <span className="text-white max-w-[11.25rem] truncate"> - {' '} {domain} </span> } /> -</div>{' '} +</div>
82-82: Use consistent border styling with Tailwind classes.Instead of using border-y-[1px], consider using Tailwind's built-in border classes for consistency.
<div - className={`flex flex-col border-y-[1px] border-foreground gap-4 mt-8 py-6 size-full z-10`} + className={`flex flex-col border-y border-foreground gap-4 mt-8 py-6 size-full z-10`} >
90-97: Add accessibility improvements to the order summary rows.The current implementation lacks proper semantic structure for the data table. Consider using a more appropriate HTML structure or ARIA roles.
-{Object.entries(orderSummary).map(([key, value], i) => ( - <div className="flex flex-row w-full text-sm" key={i}> - <span className="flex w-1/3 whitespace-nowrap text-grey"> - {key} - </span> - <span className="flex w-1/2 whitespace-nowrap">{value}</span> - </div> -))} +<dl className="flex flex-col w-full gap-4 text-sm"> + {Object.entries(orderSummary).map(([key, value], i) => ( + <div className="flex flex-row w-full" key={i}> + <dt className="flex w-1/3 whitespace-nowrap text-grey"> + {key} + </dt> + <dd className="flex w-1/2 whitespace-nowrap">{value}</dd> + </div> + ))} +</dl>
100-100: Remove unnecessary whitespace character.There's an unnecessary whitespace character inside a JSX expression.
-</div>{' '} +</div>
103-115: Apply the same accessibility improvements to the fees section.Similar to the order summary, the fees section would benefit from semantic HTML structure.
-{Object.entries(fees).map(([key, value], i) => ( - <div - className="flex flex-row w-full text-sm justify-between" - key={i} - > - <span className="flex w-1/3 whitespace-nowrap text-grey"> - {key} - </span> - <span className="flex w-1/2 whitespace-nowrap justify-end"> - {value} - </span> - </div> -))} +<dl className="flex flex-col gap-2 w-full"> + {Object.entries(fees).map(([key, value], i) => ( + <div + className="flex flex-row w-full text-sm justify-between" + key={i} + > + <dt className="flex w-1/3 whitespace-nowrap text-grey"> + {key} + </dt> + <dd className="flex w-1/2 whitespace-nowrap justify-end"> + {value} + </dd> + </div> + ))} +</dl>
134-144: Add appropriate ARIA attributes to the refresh button.The refresh button would benefit from ARIA attributes for better accessibility, especially when showing the loading state.
<button onClick={refresh} className="flex gap-2 items-center justify-center text-center text-grey pointer" + aria-label="Refresh quote" + aria-busy={!countdownString} > <RotateCw className={`text-grey size-4 ${ !countdownString ? 'animate-spin' : '' }`} + aria-hidden="true" /> Refresh </button>
121-132: Improve the quote status display with semantic HTML and clearer status indicators.The current implementation uses nested spans, which can be simplified with better semantic HTML and state handling.
-<span> - {quoteEndTimestamp < 0 ? ( - <span className="text-error">Error fetching quote</span> - ) : countdownString ? ( - <span className="whitespace-nowrap"> - Quote updates in{' '} - <span className="text-white text-bold">{countdownString}</span> - </span> - ) : ( - <span className="animate-pulse">Updating quote...</span> - )} -</span> +<div role="status"> + {quoteEndTimestamp < 0 ? ( + <span className="text-error" aria-live="polite">Error fetching quote</span> + ) : countdownString ? ( + <span className="whitespace-nowrap"> + Quote updates in{' '} + <strong className="text-white">{countdownString}</strong> + </span> + ) : ( + <span className="animate-pulse" aria-live="polite">Updating quote...</span> + )} +</div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Tooltips/ANTDetailsTip.tsx(1 hunks)src/components/cards/DomainCheckoutCard.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Tooltips/ANTDetailsTip.tsx
🔇 Additional comments (5)
src/components/cards/DomainCheckoutCard.tsx (5)
43-43: Use Tailwind color variables instead of hex codes for better maintainability.As mentioned in previous reviews, using hex codes directly makes maintenance harder. Define these colors in your Tailwind config and use the named colors.
-<div className="flex flex-col size-full bg-gradient-to-b from-[#1C1C1F] to-[#0E0E0F] rounded-t text-light-grey p-6"> +<div className="flex flex-col size-full bg-gradient-to-b from-dark-surface to-darker-surface rounded-t text-light-grey p-6">
61-62: Replace hard-coded pixel values with relative units.As mentioned in previous reviews, use relative units like rem instead of px values for better responsiveness.
overlayInnerStyle: { whiteSpace: 'nowrap', width: 'fit-content', - padding: '0.625rem', - border: '1px solid var(--text-faded)', + padding: '0.625rem', + border: '1px solid var(--text-faded)', },
76-76: Ensure proper optional chaining for nested properties.As mentioned in a previous review, ensure proper optional chaining for all nested properties to prevent potential runtime errors.
<ANTDetailsTip antId={antId} - targetId={domainInfo?.records?.['@']?.transactionId ?? targetId} + targetId={domainInfo?.records?.['@']?.transactionId ?? targetId} owner={domainInfo?.owner ?? walletAddress?.toString()} />This looks like it's already been addressed in the current version, which is good!
1-39: LGTM! All imports, props definition, and state management look well-structured.The component's imports, type definitions, and state management are well-structured. The use of custom hooks like
useCountdownanduseDomainInfohelps separate concerns and makes the component more readable.
119-146: Good error handling and loading state management in the quote timer.The implementation properly handles different states of the quote timer, including error states, countdown, and loading states. The refresh button with loading animation is a nice touch for UX.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (1)
190-224: Reduce repetition by consolidating fund source triggers.The three
Tabs.Triggerelements for'balance','any', and'stakes'share similar structure. Consider generating them from a small data array to keep the code DRY and more easily extensible.src/components/pages/Register/Checkout.tsx (2)
69-74: Simplify the conditional to remove the extraneous boolean literal.Instead of using
costDetail?.fundingPlan?.shortfall ? true : false;, consider:- return costDetail?.fundingPlan?.shortfall ? true : false; + return !!costDetail?.fundingPlan?.shortfall;This improves readability and follows the principle of avoiding unnecessary ternary expressions.
🧰 Tools
🪛 Biome (1.9.4)
[error] 71-71: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
163-163: Address the wallet connection TODO.The
// TODO: check that it's connectedcomment indicates an incomplete verification step. Would you like me to open a new issue or implement a quick check for wallet connection here?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Tooltips/ANTDetailsTip.tsx(1 hunks)src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx(1 hunks)src/components/pages/Register/Checkout.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Tooltips/ANTDetailsTip.tsx
🧰 Additional context used
🧬 Code Definitions (2)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (1)
src/components/icons/index.ts (3)
ArIOTokenIcon(74-74)TurboIcon(133-133)CircleCheck(93-93)
src/components/pages/Register/Checkout.tsx (1)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (1)
PaymentMethod(15-15)
🪛 Biome (1.9.4)
src/components/pages/Register/Checkout.tsx
[error] 71-71: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy-to-firebase
🔇 Additional comments (3)
src/components/forms/PaymentOptionsForm/PaymentOptionsForm.tsx (2)
15-17: Great job defining the PaymentMethod type.Looks straightforward and should help maintain clear boundaries between payment options.
172-180: Nested Tabs approach was previously flagged.This repeats the same nested
Tabspattern as noted in an earlier review comment. Reusing or refactoring to avoid deeply nested tab structures can improve clarity and maintainability.src/components/pages/Register/Checkout.tsx (1)
217-235: Confirm the static “10 included” undernames.The domain checkout card currently displays “Undernames: 10 included.” Verify whether this is always correct or should be dynamically fetched based on plan or tier. Let me know if you’d like an automated check.
Summary by CodeRabbit
New Features
Bug Fixes
Style