UX: Clean login page — shadcn polish [STU-4020] - #1
Open
BAWES wants to merge 4 commits into
Open
Conversation
added 2 commits
June 21, 2026 01:31
- Settings page: replaced raw rounded-lg border p-4 divs with shadcn Card components (Card, CardHeader, CardTitle, CardDescription, CardContent) - Header: removed backdrop-blur and backdrop-filter glass effects, solid bg-background instead - Login form: replaced inline spinner div with shared LoadingSpinner component for consistency All changes are Tailwind CSS only, no inline styles, no glass effects.
- Login form: wrapped in shadcn Card with CardHeader/CardContent structure, using CardTitle and CardDescription instead of bare h1/p - Register form: same Card wrapper pattern applied for consistency - Verify-email form: same Card wrapper pattern applied for consistency - All forms now use proper shadcn component hierarchy: Card → CardHeader (CardTitle, CardDescription) → CardContent (form elements) - No arbitrary Tailwind values, no inline styles, no glass effects - Consistent visual treatment across all public auth pages Part of STU-4020 — Clean login page shadcn polish
📝 WalkthroughWalkthroughAuth form components and the settings page adopt shared Card UI primitives; new Sidebar and MobileNav components provide responsive navigation; Header integrates these with simplified styling; AuthLayout adds responsive padding for desktop sidebar and mobile tab bar; design tokens and colors are updated with Zendesk branding. ChangesDesign system, navigation, and responsive layout overhaul
Sequence DiagramsequenceDiagram
participant User
participant Header
participant Sidebar
participant MobileNav
participant usePathname
User->>Header: Load page
Header->>usePathname: Read current route
usePathname-->>Header: Return pathname
Header->>Sidebar: Render (desktop)
Header->>MobileNav: Render (mobile)
Sidebar->>usePathname: Check active route
usePathname-->>Sidebar: Return pathname
Sidebar-->>Sidebar: Apply active styling
MobileNav->>usePathname: Check active route
usePathname-->>MobileNav: Return pathname
MobileNav-->>MobileNav: Apply active styling
Sidebar-->>User: Display desktop nav
MobileNav-->>User: Display mobile nav
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/app/globals.cssParsing error: This experimental syntax requires enabling one of the following parser plugin(s): "decorators", "decorators-legacy". (1:0) 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 |
added 2 commits
June 21, 2026 05:37
…okens Core changes: - Added Zendesk Coral (#eb6651) and Slack design tokens to tailwind.config.ts - Set Zendesk Blue (#1f73b7) as primary color and Coral (#eb6651) as accent in globals.css - Created Sidebar component (shadcn clean links, no borders, hover bg, blue active) - Created MobileNav component (fixed bottom tab bar with icon+label at <=760px) - Created useMediaQuery hook for responsive breakpoints - Updated MainNav with shadcn button-like link styles (rounded, bg on active/hover) - Updated Header to integrate sidebar + mobile nav - Updated auth layout with sidebar padding (lg:pl-60) and mobile bottom padding (pb-16) - Exported all new components from navigation index Design: Zendesk blue #1f73b7 primary, coral #eb6651 accent, Slack-inspired sidebar. No glass effects. Tailwind CSS only, no inline styles.
Changes: - header.tsx: Removed backdrop-blur glass effect (violates 'no glass' rule). Solid bg-background instead of semi-transparent + blur. - sidebar.tsx: Use bg-slack-sidebar (#f4f2ef) for Slack-inspired warm sidebar background instead of pure white bg-background. - mobile-nav.tsx: Add pb-[env(safe-area-inset-bottom)] for proper safe-area padding on notched phones.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/components/navigation/sidebar.tsx (1)
53-71: ⚡ Quick winExpose active link state to assistive tech.
When
isActiveis true, addaria-current="page"on the<Link>so screen readers can identify the current location.Suggested patch
<Link key={item.href} href={item.href} + aria-current={isActive ? 'page' : undefined} className={cn( 'group flex items-center gap-3 rounded-md px-3 py-2 text-sm font-medium transition-colors',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/navigation/sidebar.tsx` around lines 53 - 71, The Link component in the sidebar navigation does not expose the active page state to assistive technologies. Add the aria-current attribute to the Link element and set it to "page" when isActive is true to help screen readers identify the current page location. This can be done by adding aria-current={isActive ? "page" : undefined} as a prop on the Link component or by conditionally including the attribute only when isActive evaluates to true.src/components/navigation/main-nav.tsx (1)
15-32: ⚡ Quick winCentralize navigation items shared across desktop/mobile/header navs.
This route list duplicates
navItemsinsidebar.tsxandmobile-nav.tsx; extract one shared source (e.g.,src/components/navigation/nav-items.ts) to prevent cross-view drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/navigation/main-nav.tsx` around lines 15 - 32, The `links` array in main-nav.tsx is duplicated across sidebar.tsx and mobile-nav.tsx, creating a single source of truth problem. Extract this navigation items array into a new shared file at `src/components/navigation/nav-items.ts`, export it as a constant, and then update all three components (main-nav.tsx, sidebar.tsx, and mobile-nav.tsx) to import and use this shared export instead of maintaining their own copies.src/components/navigation/mobile-nav.tsx (1)
46-58: ⚡ Quick winAdd
aria-currenton active mobile tab link.Set
aria-current="page"for the active tab to improve navigation context for screen readers.Suggested patch
<Link key={item.href} href={item.href} + aria-current={isActive ? 'page' : undefined} className={cn( 'flex flex-col items-center gap-1 px-3 py-2 text-xs font-medium transition-colors',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/navigation/mobile-nav.tsx` around lines 46 - 58, The Link component in the mobile navigation does not include the aria-current attribute for accessibility purposes. Add an aria-current attribute to the Link component that is set to "page" when isActive is true, and undefined or omitted when isActive is false. This will provide proper navigation context to screen readers when a mobile tab is active. Reference the isActive variable that is already being used to conditionally apply the active styling to determine when to set aria-current.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/globals.css`:
- Around line 25-26: The --accent-foreground color variables on lines 26 and 53
are set to white (0 0% 100%), which creates insufficient contrast when used as
text color on the bright coral --accent background colors. To meet WCAG AA
contrast requirements, adjust the --accent-foreground values on both lines 26
and 53 from white to a significantly darker shade that provides adequate
contrast for normal-sized text against the accent background. Consider using a
dark color such as black or near-black to ensure the contrast ratio meets the
WCAG AA standard of at least 4.5:1 for normal text.
In `@src/components/navigation/sidebar.tsx`:
- Line 1: The ESLint configuration in the repository is missing rule definitions
for TypeScript ESLint rules, causing `@typescript-eslint/no-unused-vars` and
`@typescript-eslint/no-explicit-any` to be reported as unknown rules. Update the
repository's ESLint configuration file (typically .eslintrc.json, .eslintrc.js,
or eslint.config.js) to include proper rule definitions for these two TypeScript
ESLint rules by ensuring they are properly configured with appropriate severity
levels (error, warn, or off) so that static analysis can correctly detect and
report violations.
---
Nitpick comments:
In `@src/components/navigation/main-nav.tsx`:
- Around line 15-32: The `links` array in main-nav.tsx is duplicated across
sidebar.tsx and mobile-nav.tsx, creating a single source of truth problem.
Extract this navigation items array into a new shared file at
`src/components/navigation/nav-items.ts`, export it as a constant, and then
update all three components (main-nav.tsx, sidebar.tsx, and mobile-nav.tsx) to
import and use this shared export instead of maintaining their own copies.
In `@src/components/navigation/mobile-nav.tsx`:
- Around line 46-58: The Link component in the mobile navigation does not
include the aria-current attribute for accessibility purposes. Add an
aria-current attribute to the Link component that is set to "page" when isActive
is true, and undefined or omitted when isActive is false. This will provide
proper navigation context to screen readers when a mobile tab is active.
Reference the isActive variable that is already being used to conditionally
apply the active styling to determine when to set aria-current.
In `@src/components/navigation/sidebar.tsx`:
- Around line 53-71: The Link component in the sidebar navigation does not
expose the active page state to assistive technologies. Add the aria-current
attribute to the Link element and set it to "page" when isActive is true to help
screen readers identify the current page location. This can be done by adding
aria-current={isActive ? "page" : undefined} as a prop on the Link component or
by conditionally including the attribute only when isActive evaluates to true.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2015aea6-31fc-4668-910c-aaf26dd64597
📒 Files selected for processing (9)
src/app/(auth)/layout.tsxsrc/app/globals.csssrc/components/navigation/header.tsxsrc/components/navigation/index.tssrc/components/navigation/main-nav.tsxsrc/components/navigation/mobile-nav.tsxsrc/components/navigation/sidebar.tsxsrc/hooks/use-media-query.tstailwind.config.ts
✅ Files skipped from review due to trivial changes (2)
- src/components/navigation/index.ts
- tailwind.config.ts
Comment on lines
+25
to
+26
| --accent: 10 79% 62%; | ||
| --accent-foreground: 0 0% 100%; |
There was a problem hiding this comment.
Accent text contrast is likely below WCAG AA.
On Line 26 and Line 53, --accent-foreground is white while accent shades on Line 25 and Line 52 are relatively bright coral; this can produce insufficient contrast for normal-sized text in bg-accent text-accent-foreground surfaces.
Suggested token adjustment
- --accent-foreground: 0 0% 100%;
+ --accent-foreground: 0 0% 5%;
...
- --accent-foreground: 0 0% 100%;
+ --accent-foreground: 0 0% 5%;Also applies to: 52-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/globals.css` around lines 25 - 26, The --accent-foreground color
variables on lines 26 and 53 are set to white (0 0% 100%), which creates
insufficient contrast when used as text color on the bright coral --accent
background colors. To meet WCAG AA contrast requirements, adjust the
--accent-foreground values on both lines 26 and 53 from white to a significantly
darker shade that provides adequate contrast for normal-sized text against the
accent background. Consider using a dark color such as black or near-black to
ensure the contrast ratio meets the WCAG AA standard of at least 4.5:1 for
normal text.
| @@ -0,0 +1,82 @@ | |||
| 'use client'; | |||
There was a problem hiding this comment.
Fix missing ESLint rule definitions in repo lint config.
Static analysis reports @typescript-eslint/no-unused-vars and @typescript-eslint/no-explicit-any as unknown rules, which means linting is currently misconfigured and can miss real regressions.
🧰 Tools
🪛 ESLint
[error] 1-1: Definition for rule '@typescript-eslint/no-unused-vars' was not found.
(@typescript-eslint/no-unused-vars)
[error] 1-1: Definition for rule '@typescript-eslint/no-explicit-any' was not found.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/navigation/sidebar.tsx` at line 1, The ESLint configuration in
the repository is missing rule definitions for TypeScript ESLint rules, causing
`@typescript-eslint/no-unused-vars` and `@typescript-eslint/no-explicit-any` to
be reported as unknown rules. Update the repository's ESLint configuration file
(typically .eslintrc.json, .eslintrc.js, or eslint.config.js) to include proper
rule definitions for these two TypeScript ESLint rules by ensuring they are
properly configured with appropriate severity levels (error, warn, or off) so
that static analysis can correctly detect and report violations.
Source: Linters/SAST tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
STU-4020 — Clean Login Page Shadcn Polish
Changes Made
Login form (
src/components/auth/login-form.tsx):Cardcomponent (Card → CardHeader → CardContent)<h1>withCardTitleand<p>withCardDescriptionRegister form (
src/components/auth/register-form.tsx):Verify-email form (
src/components/auth/verify-email-form.tsx):Prior Changes (from last commit)
Design Compliance
Notes
tsc --noEmitcompleted — no new errors introduced (pre-existing errors in unrelated files unchanged)Screen Captures
(Screenshots available when running on dev server — current dev server not active)
Closes STU-4020
Summary by CodeRabbit
useMediaQueryhook for client-side viewport matching