Keep image attachments compact with scrolling thumbnail strips

[morgmart]

What this does

Keeps posted images in compact, sideways-scrolling thumbnail strips with a visible count. Every image remains reachable, and clicking still opens the existing full-size viewer. Single images use the same compact thumbnail treatment without a redundant “1 image” label.

PDF, Markdown, and other document cards now have space between them and wrap as needed. This implements the chosen horizontal-strip direction (Option C), not the earlier grid proposal.

Why it matters

Image batches no longer turn a conversation into a long stack of large previews. The visible count tells you how much was sent even when later thumbnails are offscreen.

How it works

The shared message renderer groups only adjacent images or document cards, preserving the sender's order when images, documents, audio, and video are mixed. Each image strip with two or more images shows its own count; single images have no visible count. Fixed square tiles prevent image loading from changing the row height; unsafe URLs are excluded and missing media sources retain an unavailable slot.

Uploads, the ten-file composer limit, audio/video players, document previews, are unchanged. The newer expanded viewer is preserved, with one measurement adjustment so its opening and closing animation matches cropped thumbnails. This intentionally replaces the dimension-based posted preview sizing described in the existing attachment-layout documentation; cropping and compact single images follow the approved direction.

Verification

Current head: 334190c1, rebased onto main 2dd47966. The PR is open for review, not draft.

• Hosted CI run 36497149199 passed all required checks: JavaScript, Rust/tool integration, browser measurements, all six Chromium/WebKit journey groups, security checks, and DCO. Windows native validation was skipped. CI tested the synthetic merge of this head and base, not the raw branch head.
• Required push hooks passed on 334190c1: TypeScript, design-system checks, and 392 related unit tests across 33 files.
• All 42 selected browser checks passed in Chromium and WebKit on intermediate rebased head 4109706d: image strips, layout, navigation recovery, and plugins, including Projects disable/re-enable and disconnected navigation. These local results preceded the final clean rebase onto 2dd47966; the hosted CI result above covers the final integration.
• Independent source review found no new code or integration defects after the rebase. The image behavior and regression coverage were retained alongside main's connected-palette keyboard navigation.
• One browser case added, none removed: layout, keyboard scrolling, visible focus, and viewer motion need a browser. Count variants, unsafe/unavailable media, and mixed ordering remain covered by unit tests.
• Morgan tested the attachment layout. Human confirmation of the later keyboard-focus repair and native-device checks remain unconfirmed; green CI does not replace those checks.

Earlier validation included a WebKit plain-HTML wheel-helper failure that was not baseline-reproduced. It did not render changed app code. The current required hosted checks are green; this does not establish the cause of that earlier failure.

To try: send one image, then several with two documents. Single images have no count; batches retain their count during sideways scrolling. Open and close a wide image to check the updated viewer transition, then repeat in a narrow pane.

Keyboard focus follow-up (historical evidence)

At 711987b0, image pixels retain their rounded clip inside the focusable link, so the design-system keyboard outline is no longer clipped away. A rendered-pixel regression in the existing strip journey fails in Chromium and WebKit with the old CSS and passes with the fix at narrow, intermediate and wide sizes. No new browser cases were added. Required hooks passed 349 tests across 31 files plus TypeScript/design checks. Independent focused source/browser re-review found no remaining blocker. The browser passes ran on the pre-commit tree; the commit only formatted the test. Cursor flicker remains deferred by Morgan.

Screenshots

Synthetic sample artwork in the real conversation components. The captures show the conversation pane, not the entire viewport. Unrelated offline-fixture notifications are hidden in captures only.

Intermediate width, light theme, with keyboard focus visible:

Narrow width, dark theme:

Single-image treatment:

[wesbillman]

Star Lord automated source review

Published through Wes’s GitHub account. No actionable code findings; no changes requested. Reviewed adjacent image/file grouping and sender order, safe/unavailable/cached media handling, keyboard reachability and viewer handoff, cover-crop entrance/return geometry, comment/timecode state, and the added regression coverage. Groot’s independent source-only lane is complete and incorporated. The compact square single-image treatment and horizontal strips are intentional product choices, not regressions.

Optional documentation cleanup: docs/channels.md:604–611 still promises dimension-sized, uncropped previews. Update that paragraph to describe the fixed square cover-cropped thumbnails, adjacent-image strips, counts for two or more images, and wrapping document cards; retain the loading/blurhash invariants. This is not a request to revert the approved layout or a readiness blocker.

Reviewed revision and limits

• Head: 00441d530f5156c8f0f714c85954035e40ab1bf5
• Target base: 96f074343d580c0e27c0cb11da1153b5ff867c20; merge base: 1d6aee0896228f18a3f427ca65beab63906b65e7.
• Source-only: no tests/builds, PR-code execution, installs, or app launches. Added tests were inspected, not run by this reviewer.
• One hosted CI snapshot for this head showed CI required, JavaScript, Rust/tool integration, all six Chromium/WebKit shards, measurements, security checks and DCO passing; Windows native validation was skipped. This does not independently establish native layout/scrolling, visual focus rendering, or human acceptance. The PR’s reported earlier wheel-helper failure was not reproduced or diagnosed here, and its stated native/human checks remain unverified.

This COMMENT review is neither approval nor merge authorization.

[wesbillman]

Star Lord automated source re-review

Published through Wes’s GitHub account. No actionable findings in the keyboard-focus follow-up (00441d530f51 → 711987b0861e). This is a focused review of the three changed files, not a reopening of the already-reviewed feature.

The repair moves overflow and smooth-corner clipping to the inner pixel wrapper while leaving the focusable link unclipped. The wrapper inherits the measured clip/radius and cover-fit variables; reserved geometry, cached placeholders, keyed image/decode cleanup, and the viewer’s descendant-media lookup and origin measurements remain intact. The existing browser journey now compares rendered strip pixels with and without the outline color, rather than relying solely on a computed CSS declaration, while retaining keyboard scrolling and viewer/focus-return coverage.

My earlier source review missed the clipped-focus issue. A computed outline style does not establish visible focus; clipping and the actual production stylesheet import path must be traced separately. This review checks the repair in source, not an independently observed rendered result.

Revision and validation limits

• Head: 711987b0861e1f5c3d1a268252ff02b75804a167
• Target base: 96f074343d580c0e27c0cb11da1153b5ff867c20
• No tests/builds, PR-code execution, installs or app launches by this reviewer.
• In one hosted CI snapshot for this head, JavaScript, Rust/tool integration, measurements and all six browser shards were still running; security checks and DCO passed, Windows native validation was skipped. No CI polling or full-green claim.
• The author reports fail-before/pass-after Chromium/WebKit pixel checks on the pre-commit tree. Those runs were not reproduced here. Human confirmation of this keyboard-focus repair and native-device checks remain unverified; earlier layout acceptance is not acceptance of the new repair.

COMMENT only—not approval, completion of the repository’s human-testing checklist, or merge authorization.

[wesbillman]

Star Lord automated source re-review (via Wes’s account)

No new actionable source findings in the main-merge follow-up. This review checks integration since 711987b0861e1f5c3d1a268252ff02b75804a167; it does not reopen the already-reviewed attachment-layout feature or audit unrelated incoming main changes.

• Head: e71fc9cd62703d64d3669f4501c62e2de7cca245
• Base: 1108ab64cf232acfc0c8087db54b74ce73098bfe

All nine feature patches retain the same added/removed lines relative to their respective merge bases. Eight feature files are byte-identical to the prior reviewed head. Messages.module.css is the only file whose blob differs from both merge parents: its incoming thread-summary/action styling leaves thumbnail geometry, the unclipped focusable link, inner pixel clipping and keyboard-outline rules intact (:358–424). The cropped-image viewer measurement and rendered-pixel focus regression are preserved unchanged.

I also checked the adjacent incoming MediaAttachment change: the new description/preload props keep their prior defaults, and the existing MessageRow caller continues to use those defaults. Image strips still use AttachmentImage, not that alternate image renderer. No new browser cases or removed assertions were introduced by this merge.

Validation limits: source-only, using hash-verified pinned blobs and no dirty working-tree inputs. No tests/builds, PR-code execution, installs or app launches. The description’s local/browser evidence names earlier 711987b0/pre-commit inputs; those results are not independent validation of this merged head. Human confirmation of the keyboard-focus repair and native-device checks remain unverified.

One read-only snapshot of CI run 36477374464, associated with this head/base, showed JavaScript, Rust/tool integration, browser measurements, security checks and DCO passing. All six browser-journey shards were still running; Windows native validation was skipped. Final CI remains unresolved. COMMENT only—not approval, completion of human acceptance, or merge authorization.

[wesbillman]

Star Lord automated source re-review (via Wes’s account)

No new actionable source findings in the test-synchronization follow-up. Scope: the two-file change since previously reviewed e71fc9cd62703d64d3669f4501c62e2de7cca245; this does not reopen the attachment feature or the prior main-merge review.

• Head: b926f3e38c8e800e160bbb8bd9c9eb000cb6cec8
• Base: 1108ab64cf232acfc0c8087db54b74ce73098bfe

tests/browser/navigation.mjs:10–44 extracts the existing selection/animation-completion wait into selectPage, retaining the startup and palette-opening steps in openPage. The selector still targets the same Actions option, and selection still waits for the dialog to close. The wait matches the delayed animation on .search-palette-scroll in src/app/shell/SearchChoices.css:41–51 and also accepts the no-animation reduced-motion case.

Both changed calls in tests/browser/layout.spec.mjs:888–889,953–954 follow expectPageOrder, which leaves the palette open. They reuse that existing wait without reopening search. The Projects ordering, disabled/re-enabled plugin, geometry, overflow and focus assertions remain intact. No browser cases were added or removed, no assertions were weakened, and no production code changed in this follow-up. Existing helper imports/calls were checked across the pinned tests/browser/*.mjs sources.

Validation limits: source-only, using hash-verified pinned blobs and no dirty working-tree inputs. No local tests/builds, PR-code execution, installs or app launches. This source review does not establish that the WebKit failure is fixed in execution. The current-head hosted-check snapshot returned only a successful DCO Check; browser/required CI completion and before/after timing evidence are not established. Earlier-head/pre-commit results in the PR description are not validation of this head. COMMENT only—not approval, human acceptance or merge authorization.

[wesbillman]

Star Lord — automated source review via Wes’s account

No new actionable findings in this merge/integration follow-up since the previously reviewed b926f3e38c8e800e160bbb8bd9c9eb000cb6cec8.

• Head: 526519512368473a7d57c2ebe55fa5925c432920
• Base: 258c6d6b0b591e7ad478abea6c3cb255e9452ae6

The merge preserves the compact attachment grouping and its regression coverage while retaining main’s message-management menu wiring and sending/failed-message separator regression. I traced the shared menu owner and the thread/media-viewer edit scopes; no merge-specific mismatch found. The remaining standalone gallery test now uses watchPageErrors without adding an allowlist or removing its clipboard failure/success assertions. No browser cases were added or removed by this follow-up.

Validation limits: source-only; no PR code, tests, builds, or app workflows executed. One hosted CI snapshot showed security/DCO and browser measurements passing, with JavaScript, Rust/tool integration, and all six functional browser shards still running; Windows native validation was skipped. Hosted measurements ran the synthetic merge da1022526e49211e8c129f44f106c0051c10aeb3, not raw head: 7/7 passed, ~176.0 s wall / ~164.2 s summed execution; slowest test was Chromium cursor paging (~81.9 s), with scroll.spec.mjs ~123.2 s and channel-opening.spec.mjs ~41.0 s. No before/after timing comparison is available, and those measurements do not validate the gallery migration or focus behavior. Current-head browser/native behavior and human confirmation remain unverified by this review. This is a non-blocking comment, not approval or merge authorization.

[wesbillman]

Star Lord — automated source re-review via Wes’s account

No new code/integration defects found in the rebase follow-up. One public-material cleanup is requested below. Scope: integration since the previously reviewed 526519512368473a7d57c2ebe55fa5925c432920, plus the public publication surface—not a reopening of the approved compact-thumbnail design.

• Head: 334190c1cbd4f404cd7b16d7f714514417f6fb75
• Target base: 2dd479666ca5bd40166b7e3e79cf3fb0872baa28

Public-material finding — P2: remove internal coordination/deployment identifiers

This repository is public. The description’s final “Originating conversation” line publishes an internal conversation deep link, including its channel/message identifiers. In addition, all nine rebased commits (91aaa3da through 334190c1) retain an internal deployment hostname in the agent co-author trailer. These are public-metadata disclosures, not a claim that the link bypasses authorization or that credentials were exposed. I missed these in the earlier reviews; this is newly identified evidence, not a claim that the rebase introduced them.

Remove the internal conversation link from the public description, or replace it with a public issue/design reference. For commit metadata, use a verified, public-safe co-author identity agreed with the contributor while preserving actual authorship and valid DCO certifications; do not invent an address or drop legitimate attribution. I have intentionally not repeated the private identifiers here. No PR material or history was changed by this review. The four attached screenshots were inspected and show synthetic fixture content; I found no additional publication issue in those images.

Integration evidence

Eight of the eleven feature paths are byte-identical to the last reviewed head, including the thumbnail renderer, message grouping, cover-crop motion, and pixel-outline browser regression. Of the other three, the message-menu regression only moves within MessageRow.test.tsx; the additional reply-summary CSS comes from the target base; and navigation.mjs preserves the base’s connected-palette/keyboard selection logic while extracting selectPage. The connected: false callers still reach the placeholder palette, and both Projects callers select from the palette they already opened. No additional browser cases or removed assertions were introduced by this follow-up. Success/cancel and error/retry focus ownership were separately traced; I found no rebase-specific change to the modal or opener restoration owner.

Optional description maintenance: the verification section still calls 711987b0 “current” and says the PR stays draft, whereas this head is non-draft. Refresh the revision/readiness wording without promoting earlier pre-commit runs or unconfirmed human checks to current-head evidence.

Validation limits

Source-only, from 1,673 hash-verified archived source entries, unchanged after inspection; no live-checkout/dirty inputs. No PR code, tests, builds, installs, or app workflows executed by this reviewer.

One read-only snapshot of CI run 36497149199 showed JavaScript, browser measurements, security checks and DCO passing; Rust/tool integration and all six functional browser shards were still running, and Windows native validation was skipped. The inspected JavaScript/measurement logs checked out synthetic merge ec2a9ab5b513652a6ffcc1f22c6beb06f1375324, combining the pinned head and base—not raw head. JavaScript reported 5,051 tests / 417 files passed, 356.34 s wall and 623.59 s summed test execution; slowest file was unread-startup.test.ts (29.05 s), and slowest test was the 2,400-row send/acknowledgement case (13.74 s). These are hosted observations, not a controlled before/after benchmark. No CI polling or all-green claim.

Current-head browser/native behavior, rendered focus through failure/retry, and human confirmation of the focus repair remain unverified. This COMMENT review is not approval, completion of the human-testing checklist, or merge authorization.

[morgmart]

Carl, an automated reviewer, commenting via Morgan’s GitHub account.

At Morgan’s request, removed the internal conversation link from the PR description and refreshed the verification/readiness section. It now distinguishes final-head hosted CI and push-hook results from intermediate-head browser runs, records that the PR is not draft, and retains the unconfirmed human/native checks. Screenshot links are unchanged.

This addresses the description portions of the latest review only. Commit attribution/history and repository documentation were not changed; the co-author metadata concern remains open.