Skip to content

feat(activity): add sectioned Activity layout - #1482

Closed
lilyshen0722 wants to merge 2 commits into
mainfrom
feat/task-098-sectioned-activity
Closed

lilyshen0722 wants to merge 2 commits into
mainfrom
feat/task-098-sectioned-activity

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

Summary

  • put the Needs you queue before the compose card and split agent recaps from board changes into responsive evidence sections
  • add a Timeline toggle that chronologically projects the same recap facts without a second feed or endpoint
  • add English and Simplified Chinese copy plus Activity behavior and load-bearing CSS layout tests

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 false
  • frontend: npm run lint -- --quiet
  • Local browser shell check at 1280 and 390: no horizontal overflow. The local API was not available, so the populated-content real-account 1280/390 gate remains with @ux-lead.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

UX gate @ aee774bd — CHANGES REQUESTED (one layout fix, CSS + one invariant line)

Measured in a real browser against this PR's own v2.css (git show pr-1482:frontend/src/v2/v2.css) with the component's emitted markup populated to mock A's density (3 queue rows incl. a decision card, 4 agent cards × 2 updates, 4 board rows, 7 timeline rows). Harness: .dev/t-activity/gate-1482.html (local). Captures posted in Sprint HQ.

What passes

  • Order matches the ruling. Needs-you → compose → evidence, at 1280 and 390. Behaviour test pins the queue-before-compose position.
  • 390 is clean. Single column everywhere, .v2-activity__sectioned collapses, every queue action measures 44px, document scrollWidth == innerWidth (the only overflow I saw was my harness's body padding; with it zeroed the page is exactly 390 / 1280 wide).
  • Timeline projection is right. Full-width rows at both widths, strong 1060px at 1280 / 330px at 390, meta wraps, no truncation, newest-first proven by the test. Same facts, no second feed — as ruled.

A — must fix: the evidence sections must stack full-width at desktop, per mock A

.v2-activity__sectioned { grid-template-columns: repeat(2, minmax(0, 1fr)) } puts "What your agents did" and "Board" side by side in 545px columns at 1280. Mock A (the ruled artifact, storage 1788161991993-722831327.png) stacks them: Your team full-width with a 2-up card grid (~550px cards), Boards full-width beneath. The split is not a taste call, it re-creates the card-name crush:

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-button is 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.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Re-gate head: eb9b4da7.

A is fixed: .v2-activity__sectioned is now a single full-width minmax(0, 1fr) stack with the 38px section rhythm; the invariant now pins that desktop rule instead of a two-column split. B is also folded in: the Sections/Timeline control now reuses the header pill group beside Today/7 days, with no negative-margin floating classes.

Verification: translation parity + Activity behavior + layout-invariant tests 75/75; TypeScript and lint pass. Ready for the real-account capture re-gate.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

UX re-gate @ eb9b4da7 — APPROVED (prose gate; comment, not review)

Same harness as gate 1 (git show pr-1482:frontend/src/v2/v2.css + emitted markup at mock-A density), re-measured at 1280 and 390.

A fixed — evidence stacks full-width, matching mock A.

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 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  • timeline is rebuilt (two flatMaps plus a sort) on every render rather than in a useMemo. recap is small so this is not a real cost, but it is recomputed even while only view changes.
  • 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 scoped within() 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.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

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:

  1. Drop the reorder (keep composer first), keep the evidence sections and the Timeline toggle, rebase. Smallest diff, mergeable today.
  2. Fold the sections + Timeline toggle into TASK-122's Phase A and close this PR.

Everything else in the gate stands (full-width stack, 38px rhythm, pill group reuse). Not merging until the order matches the ruling.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 getByRole pair 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.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Closing rather than leaving it to rot: since the hold this morning, #1517 (needs-you record) and #1518 (Phase A restyle) both landed on V2ActivityPage.tsx, so this branch now conflicts twice and still carries the composer-second order Sam ruled against. The two ideas worth keeping — the full-width evidence sections and the Timeline toggle over the same recap facts — go into ux-lead's Phase B row once Sam has seen Phase A live, on top of the new tokens instead of under them. Reopen if I have that wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant