Redesign snapshot detail: image-centered layout, checkmark approve, masks as hashtags - #82
Merged
Merged
Conversation
…asks as hashtag chips The image was competing with a full-width text Approve button, a two-row category/status header, and an always-open History section for vertical space, and masks -- despite three real backend scopes (global, per-image, category) -- were only ever visible as overlay rectangles while looking at one specific snapshot; there was no indication anywhere else, on that snapshot or any other view, that masks existed at all. - Status pill + category field collapse onto one compact header line; view-mode controls (Dual/Single, Show diff, Baseline/Candidate tabs) collapse onto one line directly above the image. - Approve becomes a small circular checkmark pinned to the top-right of the image area (the whole comparison container in Dual view, not nested in either pane) instead of a full-width button below it, colored via the same statusStyles().dot the status pill already uses -- amber/red/green/grey -- and disabled once status is "pass". Confirmed with the author this is a deliberate tradeoff: approving a passing snapshot isn't actually a no-op server-side (compare()'s MAX_DIFF_RATIO tolerates ~0.1% drift, so it can still re-pin the baseline), but a simpler "green = nothing to click" affordance won out over keeping that reachable from this button. - Masks render as a row of Instagram-style hashtag chips below the image (#global, #this image, #<category>), color-dotted by scope (categoryColor() for category scope, two new fixed constants for the other two), with a remove control only when the mask's id is actually known -- mirrors the on-image overlay's own pre-existing known-id-only-delete limit exactly (resolveMaskIds(), called a second time against the same inputs) rather than inventing a second, inconsistent notion of "unknown mask." - History becomes a collapsed-by-default <details> section. Design went through two architect SOLID-gate rounds (round 1 CONCERNS: the chip row's delete affordance couldn't always resolve a per-image mask's id, and the original "disable Approve on pass" justification rested on a false "it's a no-op" premise; both fixed -- round 2 SOUND). Real Docker + Playwright verification against the built image caught one further bug neither review nor the unit suite could have: in Single view the checkmark floated far right of the actual image, because its positioned ancestor was a full-width flex row instead of shrink-wrapped to the visible pane. Fixed by nesting the image content in its own `relative` div, itself centered by an outer `flex justify-center`, so the checkmark's absolute positioning anchors to the pane's real rendered width in both view modes. Verification: both architect-flagged fixes mutation-tested (reverted, confirmed the dependent tests fail, restored); new tests added for the disabled-on-pass checkmark and the chip row's scope labels/remove availability (60 viewer tests total, up from 55); full root-level lint/test/e2e suite green, including the real e2e suite's live-browser drive through Approve, masks, and category flows against this exact redesigned markup. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NrQeh9CXrigN1cvpJuyzy4
This was referenced Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up UX pass on the snapshot detail view, prompted by two observations:
two-row header, and an always-open History section.
only ever visible as overlay rectangles on one specific snapshot's own
candidate image -- nowhere else, on that view or any other, was there any
indication a snapshot had masks applied at all.
Baseline/Candidate tabs.
image area (covers both panes in Dual view), colored via the same
statusStyles().dotthe status pill uses -- amber/red/green/grey -- anddisabled once
status === "pass". This is a deliberate tradeoff: approveon
passisn't actually a no-op server-side (compare()'sMAX_DIFF_RATIOtolerates ~0.1% drift, so it can still re-pin the baseline), but a simpler
"green = nothing to click" affordance won out over keeping that reachable
from this button.
(
#global,#this image,#<category>), color-dotted by scope, with aremove control only when the mask's id is actually resolvable -- mirrors
the overlay's own pre-existing limit rather than inventing a new one.
<details>section.Process
Went through two architect SOLID-gate rounds:
per-image mask's id (no backend endpoint lists them), and the original
"disable Approve on pass" reasoning rested on a false "it's a no-op"
premise. Both fixed in the design before implementation.
Real Docker + Playwright verification against the actual built image caught
a bug neither review nor the unit suite could have: in Single view the
checkmark floated far right of the image, because its positioned ancestor
was a full-width flex row rather than shrink-wrapped to the visible pane.
Fixed by nesting the image content in its own
relativediv, centered by anouter
flex justify-center, so the checkmark anchors to the pane's realrendered width in both view modes. Confirmed visually after the fix.
Test plan
dependent test fails, restored)
pass/ enabled onfailandapproved-baseline-missing; chip row scope labels + remove-only-when-id-known (60 viewer tests total, up from 55)
npm run lint --workspacescleannpm test --workspaces-- 36 client + 60 viewer, all passingnpm run test:e2e -w examples/demo-app-- 6/6, live-browser run throughApprove/masks/category flows against this exact redesigned markup
ruff check+pytest-- 141/141 (no backend changes)expanded history, and the disabled green checkmark state
viewer/CODEMAP.mdupdatedCo-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01NrQeh9CXrigN1cvpJuyzy4