Repository navigation
docs(spec): ChatLayoutScrollButton records its shipped contract as current - #9124
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:spec-owner-review
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
The record matches the shipped component, and making it current now waits on an engineering owner's decision: an engineering owner records approval by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. A few fixes to the record are needed either way:
- FR6a says the target MUST expose its visible and expanded states while OQ1 still asks whether it should; keep it as a gap that points to OQ1, or rule OQ1 first.
- FR7's basis says "Current source and tests", but no test fails if the reduced-motion branch is removed; list FR1a, FR6a and FR7 under "Read from source, with no focused test yet", as the description says.
- DEC-1 still narrates the repair and keeps multi-line rejected alternatives; state where the target sits in the present tense, with one-line rejections.
- The readiness row says seven open questions (there are eight), and the FR1a, FR3a and FR6a rows keep a leftover fourth cell.
[Automated review]
| | FR4 | A **non-empty** `label` MUST render as visible text; omitted or empty renders icon-only with no visible text. | Current source and tests | | ||
| | FR5 | The pill MUST take its resolved height from the element-size token so no theme scale makes it clip the Button it wraps. | Current source and tests | | ||
| | FR6 | The public theming target MUST sit on the pill — the element painting fill, elevation, and radius — not on the row that centres it. | `architecture:component-theming-surface/INV4` | | ||
| | FR6a | The target MUST expose the state axes that drive its painted styles. Implementation gap: it reflects none (Current-state impact). | `architecture:component-theming-surface/INV6` | **violation of INV6** | |
There was a problem hiding this comment.
FR6a states the target MUST expose its visible/hidden and collapsed/expanded state axes, but OQ1 (line 291) still asks whether it should, and the FR6a verification row says no test may assert the shape before the owner decides. INV6 of the theming-surface architecture lists variant, size, selection, disabled and interaction state, not these two. Keep FR6a as a recorded gap pointing to OQ1 without a MUST, or rule OQ1 first.
| | FR5 | The pill MUST take its resolved height from the element-size token so no theme scale makes it clip the Button it wraps. | Current source and tests | | ||
| | FR6 | The public theming target MUST sit on the pill — the element painting fill, elevation, and radius — not on the row that centres it. | `architecture:component-theming-surface/INV4` | | ||
| | FR6a | The target MUST expose the state axes that drive its painted styles. Implementation gap: it reflects none (Current-state impact). | `architecture:component-theming-surface/INV6` | **violation of INV6** | | ||
| | FR7 | The transition between states MUST collapse to `0s` under `prefers-reduced-motion: reduce`. | Current source and tests | |
There was a problem hiding this comment.
The basis says "Current source and tests", but no test fails if the reduced-motion branch is removed: holdMotionStill forces transition-duration: 0s !important on every element, and neither Chromium spec asserts the duration. Use "Current source" here and move FR7 (with FR1a and FR6a) to a "Read from source, with no focused test yet" list.
| | Behavior | A pill holding one ghost Button. Hidden state paints nothing and leaves the tab order; a non-empty `label` expands the pill and becomes the visible text; omitted or empty renders icon-only. | | ||
| | End-user impact | Readers who scrolled up get one control back to the newest message, which is hidden and out of the tab order when they are already there; a theme that restyles its target repaints the pill they see. | | ||
| | Builder impact | None. | | ||
| | Compatibility/readiness | Records shipped behavior; no public change. Three implementation gaps (FR1a, FR3a, FR6a) are listed under Current-state impact; seven owner questions stay open. | |
There was a problem hiding this comment.
The record lists eight open questions (OQ1–OQ8), not seven.
| | ID | Invariant | Basis | | ||
| | ---- | ------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------ | | ||
| | FR1 | The control MUST expose a non-empty accessible name in every reachable state. | WCAG 2.2 SC 4.1.2 | | ||
| | FR1a | `label` omitted uses the translated default; a non-empty `label` is the name. Implementation gap: `label=''` satisfies neither (Current-state impact). | Current source | **violation of FR1** | |
There was a problem hiding this comment.
This row (and FR3a on line 125 and FR6a on line 129) keeps a fourth cell under the three-column header; GitHub drops it when rendering. The "Implementation gap" text already carries it, so the cell can go.
… claims, and states DEC-1 in the present
There was a problem hiding this comment.
The record now matches the shipped component and the earlier review points are addressed, apart from one leftover row; making it current waits on an engineering owner's decision, recorded by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner). Since approval binds the exact spec text, two small record fixes are worth making first:
- The FR7 verification-map row still credits
holdMotionStillcapture conditions, which zero every transition and cannot fail; drop the row (the source-only list already covers FR7) or reword it like the FR1a row. - The FR3a paragraph keeps audit measurements (pixel sizes and frame comparisons); keep the durable fact that the button fills the clipping pill and leave the numbers to the test receipts.
[Automated review]
| | FR6 | `ChatLayoutScrollButtonSurface.a11y.chromium.spec.ts` placement and probe-theme reach tests | visible, collapsed, neutral and probe | moving the target back to the centring row leaves the pill unchanged under the probe theme | `audit:ChatLayoutScrollButton/theming` | | ||
| | FR6a | source: `themeProps()` is called with no state argument, and `Chat.doc.mjs` declares the target with no `visualProps`/`states` | visible/hidden; collapsed/expanded | adding either axis is new public theming surface, so no test may assert its shape before the owner decides | `audit:ChatLayoutScrollButton/theming` | | ||
| | hover, pressed | `ChatLayoutScrollButtonSurface.a11y.chromium.spec.ts` interaction pass — each frame asserts the pseudo-class matched and that the captured bytes differ from a rest frame of the same page | collapsed and labelled, light and dark | a frame captured after release, or with the pointer elsewhere, fails its own engagement assertion | `audit:ChatLayoutScrollButton/design` | | ||
| | FR7 | source declaration plus `holdMotionStill` capture conditions | any transition | removing the reduced-motion branch restores animation for readers who asked for none | `audit:ChatLayoutScrollButton/a11y` | |
There was a problem hiding this comment.
This row still credits holdMotionStill capture conditions, but that helper forces transition-duration: 0s !important on every element, so nothing fails if the reduced-motion branch is removed, and the list at line 262 already says FR7 has no focused test. Drop the row, or reword it as the FR1a row does ("No focused test; read from source").
…FR3a measurements
There was a problem hiding this comment.
The earlier review points are all addressed; making this record current now waits on an engineering owner's decision, recorded by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner). Since approval binds the exact text, two small wording fixes are worth making first:
- FR8 and AR3 (and the label-omitted state row) say the collapsed control "MUST fit a 24x24 CSS-px square", which reads as a maximum; WCAG 2.2 SC 2.5.8 and the surface spec require at least 24 by 24, so "at least 24 by 24 CSS pixels", as the Avatar records say, matches the intent.
- The
onClickconcept says the callback "receives no event", but Button calls it with the click event; saying its type declares no arguments keeps the record accurate.
[Automated review]
| | FR6 | The public theming target MUST sit on the pill — the element painting fill, elevation, and radius — not on the row that centres it. | `architecture:component-theming-surface/INV4` | | ||
| | FR6a | The target reflects neither the visible nor the expanded state that drives its paint. Implementation gap: a theme cannot address either state; whether the target should reflect them is OQ1 (Current-state impact). | Current source | | ||
| | FR7 | The transition between states MUST collapse to `0s` under `prefers-reduced-motion: reduce`. | Current source | | ||
| | FR8 | The collapsed control MUST fit a 24x24 CSS-px square. | WCAG 2.2 SC 2.5.8 (AA) | |
There was a problem hiding this comment.
"MUST fit a 24x24 CSS-px square" reads as a maximum, but WCAG 2.2 SC 2.5.8 is a minimum and the surface spec asserts width and height >= 24. Consider "MUST be at least 24 by 24 CSS pixels" (the Avatar records' wording), here, in AR3 and in the label-omitted state row.
…m and onClick's declared type
There was a problem hiding this comment.
This record now matches the shipped component, and every earlier review point is addressed; making it current waits only on an engineering owner's decision. An engineering owner records approval by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. No further record changes are needed.
[Automated review]
|
/approve-spec 14e1ba0 |
github-actions
Bot
removed
the
needs:spec-owner-review
Exact decision needed
Promote
component:ChatLayoutScrollButtonfrom draft to current. No behavior, prop, default, target, or export changes.Existing authority searched
packages/core/src/Chat/ChatLayoutScrollButton.spec.mdand the component's source, tests, and consumer docs on main.spec:AST-029owns the audit evidence inventory the verification map follows; the record's other cited owners are unchanged.Decision
component:ChatLayoutScrollButton,authority: current, approved by cixzhang. Draft and audit wording is removed, basis cells read from current source and tests, and the verification map credits only tests that fail when a claim breaks, with a "Read from source, with no focused test yet" list for the rest. Three implementation gaps, each with a matching Current-state impact entry:label=''yields neither the default nor a caller name (FR1a), the focus ring is clipped away (FR3a, WCAG 2.2 SC 2.4.7), and the target reflects none of its style-driving states (FR6a). OQ1–OQ8 stay open.Rejected alternatives
None.
Claim scope and non-goals
Claims are scoped to what main's source and tests show.
Evidence and implementation relationship
The component's colocated tests, as listed in the verification map.
Writing rubric results
Scope
Validation
pnpm check:knowledge: alignedgit diff --check: clean