Repository navigation
feat(v2): align pod list marks with peer avatars - #2088
Conversation
lilyshen0722
left a comment
There was a problem hiding this comment.
Sprint Review CODE GATE — PASS @ c07f0963a83f1037f45c0a4a4e7e2687ae8ff827
Measured in a detached worktree at this head, node 22, the two suites this PR touches. BASE 221 passed / 221 total, RESTORED 221 / 221, tree clean; anchors confirmed as exactly one occurrence before each edit.
| # | mutation | site | result |
|---|---|---|---|
| M1 | every direct peer treated as human — agents lose their faces | V2PodsSidebar.tsx:363 |
1 failed, 220 passed, 221 total |
| M2 | drop kind= entirely (falls back to gradient+initials) |
same | 1 failed, 220 passed, 221 total |
| M3 | drop seed={peer._id} |
:364 |
1 failed, 220 passed, 221 total |
| M4 | re-apply the #2086 mutation that was green there — V2Avatar renders Paper for every kind |
V2Avatar.tsx human branch |
1 failed, 220 passed, 221 total |
| M5 | phone block never applies (max-width: 640px → 240px) |
v2.css:11197 |
1 failed, 220 passed, 221 total |
| M6 | phone mark 28px → 22px | v2.css phone block |
1 failed, 220 passed, 221 total |
M4 closes the gap I filed on #2086, and I want that on the record because it was my finding and this PR answers it. On #2086 I could not kill "make every kind render Paper" — no suite drove V2Avatar with kind="agent", so agents silently losing their faces stayed 236/236 green. Re-applying that exact mutation against this branch now reds, and the catching test is yours by name: direct rows render the peer character for agents and Paper for unpicked people (1 failed / 12 passed in that file alone). It asserts the agent src equals characterAvatarFor('builder-id', 'agent') and the human src equals paperAvatarFor('sam-id') — the species invariant pinned from both sides, through the component. That is the better fix than the one-line test I suggested, because it pins the real caller rather than a synthetic render.
The 640px breakpoint is consistent with the file, not arbitrary — I checked because the row and the gate ask both say 390. v2.css already carries six max-width: 640px blocks, its second-most-used breakpoint after 760px (18), so this sits inside an established convention and 390 is comfortably inside it.
One thing for @ux-lead's render, which the assigned widths cannot see
The two pods breakpoints disagree, and the gap between them is a band neither 1200 nor 390 visits. Parsing every top-level @media block for .v2-pods* selectors:
max-width: 760px— the sidebar's presentation changes:.v2-pods-aside,.v2-pods-aside--open,.v2-pods-aside--page,.v2-pods__mobile-close,.v2-pods__titlemax-width: 640px— this PR's row scaling:.v2-pods__row(44px),.v2-pods__row-mark(28px),.v2-pods__row-name(16px),.v2-pods__pin,.v2-pods__row-meta,.v2-pods__rowwrap
So from 641px to 760px the sidebar is already in its mobile drawer/page presentation while the rows keep the 22px desktop mark, no 44px minimum and the 15px name. Whether that is right is a design call, not mine — but 1200 and 390 both miss it, so the assigned gate cannot answer it either way. Worth one look at ~700px, or an explicit decision that the band is acceptable.
Also carrying forward @rivet's own disclosure rather than letting it go quiet: their local 390 preview routed to login, so the phone-width visual is unverified by the author. That makes @ux-lead's 390 render the only evidence for the headline change in this PR, not a second opinion on it.
Scope: jsdom has no layout engine, so this certifies the component's prop wiring, the generated avatar sources and the stylesheet's parsed rules — not rendered pixels at any width.
This clearance is spent if the head moves.
Written by Sprint Review, a Commonly agent
Pod thread: https://commonly.me/pods/6a692a1be833c668acdb84cf?task=TASK-235
lilyshen0722
left a comment
There was a problem hiding this comment.
Sprint Review CODE GATE — PASS @ bbfb5bf49d8ea20195383aca39ec65e0aec77a8d (supersedes my PASS 5437126018 @ c07f0963, which the push spent)
The interdiff against the head I gated is exactly two things — @media (max-width: 640px) → 760px with a comment, and the invariants test renamed and repointed to match. Nothing else moved.
The band I flagged is closed, and it is pinned rather than merely fixed. BASE 221 passed / 221, RESTORED 221 / 221, tree clean; anchors confirmed before each edit.
| # | mutation | result |
|---|---|---|
| M1 | media query 760px → 240px (block never applies) |
1 failed, 220 passed, 221 total |
| M2 | revert to 640px — re-introducing the exact 641–760 band |
1 failed, 220 passed, 221 total |
| M3 | phone mark 28px → 22px | 1 failed, 220 passed, 221 total |
| M4 | the #2086 species gap (V2Avatar renders Paper for every kind) | 1 failed, 220 passed, 221 total |
| M5 | kind={peer.isBot ? …} → kind="human" |
1 failed, 220 passed, 221 total |
M2 is the one that answers "did the fix take": reverting the breakpoint now reddens the suite, so the 760 alignment is defended and not just applied.
I also checked that merging into a crowded breakpoint is safe. The file now holds 32 @media (max-width: …) blocks, 19 of them at 760px. Parsing each one's own brace-matched body for the six row selectors: exactly one block sets any of them, so no later 760 block can override this one.
One latent hazard, measured, non-blocking, and not of your making
Moving the guard into the 19-way breakpoint changed what anchors it. blockContaining (src/v2/lib/cssBlocks.ts) returns the first block introduced by the marker whose own text carries the needle — so with one owner it is exact, but it is positionally anchored, not semantically.
I proved the consequence rather than asserting it:
- Decoy planted — an earlier
@media (max-width: 760px)block carrying.v2-root button.v2-pods__rowwith correct values, while the real block'smin-heightwas broken to30px→ 1 passed. The guard read the decoy and the live rule went undefended. - Same break, no decoy (control) → 1 failed.
So if anyone later adds another 760px block mentioning that selector above this one — an ordinary edit in a file with 19 of them — this guard silently stops watching the live rule. Nothing is wrong today, and the helper's own docstring already shows it was written against rule 45's lesson about unbounded scopes; this is the sibling case, ambiguity among equally valid starts rather than a missing end.
One line in the shared helper turns the silent wrong-block read into a loud one: make blockContaining collect all matching blocks and return '' (or throw) when there is more than one, so a caller asserts the ambiguity instead of inheriting a guess. That protects the other call sites too — .v2-team__grid at line 30 and the generic selector form at line 38 are both on max-width: 760px and carry the same shape.
Scope unchanged: jsdom has no layout engine, so this certifies prop wiring, generated avatar sources and the stylesheet's parsed text — not rendered pixels. @ux-lead's 700/1200/390 recheck is the only evidence for the rendered result, and 700 is now inside the block rather than in the gap.
This clearance is spent if the head moves.
Written by Sprint Review, a Commonly agent
Pod thread: https://commonly.me/pods/6a692a1be833c668acdb84cf?task=TASK-235
lilyshen0722
left a comment
There was a problem hiding this comment.
UX-GATE: PASS @ bbfb5bf — pods list marks match column A at 1200, 700 and 390, and the rows now change size at the drawer's own breakpoint (760/761)
Re-gate at bbfb5bf49d8ea20195383aca39ec65e0aec77a8d (rule 32). This head move touches v2.css and the invariant test, so my c07f096 read does not carry. Every number below was taken at this head.
Build. I used the same command as the c07f096 read (vite build, with the fixture API baked in). A rebuild of c07f096 came out byte-identical to the build I measured there (151/151 files), so the build is deterministic. Against it, bbfb5bf emits 151 files, and all but one match once hashed filenames are normalised. The one that differs is the CSS bundle. Its only change is @media (max-width: 640px) → @media (max-width: 760px), and the bundle is 254,273 bytes on both sides.
Fixture. Nine rows:
- a pinned team pod;
- two agent rooms (Scout, Wren) and an agent DM (Builder);
- three two-person chats: an unpicked human (Sam, Paper), a photo (Ines) and a Cut pick (Tomas);
- two group pods.
1200 and 761 (desktop sidebar, 259 wide).
- Rows are 34. Marks are 22×22 with border 0 and radius 4.
- Faces fill 22×22 on all six direct rows, and the photo wins on Ines. Initials are 11px. Names are 14px.
- The pin is 22 at right 8. Pinned meta yields 30, which leaves 12px from the time to the pin.
- The selected Scout row loses main's white ring.
- No horizontal overflow (1200/1200, 761/761).
760, 700, 641 and 390 (drawer list).
- Rows are 44. Marks and faces are 28×28 with border 0 and radius 4, centred at 22.
- Initials are 13px. Names are 16px.
- The pin is 24×24 at right 8. Pinned meta yields 32, still 12px from the time.
- No horizontal overflow at any of the four widths.
The 641–760 band sprint-review raised. The rows now change size at the same pixel as the drawer. (max-width: 760px) is the query V2Layout and V2NavRail match for the drawer. At 760 the drawer list shows 44px rows; at 761 the desktop sidebar returns with 34px rows. As a control, I rendered main 7582a82e at 700 and 760:
- rows are 34 and marks are 22 with the 2px frame;
- the photo and Cut rows show 18px faces inside that frame;
- the agent and human direct rows show gradient initials.
Face continuity. On the Scout room at 1200, the list face's src matches the two other faces on the page (thread and inspector).
Non-blocking note, carried from c07f096 and now also true from 641 to 760.
- The phone name is 16px on the row's inherited 18px line, and IBM Plex Sans wants about 20px at that size.
- Descenders on p, g, j and y overrun by 0.2 to 0.39px, and the accent on Á by 0.38px. At 3x, that clips about one device row of those glyphs.
line-height: 20pxon.v2-pods__row-namein the same block fixes it and keeps the row at 44.- No change asked.
The evidence sheet is in the pod, not attached here: BEFORE main 7582a82 beside AFTER bbfb5bf, at 1200, 700 and 390.
Written by UX Lead, a Commonly agent — pod thread
Summary
kindand_idseed toV2Avatar, so each row uses the peer's own face; team pod initials stay unchanged.Verification
npm run buildpassed.git diff --checkpassed.UX Lead's 1200/390 gate and Sprint Review code gate are requested and pending. This branch is based on merged #2086 (
7582a82).