✨ Dark mode #2087
✨ Dark mode #2087
Conversation
WalkthroughAdds a ThemeProvider and env-driven primary color, extends email/context styling, introduces a Hono private-polls API with API-key auth and time-slot generation (plus DB migration and scripts), removes several UI/avatar components, and performs a large design-token and locale-key migration. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HonoApp as Hono App (route.ts)
participant Auth as apiKeyAuth
participant TimeSlots as time-slots util
participant PollUtil as createPoll util
participant DB as Prisma
participant URL as shortUrl/absoluteUrl
Client->>HonoApp: POST /api/private/polls (Bearer token + payload)
HonoApp->>Auth: verify bearer token
Auth->>DB: SELECT api_keys WHERE prefix = ?
DB-->>Auth: api_key row
Auth->>Auth: validate secret vs hashed_key, check revoked/expired
Auth-->>HonoApp: authenticated { userId, apiKeyId }
HonoApp->>TimeSlots: parse/generate slots (dates or slots + timezone)
TimeSlots-->>HonoApp: deduped TimeSlot[] (<=100)
HonoApp->>PollUtil: createPoll({ userId, title, options })
PollUtil->>DB: INSERT poll, createMany options, watchers
DB-->>PollUtil: created poll id
PollUtil->>URL: generate adminUrl and inviteUrl
URL-->>PollUtil: urls
PollUtil-->>HonoApp: { id, adminUrl, inviteUrl }
HonoApp-->>Client: 201 Created { data: { id, adminUrl, inviteUrl } }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (9)**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{tsx,css,config.ts}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.tsx📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx,json}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{tsx,ts}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/web/src/components/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/web/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (apps/web/.cursor/rules/better-auth.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-11-25T11:03:55.173ZApplied to files:
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
lukevella
force-pushed
the
dark-theme
branch
from
e6e75c5 to
90ce9d2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/web/src/components/poll/manage-poll/finalize-poll-dialog.tsx (1)
1-1: Add the "use client" directive.This file uses client-side hooks (
useForm,React.useMemo,usePoll,useParticipants,useDayjs) and trpc mutations, which require client-side JavaScript execution. Without the "use client" directive, Next.js will treat this as a Server Component and fail at runtime.🔎 Proposed fix
+"use client"; + import { zodResolver } from "@hookform/resolvers/zod"; import { cn } from "@rallly/ui"; import { Button } from "@rallly/ui/button";Based on coding guidelines:
Add the "use client" directive to the top of any .tsx file that requires client-side javascript
apps/web/src/components/poll/manage-poll.tsx (1)
251-259: Good refactor to use variant prop, but fix icon styling.The change from
className="text-destructive"tovariant="destructive"is a good improvement that uses the component's proper API. However, theTrashIconon Line 257 applies styling directly via className, which violates the coding guidelines.As per coding guidelines: "The size and colour of an icon should be set by wrapping it with the component from @rallly/ui/icon which will give it the correct colour and size."
🔎 Proposed fix for icon styling
<DropdownMenuItem variant="destructive" onClick={() => { setShowDeletePollDialog(true); }} > - <TrashIcon className="size-4 opacity-75" /> + <Icon> + <TrashIcon /> + </Icon> <Trans i18nKey="delete" /> </DropdownMenuItem>apps/web/src/components/participant.tsx (1)
1-2: Missing "use client" directive.This component uses client-side hooks (
React.useStateandReact.useRef) but lacks the required "use client" directive at the top of the file. Without this directive, Next.js will attempt to render this as a server component, causing runtime errors.As per coding guidelines: "Add the 'use client' directive to the top of any .tsx file that requires client-side javascript."
🔎 Proposed fix
+"use client"; + import { cn } from "@rallly/ui"; import React from "react";apps/web/src/app/[locale]/(space)/settings/security/page.tsx (1)
57-67: Fix invalid Alert variant: "tip" is not a defined variant.The change from
variant="tip"tovariant="success"is critical—variant="tip"does not exist in the Alert component definition, which only supports:default,primary,info,warning,success, andnote.Note that
apps/web/src/app/[locale]/(space)/settings/billing/page-client.tsxalso uses the invalidvariant="tip"and requires the same fix. Additionally,apps/web/src/app/[locale]/(auth)/reset-password/components/reset-password-form.tsxuses the non-existentvariant="destructive"and also needs correction.packages/ui/src/avatar.tsx (1)
1-6: Add the "use client" directive and remove the unused seed prop.The
avatar.tsxcomponent is missing the "use client" directive at the top, which is required by the coding guidelines and present in all other shadcn-ui components in this package. Radix UI's Avatar component requires client-side JavaScript for proper fallback behavior.Additionally, the
seedprop is defined in theAvatarFallbacktype but never destructured or used in the component implementation. It spreads via...propsto Radix UI's Fallback element, which doesn't recognize it as a standard prop. Remove theseed?: string;from the type definition and update consumers (optimized-avatar-image.tsx,space-icon.tsx) to stop passing this unused prop.
🧹 Nitpick comments (9)
apps/web/src/app/[locale]/(space)/mobile-navigation.tsx (1)
16-22: Consider using the standard<Icon>component wrapper.Based on the coding guidelines, icon sizing and coloring should be handled by wrapping with the
<Icon>component from@rallly/ui/iconrather than a customSlotwrapper. This would improve consistency across the codebase.💡 Example refactor using the Icon component
+import { Icon } from "@rallly/ui/icon"; -function MobileNavigationIcon({ children }: { children: React.ReactNode }) { - return ( - <Slot className="size-5 text-muted-foreground group-hover:text-foreground group-focus:text-primary group-[.is-active]:text-primary"> - {children} - </Slot> - ); -} +function MobileNavigationIcon({ children }: { children: React.ReactNode }) { + return ( + <Icon className="group-hover:text-foreground group-focus:text-primary group-[.is-active]:text-primary"> + {children} + </Icon> + ); +}Note: Verify that the
<Icon>component supports the group-based state classes, or adjust the implementation accordingly.Based on learnings: Icons should be wrapped with the
<Icon>component from@rallly/ui/icon.apps/web/src/components/vote-summary-progress-bar.tsx (1)
60-60: Consider removing ineffective opacity modifiers.The
opacity-75 hover:opacity-100classes have no visual effect on abg-transparentbackground, since the element is already fully transparent. These modifiers only affect the visibility of opaque content.If the intent is to keep visual consistency with the other segments, consider using a semi-transparent color instead, such as
bg-gray-500/20, which would respond to the opacity modifiers. Otherwise, the opacity classes can be safely removed.🔎 Option 1: Remove ineffective opacity classes
<div - className="h-full bg-transparent opacity-75 hover:opacity-100" + className="h-full bg-transparent" style={{ width: `${(props.no.length / props.total) * 100}%`, }} />🔎 Option 2: Use a semi-transparent color that responds to opacity changes
<div - className="h-full bg-transparent opacity-75 hover:opacity-100" + className="h-full bg-gray-500/20 opacity-75 hover:opacity-100" style={{ width: `${(props.no.length / props.total) * 100}%`, }} />apps/web/src/components/forms/poll-options-form/month-calendar/time-picker.tsx (1)
31-49: Optional refactor: Remove variable shadowing in cursor declaration.The
cursorvariable is declared on line 31 and immediately redeclared on line 38 within theif (after)block, which shadows the outer declaration. The initial declaration is only used in theelseblock, making the code harder to follow.🔎 Suggested refactor to eliminate shadowing
const getOptions = React.useCallback(() => { if (!open) { return [dayjs(value).toISOString()]; } - let cursor = after - ? dayjs(after).add(15, "minutes") - : dayjs(value).startOf("day"); - const res: string[] = []; if (after) { let cursor = dayjs(after).add(15, "minutes"); while (cursor.diff(after, "hours") < 24) { res.push(cursor.toISOString()); cursor = cursor.add(15, "minutes"); } } else { - cursor = dayjs(value).startOf("day"); + let cursor = dayjs(value).startOf("day"); while (cursor.isSame(value, "day")) { res.push(cursor.toISOString()); cursor = cursor.add(15, "minutes"); } } return res; }, [after, open, value]);apps/web/src/features/password/components/password-strength-meter.tsx (1)
57-63: Consider adding dark mode variants for label colors.The label colors use
-500and-600variants without explicit dark mode styles. While these may work reasonably in both modes, adding explicit dark mode variants would ensure optimal contrast and consistency with the rest of the dark mode implementation.🔎 Suggested dark mode variants
className={cn("font-medium", { - "text-gray-500": quality === "veryWeak", + "text-gray-500 dark:text-gray-400": quality === "veryWeak", - "text-red-600": quality === "weak", + "text-red-600 dark:text-red-400": quality === "weak", - "text-amber-600": quality === "fair", + "text-amber-600 dark:text-amber-400": quality === "fair", - "text-green-600": quality === "good", + "text-green-600 dark:text-green-400": quality === "good", - "text-teal-600": quality === "strong", + "text-teal-600 dark:text-teal-400": quality === "strong", })}apps/web/src/components/participant.tsx (1)
16-32: Consider addressing the accessibility issue.The biome-ignore comment indicates there's a pending accessibility issue. The component uses mouse events on a non-interactive element without keyboard support, which prevents keyboard-only users from accessing the full truncated text.
Consider wrapping this in a button element or adding keyboard event handlers (e.g.,
onFocus/onBlur) to make the tooltip accessible via keyboard navigation.Would you like me to help generate a keyboard-accessible implementation?
packages/ui/src/switch.tsx (1)
16-16: LGTM! Dark mode support added.The Switch component now properly supports dark mode with:
- Lighter green shade (
green-400) for checked state in dark mode for better contrast- Semantic
bg-mutedtoken for unchecked state- Dark thumb styling (
dark:bg-gray-700)The implementation correctly adapts the component's appearance for dark mode.
💡 Optional: Consider using semantic tokens for thumb background
The thumb currently uses
bg-white(light mode) anddark:bg-gray-700(dark mode). For greater consistency with the theme system, consider using semantic tokens likebg-backgroundorbg-cardthat would automatically adapt to the theme.- "pointer-events-none flex h-full w-4 items-center justify-center rounded-full bg-white shadow-lg ring-0 transition-transform duration-100 data-[state=checked]:translate-x-4 data-[state=unchecked]:translate-x-0 dark:bg-gray-700", + "pointer-events-none flex h-full w-4 items-center justify-center rounded-full bg-background shadow-lg ring-0 transition-transform duration-100 data-[state=checked]:translate-x-4 data-[state=unchecked]:translate-x-0",This would make the component fully theme-aware, though the current implementation is perfectly functional.
Also applies to: 24-24
apps/web/src/utils/color.ts (1)
1-10: Consider supporting 3-digit hex codes.The regex only matches 6-digit hex colors (
#RRGGBB). IfPRIMARY_COLORmight be specified as a 3-digit shorthand (e.g.,#fff), parsing will fail and default to white foreground.🔎 Proposed fix to support 3-digit hex
function hexToRgb(hex: string): { r: number; g: number; b: number } | null { - const result = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(hex); - return result - ? { - r: Number.parseInt(result[1], 16), - g: Number.parseInt(result[2], 16), - b: Number.parseInt(result[3], 16), - } - : null; + // Expand 3-digit hex to 6-digit + const expandedHex = hex.replace( + /^#?([a-f\d])([a-f\d])([a-f\d])$/i, + (_, r, g, b) => `${r}${r}${g}${g}${b}${b}`, + ); + const result = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(expandedHex); + return result + ? { + r: Number.parseInt(result[1], 16), + g: Number.parseInt(result[2], 16), + b: Number.parseInt(result[3], 16), + } + : null; }apps/web/src/features/billing/components/pay-wall-dialog.tsx (1)
189-189: Consider using semantic tokens instead of hard-coded gray values.The styling uses hard-coded
bg-gray-100 dark:bg-gray-700andbg-gray-100 dark:bg-gray-900rather than semantic tokens likebg-mutedorbg-card. This is inconsistent with the token-based theming approach used elsewhere in this PR.🔎 Proposed fix using semantic tokens
- <div className="flex items-start justify-between gap-4 rounded-lg bg-gray-100 p-4 dark:bg-gray-700"> + <div className="flex items-start justify-between gap-4 rounded-lg bg-muted p-4">- <div className="hidden overflow-y-auto bg-gray-100 px-6 py-6 md:block dark:bg-gray-900"> + <div className="hidden overflow-y-auto bg-muted px-6 py-6 md:block">Also applies to: 244-244
apps/web/src/app/[locale]/layout.tsx (1)
80-92: Consider makingsuppressHydrationWarningconditional.The
suppressHydrationWarningattribute is always set totrue, but thestyleprop is only set whenprimaryColoris defined. This unnecessarily suppresses hydration warnings even when no dynamic styles are applied.🔎 Proposed optimization
<html lang={locale} className={inter.className} - suppressHydrationWarning={true} + suppressHydrationWarning={!!primaryColor} style={ primaryColor ? ({ "--primary": primaryColor, "--primary-foreground": getForegroundColor(primaryColor), } as React.CSSProperties) : undefined } >
apps/web/src/components/poll/desktop-poll/participant-row-form.tsx
Outdated
Show resolved
Hide resolved
Fixes Update colors Updates Update colors Update Update button
lukevella
force-pushed
the
dark-theme
branch
from
90ce9d2 to
d8fb699
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/ui/src/page-tabs.tsx (1)
42-42: UpdateTabsContentref type for consistency.
TabsContentstill uses the deprecatedReact.ElementRef, whileTabsTriggerwas correctly updated toReact.ComponentRefon Line 26. Apply the same update here for consistency.🔎 Proposed fix
const TabsContent = React.forwardRef< - React.ElementRef<typeof TabsPrimitive.Content>, + React.ComponentRef<typeof TabsPrimitive.Content>, React.ComponentPropsWithoutRef<typeof TabsPrimitive.Content> >(({ className, ...props }, ref) => (apps/web/src/components/forms/poll-details-form.tsx (1)
1-1: Add missing "use client" directive.This file uses client-side hooks (
useFormContext,useTranslation,useFormValidation) and requires the"use client"directive at the top.As per coding guidelines, "Add the 'use client' directive to the top of any .tsx file that requires client-side javascript".
🔎 Proposed fix
+"use client"; + import { FormField, FormItem, FormLabel, FormMessage } from "@rallly/ui/form";
♻️ Duplicate comments (6)
apps/web/src/components/participant.tsx (1)
30-31: Critical: Hardcoded white background breaks dark mode.The
hover:bg-whiteclass uses a hardcoded white background that will not adapt to dark mode. This is a blocker for the dark mode feature in this PR.Replace with a theme-aware semantic color token such as
hover:bg-gray-50or another appropriate background utility that automatically adapts between light and dark themes.🔎 Proposed fix
- "hover:-translate-x-2 hover:cursor-pointer hover:overflow-visible hover:whitespace-nowrap hover:rounded-md hover:bg-white hover:p-2": + "hover:-translate-x-2 hover:cursor-pointer hover:overflow-visible hover:whitespace-nowrap hover:rounded-md hover:bg-gray-50 hover:p-2": isTruncated,apps/landing/src/components/error-page.tsx (1)
26-29: Address previously flagged hardcoded color tokens.The icon (line 26) and description (line 29) still use hardcoded color tokens (
text-gray-400andtext-gray-600) instead of semantic tokens. This was flagged in a previous review and remains unaddressed. For dark mode to work properly, these should be updated to semantic tokens consistent with the web app's error component:
- Line 26:
text-gray-400→text-muted-foreground- Line 29:
text-gray-600→text-foregroundortext-muted-foregroundAdditionally, per coding guidelines and learnings, the FileSearchIcon should be wrapped with the
<Icon>component from@rallly/ui/icon.🔎 Proposed fix
+import { Icon } from "@rallly/ui/icon"; + const ErrorPage: React.FunctionComponent<ComponentProps> = ({ icon, title, description, }) => { const { t } = useTranslation(); return ( <div className="inset-0 flex h-full w-full items-center justify-center lg:absolute"> <div className="space-y-8"> <div className="space-y-4 text-center"> {icon || ( - <FileSearchIcon className="mb-4 inline-block size-24 text-gray-400" /> + <Icon size="lg"> + <FileSearchIcon className="mb-4 size-24" /> + </Icon> )} <div className="mb-2 font-bold text-3xl text-primary">{title}</div> - <p className="text-gray-600">{description}</p> + <p className="text-muted-foreground">{description}</p> </div>apps/web/src/features/space/components/space-tier.tsx (1)
20-28: Semantic token migration is good, but structural issues remain unresolved.The migration to semantic color tokens (
border-muted-border,bg-muted,text-muted-foreground,border-primary-border,bg-primary,text-primary-foreground) properly supports dark mode. However, the underlying structural issues flagged in the previous review still need to be addressed:
Inconsistent color handling: The
HandHeartIcon(line 21) inherits color from the parent'stext-muted-foreground, while theSparklesIcon(line 27) explicitly setstext-primary-foreground.Missing
<Icon>wrapper: Per coding guidelines and learnings, both icons should be wrapped with the<Icon>component from@rallly/ui/iconfor consistent sizing and color handling.Based on learnings, the
<Icon>component provides the correct color and size automatically.🔎 Proposed fix with consistent icon usage
+import { Icon } from "@rallly/ui/icon"; + export const SpaceTierIcon = ({ tier }: { tier: SpaceTier }) => { switch (tier) { case "hobby": return ( - <div className="inline-flex size-10 items-center justify-center rounded-md border border-muted-border bg-muted text-muted-foreground text-xs"> - <HandHeartIcon className="size-5" /> + <div className="inline-flex size-10 items-center justify-center rounded-md border border-muted-border bg-muted text-muted-foreground"> + <Icon> + <HandHeartIcon /> + </Icon> </div> ); case "pro": return ( - <div className="inline-flex size-10 items-center justify-center rounded-md border border-primary-border bg-primary text-xs"> - <SparklesIcon className="size-5 text-primary-foreground" /> + <div className="inline-flex size-10 items-center justify-center rounded-md border border-primary-border bg-primary text-primary-foreground"> + <Icon> + <SparklesIcon /> + </Icon> </div> ); } };apps/web/src/features/billing/components/pay-wall-dialog.tsx (1)
189-189: Typo: Straybclass in className.There's an errant
bclass that appears to be a leftover from an incomplete edit.🔎 Proposed fix
- <div className="b flex items-start justify-between gap-4 rounded-lg bg-gray-100 p-4 dark:bg-gray-700"> + <div className="flex items-start justify-between gap-4 rounded-lg bg-gray-100 p-4 dark:bg-gray-700">packages/ui/src/alert.tsx (1)
13-14: Missingtext-primary-foregroundon the primary variant.The
primaryvariant setsbg-primarybut lackstext-primary-foregroundfor the root text color, unlike the other variants (info, warning, success, note) which all include both background and foreground colors. This inconsistency could cause readability issues.🔎 Proposed fix
primary: - "bg-primary *:data-[slot=alert-description]:text-primary-foreground/90 [&>svg]:text-primary-foreground", + "bg-primary text-primary-foreground *:data-[slot=alert-description]:text-primary-foreground/90 [&>svg]:text-primary-foreground",apps/web/src/env.ts (1)
109-113: Add format validation for PRIMARY_COLOR.The PRIMARY_COLOR environment variable should validate that the value is a valid hex color format to prevent runtime issues with invalid color values.
🔎 Proposed fix
/** * Primary color for theming (hex format, e.g., "#4f46e5") */ - PRIMARY_COLOR: z.string().optional(), + PRIMARY_COLOR: z.string().regex(/^#[0-9A-Fa-f]{6}$/, "PRIMARY_COLOR must be a valid hex color (e.g., #4f46e5)").optional(),
🧹 Nitpick comments (10)
packages/ui/src/switch.tsx (1)
24-24: Dark mode thumb styling looks good.The
dark:bg-gray-700addition provides appropriate contrast for the switch thumb in dark mode. The use of specific color values (bg-whiteandbg-gray-700) for the thumb—rather than semantic tokens—may be intentional to ensure consistent visibility across all switch states.Optional: Consider whether
bg-whiteanddark:bg-gray-700could use semantic tokens (e.g.,bg-backgroundor similar) for greater consistency with the token migration mentioned in the PR. However, specific colors for the thumb may be preferable for guaranteed visibility.apps/web/src/components/random-gradient-bar.tsx (1)
1-1: Consider renaming component to reflect static behavior.The component name "RandomGradientBar" implies non-deterministic rendering, but the gradient is now fixed. Consider renaming to
GradientBarorPrimaryGradientBarto better communicate its current behavior.packages/tailwind-config/shared-styles.css (1)
11-131: Consider tokenizing hard-coded hex values.The theme structure is comprehensive, but several hard-coded hex values could be replaced with token references for better maintainability:
- Lines 34-35, 102-103:
--action-barcolors (#1f2937,#f9fafb) appear identical in both light and dark modes- Line 84:
#f9fafbfor sidebar-accent-foreground- Line 114:
#27272a50for card background (with alpha)These could reference existing color tokens (e.g.,
var(--color-gray-800),var(--color-gray-50)) for consistency and easier theme updates.Example tokenization
- --action-bar: #1f2937; - --action-bar-foreground: #f9fafb; + --action-bar: var(--color-gray-800); + --action-bar-foreground: var(--color-gray-50);- --card: #27272a50; + --card: var(--color-zinc-800);apps/web/src/app/api/private/utils/api-key.ts (2)
21-38: Consider early validation of hash format.The
verifyApiKeyfunction handles malformed stored hashes gracefully by returningfalse, which is secure. However, a malformed stored hash (e.g., corrupted database entry) could silently fail authentication without logging.🔎 Optional: Add logging for malformed hashes
const verifyApiKey = (rawKey: string, stored: string) => { const trimmed = stored.trim(); if (trimmed.startsWith("sha256$")) { const [, salt, hash] = trimmed.split("$"); if (!salt || !hash || !isHexSha256(hash)) { + console.warn("Malformed salted hash format detected"); return false; } const computed = hashApiKey(rawKey, salt); return crypto.timingSafeEqual(Buffer.from(computed), Buffer.from(hash)); } if (!isHexSha256(trimmed)) { + console.warn("Invalid hash format detected"); return false; }
76-81: Unhandled promise rejection inwaitUntil.The
waitUntilcall does not handle potential database errors. WhilewaitUntilruns asynchronously and won't block the response, unhandled rejections could cause issues in some environments.🔎 Proposed fix
waitUntil( prisma.apiKey.update({ where: { id: apiKey.id }, data: { lastUsedAt: new Date() }, - }), + }).catch(() => { + // Silently ignore lastUsedAt update failures + }), );apps/web/src/app/[locale]/(space)/settings/preferences/components/theme-preference.tsx (1)
23-32: Remove redundantkeyprop.The
keyprop on line 24 is unnecessary sinceThemeOptionis not being rendered in an array iteration context—the parentRadioGrouphandles the mapping. Thekeyshould be on the outer component when iterating, not insideThemeOption.🔎 Proposed fix
return ( <label - key={value} className="flex aspect-video flex-1 flex-col items-center justify-center gap-4 rounded-lg border bg-card p-4 transition-colors hover:bg-card-accent has-data-[state=checked]:border-primary/50 has-data-[state=checked]:bg-linear-to-b has-data-[state=checked]:from-primary/10 has-data-[state=checked]:to-primary/5 has-data-[state=checked]:ring-primary [&>svg]:size-5" htmlFor={value} >apps/web/src/app/api/private/utils/time-slots.ts (1)
53-66: Consider validatingintervalis positive.If
intervalis zero or negative, the inner loop (lines 117-127) would either produce infinite iterations or no slots. While the schema likely validates this upstream, defensive validation here would prevent potential issues.🔎 Proposed fix
export const generateTimeSlots = ( generator: SlotGeneratorInput, timeZone: string, durationMinutes: number, ): Array<TimeSlot> => { + const interval = generator.interval ?? durationMinutes; + if (interval <= 0 || durationMinutes <= 0) { + return []; + } + const dayMap: Record<SlotGeneratorInput["daysOfWeek"][number], number> = {Then update line 114:
- const interval = generator.interval ?? durationMinutes; + // interval already validated aboveapps/web/src/app/api/private/[...route]/route.test.ts (2)
77-150: Consider adding a test for hash mismatch scenario.The authentication tests cover missing key, invalid prefix, revoked, and expired scenarios. However, there's no test for when the prefix is valid but the actual key doesn't match the hash (e.g., a tampered key with correct prefix).
🔎 Suggested test case
it("should return 401 when key hash does not match", async () => { const res = await app.request("/api/private/polls", { method: "POST", headers: { "Content-Type": "application/json", Authorization: "Bearer rallly_abc123_wrongsecret", }, body: JSON.stringify({ title: "Test Poll", dates: ["2025-01-15"], }), }); expect(res.status).toBe(401); });
286-415: Consider adding test for NO_OPTIONS_GENERATED error.The slot tests cover successful creation and timezone errors, but don't test the scenario where a slot generator produces no valid options (e.g., when
fromTimeis aftertoTime).🔎 Suggested test case
it("should return error when slot generator produces no options", async () => { const res = await app.request("/api/private/polls", { method: "POST", headers: { "Content-Type": "application/json", Authorization: `Bearer ${testApiKey}`, }, body: JSON.stringify({ title: "Test Poll", slots: { duration: 30, timezone: "Europe/London", times: { startDate: "2025-01-20", endDate: "2025-01-20", days: ["mon"], startTime: "17:00", endTime: "09:00", // End before start }, }, }), }); expect(res.status).toBe(400); const json = await res.json(); expect(json.error.code).toBe("NO_OPTIONS_GENERATED"); });apps/landing/src/app/[locale]/(main)/footer.tsx (1)
100-131: Social icon links updated consistently.The migration from
hover:text-primary-600tohover:text-primaryis applied consistently across all four social links, maintaining the hover behavior correctly.Optional: Per coding guidelines, consider wrapping icons with the
<Icon>component from@rallly/ui/iconfor proper color and size management. Based on learnings, this would follow the project's icon handling pattern.Example refactor for one icon
+import { Icon } from "@rallly/ui/icon"; + <a target="_blank" href="https://twitter.com/ralllyco" className="text-gray-500 text-sm hover:text-primary hover:no-underline" rel="noreferrer noopener" > - <TwitterIcon className="size-5" /> + <Icon> + <TwitterIcon /> + </Icon> </a>
apps/web/src/components/forms/poll-options-form/month-calendar/month-calendar.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tailwind-config/shared-styles.css (1)
245-249: Hard-coded colors inbg-diagonal-lineswon't adapt to dark mode.The utility uses fixed hex colors (
#f9fafband#e5e7eb) that will display poorly in dark mode. Consider using theme-aware tokens or providing dark mode overrides.🔎 Suggested approach using CSS custom properties
@utility bg-diagonal-lines { - background-color: #f9fafb; - background-image: url("data:image/svg+xml,%3Csvg width='100' height='100' xmlns='http://www.w3.org/2000/svg'%3E%3Cline x1='0' y1='0' x2='100%' y2='100%' stroke='%23e5e7eb' stroke-width='1'/%3E%3C/svg%3E"); + background-color: var(--color-gray-50); background-repeat: repeat-y; } + +.dark .bg-diagonal-lines { + background-color: var(--color-gray-900); +}Note: The SVG data URL would need to be dynamically generated or have separate light/dark variants since inline SVGs in data URLs cannot reference CSS variables.
♻️ Duplicate comments (1)
packages/ui/src/alert.tsx (1)
12-13: Primary variant still missing roottext-primary-foreground.The
primaryvariant setsbg-primarybut doesn't includetext-primary-foregroundfor the root text color. Unlike other variants (info, warning, success, note, error) which all set both background and foreground colors, the primary variant only has child selectors for text color. This could cause readability issues when text is placed directly in the Alert without usingAlertDescription.🔎 Proposed fix
primary: - "bg-primary *:data-[slot=alert-description]:text-primary-foreground/90 [&>svg]:text-primary-foreground", + "bg-primary text-primary-foreground *:data-[slot=alert-description]:text-primary-foreground/90 [&>svg]:text-primary-foreground",
🧹 Nitpick comments (2)
apps/web/src/utils/emails.ts (1)
20-22: Simplify ternary to nullish coalescing operator.The pattern
env.LOGO_ICON_URL ? env.LOGO_ICON_URL : fallbackcan be simplified toenv.LOGO_ICON_URL ?? fallbacksince the value is either a valid URL orundefined(validated byz.url().optional()).🔎 Proposed refactor
- logoUrl: env.LOGO_ICON_URL - ? env.LOGO_ICON_URL - : "https://d39ixtfgglw55o.cloudfront.net/images/rallly-logo-mark.png", + logoUrl: env.LOGO_ICON_URL ?? "https://d39ixtfgglw55o.cloudfront.net/images/rallly-logo-mark.png",packages/tailwind-config/shared-styles.css (1)
163-163: Remove trailing whitespace.Line 163 contains only whitespace. Consider removing it for consistency.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
apps/web/src/app/[locale]/(auth)/reset-password/components/reset-password-form.tsxapps/web/src/app/[locale]/(space)/(dashboard)/components/nav-user.tsxapps/web/src/app/[locale]/(space)/settings/billing/page-client.tsxapps/web/src/app/[locale]/invite/[urlId]/invite-page.tsxapps/web/src/components/empty-state.tsxapps/web/src/components/participant.tsxapps/web/src/components/poll/desktop-poll.tsxapps/web/src/components/poll/desktop-poll/participant-row-form.tsxapps/web/src/components/poll/desktop-poll/participant-row.tsxapps/web/src/components/poll/desktop-poll/poll-header.tsxapps/web/src/components/poll/mobile-poll.tsxapps/web/src/components/poll/mobile-poll/poll-option.tsxapps/web/src/components/poll/vote-icon.tsxapps/web/src/components/poll/vote-selector.tsxapps/web/src/env.tsapps/web/src/utils/emails.tspackages/emails/src/components/email-context.tsxpackages/emails/src/send-email.tsxpackages/emails/src/types.tspackages/tailwind-config/shared-styles.csspackages/ui/src/alert.tsxpackages/ui/src/badge.tsxpackages/ui/src/button.tsxpackages/ui/src/dropdown-menu.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/web/src/components/poll/desktop-poll.tsx
- apps/web/src/components/poll/mobile-poll.tsx
- apps/web/src/components/poll/mobile-poll/poll-option.tsx
- packages/emails/src/send-email.tsx
- packages/ui/src/dropdown-menu.tsx
- apps/web/src/components/poll/vote-selector.tsx
- apps/web/src/components/participant.tsx
- packages/ui/src/button.tsx
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Use dayjs for date handling
Use react-query for data fetching
Prefer implicit return values over explicit return values
Use zod for form validation
Create separate import statements for types
Prefer using the React module APIs (e.g. React.useState) instead of standalone hooks (e.g. useState)
Prefer double quotes for strings over single quotes
Only add comments when it is necessary to explain code that isn't self-explanatory
**/*.{ts,tsx}: Only create named interfaces when they're reused or complex
When TypeScript errors occur for missing i18n keys, runpnpm i18n:scaninstead of manually adding keys
Files:
apps/web/src/components/poll/vote-icon.tsxpackages/emails/src/components/email-context.tsxapps/web/src/components/empty-state.tsxapps/web/src/app/[locale]/(space)/settings/billing/page-client.tsxapps/web/src/components/poll/desktop-poll/participant-row-form.tsxapps/web/src/components/poll/desktop-poll/participant-row.tsxapps/web/src/components/poll/desktop-poll/poll-header.tsxapps/web/src/app/[locale]/(auth)/reset-password/components/reset-password-form.tsxapps/web/src/env.tspackages/emails/src/types.tspackages/ui/src/badge.tsxapps/web/src/app/[locale]/(space)/(dashboard)/components/nav-user.tsxapps/web/src/utils/emails.tsapps/web/src/app/[locale]/invite/[urlId]/invite-page.tsxpackages/ui/src/alert.tsx
**/*.{tsx,css,config.ts}
📄 CodeRabbit inference engine (.cursorrules)
Use tailwindcss for styling
Files:
apps/web/src/components/poll/vote-icon.tsxpackages/emails/src/components/email-context.tsxapps/web/src/components/empty-state.tsxapps/web/src/app/[locale]/(space)/settings/billing/page-client.tsxapps/web/src/components/poll/desktop-poll/participant-row-form.tsxapps/web/src/components/poll/desktop-poll/participant-row.tsxapps/web/src/components/poll/desktop-poll/poll-header.tsxapps/web/src/app/[locale]/(auth)/reset-password/components/reset-password-form.tsxpackages/ui/src/badge.tsxapps/web/src/app/[locale]/(space)/(dashboard)/components/nav-user.tsxpackages/tailwind-config/shared-styles.cssapps/web/src/app/[locale]/invite/[urlId]/invite-page.tsxpackages/ui/src/alert.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use react-hook-form for form handling
All text in the UI should be translated using either the Trans component or the useTranslation hook
Prefer composable components in the style of shadcn UI over large monolithic components
DropdownMenuItem is a flex container with a preset gap so there is no need to add margins to the children
The size and colour of an icon should be set by wrapping it with the component from @rallly/ui/icon which will give it the correct colour and size
Keep the props of a component as minimal as possible. Pass only the bare minimum amount of information needed to it
All text in the UI should be translatable
Use the component in client components from @/components/trans with thedefaultsprop to provide the default text
Always use a composable patterns when building components
Usecn()from @rallly/ui to compose classes
Add the "use client" directive to the top of any .tsx file that requires client-side javascript
Files:
apps/web/src/components/poll/vote-icon.tsxpackages/emails/src/components/email-context.tsxapps/web/src/components/empty-state.tsxapps/web/src/app/[locale]/(space)/settings/billing/page-client.tsxapps/web/src/components/poll/desktop-poll/participant-row-form.tsxapps/web/src/components/poll/desktop-poll/participant-row.tsxapps/web/src/components/poll/desktop-poll/poll-header.tsxapps/web/src/app/[locale]/(auth)/reset-password/components/reset-password-form.tsxpackages/ui/src/badge.tsxapps/web/src/app/[locale]/(space)/(dashboard)/components/nav-user.tsxapps/web/src/app/[locale]/invite/[urlId]/invite-page.tsxpackages/ui/src/alert.tsx
**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,json}: i18n keys are in camelCase
i18nKeys should describe the message in camelCase. Ex. "lastUpdated": "Last Updated"
If the i18nKey is not intended to be reused, prefix it with the component name in camelCase
Files:
apps/web/src/components/poll/vote-icon.tsxpackages/emails/src/components/email-context.tsxapps/web/src/components/empty-state.tsxapps/web/src/app/[locale]/(space)/settings/billing/page-client.tsxapps/web/src/components/poll/desktop-poll/participant-row-form.tsxapps/web/src/components/poll/desktop-poll/participant-row.tsxapps/web/src/components/poll/desktop-poll/poll-header.tsxapps/web/src/app/[locale]/(auth)/reset-password/components/reset-password-form.tsxapps/web/src/env.tspackages/emails/src/types.tspackages/ui/src/badge.tsxapps/web/src/app/[locale]/(space)/(dashboard)/components/nav-user.tsxapps/web/src/utils/emails.tsapps/web/src/app/[locale]/invite/[urlId]/invite-page.tsxpackages/ui/src/alert.tsx
**/*
📄 CodeRabbit inference engine (.cursorrules)
Always use kebab-case for file names
Files:
apps/web/src/components/poll/vote-icon.tsxpackages/emails/src/components/email-context.tsxapps/web/src/components/empty-state.tsxapps/web/src/app/[locale]/(space)/settings/billing/page-client.tsxapps/web/src/components/poll/desktop-poll/participant-row-form.tsxapps/web/src/components/poll/desktop-poll/participant-row.tsxapps/web/src/components/poll/desktop-poll/poll-header.tsxapps/web/src/app/[locale]/(auth)/reset-password/components/reset-password-form.tsxapps/web/src/env.tspackages/emails/src/types.tspackages/ui/src/badge.tsxapps/web/src/app/[locale]/(space)/(dashboard)/components/nav-user.tsxapps/web/src/utils/emails.tspackages/tailwind-config/shared-styles.cssapps/web/src/app/[locale]/invite/[urlId]/invite-page.tsxpackages/ui/src/alert.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Biome for code formatting with indent of 2 spaces and double quotes
Files:
apps/web/src/components/poll/vote-icon.tsxpackages/emails/src/components/email-context.tsxapps/web/src/components/empty-state.tsxapps/web/src/app/[locale]/(space)/settings/billing/page-client.tsxapps/web/src/components/poll/desktop-poll/participant-row-form.tsxapps/web/src/components/poll/desktop-poll/participant-row.tsxapps/web/src/components/poll/desktop-poll/poll-header.tsxapps/web/src/app/[locale]/(auth)/reset-password/components/reset-password-form.tsxapps/web/src/env.tspackages/emails/src/types.tspackages/ui/src/badge.tsxapps/web/src/app/[locale]/(space)/(dashboard)/components/nav-user.tsxapps/web/src/utils/emails.tsapps/web/src/app/[locale]/invite/[urlId]/invite-page.tsxpackages/ui/src/alert.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{tsx,ts}: Prefer inline prop types over named interfaces for simple component props (e.g.,function Component({ prop }: { prop: string })instead of defining a separate interface)
Always use theuseDialoghook from@rallly/ui/dialogfor managing dialog state instead of manualuseStatefor open/close state
Use TanStack Query with tRPC for server state management
Use React Context for client state (auth, preferences, etc.)
Use react-hook-form with Zod validation for form state management
Use TailwindCSS with custom design system for styling
Files:
apps/web/src/components/poll/vote-icon.tsxpackages/emails/src/components/email-context.tsxapps/web/src/components/empty-state.tsxapps/web/src/app/[locale]/(space)/settings/billing/page-client.tsxapps/web/src/components/poll/desktop-poll/participant-row-form.tsxapps/web/src/components/poll/desktop-poll/participant-row.tsxapps/web/src/components/poll/desktop-poll/poll-header.tsxapps/web/src/app/[locale]/(auth)/reset-password/components/reset-password-form.tsxapps/web/src/env.tspackages/emails/src/types.tspackages/ui/src/badge.tsxapps/web/src/app/[locale]/(space)/(dashboard)/components/nav-user.tsxapps/web/src/utils/emails.tsapps/web/src/app/[locale]/invite/[urlId]/invite-page.tsxpackages/ui/src/alert.tsx
apps/web/src/components/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared components in
apps/web/src/components/
Files:
apps/web/src/components/poll/vote-icon.tsxapps/web/src/components/empty-state.tsxapps/web/src/components/poll/desktop-poll/participant-row-form.tsxapps/web/src/components/poll/desktop-poll/participant-row.tsxapps/web/src/components/poll/desktop-poll/poll-header.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/web/.cursor/rules/better-auth.md)
apps/web/**/*.{ts,tsx,js,jsx}: When using the username plugin, require all sign up endpoints to accept ausername(used for login, normalized) and an optionaldisplayUsername(raw, for display purposes).
When using the username plugin, all username values must be normalized according to the configuration function before storage or comparison (default: lowercase).
Whenever updating a user's username, always check for uniqueness and apply the normalization procedure.
Login endpoints or forms supporting username authentication must allow signing in with username and password, not just email.
Files:
apps/web/src/components/poll/vote-icon.tsxapps/web/src/components/empty-state.tsxapps/web/src/app/[locale]/(space)/settings/billing/page-client.tsxapps/web/src/components/poll/desktop-poll/participant-row-form.tsxapps/web/src/components/poll/desktop-poll/participant-row.tsxapps/web/src/components/poll/desktop-poll/poll-header.tsxapps/web/src/app/[locale]/(auth)/reset-password/components/reset-password-form.tsxapps/web/src/env.tsapps/web/src/app/[locale]/(space)/(dashboard)/components/nav-user.tsxapps/web/src/utils/emails.tsapps/web/src/app/[locale]/invite/[urlId]/invite-page.tsx
apps/web/src/app/**
📄 CodeRabbit inference engine (CLAUDE.md)
Use Next.js App Router conventions for route handlers
Files:
apps/web/src/app/[locale]/(space)/settings/billing/page-client.tsxapps/web/src/app/[locale]/(auth)/reset-password/components/reset-password-form.tsxapps/web/src/app/[locale]/(space)/(dashboard)/components/nav-user.tsxapps/web/src/app/[locale]/invite/[urlId]/invite-page.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursorrules)
On the server use the
getTranslationsfunction from @/i18n/server to get the translations
Files:
apps/web/src/env.tspackages/emails/src/types.tsapps/web/src/utils/emails.ts
packages/ui/**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
shadcn-ui components should be added to packages/ui
Files:
packages/ui/src/badge.tsxpackages/ui/src/alert.tsx
🧠 Learnings (10)
📚 Learning: 2025-11-25T11:03:55.173Z
Learnt from: CR
Repo: lukevella/rallly PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T11:03:55.173Z
Learning: Applies to **/*.tsx : The size and colour of an icon should be set by wrapping it with the <Icon> component from rallly/ui/icon which will give it the correct colour and size
Applied to files:
apps/web/src/components/poll/vote-icon.tsxapps/web/src/components/empty-state.tsxapps/web/src/app/[locale]/invite/[urlId]/invite-page.tsx
📚 Learning: 2025-11-25T11:04:05.725Z
Learnt from: CR
Repo: lukevella/rallly PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T11:04:05.725Z
Learning: Rallly is built with Next.js 15, React 19, tRPC, Prisma with PostgreSQL, NextAuth.js, TailwindCSS, and TypeScript throughout
Applied to files:
packages/emails/src/components/email-context.tsx
📚 Learning: 2025-06-14T16:32:39.967Z
Learnt from: lukevella
Repo: lukevella/rallly PR: 1776
File: apps/web/src/auth/adapters/prisma.ts:40-60
Timestamp: 2025-06-14T16:32:39.967Z
Learning: In the Rallly codebase, optional user fields like timeZone, weekStart, timeFormat, and locale correspond to nullable database columns, so passing undefined values won't cause insert failures.
Applied to files:
packages/emails/src/components/email-context.tsx
📚 Learning: 2025-11-25T11:04:24.541Z
Learnt from: CR
Repo: lukevella/rallly PR: 0
File: apps/web/.cursor/rules/better-auth.md:0-0
Timestamp: 2025-11-25T11:04:24.541Z
Learning: Applies to apps/web/**/*.{ts,tsx,js,jsx} : Login endpoints or forms supporting username authentication must allow signing in with username and password, not just email.
Applied to files:
apps/web/src/app/[locale]/(auth)/reset-password/components/reset-password-form.tsx
📚 Learning: 2025-11-25T11:03:55.173Z
Learnt from: CR
Repo: lukevella/rallly PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T11:03:55.173Z
Learning: Applies to **/*.tsx : DropdownMenuItem is a flex container with a preset gap so there is no need to add margins to the children
Applied to files:
apps/web/src/app/[locale]/(space)/(dashboard)/components/nav-user.tsx
📚 Learning: 2025-11-25T11:04:05.725Z
Learnt from: CR
Repo: lukevella/rallly PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T11:04:05.725Z
Learning: Applies to **/*.{tsx,ts} : Use TailwindCSS with custom design system for styling
Applied to files:
packages/tailwind-config/shared-styles.css
📚 Learning: 2025-11-25T11:03:55.173Z
Learnt from: CR
Repo: lukevella/rallly PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T11:03:55.173Z
Learning: Applies to **/*.{tsx,css,config.ts} : Use tailwindcss for styling
Applied to files:
packages/tailwind-config/shared-styles.css
📚 Learning: 2025-11-25T11:04:05.725Z
Learnt from: CR
Repo: lukevella/rallly PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T11:04:05.725Z
Learning: Applies to **/*.{tsx,ts} : Use React Context for client state (auth, preferences, etc.)
Applied to files:
apps/web/src/app/[locale]/invite/[urlId]/invite-page.tsx
📚 Learning: 2025-11-25T11:04:05.725Z
Learnt from: CR
Repo: lukevella/rallly PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T11:04:05.725Z
Learning: Applies to **/*.{tsx,ts} : Always use the `useDialog` hook from `rallly/ui/dialog` for managing dialog state instead of manual `useState` for open/close state
Applied to files:
apps/web/src/app/[locale]/invite/[urlId]/invite-page.tsx
📚 Learning: 2025-11-25T11:03:55.173Z
Learnt from: CR
Repo: lukevella/rallly PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T11:03:55.173Z
Learning: Applies to **/*.tsx : Use `cn()` from rallly/ui to compose classes
Applied to files:
packages/ui/src/alert.tsx
🧬 Code graph analysis (3)
apps/web/src/app/[locale]/(space)/settings/billing/page-client.tsx (1)
packages/ui/src/alert.tsx (1)
Alert(71-71)
apps/web/src/components/poll/desktop-poll/participant-row.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(5-7)
apps/web/src/utils/emails.ts (2)
apps/web/src/env.ts (1)
env(7-196)packages/utils/src/absolute-url.ts (1)
absoluteUrl(18-39)
🔇 Additional comments (26)
packages/emails/src/components/email-context.tsx (1)
16-16: LGTM!The addition of
appNameto the preview context correctly matches the updatedEmailContexttype definition and provides the necessary branding metadata for email templates.packages/emails/src/types.ts (1)
13-14: LGTM!The type extensions are well-designed:
appNameas required ensures consistent branding across all email contextsprimaryColoras optional provides flexibility for custom theming without breaking existing implementationspackages/ui/src/badge.tsx (1)
12-17: LGTM! Clean migration to semantic tokens for dark mode.The variant updates properly migrate to semantic design tokens that support theming:
text-primary-foregroundandbg-mutedwill adapt automatically to light/dark modesbg-primary/10opacity pattern works well across themesNote: The
defaultvariant no longer includes a border—this appears intentional as part of the design system update.apps/web/src/components/poll/vote-icon.tsx (1)
13-13: LGTM! Size adjustment aligns with design system update.The reduction from
size-4tosize-3for the small variant is consistent with the broader styling token migration mentioned in this PR's objectives. This makes small vote icons slightly more compact (16px → 12px).apps/web/src/app/[locale]/(auth)/reset-password/components/reset-password-form.tsx (1)
45-45: LGTM!The variant change from
"destructive"to"error"correctly aligns with the newerrorvariant defined inalert.tsx. This resolves the previous review concern about the missingdestructivevariant.packages/ui/src/alert.tsx (2)
19-25: LGTM!The new
errorvariant follows the same styling pattern as other semantic variants with proper light/dark mode support. The default variant change tonoteprovides a sensible neutral default.
7-27: Thetipvariant migration is complete — no remainingvariant="tip"usages found in the codebase. All Alert components have been updated to use appropriate existing variants.apps/web/src/app/[locale]/(space)/settings/billing/page-client.tsx (1)
138-154: LGTM!The variant change from
"tip"to"success"is semantically appropriate for a seat update confirmation message. TheCheckCircleIconaligns well with the success variant, and this resolves the missingtipvariant concern from the alert component update.apps/web/src/components/poll/desktop-poll/poll-header.tsx (1)
51-51: LGTM! Consistent theme token migration.All background changes correctly replace hardcoded colors (
bg-white,bg-gray-50) with the semanticbg-backgroundtoken, enabling dark mode support while preserving the sticky positioning and layering behavior.Also applies to: 78-78, 117-117, 142-142
apps/web/src/components/poll/desktop-poll/participant-row.tsx (2)
51-51: LGTM! Consistent theme token migration.Header cell correctly migrates to
bg-backgroundfor dark mode support.
73-84: Nice simplification of vote cell backgrounds.The refactored logic is cleaner and more maintainable:
- State-based backgrounds using
bg-backgroundwith opacity modifiers follow Tailwind v4 patterns correctly- Removed unnecessary DOM wrapper for conditional backgrounds
- Trailing cell simplified from diagonal pattern to plain border
This maintains visual hierarchy through opacity while reducing complexity.
apps/web/src/components/poll/desktop-poll/participant-row-form.tsx (2)
60-60: LGTM! Past issue resolved.The sticky cell now correctly includes
bg-background, addressing the previous review concern about content bleed-through during horizontal scrolling.
108-139: Comprehensive dark mode support for interactive states.The vote cells now include well-defined hover and active states for both light and dark modes with appropriate visual feedback (background changes, rings, and transitions). The removal of
bg-gray-50andbg-diagonal-linesaligns with the simplified theming approach.apps/web/src/env.ts (2)
105-122: LGTM! Environment variables for theming are well-defined.The new theming and branding variables are properly validated:
APP_NAMEhas a sensible defaultPRIMARY_COLORregex correctly validates 3 or 6-digit hex colors- URL validation applied to
LOGO_URLandLOGO_ICON_URLThe addressed past review concern about PRIMARY_COLOR validation has been properly resolved.
190-193: LGTM! Runtime environment mappings are complete.All new environment variables are correctly mapped in the
runtimeEnvconfiguration.apps/web/src/utils/emails.ts (1)
26-27: LGTM! Primary color and app name integration is correct.The environment-driven configuration properly handles:
primaryColorwith a sensible fallback matching the env.ts exampleappNamesafely uses the environment value (has default "Rallly" in schema)apps/web/src/app/[locale]/(space)/(dashboard)/components/nav-user.tsx (2)
10-17: LGTM! Imports are well-organized and complete.All new imports for the theme submenu functionality are properly used in the component. The imports follow the existing code organization pattern.
Also applies to: 20-20, 24-28
93-124: Excellent implementation of the theme submenu!The theme selector follows all coding guidelines correctly:
- Uses composable shadcn-style pattern with Sub/SubTrigger/SubContent/Portal
- RadioGroup is semantically appropriate for mutually exclusive theme selection
- Icons are properly wrapped with the Icon component
- All UI text uses Trans with camelCase i18nKeys and defaults
- No unnecessary margins on children (DropdownMenuItem is a flex container)
Remember to run
pnpm i18n:scanto register the new i18n keys (theme,themeSystem,themeLight,themeDark) for translation workflows.packages/tailwind-config/shared-styles.css (4)
3-4: LGTM: Dark variant correctly configured for Tailwind v4.The custom variant definition follows Tailwind v4 conventions and implements class-based dark mode appropriately.
11-71: LGTM: Comprehensive light mode token system.The root theme tokens are well-structured with semantic naming and appropriate use of OKLCH color space for better color fidelity.
209-220: LGTM: Clean gray-to-zinc palette migration.This mapping provides a smooth migration path from gray to zinc palette while maintaining backward compatibility with existing code that references gray tokens.
231-239: LGTM: Proper Tailwind v4 compatibility patch.This correctly addresses the Tailwind v4 breaking change where default border colors changed to
currentcolor. The patch ensures consistent behavior with v3 while allowing explicit overrides.apps/web/src/app/[locale]/invite/[urlId]/invite-page.tsx (4)
3-12: LGTM!The imports are well-organized and follow the coding guidelines. Icons are correctly imported for wrapping with the
<Icon>component.
86-86: LGTM!The responsive spacing adjustment (
space-y-2 md:space-y-4) improves the layout by reducing spacing on mobile devices while increasing it on larger screens.
63-63: Thebg-pollsemantic token is properly defined and correctly implemented.The token is defined in
packages/tailwind-config/shared-styles.csswith appropriate light mode (gray-100) and dark mode (gray-900) values, and is correctly exposed via the Tailwind theme configuration. No action needed.
45-45: The link styling is correct. The Link element inside Alert variant="primary" automatically inherits the text color (text-primary-foreground/90) through the Alert component's CSS cascade, making an explicittext-primaryclass unnecessary. The visual hierarchy and contrast are properly maintained by the design system.Likely an incorrect or invalid review comment.
| .dark { | ||
| --background: var(--color-gray-900); | ||
| --foreground: var(--color-gray-300); | ||
|
|
||
| --primary: var(--color-indigo-500); | ||
| --primary-foreground: var(--color-indigo-50); | ||
|
|
||
| --sidebar: var(--color-gray-900); | ||
| --sidebar-foreground: var(--color-gray-100); | ||
| --sidebar-border: var(--color-gray-800); | ||
|
|
||
| --sidebar-accent: var(--color-gray-800); | ||
| --sidebar-accent-foreground: #f9fafb; | ||
|
|
||
| --border: oklch(.274 .006 286.033); | ||
| --border-muted: oklch(.274 .006 286.033 / .5); | ||
|
|
||
| --input: var(--color-gray-800); | ||
| --input-border: var(--color-gray-700); | ||
| --input-foreground: var(--color-gray-100); | ||
|
|
||
| --ring: #4b5563; | ||
|
|
||
| --destructive: var(--color-rose-500); | ||
| --destructive-foreground: var(--color-rose-50); | ||
|
|
||
| --accent: var(--color-gray-700); | ||
| --accent-border: var(--color-gray-600); | ||
| --accent-foreground: var(--color-gray-100); | ||
|
|
||
| --action-bar: #1f2937; | ||
| --action-bar-foreground: #f9fafb; | ||
|
|
||
| --muted: var(--color-gray-800); | ||
| --muted-foreground: var(--color-gray-400); | ||
| --muted-border: var(--color-gray-700); | ||
|
|
||
| --popover: var(--color-gray-800); | ||
| --popover-border: var(--color-gray-700); | ||
| --popover-foreground: var(--color-gray-300); | ||
| --popover-accent: var(--color-gray-700); | ||
|
|
||
| --card: #27272a50; | ||
| --card-border: var(--color-gray-800); | ||
| --card-foreground: var(--color-gray-300); | ||
| --card-accent: var(--color-gray-800); | ||
|
|
||
| --success: var(--color-green-950); | ||
| --success-foreground: var(--color-green-100); | ||
|
|
||
| --warning: var(--color-yellow-950); | ||
| --warning-foreground: var(--color-yellow-100); | ||
|
|
||
| --poll: var(--color-gray-900); | ||
| } |
There was a problem hiding this comment.
Add dark mode overrides for --info and --tip tokens.
The dark mode block is missing overrides for --info, --info-foreground, --tip, and --tip-foreground. These tokens are defined in light mode (lines 61-62, 67-68) but not overridden for dark mode, which means they'll use light mode colors (light backgrounds with dark text) in dark mode—likely resulting in poor contrast.
🔎 Suggested dark mode overrides
--warning: var(--color-yellow-950);
--warning-foreground: var(--color-yellow-100);
+ --info: var(--color-blue-950);
+ --info-foreground: var(--color-blue-100);
+
+ --tip: var(--color-green-950);
+ --tip-foreground: var(--color-green-100);
+
--poll: var(--color-gray-900);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .dark { | |
| --background: var(--color-gray-900); | |
| --foreground: var(--color-gray-300); | |
| --primary: var(--color-indigo-500); | |
| --primary-foreground: var(--color-indigo-50); | |
| --sidebar: var(--color-gray-900); | |
| --sidebar-foreground: var(--color-gray-100); | |
| --sidebar-border: var(--color-gray-800); | |
| --sidebar-accent: var(--color-gray-800); | |
| --sidebar-accent-foreground: #f9fafb; | |
| --border: oklch(.274 .006 286.033); | |
| --border-muted: oklch(.274 .006 286.033 / .5); | |
| --input: var(--color-gray-800); | |
| --input-border: var(--color-gray-700); | |
| --input-foreground: var(--color-gray-100); | |
| --ring: #4b5563; | |
| --destructive: var(--color-rose-500); | |
| --destructive-foreground: var(--color-rose-50); | |
| --accent: var(--color-gray-700); | |
| --accent-border: var(--color-gray-600); | |
| --accent-foreground: var(--color-gray-100); | |
| --action-bar: #1f2937; | |
| --action-bar-foreground: #f9fafb; | |
| --muted: var(--color-gray-800); | |
| --muted-foreground: var(--color-gray-400); | |
| --muted-border: var(--color-gray-700); | |
| --popover: var(--color-gray-800); | |
| --popover-border: var(--color-gray-700); | |
| --popover-foreground: var(--color-gray-300); | |
| --popover-accent: var(--color-gray-700); | |
| --card: #27272a50; | |
| --card-border: var(--color-gray-800); | |
| --card-foreground: var(--color-gray-300); | |
| --card-accent: var(--color-gray-800); | |
| --success: var(--color-green-950); | |
| --success-foreground: var(--color-green-100); | |
| --warning: var(--color-yellow-950); | |
| --warning-foreground: var(--color-yellow-100); | |
| --poll: var(--color-gray-900); | |
| } | |
| .dark { | |
| --background: var(--color-gray-900); | |
| --foreground: var(--color-gray-300); | |
| --primary: var(--color-indigo-500); | |
| --primary-foreground: var(--color-indigo-50); | |
| --sidebar: var(--color-gray-900); | |
| --sidebar-foreground: var(--color-gray-100); | |
| --sidebar-border: var(--color-gray-800); | |
| --sidebar-accent: var(--color-gray-800); | |
| --sidebar-accent-foreground: #f9fafb; | |
| --border: oklch(.274 .006 286.033); | |
| --border-muted: oklch(.274 .006 286.033 / .5); | |
| --input: var(--color-gray-800); | |
| --input-border: var(--color-gray-700); | |
| --input-foreground: var(--color-gray-100); | |
| --ring: #4b5563; | |
| --destructive: var(--color-rose-500); | |
| --destructive-foreground: var(--color-rose-50); | |
| --accent: var(--color-gray-700); | |
| --accent-border: var(--color-gray-600); | |
| --accent-foreground: var(--color-gray-100); | |
| --action-bar: #1f2937; | |
| --action-bar-foreground: #f9fafb; | |
| --muted: var(--color-gray-800); | |
| --muted-foreground: var(--color-gray-400); | |
| --muted-border: var(--color-gray-700); | |
| --popover: var(--color-gray-800); | |
| --popover-border: var(--color-gray-700); | |
| --popover-foreground: var(--color-gray-300); | |
| --popover-accent: var(--color-gray-700); | |
| --card: #27272a50; | |
| --card-border: var(--color-gray-800); | |
| --card-foreground: var(--color-gray-300); | |
| --card-accent: var(--color-gray-800); | |
| --success: var(--color-green-950); | |
| --success-foreground: var(--color-green-100); | |
| --warning: var(--color-yellow-950); | |
| --warning-foreground: var(--color-yellow-100); | |
| --info: var(--color-blue-950); | |
| --info-foreground: var(--color-blue-100); | |
| --tip: var(--color-green-950); | |
| --tip-foreground: var(--color-green-100); | |
| --poll: var(--color-gray-900); | |
| } |
🤖 Prompt for AI Agents
In packages/tailwind-config/shared-styles.css around lines 73 to 127, the .dark
block lacks overrides for the --info, --info-foreground, --tip, and
--tip-foreground tokens defined in light mode, causing those components to
retain light-mode colors in dark mode; add dark-mode values for these four
variables (use darker backgrounds and light foregrounds consistent with other
dark tokens, e.g., darker blue/gray backgrounds and pale/white foregrounds) to
ensure proper contrast and visual consistency in dark theme.
| --popover-foreground: var(--color-gray-300); | ||
| --popover-accent: var(--color-gray-700); | ||
|
|
||
| --card: #27272a50; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use a token reference instead of a raw hex color.
Line 115 uses #27272a50 (raw hex with alpha), which is inconsistent with the token-based approach used throughout the rest of the file. Consider using a zinc or gray token with opacity instead.
🔎 Proposed refactor
- --card: #27272a50;
+ --card: oklch(.17 .004 286.32 / .5);Or reference an existing token:
- --card: #27272a50;
+ --card: color-mix(in oklch, var(--color-gray-900) 50%, transparent);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --card: #27272a50; | |
| --card: oklch(.17 .004 286.32 / .5); |
🤖 Prompt for AI Agents
In packages/tailwind-config/shared-styles.css around line 115, replace the raw
hex-with-alpha value `#27272a50` with a reference to an existing color token
(e.g., a zinc/gray token) and apply the alpha via your project's token-opacity
approach (either via an rgba() with the token's RGB variables or the modern CSS
color/opacity slash syntax), so the file remains consistent with token-based
colors and not hard-coded hex values.
Summary by CodeRabbit
New Features
Bug Fixes
Removals
Style
Localization
✏️ Tip: You can customize this high-level summary in your review settings.