feat: crypto checkout redesign — 4-step flow with search, QR, confirmation#62
Open
feat: crypto checkout redesign — 4-step flow with search, QR, confirmation#62
Conversation
…orms Tab-based admin page at /admin/products for managing product configuration. Uses direct fetch to tRPC endpoints (REST-style) to avoid type dependency on unwired backend router. Five sub-components cover Brand, Navigation, Features, Fleet, and Billing with toast feedback and disabled-while-saving. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fetches brand config from tRPC endpoint at app init, falls back to env var defaults if API unavailable. 60s revalidation cache. Product backends serve this via product.getBrandConfig endpoint. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- mutateProductConfig checks tRPC-level errors (not just HTTP status) - initBrandConfig includes credentials, handles non-JSON responses - Numeric inputs handle empty/NaN values - Remove dead loading.tsx (client component handles own loading) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…r, DepositView, ConfirmationTracker Four independent components for the 4-step crypto checkout flow: - AmountSelector: preset + custom amount, min $10 - PaymentMethodPicker: searchable list with Popular/Stablecoins/L2/Native filters - DepositView: QR code, address copy, countdown timer - ConfirmationTracker: progress bar, step timeline 16 tests passing across 4 test files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- CryptoCheckout manages step state machine (amount → method → deposit → confirming) - Polls charge status every 5s, auto-advances to confirmation step - Old BuyCryptoCreditPanel replaced with thin re-export (preserves imports) - 18 tests across 5 files, all passing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (19)
✨ 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 |
Reviewer's GuideReplaces the legacy BuyCryptoCreditPanel with a new step-based CryptoCheckout flow (amount selection, payment method search/filter, QR deposit, confirmation tracker) while adding an admin Product Configuration UI and a brand config bootstrap helper that consumes platform API configuration. Sequence diagram for the new CryptoCheckout 4-step flowsequenceDiagram
actor User
participant CryptoCheckout
participant AmountSelector
participant PaymentMethodPicker
participant DepositView
participant ConfirmationTracker
participant Api as PlatformApi
User->>CryptoCheckout: Open credits billing panel
CryptoCheckout->>PlatformApi: getSupportedPaymentMethods()
PlatformApi-->>CryptoCheckout: SupportedPaymentMethod[]
CryptoCheckout->>AmountSelector: Render amount step
User->>AmountSelector: Select preset or enter custom amount
AmountSelector-->>CryptoCheckout: onSelect(amountUsd)
CryptoCheckout->>CryptoCheckout: setStep("method")
CryptoCheckout->>PaymentMethodPicker: Render method picker
User->>PaymentMethodPicker: Search/filter methods
User->>PaymentMethodPicker: Choose SupportedPaymentMethod
PaymentMethodPicker-->>CryptoCheckout: onSelect(method)
CryptoCheckout->>PlatformApi: createCheckout(method.id, amountUsd)
PlatformApi-->>CryptoCheckout: CheckoutResult
CryptoCheckout->>CryptoCheckout: setStep("deposit"), status="waiting"
CryptoCheckout->>DepositView: Render QR deposit view
User->>DepositView: Scan QR or copy address
loop Poll charge status
CryptoCheckout->>PlatformApi: getChargeStatus(referenceId)
PlatformApi-->>CryptoCheckout: chargeStatus
CryptoCheckout->>CryptoCheckout: update status, confirmations
alt amountReceivedCents >= amountExpectedCents
CryptoCheckout->>CryptoCheckout: setStep("confirming"), status="confirming"
end
end
CryptoCheckout->>ConfirmationTracker: Render confirmation tracker
ConfirmationTracker->>ConfirmationTracker: Update progress bar from confirmations
alt credited
CryptoCheckout->>CryptoCheckout: status="credited"
User->>CryptoCheckout: Click "Done — buy more credits"
CryptoCheckout->>CryptoCheckout: Reset state to step="amount"
else expired or failed
CryptoCheckout->>CryptoCheckout: status="expired" or "failed"
User->>CryptoCheckout: Navigate back or restart
end
Updated class diagram for CryptoCheckout and billing UI componentsclassDiagram
class CryptoCheckout {
-Step step
-SupportedPaymentMethod[] methods
-number amountUsd
-CheckoutResult checkout
-PaymentStatus status
-number confirmations
-number confirmationsRequired
-boolean loading
+CryptoCheckout()
+handleAmount(amountUsd number) void
+handleMethod(method SupportedPaymentMethod) Promise~void~
+handleReset() void
}
class AmountSelector {
-number selected
-string custom
+AmountSelector(onSelect function)
+onSelect(amount number) void
}
class PaymentMethodPicker {
-SupportedPaymentMethod[] methods
-number amountUsd
-string search
-Filter filter
+PaymentMethodPicker(methods SupportedPaymentMethod[], amountUsd number, onSelect function, onBack function)
+onSelect(method SupportedPaymentMethod) void
+onBack() void
}
class DepositView {
-CheckoutResult checkout
-PaymentStatus status
-boolean copied
-number timeLeft
+DepositView(checkout CheckoutResult, status PaymentStatus, onBack function)
+onBack() void
}
class ConfirmationTracker {
-number confirmations
-number confirmationsRequired
-string displayAmount
-boolean credited
-string txHash
+ConfirmationTracker(confirmations number, confirmationsRequired number, displayAmount string, credited boolean, txHash string)
}
class BuyCryptoCreditPanel {
+BuyCryptoCreditPanel()
}
CryptoCheckout --> AmountSelector : renders
CryptoCheckout --> PaymentMethodPicker : renders
CryptoCheckout --> DepositView : renders
CryptoCheckout --> ConfirmationTracker : renders
CryptoCheckout ..> SupportedPaymentMethod : uses
CryptoCheckout ..> CheckoutResult : uses
CryptoCheckout ..> PaymentStatus : uses
PaymentMethodPicker ..> SupportedPaymentMethod : filters
DepositView ..> CheckoutResult : displays
class SupportedPaymentMethod {
+string id
+string token
+string chain
+string displayName
+string type
+string iconUrl
}
class CheckoutResult {
+string referenceId
+string displayAmount
+string depositAddress
+string token
+string chain
+number amountUsd
}
class PaymentStatus {
}
class Step {
}
class createCheckout {
+createCheckout(methodId string, amountUsd number) Promise~CheckoutResult~
}
class getSupportedPaymentMethods {
+getSupportedPaymentMethods() Promise~SupportedPaymentMethod[]~
}
class getChargeStatus {
+getChargeStatus(referenceId string) Promise~any~
}
CryptoCheckout ..> createCheckout : calls
CryptoCheckout ..> getSupportedPaymentMethods : calls
CryptoCheckout ..> getChargeStatus : polls
BuyCryptoCreditPanel <.. CryptoCheckout : re-export
Updated class diagram for admin Product Configuration UI and brand initclassDiagram
class AdminProductsPage {
-ProductConfig config
-boolean loading
-string loadError
+AdminProductsPage()
+load() Promise~void~
}
class ProductConfig {
+BrandProduct product
+NavItem[] navItems
+FeaturesConfig features
+FleetConfig fleet
+BillingConfig billing
}
class BrandForm {
-BrandConfig form
-boolean saving
+BrandForm(initial BrandConfig, onSave function)
+handleSave() Promise~void~
}
class NavEditor {
-NavItem[] items
-boolean saving
+NavEditor(initial NavItem[], onSave function)
+handleSave() Promise~void~
}
class FeaturesForm {
-FeaturesConfig form
-boolean saving
+FeaturesForm(initial FeaturesConfig, onSave function)
+handleSave() Promise~void~
}
class FleetForm {
-FleetConfig form
-boolean saving
+FleetForm(initial FleetConfig, onSave function)
+handleSave() Promise~void~
}
class BillingForm {
-BillingConfig form
-boolean saving
+BillingForm(initial BillingConfig, onSave function)
+handleSave() Promise~void~
}
class adminFetch {
+adminFetch(path string, init RequestInit) Promise~Response~
}
class fetchProductConfig {
+fetchProductConfig() Promise~ProductConfig~
}
class mutateProductConfig {
+mutateProductConfig(endpoint string, input any) Promise~void~
}
class BrandConfig {
+string id
+string slug
+string brandName
+string productName
+string tagline
+string domain
+string appDomain
+string cookieDomain
+string companyLegal
+string priceLabel
+string defaultImage
+string emailSupport
+string emailPrivacy
+string emailLegal
+string fromEmail
+string homePath
+string storagePrefix
}
class FeaturesConfig {
+boolean chatEnabled
+boolean onboardingEnabled
+string onboardingDefaultModel
+number onboardingMaxCredits
+string onboardingWelcomeMsg
+boolean sharedModuleBilling
+boolean sharedModuleMonitoring
+boolean sharedModuleAnalytics
}
class FleetConfig {
+string containerImage
+number containerPort
+string lifecycle
+string billingModel
+number maxInstances
+string dockerNetwork
+string placementStrategy
+string fleetDataDir
}
class BillingConfig {
+string stripePublishableKey
+Record~string, number~ creditPrices
+string affiliateBaseUrl
+string affiliateMatchRate
+number affiliateMaxCap
+string dividendRate
}
class NavItem {
+string id
+string label
+string href
+string icon
+number sortOrder
+string requiresRole
+boolean enabled
}
AdminProductsPage --> BrandForm : renders
AdminProductsPage --> NavEditor : renders
AdminProductsPage --> FeaturesForm : renders
AdminProductsPage --> FleetForm : renders
AdminProductsPage --> BillingForm : renders
AdminProductsPage ..> fetchProductConfig : calls
AdminProductsPage ..> mutateProductConfig : passes_to_forms
fetchProductConfig ..> adminFetch : uses
mutateProductConfig ..> adminFetch : uses
BrandForm ..> BrandConfig : edits
NavEditor ..> NavItem : edits
FeaturesForm ..> FeaturesConfig : edits
FleetForm ..> FleetConfig : edits
BillingForm ..> BillingConfig : edits
class initBrandConfig {
+initBrandConfig(apiBaseUrl string) Promise~void~
}
class setBrandConfig {
+setBrandConfig(partialConfig Partial~BrandConfig~) void
}
initBrandConfig ..> setBrandConfig : applies
AdminProductsPage ..> ProductConfig : manages
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 5 issues, and left some high level feedback:
- In
PaymentMethodPickertheamountUsdprop is currently unused; either wire it into the UI (e.g., show approximate cost per method) or remove the prop to avoid confusion and keep the component interface minimal. - When
getSupportedPaymentMethodsfails inCryptoCheckout, the component returnsnull, which silently hides the panel; consider rendering a small inline error or fallback state so users understand why crypto checkout is unavailable instead of seeing nothing. - In
DepositViewand other clipboard usages, you callnavigator.clipboard.writeTextwithout capability checks; consider guarding with a feature check or catching and surfacing errors so unsupported browsers or restricted contexts degrade more gracefully.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `PaymentMethodPicker` the `amountUsd` prop is currently unused; either wire it into the UI (e.g., show approximate cost per method) or remove the prop to avoid confusion and keep the component interface minimal.
- When `getSupportedPaymentMethods` fails in `CryptoCheckout`, the component returns `null`, which silently hides the panel; consider rendering a small inline error or fallback state so users understand why crypto checkout is unavailable instead of seeing nothing.
- In `DepositView` and other clipboard usages, you call `navigator.clipboard.writeText` without capability checks; consider guarding with a feature check or catching and surfacing errors so unsupported browsers or restricted contexts degrade more gracefully.
## Individual Comments
### Comment 1
<location path="src/components/billing/crypto-checkout.tsx" line_range="68-77" />
<code_context>
+ setStep("method");
+ }, []);
+
+ const handleMethod = useCallback(
+ async (method: SupportedPaymentMethod) => {
+ setLoading(true);
+ try {
+ const result = await createCheckout(method.id, amountUsd);
+ setCheckout(result);
+ setStatus("waiting");
+ setStep("deposit");
+ } catch {
+ // Stay on method step
+ } finally {
+ setLoading(false);
+ }
+ },
+ [amountUsd],
+ );
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Errors from `createCheckout` are swallowed, leaving the user without feedback if checkout creation fails.
The empty `catch` in `handleMethod` means `createCheckout` failures (e.g. network/server errors) are completely silent—the user just stays on the method step with no feedback. Consider surfacing an error (local error state or toast), and ensure loading is cleared with an obvious way to retry so failures aren’t invisible.
</issue_to_address>
### Comment 2
<location path="src/components/admin/products/features-form.tsx" line_range="40-42" />
<code_context>
+ setForm((prev) => ({ ...prev, [key]: value || null }));
+ }
+
+ function setNum(key: keyof FeaturesConfig, value: string) {
+ const n = Number.parseInt(value, 10);
+ if (!Number.isNaN(n)) setForm((prev) => ({ ...prev, [key]: n }));
+ }
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Numeric onboarding fields cannot be cleared back to an empty/zero state by deleting the input, which may be surprising UX.
In `setNum`, clearing the input (`value === ""`) leads to `Number.parseInt` returning `NaN`, so the state isn’t updated and the previous value remains. Consider handling `""` explicitly (e.g., map it to `0` or `null`), or allow setting the state even when the parsed value is `NaN` and normalize the value on submit, so users can truly clear the field.
Suggested implementation:
```typescript
function setStr(key: keyof FeaturesConfig, value: string) {
setForm((prev) => ({ ...prev, [key]: value || null }));
}
function setNum(key: keyof FeaturesConfig, value: string) {
if (value === "") {
// Allow clearing numeric fields by mapping an empty input to null
setForm((prev) => ({ ...prev, [key]: null }));
return;
}
const n = Number.parseInt(value, 10);
if (!Number.isNaN(n)) {
setForm((prev) => ({ ...prev, [key]: n }));
}
}
import { useState } from "react";
```
1. Ensure the corresponding numeric fields in `FeaturesConfig` are typed as `number | null` (or a compatible union) to accept the `null` value used when clearing input.
2. If there is a submit/normalization layer that expects a different cleared value (e.g., `0` instead of `null`), adjust the empty-string mapping in `setNum` accordingly.
</issue_to_address>
### Comment 3
<location path="src/__tests__/payment-method-picker.test.tsx" line_range="82-86" />
<code_context>
+ expect(screen.queryByText("Bitcoin")).not.toBeInTheDocument();
+ });
+
+ it("calls onSelect when a method is clicked", async () => {
+ const onSelect = vi.fn();
+ render(<PaymentMethodPicker methods={METHODS} amountUsd={25} onSelect={onSelect} />);
+ await userEvent.click(screen.getByText("Bitcoin"));
+ expect(onSelect).toHaveBeenCalledWith(METHODS[0]);
+ });
+});
</code_context>
<issue_to_address>
**suggestion (testing):** Consider testing the optional `onBack` behavior when provided
A focused test that renders `PaymentMethodPicker` with `onBack={vi.fn()}`, clicks the `← Back` button, and asserts the callback is called would help ensure the back navigation remains wired correctly in the multi-step flow.
</issue_to_address>
### Comment 4
<location path="src/__tests__/deposit-view.test.tsx" line_range="19-28" />
<code_context>
+describe("DepositView", () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for non-waiting statuses and back navigation in DepositView
Current tests only cover the waiting/QR/copy flow. Please also add tests for `status="partial"`, `"expired"`, and `"failed"` to verify the expected text and indicator styles, and a test that the `← Back` button triggers the `onBack` callback. This will cover all user-visible states and navigation behavior for the deposit step.
Suggested implementation:
```typescript
describe("DepositView", () => {
it("shows deposit address and amount", () => {
render(<DepositView checkout={CHECKOUT} status="waiting" onBack={vi.fn()} />);
expect(screen.getByText(/32\.24 TRX/)).toBeInTheDocument();
expect(screen.getByText(/THwbQb1s/)).toBeInTheDocument();
});
it("shows waiting status", () => {
render(<DepositView checkout={CHECKOUT} status="waiting" onBack={vi.fn()} />);
expect(screen.getByText(/waiting for payment/i)).toBeInTheDocument();
});
it("shows partial status", () => {
render(<DepositView checkout={CHECKOUT} status="partial" onBack={vi.fn()} />);
expect(screen.getByText(/payment partially received/i)).toBeInTheDocument();
});
it("shows expired status", () => {
render(<DepositView checkout={CHECKOUT} status="expired" onBack={vi.fn()} />);
expect(screen.getByText(/payment request expired/i)).toBeInTheDocument();
});
it("shows failed status", () => {
render(<DepositView checkout={CHECKOUT} status="failed" onBack={vi.fn()} />);
expect(screen.getByText(/payment failed/i)).toBeInTheDocument();
});
it("calls onBack when back button is clicked", () => {
const handleBack = vi.fn();
render(<DepositView checkout={CHECKOUT} status="waiting" onBack={handleBack} />);
const backButton = screen.getByRole("button", { name: /←\s*back/i });
fireEvent.click(backButton);
expect(handleBack).toHaveBeenCalledTimes(1);
});
```
The new tests assume:
1. The UI strings are:
- `"payment partially received"` for `status="partial"`,
- `"payment request expired"` for `status="expired"`,
- `"payment failed"` for `status="failed"`.
If `DepositView` uses different copy, adjust the regexes in `getByText` to match the actual text.
2. The back button is rendered as a `<button>` with accessible name containing `"← Back"`. If the arrow or casing differ, tweak the `name` matcher (e.g. `/back/i`), or use `getByText`/`getByRole` with the correct label.
3. If you expose status-specific indicator styles via test IDs or class names (e.g. `data-testid="status-indicator"`), you can extend each status test to assert the correct indicator styling (e.g. `expect(indicator).toHaveClass("status-partial")`).
</issue_to_address>
### Comment 5
<location path="src/__tests__/deposit-view.test.tsx" line_range="26-28" />
<code_context>
+ expect(screen.getByText(/THwbQb1s/)).toBeInTheDocument();
+ });
+
+ it("shows waiting status", () => {
+ render(<DepositView checkout={CHECKOUT} status="waiting" onBack={vi.fn()} />);
+ expect(screen.getByText(/waiting for payment/i)).toBeInTheDocument();
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider testing the countdown timer behavior and its stopping conditions
Because `DepositView` keeps a `timeLeft` countdown while `status === "waiting"`, please add a fake-timer test that:
- Advances timers and asserts the MM:SS display updates correctly.
- Asserts that with `status !== "waiting"`, the timer does not keep decrementing.
This will help catch regressions in countdown logic and interval cleanup.
Suggested implementation:
```typescript
it("shows deposit address and amount", () => {
render(<DepositView checkout={CHECKOUT} status="waiting" onBack={vi.fn()} />);
expect(screen.getByText(/32\.24 TRX/)).toBeInTheDocument();
expect(screen.getByText(/THwbQb1s/)).toBeInTheDocument();
});
it("updates countdown timer while waiting", () => {
vi.useFakeTimers();
render(<DepositView checkout={CHECKOUT} status="waiting" onBack={vi.fn()} />);
const getTimeLeft = () => screen.getByTestId("countdown-timer").textContent;
const initialTime = getTimeLeft();
vi.advanceTimersByTime(5_000);
expect(getTimeLeft()).not.toBe(initialTime);
vi.useRealTimers();
});
it("stops countdown timer when status is no longer waiting", () => {
vi.useFakeTimers();
const { rerender } = render(
<DepositView checkout={CHECKOUT} status="waiting" onBack={vi.fn()} />,
);
const getTimeLeft = () => screen.getByTestId("countdown-timer").textContent;
vi.advanceTimersByTime(5_000);
const timeAfterWaiting = getTimeLeft();
rerender(<DepositView checkout={CHECKOUT} status="completed" onBack={vi.fn()} />);
vi.advanceTimersByTime(5_000);
expect(getTimeLeft()).toBe(timeAfterWaiting);
vi.useRealTimers();
});
vi.mock("qrcode.react", () => ({
```
To make these tests pass you may need to:
1. Ensure `DepositView` renders the countdown with `data-testid="countdown-timer"` (e.g. `<span data-testid="countdown-timer">{mm}:{ss}</span>`).
2. Confirm that the countdown is driven by `setInterval` / `setTimeout` so that `vi.useFakeTimers()` can control it.
3. Verify that `DepositView` stops decrementing `timeLeft` when `status !== "waiting"` (e.g. by clearing the interval in an effect cleanup or when status changes). If the status used for "done" is not `"completed"`, adjust the test’s `status` prop accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+68
to
+77
| const handleMethod = useCallback( | ||
| async (method: SupportedPaymentMethod) => { | ||
| setLoading(true); | ||
| try { | ||
| const result = await createCheckout(method.id, amountUsd); | ||
| setCheckout(result); | ||
| setStatus("waiting"); | ||
| setStep("deposit"); | ||
| } catch { | ||
| // Stay on method step |
There was a problem hiding this comment.
issue (bug_risk): Errors from createCheckout are swallowed, leaving the user without feedback if checkout creation fails.
The empty catch in handleMethod means createCheckout failures (e.g. network/server errors) are completely silent—the user just stays on the method step with no feedback. Consider surfacing an error (local error state or toast), and ensure loading is cleared with an obvious way to retry so failures aren’t invisible.
Comment on lines
+40
to
+42
| function setNum(key: keyof FeaturesConfig, value: string) { | ||
| const n = Number.parseInt(value, 10); | ||
| if (!Number.isNaN(n)) setForm((prev) => ({ ...prev, [key]: n })); |
There was a problem hiding this comment.
suggestion (bug_risk): Numeric onboarding fields cannot be cleared back to an empty/zero state by deleting the input, which may be surprising UX.
In setNum, clearing the input (value === "") leads to Number.parseInt returning NaN, so the state isn’t updated and the previous value remains. Consider handling "" explicitly (e.g., map it to 0 or null), or allow setting the state even when the parsed value is NaN and normalize the value on submit, so users can truly clear the field.
Suggested implementation:
function setStr(key: keyof FeaturesConfig, value: string) {
setForm((prev) => ({ ...prev, [key]: value || null }));
}
function setNum(key: keyof FeaturesConfig, value: string) {
if (value === "") {
// Allow clearing numeric fields by mapping an empty input to null
setForm((prev) => ({ ...prev, [key]: null }));
return;
}
const n = Number.parseInt(value, 10);
if (!Number.isNaN(n)) {
setForm((prev) => ({ ...prev, [key]: n }));
}
}
import { useState } from "react";- Ensure the corresponding numeric fields in
FeaturesConfigare typed asnumber | null(or a compatible union) to accept thenullvalue used when clearing input. - If there is a submit/normalization layer that expects a different cleared value (e.g.,
0instead ofnull), adjust the empty-string mapping insetNumaccordingly.
Comment on lines
+82
to
+86
| it("calls onSelect when a method is clicked", async () => { | ||
| const onSelect = vi.fn(); | ||
| render(<PaymentMethodPicker methods={METHODS} amountUsd={25} onSelect={onSelect} />); | ||
| await userEvent.click(screen.getByText("Bitcoin")); | ||
| expect(onSelect).toHaveBeenCalledWith(METHODS[0]); |
There was a problem hiding this comment.
suggestion (testing): Consider testing the optional onBack behavior when provided
A focused test that renders PaymentMethodPicker with onBack={vi.fn()}, clicks the ← Back button, and asserts the callback is called would help ensure the back navigation remains wired correctly in the multi-step flow.
Comment on lines
+19
to
+28
| describe("DepositView", () => { | ||
| it("shows deposit address and amount", () => { | ||
| render(<DepositView checkout={CHECKOUT} status="waiting" onBack={vi.fn()} />); | ||
| expect(screen.getByText(/32\.24 TRX/)).toBeInTheDocument(); | ||
| expect(screen.getByText(/THwbQb1s/)).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("shows waiting status", () => { | ||
| render(<DepositView checkout={CHECKOUT} status="waiting" onBack={vi.fn()} />); | ||
| expect(screen.getByText(/waiting for payment/i)).toBeInTheDocument(); |
There was a problem hiding this comment.
suggestion (testing): Add coverage for non-waiting statuses and back navigation in DepositView
Current tests only cover the waiting/QR/copy flow. Please also add tests for status="partial", "expired", and "failed" to verify the expected text and indicator styles, and a test that the ← Back button triggers the onBack callback. This will cover all user-visible states and navigation behavior for the deposit step.
Suggested implementation:
describe("DepositView", () => {
it("shows deposit address and amount", () => {
render(<DepositView checkout={CHECKOUT} status="waiting" onBack={vi.fn()} />);
expect(screen.getByText(/32\.24 TRX/)).toBeInTheDocument();
expect(screen.getByText(/THwbQb1s/)).toBeInTheDocument();
});
it("shows waiting status", () => {
render(<DepositView checkout={CHECKOUT} status="waiting" onBack={vi.fn()} />);
expect(screen.getByText(/waiting for payment/i)).toBeInTheDocument();
});
it("shows partial status", () => {
render(<DepositView checkout={CHECKOUT} status="partial" onBack={vi.fn()} />);
expect(screen.getByText(/payment partially received/i)).toBeInTheDocument();
});
it("shows expired status", () => {
render(<DepositView checkout={CHECKOUT} status="expired" onBack={vi.fn()} />);
expect(screen.getByText(/payment request expired/i)).toBeInTheDocument();
});
it("shows failed status", () => {
render(<DepositView checkout={CHECKOUT} status="failed" onBack={vi.fn()} />);
expect(screen.getByText(/payment failed/i)).toBeInTheDocument();
});
it("calls onBack when back button is clicked", () => {
const handleBack = vi.fn();
render(<DepositView checkout={CHECKOUT} status="waiting" onBack={handleBack} />);
const backButton = screen.getByRole("button", { name: /←\s*back/i });
fireEvent.click(backButton);
expect(handleBack).toHaveBeenCalledTimes(1);
});The new tests assume:
- The UI strings are:
"payment partially received"forstatus="partial","payment request expired"forstatus="expired","payment failed"forstatus="failed".
IfDepositViewuses different copy, adjust the regexes ingetByTextto match the actual text.
- The back button is rendered as a
<button>with accessible name containing"← Back". If the arrow or casing differ, tweak thenamematcher (e.g./back/i), or usegetByText/getByRolewith the correct label. - If you expose status-specific indicator styles via test IDs or class names (e.g.
data-testid="status-indicator"), you can extend each status test to assert the correct indicator styling (e.g.expect(indicator).toHaveClass("status-partial")).
Comment on lines
+26
to
+28
| it("shows waiting status", () => { | ||
| render(<DepositView checkout={CHECKOUT} status="waiting" onBack={vi.fn()} />); | ||
| expect(screen.getByText(/waiting for payment/i)).toBeInTheDocument(); |
There was a problem hiding this comment.
suggestion (testing): Consider testing the countdown timer behavior and its stopping conditions
Because DepositView keeps a timeLeft countdown while status === "waiting", please add a fake-timer test that:
- Advances timers and asserts the MM:SS display updates correctly.
- Asserts that with
status !== "waiting", the timer does not keep decrementing.
This will help catch regressions in countdown logic and interval cleanup.
Suggested implementation:
it("shows deposit address and amount", () => {
render(<DepositView checkout={CHECKOUT} status="waiting" onBack={vi.fn()} />);
expect(screen.getByText(/32\.24 TRX/)).toBeInTheDocument();
expect(screen.getByText(/THwbQb1s/)).toBeInTheDocument();
});
it("updates countdown timer while waiting", () => {
vi.useFakeTimers();
render(<DepositView checkout={CHECKOUT} status="waiting" onBack={vi.fn()} />);
const getTimeLeft = () => screen.getByTestId("countdown-timer").textContent;
const initialTime = getTimeLeft();
vi.advanceTimersByTime(5_000);
expect(getTimeLeft()).not.toBe(initialTime);
vi.useRealTimers();
});
it("stops countdown timer when status is no longer waiting", () => {
vi.useFakeTimers();
const { rerender } = render(
<DepositView checkout={CHECKOUT} status="waiting" onBack={vi.fn()} />,
);
const getTimeLeft = () => screen.getByTestId("countdown-timer").textContent;
vi.advanceTimersByTime(5_000);
const timeAfterWaiting = getTimeLeft();
rerender(<DepositView checkout={CHECKOUT} status="completed" onBack={vi.fn()} />);
vi.advanceTimersByTime(5_000);
expect(getTimeLeft()).toBe(timeAfterWaiting);
vi.useRealTimers();
});
vi.mock("qrcode.react", () => ({To make these tests pass you may need to:
- Ensure
DepositViewrenders the countdown withdata-testid="countdown-timer"(e.g.<span data-testid="countdown-timer">{mm}:{ss}</span>). - Confirm that the countdown is driven by
setInterval/setTimeoutso thatvi.useFakeTimers()can control it. - Verify that
DepositViewstops decrementingtimeLeftwhenstatus !== "waiting"(e.g. by clearing the interval in an effect cleanup or when status changes). If the status used for "done" is not"completed", adjust the test’sstatusprop accordingly.
|
@claude Please fix all issues raised by the reviewers on this PR. Feedback available from: Steps:
Do not close or delete the PR. Do not skip issues. If a reviewer flags something, fix it or leave a clear explanation as a PR comment. |
Greptile SummaryThis PR replaces the horizontal-tab crypto checkout with a 4-step state machine ( Key issues found:
Confidence Score: 2/5
|
| Filename | Overview |
|---|---|
| src/components/billing/crypto-checkout.tsx | Core 4-step state machine; has a silent error swallow on checkout creation failure and confirmationsRequired not reset on flow restart |
| src/components/billing/deposit-view.tsx | QR + countdown view; countdown timer is hardcoded to 30 minutes instead of using the actual charge expiry, and uses hardcoded bg-white against the dark-mode-first convention |
| src/components/billing/amount-selector.tsx | Clean preset + custom amount selector with proper validation; no issues found |
| src/components/billing/payment-method-picker.tsx | Search + filter pill picker with good useMemo optimization; amountUsd prop accepted but intentionally unused, which pollutes the public API |
| src/components/billing/confirmation-tracker.tsx | Progress bar + 3-step timeline component; clean implementation, confirmationsRequired stale state is a parent concern |
| src/components/billing/buy-crypto-credits-panel.tsx | Trivial backward-compat re-export of CryptoCheckout as BuyCryptoCreditPanel; no issues |
| package-lock.json | Adds @scure/bip32, @scure/bip39, and @noble/hashes as production dependencies — HD wallet key-derivation libraries that are not visibly used by any new component; likely an accidental inclusion |
| src/app/admin/products/page.tsx | New admin product config page with tabbed Brand/Nav/Features/Fleet/Billing forms; clean structure with proper null-safety defaults for optional sections |
| src/components/admin/products/billing-form.tsx | Admin billing config form (Stripe key, credit price tiers, affiliate/dividend rates); well-structured with controlled inputs and toast feedback |
| src/components/admin/products/nav-editor.tsx | Drag-free nav item editor with move up/down, add, remove, and per-item enabled toggle; clean local state management |
Sequence Diagram
sequenceDiagram
participant U as User
participant CC as CryptoCheckout
participant AS as AmountSelector
participant PMP as PaymentMethodPicker
participant DV as DepositView
participant CT as ConfirmationTracker
participant API as /api
CC->>API: getSupportedPaymentMethods()
API-->>CC: SupportedPaymentMethod[]
U->>AS: select $25 → Continue
AS-->>CC: onSelect(25)
CC->>PMP: render(methods, amountUsd=25)
U->>PMP: pick USDT/Tron
PMP-->>CC: onSelect(method)
CC->>API: createCheckout(method.id, 25)
API-->>CC: CheckoutResult (depositAddress, referenceId…)
CC->>DV: render(checkout, status="waiting")
loop every 5s
CC->>API: getChargeStatus(referenceId)
API-->>CC: {status, amountReceivedCents, confirmations}
alt full amount received
CC->>CT: setStep("confirming"), status="confirming"
else partial received
CC->>DV: status="partial"
else credited
CC->>CT: status="credited", clearInterval
else expired/failed
CC->>DV: status="expired"|"failed", clearInterval
end
end
U->>CT: "Done — buy more credits"
CT-->>CC: handleReset()
CC->>AS: setStep("amount")
Comments Outside Diff (1)
-
package-lock.json, line 20-22 (link)Unexplained BIP32/BIP39 production dependencies
@noble/hashes,@scure/bip32, and@scure/bip39are added as productiondependencies(notdevDependencies). These are HD wallet key-derivation libraries. None of the new billing components (amount-selector,deposit-view,payment-method-picker,confirmation-tracker,crypto-checkout) import or use them.A standard crypto deposit-address checkout flow gets the address from the backend via
createCheckout— there is no need for client-side HD wallet derivation in a UI library.Concerns:
- Bundle size —
@scure/bip32+@scure/bip39+@noble/hashes@2.xadd meaningful bytes to the production bundle. - Security — If a future developer accidentally exposes seed phrases or private keys through these libraries in a client-side context, it would be catastrophic.
- Accidental addition — These may have been pulled in during an exploratory spike and left in
package.jsonunintentionally.
Please confirm whether these packages are actually used anywhere, and if not, remove them from
dependencies.Prompt To Fix With AI
This is a comment left during a code review. Path: package-lock.json Line: 20-22 Comment: **Unexplained BIP32/BIP39 production dependencies** `@noble/hashes`, `@scure/bip32`, and `@scure/bip39` are added as production `dependencies` (not `devDependencies`). These are HD wallet key-derivation libraries. None of the new billing components (`amount-selector`, `deposit-view`, `payment-method-picker`, `confirmation-tracker`, `crypto-checkout`) import or use them. A standard crypto deposit-address checkout flow gets the address from the backend via `createCheckout` — there is no need for client-side HD wallet derivation in a UI library. Concerns: 1. **Bundle size** — `@scure/bip32` + `@scure/bip39` + `@noble/hashes@2.x` add meaningful bytes to the production bundle. 2. **Security** — If a future developer accidentally exposes seed phrases or private keys through these libraries in a client-side context, it would be catastrophic. 3. **Accidental addition** — These may have been pulled in during an exploratory spike and left in `package.json` unintentionally. Please confirm whether these packages are actually used anywhere, and if not, remove them from `dependencies`. How can I resolve this? If you propose a fix, please make it concise.
- Bundle size —
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/components/billing/crypto-checkout.tsx
Line: 76-78
Comment:
**Silent failure on checkout creation**
When `createCheckout` throws, the catch block is completely silent — the loading spinner disappears but no error is surfaced to the user. They'll see the payment picker remain unchanged with no indication of what went wrong, and may assume the app froze or the click didn't register.
```suggestion
} catch (err) {
// Stay on method step — surface the error to the user
console.error("Checkout creation failed", err);
// TODO: surface an error toast or inline error state here
}
```
At minimum, add a toast (`toast.error(...)`) or an inline error state so the user knows the operation failed and can try again.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/components/billing/crypto-checkout.tsx
Line: 85-91
Comment:
**`confirmationsRequired` not reset on flow restart**
`handleReset` zeroes out `confirmations` but never resets `confirmationsRequired`. If the user completes a purchase (e.g. required = 20) and then clicks "Done — buy more credits" to start over, the progress bar and confirmation counter will briefly display stale `confirmationsRequired` data from the previous transaction until the new polling cycle updates it.
```suggestion
const handleReset = useCallback(() => {
setStep("amount");
setCheckout(null);
setStatus("waiting");
setAmountUsd(0);
setConfirmations(0);
setConfirmationsRequired(0);
}, []);
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/components/billing/deposit-view.tsx
Line: 17
Comment:
**Countdown hardcoded to 30 minutes — not tied to actual charge expiry**
`timeLeft` is initialized to `30 * 60` unconditionally regardless of the actual expiry the Payram/backend returns in `checkout`. If a charge actually expires in 15 minutes (or 60), the displayed countdown will be misleading and could cause user confusion or payment failures.
The `CheckoutResult` type from `@/lib/api` should include an expiry timestamp (e.g. `expiresAt: number | string`). Use that to derive the initial `timeLeft`:
```suggestion
const [timeLeft, setTimeLeft] = useState(() => {
if (checkout.expiresAt) {
const msLeft = new Date(checkout.expiresAt).getTime() - Date.now();
return Math.max(0, Math.floor(msLeft / 1000));
}
return 30 * 60; // fallback
});
```
If `CheckoutResult` doesn't yet expose `expiresAt`, that field should be added to the API type and backend response.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/components/billing/deposit-view.tsx
Line: 48-50
Comment:
**Hardcoded `bg-white` violates dark-mode-first convention**
The WOPR codebase uses dark-mode-first design with semantic tokens (`bg-background`, `text-foreground`). Using `bg-white` here will render an abrupt white box in dark-themed deployments.
That said, QR codes require sufficient contrast to scan reliably. A `bg-white` wrapper around the QR is functionally correct — the right fix is to keep the white background but scope it to the QR element only while acknowledging the override:
```suggestion
<div className="mx-auto w-fit rounded-lg bg-white p-3" aria-hidden="true">
{/* bg-white is intentional: QR code scanners require light background */}
<QRCodeSVG value={checkout.depositAddress} size={140} />
</div>
```
Add a comment to document the intentional deviation so future linters or reviewers don't flag it accidentally.
**Rule Used:** WOPR codebase conventions:
- Repository pattern is... ([source](https://app.greptile.com/review/custom-context?memory=103fd9f6-56fa-4ef7-b9bf-dc0cb285f062))
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: package-lock.json
Line: 20-22
Comment:
**Unexplained BIP32/BIP39 production dependencies**
`@noble/hashes`, `@scure/bip32`, and `@scure/bip39` are added as production `dependencies` (not `devDependencies`). These are HD wallet key-derivation libraries. None of the new billing components (`amount-selector`, `deposit-view`, `payment-method-picker`, `confirmation-tracker`, `crypto-checkout`) import or use them.
A standard crypto deposit-address checkout flow gets the address from the backend via `createCheckout` — there is no need for client-side HD wallet derivation in a UI library.
Concerns:
1. **Bundle size** — `@scure/bip32` + `@scure/bip39` + `@noble/hashes@2.x` add meaningful bytes to the production bundle.
2. **Security** — If a future developer accidentally exposes seed phrases or private keys through these libraries in a client-side context, it would be catastrophic.
3. **Accidental addition** — These may have been pulled in during an exploratory spike and left in `package.json` unintentionally.
Please confirm whether these packages are actually used anywhere, and if not, remove them from `dependencies`.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/components/billing/payment-method-picker.tsx
Line: 29-33
Comment:
**`amountUsd` prop accepted but intentionally unused**
The `amountUsd` prop is destructured as `_amountUsd` to suppress lint warnings, meaning it's accepted in the public API but has no effect. This silently discards data that could meaningfully improve UX (e.g. showing "≈ 0.00042 BTC" next to each payment method in the list).
If the feature isn't ready, consider either:
- Removing the prop from `PaymentMethodPickerProps` and the parent until it's needed, or
- Adding a `// TODO: display estimated amount per method` comment to document the intent.
Leaving an unused prop in a public interface adds confusion and unnecessary surface area to the component API.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat: CryptoCheckout parent wires 4-step..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+76
to
+78
| } catch { | ||
| // Stay on method step | ||
| } finally { |
There was a problem hiding this comment.
When createCheckout throws, the catch block is completely silent — the loading spinner disappears but no error is surfaced to the user. They'll see the payment picker remain unchanged with no indication of what went wrong, and may assume the app froze or the click didn't register.
Suggested change
| } catch { | |
| // Stay on method step | |
| } finally { | |
| } catch (err) { | |
| // Stay on method step — surface the error to the user | |
| console.error("Checkout creation failed", err); | |
| // TODO: surface an error toast or inline error state here | |
| } |
At minimum, add a toast (toast.error(...)) or an inline error state so the user knows the operation failed and can try again.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/billing/crypto-checkout.tsx
Line: 76-78
Comment:
**Silent failure on checkout creation**
When `createCheckout` throws, the catch block is completely silent — the loading spinner disappears but no error is surfaced to the user. They'll see the payment picker remain unchanged with no indication of what went wrong, and may assume the app froze or the click didn't register.
```suggestion
} catch (err) {
// Stay on method step — surface the error to the user
console.error("Checkout creation failed", err);
// TODO: surface an error toast or inline error state here
}
```
At minimum, add a toast (`toast.error(...)`) or an inline error state so the user knows the operation failed and can try again.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+85
to
+91
| const handleReset = useCallback(() => { | ||
| setStep("amount"); | ||
| setCheckout(null); | ||
| setStatus("waiting"); | ||
| setAmountUsd(0); | ||
| setConfirmations(0); | ||
| }, []); |
There was a problem hiding this comment.
confirmationsRequired not reset on flow restart
handleReset zeroes out confirmations but never resets confirmationsRequired. If the user completes a purchase (e.g. required = 20) and then clicks "Done — buy more credits" to start over, the progress bar and confirmation counter will briefly display stale confirmationsRequired data from the previous transaction until the new polling cycle updates it.
Suggested change
| const handleReset = useCallback(() => { | |
| setStep("amount"); | |
| setCheckout(null); | |
| setStatus("waiting"); | |
| setAmountUsd(0); | |
| setConfirmations(0); | |
| }, []); | |
| const handleReset = useCallback(() => { | |
| setStep("amount"); | |
| setCheckout(null); | |
| setStatus("waiting"); | |
| setAmountUsd(0); | |
| setConfirmations(0); | |
| setConfirmationsRequired(0); | |
| }, []); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/billing/crypto-checkout.tsx
Line: 85-91
Comment:
**`confirmationsRequired` not reset on flow restart**
`handleReset` zeroes out `confirmations` but never resets `confirmationsRequired`. If the user completes a purchase (e.g. required = 20) and then clicks "Done — buy more credits" to start over, the progress bar and confirmation counter will briefly display stale `confirmationsRequired` data from the previous transaction until the new polling cycle updates it.
```suggestion
const handleReset = useCallback(() => {
setStep("amount");
setCheckout(null);
setStatus("waiting");
setAmountUsd(0);
setConfirmations(0);
setConfirmationsRequired(0);
}, []);
```
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| export function DepositView({ checkout, status, onBack }: DepositViewProps) { | ||
| const [copied, setCopied] = useState(false); | ||
| const [timeLeft, setTimeLeft] = useState(30 * 60); |
There was a problem hiding this comment.
timeLeft is initialized to 30 * 60 unconditionally regardless of the actual expiry the Payram/backend returns in checkout. If a charge actually expires in 15 minutes (or 60), the displayed countdown will be misleading and could cause user confusion or payment failures.
The CheckoutResult type from @/lib/api should include an expiry timestamp (e.g. expiresAt: number | string). Use that to derive the initial timeLeft:
Suggested change
| const [timeLeft, setTimeLeft] = useState(30 * 60); | |
| const [timeLeft, setTimeLeft] = useState(() => { | |
| if (checkout.expiresAt) { | |
| const msLeft = new Date(checkout.expiresAt).getTime() - Date.now(); | |
| return Math.max(0, Math.floor(msLeft / 1000)); | |
| } | |
| return 30 * 60; // fallback | |
| }); |
If CheckoutResult doesn't yet expose expiresAt, that field should be added to the API type and backend response.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/billing/deposit-view.tsx
Line: 17
Comment:
**Countdown hardcoded to 30 minutes — not tied to actual charge expiry**
`timeLeft` is initialized to `30 * 60` unconditionally regardless of the actual expiry the Payram/backend returns in `checkout`. If a charge actually expires in 15 minutes (or 60), the displayed countdown will be misleading and could cause user confusion or payment failures.
The `CheckoutResult` type from `@/lib/api` should include an expiry timestamp (e.g. `expiresAt: number | string`). Use that to derive the initial `timeLeft`:
```suggestion
const [timeLeft, setTimeLeft] = useState(() => {
if (checkout.expiresAt) {
const msLeft = new Date(checkout.expiresAt).getTime() - Date.now();
return Math.max(0, Math.floor(msLeft / 1000));
}
return 30 * 60; // fallback
});
```
If `CheckoutResult` doesn't yet expose `expiresAt`, that field should be added to the API type and backend response.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+48
to
+50
| <div className="mx-auto w-fit rounded-lg bg-white p-3" aria-hidden="true"> | ||
| <QRCodeSVG value={checkout.depositAddress} size={140} /> | ||
| </div> |
There was a problem hiding this comment.
bg-white violates dark-mode-first convention
The WOPR codebase uses dark-mode-first design with semantic tokens (bg-background, text-foreground). Using bg-white here will render an abrupt white box in dark-themed deployments.
That said, QR codes require sufficient contrast to scan reliably. A bg-white wrapper around the QR is functionally correct — the right fix is to keep the white background but scope it to the QR element only while acknowledging the override:
Suggested change
| <div className="mx-auto w-fit rounded-lg bg-white p-3" aria-hidden="true"> | |
| <QRCodeSVG value={checkout.depositAddress} size={140} /> | |
| </div> | |
| <div className="mx-auto w-fit rounded-lg bg-white p-3" aria-hidden="true"> | |
| {/* bg-white is intentional: QR code scanners require light background */} | |
| <QRCodeSVG value={checkout.depositAddress} size={140} /> | |
| </div> |
Add a comment to document the intentional deviation so future linters or reviewers don't flag it accidentally.
Rule Used: WOPR codebase conventions:
- Repository pattern is... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/billing/deposit-view.tsx
Line: 48-50
Comment:
**Hardcoded `bg-white` violates dark-mode-first convention**
The WOPR codebase uses dark-mode-first design with semantic tokens (`bg-background`, `text-foreground`). Using `bg-white` here will render an abrupt white box in dark-themed deployments.
That said, QR codes require sufficient contrast to scan reliably. A `bg-white` wrapper around the QR is functionally correct — the right fix is to keep the white background but scope it to the QR element only while acknowledging the override:
```suggestion
<div className="mx-auto w-fit rounded-lg bg-white p-3" aria-hidden="true">
{/* bg-white is intentional: QR code scanners require light background */}
<QRCodeSVG value={checkout.depositAddress} size={140} />
</div>
```
Add a comment to document the intentional deviation so future linters or reviewers don't flag it accidentally.
**Rule Used:** WOPR codebase conventions:
- Repository pattern is... ([source](https://app.greptile.com/review/custom-context?memory=103fd9f6-56fa-4ef7-b9bf-dc0cb285f062))
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comment on lines
+29
to
+33
| methods, | ||
| amountUsd: _amountUsd, | ||
| onSelect, | ||
| onBack, | ||
| }: PaymentMethodPickerProps) { |
There was a problem hiding this comment.
amountUsd prop accepted but intentionally unused
The amountUsd prop is destructured as _amountUsd to suppress lint warnings, meaning it's accepted in the public API but has no effect. This silently discards data that could meaningfully improve UX (e.g. showing "≈ 0.00042 BTC" next to each payment method in the list).
If the feature isn't ready, consider either:
- Removing the prop from
PaymentMethodPickerPropsand the parent until it's needed, or - Adding a
// TODO: display estimated amount per methodcomment to document the intent.
Leaving an unused prop in a public interface adds confusion and unnecessary surface area to the component API.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/billing/payment-method-picker.tsx
Line: 29-33
Comment:
**`amountUsd` prop accepted but intentionally unused**
The `amountUsd` prop is destructured as `_amountUsd` to suppress lint warnings, meaning it's accepted in the public API but has no effect. This silently discards data that could meaningfully improve UX (e.g. showing "≈ 0.00042 BTC" next to each payment method in the list).
If the feature isn't ready, consider either:
- Removing the prop from `PaymentMethodPickerProps` and the parent until it's needed, or
- Adding a `// TODO: display estimated amount per method` comment to document the intent.
Leaving an unused prop in a public interface adds confusion and unnecessary surface area to the component API.
How can I resolve this? If you propose a fix, please make it concise.
|
@claude Please fix all issues raised by the reviewers on this PR. Feedback available from: Steps:
Do not close or delete the PR. Do not skip issues. If a reviewer flags something, fix it or leave a clear explanation as a PR comment. |
…colors - Remove amountUsd from PaymentMethodPicker (unused, confusing API surface) - Replace bg-white on QR container with bg-background + semantic CSS vars - QR uses hsl(var(--background/--foreground)) for dark mode compatibility Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Comment on lines
+19
to
+23
| const handleCopy = useCallback(() => { | ||
| navigator.clipboard.writeText(checkout.depositAddress); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); | ||
| }, [checkout.depositAddress]); |
There was a problem hiding this comment.
🟢 Low billing/deposit-view.tsx:19
navigator.clipboard.writeText() is not awaited, so setCopied(true) executes immediately even if the clipboard operation fails. Additionally, navigator.clipboard may be undefined in insecure contexts, causing a crash. Consider awaiting the Promise and checking for API availability before calling it.
const handleCopy = useCallback(() => {
- navigator.clipboard.writeText(checkout.depositAddress);
- setCopied(true);
- setTimeout(() => setCopied(false), 2000);
+ if (!navigator.clipboard) return;
+ navigator.clipboard.writeText(checkout.depositAddress).then(() => {
+ setCopied(true);
+ setTimeout(() => setCopied(false), 2000);
+ });
}, [checkout.depositAddress]);🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file src/components/billing/deposit-view.tsx around lines 19-23:
`navigator.clipboard.writeText()` is not awaited, so `setCopied(true)` executes immediately even if the clipboard operation fails. Additionally, `navigator.clipboard` may be `undefined` in insecure contexts, causing a crash. Consider awaiting the Promise and checking for API availability before calling it.
Evidence trail:
src/components/billing/deposit-view.tsx lines 19-23 at REVIEWED_COMMIT: `handleCopy` callback calls `navigator.clipboard.writeText()` without await/then and sets `setCopied(true)` synchronously. No check for `navigator.clipboard` existence. MDN Web Docs confirms Clipboard API requires secure context: https://developer.mozilla.org/en-US/docs/Web/API/Clipboard
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BuyCryptoCreditPanelre-exported for backward compatibilityComponents
AmountSelector— preset ($10-$100) + custom inputPaymentMethodPicker— search + filter pills over dynamic method listDepositView— QR code, address copy, live countdown, status indicatorConfirmationTracker— progress bar, 3-step timeline, auto-redirectCryptoCheckout— parent step state machineTest plan
🤖 Generated with Claude Code
Summary by Sourcery
Redesign the crypto credits checkout into a multi-step, extensible flow and introduce an admin UI for managing product branding, navigation, features, fleet, and billing configuration.
New Features:
Enhancements:
Tests:
Note
Add 4-step crypto checkout flow with amount selection, payment method search, QR deposit, and confirmation tracking
BuyCryptoCreditPanelwith a newCryptoCheckoutcomponent that guides users through amount selection → payment method → deposit instructions → confirmation.AmountSelectoroffers preset amounts ($10–$100) and a custom input;PaymentMethodPickerprovides searchable, filterable crypto method selection.DepositViewshows a QR code, copy-to-clipboard address, and a countdown timer;ConfirmationTrackerpolls charge status and displays a progress bar through detection, confirming, and credited states.BuyCryptoCreditPanelnow renders the multi-step checkout UI instead of its previous inline implementation.📊 Macroscope summarized 45567cf. 3 files reviewed, 2 issues evaluated, 0 issues filtered, 1 comment posted
🗂️ Filtered Issues