fix: restore share page navigation and dark theme - #2261
Conversation
|
Hey @pavzagor thanks so much! can you get this to 5/5? you can request from greptile in the comments |
|
@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 Please report any remaining findings and update the confidence assessment for this commit. |
|
@richiemcilroy lfg! |
|
@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 |
|
@richiemcilroy 5/5! Pls trigger docker image update when merging 👀 |
|
@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. |
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
tsc --noEmitcheck pass.TimelineHoverGhost.tsxand the player module; diff whitespace check passes.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.
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
The PR appears safe to merge with no remaining actionable findings.
Summary
Reviews (4) · Last reviewed commit: "fix: keep narrow share controls and dark..."