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.
This pull request is on hold for an owner decision: AlertDialog's touch motion rests only on draft spec:AST-094 (FR4, FR5, FR7, FR8 and DEC-1), and FR4 narrows the current architecture:theme-tokens INV3, so an engineering owner has to approve or amend those requirements. An engineering owner records the decision by commenting /approve-spec <full head SHA> on the pull request, and the author may do so if they are an engineering owner. Separately from that decision:
- Land the stacked pull requests first and rebase, so this diff is only the AlertDialog motion commit instead of 25,379 authored lines.
- While the alert fades out under touch, keyboard and assistive-technology activations still run
onAction, and Escape callsonOpenChange(false)again; make the leaving alert inert. - Under React StrictMode, an AlertDialog that mounts open never starts its presentation and stays invisible and modal; the status effect should own and clean up its animation frame.
- The
adaptivethreshold change contradictscomponent:SelectorDEC-1; split it out or keep the current query. - The
lintjob failed on this head with an ESLint out-of-memory error and needs a rerun.
[Automated review]
| ref={ref} | ||
| isOpen={isOpen} | ||
| ref={mergedRef} | ||
| isOpen={motion.isPresented} |
There was a problem hiding this comment.
Under a coarse pointer this keeps the native dialog open for the 404 ms fade (504 ms backstop), but only touch and pen are inert (useActionGroupTracking.ts:186). The still-focused Delete takes a second Enter or AT activation and runs onAction again (:271), and Escape calls onOpenChange(false) again. Consider making the content inert while isLeaving, or dropping onAction, cancel and dismissal once isOpen is false.
There was a problem hiding this comment.
Fixed in 6dca4bf: while the alert leaves (closed by the caller, still presented during its fade), AlertDialog drops a click, Enter or assistive-technology activation on an action, Cancel, and Escape, as UIKit delivers no input during a dismissal. Focus stays where it is, so Dialog returns it to the opener. The new case 'on iOS, while the alert leaves, runs nothing' failed before the fix (onAction ran).
| const was = statusRef.current; | ||
| const now: Status = !isPresented ? 'hidden' : isOpen ? 'open' : 'leaving'; | ||
| statusRef.current = now; | ||
| if (!isEnabled || was === now) { |
There was a problem hiding this comment.
Under StrictMode replay of a mount-open alert (for example useImperativeAlertDialog's first show()), statusRef is already 'open', so this returns. Meanwhile the cleanup at :211 has cancelled the presentation frame, so the panel stays at opacity 0 and scale 1.2 and touches stay inert. Giving this effect its own cleanup (cancel the frame, restore statusRef) would make the replay re-run the opening (architecture:react-component-runtime INV3).
There was a problem hiding this comment.
The presentation no longer waits for an animation frame (it starts in the effect that opens the dialog), so a StrictMode replay has nothing to cancel, as the later review notes. 6dca4bf adds the StrictMode mount-open case: the alert presents and takes touches once the presentation completes.
| // and large touch displays stay anchored, phones in either orientation get | ||
| // the sheet. The query is the shared touch presentation's | ||
| // (utils/touchPresentation.ts). | ||
| export const COMPACT_TOUCH_PRESENTATION_QUERY = COMPACT_TOUCH_QUERY; |
There was a problem hiding this comment.
This moves adaptive from (max-width: 768px) and (pointer: coarse) to (width < 744px) or (height < 500px). Current component:Selector DEC-1 records that "adaptive keeps its width-and-pointer query". The change belongs in its own pull request with an owner decision, or this should keep the current query.
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 | +30.7KB |
+73B |
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 | 35.4KB | 35.4KB | +44B |
Accessibility Audit
Status: No accessibility violations detected.
Visual Regression
55 of 504 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
f3b37d2 to
6e2412f
Compare
There was a problem hiding this comment.
This pull request is on hold for an owner decision: AlertDialog's touch motion rests only on draft spec:AST-094 (FR4, FR5, FR7, FR8 and DEC-1), and FR4 narrows the current architecture:theme-tokens INV3, so an engineering owner needs to approve or amend those requirements. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Separately from that decision:
- Land the stacked pull requests first and rebase, so this diff is only the AlertDialog motion commit instead of 26,547 authored lines.
- While the alert fades out under touch, keyboard and assistive-technology activations still run
onActionand Escape callsonOpenChange(false)again; make the leaving alert inert. - Under React StrictMode, an AlertDialog that mounts open never starts its presentation and stays invisible and unresponsive to touch; let the status effect own and clean up its animation frame.
- The
adaptivethreshold change contradictscomponent:SelectorDEC-1; split it out or keep the current query. - The
lintjob runs out of memory on this head while the base passes.
[Automated review]
6e2412f to
b635c03
Compare
There was a problem hiding this comment.
This pull request is on hold for an owner decision: AlertDialog's touch motion rests only on draft spec:AST-094 (FR4, FR5, FR7, FR8 and DEC-1), and FR4 narrows the current architecture:theme-tokens INV3, so an engineering owner needs to approve or amend those requirements. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Separately from that decision:
- Land the stacked pull requests first and rebase, so this diff is only the AlertDialog motion commit instead of 27,525 authored lines.
- While the alert fades out under touch, keyboard, click and assistive-technology activations still run
onAction, and Escape callsonOpenChange(false)again; make the leaving alert inert. - The
adaptivethreshold change contradictscomponent:SelectorDEC-1; split it out or keep the current query. - The
lintjob runs out of memory on this head whilemainpasses. - Add a StrictMode mount-open test for the new motion effect; the StrictMode presentation issue raised on the earlier head no longer applies.
[Automated review]
b635c03 to
1c55503
Compare
vjeux
changed the title
There was a problem hiding this comment.
This pull request is on hold for an owner decision: AlertDialog's iPhone and iPad motion rests only on draft spec:AST-094 (FR5, FR7, DEC-1), and that record's FR4 and DEC-1 forbid the theme-tunable touch motion this head adds through defineTheme({motion: {springs, durations, curves}}), so an engineering owner needs to settle which holds, and whether the two non-component experimental subpaths are an acceptable boundary under spec:AST-017 FR21. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request once the record says so, and the author may do so if they are an engineering owner. Separately from that decision:
- While the alert fades out on iOS, keyboard and assistive-technology activations still run
onActionand Escape callsonOpenChange(false)again; make the leaving alert inert. - Under Reduce Motion the iOS fades last 404 ms on a spring, but
design:reduced-motionDR2 allows an opacity-only change of 200 ms or less, and DR4 bars theme values that lengthen it. - The compact test's 744 px width contradicts
spec:AST-012FR9 (the theme'smdpoint or a height under 500), and theplatformadaptation condition extends the vocabulary FR2 closes. - Land the stacked pull requests first and rebase, so this diff is only the AlertDialog motion commit instead of 31,379 authored lines.
- Add a StrictMode mount-open test for the new motion effect.
[Automated review]
| ref={ref} | ||
| isOpen={isOpen} | ||
| ref={mergedRef} | ||
| isOpen={motion.isPresented} |
There was a problem hiding this comment.
While the alert fades out on iOS, Dialog stays an open modal here, but only the touch tracker consults the inert window (:250). A second Enter or assistive-technology activation of the still-focused Delete runs onAction again (:285), and Escape calls onOpenChange(false) on an alert the caller already closed. Making the content inert while isPresented && !isOpen, or dropping the handlers once isOpen is false, would close that window.
| const handle = panel.animate( | ||
| {opacity: 1, scale: 1}, | ||
| {spring: springs.alertPresent}, | ||
| {crossfade: springs.alertPresent}, |
There was a problem hiding this comment.
Under Reduce Motion this fades on springs.alertPresent, a 404 ms curve, and the exit at :252 does the same on springs.alertDismiss. design:reduced-motion DR2 (current) requires an opacity-only change that finishes in 200 ms or less on a coarse pointer, and DR1 rules out springs.
| 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.
spec:AST-012 FR9 (current) defines compact as width below the theme's effective md point or height below 500, used by this resolver and by Selector's adaptive, with no size numbers of their own. 744px keeps a number of its own and gives the popover at 744–767 px where FR9 gives the sheet.
| * 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.
spec:AST-012 FR2 (current) 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 first, or should come out of this pull request; AlertDialog's motion does not use it.
1c55503 to
6dca4bf
Compare
|
Addressed at 6dca4bf:
Waiting in the reviewers' queue: the spec decision (draft AST-094 FR4, FR5, FR7, FR8, DEC-1 and theme-tokens INV3; the alert's springs and durations are now theme motion roles) and the |
There was a problem hiding this comment.
This pull request is on hold for an owner decision: AlertDialog's iPhone and iPad motion rests only on draft spec:AST-094 (FR5, FR7, DEC-1), and that record's FR4 and DEC-1 forbid the theme-tunable touch motion this head adds through defineTheme({motion: {springs, durations, curves}}), so an engineering owner needs to settle which holds, and whether the two non-component experimental subpaths are an acceptable boundary under spec:AST-017 FR21. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request once the record says so, and the author may do so as an engineering owner. Separately from that decision:
- Under Reduce Motion the iOS fades last 404 ms on a spring, but
design:reduced-motionDR2 allows an opacity-only change of 200 ms or less, and DR4 bars theme values that lengthen it. - The changeset and docs say no click, key or assistive-technology activation reaches the alert during either transition, but during the entrance they still run
onActionandonOpenChange; gate them until the presentation completes or narrow the text. - The compact test's 744 px width contradicts
spec:AST-012FR9 and the compact definitionmainnow exports, and theplatformadaptation condition extends the vocabulary FR2 closes. - Land the stacked pull requests first and rebase, so this diff is only the AlertDialog motion commit instead of 31,567 authored lines.
[Automated review]
| // or an assistive-technology activation on an action, and Escape, do | ||
| // nothing until it is hidden. Focus stays where it is, so Dialog returns it | ||
| // to the opener when the fade ends. | ||
| const isLeaving = motion.isDriving && motion.isPresented && !isOpen; |
There was a problem hiding this comment.
This drops clicks, keys and Escape only while the alert leaves. During the entrance only the touch tracker consults the 435 ms window (:270), so a keyboard or assistive-technology activation of Delete still runs onAction, and Cancel or Escape still calls onOpenChange(false), while .changeset/alertdialog-touch-motion.md and the usage docs say nothing reaches the alert during either transition. Either gate these handlers on the same window as acceptsTouches() (with a keyboard-during-entrance test), or narrow the changeset, docs and file header to touches.
6dca4bf to
3b4bd41
Compare
…alog tracking base
There was a problem hiding this comment.
This pull request is on hold for an engineering-owner decision: AlertDialog's touch entrance and exit, dropping keys and assistive-technology activation while the alert leaves, and the alert.present and alert.dismiss rows added to AST-094's FR4 table have no current spec, because AST-094 leaves each overlay's presentation and exit to its own record and component:AlertDialog is still a draft. An engineering owner records the decision by stating it in the AlertDialog record and commenting /approve-spec <full head SHA>, or by approving a spec pull request that states it; the author may do this as an engineering owner. Independent of that decision, please:
- get
scripts/touch-support-bundle.test.mjspassing, since its root-entry case fails on this head (the description points to #8671 for that); - render the alert's opening state in the commit instead of repairing it in a passive effect (
useAlertMotion.ts:273); - add the WebKit busy-thread proof and the real-device evidence AST-094 asks for, or narrow the changeset's claim that the animation keeps running when the page is busy;
- land the stacked layers first so this diff carries only the AlertDialog change (33,172 changed lines today), and align the usage text, FR6 and the DEC-11 test with the code.
[Automated review]
| // dialog can stay on the main thread (motion/animator.ts, known limits); | ||
| // that case is not yet proven here. | ||
| transitionEndsAtRef.current = Number.POSITIVE_INFINITY; | ||
| panel.set({opacity: 0, scale: presentScaleFrom(elementRef.current)}); |
There was a problem hiding this comment.
The opening commit has already rendered the dialog at opacity: 1 with the theme keyframes off (AlertDialog.tsx:85-86 plus Dialog's open style), and these start values arrive only in this passive Effect, which architecture:react-component-runtime INV1 forbids. Rendering the start state in the commit, or writing it in a layout Effect, closes the window that an open from a timer or a network response leaves before paint.
| <div | ||
| data-astryx-theme="slow" | ||
| style={{ | ||
| ['--motion-springs-alert-present' as string]: '0.8s 0', |
There was a problem hiding this comment.
Since the rename the roles read --motion-alert-present and --motion-alert-dismiss (motion/roleValue.ts:29-36), so setting --motion-springs-alert-* here cannot make this test fail. Setting the roles' own variables makes it test DEC-11.
| | 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 keeps 400 under a finger with presentation="dialog" and honours an explicit width (AlertDialog.tsx:255-256); FR6 could say it sets the default and name that exception. The FR6 verification row below fills 2 of the table's 5 columns.
| 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. 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\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 still says a mouse keeps the click of each button, but the head tracks a mouse across the actions (the "tracks a mouse too" test), and the presentation description says the actions are tracked under every pointer.
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.
There was a problem hiding this comment.
This pull request is on hold for an engineering-owner decision. AlertDialog's touch entrance and exit, and its dropping of keys and assistive-technology activation while it fades out, rest only on the draft component:AlertDialog record, because current AST-094 leaves each overlay's presentation and exit to that record. The two alert rows added to AST-094's FR4 table also go against its DEC-5. An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request for the FR4 rows, and by approving the AlertDialog behavior here or making that record current; the author may do this as an engineering owner. Independent of that decision, please:
- get
scripts/touch-support-bundle.test.mjspassing: its root-entry case fails on this head and passes on main, and #8671 is still open; - land the parent stack first, so this diff carries only the AlertDialog change (33,046 authored lines today);
- add the WebKit busy-main-thread proof and the device evidence AST-094 FR5 asks for, or start the entrance in the next animation frame as
animator.tsadvises; - make the usage docs say the actions are tracked for a mouse too.
[Automated review]
| 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. 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\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 the actions are tracked "under a finger or a pen" and that "A mouse, the keyboard and assistive technology keep the click of each button", but useActionGroupTracking defaults to every pointer and AlertDialog passes no pointerTypes. The test "tracks a mouse too (family:action-groups FR5)" confirms it, and family:action-groups FR5 applies the tracking to every web pointer. A mouse press on Delete released over Cancel runs Cancel. Suggest: "Under every pointer, mouse included, the two actions are tracked as one group …; the keyboard and assistive technology keep each button's own activation." The dense text at line 199 needs the same change.
| if (now === 'open' && was === 'hidden') { | ||
| // The Dialog opened its modal in this same flush (its effect runs | ||
| // first), and nothing has painted it yet. The alert starts at its | ||
| // starting values and moves from rest with no start time: the browser | ||
| // starts it at the commit that first shows it, as Core Animation starts | ||
| // an animation at its transaction's commit, and the animator promotes | ||
| // it in this task. Chromium runs it on the compositor from that first | ||
| // frame through a busy main thread (the busy-thread browser proof). In | ||
| // iOS Safari an animation created in the task that first displays a | ||
| // dialog can stay on the main thread (motion/animator.ts, known limits); | ||
| // that case is not yet proven here. | ||
| transitionEndsAtRef.current = Number.POSITIVE_INFINITY; | ||
| panel.set({opacity: 0, scale: presentScaleFrom(elementRef.current)}); | ||
| backdrop.set({opacity: 0}); | ||
| const handle = present(panel, backdrop); | ||
| presentRef.current = handle; | ||
| armTouchWindow(handle); | ||
| return; |
There was a problem hiding this comment.
The presentation is created in the same passive-effect flush that runs Dialog's showModal(). motion/animator.ts records that in iOS Safari "an animation created in the task that first displays a dialog stays on the main thread: start it in the next animation frame", and the comment here says that case "is not yet proven". AST-094 FR5 requires the motion to keep advancing through a main-thread block, and its Platform support asks for a WebKit busy proof and device evidence. The alert's busy test skips WebKit, while the changeset promises the animation keeps running on a busy page. Could the presentation start in the next frame, with the StrictMode-safe cancel, plus a WebKit busy proof or a device clip?
| | `springs.alertPresent` | mass 1, stiffness 522.35, damping 45.71 | measured: UIAlertController alert appearing, iOS 26.2 simulator, 2026-10-23 | | ||
| | `springs.alertDismiss` | mass 1, stiffness 522.35, damping 45.71 | measured: UIAlertController alert dismissing, iOS 26.2 simulator, 2026-10-23 | |
There was a problem hiding this comment.
These rows put the alert's measured springs into AST-094's own FR4 table. FR4 and DEC-5 list only the roles this record names, and send roles a component's own record measures through the generated motion role table. This needs an engineering owner's decision (/approve-spec <full head SHA>), or the rows move to the AlertDialog record's measurement.
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.
There was a problem hiding this comment.
This pull request is on hold for an engineering-owner decision: AlertDialog's touch entrance and exit, and its dropping of keys and assistive-technology activation while it fades out, rest only on the draft AlertDialog record, because current AST-094 leaves each overlay's presentation and exit to that record, and the two alert rows added to AST-094's FR4 table go against its DEC-5. An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request for the FR4 rows, and by approving the AlertDialog behavior here or making that record current; the author may do this as an engineering owner. Independent of that decision, please:
- get
scripts/touch-support-bundle.test.mjspassing: its root-entry case fails on this head and passes at the base, and #8671 is still an open draft; - land the parent stack first, so this diff carries only the AlertDialog change (31,326 authored lines today);
- add the WebKit busy-main-thread proof and device evidence AST-094 FR5 asks for, or start the entrance in the next animation frame as
animator.tsadvises; - make the usage docs say the actions are tracked for a mouse too.
[Automated review]
| 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. 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\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 the actions are tracked under a finger or a pen and that "A mouse, the keyboard and assistive technology keep the click of each button", but useActionGroupTracking tracks every pointer by default (EVERY_POINTER) and AlertDialog passes no pointerTypes; the test "tracks a mouse too (family:action-groups FR5)" passes, and both changesets say "every pointer". A mouse press on Delete released over Cancel runs Cancel. Suggest: the two actions are tracked as one group under every pointer, mouse included; the keyboard and assistive technology keep each button's own activation. The dense description at line 199 says the same thing.
| transitionEndsAtRef.current = Number.POSITIVE_INFINITY; | ||
| panel.set({opacity: 0, scale: presentScaleFrom(elementRef.current)}); | ||
| backdrop.set({opacity: 0}); | ||
| const handle = present(panel, backdrop); |
There was a problem hiding this comment.
The entrance is created here, in the same effect flush as Dialog's showModal(). animator.ts:100-101 records that iOS Safari keeps "an animation created in the task that first displays a dialog" on the main thread and says to start it in the next animation frame, and the comment above (line 274) says this case "is not yet proven here". AST-094 FR5 and its Platform support ask for the busy-main-thread proof in WebKit and real-device evidence, and the busy test skips WebKit, while the changeset promises the animation keeps running on a busy page. Either start the presentation in the next frame (keeping the StrictMode-safe start) and add the WebKit proof and a device clip, or narrow the changeset's promise.
| | `springs.alertPresent` | mass 1, stiffness 522.35, damping 45.71 | measured: UIAlertController alert appearing, iOS 26.2 simulator, 2026-10-23 | | ||
| | `springs.alertDismiss` | mass 1, stiffness 522.35, damping 45.71 | measured: UIAlertController alert dismissing, iOS 26.2 simulator, 2026-10-23 | |
There was a problem hiding this comment.
These rows put a component's measured pair into AST-094's own FR4 table. FR4 says "further roles a component's own record measures join through the generated motion role table (DEC-5)", and DEC-5 says FR4 lists the roles this record names and cites the generated table for the roles a component's record measures. This is a normative change to a current record, so it needs an engineering owner's decision (an exact-head /approve-spec) or the rows should move to the alert's own record and the generated table.
Depends on #8083. Review only this PR's change: vjeux/astryx@458e2d9...19af775
Why
On a phone, AlertDialog plays the theme's desktop entrance: it rises 16 px and grows from 95% as the dimming pops on, and it vanishes in one frame when closed. A tap during the entrance already runs a button. The iOS alert shrinks into place, ignores touches until it settles, and fades out.
What led to it
The UIKit-parity work, which brings Astryx's touch behavior to iOS's, measured the iOS 26.2 alert (
motion/uikit/alert.ts), andspec:AST-094asks every touch device to present it that way (FR4, FR13, FR35). Cindy's AlertDialog record (#8594) now specifies it: FR6 (the motion), FR7 (input held while it enters) and FR9 (320 px on a phone). This PR implements FR6 and FR9; #8373 completes FR7 on the shared input hold (#8342).Options and tradeoffs
@starting-style. Simple, but no spring (no UIKit curve, no turning around mid-way), and Safari can't animate the exit withoutoverlay.scaleandopacitykeyframes (the::backdrop's too). Smooth on a busy page and interruptible, at the cost of a narrower alert on phones (320 vs 400).Why this choice
Option 3 is the only one that matches UIKit and stays smooth. Under a finger the alert follows the
springs.alertPresentandsprings.alertDismissroles, ignores touches on its actions for 435 ms from the start, runs nothing while it leaves, and cross-fades under Reduce Motion. A mouse keeps today's behavior. As her record says, the public API doesn't change: there is no opt-out prop, and awidththe caller sets still wins on a phone.spec:AST-108) aren't on main yet (feat(core): the animator's springs follow the theme's tempo and character #8677, feat(core): a CSS part times itself from its spring's settle at the theme's knobs #8686). Once they land, the alert's springs, touch window and exit backstop will follow them.How to check it
On a phone or the iOS simulator, in the
Core/AlertDialogDelete story:Steps 1 to 4 were run on the iOS 26.2 simulator (step 1 also on the Android 15 emulator), and step 5 in the browser and unit tests: each behaved as written. This head's browser tests passed in Chromium (24) and WebKit (20, plus 4 Chromium-only cases skipped by design).
Evidence
iOS 26.2 simulator, Mobile Safari, main on the left (present and dismiss storyboards):
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).Writing rubric results
spec:AST-094FR4's role table gains the alert's two springs,springs.alertPresentandsprings.alertDismiss. FR4's table renamed to match AlertDialog FR6.