-
Notifications
You must be signed in to change notification settings - Fork 140
UI enforce design consistency #193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughIntroduces a centralized design-tokens module and updates Tailwind to consume those tokens, replaces many hardcoded colors across ~30+ pages and components with semantic tokens, and adds design system and brand-consistency documentation. No runtime/control-flow or public API changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/app/(main)/sheet/[moduleId]/page.tsx (1)
161-242: Migrate deprecatedox-*tokens and fix double text prefix to follow design system.The sheet page uses legacy tokens marked for deprecation in
tailwind.config.ts:
ox-header,ox-content(andox-sidebar,ox-purplein account page) are explicitly noted as deprecated- Double-prefix bug:
text-text-primary,text-text-secondaryshould betext-primary,text-secondaryReplace with documented semantic tokens per
DESIGN_SYSTEM.md:
bg-ox-header→bg-border-lightbg-ox-content→bg-surface-tertiarytext-text-primary→text-primarytext-text-secondary→text-secondarySame issues exist in the account page (
apps/web/src/app/(main)/dashboard/account/page.tsx, line 31). Apply consistent migration across both files.apps/web/src/app/(main)/dashboard/account/page.tsx (1)
21-65: Update legacyox-*tokens to new naming convention per design system migration.The tailwind configuration confirms
ox-*tokens are marked as "Legacy...for gradual migration - will be deprecated." While the migration is ongoing, the tokens in this file should be updated to use the new semantic naming:
bg-ox-sidebar→bg-surface-secondary(line 31)bg-ox-purple→bg-brand-purple(line 46)hover:bg-ox-purple-2→hover:bg-brand-purple-dark(line 46)bg-ox-content→bg-surface-tertiary(line 62)Note:
border-dash-borderandtext-text-*tokens are already using the correct new naming convention and should remain unchanged.
🧹 Nitpick comments (8)
apps/web/src/components/sheet/OpensoxProBadge.tsx (1)
19-21: Consider refactoring the conditional text size logic.The current implementation couples font size to a padding class via string search (
className?.includes("px-1.5")), which is fragile and implicit. Consider introducing an explicit prop (e.g.,size?: "default" | "small") to control text sizing independently.Example refactor:
interface OpensoxProBadgeProps { className?: string; + size?: "default" | "small"; } -export function OpensoxProBadge({ className }: OpensoxProBadgeProps) { +export function OpensoxProBadge({ className, size = "default" }: OpensoxProBadgeProps) { return ( <Link href="/pricing"> <div className={cn( "inline-flex items-center justify-center px-2.5 py-0.5 rounded-full border border-dashed border-brand-purple bg-brand-purple/20 cursor-pointer hover:bg-brand-purple/30 transition-colors", className )} > <span className={cn( "text-[10px] sm:text-xs font-bold text-brand-purple-light", - className?.includes("px-1.5") && "text-[8px]" + size === "small" && "text-[8px]" )} > OX Pro </span> </div> </Link> ); }apps/web/src/components/dashboard/ProjectsContainer.tsx (1)
70-70: Mixed token naming convention detected.The component uses both old
ox-prefixed tokens (bg-ox-purple,bg-ox-content) and newdash-tokens (border-dash-border). Based on the PR's design system migration (e.g.,dashboard/layout.tsxusesbg-dash-base), these should be updated for consistency:
- Line 70:
bg-ox-purple- Lines 81, 101:
bg-ox-content(used twice)Consider updating to the new naming convention:
- className="font-semibold text-text-primary bg-ox-purple text-sm sm:text-base h-10 sm:h-11 px-5 sm:px-6 hover:bg-white-500 rounded-md" + className="font-semibold text-text-primary bg-brand-purple text-sm sm:text-base h-10 sm:h-11 px-5 sm:px-6 hover:bg-white-500 rounded-md"- w-full bg-ox-content border border-dash-border rounded-lg + w-full bg-dash-surface border border-dash-border rounded-lg- "sticky top-0 z-30 bg-ox-content", + "sticky top-0 z-30 bg-dash-surface",Also applies to: 81-81, 101-101
apps/web/src/app/layout.tsx (1)
32-33: Consider removing the unnecessary alias.The
geistSansalias simply referencesGeistSanswithout adding any value. You could useGeistSans.classNamedirectly in line 53 to reduce indirection.Apply this diff if you'd like to simplify:
-// Geist Sans - Primary font for body text and UI -const geistSans = GeistSans;And update line 53:
- className={`${geistSans.className} ${dmMono.variable} antialiased bg-background`} + className={`${GeistSans.className} ${dmMono.variable} antialiased bg-background`}apps/web/docs/DESIGN_SYSTEM.md (1)
1-526: Excellent comprehensive design system documentation.This documentation provides clear guidelines for:
- Color system with semantic naming
- Typography standards
- Spacing conventions
- Component patterns
- Migration guide with concrete examples
- Best practices and anti-patterns
The examples throughout are helpful and the structure is easy to follow. This will be valuable for maintaining design consistency.
Minor documentation improvement: Lines 485-486 contain bare URLs. Consider formatting them as proper Markdown links:
-**Tailwind CSS Documentation:** https://tailwindcss.com/docs -**Shadcn/ui Components:** https://ui.shadcn.com +**Tailwind CSS Documentation:** [https://tailwindcss.com/docs](https://tailwindcss.com/docs) +**Shadcn/ui Components:** [https://ui.shadcn.com](https://ui.shadcn.com)apps/web/src/components/landing-sections/Brands.tsx (1)
29-68: Token usage looks good; consider migrating remaining inline hex colorsThe updated classes (
bg-surface-primary,from-brand-purple, borders) are consistent with the new token system. The only outliers are the hard-coded colors in the box-shadow and gradient tails ([box-shadow:0_0_100px_-10px_#14111C_inset],to-[#341e7b],before:to-[#321D76]). If you expect to reuse these, consider adding them todesign-tokens.ts(e.g., as shadow/gradient tokens) to keep all brand colors centralized.apps/web/src/components/sheet/ProgressBar.tsx (1)
19-61: Design-token migration for ProgressBar looks solid; optional percentage clampThe move to
bg-dash-surface,border-dash-border, and token-based text/circle colors is consistent and should render correctly with the new theme. One optional hardening would be to clamppercentagebetween 0 and 100 before computingoffsetto avoid odd visuals ifcompleted > totalis ever passed in.apps/web/src/components/dashboard/Sidebar.tsx (1)
63-111: Sidebar token migration looks coherent; consider phasing outox-*surfaces laterThe sidebar/container, headers, and profile dropdown now consistently use
bg-dash-surface,border-dash-border, andtext-text-*/text-brand-purple, which fits the new dashboard palette. The only lingering legacy bits arebg-ox-profile-card(and elsewherebg-ox-content/bg-ox-sidebarin other components).If the goal is eventually to retire
ox-*tokens, it’s worth earmarking this profile card and any other newly-touchedox-*usages for a later pass to move them ontodash/surfacecolors as well. Functionally everything is fine as-is.Also applies to: 131-153, 189-253
apps/web/src/app/(main)/dashboard/sheet/page.tsx (1)
52-76: Sheet page styling now matches dash/text tokens;ox-*usage can be revisited laterThe migration to
border-dash-border,bg-dash-surfacefor the header row, andtext-text-*across cells/buttons looks consistent with the new design system. The checkbox and badge also useox-purplemapped to the brand purple, which is fine technically.Since
tailwind.config.tsflagsox-*as legacy, you may eventually want to swapbg-ox-content,hover:bg-ox-sidebar,hover:bg-ox-header/50, and theox-purplevariants over tosurface.*/dash.*colors, but that can be a separate cleanup without blocking this PR.Also applies to: 117-126, 323-400
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
.cursorrules(1 hunks)BRAND_CONSISTENCY_REPORT.md(1 hunks)apps/web/docs/DESIGN_SYSTEM.md(1 hunks)apps/web/src/app/(main)/dashboard/account/page.tsx(2 hunks)apps/web/src/app/(main)/dashboard/layout.tsx(2 hunks)apps/web/src/app/(main)/dashboard/pro/dashboard/page.tsx(2 hunks)apps/web/src/app/(main)/dashboard/sheet/page.tsx(8 hunks)apps/web/src/app/(main)/legal/cancellation-and-refund/page.tsx(3 hunks)apps/web/src/app/(main)/legal/privacy/page.tsx(22 hunks)apps/web/src/app/(main)/legal/shipping-and-exchange/page.tsx(4 hunks)apps/web/src/app/(main)/legal/terms/page.tsx(9 hunks)apps/web/src/app/(main)/sheet/[moduleId]/page.tsx(4 hunks)apps/web/src/app/globals.css(1 hunks)apps/web/src/app/layout.tsx(2 hunks)apps/web/src/components/dashboard/DashboardHeader.tsx(2 hunks)apps/web/src/components/dashboard/ProjectsContainer.tsx(5 hunks)apps/web/src/components/dashboard/Sidebar.tsx(9 hunks)apps/web/src/components/landing-sections/Brands.tsx(3 hunks)apps/web/src/components/landing-sections/Hero.tsx(3 hunks)apps/web/src/components/landing-sections/navbar.tsx(1 hunks)apps/web/src/components/legal/ContactInfo.tsx(3 hunks)apps/web/src/components/legal/LegalCard.tsx(1 hunks)apps/web/src/components/legal/LegalContent.tsx(1 hunks)apps/web/src/components/legal/LegalFooter.tsx(1 hunks)apps/web/src/components/legal/LegalPageHeader.tsx(1 hunks)apps/web/src/components/legal/LegalPageLayout.tsx(1 hunks)apps/web/src/components/sheet/OpensoxProBadge.tsx(1 hunks)apps/web/src/components/sheet/ProgressBar.tsx(2 hunks)apps/web/src/components/sidebar/SidebarItem.tsx(2 hunks)apps/web/src/components/ui/ActiveTag.tsx(1 hunks)apps/web/src/components/ui/custom-button.tsx(1 hunks)apps/web/src/components/ui/header.tsx(1 hunks)apps/web/src/lib/design-tokens.ts(1 hunks)apps/web/tailwind.config.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (13)
apps/web/src/components/sheet/OpensoxProBadge.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/components/legal/LegalCard.tsx (2)
apps/web/src/components/ui/card.tsx (2)
props(23-29)props(8-17)apps/web/src/components/ui/divider.tsx (1)
className(4-8)
apps/web/src/app/(main)/legal/cancellation-and-refund/page.tsx (2)
apps/web/src/components/legal/LegalCard.tsx (1)
LegalCard(10-28)apps/web/src/components/legal/index.ts (1)
LegalCard(4-4)
apps/web/src/components/ui/custom-button.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/components/dashboard/Sidebar.tsx (2)
apps/web/src/components/ui/IconWrapper.tsx (1)
IconWrapper(10-22)apps/web/src/components/dashboard/ProfilePic.tsx (1)
ProfilePic(11-59)
apps/web/src/app/(main)/legal/shipping-and-exchange/page.tsx (1)
apps/web/src/components/legal/LegalCard.tsx (1)
LegalCard(10-28)
apps/web/src/components/ui/header.tsx (2)
apps/web/src/components/ui/flickering-grid.tsx (1)
FlickeringGrid(23-199)apps/web/src/lib/design-tokens.ts (1)
colors(8-118)
apps/web/src/components/dashboard/ProjectsContainer.tsx (1)
apps/web/src/components/ui/table.tsx (5)
Table(112-112)TableHeader(113-113)TableRow(117-117)TableHead(116-116)TableCell(118-118)
apps/web/src/app/(main)/dashboard/account/page.tsx (1)
apps/web/src/components/ui/ActiveTag.tsx (1)
ActiveTag(8-23)
apps/web/src/app/(main)/dashboard/layout.tsx (1)
apps/web/src/components/ui/IconWrapper.tsx (1)
IconWrapper(10-22)
apps/web/src/app/(main)/dashboard/sheet/page.tsx (2)
apps/web/src/components/ui/table.tsx (5)
TableCell(118-118)Table(112-112)TableHeader(113-113)TableRow(117-117)TableHead(116-116)apps/web/src/components/ui/badge.tsx (1)
Badge(36-36)
apps/web/tailwind.config.ts (1)
apps/web/src/lib/design-tokens.ts (1)
colors(8-118)
apps/web/src/app/(main)/sheet/[moduleId]/page.tsx (1)
apps/web/src/components/ui/badge.tsx (1)
Badge(36-36)
🪛 LanguageTool
apps/web/docs/DESIGN_SYSTEM.md
[style] ~523-~523: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...on --- Last Updated: November 19, 2025 Maintained by: Opensox Team **Q...
(MISSING_COMMA_AFTER_YEAR)
🪛 markdownlint-cli2 (0.18.1)
apps/web/docs/DESIGN_SYSTEM.md
485-485: Bare URL used
(MD034, no-bare-urls)
486-486: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (25)
apps/web/src/components/sheet/OpensoxProBadge.tsx (1)
13-13: Design tokens verified and code is correct.All referenced tokens (
brand-purple,brand-purple-light) are properly defined inapps/web/src/lib/design-tokens.tsand correctly exposed throughapps/web/tailwind.config.ts. The opacity modifiers (/20,/30) are valid Tailwind syntax. The migration from legacyox-purpleto the new design token system is complete and working as intended.apps/web/src/components/landing-sections/navbar.tsx (1)
79-79: LGTM! Design token migration applied correctly.The replacement of
text-[#d1d1d1]withtext-text-tertiarycorrectly applies the semantic token for tertiary/muted text, which is appropriate for navigation links..cursorrules (1)
1-140: LGTM! Excellent design system documentation.This comprehensive guide provides clear guidelines for the design token system, including explicit rules against hardcoded colors and detailed documentation of available tokens. This will help maintain consistency across the codebase.
apps/web/src/app/(main)/dashboard/pro/dashboard/page.tsx (1)
20-20: LGTM! Correct design token usage.The migration from
text-whitetotext-text-primarycorrectly applies the semantic token for primary white text, aligning with the design system guidelines.Also applies to: 32-32
apps/web/src/components/legal/ContactInfo.tsx (1)
30-30: LGTM! Link tokens applied correctly.The replacement of
text-blue-400 hover:text-blue-300withtext-link hover:text-link-hovercorrectly applies the semantic link tokens, improving maintainability and consistency with the design system.Also applies to: 39-39, 49-49
apps/web/src/components/ui/ActiveTag.tsx (1)
14-16: LGTM! Clean token migration.The replacement of hardcoded hex colors with semantic success tokens (
bg-success-bg,text-success-text,border-success-border) aligns perfectly with the design system migration.apps/web/src/components/dashboard/DashboardHeader.tsx (2)
30-37: LGTM! Consistent token usage for branding elements.The migration from
ox-purpletobrand-purpletokens for the menu icon and link hover states maintains consistent branding.
44-44: Color contrast is sufficient—no changes needed.Verification confirms the combination of
text-text-primary(#ffffff) onbg-ox-purple(#5519f7) achieves a contrast ratio of 7.32:1, exceeding both WCAG AA (4.5:1) and WCAG AAA (7:1) requirements. The button is accessible as written.Likely an incorrect or invalid review comment.
apps/web/src/app/(main)/legal/cancellation-and-refund/page.tsx (1)
43-43: LGTM! Consistent semantic token usage.The migration to
text-brand-purplefor emphasis andtext-mutedfor secondary text follows the design system conventions properly.Also applies to: 83-107
apps/web/src/components/landing-sections/Hero.tsx (1)
26-27: LGTM! Proper token adoption.The updates consistently use semantic tokens (
text-text-secondary,from-surface-primary) for secondary text and gradient surfaces, aligning with the design system.Also applies to: 53-53, 77-77
apps/web/src/components/legal/LegalCard.tsx (1)
20-21: LGTM! Semantic tokens for both variants.Both card variants now use
bg-surface-tertiarywith appropriate border tokens, maintaining consistency with the design system.apps/web/src/components/dashboard/ProjectsContainer.tsx (2)
65-75: LGTM! Proper text token usage.The heading and button correctly use
text-text-primarytoken. However, note the button still usesbg-ox-purple(see separate comment).
87-89: LGTM! Consistent token usage for brand elements.The scrollbar styling and table header properly use
brand-purpleanddash-bordertokens, aligning with the design system.Also applies to: 95-100
apps/web/src/app/(main)/dashboard/layout.tsx (1)
19-43: LGTM! Exemplary token consistency.The layout demonstrates proper usage of the new design token system with consistent
dash-prefixes for surfaces/borders andbrand-prefixes for accent colors. This serves as a good reference for the token migration.apps/web/src/app/layout.tsx (1)
14-30: LGTM! Font configuration improved.The consolidation of separate font declarations into a single
dmMonodeclaration with multiple sources is cleaner and follows Next.js best practices. The addition offont-display: swapimproves performance by preventing flash of invisible text (FOIT).apps/web/src/components/ui/header.tsx (1)
5-9: LGTM! Design token integration is correct.The import of design tokens and the simplification of
border-[#252525]toborderaligns with the centralized design system. The default border color is properly defined in the Tailwind config.apps/web/src/components/sidebar/SidebarItem.tsx (1)
1-42: LGTM! Design token migration is consistent.All styling changes correctly migrate from hardcoded colors to design tokens:
hover:bg-[#292929]→hover:bg-dash-hover- Text color classes updated to semantic tokens (
text-text-secondary,text-text-tertiary,text-text-primary)The component maintains its functionality while adopting the centralized design system.
BRAND_CONSISTENCY_REPORT.md (1)
1-478: Excellent documentation of design consistency issues.This comprehensive analysis report provides valuable context for the PR changes. It clearly documents:
- The extent of the problem (130+ hardcoded colors)
- Specific inconsistencies (8+ purple variants)
- A prioritized remediation plan
- Concrete migration examples
This documentation will be helpful for maintaining design consistency going forward.
apps/web/src/app/(main)/legal/shipping-and-exchange/page.tsx (1)
44-207: LGTM! Consistent design token migration across the legal page.All color and styling updates correctly migrate from hardcoded values to semantic design tokens:
- Brand colors:
text-[#9455f4]→text-brand-purple- Text colors:
text-[#b1b1b1]→text-light- Link colors:
text-blue-400 hover:text-blue-300→text-link hover:text-link-hover- Surface/border colors properly tokenized
The changes maintain visual consistency while adopting the centralized design system.
apps/web/src/app/globals.css (1)
61-69: Body + code typography aligned with new font tokensUsing
font-sansonbodyandfont-monoforcode/pre/kbdcleanly ties the global typography to the updated Tailwind font families; no issues from a layout or UX perspective.apps/web/tailwind.config.ts (1)
176-186: Font-family configuration matches global usageDefining:
sans→var(--font-geist-sans)andmono/DMfont→var(--font-dm-mono)+ system monospace fallbackslines up with
font-sanson<body>andfont-monofor code blocks inglobals.css. This should keep typography consistent across the app without surprises.apps/web/src/app/(main)/legal/terms/page.tsx (1)
530-541: Tailwind color utilities are correctly configured—review comment is incorrectThe tailwind.config.ts explicitly defines
border.lightandtext.lightwithin the extended color palette, which generates the CSS utilitiesborder-lightandtext-lightas expected. The code in lines 531 and 540 ofterms/page.tsx(and the other legal pages flagged by the search) is correct and will render CSS without issue.Likely an incorrect or invalid review comment.
apps/web/src/app/(main)/legal/privacy/page.tsx (1)
370-375: The review comment is incorrect and should be disregarded.The utilities
text-lightandborder-lightcorrectly map to the configured tokens intailwind.config.ts. Tailwind's nested color configuration generates utilities from the nested keys directly:
text: { light: colors.text.light }generates thetext-lightutility (nottext-text-light)border: { light: colors.border.light }generates theborder-lightutility (notborder-border-light)The suggested replacements (
text-text-lightandborder-border-light) would reference non-existent configuration keys and break the code. All instances across the legal pages are using the correct utility names and will properly apply the configured colors fromdesign-tokens.ts.Likely an incorrect or invalid review comment.
apps/web/src/lib/design-tokens.ts (2)
1-7: LGTM - Clear documentation.The file header effectively establishes the purpose and usage guidelines for the design tokens system.
120-187: LGTM - Well-structured token definitions.The
gradients,shadows,fonts,spacing,durations, andradiiobjects are properly formatted and follow CSS/design system best practices. Once the color hex values are corrected, the entire design token system will function correctly.
Summary by CodeRabbit
Documentation
Style