Skip to content

fix: restore share page navigation and dark theme - #2261

Open
pavzagor wants to merge 4 commits into
CapSoftware:mainfrom
pavzagor:codex/share-navigation-dark-theme
Open

fix: restore share page navigation and dark theme#2261
pavzagor wants to merge 4 commits into
CapSoftware:mainfrom
pavzagor:codex/share-navigation-dark-theme

Conversation

@pavzagor

@pavzagor pavzagor commented Sep 9, 2026

Copy link
Copy Markdown

Opening a shared video could reset a saved dark theme to light and leave the return-to-dashboard action hidden inside Manage Cap. This adds a visible My Caps link in both player and timeline views and keeps share pages consistent with the viewer's theme.

Dashboard/share theme setup and cleanup now run before paint. Notifications observe the resolved page theme. The share route restores the theme on direct loads and client navigation, honors an explicit light/dark preference, and falls back to the system theme when none is saved. Fixed white panels and fixed gray text use theme-aware colors. The mobile return control has a 44 × 44 px minimum target. Signed-out viewers retain the existing navigation.

Validation

  • 15 focused tests pass: theme initialization, cookie handling, system fallback, signed-in/signed-out navigation, pre-paint dashboard/share transitions, cleanup, and system-dark notifications.
  • Next.js route type generation and the web tsc --noEmit check pass.
  • Scoped Biome checks pass with existing warnings in TimelineHoverGhost.tsx and the player module; diff whitespace check passes.
  • Browser checks confirm direct-load and dashboard-to-video dark theme, player/timeline return links, and no horizontal overflow at 393 px.
  • The clean-checkout TypeScript project build is blocked by 14 Buffer/ArrayBufferView compatibility errors in unchanged shared packages: database auth/crypto and backend comment/storage signing. These packages have no diff against main.

Evidence

Captured against the real local app with the synthetic Diarization Tester account and its existing test recording. Local media storage on port 9000 was unavailable, so the player displays an error in both versions. This validates navigation and appearance, not successful playback or production delivery.

Before After
Before: light share page, no visible My Caps link After: dark share page with My Caps link

Mobile before · Mobile after

The videos contain actual browser frames sampled at approximately 4 fps and encoded at 30 fps; idle gaps are shortened. They are not continuous 30 fps screen recordings. Evidence is committed with this change so the links remain tied to the reviewed commit.

Independent review follow-up

The 320 px timeline header now stacks its title/navigation above the help and view controls, preventing the avatar from overlapping the help button. Desktop layout remains unchanged from 640 px upward. Player tooltips and shortcut badges use paired theme colors, correcting an existing low-contrast keycap.

The local app was stopped during this review pass. The following evidence uses an isolated browser fixture with the current source classes, real Tailwind configuration, local font, and placeholder owner/title content; it is not a full app replay. Measured overlap at 320 px fell from 25.11 px to zero, and the header remains 56 px tall at 640/1280 px.

Timeline fixture before · Timeline fixture after · Measured rectangles · Tooltip contrast before/after

RetriggerConfidence Score: 5/5

The PR appears safe to merge with no remaining actionable findings.

Summary

  • Applies the share theme before paint on direct loads and client-side navigation.
  • Adds an accessible My Caps link without changing signed-out navigation.
  • Makes share surfaces and player tooltip colors theme-aware.
  • Stacks the timeline header on narrow screens to prevent control overlap.
  • Adds focused coverage for theme initialization, navigation transitions, cleanup, notifications, and account navigation.

Reviews (4) · Last reviewed commit: "fix: keep narrow share controls and dark..."

Comment thread apps/web/app/s/ShareTheme.tsx Outdated
@richiemcilroy

Copy link
Copy Markdown
Member

Hey @pavzagor thanks so much! can you get this to 5/5? you can request from greptile in the comments

@pavzagor

Copy link
Copy Markdown
Author

@greptileai Please review the latest head, bb3dce3, including the remaining theme-lifecycle edge cases.

The reported flash is addressed on both sides: dashboard and share theme setup/cleanup now use layout effects, so the outgoing cleanup and incoming theme restoration complete before paint. Changing only the share hook would leave the dashboard passive cleanup able to overwrite it.

Four new regression tests mount the actual dashboard/share components and inspect the theme during layout effects: dark navigation in both directions, explicit light overriding system dark, system fallback with dashboard-default restoration, and cleanup when leaving for marketing. Three failed before the fix; all four now pass. All 14 focused tests and the web tsc --noEmit check pass.

Please report any remaining findings and update the confidence assessment for this commit.

@pavzagor

Copy link
Copy Markdown
Author

@richiemcilroy lfg!

@pavzagor

Copy link
Copy Markdown
Author

@greptileai Please review the new final head, b843f92.

The additional independent pass reproduced one gap beyond the original finding: with system dark and no theme cookie, share pages were dark but Sonner notifications remained light. Notifications now observe the resolved body theme instead of polling the cookie. The new regression test verifies system-dark share notifications and restoration of light notifications on returning to a dashboard with no saved preference.

All 15 focused tests, scoped Biome checks, and the web tsc --noEmit check pass. Please check for remaining issues and update the confidence assessment for this head.

@pavzagor

Copy link
Copy Markdown
Author

@richiemcilroy 5/5!

Pls trigger docker image update when merging 👀

@pavzagor

Copy link
Copy Markdown
Author

@greptileai Please review final head b577231.

The independent follow-up found and fixed a confirmed 320 px timeline header overlap (25.11 px between avatar and help control). The header now stacks below 640 px, with no overlap at 320/393 px and the existing 56 px desktop header preserved at 640/1280 px. It also corrects the existing white shortcut badge contrast in the dark player. Browser fixture screenshots and measured rectangles are linked in the PR description, explicitly distinguished from full-app evidence.

All 15 focused tests, scoped formatting checks, and the web TypeScript check pass. Please review this head for any remaining findings and update its confidence assessment.

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