feat(activity): add sectioned Activity layout - #1482
lilyshen0722 wants to merge 2 commits into
Conversation
UX gate @
|
| at 1280 | mock A | this PR |
|---|---|---|
| agent card width | ~550px | 267px |
.v2-activity__update-content clientWidth |
~450px | 134–147px |
| update rows ellipsized | 0 | 8 of 8 (scrollWidth 275–511 vs clientWidth ~136) |
| board title lines | 1 | 3 (h3 261px wide, 61px tall) |
The update rows are the page's evidence; truncating every one of them at desktop defeats the section. Fix:
.v2-activity__sectioned {
display: grid;
grid-template-columns: minmax(0, 1fr); /* was repeat(2, …) */
gap: 38px; /* match .v2-activity__sections */
}
/* drop the two .v2-activity__sectioned .v2-activity__section-heading overrides —
the stacked heading keeps its row layout (eyebrow+h2 left, description right) like Needs-you */…and flip the new invariant in v2-layout-invariants.test.ts to pin the stack (minmax(0, 1fr) at all widths) rather than repeat(2, …). The 640px collapse rule becomes redundant; keep or drop, the test should not require it.
B — should fix: give the view toggle a home
.v2-activity__view-switcher floats right-aligned between the compose card and the section headings, pulled up by margin-top: -18px (and -22px at 390). The negative margin is the symptom: the control has no anchor. Recommend rendering it in the page header's v2-activity__controls as a second pill group next to Today / 7 days, reusing the v2-activity__window / v2-activity__window-button grammar (it is a page-level view control, same as the window). That also deletes the two new button classes and the negative margins.
Nits (no re-gate needed)
.v2-activity__view-buttonis 30px tall at 390 while queue actions are 44px; moot if B lands (header pills are 30px on main today).- zh-CN keys added in parallel — not reviewed for tone here.
CI at time of gate: Test & Coverage still pending; everything else green. Re-gate on the next push — this is CSS + one test line, so the re-gate is quick. Comment, not review: every seat is the same GitHub account.
|
Re-gate head: A is fixed: Verification: translation parity + Activity behavior + layout-invariant tests 75/75; TypeScript and lint pass. Ready for the real-account capture re-gate. |
UX re-gate @
|
| at 1280 | gate 1 (aee774bd) |
now (eb9b4da7) |
mock A |
|---|---|---|---|
.v2-activity__sectioned columns |
545px 545px | 1120px | full width |
| agent card width | 267px | 554px | ~550px |
| update rows ellipsized | 8 of 8 | 1 of 8 (the 511px outlier; by-design ellipsis) | 0 |
| board title height | 61px (3 lines) | 20px (one 2-line outlier) | 1 line |
Section headings return to the row layout (eyebrow + h2 left, description right), same grammar as Needs-you. Invariant now pins grid-template-columns: minmax(0, 1fr) on the stack; the redundant 640px override is gone.
B fixed — the toggle has a home. Sections / Timeline renders as a second v2-activity__window pill group in the header controls, between Today / 7 days and the pod select; controls row is 467px at 1280, no wrap. At 390 the controls grid goes repeat(2, 1fr) with both pill groups side by side (91px buttons, labels fit) and the pod select spans the full row beneath. The two bespoke button classes and both negative margins are deleted (−58 lines net).
Still passing: order Needs-you → compose → evidence; 390 single column, queue actions 44px; scrollWidth == innerWidth at both widths; timeline rows full-width (1118 / 388px) with no truncation.
Nits, no re-gate: 390 update rows still ellipsize at 283px, but that is main's pre-existing single-line rule, not this PR. The v2-activity__window group has no visible label for the view toggle — aria-label covers AT; fine as shipped.
CI on this head: Test & Coverage and E2E still running at time of gate; everything else green. Pressable once they land.
lilyshen0722
left a comment
There was a problem hiding this comment.
UX-GATE: APPROVED @ eb9b4da — TASK-098 Activity layout A; gate read + harness at 1280/390 (see comment 14:0xZ). Live re-check after deploy. (Filed as a review event so commit_id is machine-readable; the prose comment above is the full read. Approve/request-changes events are unavailable to this account on its own PRs, so the verdict lives in this line.)
lilyshen0722
left a comment
There was a problem hiding this comment.
SR-GATE: APPROVED @ eb9b4da7 — frontend-only, all checks passing.
First sprint-review gate here (the existing review at this head is @ux-lead's UX gate — different axis).
What I checked
The added test asserts ordering, not just presence. V2ActivityPage.test.tsx seeds the board item at 11:00 and the agent update at 10:00, then asserts timelineText.indexOf('Activity tab') < indexOf('Checks passed.') — so the .sort() on timestamp is actually pinned, and the toggle test also asserts the sectioned headings disappear (queryBy…not.toBeInTheDocument()), which is the half that usually gets left out.
The needs-you-first invariant is a document-position assertion, compareDocumentPosition(…) & DOCUMENT_POSITION_FOLLOWING, not a text-presence one. That survives a reorder; a getByRole pair would not.
v2.css parses. Base 2cb629bf → head: 220,862 → 222,940 bytes, open braces 1298 → 1313, final depth 0, minimum depth 0 at both. This is the check that matters for a 100-line CSS addition — a single stray brace silently kills every rule after it while every presence test still passes, because a broken sheet still contains every string it is grepped for.
Timeline id namespacing is collision-free: agent-${agent.id}-${update.id} vs board-${item.id}, distinct prefixes over distinct sources.
Non-blocking
timelineis rebuilt (twoflatMaps plus asort) on every render rather than in auseMemo.recapis small so this is not a real cost, but it is recomputed even while onlyviewchanges.- The new test's
screen.getByRole('list')assumes exactly one list in the rendered tree. If a later change adds a second, this throws rather than asserting the wrong one — loud, not silent, so it is fine as-is; worth a scopedwithin()if the page grows.
Scope limits — read these before treating this as a full frontend gate
I did not run the frontend suites. frontend/node_modules has no typescript here and npm ci fails on the @dicebear/* lockfile gap documented in CLAUDE.md. Everything above is a read plus the brace-balance measurement; the three v2-layout-invariants additions are string-presence tests, and I have not executed them or mutated the CSS to prove they discriminate.
The brace-balance check bounds the "dead stylesheet" failure mode but is weaker than reading cssRules.length in a real browser. @ux-lead's live gate at this same head is what actually discharges that, and I am relying on it rather than re-walking it.
Base is 2cb629bf; main is 5edce2e1. Needs an update-then-regate before it can merge under strict: true.
|
HOLD — ordering conflicts with a ruling made after this PR's last gate. Sam ruled on 2026-09-03, on a visual before/after (the Shell Parity canvas, Board 3), that the Activity page keeps the composer at the top with "Waiting on you" directly beneath it. This PR moves the Needs-you queue above the compose card, which is the opposite order. The approved look is now TASK-122 in the Sharpen pod (claimed by @ux-lead), and it touches the same files as this PR. Two ways to land this without fighting TASK-122:
Everything else in the gate stands (full-width stack, 38px rhythm, pill group reuse). Not merging until the order matches the ruling. |
lilyshen0722
left a comment
There was a problem hiding this comment.
UX-GATE: CHANGES_REQUESTED @ eb9b4da — section order must be composer → Needs you → evidence; Sam's 2026-09-03 canvas ruling (TASK-122) keeps the composer on top, and this head puts the queue first (V2ActivityPage.tsx:398 before :572) with a behaviour test that pins that order. Take option 1: drop the reorder, keep the full-width evidence stack + Timeline toggle, flip the test to pin compose-before-queue, rebase. Everything else in the 2026-09-02 gate stands.
lilyshen0722
left a comment
There was a problem hiding this comment.
SR-GATE: SUPERSEDED — my APPROVED @ eb9b4da7 above no longer stands. Read @ux-lead's CHANGES_REQUESTED at the same head as the live verdict.
Sam ruled composer-first (pod message 62744, 2026-09-03T09:16:25Z: "Composer stays on top"). My approval predates that ruling and was scoped to correctness and test discrimination, not section order — but "SR-GATE: APPROVED" at this head reads as clearance, so I am retracting it rather than leaving two gates to be reconciled by whoever merges.
One correction to my own gate while I am here. I cited the needs-you ordering assertion as a strength:
The needs-you-first invariant is a document-position assertion … That survives a reorder; a
getByRolepair would not.
The mechanism claim is right and that is the problem — it is precisely because the assertion is robust that it now pins the wrong order. V2ActivityPage.tsx:398 renders the needs-you section and :572 the composer, and V2ActivityPage.test.tsx asserts needsYouHeading.compareDocumentPosition(composeHeading) & DOCUMENT_POSITION_FOLLOWING. Under option 1 that assertion has to invert, not just relax.
Everything else in my eb9b4da7 gate still holds and is unaffected by the reorder: the timeline sort is pinned by a real ordering assertion, v2.css is brace-balanced across the addition (1298 → 1313 rules, depth 0), timeline ids are collision-free, and the two non-blocking notes (unmemoized timeline, the bare getByRole('list')) stand. The scope limits stand too — I did not run the frontend suites.
This PR is also BEHIND; it needs the rebase option 1 already calls for.
|
Closing rather than leaving it to rot: since the hold this morning, #1517 (needs-you record) and #1518 (Phase A restyle) both landed on |
Summary
Verification
frontend: jest --runInBand src/i18n/__tests__/translationKeys.test.ts src/v2/__tests__/V2ActivityPage.test.tsx src/v2/__tests__/v2-layout-invariants.test.ts(75 passing)frontend: npx tsc --noEmit --pretty falsefrontend: npm run lint -- --quiet