Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval waits on an owner decision: the compact two-action row (Cancel leading while each label fits half the row) and the narrower-than-744 px or shorter-than-500 px compact condition have no governing spec yet, because draft component:AlertDialog FR3 still stacks the action above Cancel on small screens and draft AST-094's FR1–FR23 are unapproved. An engineering owner needs to provide or identify that rule (for example the FR3 amendment in the description) and approve or reject AST-094, then record the decision by commenting /approve-spec 5a2e10f269b1c79e5a9c4422c080991b1fed9b82 on this pull request; the author may do so as an engineering owner. Two fixes are needed whatever is decided:
- The first open of a mounted dialog on a compact viewport paints one stacked frame (188 px tall) before the row (148 px) in
useAlertActionLayout.tsL80–115; decide the layout before that frame paints and add a first-frame test. - The PR also carries its 24 predecessor commits (#8052–#8084); retarget it after they land so it contains only the AlertDialog commit.
[Automated review]
| rowRef: RefObject<HTMLElement | null>, | ||
| {isCompact, itemSelector, labels}: AlertActionLayoutOptions, | ||
| ): AlertActionLayout { | ||
| const [layout, setLayout] = useState<AlertActionLayout>('stack'); |
There was a problem hiding this comment.
While the dialog is closed row.clientWidth is 0, so measure() keeps this 'stack' placeholder, and the first real measurement comes from the ResizeObserver after the opening frame has painted. At 402 × 714 the Delete story's first open paints Delete above Cancel (dialog 188 px tall), then collapses to the 148 px row a frame later, which reduced-motion users see. Measuring when the dialog opens, in the layout phase, would keep the first frame settled (architecture:layer-runtime INV12).
There was a problem hiding this comment.
Fixed in a243866: the resize observer reports between layout and paint, and the hook now commits its decision there (flushSync), so the opening frame paints the row. A new browser case (Chromium and WebKit) reads the layout from a resize observer made after the alert's, which sees what the opening frame paints: Delete and Cancel share a row. Before the fix it read Delete at y 386 and Cancel at 434.
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsAlertDialog (@astryxdesign/core) · View in Storybook
Component Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included.
|
| Component | JavaScript | CSS |
|---|---|---|
| core/AlertDialog | +31.3KB |
+87B |
Bundle Size Summary
Gzip figures measure each package's entry file, not the code an import pulls in.
| Package | Entry (ESM) | Entry gzip before | Entry gzip after | Delta |
|---|---|---|---|---|
| @astryxdesign/core | 4.9KB | 1.2KB | 1.2KB | 0B |
| Package stylesheet | Gzip before | Gzip after | Delta |
|---|---|---|---|
| @astryxdesign/core | 34.6KB | 34.6KB | +43B |
Accessibility Audit
Status: No accessibility violations detected.
Visual Regression
55 of 502 shot(s) changed.
A change here is a question, not a failure: check whether the after is the
picture you intended. Record that review on the PR. Baseline maintenance is an
explicit dispatch of CI; this report never rewrites the baseline or adds a release gate.
| component | story | theme | mode | pixels |
|---|---|---|---|---|
| Selector | Indicator Space Bottom Sheet Narrow End RTL | neutral | light | 753,933 |
| Selector | Indicator Space Bottom Sheet Wide Start | neutral | light | 740,072 |
| RichTextEditor | Markdown parity: side by side | probe | dark | 558,840 |
| RichTextEditor | Markdown parity: side by side | probe | light | 540,756 |
| Chat | Full AI Chat | probe | light | 230,823 |
| Chat | Full AI Chat | probe | dark | 229,805 |
| AppShell | With Mobile Nav | probe | light | 217,010 |
| AppShell | With Mobile Nav | probe | dark | 216,960 |
| Collapsible | Dividers | probe | dark | 66,445 |
| Collapsible | Dividers | probe | light | 66,445 |
| AppShell | Playground | neutral | dark | 65,198 |
| AppShell | Playground | neutral | light | 63,788 |
| Chat | Default | neutral | dark | 36,304 |
| Chat | Default | neutral | light | 35,767 |
| Chat | Empty State | probe | light | 23,531 |
| Chat | Empty State | probe | dark | 22,622 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | dark | 15,378 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | light | 14,107 |
| Chat | With Attachments | probe | light | 13,406 |
| Chat | With Attachments | probe | dark | 11,232 |
and 35 more.
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report
5a2e10f to
89bac54
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: the compact two-action row and its 744 px / 500 px condition have no governing spec, contradict the draft AlertDialog FR3, and arrive with the unapproved draft AST-094 (FR1–FR23). Any engineering owner, the author included, can record that decision by commenting /approve-spec 89bac54ae97a8fae47160d28972519a8c5263d7e on this pull request. Whatever the decision, two changes are still needed:
- The first open of a compact alert paints one stacked frame before the row (
useAlertActionLayout.tsL80–107), so settle the layout before the opening frame paints and add a first-frame test. - The diff carries the whole stack (27,161 authored lines) for a 624-line change, so retarget onto #8084 or rebase once it and its predecessors land.
[Automated review]
| rowRef: RefObject<HTMLElement | null>, | ||
| {isCompact, itemSelector, labels}: AlertActionLayoutOptions, | ||
| ): AlertActionLayout { | ||
| const [layout, setLayout] = useState<AlertActionLayout>('stack'); |
There was a problem hiding this comment.
On the first open of each instance this 'stack' is what paints: the row has no width until Dialog's passive effect calls showModal(), so measure returns early (L92) and the first decision comes from the resize observer after paint. At 402×714 with reduced motion the dialog paints 188 px tall and stacked, then jumps to the 148 px row in the next frame (layer-runtime INV12: a layer never paints at a geometry it is about to correct). Settling the first decision before paint, for example with flushSync on the observer's first call or a layout effect keyed on the open state, plus a first-frame test, would fix it.
There was a problem hiding this comment.
Fixed in a243866: the resize observer reports between layout and paint, and the hook now commits its decision there (flushSync), so the opening frame paints the row. A new browser case (Chromium and WebKit) reads the layout from a resize observer made after the alert's, which sees what the opening frame paints: Delete and Cancel share a row. Before the fix it read Delete at y 386 and Cancel at 434.
89bac54 to
7fcc382
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: the compact two-action row and its 744 px / 500 px condition have no governing spec, contradict the draft AlertDialog FR3, and arrive with the unapproved draft AST-094 (FR1–FR23). Any engineering owner, the author included, can record that decision by commenting /approve-spec 7fcc382ceb97adae5e5ea2fd6eb73dd439537bab on this pull request. Whatever the decision, two changes are still needed:
- The first open of a compact alert paints one stacked frame before the row (
useAlertActionLayout.tsL80–115), so settle the layout before the opening frame paints and add a first-frame test. - The diff carries the whole stack (28,139 authored lines) for a 624-line change, so retarget onto #8084 or rebase once it and its predecessors land.
[Automated review]
… platform (#8274) ## Exact decision needed What "compact" means when a component picks a presentation by screen size, and whether it differs by platform. ## Existing authority searched - `spec:AST-012` FR1–FR2: width points (`md` is 768) and a closed `when` vocabulary with no height field. - `architecture:public-component-api` INV14 and change coupling: components keep no parallel hardcoded breakpoint table. - `component:Selector` FR4: `adaptive` uses the sheet on "compact coarse-pointer screens", with no number. - `design:mobile-typography` DR3 uses `(width < 1024px) and (pointer: coarse)` for text sizing, a separate concern this PR leaves alone. - No current record defines compact. Code uses `(max-width: 768px) and (pointer: coarse)`. - Search terms: compact, 744, 768, height 500, size class, adaptive, coarse pointer, breakpoint. Overlapping open pull requests: #8062 (shared touch resolver and its compact query), #8052 (AST-094 resolver), #8015 (NumberInput steppers on coarse pointers; not a size test), #6817 (compact density, a different meaning), #8089, #8104, #8095 (phone presentations that consume the resolver). ## Decision - `spec:AST-012` FR9 / DEC-6: a screen is compact while its width is below the effective `md` point or its height is below 500, with the same numbers on every platform (CSS px on the web, pt on iOS, dp on Android); native does not map compact to platform size classes. The theme exports this one definition; the web touch presentation resolver, Selector's `adaptive` presentation, and `@astryxdesign/native` all use it. - `spec:AST-012` FR2: `when` gains `height`, accepting only `{below: 500}`. - `architecture:public-component-api` INV17: a component that chooses a presentation by screen size uses that definition and keeps no numbers of its own. - `spec:AST-082` FR18: `PortableTheme` carries the compact definition, so native reads the same numbers; FR11 and `spec:AST-093` FR3: a native choice between presentations by screen size uses it, not size classes, while the platform's own control still presents the surface. - Compact governs presentation choice only; a layout width a caller picks, such as AppShell's `mobileNav.breakpoint`, is not compact. - `component:Selector` records the `adaptive` edge change as pending and compatible under `spec:AST-017` FR1. ## Rejected alternatives - Mapping compact to each platform's size classes: the same screen would be compact on one platform and not another. ## Claim scope and non-goals The compact definition and the height condition only. Text sizing under `design:mobile-typography`, which surfaces adopt a touch presentation, and resolver code are outside this PR. ## Evidence and implementation relationship The touch presentation resolver contract lives only in the open `spec:AST-094` revision (#8052), which this PR does not edit; it must use FR9. Native adopts FR9 when it gains a compact resolver. Behavior delta when the code conforms: a coarse-pointer screen exactly 768 wide and at least 500 tall opens the popover, and a landscape phone wider than 768 but under 500 tall opens the bottom sheet. Code follow-up belongs to separate implementation PRs. INV16 is the caller-control invariant from #8273 (merged); this PR adds INV17. No implementation is included. ## Writing rubric results - R1 narration: none. - R2 research in the record: none. - R3 evidence naming a change: none; verification names edge states. - R4 rejected alternatives over one line or outside a decision: one, inside DEC-6, one line. - R5 pull request or issue references remaining: none added. - R6 ruled open questions still listing options: none. ## Scope - [x] This PR contains specification records only. - [x] It records observable behavior and preserves equivalent internal implementations. - [x] Any named internal mechanism is intentionally public, names its dependent caller/system, and explains why an equivalent implementation would not satisfy the contract. - [x] Verification states the evidence layer and failure signal without prescribing CI job/workflow topology. - [x] It records one intentional durable decision, not a rescue for a separable tagalong. - [x] The canonical knowledge owner, affected current claims, and overlapping open work are named; any ownership collision is resolved or called out for human decision. - [x] Adjacent API, visual, accessibility, composition, and implementation facts remain outside scope unless this exact decision depends on them. - [x] Public text and artifacts contain no internal Meta context. ## Validation - `pnpm check:knowledge` (CI lint) - Prettier - `git diff --check`
7fcc382 to
1a2438d
Compare
vjeux
changed the title
1a2438d to
a243866
Compare
|
Addressed at a243866:
Waiting in the reviewers' queue: the spec decision on the compact row and its 744 px / 500 px condition (the FR3 amendment in the description). This pull request carries its stack until it lands; review the last commit. |
There was a problem hiding this comment.
Approval waits on an engineering owner's decision. The iOS compact two-action row has no governing spec (the draft AlertDialog FR3 still stacks on small screens), and the draft AST-094 (FR1–FR23) is unapproved. Its 744 px compact condition and the alert's reduced-motion spring fade contradict the current AST-012 FR9 and reduced-motion DR2 and DR4, so either an owner updates those specs or the code conforms to them. Any engineering owner, the author included, can record that decision by commenting /approve-spec a243866944f89a0aa16501b83b06d6c8deac1402 on this pull request. Whatever the decision, these still need changes:
- The first compact open on iOS raises a "ResizeObserver loop completed with undelivered notifications" error because the observer's callback resizes the row it observes, and re-deciding while open logs three React
flushSyncerrors (useAlertActionLayout.tsL105–129). - The docs and changeset say clicks, keys and assistive technology are ignored during both transitions, but while the alert appears only touches are.
- The diff carries the whole stack (32,443 authored lines) for a 775-line change; retarget onto #8084, or rebase once the stack lands.
[Automated review]
| flushSync(() => { | ||
| setLayout(next); | ||
| }); |
There was a problem hiding this comment.
This commit runs inside the observer callback for the row, and it shrinks that same row from 72 px to 32 px when the stack turns into a row. The browser then skips re-delivering the row and dispatches "ResizeObserver loop completed with undelivered notifications." as a window error event on every first compact open in the iOS idiom (reproduced in Chromium at 402×714 with no other observer on the page). Observing an element whose size the decision does not change would avoid it, for example a zero-height element spanning the row's width plus the labels.
| const stops = [row, ...labelsShown].map(element => | ||
| observeResize(element, onResize), | ||
| ); |
There was a problem hiding this comment.
observeResize fires its callback synchronously on registration (utils/sharedResizeObserver.ts L58, L92). When this effect re-runs while the dialog is laid out, onResize therefore calls flushSync inside the layout effect, and React logs "flushSync was called from inside a lifecycle method" once per observed element. That is three errors when an open alert is resized from 900 px to 402 px wide. The effect body already measures (L125–129), so skipping the registration call is enough.
| usage: { | ||
| description: | ||
| 'AlertDialog asks the user to confirm a destructive or irreversible action before it happens. Use it for things like deleting content, revoking access, or discarding unsaved changes.\n\nIt implements the WAI-ARIA APG [Alert Dialog pattern](https://www.w3.org/WAI/ARIA/apg/patterns/alertdialog/): `role="alertdialog"`, a title linked by `aria-labelledby`, a consequence description linked by `aria-describedby`, focus moved into the dialog on open and returned to the trigger on close, and no dismissal by clicking outside. Escape cancels.\n\nAlertDialog passes its requested width through to Dialog, which clamps the surface to the container and dynamic viewport with token gutters. Generic Dialog footers should wrap, but Dialog does not own action semantics or order; consumer composition controls that. AlertDialog owns its confirmation semantics: above 640px, actions render horizontally and may move onto another row; at 640px and below, the destructive action appears above Cancel and both buttons fill the footer width. Button labels retain their standard single-line behavior. The breakpoint follows available width, not pointer or hover capability. The body scrolls when block space is constrained.\n\nFor cases where you want to show an alert without managing open state, use the `useImperativeAlertDialog` hook: call `alert.show(options)` and render `alert.element` in your tree.', | ||
| 'AlertDialog asks the user to confirm a destructive or irreversible action before it happens. Use it for things like deleting content, revoking access, or discarding unsaved changes.\n\nIt implements the WAI-ARIA APG [Alert Dialog pattern](https://www.w3.org/WAI/ARIA/apg/patterns/alertdialog/): `role="alertdialog"`, a title linked by `aria-labelledby`, a consequence description linked by `aria-describedby`, focus moved into the dialog on open and returned to the trigger on close, and no dismissal by clicking outside. Escape cancels.\n\nAlertDialog passes its requested width through to Dialog, which clamps the surface to the container and dynamic viewport with token gutters. Generic Dialog footers should wrap, but Dialog does not own action semantics or order; consumer composition controls that. AlertDialog owns its confirmation semantics: above 640px, actions render horizontally and may move onto another row; at 640px and below, the destructive action appears above Cancel and both buttons fill the footer width. On iPhone and iPad a compact viewport (narrower than 744px or shorter than 500px) follows the iOS alert instead: the actions share a row, Cancel leading, when each label fits its half of it on one line, and otherwise stack with the destructive action above Cancel, both filling the footer width. Button labels retain their standard single-line behavior. Outside iOS the breakpoint follows available width, not pointer or hover capability. The body scrolls when block space is constrained.\n\nOn a touch screen the two actions are tracked as one group under a finger or a pen, the way iOS tracks the actions of an alert: the action under the finger highlights as soon as it is touched, the highlight follows the finger from one action to the other, and lifting the finger runs the action under it. Lifting it off both actions runs nothing. A mouse, the keyboard and assistive technology keep the click of each button.\n\nOn iPhone and iPad the alert also appears and leaves the way an iOS alert does: it scales down slightly into place while it and the dimming fade in, and it fades out with the dimming when it closes. Nothing reaches it during either transition: a touch, a click, a key or an assistive-technology activation does nothing. With reduced motion it only fades. Everywhere else (a mouse, or a finger on Android) it keeps the theme entrance and closes at once.\n\nFor cases where you want to show an alert without managing open state, use the `useImperativeAlertDialog` hook: call `alert.show(options)` and render `alert.element` in your tree.', |
There was a problem hiding this comment.
"Nothing reaches it during either transition: a touch, a click, a key or an assistive-technology activation does nothing" holds only for the fade-out. While the alert presents, only finger and pen presses are dropped (useActionGroupTracking isInert). handleCancel, handleAction and handleOpenChange check only isLeaving, so Escape, Enter or a VoiceOver activation still acts in the first 435 ms. The same wording is in .changeset/alertdialog-touch-motion.md.
| export const TOUCH_QUERY = '(pointer: coarse)'; | ||
|
|
||
| /** The viewport is compact: layout behaviours. */ | ||
| export const COMPACT_QUERY = '(width < 744px), (height < 500px)'; |
There was a problem hiding this comment.
Current spec:AST-012 FR9 makes compact "below the effective md point (FR1) or … below 500" and says the web touch presentation resolver keeps "no size numbers of its own". DEC-6 rejects mapping compact to platform size classes. The literal 744 departs from both, so a coarse 744–767 px screen is not compact here. Either FR9 is amended, or this reads the theme's md point.
| * is everything else. Pair `ios` with `pointer: 'coarse'` for a touch | ||
| * profile, such as touch motion tuned per platform. | ||
| */ | ||
| platform?: 'ios' | 'other'; |
There was a problem hiding this comment.
Current spec:AST-012 FR2 closes the when vocabulary to width, height, pointer, contrast and motion: "Future environment fields require explicit additions to this closed vocabulary." platform needs that amendment or has to come out.
| const handle = panel.animate( | ||
| {opacity: 1, scale: 1}, | ||
| {spring: springs.alertPresent}, | ||
| {crossfade: springs.alertPresent}, | ||
| ); |
There was a problem hiding this comment.
With reduced motion this fades the dialog and its ::backdrop on springs.alertPresent, about 404 ms, as measured. A theme can retune that spring. Current design:reduced-motion DR2 asks for an opacity-only fade of 200 ms or less on a coarse pointer, DR1 and its verification row exclude a spring, and DR4 forbids a theme value that lengthens it. The open owner question covers whether those records change.
a243866 to
aa32c53
Compare
github-actions
Bot
added
the
needs:design-review
There was a problem hiding this comment.
Approval waits on an engineering owner's spec decision: no current spec covers AlertDialog's compact-touch row (Cancel and the action sharing one row under a finger), the new draft FR6 320 px width cites an owner decision this pull request does not show, the change adds alert.present and alert.dismiss to spec:AST-094 FR4's own table, and AlertDialog's record does not state its touch exit or its action-group tracking. An engineering owner, the author included if they are one, records that decision by commenting /approve-spec d586e2fd962f6e534297d4a2949b6aa9427a79cb on this pull request. Independently of that decision, please fix the following:
- Stop the first compact-touch open from raising "ResizeObserver loop completed with undelivered notifications." by not resizing the row the layout's own observer watches.
- Skip the
flushSyncinuseAlertActionLayoutduring the resize observer's registration call, which logs three React errors per inline compact-touch alert. - Correct the story notes,
AlertDialog.doc.mjs, FR6 and the touch-motion changeset, which misstate the 768 px compact edge, the finger condition, the mouse tracking and when the 320 px width applies. - Land the parent stack and rebase (or retarget onto #8084) so the diff carries this change alone.
[Automated review]
| // the decision here settles the layout in that same frame, so no frame | ||
| // paints the layout it is about to correct (layer-runtime INV12). | ||
| // eslint-disable-next-line @eslint-react/dom-no-flush-sync -- the opening frame must paint the decided layout, and the observer runs between layout and paint | ||
| flushSync(() => { |
There was a problem hiding this comment.
This commit runs inside the observer delivery for the row it resizes: on the Delete story under touch (390 x 844 and 844 x 390, Chromium) the first open raises "ResizeObserver loop completed with undelivered notifications." on window. Because observeResize fires each callback synchronously on registration (lines 122-124), it also runs inside this layout effect, where React logs "flushSync was called from inside a lifecycle method" three times per inline compact-touch mount. Observing only what the decision does not resize, and skipping the commit on the registration call (the effect already measures at line 125), would remove both.
| * Wide reference state. In a wide viewport, Dialog preserves AlertDialog's | ||
| * preferred 400px surface and AlertDialog renders a horizontal action row. | ||
| * preferred 400px surface and AlertDialog renders a horizontal action row. On a | ||
| * compact viewport (narrower than 744px or shorter than 500px) the two short |
There was a problem hiding this comment.
The code reads the theme's compact definition (768 px by default, not 744) and gives the row only under a coarse pointer; with a mouse this story stacks at 640 px and below whatever the labels.
| guidance: true, | ||
| description: | ||
| 'Use concise action labels; >640px complete buttons may move to another row, and <=640px puts the destructive action above Cancel with both buttons full width regardless of pointer type.', | ||
| 'Use concise action labels; wide: complete buttons may move to another row; compact: short labels share a row with Cancel leading, a label too long for its half stacks the destructive action above Cancel, both full width, regardless of pointer type.', |
There was a problem hiding this comment.
"regardless of pointer type" contradicts the code: with a mouse a compact viewport keeps today's stack at 640 px and below, and the shared row applies only under a coarse pointer.
| | FR3 | The anatomy is title, description, then the buttons: Cancel then action on wider screens, the action stacked above Cancel on small screens, Cancel keeping initial focus either way; the action may show a loading state while `isActionLoading`. | shipped source and focused tests | verify | | ||
| | FR4 | `isInline` renders the same anatomy without modality and without the alert-dialog role. | shipped source and public docs | verify | | ||
| | FR5 | `useImperativeAlertDialog` returns `show`, `hide`, and the element to render, and owns `isOpen`/`onOpenChange` for its caller. | shipped source and focused tests | verify | | ||
| | FR6 | On phones (compact width with a touch pointer), alerts are 320 px wide in every theme, matching iOS, with no new theme role; elsewhere the default width is 400. | owner decision, 2026-10-26 | settled | |
There was a problem hiding this comment.
The code gives 320 px to any coarse pointer on a compact viewport, a landscape phone compact by height included (320 px at 844 x 390), not only to phones by width; and no record or comment on this pull request shows the cited owner decision.
Her merged record (meta#8594) keeps AlertDialog's public contract unchanged: the touch presentation is a behavior change, and on a phone a caller's width is its way back (FR9). Pin AlertDialogProps so it has no presentation prop and keeps width. Fails typecheck while the prop exists.
…keeps the public contract Her merged AlertDialog record (meta#8594) changes behavior under a touch presentation without changing the public API: a caller's width is the way back on a phone (FR9), and no other prop is added. Remove the presentation prop, its AlertDialogPresentation type, the DialogPresentation story, its doc entry and the tests that used it. The width test also covers a caller's width on a phone.
…lertDismiss Her merged AlertDialog FR6 (meta#8594) names the alert's motion roles springs.alertPresent and springs.alertDismiss, the springs table's rows. Expect those role ids, the alert's two springs among the registered springs roles, and AlertDialog registering those two rows alone. Fails while the roles are registered as alert.present and alert.dismiss.
…d springs.alertDismiss Her merged AlertDialog FR6 (meta#8594) names the alert's motion roles by their springs table rows. Register the alert's two springs among the springs roles under those names, rename AST-094 FR4's two rows to match, and let AlertDialog register those two rows alone (registerAlertSprings), so its bundle still carries no other spring. The alert table keeps only its measured values. Same springs and timings; only the role ids change.
Main merged meta#8045 and meta#8046. The one conflict is utils/pressFeedback.ts: it takes main's merged meta#8046 text (the finger and button slots, the mouse and key press) and keeps this pull request's own additions, the one writer of the press attribute (writePress, paintPress, readPressPaint) and the header line naming them.
# Conflicts: # internal/interaction-harness/README.md # internal/interaction-harness/src/busy.test.ts # internal/interaction-harness/src/busy.ts # internal/interaction-harness/src/frames.ts # internal/interaction-harness/src/harness.interaction.browser.spec.ts
Main's button slot, merged with meta#8046, ends a press's release at once when the next press starts, by clearing the press attribute directly. On a row another painter holds, that clear erased the hold, which this pull request's one writer refuses. A mouse press on a held row, released, then a press elsewhere must leave the row held. Fails until the early end goes through the one writer.
finish() writes none through writePress, as this pull request's controller paints, so a held row stays its owner's when the next press ends a release early, under the finger and the button slots alike.
Both upstream heads are folded onto main 70ea859 (her meta#8045 frame evidence and meta#8046 coarse press): meta#8446 takes main's harness files, and meta#8073 carries the press controller onto meta#8046's slots. meta#8076 has no change of its own in either set of files, so the merge takes theirs with no hand resolution, and meta#8076 is clean against main again.
# Conflicts: # internal/interaction-harness/README.md # internal/interaction-harness/src/busy.test.ts # internal/interaction-harness/src/busy.ts # internal/interaction-harness/src/frames.ts # internal/interaction-harness/src/harness.interaction.browser.spec.ts # packages/core/src/utils/pressFeedback.ts
meta#8076's new head carries main 70ea859 with meta#8073's writer of the press attribute on main's press controller, and its README names both writers. utils/pressFeedback.ts keeps that controller with this line's own changes on it: the pointer, drag, scroll and visibility listeners through the shared input stream (meta#8356) and touchcancel only while the primary pointer is coarse (meta#8270); the key path stays at the window's bubble phase, so a Space press the page cancels paints nothing. usePressTracker stays in its own module, one tracker for a component's life (meta#8315), so meta#8073's rebuilt-tracker hook and its test are not taken. The harness files are meta#8076's.
…s, and the action-group tracker writes its own The header and writePress called it the one writer of the press attribute, but main's action-group tracker writes its groups' presses itself. They now say what holds, as meta#8076's gestures README does.
…he press attribute The release note said every press painter writes data-astryx-press through one internal writer, but the action-group tracker (hooks/actionGroupTracker.ts) writes its groups' presses itself, as the gestures README and utils/pressFeedback.ts already say. The note now says paintPress writes every press it tracks and the action-group tracker writes its own.
…tion to its tracker meta#8046 made the controller paint a mouse press itself where the primary pointer is coarse (a tablet with a trackpad). On an action its group tracks (family:action-groups), the group's tracker paints that press, so the controller must paint nothing there, as it already does for a finger; it paints the press, then turns the tracker's paint to 'fading' at the release. A Space press on the same action still paints: the group tracks pointers only (guard).
… its tracker Where the primary pointer is coarse the controller paints a mouse press itself (meta#8046). It now skips an action a group's tracker paints for a pointer's press, a mouse's included, as it already did for a finger, so the two never write the press attribute over each other. A Space press on the action still paints: the group tracks pointers only.
…he keyboard keeps the buttons Her AlertDialog record (meta#8594) verifies FR8 with a press lifted outside the actions under a mouse, a pen and a finger, and with Enter and Space on each action. Add the two cases the tests lacked: a mouse press that slides off both actions highlights nothing and runs nothing at the lift, and under a coarse and a fine pointer Enter runs the action and Space cancels. A mutant that stops tracking a mouse's moves fails the first; one that drops the buttons' own click fails the second.
…its tests give the lengths a browser computes The row's gap fell back to its row gap and then to 0, and each action's padding and borders fell back to 0. A browser always computes those as lengths (a flex row with no gap computes its column gap as `normal`), so the fallbacks only ever served jsdom, which resolves no `var()` (the row's column gap computes to `var(--spacing-2)`) and computes no padding for the actions: the tests passed on lengths no browser produces, and the long-labels case passed only because jsdom lays nothing out. The column gap is now read as computed, `normal` being no gap, and padding and borders with no fallback. The AlertDialog tests stub the row's geometry and the computed lengths together, the long-labels case included, and the hook's own fixture sets every length it measures. New cases pin that padding and borders count against an action's half and that a `normal` column gap counts as none. Without the fixture changes, four cases fail on this code: they relied on the fallbacks. No change in a browser. [skip ci]
There was a problem hiding this comment.
Approval waits on an engineering owner's spec decision: no current spec covers AlertDialog's compact-touch row (Cancel and the action sharing one row under a finger), because the draft component:AlertDialog FR3 still stacks the action above Cancel on small screens, and this change also adds springs.alertPresent and springs.alertDismiss to spec:AST-094 FR4's own role table, where FR4 and DEC-5 send a component's measured roles through the generated motion role table. An engineering owner, the author included if they are one, records that decision by commenting /approve-spec c743c15dd9650aeece28377cf19555c9f138f458 on this pull request. Independently of that decision, please fix the following:
- Stop the first compact-touch open from raising "ResizeObserver loop completed with undelivered notifications." by not resizing the row the layout's own observer watches.
- Skip the
flushSyncinuseAlertActionLayoutfor the observer's registration calls, which run inside the layout effect and make React log errors for an inline or already-open compact-touch alert. - Correct
AlertDialog.doc.mjs, the AlertDialog stories and the touch-motion changeset, which say a mouse keeps each button's click, give the compact row no pointer condition or a 744 px edge, and give the 320 px width only to "a compact width". - Land the parent stack and rebase (or retarget onto #8084) so the diff carries this change alone.
[Automated review]
| const stops = [row, ...labelsShown].map(element => | ||
| observeResize(element, onResize), | ||
| ); |
There was a problem hiding this comment.
Two issues meet here. Observing the row itself and committing the decision with flushSync inside this observer's callback (lines 105-118) resizes an observed element during its own delivery, so the first compact-touch open from the 'stack' placeholder raises "ResizeObserver loop completed with undelivered notifications."; observing only what the decision does not resize (the labels and the row's container width) avoids it. And observeResize calls onResize once synchronously on registration, so these three calls run flushSync inside the layout effect whenever the row is already laid out (an inline alert, or one mounted open), which React reports as "flushSync was called from inside a lifecycle method"; skipping the commit for the registration calls avoids it, since the effect measures right after (lines 125-129).
| usage: { | ||
| description: | ||
| 'AlertDialog asks the user to confirm a destructive or irreversible action before it happens. Use it for things like deleting content, revoking access, or discarding unsaved changes.\n\nIt implements the WAI-ARIA APG [Alert Dialog pattern](https://www.w3.org/WAI/ARIA/apg/patterns/alertdialog/): `role="alertdialog"`, a title linked by `aria-labelledby`, a consequence description linked by `aria-describedby`, focus moved into the dialog on open and returned to the trigger on close, and no dismissal by clicking outside. Escape cancels.\n\nAlertDialog passes its requested width through to Dialog, which clamps the surface to the container and dynamic viewport with token gutters. Generic Dialog footers should wrap, but Dialog does not own action semantics or order; consumer composition controls that. AlertDialog owns its confirmation semantics: above 640px, actions render horizontally and may move onto another row; at 640px and below, the destructive action appears above Cancel and both buttons fill the footer width. Button labels retain their standard single-line behavior. The breakpoint follows available width, not pointer or hover capability. The body scrolls when block space is constrained.\n\nFor cases where you want to show an alert without managing open state, use the `useImperativeAlertDialog` hook: call `alert.show(options)` and render `alert.element` in your tree.', | ||
| 'AlertDialog asks the user to confirm a destructive or irreversible action before it happens. Use it for things like deleting content, revoking access, or discarding unsaved changes.\n\nIt implements the WAI-ARIA APG [Alert Dialog pattern](https://www.w3.org/WAI/ARIA/apg/patterns/alertdialog/): `role="alertdialog"`, a title linked by `aria-labelledby`, a consequence description linked by `aria-describedby`, focus moved into the dialog on open and returned to the trigger on close, and no dismissal by clicking outside. Escape cancels.\n\nAlertDialog passes its requested width through to Dialog, which clamps the surface to the container and dynamic viewport with token gutters. Generic Dialog footers should wrap, but Dialog does not own action semantics or order; consumer composition controls that. AlertDialog owns its confirmation semantics: above 640px, actions render horizontally and may move onto another row; at 640px and below, the destructive action appears above Cancel and both buttons fill the footer width. Under a finger on a compact viewport (narrower than 768px or shorter than 500px by default, the compact definition of the theme) it follows the iOS alert instead: the actions share a row, Cancel leading, when each label fits its half of it on one line, and otherwise stack with the destructive action above Cancel, both filling the footer width. Button labels retain their standard single-line behavior. With a mouse the breakpoint follows available width alone. The body scrolls when block space is constrained.\n\nOn a touch screen the two actions are tracked as one group under a finger or a pen, the way iOS tracks the actions of an alert: the action under the finger highlights as soon as it is touched, the highlight follows the finger from one action to the other, and lifting the finger runs the action under it. Lifting it off both actions runs nothing. A mouse, the keyboard and assistive technology keep the click of each button.\n\nUnder a finger, on any touch device, the alert also appears and leaves the way an iOS alert does: it scales down slightly into place while it and the dimming fade in, and it fades out with the dimming when it closes. While it appears a touch does nothing (keys and assistive technology still act), and while it leaves nothing reaches it: a touch, a click, a key or an assistive-technology activation does nothing. With reduced motion it only cross-fades. With a mouse it keeps the theme entrance and closes at once.\n\nFor cases where you want to show an alert without managing open state, use the `useImperativeAlertDialog` hook: call `alert.show(options)` and render `alert.element` in your tree.', |
There was a problem hiding this comment.
This says a mouse keeps the click of each button, but AlertDialog.tsx tracks the two actions under every pointer, a mouse included, as family:action-groups FR5 requires; the dense copy at line 199 says the same. Line 44 also gives the compact row with no pointer condition and line 219 adds "regardless of pointer type", while a mouse keeps the 640 px stack or the trailing row.
| * preferred 400px surface and AlertDialog renders a horizontal action row. On a | ||
| * compact viewport (narrower than 744px or shorter than 500px) the two short | ||
| * actions keep sharing a row, Cancel leading, as an iOS alert's do. |
There was a problem hiding this comment.
The row applies only under a coarse pointer and uses the theme's compact definition (768 px wide or 500 px tall by default), so for this fine-pointer story a compact viewport keeps the 640 px stack. The same 744 px figure appears at line 108.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] AlertDialog: under a finger the alert appears and leaves the way an iOS alert does. It scales down into place from 1.2 while it and the dimming fade in, and it fades out with the dimming when it closes, on the alert's measured spring; the animation keeps running when the page is busy. While it appears a touch does nothing (keys and assistive technology still act), and while it leaves nothing reaches it: a touch, a click, a key or an assistive-technology activation does nothing. With reduced motion it only cross-fades. Defaults under a finger: on a phone (a compact width) the width with no `width` prop goes from 400 px to UIKit's 320 px in every theme, and this motion replaces the Dialog's entrance (rising 16 px and scaling up from 0.95 while it fades in) and its exit. With a mouse nothing changes. A `width` you pass still wins on a phone. The actions are tracked as one group under every pointer. |
There was a problem hiding this comment.
The 320 px default applies to any coarse pointer on a compact viewport (below md or under 500 px tall), so a landscape phone gets it too; "a phone (a compact width)" undersells that.
Depends on #8084. Review only this PR's change: vjeux/astryx@19af775...c743c15
Why
On a phone, AlertDialog stacks its two buttons at full width at 640 px and below, so "Cancel" and "Delete" take two rows. iOS keeps two buttons side by side, Cancel on the leading side, unless one label is too long for its half.
What led to it
The UIKit-parity work, which brings Astryx's touch behavior to iOS's, measured iOS 26.2: beside Cancel a label stays in the row up to 115 pt and stacks from 119 pt, per label, not by the two widths together (
alertController.actions.rowShare). Every touch device gets the row (spec:AST-094FR13).Options and tradeoffs
flex-wrap. No script, but it wraps on the sum of both widths and keeps the row's order when it wraps, so Cancel would land on top.Why this choice
Only option 3 expresses UIKit's per-label rule. A mouse keeps today's stack; sizes, type and colors stay the theme's. The row reads its gap and each action's padding and borders as the browser computes them, with no fallbacks.
Owner question:
component:AlertDialogFR3 stacks the action above Cancel on every small screen, and PR1 lists no resize observer; can FR3 take "Under a finger, on a compact viewport, Cancel and the action share a row, Cancel leading, when each label fits its half on one line; otherwise the action stacks above Cancel. With a mouse they stack at 640 px and below. Cancel keeps initial focus." and PR1 this row's observer?How to check it
On a phone or the iOS simulator:
Core/AlertDialogDelete story: Cancel and Delete share a row, Cancel on the left. On main they stack.dir="rtl": the row mirrors, Cancel on the right.Steps 1 to 3 were run on the iOS 26.2 simulator (step 1 also on the Android 15 emulator), steps 4 and 5 in the browser tests: each behaved as written. This head's unit and browser tests (Chromium and WebKit) pass locally; local gates are the evidence, and no CI run is expected.
Evidence
iOS 26.2 simulator, Mobile Safari, main and this PR; then right to left:
Android 15 emulator, Chrome 124 (blur off for capture), main on the left:
touch-support-bundle.test.mjs(test(core): a touch component's import carries its gesture code, and nothing else does #8671 fixes it).