Add appearance preference previews - #6193
Conversation
17a4408 to
9117017
Compare
0c697f2 to
067b7d8
Compare
067b7d8 to
14ed894
Compare
Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com>
14ed894 to
bb13eac
Compare
Rich link preview — dark, narrowThe grouped setting stacks responsively; illustration details use the active semantic foreground token. Focus thread layout — dark, narrowThe overlaid thread and channel surfaces use semantic background and muted tokens for quieter dark-mode depth. Compact link preview — light, wideThe compact card uses soft semantic background shapes against the Buzz gradient. Split thread layout — light, wideThe split diagram preserves the side-by-side relationship and shared visual language. |
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: COMMENT — code clear; do not merge until required exact-head CI is rerun successfully
Reviewed: 93114c9c65138397de39729fde0a816eb9f314ab..bb13eac0164ca3b3ba2460f0b378c73fd74e8290 (exact head bb13eac0164ca3b3ba2460f0b378c73fd74e8290)
Risk: low code risk; merge-gate risk remains because required Desktop jobs were cancelled
Behavior/contracts traced
- The segmented control separates transient scrub preview from committed selection. Cancellation on blur, pointer cancel/lost capture, and unmount clears transient state; durable preference setters remain behind
onValueChange(desktop/src/shared/ui/segmented-control.tsx:72-101,138-156,202-208;desktop/src/features/settings/AppearanceSettingsControls.tsx:340-374,605-649). - Existing device-local, best-effort persistence and global thread-view scope are unchanged (
desktop/src/shared/lib/linkPreviewStylePreference.ts:40-47;desktop/src/shared/lib/threadViewModePreference.ts:11-14,61-73). No relay, tenancy, auth, IPC, or native persistence boundary changes. - The extracted presentation component preserves the production wrapper’s media URL rewrite and saved-preference behavior (
desktop/src/shared/ui/link-preview-attachment.tsx:24-85). Rich-card expansion still defaults on in production and is disabled only in the inert Settings sample (desktop/src/shared/ui/rich-link-preview-attachment.tsx:235-263,335-349;AppearanceSettingsControls.tsx:316-333). - The sample uses static data and an inline SVG, adds no network/native image work, and is excluded from interaction/accessibility traversal with
aria-hiddenplusinert(AppearanceSettingsControls.tsx:256-335). Native buttons in a labelledfieldset,aria-pressed, visible focus, reduced-motion handling, and normal keyboard activation preserve the accessibility contract (segmented-control.tsx:81-101,103-215). - Stacked-base risk was separated from the PR’s own single-commit delta: the commit changes 8 Desktop files (
+550/-174); unrelated mobile changes appear only in the misleading sibling-branch two-dot comparison. The sharedSegmentedControlmatches merged PR #5644 commit7e2651791.
Findings: No material code, product/UI, interaction, responsive, accessibility, persistence, security, or integration defect found.
Validation at clean matching HEAD
just desktop-typecheck— PASS.just desktop-test— PASS, 5,037/5,037.pnpm -C desktop build:e2e— PASS.pnpm -C desktop exec playwright test tests/e2e/appearance-previews.spec.ts --project=smoke— PASS, 3/3. Coverage includes transient preview without persistence, blur rollback, committed selection persistence, inert focus exclusion, expand-control suppression, and narrow geometry.- Four generated exact-artifact screenshots were inspected across dark/light, narrow/wide, and all preview modes: controls remained grouped; labels/descriptions were readable; previews were unclipped; selected states were distinct.
- Fresh review preflight found the exact remote head unchanged and no unresolved review threads.
Merge-gate blocker
GitHub run 32219145771 at this head is cancelled: Desktop Core, Desktop Smoke E2E shard 3, and Desktop E2E Integration shard 1/2 were cancelled, causing the aggregate Desktop checks to fail. Passing sibling shards, macOS build, and local exact-head evidence indicate no reproduced code failure, but they do not turn incomplete required CI green. Rerun the cancelled required jobs before merge.
Residual risk: No native Tauri window or Windows/Linux rendering pass was run. This frontend-only change has strong mock-bridge E2E and visual evidence, but platform-native rendering remains unverified.
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: APPROVE
Reviewed: 93114c9c65138397de39729fde0a816eb9f314ab..bb13eac0164ca3b3ba2460f0b378c73fd74e8290 (exact head bb13eac0164ca3b3ba2460f0b378c73fd74e8290)
Risk: medium — user-visible settings controls and a shared interaction primitive, bounded to Desktop renderer state
The prior code review remains clear at the unchanged exact head. No material systems/integration, persistence, security, product/UI, interaction, responsive, or accessibility defect was found. The change preserves commit-only preference updates while transient pointer scrubbing remains local; cancellation clears transient state; existing device-local persistence, production media rewriting, and saved-preference behavior remain unchanged. The inert sample adds no relay, IPC, native persistence, tenancy, auth, or network boundary.
Exact-head local validation previously passed:
just desktop-typecheckjust desktop-test— 5,037/5,037pnpm -C desktop build:e2epnpm -C desktop exec playwright test tests/e2e/appearance-previews.spec.ts --project=smoke— 3/3, covering transient preview without persistence, rollback, committed selection, inert focus exclusion, expand-control suppression, and responsive geometry
The former merge-gate caveat is resolved. GitHub workflow run 32219145771, attempt 2, completed successfully at this exact head. The previously cancelled Desktop Core, Desktop Smoke E2E (3), and Desktop E2E Integration (1/2) jobs now pass; aggregate Desktop and Desktop E2E Integration checks pass; gh pr checks 6193 --required exits 0.
Evidence provenance was rechecked: the retained attestation pins both initial and final HEAD to bb13eac..., and its run began after the commit was created. The retained build and Playwright logs are reopenable. Four screenshots were inspected during the original exact-head review but were not retained, so they are not being presented here as independently reopenable artifacts.
Residual risk: no native Tauri-window or Windows/Linux rendering pass was run. For this frontend-only change, exact-head mock-bridge E2E, local full Desktop tests, macOS build CI, and successful Desktop integration CI provide proportionate evidence.
…oundation * origin/main: Add appearance preference previews (#6193) fix(desktop): restore emoji recents (#6263) chore: serialize mobile pre-push checks (#6322) fix(buzz-acp): loosen workspace-scan guardrail to allow named paths (#6261) fix(buzz-dev-mcp): expand leading ~ in read_file/str_replace paths (#6271) perf(desktop): move five hot renderer paths from JS into Rust (#6024) fix(media): accept portrait video resolutions (#6058) Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
…urneys * origin/main: Add appearance preference previews (block#6193) fix(desktop): restore emoji recents (block#6263) Signed-off-by: Jude Edwards <judeedwards@squareup.com>
…-shortcut * origin/main: (341 commits) feat(desktop): make the Projects overview follow the selected section (block#6335) refactor(desktop): coordinate TTS playback (block#6341) fix(desktop): show complete repository trees (block#5102) Add appearance preference previews (block#6193) fix(desktop): restore emoji recents (block#6263) chore: serialize mobile pre-push checks (block#6322) fix(buzz-acp): loosen workspace-scan guardrail to allow named paths (block#6261) fix(buzz-dev-mcp): expand leading ~ in read_file/str_replace paths (block#6271) perf(desktop): move five hot renderer paths from JS into Rust (block#6024) fix(media): accept portrait video resolutions (block#6058) fix(desktop): hide archived channels from #/Tab autocomplete (block#6156) Unify mobile channel details (block#6113) Revert "fix(acp): gate relay-signed workflow messages on their attributed author" (block#6311) fix(desktop): morph the drawer panel icon instead of sliding it (block#6306) feat(desktop): refine repository-aware project workspaces (block#6003) Fix mobile Activity thread navigation (block#5850) perf(desktop): parallelize relay agent directory rebuild (block#6258) Refine the mobile emoji picker (block#5853) fix(desktop): exclude archived agents from nest, order regeneration (block#5905) Add font size and conversation density preferences (block#5644) ... Signed-off-by: John Funge <funge@squareup.com> # Conflicts: # desktop/src/features/messages/ui/MessageComposer.tsx # desktop/src/features/settings/ui/KeyboardShortcutsCard.tsx




Summary
Dependency
SegmentedControl, and responsiveSettingsOptionRow.main.Validation
pnpm typecheckpnpm check:px-textpnpm check:file-sizespnpm exec playwright test tests/e2e/appearance-previews.spec.ts --project=smoke(3 passed)