feat: add dark mode #105
feat: add dark mode #105
Conversation
WalkthroughAdds ThemeProvider (next-themes) and suppressHydrationWarning to the app shell, introduces a client ModeToggle, makes BackgroundWrapper dark-mode aware and mount-safe, replaces many hardcoded Tailwind color utilities with semantic design tokens across UI components, adds a copy-email action in user-menu, and swaps a dashboard "Create Invoice" Link to use Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as RootLayout
participant Theme as ThemeProvider
participant Header as Header
participant Toggle as ModeToggle
participant BG as BackgroundWrapper
participant UI as App Components
User->>App: Request page
App->>Theme: Render children inside ThemeProvider
Theme->>Header: Provide theme context
Header->>Toggle: Render ModeToggle
App->>BG: Render BackgroundWrapper (uses theme + mounted flag)
Note over Toggle,Theme: Toggle triggers setTheme(light|dark)
Toggle->>Theme: setTheme(...)
Theme->>UI: Theme change causes re-render using semantic tokens
Note over BG,UI: Background decorations hidden/shown based on isDark + isMounted
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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. Tip 🧪 Early access (models): enabledWe are currently testing Sonnet 4.5 code review models, which should lead to better review quality. However, this model may result in higher noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
|
Hello @mayur1377, thank you for submitting your first pull request to the easy-invoice repository. We value your contribution and encourage you to review our contribution guidelines to ensure your submission meets our standards. Please note that every merged PR is automatically enrolled in our Best PR Initiative, offering a chance to win $500 each quarter. Our team is available via GitHub Discussions or Discord if you have any questions. Welcome aboard! |
|
Thank you for your submission! As you prepare for the review process, please ensure that your PR title, description, and any linked issues fully comply with our contribution guidelines. A clear explanation of your changes and their context will help expedite the review process. Every merged PR is automatically entered into our Best PR Initiative, offering a chance to win $500 every quarter. We appreciate your attention to detail and look forward to reviewing your contribution! |
![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 (7)
src/components/short-address.tsx (1)
25-30: Improve accessibility with an explicit labelScreen-reader users cannot rely on the visual tooltip; add an
aria-labelso the button’s purpose is announced even without hover/focus.- <button - type="button" - onClick={handleCopy} - className={`text-muted-foreground hover:text-foreground transition-colors flex items-center gap-2 ${className}`} + <button + type="button" + aria-label="Copy address to clipboard" + onClick={handleCopy} + className={`text-muted-foreground hover:text-foreground transition-colors flex items-center gap-2 ${className}`}src/components/subscription-plans/blocks/create-subscription-plan.tsx (1)
281-289: Minor UX nit – keep disabled state truly muted
WhenisLoadingis true the button is disabled, yet thehover:text-foregroundclass still applies and can visually “brighten” the label on hover (HTML disables click events, but the hover style still triggers).
Consider guarding the hover style so it does not activate while disabled:-className="flex items-center text-sm text-muted-foreground hover:text-foreground transition-colors" +className="flex items-center text-sm text-muted-foreground transition-colors + hover:text-foreground disabled:hover:text-muted-foreground"src/components/view-recurring-payments/blocks/status-badge.tsx (1)
7-11: Inconsistent background opacity for “paused” badge
All other statuses use thecolor/10translucent background pattern, butpausedfalls back tobg-muted(opaque). For visual consistency you might want:- paused: "bg-muted text-muted-foreground border-border", + paused: "bg-muted/10 text-muted-foreground border-border",(if your tailwind config exposes a muted/10 token).
src/components/view-recurring-payments/view-recurring-payments.tsx (1)
82-85: Prefer token without embedded opacity for destructive borders
border-destructive/30mixes semantic token + hard-coded opacity. Consider a pure token (e.g.border-destructive) and control transparency through CSS variables or the design-system’s.opacity-*utilities to stay fully theme-driven.src/components/dashboard/invoices-sent.tsx (1)
95-98: Use the Button’svariantAPI instead of hard-coding colorsManually setting
bg-primary,hover:bg-primary/80, andtext-primary-foregroundbypasses the button component’s variant system and may diverge if the theme palette changes.-<Button className="bg-primary hover:bg-primary/80 text-primary-foreground"> +<Button variant="default">Keeps styling consistent and centralised.
src/components/payment-route.tsx (1)
129-133: Replace hard-codedtext-purple-600with a theme tokenThis is the last hard-coded colour in the component. Using something like
text-primary-foreground(or a dedicated “brand” token) keeps dark-mode contrast predictable.-<span className="text-purple-600"> +<span className="text-primary-foreground">src/components/background-wrapper.tsx (1)
67-87: Consider dark mode decorative elements.The decorative gradient elements are completely hidden in dark mode, which might make the dark theme feel visually empty compared to the light theme.
Consider adding subtle dark mode decoratives:
- {/* Decorative elements - only show in light mode */} - {!isDark && ( + {/* Decorative elements */} + {isDark ? ( + <> + <div className="absolute top-0 right-0 w-[600px] h-[600px] -translate-y-1/2 translate-x-1/2"> + <div + className="w-full h-full rounded-full opacity-10 blur-3xl" + style={{ + background: `linear-gradient(to bottom right, ${getTailwindColor("slate-800")}, ${getTailwindColor("slate-900")})`, + }} + /> + </div> + <div className="absolute bottom-0 left-0 w-[600px] h-[600px] translate-y-1/2 -translate-x-1/2"> + <div + className="w-full h-full rounded-full opacity-10 blur-3xl" + style={{ + background: `linear-gradient(to top right, ${getTailwindColor("zinc-800")}, ${getTailwindColor("zinc-900")})`, + }} + /> + </div> + </> + ) : ( <> <div className="absolute top-0 right-0 w-[600px] h-[600px] -translate-y-1/2 translate-x-1/2"> <div className="w-full h-full rounded-full opacity-30 blur-3xl" style={{ background: `linear-gradient(to bottom right, ${getTailwindColor(topGradient.from)}, ${getTailwindColor(topGradient.to)})`, }} /> </div> <div className="absolute bottom-0 left-0 w-[600px] h-[600px] translate-y-1/2 -translate-x-1/2"> <div className="w-full h-full rounded-full opacity-30 blur-3xl" style={{ background: `linear-gradient(to top right, ${getTailwindColor(bottomGradient.from)}, ${getTailwindColor(bottomGradient.to)})`, }} /> </div> </> - )} + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (38)
src/app/dashboard/layout.tsx(2 hunks)src/app/i/[id]/page.tsx(1 hunks)src/app/invoices/create/page.tsx(1 hunks)src/app/layout.tsx(2 hunks)src/components/background-wrapper.tsx(3 hunks)src/components/batch-payout.tsx(17 hunks)src/components/compliance-status.tsx(1 hunks)src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx(2 hunks)src/components/create-recurring-payment/create-recurring-payment.tsx(5 hunks)src/components/dashboard/blocks/empty-state.tsx(1 hunks)src/components/dashboard/blocks/invoice-row.tsx(5 hunks)src/components/dashboard/blocks/pagination.tsx(2 hunks)src/components/dashboard/blocks/stat-card.tsx(1 hunks)src/components/dashboard/blocks/table-head-cell.tsx(1 hunks)src/components/dashboard/invoices-received.tsx(6 hunks)src/components/dashboard/invoices-sent.tsx(2 hunks)src/components/dashboard/subscriptions.tsx(4 hunks)src/components/direct-payout.tsx(10 hunks)src/components/footer.tsx(1 hunks)src/components/header.tsx(4 hunks)src/components/invoice-form.tsx(2 hunks)src/components/invoice-me-link.tsx(1 hunks)src/components/invoice-me-links.tsx(3 hunks)src/components/invoice-preview.tsx(7 hunks)src/components/mode-toggle.tsx(1 hunks)src/components/payment-route.tsx(4 hunks)src/components/payment-section.tsx(10 hunks)src/components/payment-secured-using-request.tsx(1 hunks)src/components/short-address.tsx(1 hunks)src/components/subscription-plan-preview.tsx(5 hunks)src/components/subscription-plans/blocks/create-subscription-plan.tsx(1 hunks)src/components/subscription-plans/blocks/subscription-plan-link.tsx(3 hunks)src/components/subscription-plans/subscription-plans.tsx(2 hunks)src/components/ui/tooltip.tsx(1 hunks)src/components/view-recurring-payments/blocks/completed-payments.tsx(1 hunks)src/components/view-recurring-payments/blocks/frequency-badge.tsx(1 hunks)src/components/view-recurring-payments/blocks/status-badge.tsx(1 hunks)src/components/view-recurring-payments/view-recurring-payments.tsx(8 hunks)
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#21
File: src/components/background-wrapper.tsx:31-31
Timestamp: 2025-02-20T10:26:33.809Z
Learning: Dynamic Tailwind class interpolation for gradient classes (from-${color} and to-${color}) in the BackgroundWrapper component is intentionally implemented this way and doesn't require a mapping object solution.
📚 Learning: dynamic tailwind class interpolation for gradient classes (from-${color} and to-${color}) in the bac...
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#21
File: src/components/background-wrapper.tsx:31-31
Timestamp: 2025-02-20T10:26:33.809Z
Learning: Dynamic Tailwind class interpolation for gradient classes (from-${color} and to-${color}) in the BackgroundWrapper component is intentionally implemented this way and doesn't require a mapping object solution.
Applied to files:
src/app/i/[id]/page.tsxsrc/components/view-recurring-payments/blocks/status-badge.tsxsrc/components/background-wrapper.tsx
📚 Learning: the `payrequest` function in the invoice router retrieves payment transactions calldata from the req...
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#2
File: src/server/routers/invoice.ts:0-0
Timestamp: 2025-02-12T13:04:46.135Z
Learning: The `payRequest` function in the invoice router retrieves payment transactions calldata from the Request API and does not process the payment itself. The actual payment status update happens through the webhook handler.
Applied to files:
src/components/payment-secured-using-request.tsx
📚 Learning: the handleformsubmit function in src/components/invoice-form.tsx correctly uses data.clientemail fro...
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/components/invoice-form.tsx:316-319
Timestamp: 2025-05-19T13:00:48.790Z
Learning: The handleFormSubmit function in src/components/invoice-form.tsx correctly uses data.clientEmail from the form submission data to find matching payers, which is the proper implementation.
Applied to files:
src/app/dashboard/layout.tsxsrc/components/create-recurring-payment/blocks/create-recurring-payment-form.tsxsrc/components/payment-section.tsxsrc/components/direct-payout.tsxsrc/components/dashboard/invoices-sent.tsxsrc/components/invoice-form.tsxsrc/components/invoice-preview.tsxsrc/components/dashboard/invoices-received.tsxsrc/components/dashboard/blocks/invoice-row.tsx
📚 Learning: the appkit wallet module requires initialization time before it's ready for use. a 2-second delay is...
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#23
File: src/components/direct-payment.tsx:63-89
Timestamp: 2025-02-27T13:58:33.590Z
Learning: The AppKit wallet module requires initialization time before it's ready for use. A 2-second delay is intentionally used in the DirectPayment component to ensure the wallet is fully initialized before allowing interactions.
Applied to files:
src/components/subscription-plan-preview.tsxsrc/components/create-recurring-payment/create-recurring-payment.tsxsrc/components/direct-payout.tsxsrc/components/batch-payout.tsx
📚 Learning: in src/components/batch-payout.tsx, the user prefers to keep the simple 2-second timeout for appkit ...
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#64
File: src/components/batch-payout.tsx:100-106
Timestamp: 2025-06-04T10:08:40.123Z
Learning: In src/components/batch-payout.tsx, the user prefers to keep the simple 2-second timeout for AppKit initialization over more complex polling mechanisms when the current approach is working adequately. They favor simplicity over potentially more robust but complex solutions.
Applied to files:
src/components/subscription-plan-preview.tsxsrc/components/create-recurring-payment/create-recurring-payment.tsxsrc/components/direct-payout.tsxsrc/components/batch-payout.tsx
📚 Learning: the easy-invoice project prefers simpler, direct implementations over abstract utilities. for exampl...
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#21
File: src/app/invoices/[ID]/page.tsx:113-148
Timestamp: 2025-02-20T10:27:02.993Z
Learning: The easy-invoice project prefers simpler, direct implementations over abstract utilities. For example, using `.toFixed(2)` directly instead of creating separate number formatting utilities.
Applied to files:
src/components/payment-section.tsxsrc/components/invoice-preview.tsxsrc/components/dashboard/blocks/invoice-row.tsx
📚 Learning: in the requestnetwork/easy-invoice codebase, monetary amounts are stored as text in the database sch...
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#91
File: drizzle/0007_messy_silver_fox.sql:1-12
Timestamp: 2025-07-14T14:17:05.340Z
Learning: In the RequestNetwork/easy-invoice codebase, monetary amounts are stored as text in the database schema rather than numeric types. This is a deliberate architectural decision to maintain consistency across all amount fields in the system.
Applied to files:
src/components/payment-section.tsx
📚 Learning: in the paymentsection component of easyinvoice, selectedroute is automatically set to the first rout...
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#42
File: src/components/payment-section.tsx:534-544
Timestamp: 2025-03-25T09:31:47.519Z
Learning: In the PaymentSection component of EasyInvoice, selectedRoute is automatically set to the first route when payment routes are loaded, and users can only switch between routes, not deselect them entirely.
Applied to files:
src/components/payment-section.tsxsrc/components/payment-route.tsxsrc/components/batch-payout.tsx
📚 Learning: for the directpayment component, amount validation is already handled through zod schema validation ...
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#23
File: src/components/direct-payment.tsx:343-360
Timestamp: 2025-02-27T14:02:35.047Z
Learning: For the DirectPayment component, amount validation is already handled through Zod schema validation in paymentFormSchema, which ensures the amount is greater than zero.
Applied to files:
src/components/direct-payout.tsx
📚 Learning: in the easy invoice project, settimeout is required when submitting a form after modal state changes...
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/components/invoice-form.tsx:451-453
Timestamp: 2025-05-20T12:59:44.665Z
Learning: In the Easy Invoice project, setTimeout is required when submitting a form after modal state changes in the crypto-to-fiat payment flow. Directly calling handleFormSubmit without setTimeout after closing modals and updating state causes issues.
Applied to files:
src/components/direct-payout.tsxsrc/components/invoice-form.tsx
📚 Learning: for webhook handlers in the easy invoice project, unknown or unexpected substatus values in payment ...
Learnt from: MantisClone
PR: RequestNetwork/easy-invoice#59
File: src/app/api/webhook/route.ts:77-95
Timestamp: 2025-05-22T18:19:12.366Z
Learning: For webhook handlers in the Easy Invoice project, unknown or unexpected subStatus values in payment processing should be treated as errors (using console.error) rather than warnings, and should return a 422 Unprocessable Entity status code.
Applied to files:
src/components/direct-payout.tsx
📚 Learning: the invoice me links in the easy-invoice project do not have an expiration mechanism. the invoicemet...
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#8
File: src/app/i/[id]/page.tsx:18-22
Timestamp: 2025-02-13T21:57:39.869Z
Learning: The invoice me links in the easy-invoice project do not have an expiration mechanism. The invoiceMeTable schema only includes id, label, userId, and createdAt fields.
Applied to files:
src/components/dashboard/invoices-sent.tsxsrc/components/dashboard/invoices-received.tsx
📚 Learning: in `src/server/routers/payment.ts`, the batchpay input validation already handles empty arrays corre...
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#67
File: src/server/routers/payment.ts:47-49
Timestamp: 2025-06-04T12:02:39.411Z
Learning: In `src/server/routers/payment.ts`, the batchPay input validation already handles empty arrays correctly. The `batchPaymentFormSchema.shape.payouts.optional()` inherits the `.min(1, "At least one payment is required")` validation from the original schema, so empty payouts arrays are automatically rejected even when the field is made optional.
Applied to files:
src/components/batch-payout.tsx
📚 Learning: invoice items are validated at creation time through the schema validation in src/lib/schemas/invoic...
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#2
File: src/app/invoices/[ID]/page.tsx:160-175
Timestamp: 2025-02-12T12:42:40.076Z
Learning: Invoice items are validated at creation time through the schema validation in src/lib/schemas/invoice.ts, ensuring that items array is non-empty and each item has required description, quantity, and price fields. Runtime validation in display components is not necessary.
Applied to files:
src/components/invoice-preview.tsx
🧬 Code Graph Analysis (9)
src/components/dashboard/blocks/table-head-cell.tsx (2)
src/components/ui/table.tsx (1)
TableHead(113-113)src/lib/utils.ts (1)
cn(4-6)
src/components/view-recurring-payments/view-recurring-payments.tsx (2)
src/components/ui/card.tsx (4)
Card(79-79)CardContent(79-79)CardHeader(79-79)CardTitle(79-79)src/components/ui/table.tsx (1)
TableCell(115-115)
src/app/layout.tsx (5)
src/components/app-kit.tsx (1)
AppKit(40-42)src/trpc/react.tsx (1)
TRPCReactProvider(14-60)src/components/background-wrapper.tsx (1)
BackgroundWrapper(17-104)src/components/ui/sonner.tsx (1)
Toaster(31-31)src/components/version-badge.tsx (1)
VersionDisplay(9-19)
src/components/payment-section.tsx (1)
src/lib/constants/currencies.ts (2)
MAINNET_CURRENCIES(2-18)formatCurrencyLabel(94-139)
src/components/header.tsx (1)
src/components/mode-toggle.tsx (1)
ModeToggle(6-40)
src/components/mode-toggle.tsx (1)
src/components/ui/button.tsx (1)
Button(56-56)
src/components/invoice-preview.tsx (2)
src/components/ui/card.tsx (2)
Card(79-79)CardContent(79-79)src/lib/date-utils.ts (1)
formatDate(9-17)
src/components/dashboard/invoices-received.tsx (3)
src/components/dashboard/blocks/table-head-cell.tsx (1)
TableHeadCell(9-13)src/components/dashboard/blocks/stat-card.tsx (1)
StatCard(9-21)src/components/ui/card.tsx (1)
Card(79-79)
src/components/subscription-plans/blocks/subscription-plan-link.tsx (2)
src/components/ui/card.tsx (2)
Card(79-79)CardContent(79-79)src/components/ui/button.tsx (1)
Button(56-56)
🔇 Additional comments (61)
src/components/view-recurring-payments/blocks/completed-payments.tsx (1)
17-19: Adopts semantic token – looks goodSwapping
text-zinc-500fortext-muted-foregroundaligns the component with the new theme system and keeps contrast appropriate in both light / dark modes.src/components/footer.tsx (1)
5-7: Consistent theming – no issues
text-muted-foregroundprovides the right semantic colour for footer copy and matches tokens used elsewhere in the PR.src/components/dashboard/blocks/pagination.tsx (2)
22-24: Verify design tokens exist in Tailwind configThe new
border-bordertoken assumes a corresponding entry in the theme’scolorsmap; if it’s missing, the border will be transparent. Please confirm the token is defined (light & dark).
32-33: Semantic text colour – looks good
text-muted-foregroundis the right choice for the page indicator; contrast remains acceptable in both themes.src/components/ui/tooltip.tsx (1)
16-22: Theme tokens applied correctly
bg-popover,text-popover-foreground, andfill-popovermatch the token naming used elsewhere; no functional impact detected.src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx (2)
193-193: LGTM! Correct semantic color token usage.The replacement of
text-zinc-500withtext-muted-foregroundis appropriate for help text, ensuring proper contrast in both light and dark themes.
279-279: LGTM! Proper theme-aware button styling.The replacement of
text-zinc-500 hover:text-zinc-800withtext-muted-foreground hover:text-foregroundcorrectly implements semantic color tokens for both normal and hover states, ensuring proper theming across light and dark modes.src/app/i/[id]/page.tsx (1)
40-40: LGTM! Consistent navigation link theming.The replacement of
text-zinc-600 hover:text-blackwithtext-muted-foreground hover:text-foregroundproperly implements semantic color tokens for the back navigation link, ensuring consistent theming across light and dark modes.src/components/dashboard/blocks/table-head-cell.tsx (1)
10-10: LGTM! Appropriate semantic token for table headers.The replacement of
text-zinc-500withtext-muted-foregroundis ideal for table headers, providing appropriate visual hierarchy while ensuring proper contrast across light and dark themes.src/app/invoices/create/page.tsx (1)
30-30: LGTM! Consistent with navigation theming pattern.The color token replacement follows the same pattern as other navigation links in the codebase, ensuring consistent theming for back navigation elements across the application.
src/app/dashboard/layout.tsx (1)
5-5: LGTM! Improved UI consistency with design system.The replacement of a custom-styled Link with a Button component using
asChildimproves consistency with the design system while maintaining the same functionality. This approach provides better theme integration and standardized button styling across the application.Also applies to: 28-33
src/components/dashboard/blocks/empty-state.tsx (1)
17-21: Semantic token migration looks good
bg-muted,text-foreground, andtext-muted-foregroundcorrectly replace the previous hard-coded zinc shades and align with the new theme system. No functional impact detected.src/components/payment-secured-using-request.tsx (1)
3-8: Consistent success palette usage confirmed
Usingbg-success/10,border-success, and the corresponding foreground tokens keeps the component aligned with the new design-token palette. No issues found.src/components/view-recurring-payments/blocks/frequency-badge.tsx (1)
7-10: LGTM – badges now leverage theme tokens
The frequency badge palette mirrors the status-badge approach and cleanly adopts semantic tokens. No functional or visual concerns noted.src/components/subscription-plans/subscription-plans.tsx (1)
35-35: LGTM! Consistent theming implementation.The color class replacements correctly follow the semantic theming pattern - using
text-muted-foregroundfor secondary elements andbg-primarywithtext-primary-foregroundfor primary actions. The hover states are properly maintained.Also applies to: 45-45
src/components/dashboard/subscriptions.tsx (1)
39-39: Excellent theming consistency across the component.All color replacements follow the semantic token pattern correctly:
- Table hover states use
hover:bg-muted/50for subtle interactions- Icons consistently use
text-muted-foregroundfor secondary visual elements- Card borders use the semantic
border-bordertokenAlso applies to: 85-85, 179-179, 184-184, 189-189, 193-193, 204-206
src/components/subscription-plan-preview.tsx (1)
120-120: Comprehensive theming implementation across all text elements.The color replacements consistently follow the semantic token pattern:
- Secondary labels and descriptive text use
text-muted-foreground- Interactive elements maintain proper hover states with
hover:text-foreground- Loading states and instructions appropriately use muted styling
Also applies to: 141-143, 151-153, 159-161, 169-171, 179-181, 187-189, 252-252, 283-283, 294-297
src/components/invoice-form.tsx (1)
590-590: Proper theming implementation in modal and form elements.The color updates correctly use semantic tokens:
- Spinner border uses
border-foregroundfor primary visual elements- Modal text uses
text-muted-foregroundfor secondary content- Submit button follows the primary color scheme pattern with proper hover states
Also applies to: 591-591, 595-595, 1039-1039
src/components/dashboard/blocks/stat-card.tsx (1)
10-10: Perfect implementation of card theming pattern.The semantic token usage is exactly right:
bg-card,border-border, andtext-card-foregroundprovide proper theme support for the card containertext-muted-foregroundappropriately styles the secondary title textThis follows the established theming pattern consistently.
Also applies to: 12-12
src/components/invoice-me-link.tsx (1)
49-76: LGTM! Consistent theming implementation.The styling updates properly replace hardcoded zinc colors with semantic theme tokens (
text-foreground,text-muted-foreground,bg-muted,hover:bg-muted). The destructive delete button correctly maintains its red styling for visual clarity.src/app/layout.tsx (3)
12-12: LGTM! Proper ThemeProvider import.
36-36: LGTM! Correct hydration warning suppression.The
suppressHydrationWarningattribute prevents React hydration mismatches that occur during theme initialization, which is a standard practice when implementing themes.
41-51: LGTM! Excellent ThemeProvider configuration.The ThemeProvider is properly configured with:
attribute="class"for CSS class-based themingdefaultTheme="system"to respect user's system preferenceenableSystemto detect system theme changes- Correct nesting with other providers maintains the component hierarchy
src/components/dashboard/blocks/invoice-row.tsx (1)
52-119: LGTM! Comprehensive and consistent theming updates.All styling changes properly implement the semantic token approach:
- Row hover states use
bg-muted/50for subtle interaction feedback- Secondary text elements (recurrence, email, dates) consistently use
text-muted-foreground- Action buttons use semantic tokens (
bg-card,border-border,hover:bg-muted)- Icons appropriately use
text-muted-foregroundfor consistent visual hierarchysrc/components/dashboard/invoices-received.tsx (4)
38-47: LGTM! Consistent table header styling.Table header cells properly use
text-muted-foregroundfor secondary text hierarchy.
190-211: LGTM! Well-designed selection summary and CTA.The selected invoices summary box uses appropriate muted styling (
bg-muted,border-border), while the batch pay button correctly uses primary colors to emphasize the call-to-action.
219-229: LGTM! Consistent stat card icon styling.All stat card icons consistently use
text-muted-foregroundfor proper visual hierarchy.
233-246: LGTM! Proper card and empty state theming.The table card uses semantic border tokens, and the empty state icon is appropriately styled with muted foreground color.
src/components/invoice-me-links.tsx (3)
60-60: LGTM! Proper navigation link theming.The back navigation link correctly uses
text-muted-foregroundtotext-foregroundtransition on hover, maintaining consistent navigation styling.
70-70: LGTM! Appropriate primary button styling.The "New Link" button properly uses primary color tokens for call-to-action emphasis.
99-99: LGTM! Consistent CTA button styling.The "Create" button maintains consistency with the primary button styling pattern established throughout the application.
src/components/compliance-status.tsx (1)
29-55: LGTM! Excellent semantic token implementation.The STATUS_CONFIG object has been consistently updated to use semantic design tokens instead of hardcoded color utilities. The mapping is well-structured:
destructivetokens for error states (not_started)warningtokens for in-progress states (initiated, pending)successtokens for completed states (completed, approved)This change enables proper theme support and maintains visual consistency across light and dark modes.
src/components/mode-toggle.tsx (1)
1-41: Excellent theme toggle implementation!The ModeToggle component is well-implemented with several notable strengths:
- Smooth animations: Uses CSS transforms with cubic-bezier timing for professional feel
- Accessibility: Includes proper
aria-labelfor screen readers- Visual feedback: Clear Sun/Moon icons with rotation animation
- Theme-aware styling: Uses semantic tokens (
bg-muted,text-foreground) that adapt to current theme- Clean API: Simple toggle between light/dark modes
The sliding animation with the calculated
translateXand icon rotation creates an intuitive user experience.src/components/invoice-preview.tsx (3)
32-32: Good semantic token adoption for card container.The Card component now uses proper theme-aware tokens (
bg-card,text-card-foreground,border-border) instead of hardcoded colors, enabling proper dark mode support.
38-242: Consistent text color token updates throughout the component.All instances of hardcoded
text-neutral-500andtext-neutral-600have been systematically replaced withtext-muted-foreground. This creates consistent theming for:
- Section labels (INVOICE NO, ISSUED, DUE DATE, etc.)
- Email addresses and contact information
- Table headers and secondary text
- Notes and payment details
The semantic token approach ensures proper contrast ratios in both light and dark themes.
128-157: Border styling properly updated for theme consistency.Table borders and dividers now use
border-borderinstead ofborder-neutral-200, ensuring they adapt appropriately to the current theme.src/components/header.tsx (4)
3-3: Good integration of theme toggle component.The ModeToggle component is properly imported and will be integrated into the header navigation.
24-30: Logo styling updated for theme consistency.The logo container now uses semantic tokens (
bg-primary,text-primary-foreground,text-foreground) ensuring it adapts properly to both light and dark themes while maintaining brand identity.
37-68: Navigation links properly themed.All navigation links now use consistent semantic tokens:
text-foregroundfor normal statetext-muted-foregroundfor hover state- Proper theme-aware styling for the "Book A Demo" link
This ensures good readability and visual hierarchy in both light and dark modes.
93-93: Theme toggle well-positioned in navigation.The ModeToggle component is appropriately placed at the end of the navigation items, providing easy access to theme switching without cluttering the main navigation flow.
src/components/batch-payout.tsx (6)
262-280: Card styling properly updated for theme support.The main Card container and header now use semantic tokens (
bg-card,text-card-foreground,border-border,bg-muted) ensuring consistent theming across light and dark modes while maintaining the visual hierarchy.
285-289: Loading state styling theme-aware.The loading spinner and text now use
text-muted-foregroundfor consistent secondary text styling that adapts to the current theme.
298-330: Step indicators properly themed.The wallet connection and payment steps now use semantic tokens for:
- Active/inactive state colors (
text-foregroundvstext-muted-foreground)- Step backgrounds (
bg-mutedfor active,border-borderfor inactive)- Progress bar styling
This maintains clear visual progression while supporting both themes.
350-498: Table styling comprehensively updated.The payments table now uses consistent semantic tokens throughout:
- Table borders (
border-border)- Row hover states (
hover:bg-muted/50)- Input focus rings (
focus-visible:ring-border)- Action button styling with proper destructive colors
- Cell text hierarchy with
text-muted-foregroundfor row numbersAll interactive elements maintain proper accessibility while supporting theme switching.
509-572: Summary and action sections well-themed.The batch summary card and action buttons use appropriate semantic tokens:
- Summary card background (
bg-muted)- Text hierarchy for labels and values
- Add payment button with dashed border styling
- Consistent use of
text-muted-foregroundfor secondary informationThe theming maintains the component's functional clarity while enabling dark mode support.
578-585: Footer wallet address styling updated.The connected wallet address display now uses theme-aware colors (
text-muted-foreground,hover:text-foreground) ensuring good readability in both light and dark modes.src/components/view-recurring-payments/view-recurring-payments.tsx (1)
150-152: LGTM – token migration looks solidThe switch to
bg-muted,border-border, andtext-card-foregroundis consistent with the rest of the PR. No functional impact detected.src/components/create-recurring-payment/create-recurring-payment.tsx (1)
49-53: LGTM – loader adopts semantic colourReplacing the hard-coded zinc tint with
text-muted-foregroundaligns the spinner with both light and dark themes.src/components/direct-payout.tsx (1)
216-227: No changes needed:border-foregroundis valid
The Tailwind config defines a top-levelforegroundcolor token, and Tailwind generatesborder-<color>utilities for every theme color. Thereforeborder-foregrounddoes exist and will correctly apply the foreground border color.src/components/payment-section.tsx (4)
404-410: LGTM! Proper semantic token usage for success styling.The secure payment section correctly uses semantic success tokens (
bg-success/10,border-success,text-success,text-success-foreground) which will adapt properly to both light and dark themes.
413-428: LGTM! Consistent warning styling with semantic tokens.The cryptocurrency warning section properly implements semantic warning tokens (
bg-warning/10,border-warning,text-warning,text-warning-foreground) for theme-aware styling.
439-448: LGTM! Proper muted styling implementation.The currency conversion display and recipient address sections correctly use semantic muted tokens (
text-muted-foreground,bg-muted,border-border) for consistent theming.Also applies to: 455-463
472-516: LGTM! Consistent step indicators and loading states styling.The payment steps, wallet connection UI, and loading states properly implement semantic tokens (
text-foreground,text-muted-foreground,bg-muted,border-foreground,border-muted) for theme consistency.Also applies to: 522-567, 584-648
src/components/background-wrapper.tsx (4)
1-2: LGTM! Proper theme integration.The component correctly integrates the
useThemehook fromnext-themesto enable theme-aware styling. The implementation follows React best practices for theme detection.Also applies to: 28-29, 61-61
49-54: LGTM! Dark mode color additions.The dark mode color hex codes are properly added to the color mapping. The zinc and slate variations provide good options for dark theme styling.
64-66: LGTM! Theme-aware background implementation.The dynamic background color switching between light (
#FAFAFA) and dark (#0A0A0A) themes is properly implemented using conditional rendering.
93-95: LGTM! Theme-aware dot pattern implementation.The dot pattern background correctly adapts its color based on the theme (
#1f1f1ffor dark,#e5e5e5for light), providing consistent visual texture across themes.src/components/subscription-plans/blocks/subscription-plan-link.tsx (4)
79-79: LGTM! Proper Card styling with semantic tokens.The Card component correctly uses semantic design tokens (
bg-card,border-border,text-card-foreground) for consistent theming across light and dark modes.
84-85: LGTM! Consistent text styling with semantic tokens.The text elements properly use semantic tokens (
text-foregroundfor primary text,text-muted-foregroundfor secondary text) ensuring proper contrast and readability in both themes.Also applies to: 101-101, 105-105
114-114: LGTM! Proper button and icon styling.The action buttons correctly use
hover:bg-mutedfor hover states andtext-muted-foregroundfor icons, providing consistent interactive feedback across themes.Also applies to: 117-117, 123-123, 127-127
135-139: LGTM! Appropriate preservation of destructive styling.The delete button intentionally maintains its red styling (
hover:bg-red-50,text-red-500) which is correct UX practice - destructive actions should maintain their warning appearance regardless of theme to prevent accidental deletions.
bassgeta
left a comment
bassgeta
left a comment
There was a problem hiding this comment.
Thank you for your contribution to Easy Invoice <3
Really good work, thanks for switching our tailwind variable names to something semantic instead of just using the colour names...
On top of the 3 things I commented on, I've noticed a pretty weird bug which happens only when loading the app the very first time:
Something gets mixed up and some components end up in their dark theme variant, while others do not.
Tried this out on multiple Chromium browsers and I can reproduce it constantly.
The error goes away after toggling the theme at least once.
Not sure what the solution could be, but maybe the theme switcher should check if theme isn't yet set, to just default to the light theme.
Other than that loving the dark mode look 😎
Some more issues with dark mode:
- Invoice me links
2. The alert modal's delete text is dark, should probably be white
3. The user popup is still white in dark mode
4. The `/invoices/:id` page is borked
And sadly, mandatory merge conflicts due to one of our huge PRs coming in, sorry about that 😅
After merging, do check the /subscription-plans route, there's a ton of added things there now.
After the UI issues and review comments are taken care of, I'll gladly drop an approval 💯
mayur1377
force-pushed
the
feature-dark-mode
branch
from
812190e to
b229dcd
Compare
MantisClone
requested review from
MantisClone,
aimensahnoun,
bassgeta and
rodrigopavezi
bassgeta
left a comment
bassgeta
left a comment
There was a problem hiding this comment.
Hey! Thanks for resolving the merge conflicts, sadly some new ones appeared 😅
Additionally, I can still reproduce all the bugs I've pointed out in my previous review, can we take care of those too? 🙏
Feel free to gimme a shout if you need some help or pointers!
hey , have been quite busy with other stuff lately hence the delay! |
bassgeta
force-pushed
the
feature-dark-mode
branch
from
b229dcd to
8377354
Compare
No problem at all, just rebased, resolved the conflicts and pushed to your branch, hope it helps! |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
src/components/compliance-status.tsx (2)
41-44: Pending shares identical visuals with Initiated—consider differentiating, or confirm this is intentional; also apply a11y/contrast fix.If the same color/icon is desired, ignore; otherwise, a distinct tone (e.g., neutral/muted for initiated vs warning for pending) improves scannability. Also mirror the
aria-hiddenand text color change for contrast.- icon: <Clock className="h-5 w-5 text-warning" />, + icon: <Clock aria-hidden="true" className="h-5 w-5 text-warning" />, - color: "bg-warning/10 border-warning text-warning-foreground", + color: "bg-warning/10 border-warning text-warning", ```<!-- review_comment_end --> --- `46-49`: **Use text-success on a light success background; add aria-hidden to the icon.** Same reasoning as above—foreground tokens are tuned for solid variant backgrounds; `text-success` typically gives better contrast on `bg-success/10`. ```diff - icon: <CheckCircle className="h-5 w-5 text-success" />, + icon: <CheckCircle aria-hidden="true" className="h-5 w-5 text-success" />, - color: "bg-success/10 border-success text-success-foreground", + color: "bg-success/10 border-success text-success", ```<!-- review_comment_end --> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (10)</summary><blockquote> <details> <summary>src/components/short-address.tsx (3)</summary><blockquote> `25-33`: **Add accessible label and focus-visible styles** - Screen readers will currently announce only the shortened address, not the action. Add an explicit label and hide the decorative icon. - Add a visible focus state to support keyboard users, especially important in dark mode. Apply: ```diff - <button - type="button" - onClick={handleCopy} - className={`text-muted-foreground hover:text-foreground transition-colors flex items-center gap-2 ${className}`} - > + <button + type="button" + onClick={handleCopy} + aria-label="Copy address" + className={`text-muted-foreground hover:text-foreground transition-colors flex items-center gap-2 rounded-sm focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 ring-offset-background ${className}`} + > <span className="font-mono text-sm">{shortAddress}</span> - <Copy className="h-3 w-3" /> + <Copy className="h-3 w-3" aria-hidden="true" /> </button>
17-20: Handle clipboard failures and await the async call
navigator.clipboard.writeTextreturns a Promise and can fail (permissions, unsupported context). Await it and surface an error toast.- const handleCopy = () => { - navigator.clipboard.writeText(address); - toast.success("Address copied to clipboard"); - }; + const handleCopy = async () => { + try { + await navigator.clipboard.writeText(address); + toast.success("Address copied to clipboard"); + } catch { + toast.error("Failed to copy address"); + } + };
13-15: Guard against very short inputs and prefersliceFor unexpectedly short strings, the current logic could duplicate text around the ellipsis. Guard by length and use
slicewith a negative index for clarity.- const shortAddress = `${address.substring(0, 6)}...${address.substring( - address.length - 4, - )}`; + const shortAddress = + address.length > 10 + ? `${address.slice(0, 6)}...${address.slice(-4)}` + : address;src/components/ui/tooltip.tsx (1)
17-22: Optional: add border and consistent radius to match tokenized surface patterns and improve contrastAcross tokenized components, popover-like surfaces typically include a subtle border and a consistent radius. Consider aligning Tooltip with that pattern for better separation in both light/dark themes and visual consistency.
Apply this minimal styling tweak:
- className="bg-popover text-popover-foreground text-xs px-2 py-1 rounded shadow-lg z-50" + className="z-50 rounded-md border border-border bg-popover px-2.5 py-1.5 text-xs text-popover-foreground shadow-md"Note: If you add a border to Content, you may optionally mirror it on the Arrow to avoid a slight seam. Two common approaches:
- Add a second Arrow behind it with
fill-border(requires fine-tuned positioning), or- Use a subtle drop shadow on the Arrow to simulate a border (e.g., an arbitrary
drop-shadowwiththeme(colors.border)), if your Tailwind configuration allows it.src/app/dashboard/layout.tsx (1)
28-33: Minor spacing + a11y: remove redundant margin and hide decorative iconYou already have gap-2 on the container; mr-2 on the icon is redundant. Also mark the icon decorative so it’s ignored by screen readers.
Apply this diff:
- <Button asChild variant="default" className="flex items-center gap-2"> - <Link href="/invoices/create"> - <PlusCircle className="mr-2 h-4 w-4" /> + <Button asChild variant="default" className="flex items-center gap-2"> + <Link href="/invoices/create"> + <PlusCircle className="h-4 w-4" aria-hidden="true" /> Create Invoice </Link> </Button>src/components/payment-route.tsx (4)
51-53: Primary badge: prefertext-primaryon a tintedbg-primary/10.
text-primary-foregroundis typically tuned for solidbg-primary. On a 10% tint background,text-primarygenerally provides better contrast and aligns with token semantics.- bgColor: "bg-primary/10", - textColor: "text-primary-foreground", + bgColor: "bg-primary/10", + textColor: "text-primary",
59-61: Success badge: mirror the pattern used for primary/warning.As above, prefer
text-successfor tinted backgrounds to avoid low contrast in dark mode.- bgColor: "bg-success/10", - textColor: "text-success-foreground", + bgColor: "bg-success/10", + textColor: "text-success",
66-68: Default badge tone: considersecondarytokens for better contrast.
bg-muted text-muted-foregroundcan be too low-contrast for a status badge.secondarytokens are often intended for subtle yet legible chips.- bgColor: "bg-muted", - textColor: "text-muted-foreground", + bgColor: "bg-secondary", + textColor: "text-secondary-foreground",
80-84: Unify selection state and addaria-pressedfor accessibility.You have two sources of truth (
variant === "selected"andisSelected). Consolidate to a singleselectedflag and expose it viaaria-pressedfor screen readers. This removes styling divergence (e.g.,border-2applied forvariantbut not forisSelected).Apply in this block:
- className={`w-full p-4 border rounded-lg transition-colors ${ - variant === "selected" - ? "border-2 border-primary bg-card" - : isSelected - ? "bg-card border-primary" - : "bg-background hover:border-border" - }`} + aria-pressed={selected} + className={`w-full p-4 border rounded-lg transition-colors ${ + selected ? "border-2 border-primary bg-card" : "bg-background hover:border-border" + }`}And add once above the return (outside this hunk):
const selected = variant === "selected" || isSelected;Optionally, move the styling to a
cvaorcnhelper for maintainability.src/components/compliance-status.tsx (1)
29-55: Reduce duplication and enforce consistency with a status pill utility (cva/variants).Multiple statuses share the same shape and differ only by token. Centralizing via a
cvavariant (or a small helper) will keep light/dark contrast rules consistent and reduce future churn.Example (outside this range) of a minimal helper you could add:
// ui/status-pill.tsx import { cva } from "class-variance-authority"; export const statusPill = cva( "flex items-center gap-2 px-3 py-1 rounded-full border text-sm font-medium", { variants: { intent: { destructive: "bg-destructive/10 border-destructive text-destructive", warning: "bg-warning/10 border-warning text-warning", success: "bg-success/10 border-success text-success", }, }, defaultVariants: { intent: "warning" }, }, );Then in this file:
<div className={statusPill({ intent: "destructive" })}> {config.icon} <span className="text-sm font-medium">{config.text}</span> </div>I’m happy to draft this refactor if you’d like.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (36)
src/app/dashboard/layout.tsx(2 hunks)src/app/i/[id]/page.tsx(1 hunks)src/app/invoices/create/page.tsx(1 hunks)src/app/layout.tsx(2 hunks)src/components/background-wrapper.tsx(3 hunks)src/components/batch-payout.tsx(17 hunks)src/components/compliance-status.tsx(1 hunks)src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx(2 hunks)src/components/create-recurring-payment/create-recurring-payment.tsx(5 hunks)src/components/dashboard/blocks/invoice-row.tsx(5 hunks)src/components/dashboard/invoices-received.tsx(6 hunks)src/components/dashboard/invoices-sent.tsx(2 hunks)src/components/dashboard/subscriptions.tsx(5 hunks)src/components/direct-payout.tsx(10 hunks)src/components/footer.tsx(1 hunks)src/components/header.tsx(4 hunks)src/components/invoice-form.tsx(2 hunks)src/components/invoice-me-link.tsx(1 hunks)src/components/invoice-me-links.tsx(3 hunks)src/components/invoice-preview.tsx(7 hunks)src/components/mode-toggle.tsx(1 hunks)src/components/payment-route.tsx(4 hunks)src/components/payment-section.tsx(10 hunks)src/components/payment-secured-using-request.tsx(1 hunks)src/components/short-address.tsx(1 hunks)src/components/stat-card.tsx(1 hunks)src/components/subscription-plan-preview.tsx(5 hunks)src/components/subscription-plans/blocks/subscription-plan-link.tsx(3 hunks)src/components/ui/table/empty-state.tsx(1 hunks)src/components/ui/table/pagination.tsx(2 hunks)src/components/ui/table/table-head-cell.tsx(1 hunks)src/components/ui/tooltip.tsx(1 hunks)src/components/view-recurring-payments/blocks/completed-payments.tsx(1 hunks)src/components/view-recurring-payments/blocks/frequency-badge.tsx(1 hunks)src/components/view-recurring-payments/blocks/status-badge.tsx(1 hunks)src/components/view-recurring-payments/view-recurring-payments.tsx(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/dashboard/invoices-received.tsx
🚧 Files skipped from review as they are similar to previous changes (29)
- src/app/i/[id]/page.tsx
- src/components/ui/table/pagination.tsx
- src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx
- src/components/footer.tsx
- src/components/dashboard/blocks/invoice-row.tsx
- src/components/payment-secured-using-request.tsx
- src/components/view-recurring-payments/blocks/completed-payments.tsx
- src/components/direct-payout.tsx
- src/app/layout.tsx
- src/components/ui/table/table-head-cell.tsx
- src/components/batch-payout.tsx
- src/components/invoice-me-link.tsx
- src/components/subscription-plan-preview.tsx
- src/components/view-recurring-payments/view-recurring-payments.tsx
- src/components/background-wrapper.tsx
- src/components/mode-toggle.tsx
- src/components/invoice-form.tsx
- src/components/stat-card.tsx
- src/components/invoice-me-links.tsx
- src/components/view-recurring-payments/blocks/status-badge.tsx
- src/components/dashboard/invoices-sent.tsx
- src/components/payment-section.tsx
- src/components/invoice-preview.tsx
- src/components/subscription-plans/blocks/subscription-plan-link.tsx
- src/components/view-recurring-payments/blocks/frequency-badge.tsx
- src/components/ui/table/empty-state.tsx
- src/components/dashboard/subscriptions.tsx
- src/components/header.tsx
- src/components/create-recurring-payment/create-recurring-payment.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/app/dashboard/layout.tsx (1)
src/components/ui/button.tsx (1)
Button(56-56)
🔇 Additional comments (7)
src/components/short-address.tsx (1)
28-28: Semantic tokens migration aligns with dark modeGood switch from hardcoded zinc classes to
text-muted-foregroundandhover:text-foreground. This matches the design-system direction and should adapt cleanly across themes.src/components/ui/tooltip.tsx (2)
17-17: LGTM: semantic tokens applied to Tooltip.ContentSwapping hardcoded zinc classes for
bg-popoverandtext-popover-foregroundaligns with the PR’s dark-mode tokenization work. No behavioral changes introduced.
21-21: fill-popover class compilation confirmed
- tailwind.config.ts defines
extend.colors.popoverwithDEFAULTandforeground(lines 19–22).- src/app/globals.css declares
--popoverand--popover-foregroundCSS variables (lines 10–12).- The
contentarray in tailwind.config.ts includes./src/components/**/*.{js,ts,jsx,tsx,mdx}, sofill-popoverwill be scanned and preserved by Tailwind JIT.No further action required.
src/app/dashboard/layout.tsx (1)
5-5: Good move: unify CTA styling via design-system ButtonUsing the Button component aligns this layout with the new tokenized UI system and keeps semantics correct with asChild.
src/app/invoices/create/page.tsx (1)
30-30: Token migration LGTMSwitching to text-muted-foreground and hover:text-foreground fits the dark-mode semantics.
src/components/payment-route.tsx (1)
110-110: Good semantic token migration for secondary text.Switching to
text-muted-foregroundaligns with the dark-mode theming strategy and improves consistency.src/components/compliance-status.tsx (1)
31-34: I’ve requested the full CSS variable context for background and foreground colors. Once we see the actual--backgroundand other relevant definitions, we can compute WCAG contrast to confirm iftext-destructive-foregroundonbg-destructive/10fails.
mayur1377
force-pushed
the
feature-dark-mode
branch
from
8377354 to
9e8bd27
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/batch-payout.tsx (1)
177-201: Currency-aware decimals in totalsByCurrency.You’re parsing and formatting with 18 decimals for all currencies. For fiat or tokens with non-18 decimals, this misrepresents totals. Use currency-specific decimals (e.g., USD: 2; USDC/USDT: 6; DAI: 18; ETH: 18).
Proposed refactor:
const getTotalsByCurrency = () => { const payouts = form.watch("payouts"); - const totals: Record<string, ethers.BigNumber> = {}; + const totals: Record<string, ethers.BigNumber> = {}; + // Simple decimals map; ideally centralize alongside currency metadata. + const getDecimals = (currency: string) => { + switch (currency) { + case "USD": + return 2; + case "USDC-base": + case "USDT-base": + case "USDCn-optimism": + case "USDT-optimism": + case "USDCn-matic": + case "USDT-matic": + case "USDC-mainnet": + case "USDT-mainnet": + case "USDC-arbitrum-one": + case "USDT-arbitrum-one": + return 6; + default: + return 18; // most ERC20s/ETH + } + }; for (const payout of payouts) { if (payout.amount > 0) { const currency = payout.invoiceCurrency; - const amount = ethers.utils.parseUnits(payout.amount.toString(), 18); + const amount = ethers.utils.parseUnits( + payout.amount.toString(), + getDecimals(currency) + ); totals[currency] = (totals[currency] || ethers.BigNumber.from(0)).add( amount, ); } } const humanReadableTotals: Record<string, string> = {}; for (const [currency, bigNumberTotal] of Object.entries(totals)) { - humanReadableTotals[currency] = ethers.utils.formatUnits( - bigNumberTotal, - 18, - ); + humanReadableTotals[currency] = ethers.utils.formatUnits( + bigNumberTotal, + getDecimals(currency), + ); } return humanReadableTotals; };If you prefer, I can consolidate decimals in src/lib/constants/currencies.ts so it’s a single source of truth.
src/components/payment-section.tsx (1)
323-346: Guard selectedRoute and compute targetChain correctly for direct payments.If selectedRoute is null or is a direct REQUEST_NETWORK_PAYMENT (no route.chain), targetChain becomes undefined and you’ll attempt a network switch to undefined. Also, direct payments should target the invoice chain, not route.chain.
Apply this diff:
- const handlePayment = async () => { + const handlePayment = async () => { if (paymentProgress !== "idle") return; + if (!selectedRoute) { + toast("No payment route selected"); + return; + } setPaymentProgress("getting-transactions"); - const targetChain = - CHAIN_TO_ID[selectedRoute?.chain as keyof typeof CHAIN_TO_ID]; + // For direct REQUEST_NETWORK_PAYMENT, use the invoice chain. + const invoiceChainKey = + invoiceChain && + REQUEST_NETWORK_CHAIN_TO_PAYMENT_NETWORK[ + invoiceChain as keyof typeof REQUEST_NETWORK_CHAIN_TO_PAYMENT_NETWORK + ]; + const chainKey = + selectedRoute.id === "REQUEST_NETWORK_PAYMENT" + ? invoiceChainKey + : selectedRoute.chain?.toLowerCase(); + const targetChain = + chainKey && CHAIN_TO_ID[chainKey as keyof typeof CHAIN_TO_ID]; if (targetChain !== chainId) { - const targetAppkitNetwork = - ID_TO_APPKIT_NETWORK[targetChain as keyof typeof ID_TO_APPKIT_NETWORK]; + if (!targetChain) { + toast("Unable to determine target network for this route"); + setPaymentProgress("idle"); + return; + } + const targetAppkitNetwork = + ID_TO_APPKIT_NETWORK[targetChain as keyof typeof ID_TO_APPKIT_NETWORK]; toast("Switching to network", { description: `Switching to ${targetAppkitNetwork.name} network`, });src/components/invoice-me-link.tsx (1)
90-104: AlertDialog theming should rely on tokens; drop dark: overrides.bg-background/text-foreground already adapt by theme. The dark:* overrides invert them and can reduce readability.
Apply this diff:
- <AlertDialogContent className="bg-background text-foreground dark:bg-foreground dark:text-background"> + <AlertDialogContent> ... - <AlertDialogCancel className="bg-background text-foreground dark:bg-foreground dark:text-background"> - Cancel - </AlertDialogCancel> + <AlertDialogCancel>Cancel</AlertDialogCancel>Also applies to: 101-104
♻️ Duplicate comments (1)
src/components/payment-section.tsx (1)
654-655: Resolved: primary action button now uses semantic tokens.The button no longer mixes hardcoded and semantic tokens. This fixes the prior inconsistency flagged in earlier reviews.
🧹 Nitpick comments (23)
src/components/ui/table/empty-state.tsx (1)
17-17: Tint the icon via semantic token and hide decorative icon from SRsAdd text-muted-foreground so SVGs using currentColor inherit the right tint in both themes, and mark the icon container aria-hidden if purely decorative.
- <div className="h-12 w-12 rounded-full bg-muted flex items-center justify-center mb-4"> + <div + className="h-12 w-12 rounded-full bg-muted text-muted-foreground flex items-center justify-center mb-4" + aria-hidden="true" + >src/components/subscription-plan-preview.tsx (4)
126-131: Add accessible name and focus-visible styles to the icon-only back linkThe link is icon-only; screen readers won’t get a meaningful name. Also, no focus-visible styles for keyboard users.
Apply this diff to improve a11y without changing visuals:
- <Link - href="/" - className="text-muted-foreground hover:text-foreground transition-colors mr-4" - > + <Link + href="/" + aria-label="Back" + className="text-muted-foreground hover:text-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 transition-colors mr-4" + >Optional: if you use shadcn’s ring offset tokens, add
focus-visible:ring-offset-backgroundas well for better contrast on varied surfaces.
149-151: Token migration to text-muted-foreground looks good; consider semantics and contrast checkLGTM on swapping hardcoded zinc shades for semantic tokens—this should hold up in dark mode.
Optional improvements:
- Use semantic markup for label/value pairs (e.g., a description list with dt/dd) to aid screen readers.
- While you’re migrating to tokens, consider replacing the chip colors on Lines 141–143 (
bg-blue-50 text-blue-700) with design tokens (e.g.,bg-muted text-foregroundor a variant) to ensure parity in dark mode.- Double-check contrast for
text-muted-foregroundon all backgrounds in both themes (especially within Cards).Also applies to: 159-161, 167-169, 177-179, 187-189, 197-199
259-264: Improve wallet button a11y: focus-visible styles (and consider dialog semantics)The wallet control opens a modal, but the button lacks focus-visible styling for keyboard users.
Apply this diff to add accessible focus indication:
- <button + <button type="button" onClick={() => open()} - className="flex items-center text-sm text-muted-foreground hover:text-foreground transition-colors" + className="flex items-center text-sm text-muted-foreground hover:text-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 transition-colors" disabled={isProcessing} >Suggestion (no diff here): Add
aria-haspopup="dialog"to hint the action opens a dialog.
304-307: Make the loading state screen-reader friendlyThe spinner is decorative and the status text should announce politely to assistive tech.
Apply this diff:
- <Loader2 className="h-8 w-8 animate-spin text-muted-foreground" /> - <p className="text-muted-foreground"> + <Loader2 aria-hidden="true" className="h-8 w-8 animate-spin text-muted-foreground" /> + <p role="status" aria-live="polite" className="text-muted-foreground"> Initializing payment system... </p>This prevents the icon from being read and ensures the status is announced. If feasible, consider placing
role="status" aria-live="polite"on a parent wrapper instead, to cover future status changes.src/components/create-recurring-payment/create-recurring-payment.tsx (6)
38-40: Trim Card/Header classes to avoid redundancy and prevent double-rounded seams
- If your Card component already applies border/bg/text via design tokens (typical with shadcn/ui), these extras are redundant and can create mismatched rounded corners with the header’s rounded-t-lg.
- Consider letting Card own its radius/border/bg and remove the header’s top rounding.
- <Card className="w-full shadow-lg border border-border bg-card text-card-foreground"> - <CardHeader className="bg-muted rounded-t-lg border-b border-border"> + <Card className="w-full shadow-lg"> + <CardHeader className="bg-muted border-b border-border">
47-55: Improve loading state accessibility (role=status, aria-live, aria-busy, hide spinner from SR)
- Announce progress to screen readers and mark the region busy.
- Keep the spinner purely decorative; the text serves as the announcement.
- Note: The 2s readiness gate aligns with our prior learning about AppKit initialization. Good to keep.
- <CardContent className="py-16"> - <div className="flex flex-col items-center justify-center space-y-4"> - <Loader2 className="h-10 w-10 animate-spin text-muted-foreground" /> - <p className="text-muted-foreground"> - Initializing payment system... - </p> - </div> - </CardContent> + <CardContent className="py-16" aria-busy={!isAppKitReady}> + <div + className="flex flex-col items-center justify-center space-y-4" + role="status" + aria-live="polite" + aria-atomic="true" + aria-describedby="appkit-loading" + > + <Loader2 className="h-10 w-10 animate-spin text-muted-foreground" aria-hidden="true" /> + <p id="appkit-loading" className="text-muted-foreground"> + Initializing payment system... + </p> + </div> + </CardContent>If you want, I can add a quick RTL test that fakes timers to verify the loader announces and disappears after ~2s.
61-68: The condition is always true; simplify and/or introduce explicit “completed” vs “active” styling; add aria-current
- currentStep is a union of "connect-wallet" | "create-payment". The check:
currentStep === "connect-wallet" || currentStep === "create-payment"
is always true, so these classes are effectively unconditional.- Either: keep step 1 always highlighted (simplify classes), or introduce a distinct “completed” style when on step 2.
- Also expose aria-current on the active step for assistive tech.
- <div - className={`flex items-center ${ - currentStep === "connect-wallet" || - currentStep === "create-payment" - ? "text-foreground" - : "text-muted-foreground" - }`} - > + <div + className="flex items-center text-foreground" + aria-current={currentStep === "connect-wallet" ? "step" : undefined} + > <div - className={`w-8 h-8 rounded-full border-2 flex items-center justify-center ${ - currentStep === "connect-wallet" || - currentStep === "create-payment" - ? "border-foreground bg-muted" - : "border-border" - }`} + className="w-8 h-8 rounded-full border-2 flex items-center justify-center border-foreground bg-muted" >Alternative (if you want a “completed” look on step 1 when on step 2): toggle a subtle checkmark or swap bg-muted for bg-foreground text-background. I can draft that if helpful.
Also applies to: 70-76
83-88: Mark decorative connector as aria-hidden and add smooth color transition
- This line is purely visual; hide it from SR.
- Minor UX polish with transition-colors.
- <div - className={`w-16 h-0.5 ${ + <div + className={`w-16 h-0.5 transition-colors duration-200 ${ currentStep === "create-payment" ? "bg-foreground" : "bg-border" }`} - /> + aria-hidden="true" + />
91-96: Expose current step with aria-current for step 2 as well
- Communicates the active step to assistive tech; mirrors the change suggested for step 1.
- <div - className={`flex items-center ${ - currentStep === "create-payment" - ? "text-foreground" - : "text-muted-foreground" - }`} - > + <div + className={`flex items-center ${ + currentStep === "create-payment" + ? "text-foreground" + : "text-muted-foreground" + }`} + aria-current={currentStep === "create-payment" ? "step" : undefined} + > <div className={`w-8 h-8 rounded-full border-2 flex items-center justify-center ${ currentStep === "create-payment" ? "border-foreground bg-muted" : "border-border" }`} >Also applies to: 98-103
58-109: Consider extracting the step indicator into a reusable Stepper component
- Same structural pattern appears twice and will likely be reused elsewhere (batch payouts, single payments, etc.).
- Encapsulate state props (currentStep, steps[]) and handle aria semantics inside the component.
Happy to sketch a Stepper API like:
- props: steps: { key: string; label: string; icon: ReactNode }[], currentKey: string
- handles aria-current, completed/active styles, and theming tokens.
src/components/user-menu.tsx (4)
47-50: Guard against overflow: truncate and expose full value via titleLong names can wrap or overflow the 14rem panel. Add
truncateand atitlefor hover reveal.- <p className="text-sm font-medium text-foreground"> + <p className="text-sm font-medium text-foreground truncate" title={user.name ?? "User"}> {user.name ?? "User"} </p>
54-56: Same overflow concern for email + optional hierarchy
- Apply
truncateand atitleto avoid multi-line layout shifts.- Optional: consider
text-muted-foregroundfor the email to de-emphasize it relative to the display name (design-dependent).- <p className="text-sm font-medium text-foreground"> + <p + className="text-sm font-medium text-foreground truncate" + title={user.email ?? ""} + > {truncateEmail(user.email ?? "")} </p>If you prefer reduced emphasis:
- <p - className="text-sm font-medium text-foreground truncate" + <p + className="text-sm font-medium text-muted-foreground truncate" title={user.email ?? ""} >
71-75: Disable logout while pending to prevent double-submitsPrevent repeated clicks and reflect busy state by disabling the item while the mutation is in-flight.
- <DropdownMenuItem - onClick={() => logout.mutate()} - className="text-sm text-muted-foreground cursor-pointer hover:text-foreground" - > + <DropdownMenuItem + disabled={logout.isPending} + onClick={() => { if (!logout.isPending) logout.mutate(); }} + className="text-sm text-muted-foreground hover:text-foreground" + >
34-36: Ensure theme tokens and accessibility for the user-menu triggerIn
src/components/user-menu.tsx
- Replace the hardcoded neutral border with the semantic border token so it adapts in dark mode.
- Add an
aria-labelon the button to improve screen-reader support.Apply this minimal diff:
--- a/src/components/user-menu.tsx +++ b/src/components/user-menu.tsx @@ -32,7 +32,8 @@ function UserMenu() { <Button variant="ghost" - className="relative h-10 w-10 rounded-full border border-neutral-200 bg-background" + aria-label="Open user menu" + className="relative h-10 w-10 rounded-full border border-border bg-background" > <Avatar> …(Optional) If you’d rather let the design system handle border & background styles:
- <Button - variant="ghost" - aria-label="Open user menu" - className="relative h-10 w-10 rounded-full border border-border bg-background" + <Button + variant="outline" + aria-label="Open user menu" + className="relative h-10 w-10 rounded-full" >
Other parts of the codebase still use hardcoded neutral/gray utilities (e.g.
bg-gray-50,text-gray-700,text-gray-500, etc., innot-found.tsx,invoice-form.tsx,bank-account-form.tsx, and more). These should be rolled into semantic theme tokens (e.g.bg-background,text-muted,text-foreground) in a separate follow-up refactor to ensure full dark-mode compatibility.src/components/batch-payout.tsx (2)
470-491: Disable row actions while processing to avoid mid-flight edits.During a batch “processing” state, users can still duplicate/remove rows, which can confuse totals and validation mid-submit. Consider disabling these actions while processing.
Apply this diff:
<Button type="button" variant="ghost" size="sm" onClick={() => duplicatePayment(index)} - disabled={fields.length >= MAX_PAYMENTS} + disabled={fields.length >= MAX_PAYMENTS || paymentStatus === "processing"} className="h-8 w-8 p-0 text-muted-foreground hover:text-foreground" > ... {fields.length > 1 && ( <Button type="button" variant="ghost" size="sm" onClick={() => removePayment(index)} - className="h-8 w-8 p-0 text-destructive hover:text-destructive-foreground hover:bg-destructive/10" + disabled={paymentStatus === "processing"} + className="h-8 w-8 p-0 text-destructive hover:text-destructive-foreground hover:bg-destructive/10 disabled:opacity-50" >
415-418: Make focus handling consistent across form controls (prefer focus-visible).Inputs use focus-visible but SelectTriggers use focus. Aligning to focus-visible improves keyboard UX without adding extra focus rings for mouse users.
Apply this diff:
- <SelectTrigger className="text-sm border-0 shadow-none focus:ring-1 focus:ring-border"> + <SelectTrigger className="text-sm border-0 shadow-none focus-visible:ring-1 focus-visible:ring-border">Repeat the same change for the payment currency SelectTrigger.
Also applies to: 446-447
src/components/payment-section.tsx (2)
485-486: Use border-border instead of border-muted for inactive step rings.border-muted is likely undefined in the token set and can degrade contrast. border-border is the standard semantic token for borders (active states rightly use border-foreground).
Apply this diff:
- ? "border-foreground bg-muted" - : "border-muted" + ? "border-foreground bg-muted" + : "border-border"Make the same change for both step circles.
Also applies to: 493-495, 506-509
458-462: Replace hardcoded text-amber-700 with semantic token.This one slipped through the migration and will be off in dark mode.
Apply this diff:
- <div className="mt-2 text-sm text-amber-700"> + <div className="mt-2 text-sm text-warning-foreground">src/app/invoices/[ID]/page.tsx (3)
54-56: Consider relying on Card tokens for surface separation instead of border-0.Removing the border entirely can reduce separation in dark mode where shadows are subtler. Using the default Card styling (bg-card, border-border, text-card-foreground) generally balances both themes.
Apply this diff if you want consistent separation:
- <Card className="w-full shadow-sm border-0"> + <Card className="w-full shadow-sm">
40-42: Optional: make BackgroundWrapper gradients theme-aware.The hardcoded green-100/200 and blue-100/200 will be muted in dark mode. If BackgroundWrapper supports tokens/CSS vars, consider semantic or CSS-var-based gradients.
Example (if CSS vars are available):
- topGradient={{ from: "green-100", to: "green-200" }} - bottomGradient={{ from: "blue-100", to: "blue-200" }} + topGradient={{ from: "[hsl(var(--primary))]/15", to: "[hsl(var(--primary))]/5" }} + bottomGradient={{ from: "[hsl(var(--secondary))]/12", to: "[hsl(var(--secondary))]/4" }}If BackgroundWrapper internally maps tokens, passing semantic names (e.g., primary-100/200) would also work.
181-194: Format amounts and currency labels with locale-aware formatting.Number(...).toString() won’t add separators and may mislead for large values. Using Intl.NumberFormat yields consistent UX.
Example:
const fmt = new Intl.NumberFormat(undefined, { maximumFractionDigits: 6 }); fmt.format(Number(invoice.amount));src/components/invoice-me-link.tsx (1)
85-88: Use semantic variant/tokens for the delete trigger; avoid manual dark overrides.The trigger button forces bg/text colors and adds dark: overrides that fight theme tokens. Prefer semantic intent (destructive) and keep background transparent until hover.
Apply this diff:
- <Button - variant="ghost" - size="sm" - className="h-8 w-8 p-0 bg-background text-foreground hover:bg-muted dark:bg-foreground dark:text-background dark:hover:bg-muted" - title="Delete link" - > - <Trash2 className="h-4 w-4" /> - </Button> + <Button + variant="ghost" + size="sm" + className="h-8 w-8 p-0 text-destructive hover:bg-destructive/10 hover:text-destructive" + title="Delete link" + > + <Trash2 className="h-4 w-4" /> + </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (44)
src/app/dashboard/layout.tsx(2 hunks)src/app/i/[id]/page.tsx(1 hunks)src/app/invoices/[ID]/page.tsx(8 hunks)src/app/invoices/create/page.tsx(1 hunks)src/app/layout.tsx(2 hunks)src/app/subscription-plans/page.tsx(1 hunks)src/components/background-wrapper.tsx(3 hunks)src/components/batch-payout.tsx(17 hunks)src/components/compliance-status.tsx(1 hunks)src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx(2 hunks)src/components/create-recurring-payment/create-recurring-payment.tsx(5 hunks)src/components/dashboard/blocks/invoice-row.tsx(5 hunks)src/components/dashboard/invoices-received.tsx(6 hunks)src/components/dashboard/invoices-sent.tsx(2 hunks)src/components/dashboard/subscriptions.tsx(5 hunks)src/components/direct-payout.tsx(10 hunks)src/components/footer.tsx(1 hunks)src/components/header.tsx(4 hunks)src/components/invoice-form.tsx(2 hunks)src/components/invoice-me-link.tsx(3 hunks)src/components/invoice-me-links.tsx(3 hunks)src/components/invoice-preview.tsx(7 hunks)src/components/mode-toggle.tsx(1 hunks)src/components/multi-currency-stat-card.tsx(1 hunks)src/components/payment-route.tsx(4 hunks)src/components/payment-section.tsx(10 hunks)src/components/payment-secured-using-request.tsx(1 hunks)src/components/short-address.tsx(1 hunks)src/components/stat-card.tsx(1 hunks)src/components/subscription-plan-preview.tsx(5 hunks)src/components/subscription-plans/blocks/payments-table.tsx(7 hunks)src/components/subscription-plans/blocks/subscribers-table.tsx(6 hunks)src/components/subscription-plans/blocks/subscription-plan-link.tsx(5 hunks)src/components/subscription-plans/blocks/subscription-plans-list.tsx(3 hunks)src/components/subscription-plans/subscription-plan-tabs.tsx(1 hunks)src/components/ui/table/empty-state.tsx(1 hunks)src/components/ui/table/pagination.tsx(2 hunks)src/components/ui/table/table-head-cell.tsx(1 hunks)src/components/ui/tooltip.tsx(1 hunks)src/components/user-menu.tsx(2 hunks)src/components/view-recurring-payments/blocks/completed-payments.tsx(1 hunks)src/components/view-recurring-payments/blocks/frequency-badge.tsx(1 hunks)src/components/view-recurring-payments/blocks/status-badge.tsx(1 hunks)src/components/view-recurring-payments/view-recurring-payments.tsx(8 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/components/subscription-plans/blocks/subscription-plans-list.tsx
- src/app/subscription-plans/page.tsx
- src/app/i/[id]/page.tsx
- src/components/view-recurring-payments/blocks/completed-payments.tsx
🚧 Files skipped from review as they are similar to previous changes (27)
- src/components/ui/tooltip.tsx
- src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx
- src/app/layout.tsx
- src/components/view-recurring-payments/blocks/frequency-badge.tsx
- src/components/ui/table/table-head-cell.tsx
- src/components/view-recurring-payments/blocks/status-badge.tsx
- src/components/footer.tsx
- src/components/payment-secured-using-request.tsx
- src/components/dashboard/subscriptions.tsx
- src/components/stat-card.tsx
- src/components/dashboard/invoices-received.tsx
- src/components/header.tsx
- src/components/dashboard/blocks/invoice-row.tsx
- src/components/short-address.tsx
- src/components/view-recurring-payments/view-recurring-payments.tsx
- src/app/invoices/create/page.tsx
- src/app/dashboard/layout.tsx
- src/components/direct-payout.tsx
- src/components/payment-route.tsx
- src/components/mode-toggle.tsx
- src/components/invoice-preview.tsx
- src/components/dashboard/invoices-sent.tsx
- src/components/ui/table/pagination.tsx
- src/components/invoice-me-links.tsx
- src/components/compliance-status.tsx
- src/components/background-wrapper.tsx
- src/components/invoice-form.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-02-27T13:58:33.590Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#23
File: src/components/direct-payment.tsx:63-89
Timestamp: 2025-02-27T13:58:33.590Z
Learning: The AppKit wallet module requires initialization time before it's ready for use. A 2-second delay is intentionally used in the DirectPayment component to ensure the wallet is fully initialized before allowing interactions.
Applied to files:
src/components/create-recurring-payment/create-recurring-payment.tsxsrc/components/batch-payout.tsx
📚 Learning: 2025-02-12T12:42:40.076Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#2
File: src/app/invoices/[ID]/page.tsx:160-175
Timestamp: 2025-02-12T12:42:40.076Z
Learning: Invoice items are validated at creation time through the schema validation in src/lib/schemas/invoice.ts, ensuring that items array is non-empty and each item has required description, quantity, and price fields. Runtime validation in display components is not necessary.
Applied to files:
src/app/invoices/[ID]/page.tsx
📚 Learning: 2025-03-25T09:31:47.519Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#42
File: src/components/payment-section.tsx:534-544
Timestamp: 2025-03-25T09:31:47.519Z
Learning: In the PaymentSection component of EasyInvoice, selectedRoute is automatically set to the first route when payment routes are loaded, and users can only switch between routes, not deselect them entirely.
Applied to files:
src/components/payment-section.tsx
🧬 Code graph analysis (7)
src/app/invoices/[ID]/page.tsx (1)
src/lib/date-utils.ts (1)
formatDate(9-17)
src/components/payment-section.tsx (1)
src/lib/constants/currencies.ts (2)
MAINNET_CURRENCIES(2-18)formatCurrencyLabel(94-139)
src/components/subscription-plans/blocks/payments-table.tsx (1)
src/components/multi-currency-stat-card.tsx (1)
MultiCurrencyStatCard(12-41)
src/components/subscription-plans/blocks/subscribers-table.tsx (1)
src/components/multi-currency-stat-card.tsx (1)
MultiCurrencyStatCard(12-41)
src/components/batch-payout.tsx (1)
src/lib/constants/currencies.ts (1)
formatCurrencyLabel(94-139)
src/components/invoice-me-link.tsx (3)
src/components/ui/card.tsx (2)
Card(79-79)CardContent(79-79)src/components/ui/button.tsx (1)
Button(56-56)src/components/ui/alert-dialog.tsx (4)
AlertDialog(130-130)AlertDialogTrigger(133-133)AlertDialogContent(134-134)AlertDialogCancel(140-140)
src/components/subscription-plans/blocks/subscription-plan-link.tsx (2)
src/components/ui/card.tsx (2)
Card(79-79)CardContent(79-79)src/components/ui/button.tsx (1)
Button(56-56)
🔇 Additional comments (35)
src/components/ui/table/empty-state.tsx (2)
20-20: Token migration for title looks goodSwitch to text-foreground aligns with the theme tokens and keeps contrast in dark mode.
21-21: Subtitle uses the correct muted tokentext-muted-foreground is the right choice for secondary copy across themes.
src/components/subscription-plan-preview.tsx (1)
292-299: LGTM on muted foreground for the “connect wallet” hintThe move to
text-muted-foregroundaligns with the dark-mode token strategy and keeps the emphasis on the CTA.src/components/create-recurring-payment/create-recurring-payment.tsx (1)
113-113: LGTM: token-based muted text for dark modeUsing text-muted-foreground here is consistent with the design-token migration and reads well in both themes.
src/components/user-menu.tsx (2)
38-38: LGTM: foreground token adoptionUsing
text-foregroundfor the avatar initial is correct and theme-aware.
44-44: LGTM: rely on DropdownMenu themingKeeping
DropdownMenuContentfree of hardcoded background classes and only setting width aligns with theme tokens.src/components/subscription-plans/subscription-plan-tabs.tsx (1)
47-48: LGTM! Theme token migration applied consistently.The loading state text and icon colors have been successfully updated from hardcoded zinc colors to semantic design tokens (
text-muted-foreground), aligning with the dark mode implementation across the application.src/components/multi-currency-stat-card.tsx (3)
21-21: LGTM! Title styling updated to semantic token.The title now uses
text-muted-foregroundinstead oftext-zinc-600, which is consistent with the theme-aware design system being implemented.
25-25: LGTM! Empty state styling updated for theming.The empty state display now uses
text-foregroundinstead oftext-zinc-900, ensuring proper contrast in both light and dark themes.
29-34: LGTM! Amount and currency styling consistently updated.Both the amount and currency values now use semantic design tokens (
text-foregroundfor amounts andtext-muted-foregroundfor currencies) instead of hardcoded zinc colors, maintaining proper visual hierarchy in all themes.src/components/subscription-plans/blocks/subscribers-table.tsx (6)
83-83: LGTM! Row hover state updated to semantic token.The table row hover background has been updated from
hover:bg-zinc-50/50tohover:bg-muted/50, which will provide consistent hover effects across light and dark themes.
109-109: LGTM! Status text styling updated consistently.Both "scheduled" and "completed" status labels now use
text-muted-foregroundinstead oftext-zinc-500, ensuring proper readability in all theme modes.Also applies to: 117-117
154-154: LGTM! Icon colors migrated to semantic tokens.All CreditCard and DollarSign icons have been consistently updated from
text-zinc-600totext-muted-foregroundin both error states and normal rendering, maintaining visual consistency across the theme system.Also applies to: 158-158, 220-220, 224-224
231-233: LGTM! Filter UI updated with semantic tokens.The filter icon and label have been properly updated to use
text-muted-foregroundandtext-foregroundrespectively, maintaining the correct visual hierarchy in the filtering interface.
256-256: LGTM! Card border updated to semantic token.The card border has been changed from
border-zinc-100toborder-border, ensuring consistent border styling across all themes.
267-269: LGTM! Empty state icon updated for theming.The CreditCard icon in the empty state now uses
text-muted-foreground, maintaining visual consistency with the overall design system.src/components/subscription-plans/blocks/payments-table.tsx (8)
64-64: LGTM! Row hover state updated consistently.The table row hover background has been updated from
hover:bg-zinc-50/50tohover:bg-muted/50, matching the pattern used in other tables throughout the application.
78-80: LGTM! Payment number subtext updated for theming.The "of X" subtext now uses
text-muted-foregroundinstead oftext-zinc-500, ensuring proper readability across all themes.
92-92: LGTM! Link styling updated to semantic tokens.The "View Request" link now uses
text-primaryandhover:text-primary/80instead of hardcoded blue colors, which will properly adapt to different theme configurations.
98-98: LGTM! N/A text updated with semantic token.The fallback "N/A" text now uses
text-muted-foregroundinstead oftext-zinc-400, maintaining consistency with other muted text elements.
130-130: LGTM! Stat card icons updated consistently.All Receipt and DollarSign icons in both error states and normal rendering have been updated from hardcoded colors to
text-muted-foreground, ensuring proper theming support.Also applies to: 134-134, 191-191, 195-195
202-204: LGTM! Filter UI styling updated properly.The filter icon and label have been updated to use
text-muted-foregroundandtext-foregroundrespectively, maintaining proper visual hierarchy in the theme system.
222-222: LGTM! Card border updated to semantic token.The card wrapper now uses
border-borderinstead ofborder-zinc-100, ensuring consistent border appearance across themes.
233-235: LGTM! Empty state icon updated for theming.The CreditCard icon in the empty state now uses
text-muted-foreground, maintaining visual consistency throughout the application.src/components/subscription-plans/blocks/subscription-plan-link.tsx (9)
121-121: LGTM! Card styling comprehensively updated for theming.The card has been properly updated to use semantic design tokens:
bg-card,border-border, andtext-card-foreground. This ensures the component will adapt correctly to both light and dark themes.
126-126: LGTM! Plan title updated to semantic token.The plan label now uses
text-foregroundinstead oftext-zinc-900, ensuring proper contrast and readability across all themes.
127-127: LGTM! Metadata styling updated consistently.The subscriber and revenue information now uses
text-muted-foregroundinstead oftext-zinc-600, maintaining proper visual hierarchy in the theme system.
140-140: LGTM! Plan details text updated for theming.The plan description paragraph now uses
text-muted-foregroundinstead of hardcoded colors, ensuring proper readability across themes.
147-147: LGTM! Code block styling updated with semantic tokens.The URL code block now uses
text-muted-foregroundfor text andbg-mutedfor background instead of hardcoded zinc colors, ensuring proper contrast in both light and dark modes.
156-156: LGTM! Action button hover states updated.Both the copy and external link buttons now use
hover:bg-mutedinstead of hardcoded hover colors, providing consistent interaction feedback across themes.Also applies to: 166-166
160-160: LGTM! Icon colors updated to semantic tokens.The Copy and ExternalLink icons now use
text-muted-foregroundinstead of hardcoded colors, maintaining visual consistency in the action buttons.Also applies to: 170-170
181-181: LGTM! Delete button hover enhanced for dark mode.The delete button hover state has been updated to include dark mode support (
dark:hover:bg-red-950) while maintaining the light mode red background, ensuring proper visual feedback in both themes.
215-215: LGTM! Destructive action button updated to semantic tokens.The deactivation button now uses
bg-destructive,text-destructive-foreground, andhover:bg-destructive/90instead of hardcoded red classes, ensuring proper theming support for destructive actions.src/components/batch-payout.tsx (1)
262-289: Dark-mode token migration looks consistent and clean.Nice pass converting to semantic tokens (bg-card, text-card-foreground, text-muted-foreground, border-border, bg-muted, bg-primary, etc.). This will travel well across themes and aligns with the broader PR direction. No functional changes introduced in these sections.
Also applies to: 298-330, 333-344, 350-361, 381-405, 415-418, 446-461, 463-466, 476-489, 509-514, 516-573, 579-585
src/components/invoice-me-link.tsx (1)
59-63: LGTM: copy/open actions use muted tokens and hover states.Consistent with the new design system; nice touch keeping icons at text-muted-foreground and using hover:bg-muted for affordance.
Also applies to: 69-77
bassgeta
left a comment
bassgeta
left a comment
There was a problem hiding this comment.
Alright thanks a bunch for the updates! Just got around to checking this PR out again.
Let's go over the issues I've pointed out last time first:
- Invoice me links are now correctly styled ✔️
2. The delete modal from invoice me in now in light mode, probably not intended ✖️
3.User popup is now in dark mode ✔️
4. The `/invoices/:id` page now looks nice ✔️
The dark mode blip doesn't happen to me anymore, nice job sorting that out!
There is one more issue similar to the blip we had before, but now it at least resolves itself.
When I load any page in dark mode initially, it shows up in a half light and half dark mode state, but resolves itself after all the data is filled in.
initial-load-dark-mode-2025-09-01_11.46.41.mp4
Do you think you can solve this too?
Reproduction steps: Just set your theme to dark and reload any page
Also needs a rebase, let me know if I can be of help there too 🙏
hello , apologies but have been really busy in the past few weeks , kindly rebase and i will take a look! |
bassgeta
force-pushed
the
feature-dark-mode
branch
from
9e8bd27 to
cec58e5
Compare
Hey @mayur1377 , apologies that it took so long to rebase this, it's been a really busy couple of weeks. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/background-wrapper.tsx (1)
28-69: Fix initial light/dark “blip” (hydrate after theme resolves).The component renders light assets before next-themes resolves, causing the reported half light/half dark flash. Gate rendering until mounted and rely on resolvedTheme; also prefer bg-background over hex.
-import { useTheme } from "next-themes"; -import type { ReactNode } from "react"; +import { useTheme } from "next-themes"; +import type { ReactNode } from "react"; +import { useEffect, useState } from "react"; @@ - const { theme } = useTheme(); + const { resolvedTheme } = useTheme(); + const [mounted, setMounted] = useState(false); + useEffect(() => setMounted(true), []); @@ - const isDark = theme === "dark"; + const isDark = resolvedTheme === "dark"; @@ - return ( - <div - className={`min-h-screen relative overflow-hidden ${isDark ? "bg-[#0A0A0A]" : "bg-[#FAFAFA]"}`} - > + if (!mounted) { + return ( + <div className="min-h-screen bg-background"> + <div className="relative min-h-screen flex flex-col">{children}</div> + </div> + ); + } + + return ( + <div className="min-h-screen relative overflow-hidden bg-background"> @@ - {/* Decorative elements - only show in light mode */} - {!isDark && ( + {/* Decorative elements - only show in light mode */} + {!isDark && ( @@ - <div - className="absolute inset-0" - style={{ - backgroundImage: isDark - ? "radial-gradient(circle at 1px 1px, #1f1f1f 1px, transparent 0)" - : "radial-gradient(circle at 1px 1px, #e5e5e5 1px, transparent 0)", - backgroundSize: "40px 40px", - }} - /> + <div + className="absolute inset-0" + style={{ + backgroundImage: isDark + ? "radial-gradient(circle at 1px 1px, #1f1f1f 1px, transparent 0)" + : "radial-gradient(circle at 1px 1px, #e5e5e5 1px, transparent 0)", + backgroundSize: "40px 40px", + }} + />Also applies to: 93-96, 65-67
src/components/direct-payout.tsx (1)
146-153: Don’t assume payment tx is index 1 when approval is neededOrder from the API may change; use explicit indices and guard missing metadata.
- const approvalIndex = paymentData.metadata.approvalTransactionIndex; - - const approvalTransaction = await signer.sendTransaction( - paymentData.transactions[approvalIndex], - ); + const approvalIndex = paymentData.metadata?.approvalTransactionIndex; + if (typeof approvalIndex !== "number") { + throw new Error("Missing approvalTransactionIndex in metadata"); + } + const approvalTransaction = await signer.sendTransaction( + paymentData.transactions[approvalIndex], + ); await approvalTransaction.wait(); } toast.info("Sending payment", { description: "Please confirm the transaction in your wallet", }); - const paymentTransaction = await signer.sendTransaction( - paymentData.transactions[isApprovalNeeded ? 1 : 0], - ); + const paymentIndex = + paymentData.metadata?.paymentTransactionIndex ?? + // pick first non-approval tx as a fallback + (typeof paymentData.metadata?.approvalTransactionIndex === "number" + ? paymentData.transactions + .map((_, i) => i) + .filter((i) => i !== paymentData.metadata!.approvalTransactionIndex)[0] + : 0); + const paymentTxRequest = paymentData.transactions[paymentIndex]; + if (!paymentTxRequest) throw new Error("Missing payment transaction"); + const paymentTransaction = await signer.sendTransaction(paymentTxRequest);Also applies to: 159-161
src/components/view-recurring-payments/view-recurring-payments.tsx (1)
305-307: Hardcoded red breaks theming; use destructive tokensReplace fixed reds with semantic destructive classes for dark-mode correctness and consistency.
- className="bg-red-600 hover:bg-red-700" + className="bg-destructive text-destructive-foreground hover:bg-destructive/90"Optionally consider a Button variant inside AlertDialogAction if your UI kit supports it.
🧹 Nitpick comments (13)
src/components/footer.tsx (1)
6-6: Ensure link contrast inside muted text.Because the whole row is muted, consider overriding the link color for accessibility/affordance.
Example change outside this hunk:
- <Link href="https://request.network" className="underline"> + <Link href="https://request.network" className="underline text-foreground hover:text-primary"> Request Network </Link>src/app/dashboard/layout.tsx (1)
28-33: LGTM: Improved consistency with design systemThe migration from a plain Link to a Button component with
asChildis well-executed and maintains the same functionality while improving consistency with the design system. The semantic approach usingvariant="default"will properly support the dark mode theme changes mentioned in the PR objectives.Minor observation: There's a redundant
mr-2class on the PlusCircle icon (line 30) when the parent Button already hasgap-2spacing, but this doesn't affect functionality.Consider removing the redundant margin class for cleaner styling:
- <PlusCircle className="mr-2 h-4 w-4" /> + <PlusCircle className="h-4 w-4" />src/components/subscription-plans/subscription-plan-tabs.tsx (1)
46-49: Add live region semantics to the loading fallback.Expose status to screen readers to avoid a silent wait.
- <div className="flex items-center justify-center w-full h-40"> + <div + role="status" + aria-live="polite" + aria-busy="true" + className="flex items-center justify-center w-full h-40" + > <Loader2 className="h-6 w-6 animate-spin text-muted-foreground" /> <span className="ml-2 text-muted-foreground">Loading...</span> </div>src/components/short-address.tsx (1)
28-32: Add an explicit accessible name to the copy button.The visible text is a shortened address; add aria-label to clarify the action.
- className={`text-muted-foreground hover:text-foreground transition-colors flex items-center gap-2 ${className}`} + aria-label={`Copy address ${shortAddress}`} + className={`text-muted-foreground hover:text-foreground transition-colors flex items-center gap-2 ${className}`}src/components/invoice-form/invoice-form.tsx (1)
1028-1032: Prefer Button variants over manual color classes.Leverage the design system to avoid drift and ensure hover/focus consistency.
- <Button - type="submit" - className="bg-primary hover:bg-primary/80 text-primary-foreground transition-colors" - disabled={isLoading || isSubmitting || invoiceCreated} - > + <Button + type="submit" + variant="default" + disabled={isLoading || isSubmitting || invoiceCreated} + >src/components/payment-route.tsx (1)
75-85: Expose selection state for accessibility.Reflect selected state for assistive tech.
- <button + <button + aria-pressed={variant === "selected" || isSelected} type="button" onClick={onClick}src/components/payment-section.tsx (1)
467-477: Replace remaining hardcoded amber with a semantic token.Align with the rest of the migration.
- <div className="mt-2 text-sm text-amber-700"> + <div className="mt-2 text-sm text-warning"> Note: This address belongs to the Request Network Foundation for processing your crypto-to-fiat payment. </div>src/components/ui/table/empty-state.tsx (1)
17-21: Tokenized colors: good; add aria-hidden for decorative iconNice switch to semantic tokens. If the icon is decorative, hide it from SR to reduce noise.
- <div className="h-12 w-12 rounded-full bg-muted flex items-center justify-center mb-4"> + <div + className="h-12 w-12 rounded-full bg-muted flex items-center justify-center mb-4" + aria-hidden="true" + >src/components/create-recurring-payment/create-recurring-payment.tsx (2)
46-53: Minor a11y: announce loading stateAdd role="status" and a visually-hidden text for screen readers.
- <div className="flex flex-col items-center justify-center space-y-4"> + <div className="flex flex-col items-center justify-center space-y-4" role="status" aria-live="polite"> <Loader2 className="h-10 w-10 animate-spin text-muted-foreground" /> - <p className="text-muted-foreground"> - Initializing payment system... - </p> + <p className="text-muted-foreground">Initializing payment system...</p> + <span className="sr-only">Loading</span> </div>
58-109: Stepper duplicated across components—extract small StepperSame stepper logic here and in DirectPayment. Extract a tiny Stepper to keep styles/ARIA consistent.
src/components/view-recurring-payments/view-recurring-payments.tsx (1)
93-95: Minor a11y: make loading text liveAnnounce loading to SRs.
- <p className="text-muted-foreground"> - Loading recurring payments... - </p> + <p className="text-muted-foreground" role="status" aria-live="polite"> + Loading recurring payments... + </p>src/components/invoice-me-link.tsx (2)
84-84: Simplify delete trigger button styling.The current approach uses explicit dark mode classes which creates redundancy and potential inconsistency. Consider using semantic tokens consistently.
Apply this diff to simplify the delete trigger button styling:
- className="h-8 w-8 p-0 bg-background text-foreground hover:bg-muted dark:bg-foreground dark:text-background dark:hover:bg-muted" + className="h-8 w-8 p-0 hover:bg-muted"- <Trash2 className="h-4 w-4" /> + <Trash2 className="h-4 w-4 text-muted-foreground" />Also applies to: 87-87
90-90: Simplify alert dialog styling with semantic tokens.The manual dark mode class handling creates maintenance overhead and potential inconsistency. The design system should handle theme variants automatically through semantic tokens.
Apply this diff to use consistent semantic tokens:
- <AlertDialogContent className="bg-background text-foreground dark:bg-foreground dark:text-background"> + <AlertDialogContent>- <AlertDialogCancel className="bg-background text-foreground dark:bg-foreground dark:text-background"> - Cancel - </AlertDialogCancel> + <AlertDialogCancel> + Cancel + </AlertDialogCancel>Also applies to: 101-103
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (44)
src/app/dashboard/layout.tsx(2 hunks)src/app/i/[id]/page.tsx(1 hunks)src/app/invoices/[ID]/page.tsx(8 hunks)src/app/invoices/create/page.tsx(1 hunks)src/app/layout.tsx(2 hunks)src/app/subscription-plans/page.tsx(1 hunks)src/components/background-wrapper.tsx(3 hunks)src/components/batch-payout.tsx(17 hunks)src/components/compliance-status.tsx(1 hunks)src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx(2 hunks)src/components/create-recurring-payment/create-recurring-payment.tsx(5 hunks)src/components/dashboard/blocks/invoice-row.tsx(5 hunks)src/components/dashboard/invoices-received.tsx(6 hunks)src/components/dashboard/invoices-sent.tsx(2 hunks)src/components/dashboard/subscriptions.tsx(5 hunks)src/components/direct-payout.tsx(10 hunks)src/components/footer.tsx(1 hunks)src/components/header.tsx(4 hunks)src/components/invoice-form/invoice-form.tsx(2 hunks)src/components/invoice-me-link.tsx(3 hunks)src/components/invoice-me-links.tsx(3 hunks)src/components/invoice-preview.tsx(7 hunks)src/components/mode-toggle.tsx(1 hunks)src/components/multi-currency-stat-card.tsx(1 hunks)src/components/payment-route.tsx(4 hunks)src/components/payment-section.tsx(10 hunks)src/components/payment-secured-using-request.tsx(1 hunks)src/components/short-address.tsx(1 hunks)src/components/stat-card.tsx(1 hunks)src/components/subscription-plan-preview.tsx(5 hunks)src/components/subscription-plans/blocks/payments-table.tsx(7 hunks)src/components/subscription-plans/blocks/subscribers-table.tsx(6 hunks)src/components/subscription-plans/blocks/subscription-plan-link.tsx(5 hunks)src/components/subscription-plans/blocks/subscription-plans-list.tsx(3 hunks)src/components/subscription-plans/subscription-plan-tabs.tsx(1 hunks)src/components/ui/table/empty-state.tsx(1 hunks)src/components/ui/table/pagination.tsx(2 hunks)src/components/ui/table/table-head-cell.tsx(1 hunks)src/components/ui/tooltip.tsx(1 hunks)src/components/user-menu.tsx(2 hunks)src/components/view-recurring-payments/blocks/completed-payments.tsx(1 hunks)src/components/view-recurring-payments/blocks/frequency-badge.tsx(1 hunks)src/components/view-recurring-payments/blocks/status-badge.tsx(1 hunks)src/components/view-recurring-payments/view-recurring-payments.tsx(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (28)
- src/components/dashboard/blocks/invoice-row.tsx
- src/components/create-recurring-payment/blocks/create-recurring-payment-form.tsx
- src/components/view-recurring-payments/blocks/completed-payments.tsx
- src/app/layout.tsx
- src/components/dashboard/invoices-sent.tsx
- src/components/dashboard/invoices-received.tsx
- src/components/mode-toggle.tsx
- src/app/subscription-plans/page.tsx
- src/components/subscription-plans/blocks/subscribers-table.tsx
- src/components/payment-secured-using-request.tsx
- src/app/i/[id]/page.tsx
- src/components/view-recurring-payments/blocks/frequency-badge.tsx
- src/components/ui/table/pagination.tsx
- src/components/ui/tooltip.tsx
- src/components/invoice-preview.tsx
- src/components/subscription-plan-preview.tsx
- src/components/batch-payout.tsx
- src/components/invoice-me-links.tsx
- src/components/dashboard/subscriptions.tsx
- src/app/invoices/[ID]/page.tsx
- src/app/invoices/create/page.tsx
- src/components/compliance-status.tsx
- src/components/subscription-plans/blocks/subscription-plans-list.tsx
- src/components/multi-currency-stat-card.tsx
- src/components/stat-card.tsx
- src/components/ui/table/table-head-cell.tsx
- src/components/subscription-plans/blocks/subscription-plan-link.tsx
- src/components/user-menu.tsx
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-05-19T13:00:48.790Z
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/components/invoice-form.tsx:316-319
Timestamp: 2025-05-19T13:00:48.790Z
Learning: The handleFormSubmit function in src/components/invoice-form.tsx correctly uses data.clientEmail from the form submission data to find matching payers, which is the proper implementation.
Applied to files:
src/components/invoice-form/invoice-form.tsx
📚 Learning: 2025-02-20T10:26:33.809Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#21
File: src/components/background-wrapper.tsx:31-31
Timestamp: 2025-02-20T10:26:33.809Z
Learning: Dynamic Tailwind class interpolation for gradient classes (from-${color} and to-${color}) in the BackgroundWrapper component is intentionally implemented this way and doesn't require a mapping object solution.
Applied to files:
src/components/background-wrapper.tsx
📚 Learning: 2025-02-27T13:58:33.590Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#23
File: src/components/direct-payment.tsx:63-89
Timestamp: 2025-02-27T13:58:33.590Z
Learning: The AppKit wallet module requires initialization time before it's ready for use. A 2-second delay is intentionally used in the DirectPayment component to ensure the wallet is fully initialized before allowing interactions.
Applied to files:
src/components/create-recurring-payment/create-recurring-payment.tsxsrc/components/direct-payout.tsx
📚 Learning: 2025-03-25T09:31:47.519Z
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#42
File: src/components/payment-section.tsx:534-544
Timestamp: 2025-03-25T09:31:47.519Z
Learning: In the PaymentSection component of EasyInvoice, selectedRoute is automatically set to the first route when payment routes are loaded, and users can only switch between routes, not deselect them entirely.
Applied to files:
src/components/payment-section.tsx
📚 Learning: 2025-08-08T09:52:43.700Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#117
File: src/app/not-found.tsx:29-67
Timestamp: 2025-08-08T09:52:43.700Z
Learning: Repository: RequestNetwork/easy-invoice
Context: src/app/not-found.tsx (Next.js, TypeScript/React)
Learning: It is acceptable (by current product decision) for the NotFound page to display links to /invoices/create and /subscription-plans even though these routes are auth-gated. Rationale: prevent the 404 from feeling empty; most users are authenticated. No lock icons or conditional rendering required for now; can be revisited later as an enhancement.
Applied to files:
src/components/header.tsx
🧬 Code graph analysis (5)
src/components/payment-section.tsx (1)
src/lib/constants/currencies.ts (2)
MAINNET_CURRENCIES(2-18)formatCurrencyLabel(94-139)
src/components/view-recurring-payments/view-recurring-payments.tsx (2)
src/components/ui/card.tsx (4)
Card(79-79)CardContent(79-79)CardHeader(79-79)CardTitle(79-79)src/components/ui/table/table.tsx (1)
TableCell(115-115)
src/components/header.tsx (1)
src/components/mode-toggle.tsx (1)
ModeToggle(6-40)
src/app/dashboard/layout.tsx (1)
src/components/ui/button.tsx (1)
Button(56-56)
src/components/subscription-plans/blocks/payments-table.tsx (1)
src/components/multi-currency-stat-card.tsx (1)
MultiCurrencyStatCard(12-41)
🔇 Additional comments (28)
src/components/footer.tsx (2)
6-6: Semantic token swap looks good for dark mode.text-muted-foreground aligns with the new theme tokens and will adapt across themes.
6-6: Confirmed — --muted-foreground defined for both themes and wired into Tailwindglobals.css defines --muted-foreground in both theme blocks, and tailwind.config.* maps muted.foreground → hsl(var(--muted-foreground)). No changes needed.
src/app/dashboard/layout.tsx (1)
5-5: LGTM: Clean import additionThe Button component import follows the established pattern and aligns with the project's design system.
src/components/header.tsx (6)
3-3: LGTM: ModeToggle import added correctly.The import is properly structured and follows the established pattern for component imports in this file.
24-30: LGTM: Logo styling updated for dark mode support.The migration from hardcoded colors to semantic theme tokens is well-implemented:
bg-black→bg-primaryfor the logo containertext-white→text-primary-foregroundfor the "EI" texttext-zinc-900→text-foregroundfor the "EasyInvoice" labelThis ensures proper contrast and theming across light and dark modes.
37-64: LGTM: Navigation links updated with semantic theme tokens.The consistent migration from zinc-based colors to semantic theme tokens (
text-foregroundandhover:text-muted-foreground) ensures proper readability and accessibility across both light and dark themes.
68-68: LGTM: External demo link styling improved for theming.The update from
decoration-zinc-400todecoration-borderwith semantic color tokens maintains visual hierarchy while supporting both theme modes.
82-82: LGTM: Demo button styling updated for theme consistency.The migration from hardcoded
bg-black hover:bg-black/80 text-whiteto semanticbg-primary hover:bg-primary/80 text-primary-foregroundensures the button adapts properly to both light and dark themes while maintaining the intended visual prominence.
93-93: LGTM: ModeToggle component integrated correctly.The ModeToggle is properly placed at the end of the navigation controls, maintaining the existing layout hierarchy while providing easy access to theme switching functionality.
src/components/invoice-form/invoice-form.tsx (1)
591-600: LGTM on dialog theming.Spinner and copy are aligned with tokens; reads well in both themes.
src/components/payment-route.tsx (2)
43-45: Fix tinted-badge text colors and verify token existence.
- On 10% tints, use the base token (text-*-DEFAULT), not foreground, for contrast.
- Confirm warning/success tokens exist; prior scans showed they were missing.
- bgColor: "bg-warning/10", - textColor: "text-warning-foreground", + bgColor: "bg-warning/10", + textColor: "text-warning",- bgColor: "bg-primary/10", - textColor: "text-primary-foreground", + bgColor: "bg-primary/10", + textColor: "text-primary",- bgColor: "bg-success/10", - textColor: "text-success-foreground", + bgColor: "bg-success/10", + textColor: "text-success",Run to verify tokens:
#!/bin/bash set -euo pipefail echo "Searching Tailwind config for success/warning tokens…" fd -a -H -t f 'tailwind.config.*' | xargs -I{} sh -c 'echo "== {} =="; rg -n "success|warning" "{}" || true' echo -e "\nSearching CSS vars…" rg -n --glob '!**/node_modules/**' --hidden --no-ignore -C2 '\-\-(success|warning)' . echo -e "\nUsage audit…" rg -nP -C1 '\b(bg|text)-(success|warning)(/\d+)?\b' src || trueAlso applies to: 51-53, 59-61
120-123: Replace hardcoded palette colors with semantic tokens.These won’t adapt in dark mode.
- <span className="text-amber-700"> + <span className="text-warning"> {route.fee} {isDirectPayment ? nativeToken : route.token} fee </span>- <span className="text-purple-600"> + <span className="text-primary"> {platformFee?.percentage}% {route.token} platform fee </span>Also applies to: 129-133
src/components/subscription-plans/blocks/payments-table.tsx (2)
68-103: LGTM: token migration looks consistent.Row hover, subtext, link, and empty state all use semantic tokens; good dark-mode support.
210-249: LGTM: container and empty-state theming align with design tokens.Card border and icons conform to border/text-muted-foreground. No functional changes.
src/components/payment-section.tsx (3)
418-433: Define success/warning tokens or replace with palette utilities.Multiple uses of bg-success/warning and text-*-foreground assume tokens that may not exist; prior scans showed they’re missing.
Option A (add tokens in Tailwind config):
- colors.success.{DEFAULT,foreground}
- colors.warning.{DEFAULT,foreground}
Option B (palette fallback):
-<div className="p-4 bg-success/10 border border-success rounded-lg"> - <h3 className="font-semibold text-success mb-2">Secure Payment</h3> - <p className="text-sm text-success-foreground"> +<div className="p-4 bg-emerald-100/20 dark:bg-emerald-950/20 border border-emerald-400/50 rounded-lg"> + <h3 className="font-semibold text-emerald-700 dark:text-emerald-300 mb-2">Secure Payment</h3> + <p className="text-sm text-emerald-700 dark:text-emerald-300">-<div className="p-4 bg-warning/10 border border-warning rounded-lg"> - <h3 className="font-semibold text-warning mb-2 flex items-center gap-2"> - ... - <p className="text-sm text-warning-foreground"> +<div className="p-4 bg-amber-100/20 dark:bg-amber-950/20 border border-amber-400/50 rounded-lg"> + <h3 className="font-semibold text-amber-700 dark:text-amber-300 mb-2 flex items-center gap-2"> + ... + <p className="text-sm text-amber-700 dark:text-amber-300">Verification script:
#!/bin/bash set -euo pipefail fd -a -H -t f 'tailwind.config.*' | xargs -I{} sh -c 'echo "== {} =="; rg -n "success|warning" "{}" || true' rg -n --glob '!**/node_modules/**' --hidden --no-ignore -C2 '\-\-(success|warning)' . rg -nP -C1 '\b(bg|text)-(success|warning)(/\d+)?\b' src || trueAlso applies to: 427-433
598-607: Fix token typos: use text-muted-foreground.text-muted isn’t a shadcn token and won’t adapt correctly.
- <Loader2 className="w-6 h-6 text-muted animate-spin" /> + <Loader2 className="w-6 h-6 text-muted-foreground animate-spin" /> @@ - <p className="text-sm text-muted"> + <p className="text-sm text-muted-foreground"> Finding the best payment routes for your wallet </p> @@ - <AlertCircle className="w-6 h-6 text-muted" /> + <AlertCircle className="w-6 h-6 text-muted-foreground" /> @@ - <p className="text-sm text-muted"> + <p className="text-sm text-muted-foreground"> There are currently no available payment routes for this transaction. </p> @@ - <div className="text-sm text-muted mb-3"> + <div className="text-sm text-muted-foreground mb-3"> Available Payment Routes </div>Also applies to: 615-622, 646-648
665-671: LGTM on primary action button.Uses semantic tokens and guards with hasRoutes/paymentProgress properly.
src/components/create-recurring-payment/create-recurring-payment.tsx (1)
38-40: Token migration + 2s AppKit readiness: solidStyling aligns with tokens; keeping the 2s delay matches prior learning about AppKit init time.
Also applies to: 49-52, 65-75, 85-87, 100-102, 113-114
src/components/view-recurring-payments/view-recurring-payments.tsx (3)
90-99: Tokenization pass reads consistent across statesLoading/empty/error and pagination now use semantic tokens—good consistency for dark mode.
Also applies to: 106-116, 140-157, 171-178, 321-345
231-234: Microcopy tokens: goodMuted sublabels for “scheduled”/“completed” improve contrast balance in dark mode.
Also applies to: 241-244
283-313: Ensure consistent modal theming
Verify that all AlertDialog components—including Content and Overlay—use tokenized bg/text classes (no hardcodedbg-…/text-…) and that your portal container inherits thehtml.darkclass so dark mode applies everywhere.src/components/direct-payout.tsx (2)
129-134: No change required — ethers v5 confirmed
package.json lists ethers ^5.7.2, so using ethers.providers.Web3Provider(...) and getSigner() (v5) is correct; do not switch to BrowserProvider/await getSigner.
197-199: ThemeProvider confirmed in layout.tsx — approving (optional tweak)src/app/layout.tsx contains and , so the theme-token migration looks consistent. Optional: add disableTransitionOnChange to the ThemeProvider and include next-themes' SSR theme script if you still see an initial dark-mode flash.
src/components/invoice-me-link.tsx (5)
45-45: LGTM: Clean card styling with semantic theme support.The removal of explicit
bg-whiteallows the card to inherit proper background colors from the theme system while maintaining the hover and transition effects.
49-49: LGTM: Proper semantic color token usage.The migration from
text-zinc-900totext-foregroundensures the title text adapts correctly to both light and dark themes.
51-51: LGTM: Consistent muted styling for code display.The changes to
text-muted-foregroundandbg-muted/50provide appropriate contrast and readability for the URL code block in both theme modes.
59-59: LGTM: Consistent button hover states and icon colors.The migration to
hover:bg-mutedandtext-muted-foregroundfor icons creates a cohesive experience across all action buttons while ensuring proper theme compatibility.Also applies to: 62-62, 68-68, 76-76
117-117: LGTM: Proper destructive action styling.The use of
bg-destructive,text-destructive-foreground, andhover:bg-destructive/90follows semantic token conventions and ensures the delete action is clearly distinguished in both themes.
- Added `enableColorScheme` and `disableTransitionOnChange` to ThemeProvider for better theme management. - Implemented a mounted state in BackgroundWrapper to prevent theme-dependent rendering during initial load. - Updated background styles to use CSS variables for improved consistency across themes. - Simplified class names in InvoiceMeLink component for cleaner styling.
done , kindly take a look at it!! |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/components/background-wrapper.tsx (4)
29-35: Use resolvedTheme to correctly handle system theme.theme can be "system"; resolvedTheme yields the effective value.
Apply:
- const { theme } = useTheme(); + const { resolvedTheme } = useTheme();
55-60: Remove unused dark palette entries or actually use them; also hoist the color map.These entries aren’t referenced because gradients are hidden in dark mode; the map is re-created every render.
Apply (if not using these soon):
- // Dark mode colors - "zinc-800": "#27272a", - "zinc-900": "#18181b", - "slate-800": "#1e293b", - "slate-900": "#0f172a",Optionally move the colors object to a module‑level const to avoid per‑render allocation.
78-101: Decoratives: add aria-hidden and pointer-events-none; confirm stacking order.Prevents accidental hit‑testing and keeps them out of accessibility tree.
Apply:
- <div className="absolute top-0 right-0 w-[600px] h-[600px] -translate-y-1/2 translate-x-1/2"> + <div className="absolute top-0 right-0 w-[600px] h-[600px] -translate-y-1/2 translate-x-1/2 pointer-events-none" aria-hidden="true"> ... - <div className="absolute bottom-0 left-0 w-[600px] h-[600px] translate-y-1/2 -translate-x-1/2"> + <div className="absolute bottom-0 left-0 w-[600px] h-[600px] translate-y-1/2 -translate-x-1/2 pointer-events-none" aria-hidden="true">If the intent is for gradients to sit behind the dot layer, current order is correct; otherwise add z-indexes explicitly.
102-111: The dark/light backgroundImage values are identical; simplify or differentiate.Either use one value or tune contrast per theme. Also consider pointer-events-none and aria-hidden for the pattern layer.
Apply one of:
- Simplify:
- backgroundImage: isDark - ? "radial-gradient(circle at 1px 1px, hsl(var(--muted)) 1px, transparent 0)" - : "radial-gradient(circle at 1px 1px, hsl(var(--muted)) 1px, transparent 0)", + backgroundImage: "radial-gradient(circle at 1px 1px, hsl(var(--muted) / 0.10) 1px, transparent 0)",
- Or differentiate (example alphas):
- backgroundImage: isDark - ? "radial-gradient(circle at 1px 1px, hsl(var(--muted)) 1px, transparent 0)" - : "radial-gradient(circle at 1px 1px, hsl(var(--muted)) 1px, transparent 0)", + backgroundImage: isDark + ? "radial-gradient(circle at 1px 1px, hsl(var(--muted) / 0.18) 1px, transparent 0)" + : "radial-gradient(circle at 1px 1px, hsl(var(--muted) / 0.08) 1px, transparent 0)",Additionally (outside the selected lines), consider:
- <div - className="absolute inset-0" + <div + className="absolute inset-0 pointer-events-none" style={{and add aria-hidden="true" to that element.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/layout.tsx(2 hunks)src/components/background-wrapper.tsx(3 hunks)src/components/invoice-me-link.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/invoice-me-link.tsx
- src/app/layout.tsx
🔇 Additional comments (2)
src/components/background-wrapper.tsx (2)
1-3: Client component + imports look correct.Appropriate for using hooks and next-themes.
67-77: Derive isDark from resolvedTheme to prevent initial dark-mode flash.- const isDark = theme === "dark"; + const isDark = resolvedTheme === "dark";
- Verify ThemeProvider (in a "use client" providers component inside app/layout.tsx): attribute="class", defaultTheme="system", enableSystem (optional disableTransitionOnChange). Add suppressHydrationWarning to the element so next-themes' pre-load/no‑flash script can avoid FOUC in production. Ensure resolvedTheme is destructured from useTheme and supported by your next-themes version.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
bassgeta
left a comment
bassgeta
left a comment
There was a problem hiding this comment.
My bad, upon further review I've found another bug 😅
Navigating between pages doesn't work anymore, I am getting this error:
So error 1 is:
SES_UNCAUGHT_EXCEPTION: Error: Rendered more hooks than during the previous render.
And error 2 is:
Warning: Cannot update a component (`HotReload`) while rendering a different component (`Router`). To locate the bad setState() call inside `Router`, follow the stack trace as described in https://reactjs.org/link/setstate-in-render Component Stack:
Router webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/components/app-router.js:207
ErrorBoundaryHandler webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/components/error-boundary.js:113
ErrorBoundary webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/components/error-boundary.js:160
AppRouter webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/components/app-router.js:585
ServerRoot webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/app-index.js:112
Root webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/app-index.js:117
[<anonymous code>:1:145535](chrome://devtools/content/webconsole/%3Canonymous%20code%3E)
it probably has something to do with you rendering different content until everything is loaded. This didn't happen in the previous version, so the culprit is in the latest commit.
…andling - Reintroduced Google Tag Manager in the layout component. - Enhanced background rendering logic in BackgroundWrapper to maintain DOM stability and improve visibility toggling for decorative elements based on theme state.
bassgeta
left a comment
bassgeta
left a comment
There was a problem hiding this comment.
Alright, nice job! Everything works as intended now, navigation works and everything looks exactly as it should, good stuff 👌
There is only one minor bug which can probably be fixed easily.
The ModeToggle component spits out a hydration error, and it also has one initial render issue - when dark mode is selected beforehand, upon refreshing the app the switcher toggles to light mode first and then immediately goes to dark mode.
initial-load-toggler-2025-09-24_09.57.49.mp4
Since you've done a ton of work on this already, adding some kind of placeholder to ModeToggle until the component is mounted (like you have done with the background wrapper), is going to be enough.
Sorry to nag you for this one last fix 🙏
There was a problem hiding this comment.
Hey!
I'm still getting the switching error in the ModeToggle component.
- Set your theme to dark
- Refresh the page and observe that the toggler is initially in light mode and gets changed to dark mode later on
I think that what you can do for the ModeToggle is that if the component isn't mounted, just render an empty button for the time being (SSR and themes are hard 😅 )
<Button
variant="ghost"
className="relative h-8 w-16 rounded-full bg-muted p-0 dark:bg-muted"
aria-label="Toggle dark mode"
disabled
/>Or something that will fill the space accordingly.
bassgeta
left a comment
bassgeta
left a comment
There was a problem hiding this comment.
Nice one 👌
After running through the app, I can confirm that there are no more visual glitcches and that the theme toggler does in fact display as disabled if we have dark mode enabled system wide 💯
Thanks for you hard work on this, now let's 
|
Congratulations, your pull request has been merged! Thank you for your valuable contribution to Request Network. As a reminder, every merged PR is automatically entered into our Best PR Initiative, offering a quarterly prize of $500. Your work significantly supports our project's growth, and we encourage you to continue engaging with our community. Additionally, if you want to build or add crypto payments and invoicing features, explore how our API can reduce deployment time from months to hours while offering advanced features. Book a call with our expert to learn more and fast-track your development. |
|
@mayur1377 We want to reward you REQ for your contributions to Request Network (this pull request) 🎉 To claim them, please send a DM to the Request Network account on X 🙏 |
3 participants
#46 : add dark mode , a few snapshots below
Summary by CodeRabbit
New Features
Style