fix: address review feedback — params, theme flash, Monaco theme, auth race, socket cleanup, gradient tokens + merge upstream/main by SourabhX16 · Pull Request #18 · Omnikon-Org/UnVibe

SourabhX16 · 2026-07-02T14:37:51Z

Summary

Six fixes addressing the merge review feedback, plus a merge of upstream/main (7 file conflicts resolved). All six fixes are preserved after the merge. The branch is up to date with upstream/main, no remaining conflicts.

Must-fix (blocking)

1. `params` not awaited in ModulePage (Next.js 15 compat)

Commit: 9e47097 | File: apps/web/src/app/app/tracks/[trackId]/modules/[moduleId]/page.tsx

Problem: ModulePage was a Client Component destructuring params synchronously:

export default function ModulePage({ params }: { params: { trackId: string; moduleId: string } }) {

In Next.js 15, params becomes a Promise. This code would break on upgrade.

Fix: Since we're on React 18 (use() not available), we can't use use(params) in a Client Component. Instead, the page was split:

page.tsx → async Server Component that awaits params and passes them as plain props
module-page-content.tsx → new Client Component ("use client") receiving { trackId, moduleId } as regular props

This works identically on Next.js 14 today (awaiting a plain object resolves immediately) and will work on Next.js 15 after upgrade with zero changes.

Scope: Grep of the entire apps/web/src/app/ tree confirmed this was the only file destructuring params or searchParams — no other fixes needed.

Post-merge update: After merging upstream/main, the inner Client Component was updated to use upstream's tRPC queries (trpc.tracks.getById.useQuery, trpc.modules.getById.useQuery) instead of the old mock data hooks, since upstream had already replaced the mock data layer.

2. Dark-mode flash on light-mode load

Commit: 6e0d392 | File: apps/web/src/app/layout.tsx

Problem: <html> had a hardcoded className="dark". The ThemeProvider toggles the dark class via useEffect — which runs after hydration. A user with a saved light-mode preference would see a dark page flash before React corrected it.

Root cause analysis:

Theme preference is persisted via Zustand store → localStorage("unvibe-theme") → values "dark" or "light"
Zustand initialized on client only (hydration), so the server always rendered className="dark"
No inline script existed to read the stored preference before first paint

Fix (3-part):

Removed className="dark" from <html lang="en"> — the server no longer hardcodes dark mode
Added suppressHydrationWarning to <html> — silences the false-positive hydration mismatch warning. The warning would fire because our inline script (step 3) mutates the DOM before React hydrates, causing the server-rendered class and client class to differ. This is intentional and harmless — suppressHydrationWarning silences just that one attribute on <html>.

Added blocking inline <script> before <body> content:

(function(){
  try {
    var t = localStorage.getItem("unvibe-theme");
    if (t === "light") document.documentElement.classList.remove("dark");
    else document.documentElement.classList.add("dark");
  } catch(e) {}
})()

This runs synchronously before React hydrates, reads the persisted preference, and sets the correct class on <html> immediately — zero flash.

What didn't change: The existing useEffect in ThemeProvider still handles class toggling when the user changes theme after load. The inline script only covers the initial paint.

Verification: Hard-refresh the page in light mode — the page loads directly in light mode with no dark flash.

3. Monaco editor ignores theme setting

Commit: 4c8994c | File: apps/web/src/components/features/code-editor.tsx

Problem: The Monaco editor had theme="vs-dark" hardcoded. Toggling the app to light mode left the code editor in dark mode — a visual inconsistency.

Fix:

import { useUIStore } from "@/stores/ui-store";
// ...
const darkMode = useUIStore((state) => state.darkMode);
// ...
<Editor theme={darkMode ? "vs-dark" : "light"} ... />

Now the editor's theme tracks the app's theme toggle in real time.

Post-merge formatting: Upstream/main reformatted the component's function signature to multiline. Both changes are preserved in the merged result.

4. `asChild` + `onClick` race condition in auth flow

Commit: 4ac870f + merged via 1269c6f | Files: apps/web/src/app/auth/signin/page.tsx, apps/web/src/app/auth/signup/page.tsx

Original problem: Both auth pages used:

<Button asChild onClick={signIn}>
  <Link href="/app/dashboard">Enter mock workspace</Link>
</Button>

With asChild, the Button renders as a <Link>. The onClick (calling signIn() from the mock auth store) could race with the Link's native navigation — signIn might not fire before the browser navigates.

Fix (original): Removed the asChild/Link wrapper. Used a plain <Button> with onClick={() => { signIn(); router.push('/app/dashboard'); }}. Added a TODO for when real NextAuth wiring replaces the mock.

Post-merge resolution: Upstream/main had already replaced both pages with real auth forms (email/password inputs, validation, loading state, error display, real next-auth/react signIn for OAuth). The upstream version naturally avoids the asChild race — it uses onClick={handleSignIn} on a plain <Button> with no Link wrapper. We accepted upstream's version in the merge. Our TODO was retired since real auth is already wired.

The merged result:

Real OAuth buttons (signIn("github"), signIn("google") with next-auth/react)
Real email/password form with validation (check handleSignIn/handleSignUp)
Loading states with LoadingPanel
Error messages for invalid credentials
All asChild/Link patterns eliminated

Minor (cleanup)

5. Dead socket wiring in war-room-live

Commit: 9aa842f | File: apps/web/src/components/features/war-room-live.tsx

Problem: useEffect called getSocket().connect() and socket.disconnect() in cleanup, but no socket event listeners were ever registered. The only data coming into the component was from a setInterval pushing mock messages. The socket calls were dead code.

Fix:

Removed import { getSocket } from "@/lib/socket/client" (eliminates an ESLint no-unused-vars warning)
Removed const socket = getSocket() and socket.connect() from the effect body
Removed socket.disconnect() from the cleanup function
Left a // TODO: wire real socket events once War Room backend lands comment above the setInterval so the intent is clear when the backend arrives

Post-merge update: Accepted upstream's import path change (@unvibe/types instead of @/lib/mock-data/types). The socket removal and TODO are preserved.

6. Hardcoded gradient hex values in ThemeProvider

Commit: f56b01d | Files: apps/web/src/app/globals.css, apps/web/src/components/app/theme-provider.tsx

Problem: ThemeProvider contained raw hex values inline:

Dark: radial-gradient(125% 125% at 50% 100%, #000000 40%, #010133 100%)
Light: radial-gradient(125% 125% at 50% 90%, #ffffff 40%, #ec4899 100%)

These weren't tied to the design token system. If the palette changed, they'd need manual updates.

Fix:

Added --gradient-radial to :root (light) and .dark (dark) in globals.css, alongside the existing CSS custom properties
Simplified ThemeProvider to: style={{ background: "var(--gradient-radial)" }}
The CSS variable resolves automatically based on the .dark class on <html> — no conditional logic needed in the component

Merge Details

Commit: 1269c6f

Merged upstream/main into feat/frontend-app-shell with 7 file conflicts, all resolved manually:

File	Resolution strategy
`page.tsx` (module)	Keep our async Server wrapper, update inner component with upstream's tRPC
`signin/page.tsx`	Accept upstream (real auth form supersedes our mock fix)
`signup/page.tsx`	Accept upstream (real auth form supersedes our mock fix)
`layout.tsx`	Keep our flash-fix version (upstream still had old hardcoded `className="dark"`)
`theme-provider.tsx`	Keep our CSS-variable version (upstream still had old inline hexes)
`code-editor.tsx`	Merge: upstream multiline signature + our darkMode logic
`war-room-live.tsx`	Merge: upstream import path + our socket cleanup

Also fixed 4 pre-existing build blockers found during merge verification (all from upstream's new code):

Installed dotenv dependency (missing in next.config.mjs)
Installed bcryptjs + @types/bcryptjs for upstream's new auth router (api)
Ran prisma generate (stale client after schema changes)
Built @unvibe/types (unbuilt after type additions)

Verification

pnpm lint     # ✅ No ESLint warnings or errors
pnpm build    # ✅ Compiled successfully, all 13 pages generated

Route table:

Route (app)                                   Size
┌ ○ /                                         2.7 kB
├ ○ /app                                      138 B
├ ○ /app/blindspot-map                        1.76 kB
├ ○ /app/dashboard                            4.29 kB
├ ○ /app/profile                              2.22 kB
├ ○ /app/tracks                               1.7 kB
├ ƒ /app/tracks/[trackId]/modules/[moduleId]  11.5 kB
├ ○ /app/war-room                             3.49 kB
├ ○ /auth/signin                              2.71 kB
├ └ ○ /auth/signup                            2.79 kB

No regressions in route structure, page rendering, or type safety.

How to review

Each of the 6 original fixes is in its own commit for focused review:

Commit	Fix	Scope
`9e47097`	Await params	2 files (page.tsx + new module-page-content.tsx)
`6e0d392`	Dark flash	1 file (layout.tsx)
`4c8994c`	Monaco theme	1 file (code-editor.tsx)
`4ac870f`	Auth race	2 files (signin + signup pages)
`9aa842f`	Socket cleanup	1 file (war-room-live.tsx)
`f56b01d`	Gradient tokens	2 files (globals.css + theme-provider.tsx)

The merge commit 1269c6f shows how each conflict was resolved.

… 15 compat) ModulePage was a Client Component destructuring params synchronously. In Next.js 15, params is a Promise. Since React 18 doesn't have use(), wrap the client logic in a thin async Server Component that awaits params and passes them as plain props to the inner Client Component. No other files in the app/ tree destructure params or searchParams — this was the only instance.

Remove hardcoded className="dark" from <html>. Add blocking inline script that reads localStorage("unvibe-theme") and sets the class before first paint, matching the Zustand store's persistence strategy. Add suppressHydrationWarning to <html> to silence the false-positive mismatch warning since the DOM is intentionally mutated before hydration.

Replace hardcoded theme="vs-dark" with a dynamic value read from useUIStore.darkMode so the editor matches the app's light/dark setting.

Button with asChild wrapping a Link could cause onClick (signIn) to not fire before navigation. Replace with a plain Button that calls signIn() then router.push() in the handler. Add TODO noting this changes when real NextAuth wiring lands.

Socket connect/disconnect was called but no socket events were listened to — only a setInterval drove mock messages. Remove the socket calls and unused getSocket import. Leave a TODO to wire real socket events when the War Room backend lands.

Define --gradient-radial in :root (light) and .dark (dark) in globals.css. Replace the conditional inline style in ThemeProvider with a single var(--gradient-radial) reference, which resolves automatically based on the .dark class on <html>.

mergify · 2026-07-02T14:39:28Z

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

Queue this pull request

…hell # Conflicts: # apps/web/src/app/app/tracks/[trackId]/modules/[moduleId]/page.tsx # apps/web/src/app/auth/signin/page.tsx # apps/web/src/app/auth/signup/page.tsx # apps/web/src/app/layout.tsx # apps/web/src/components/app/theme-provider.tsx # apps/web/src/components/features/code-editor.tsx # apps/web/src/components/features/war-room-live.tsx

SourabhX16 · 2026-07-02T15:02:47Z

@Yuvraj-Sarathe @SharanyoBanerjee Ready for review whenever you have a moment.

Yuvraj-Sarathe · 2026-07-03T08:19:25Z

@SourabhX16 your branch isn't up-to-date with current state of main branch

Sync the branch

Add SS of UI changes in PR. If it is better than what it is currently I will merge it.

Add a loading state and matching scrollbar in the UI; this is a must!

Currently the website loads pages, but the user does not see if the page is being loaded or not. Only once the page is rendered do they see the page.

Also add a logo and favicon for the same.

Your TODOs:

Sync the repo
If your UI is not as good as the current one, don't push your UI changes
Add matching scrollbar
Add matching logo and favicon
Improve readability
Add skeleton loading state or something similar
Add ss of UI changes you are pushing in the PR

Side Note: Make sure the logo is not green or environment type but is techy but also not overly complicated. Try not to use blue or purple colors.

Yuvraj-Sarathe · 2026-07-03T08:20:49Z

@SourabhX16 Once you do the above changes, merge conflicts would be solved by themselves only if they still exist. MAke changes until they are resolved and make sure your ss are after the conflicts have been resolved!

SourabhX16 · 2026-07-03T14:21:17Z

@Yuvraj-Sarathe noted 👍 will solve it asap.

Merge upstream/main (71 commits) into feat/frontend-app-shell. Conflicts resolved: - apps/web/src/app/app/tracks/[trackId]/modules/[moduleId]/page.tsx Keep our async Server Component wrapper (Next.js 15 compat) - apps/web/src/app/globals.css Merge: keep --gradient-radial tokens + upstream --success/--warning tokens - apps/web/src/app/layout.tsx Keep our dark-mode flash-fix inline script + add metadata.icons - apps/web/src/components/app/theme-provider.tsx Keep our CSS-variable gradient approach (var(--gradient-radial)) New additions (reviewer feedback): feat(ux): add SkeletonLoader component with page variants - apps/web/src/components/ui/skeleton-loader.tsx Wraps skeleton primitives with dashboard/tracks/war-room/module variants - apps/web/src/app/app/tracks/page.tsx Replace LoadingPanel with SkeletonLoader variant='tracks' - apps/web/src/app/app/tracks/[trackId]/modules/[moduleId]/module-page-content.tsx Replace LoadingPanel with SkeletonLoader variant='module' feat(ux): add themed scrollbar styling (light + dark mode) - apps/web/src/app/globals.css Add ::-webkit-scrollbar rules outside @layer for light + .dark modes feat(brand): add logo and favicon - apps/web/public/logo.png (512x512, coral/teal geometric UV mark) - apps/web/public/favicon.ico (copied from src/app/favicon.ico) - apps/web/src/app/layout.tsx: add metadata.icons for /favicon.ico + /logo.png

…ntent Upstream removed the eyebrow prop from PageHeader component. Replace eyebrow + description with single description that includes track title as context prefix. Fixes TypeScript build error.

vercel · 2026-07-06T16:34:07Z

@SourabhX16 is attempting to deploy a commit to the Yuvraj Sarathe's projects Team on Vercel.

A member of the Team first needs to authorize it.

ThemeController conditionally renders Moon vs Sun based on Zustand darkMode state. Zustand only initializes on the client, so the server always renders the default while the client may render a different icon. This caused a React hydration error: 'Expected server HTML to contain a matching <circle> in <svg>' Fix: add a mounted guard that defers the conditional icon to after first useEffect, rendering Sun as a neutral SSR placeholder in the meantime. Both server and client agree on Sun until mount; then the client silently swaps to the correct icon.

SourabhX16 added 6 commits July 2, 2026 19:38

fix: wire Monaco editor theme to UI store darkMode

4c8994c

Replace hardcoded theme="vs-dark" with a dynamic value read from useUIStore.darkMode so the editor matches the app's light/dark setting.

SourabhX16 changed the title ~~fix: address reviewer feedback — params, theme flash, Monaco theme, auth race, socket cleanup, gradient tokens~~ fix: address review feedback — params, theme flash, Monaco theme, auth race, socket cleanup, gradient tokens + merge upstream/main Jul 2, 2026

Yuvraj-Sarathe assigned SourabhX16 Jul 3, 2026

Yuvraj-Sarathe added enhancement New feature or request help wanted Extra attention is needed labels Jul 3, 2026

SourabhX16 added 2 commits July 6, 2026 21:58

fix: remove deprecated eyebrow prop from PageHeader in module-page-co…

04645e5

…ntent Upstream removed the eyebrow prop from PageHeader component. Replace eyebrow + description with single description that includes track title as context prefix. Fixes TypeScript build error.

SourabhX16 closed this Jul 10, 2026

SourabhX16 deleted the feat/frontend-app-shell branch July 10, 2026 18:02

Navigation Menu

Uh oh!

fix: address review feedback — params, theme flash, Monaco theme, auth race, socket cleanup, gradient tokens + merge upstream/main - #18

fix: address review feedback — params, theme flash, Monaco theme, auth race, socket cleanup, gradient tokens + merge upstream/main#18
SourabhX16 wants to merge 10 commits into
Omnikon-Org:mainfrom
SourabhX16:feat/frontend-app-shell

SourabhX16 commented Jul 2, 2026 •

edited

Loading

Uh oh!

mergify Bot commented Jul 2, 2026

Uh oh!

SourabhX16 commented Jul 2, 2026

Uh oh!

Yuvraj-Sarathe commented Jul 3, 2026

Uh oh!

Yuvraj-Sarathe commented Jul 3, 2026

Uh oh!

SourabhX16 commented Jul 3, 2026

Uh oh!

vercel Bot commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

SourabhX16 commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Must-fix (blocking)

1. params not awaited in ModulePage (Next.js 15 compat)

2. Dark-mode flash on light-mode load

3. Monaco editor ignores theme setting

4. asChild + onClick race condition in auth flow

Minor (cleanup)

5. Dead socket wiring in war-room-live

6. Hardcoded gradient hex values in ThemeProvider

Merge Details

Verification

How to review

Uh oh!

mergify Bot commented Jul 2, 2026

Uh oh!

SourabhX16 commented Jul 2, 2026

Uh oh!

Yuvraj-Sarathe commented Jul 3, 2026

Uh oh!

Yuvraj-Sarathe commented Jul 3, 2026

Uh oh!

SourabhX16 commented Jul 3, 2026

Uh oh!

vercel Bot commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SourabhX16 commented Jul 2, 2026 •

edited

Loading

1. `params` not awaited in ModulePage (Next.js 15 compat)

4. `asChild` + `onClick` race condition in auth flow