Skip to content

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

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

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

Conversation

@pavzagor

@pavzagor pavzagor commented Oct 5, 2026 •

Copy link
Copy Markdown

Share pages could reset a saved dark theme to light. This keeps share pages consistent with the viewer's theme, preserves destination-aware return links, and adds a visible My Caps fallback in both player and timeline views for signed-in viewers without a destination.

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

October 5 review fixes

The mobile fallback keeps its My Caps destination label visible. Main 94ad3fd8 is merged, preserving destination-aware organization/folder/space return links, organization switching before navigation, the updated copy-link menu, and the accessible sidebar reopen control. Destination-aware links take precedence; signed-in viewers without a destination get the My Caps fallback. Timeline controls use a second row at phone widths.

Validation on daa54170: 43 focused navigation/theme/player tests passed; scoped Biome passed. Eight real-navigation-component fixtures cover player/timeline, fallback/destination, and 360/393px widths: visible text, at least 44px targets, and no horizontal overflow. Independent review found no concrete regression in the merge. Fixtures use mocked context and surrounding layout, not an authenticated full-page session. The attempted full web typecheck did not pass with the isolated checkout's missing shared-project outputs and reused primary dependency links; fresh CI needs upstream workflow approval.

360px timeline fixture · Measurements

RetriggerConfidence Score: 4/5

The PR appears safe to merge, though the mobile return control should make its destination visibly clear.

Findings

  1. P2 Mobile destination label is hidden ▶
Fix with agent prompt
### Issue 1
apps/web/app/s/[videoId]/_components/ShareNavigation.tsx:16
Below the `sm` breakpoint, the “My Caps” text is visually hidden in both the player and timeline headers. Signed-in mobile viewers see only a back arrow, so they cannot tell at a glance that it leads to their library. The link still works, but a visible destination cue would make it easier to discover.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR restores saved or system-based themes on share pages, synchronizes notification styling with the page theme, replaces fixed light colors, and adds a library return link to player and timeline views.

  • The return link’s destination text is visually hidden on mobile.

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

Comment thread apps/web/app/s/[videoId]/_components/ShareNavigation.tsx Outdated

This branch has not been deployed

No deployments
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.

1 participant