fix(channels): prevent clipped activity rows and remove separators - #377
Conversation
Match activity row corners to the popover inset and remove the dividers between rows. Cover real browser geometry and thread navigation for single and multiple unread threads. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
d84986c to
b5d3163
Compare
Close the activity popover before changing viewport geometry, then reopen through the real channel trigger. Keep scrolling and keyboard navigation in the short viewport and assert that the multi-row popup actually overflows. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord — automated source review via Wes’s account
Reviewed head 190807d71ac44a4d40427aec80c6e9d34ac89e6a against base 23929d764697d1747d0ba711ad47598d172cd07e. This is a non-blocking COMMENT, not approval or merge authorization.
Finding
P2 — Remove internal deployment/agent identifiers from the public commit metadata. Both PR commits (b5d3163e34b8e2bd3a244dd0c4c6efe73b174bdd and 190807d71ac44a4d40427aec80c6e9d34ac89e6a) include a Co-authored-by address containing an internal coordination hostname and a stable agent identifier. In this public repository, that publishes an unnecessary association between the deployment and the agent identity. This is a public-material privacy finding, not a secret-key leak or a CSS defect. Please have the author sanitize those addresses using an owner-verified, public-safe attribution identity while preserving actual contributor credit and valid DCO certification. Do not invent a replacement identity or substitute someone else’s authorship. The sensitive strings are intentionally not repeated here.
Code assessment
No actionable production-code defect found in the two-file diff. Channels.module.css:628–635 scopes the radius override to the activity list and removes only its adjacent-row divider. The formula matches the current shared frame’s panel radius, list inset and 1px border (styles/materials.css:67–71, styles/components.css:916–950); NavigationItem consumes the inherited radius. No shared token, spacing, unread acknowledgement, or navigation owner is changed.
I traced the hover/close lifecycle, stale/profile-failure presentation, thread-opening handoff, thread error/retry rendering and close-focus restoration. The CSS introduces no new focus transition. Source inspection does not establish runtime focus correctness; the added test checks programmatic row focus plus Enter, not a complete keyboard-entry/error-recovery journey.
The two new cases per engine are appropriately browser-level geometry coverage, reuse minimal upstream fixture data, and resize only while the hover popup is closed. No cases are removed. Screenshots are captured artifacts, not automated pixel comparisons. Engineering assessment: 9/10 minimalness and elegance; 9/10 source-level code correctness; 8/10 overall until the concrete publication finding above is addressed.
Evidence and limits
- Inspected the full PR description, all four attached before/after images and both commit messages. No additional disclosure found in those images, the description or changed source. The images are explicitly attributed to older snapshots, not runtime evidence for this head.
- Hosted CI run 36496893087 identifies this head/base and reports success, including all six Chromium/WebKit shards, JavaScript, Rust/tool integration and measurements. DCO/security checks also report success; Windows native validation is skipped. This snapshot supersedes the description’s pending-CI wording, but is not my execution evidence.
- The description reports a current-head 12/12 repeated local browser run in 35.8s, with 4.0–5.1s cases; I did not reproduce or independently measure those timings. Native/live-relay acceptance and explicit final human acceptance remain unverified; the repository readiness checklist is not established by this review.
- Source-only: no PR code, tests, builds or apps executed; no installs or source changes. Reviewed an isolated archive whose 1,646 Git blobs match the pinned head and original content manifest, not a modified live checkout.
…t-update-drafts * commit '0a4982797f38164d75e3e8f48e58fabb9dd59e66': (66 commits) Show saved local and relay inventory while retaining existing import controls (#286) feat(channels): edit channel details with confirmed saves (#369) test(channels): discover the hoverable width for activity corners (#416) Fix flaky WebKit menu focus browser test (#409) Test Goose connections and fix Pi test false failures (#383) feat: open threads with verified newest-first windows (#154) Add agent conversation context selection (#382) test: keep behavioral coverage without cosmetic matrices (#410) Fix reading position and composer caret on channel return (#411) fix(channels): prevent clipped activity rows and remove separators (#377) ci: publish signed macOS updater artifacts in prereleases (#387) feat(messages): add jump to latest controls (#374) Align reply summaries with message content (#408) Add centered thinking pills to agent avatars (#351) Keep focus where the user moved it when a menu finishes closing (#355) Browse legacy identities without a destination and review text before cloning (#285) Show separate identity cards and prevent duplicate imports (#225) Polish message and thread spacing, grouping, and typography (#364) Remove the Away avatar badge stroke (#395) fix(profiles): hide activity on human profiles (#391) ... Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz>
* origin/main: (25 commits) Show saved local and relay inventory while retaining existing import controls (#286) feat(channels): edit channel details with confirmed saves (#369) test(channels): discover the hoverable width for activity corners (#416) Fix flaky WebKit menu focus browser test (#409) Test Goose connections and fix Pi test false failures (#383) feat: open threads with verified newest-first windows (#154) Add agent conversation context selection (#382) test: keep behavioral coverage without cosmetic matrices (#410) Fix reading position and composer caret on channel return (#411) fix(channels): prevent clipped activity rows and remove separators (#377) ci: publish signed macOS updater artifacts in prereleases (#387) feat(messages): add jump to latest controls (#374) Align reply summaries with message content (#408) Add centered thinking pills to agent avatars (#351) Keep focus where the user moved it when a menu finishes closing (#355) Browse legacy identities without a destination and review text before cloning (#285) Show separate identity cards and prevent duplicate imports (#225) Polish message and thread spacing, grouping, and typography (#364) Remove the Away avatar badge stroke (#395) fix(profiles): hide activity on human profiles (#391) ... Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/bundled/agents/AgentsPage.test.tsx # src/bundled/agents/AgentsPage.tsx
Overview
Category: fix
User Impact: Channel activity previews have smooth, unclipped hover backgrounds without dividers between threads.
Problem: The channel hover popover clips its activity rows' rounded corners, and separators between multiple rows make the hovered background look disconnected.
Solution: Match the rows to the popover's inner curve and remove the local dividers. The outer frame, spacing, shared design tokens, scrolling, and thread-opening behavior stay unchanged.
Changes
File changes
src/bundled/channels/Channels.module.css
Derive the activity-list row radius from the existing panel radius minus its inset and border, and remove the adjacent-row border rule. This stays local to channel activity previews.
tests/browser/channel-activity-corners.spec.mjs
Add single- and multiple-row journeys through the actual app and production broker, using the existing isolated test relay. Check painted browser geometry, divider absence, light/dark modes, enlarged text, constrained viewports, and keyboard thread opening.
Current validation at
190807d7:bin/pnpm test:browser tests/browser/channel-activity-corners.spec.mjs --no-deps --project chromium --project webkit --repeat-each=3: 12/12 passed, two workers, 35.8s total on Apple Silicon macOS. Individual cases took 4.0–5.1s. This is repeated local evidence, not Linux CI confirmation.b5d3163epassed JavaScript (including the merged test(relay): stabilize per-channel replay boundary coverage #378 replay-test fix), Rust/tool integration, measurements, and four browser shards. Only this new hover regression failed on the other two shards. Fresh hosted CI for190807d7is pending.23929d76; the CSS feature patch and the original screenshots below are unchanged. Screenshots retain their original capture attribution.Validation at
d84986c:5ce7836bwithout conflicts.bin/pnpm test:browser channel-activity-corners.spec.mjs --project chromium --project webkit --no-deps --workers=2: 4 passed (9.7s locally).Reproduction Steps
Screenshots/Demos
Real production-built app in dark mode with neutral, isolated test data; cropped to the activity popover. Before uses the base CSS at
5ce7836bin the otherwise identical rebased app; after isd84986c. No UI replica or component showcase.Implementation and validation performed by Carl, an AI agent, under the author's direction.