-
Notifications
You must be signed in to change notification settings - Fork 1
Add Buy Credits modal (#100) #101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces a new BuyCreditsModal component that enables users to purchase credits through a dialog interface. The modal includes credit amount input, tax calculation at 18%, and a purchase button. The component is integrated into the tokens-usage component, replacing an upgrade button with a buy credits button that triggers the modal. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/platform/components/buy-credits-modal.tsx`:
- Around line 34-35: The close button in buy-credits-modal.tsx is rendered empty
and lacks an accessible name; update the button that calls onOpenChange(false)
to include a visible label and accessible name (e.g., add text "Close" or a
visible icon plus a visually hidden "Close" text), or set aria-label="Close" on
the button; if you use an icon component make it aria-hidden and include the
screen-reader-only text so screen readers get the control name while sighted
users see the icon.
- Around line 40-47: The label for the Credits input in BuyCreditsModal is not
associated with the input, harming accessibility; add an id (e.g.,
"credits-input") to the Input component and set the label's htmlFor to that id
(ensure the Input component accepts and forwards the id prop so the native input
receives it) so clicking the label focuses the number input.
- Around line 41-47: The numeric input handler for the Input component currently
sets credits directly from e.target.value which can become NaN or outside the
allowed [5,100] range; update the onChange (or extract to a handler used by
Input) to parse the value, guard against NaN, clamp it to the min (5) and max
(100), and then call setCredits with the validated/clamped number so credits and
any derived totals remain valid (refer to the Input component usage and the
credits and setCredits state variables).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/platform/components/buy-credits-modal.tsxapps/platform/components/tokens-usage.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/platform/components/tokens-usage.tsx (2)
packages/ui/src/components/card.tsx (4)
CardHeader(86-86)CardTitle(88-88)CardDescription(90-90)CardContent(91-91)apps/platform/components/buy-credits-modal.tsx (1)
BuyCreditsModal(15-80)
🔇 Additional comments (2)
apps/platform/components/buy-credits-modal.tsx (1)
70-76: CTA/terms copy looks solid.apps/platform/components/tokens-usage.tsx (1)
14-57: Clean modal wiring and CTA integration.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| <button onClick={() => onOpenChange(false)}> | ||
| </button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a visible, accessible close control.
The close button renders with no content or accessible name, so it’s effectively invisible to users and screen readers.
🩹 Proposed fix
- <button onClick={() => onOpenChange(false)}>
- </button>
+ <button
+ type="button"
+ aria-label="Close"
+ onClick={() => onOpenChange(false)}
+ >
+ ×
+ </button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button onClick={() => onOpenChange(false)}> | |
| </button> | |
| <button | |
| type="button" | |
| aria-label="Close" | |
| onClick={() => onOpenChange(false)} | |
| > | |
| × | |
| </button> |
🤖 Prompt for AI Agents
In `@apps/platform/components/buy-credits-modal.tsx` around lines 34 - 35, The
close button in buy-credits-modal.tsx is rendered empty and lacks an accessible
name; update the button that calls onOpenChange(false) to include a visible
label and accessible name (e.g., add text "Close" or a visible icon plus a
visually hidden "Close" text), or set aria-label="Close" on the button; if you
use an icon component make it aria-hidden and include the screen-reader-only
text so screen readers get the control name while sighted users see the icon.
| <label className="text-sm font-medium">Credits</label> | ||
| <Input | ||
| type="number" | ||
| min={5} | ||
| max={100} | ||
| value={credits} | ||
| onChange={(e) => setCredits(Number(e.target.value))} | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Associate the Credits label with the input.
The label isn’t wired to the input, which harms accessibility.
🩹 Proposed fix
- <label className="text-sm font-medium">Credits</label>
+ <label htmlFor="buy-credits-amount" className="text-sm font-medium">
+ Credits
+ </label>
<Input
+ id="buy-credits-amount"
type="number"
min={5}
max={100}
value={credits}
onChange={(e) => setCredits(Number(e.target.value))}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <label className="text-sm font-medium">Credits</label> | |
| <Input | |
| type="number" | |
| min={5} | |
| max={100} | |
| value={credits} | |
| onChange={(e) => setCredits(Number(e.target.value))} | |
| /> | |
| <label htmlFor="buy-credits-amount" className="text-sm font-medium"> | |
| Credits | |
| </label> | |
| <Input | |
| id="buy-credits-amount" | |
| type="number" | |
| min={5} | |
| max={100} | |
| value={credits} | |
| onChange={(e) => setCredits(Number(e.target.value))} | |
| /> |
🤖 Prompt for AI Agents
In `@apps/platform/components/buy-credits-modal.tsx` around lines 40 - 47, The
label for the Credits input in BuyCreditsModal is not associated with the input,
harming accessibility; add an id (e.g., "credits-input") to the Input component
and set the label's htmlFor to that id (ensure the Input component accepts and
forwards the id prop so the native input receives it) so clicking the label
focuses the number input.
| <Input | ||
| type="number" | ||
| min={5} | ||
| max={100} | ||
| value={credits} | ||
| onChange={(e) => setCredits(Number(e.target.value))} | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clamp and validate credit input to the allowed range.
Min/max on <input type="number"> doesn’t prevent out-of-range or NaN values from entering state, so totals can become invalid.
🧮 Proposed fix
- const [credits, setCredits] = useState(25)
+ const MIN_CREDITS = 5
+ const MAX_CREDITS = 100
+ const [credits, setCredits] = useState(25)
+ const clampCredits = (value: number) =>
+ Math.min(MAX_CREDITS, Math.max(MIN_CREDITS, value))
...
<Input
type="number"
- min={5}
- max={100}
+ min={MIN_CREDITS}
+ max={MAX_CREDITS}
value={credits}
- onChange={(e) => setCredits(Number(e.target.value))}
+ onChange={(e) => {
+ const next = e.currentTarget.valueAsNumber
+ if (Number.isNaN(next)) return
+ setCredits(clampCredits(next))
+ }}
/>🤖 Prompt for AI Agents
In `@apps/platform/components/buy-credits-modal.tsx` around lines 41 - 47, The
numeric input handler for the Input component currently sets credits directly
from e.target.value which can become NaN or outside the allowed [5,100] range;
update the onChange (or extract to a handler used by Input) to parse the value,
guard against NaN, clamp it to the min (5) and max (100), and then call
setCredits with the validated/clamped number so credits and any derived totals
remain valid (refer to the Input component usage and the credits and setCredits
state variables).
Fixes #100
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.