Repository navigation
docs(spec): AlertDialog records its shipped contract as current - #9305
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.
This review is on hold for an owner decision. Promoting component:AlertDialog to current makes FR1–FR6, FR9, FR7's inert dismissal fade, AR1 and DEC-1–DEC-3 governing, and no current record states them yet. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, three record fixes are needed:
- The readiness row says AR1 ships, but Dialog does not yet contain Tab, so add AR1's Tab wrap as a gap under Current-state impact.
- AR1's verification row names two tests that are not in
AlertDialog.test.tsxand calls the accessibility contract pending; point it at the__tests__/AlertDialog.a11y.*binding and record that two of that contract's expectations become required once this record is current. - Keep FR6–FR9's planned verification (the evidence each needs) and add that no test exists yet, instead of replacing it.
[Automated review]
| | Builder impact | A phone-width alert is 320 px unless the caller sets `width` (FR9); actions track a slide under every pointer (FR8). | | ||
| | Compatibility/readiness | Released behavior as observed; this draft records it and adopts the APG Alert Dialog pattern (AR1) under `spec:AST-020/FR15`. FR6–FR9 change shipped behavior and are specified, not implemented yet. | | ||
| | Builder impact | None. | | ||
| | Compatibility/readiness | FR1–FR5 and AR1 record shipped behavior. FR6–FR9 are requirements not shipped yet; each is an implementation gap listed under Current-state impact. | |
There was a problem hiding this comment.
AR1 says Tab wraps inside the alert through Dialog FR7, but Dialog.tsx on main has no Tab handling or focus-region call, so Tab from the last button leaves for the browser's interface. Please list this as an AR1 gap under Current-state impact and narrow this row to match.
| | FR7 | No test yet; FR7 is not shipped. | a tap during the presentation, Enter and Escape during it, a screen reader's activation, a tap during the dismissal; a mouse click during the presentation under a fine pointer | a held key or assistive-technology action, content made inert during the entrance, a tap that reaches a leaving alert, or a mouse click held under a fine pointer fails | `audit:AlertDialog/behavior` | | ||
| | FR8 | No test yet; FR8 is not shipped. | a press on Cancel slid onto the action and lifted, and a press lifted outside the actions, each with a mouse, a pen, and a finger; Enter and Space on each action | the press-start action running after a slide, any action running on a lift outside, or a changed keyboard or assistive-technology activation fails | `audit:AlertDialog/behavior` | | ||
| | FR9 | No test yet; FR9 is not shipped. | a phone width and a desktop width, in the default and an alternate theme; a caller `width` at a phone width; a screen narrower than 320 px; a narrow window under a fine pointer; a coarse screen that is not compact | a phone-width alert other than 320 px in any theme without a caller `width`, a caller `width` overridden, a desktop width changed, an alert wider than the viewport, or a 320 px alert in a narrow fine-pointer window or on a non-compact coarse screen fails | `audit:AlertDialog/behavior` | | ||
| | AR1 | `AlertDialog.test.tsx` ("renders with alertdialog role", "links title via aria-labelledby", "links description via aria-describedby", "marks cancel as the initial focus target", "keeps the initial focus target on the least destructive action", "calls onOpenChange(false) on Escape", "stacks long destructive action labels above cancel on a narrow fine-pointer screen", "reaches both buttons by Tab, cancel first", "is a modal alertdialog", "does not claim the alertdialog role") and `useImperativeAlertDialog.test.tsx`; the no-backdrop-dismiss outcome and focus delegation under the Alert Dialog accessibility contract (`internal/a11y-spec`, pending) | open modal; after Escape; after Cancel | The role, name link, description link, or Cancel initial focus is lost, or a click outside dismisses the dialog. | `audit:AlertDialog/accessibility` | |
There was a problem hiding this comment.
"renders with alertdialog role" and "links description via aria-describedby" are not in AlertDialog.test.tsx; its comment at lines 96-98 says the binding asserts them. The Alert Dialog accessibility contract and __tests__/AlertDialog.a11y.* are on main, so "pending" is stale. With this record current, AST-020 FR9 makes alertdialog.focus.least-destructive-first and alertdialog.dismissal.click-beside-is-inert required, and both are still advisory. Please name the binding here and in verified_by, and record that as a gap or make both required.
| | FR6, PR2 | No test yet; FR6 is not shipped. | present, dismiss, close during present, reopen during dismiss, Reduce Motion, a fine pointer, a narrow viewport | a curve off its role, a start outside the viewport, a jump on interruption, a scale under Reduce Motion, changed motion under a fine pointer, or an added element fails | `audit:AlertDialog/behavior` | | ||
| | FR7 | No test yet; FR7 is not shipped. | a tap during the presentation, Enter and Escape during it, a screen reader's activation, a tap during the dismissal; a mouse click during the presentation under a fine pointer | a held key or assistive-technology action, content made inert during the entrance, a tap that reaches a leaving alert, or a mouse click held under a fine pointer fails | `audit:AlertDialog/behavior` | | ||
| | FR8 | No test yet; FR8 is not shipped. | a press on Cancel slid onto the action and lifted, and a press lifted outside the actions, each with a mouse, a pen, and a finger; Enter and Space on each action | the press-start action running after a slide, any action running on a lift outside, or a changed keyboard or assistive-technology activation fails | `audit:AlertDialog/behavior` | | ||
| | FR9 | No test yet; FR9 is not shipped. | a phone width and a desktop width, in the default and an alternate theme; a caller `width` at a phone width; a screen narrower than 320 px; a narrow window under a fine pointer; a coarse screen that is not compact | a phone-width alert other than 320 px in any theme without a caller `width`, a caller `width` overridden, a desktop width changed, an alert wider than the viewport, or a 320 px alert in a narrow fine-pointer window or on a non-compact coarse screen fails | `audit:AlertDialog/behavior` | |
There was a problem hiding this comment.
These rows drop the evidence layer each requirement needs: browser tests under a coarse pointer in Chromium and WebKit, a spring-curve oracle, pointer-stream tests with mouse, pen and finger, and theme by width browser tests. Other current records with unshipped claims keep their planned verification. Please restore the base text and add that no test exists yet.
9ce9792 to
375903b
Compare
There was a problem hiding this comment.
This review is on hold for an owner decision: promoting component:AlertDialog to current makes FR1–FR6, FR9, FR7's inert dismissal fade, AR1's adopted parts, PR2 and DEC-1–DEC-3 governing, and no current spec states them yet. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. The record's gap entries and evidence rows match main, and no other change is required. When deciding FR7, note that the record does not say where focus sits while the alert's content is inert during the touch dismissal fade.
[Automated review]
|
/approve-spec 375903b |
github-actions
Bot
removed
the
needs:spec-owner-review
Exact decision needed
Promote
component:AlertDialogfrom draft to current with a minimal, record-only diff: keep every requirement, decision, owner, and attribution, and record FR6–FR9 — specified but not yet shipped — as implementation gaps against today's main. No behavior, prop, default, target, or export changes.Existing authority searched
component:Dialog,family:overlay-dismissal,family:action-groups,spec:AST-094,spec:AST-020FR15, andspec:AST-012govern the record.AlertDialog.tsxships none of FR6–FR9 yet.Decision
component:AlertDialog,authority: current, approved by cixzhang. The FR table's draft-state column is dropped as the current template requires. FR6–FR9 each carry "Implementation gap: not shipped yet", with a matching Current-state impact entry; their verification rows keep the planned evidence and add that no test exists yet. AR1's Tab wrap is recorded as a gap too: Dialog on main does not yet contain Tab. Two Alert Dialog contract expectations (least-destructive first focus, click beside is inert) stay advisory; the AR1 verification row records thatspec:AST-020FR9 makes them required once this record is current. PR1 and the compatibility lines describe main today.Rejected alternatives
None.
Claim scope and non-goals
No requirement is added, removed, or reworded beyond the gap markers.
Evidence and implementation relationship
AlertDialog.test.tsx,useImperativeAlertDialog.test.tsx, and the Alert Dialog accessibility binding (__tests__/AlertDialog.a11y.test.tsx,__tests__/AlertDialog.a11y.chromium.spec.ts) back FR1–FR5 and AR1. FR6–FR9 have no test because they are not shipped. PR1 is source review only, named as a gap.Writing rubric results
Scope
Validation
pnpm check:knowledge: alignedgit diff --check: clean