Skip to content

fix: resolve mobile overflow, make customizer a bottom sheet and use the brand logo everywhere - #29

Merged
itsnyein merged 1 commit into
mainfrom
fix/mobile-responsive-and-brand-logo
Aug 22, 2026
Merged

itsnyein merged 1 commit into
mainfrom
fix/mobile-responsive-and-brand-logo

Conversation

@nyeinphyoaung

Copy link
Copy Markdown
Collaborator

1. Header overflowed on small mobile

Sign in and Live demo already carried hidden sm:inline-flex, but the class was inert. They were built with buttonVariants({ className: "hidden sm:inline-flex" }), and cva appends the string rather than merging it - so the element ended up with the base inline-flex and hidden.

Class order in the attribute does not decide that; stylesheet order does. In the generated CSS .inline-flex is emitted after .hidden, so it won and the buttons stayed visible.

Fixed by routing through cn() so tailwind-merge drops the conflicting base class:

className={cn(buttonVariants({ size: "sm" }), "hidden sm:inline-flex")}

Verified at a 320px viewport: both compute to display: none. The GitHub pill is now shown on mobile instead, so the header is logo + pill.

2. A second overflow in the workflow section

Even with the header fixed the page still ran 9px wide at 320px. The workflow <li> grid items sized to min-content (309px) because of the long pnpm db:push && pnpm db:seed command, overflowing the 280px content box. Added min-w-0 to the grid item and to the <code> inside CopyCommand.

At a 320px layout width there are now zero overflowing elements and main.scrollWidth is exactly 320.

3. Customizer covered the whole screen on mobile

Both customizers pinned a fixed width (w-100 on the landing panel, w-[400px] on the dashboard one), which is wider than a phone viewport, so the sheet filled the screen and preset changes were invisible.

Both now switch on useIsMobile():

  • mobile - bottom sheet at h-[60svh], rounded top
  • desktop - unchanged; the dashboard sheet still follows sidebarConfig.side

Measured on the landing page at a mobile width: the sheet sits at top: 245, leaving 245px of page visible. Applying the Amber preset with it open visibly retinted the logo tile and badge dot above the sheet.

sm:max-w-none was added alongside the desktop width because the Sheet's own sm:max-w-sm (384px) was silently capping the intended 400px.

4. Brand logo everywhere

Ten SA text tiles across nine files replaced with the Command brand mark used by the landing navbar, footer and sign-in panel, each sized to its tile:

app/(auth)/sign-in/page.tsx        app/(auth)/sign-in-2/page.tsx
app/(auth)/sign-up-2/page.tsx      features/auth/sign-in.tsx
features/auth/sign-in-1.tsx        features/auth/sign-up-1.tsx
features/auth/reset-password-1.tsx (x2)
features/auth/reset-password-2.tsx
features/landing/mock-dashboard-ui.tsx

The last is the mock sidebar in the landing preview - included since it is a miniature of the app. A repo-wide grep for SA text tiles now returns nothing.

Verification

  • tsc --noEmit clean, eslint clean, next build compiles
  • Header link visibility and page overflow measured as computed styles at a real 320px viewport
  • Landing customizer bottom sheet measured and screenshotted, with a preset applied

Not visually verified: the dashboard customizer's mobile bottom sheet. The browser window would not resize below the desktop breakpoint on that page, so I confirmed the desktop panel renders at 400px and relied on it being the same useIsMobile() pattern already measured on the landing page. Worth a quick look on a real phone.

@vercel

vercel Bot commented Aug 22, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
shadcn-admindashboard Ready Ready Preview Aug 22, 2026 5:35pm

@claude

claude Bot commented Aug 22, 2026 •

Copy link
Copy Markdown

Claude finished @nyeinphyoaung's task in 2m 56s —— View job


Tasks

  • Gather context (diff, changed files, hooks/use-mobile, sheet.tsx, github-stars.tsx, hero/cta sections)
  • Review code against checklist (quality, architecture, conventions)
  • Post final review

Code Review: fix: resolve mobile overflow, make customizer a bottom sheet and use the brand logo everywhere

Summary

This is a small, well-scoped responsive/visual fix. The cn() fix for the cva/tailwind-merge class-order bug is correct and well diagnosed, the min-w-0 overflow fixes are appropriate for the grid/flex min-content issue, and the Command brand-mark replacement is mechanical and consistent. No functional or architectural problems found; one item is a self-disclosed verification gap worth closing before merge.


Medium Issues

1. Dashboard theme customizer's mobile bottom sheet is unverified (components/shared/theme-customizer/index.tsx:117-131)

side={
  isMobile
    ? "bottom"
    : sidebarConfig.side === "left"
      ? "right"
      : "left"
}
className={cn(
  "pointer-events-auto flex flex-col gap-0 overflow-hidden p-0 [&>button]:hidden",
  isMobile
    ? "h-[60svh] rounded-t-xl border-t"
    : "w-[400px] sm:max-w-none",
)}

The PR description explicitly flags this as not visually verified ("the browser window would not resize below the desktop breakpoint on that page"). The logic mirrors the landing customizer, which was measured, but this component also renders SheetHeader/tab content that could behave differently at 60svh inside the actual dashboard layout (different scroll container, sidebar overlap, etc.). Worth a quick manual check on a real device/narrow viewport before merge, since this is the primary in-app customizer, not just the marketing preview.


Minor Issues

2. PR description overstates the pre-existing state of the "Live demo" link (features/landing/components/landing-navbar.tsx:52-60)

<Link
  href="/dashboard"
  className={cn(
    buttonVariants({ size: "sm" }),
    "hidden sm:inline-flex",
  )}
>
  Live demo <ArrowRight className="size-4" />
</Link>

On origin/main this link had no hidden class at all (className={buttonVariants({ size: "sm" })}) — it was always visible on mobile. Only the "Sign in" link had the inert hidden sm:inline-flex. So this isn't fixing dead code on "Live demo"; it's new mobile-hiding behavior. The resulting mobile header (logo + GitHub pill only) does match what the PR description says it produces, and hero-section.tsx/cta-section.tsx still expose prominent "Explore the live demo" CTAs on mobile, so this isn't a functional regression — just worth correcting the description so reviewers don't assume "Live demo" was already broken the same way "Sign in" was.


Strengths

  • Correct root-cause diagnosis of the cva-appends-vs-cn()-merges class conflict, fixed idiomatically in features/landing/components/landing-navbar.tsx:44-57.
  • min-w-0 fixes in features/landing/components/workflow-section.tsx:17 and features/landing/components/copy-command.tsx:42 correctly target the actual cause (grid/flex items sizing to min-content) rather than papering over it with overflow-hidden.
  • Customizer responsive behavior reuses the existing useIsMobile() hook and cn() consistently across both the landing (features/landing/components/landing-theme-panel.tsx) and dashboard (components/shared/theme-customizer/index.tsx) customizers, keeping the two implementations aligned.
  • The sm:max-w-none addition correctly addresses the Sheet's own sm:max-w-sm cap silently shrinking the intended 400px panel — a real, non-obvious bug.
  • Brand mark replacement is thorough and consistent (Command sized per container) with no accessibility regression, since every tile sits next to visible "Shadcn Admin" text that still provides the link's accessible name.
  • Verification methodology described (computed styles at a real 320px viewport) is concrete and appropriate for this class of bug, and the PR is upfront about the one path it couldn't verify.

Verdict: No critical issues. Remaining items can be iterated on later — recommend a quick manual check of the dashboard customizer's mobile bottom sheet before merge.

@itsnyein
itsnyein merged commit d6646e3 into main Aug 22, 2026
4 checks passed
@itsnyein
itsnyein deleted the fix/mobile-responsive-and-brand-logo branch August 23, 2026 09:23

This branch was successfully deployed

1 active deployment
Preview — 9584e2ef Deployed Aug 22, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants