Skip to content

feat(AlertDialog): under a finger the alert appears and leaves the way iOS's does - #8084

Draft
vjeux wants to merge 12 commits into
meta:mainfrom
vjeux:uikit/c18-alerts-pr2
Draft

vjeux wants to merge 12 commits into
meta:mainfrom
vjeux:uikit/c18-alerts-pr2

Conversation

@vjeux

@vjeux vjeux commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Stack: #8084 → #8095

This pull request lands before #8095, which builds on it. #10086, now on main, keeps a toast the alert's action raises from entering twice when the alert closes; #10223, also on main, keeps the alert's moves from starting a transition on that toast; and #10251, also on main, hands that toast back in the commit that closes the alert, so no frame paints without it.

Behavior module: AlertDialog's touch motion (useAlertMotion, in packages/core/src/AlertDialog/useAlertMotion.ts), with the Layer input hold it uses (packages/core/src/Layer/inputHold.ts).

User need

On a phone, AlertDialog plays the theme's desktop entrance: it rises 16 px and grows from 95% while the dimming pops on, and it closes at once. The iOS alert shrinks into place from 1.2 while it and the dimming fade in, and fades out to close. Main already holds a tap during the entrance, through Dialog's input hold.

Problem and solution fit

  1. Look (FR6): under a touch presentation the alert scales from 1.2 and fades in with the dimming, on Dialog's entrance tokens, and leaves by a fade alone on the exit tokens. Under Reduce Motion both fades take at most 200 ms.
  2. Leaving (FR7): while the alert fades out, it holds its input with the Layer input hold that Dialog's entrance already uses. Its content is inert and the hold's shield takes every press, so a tap, a click, a key or an assistive-technology activation does nothing, and focus leaves the actions until it returns to the opener. Escape is dropped until the alert is hidden. A reopen during the fade gives the content back, with focus on the action that had it. A toast stays live meanwhile: the toast viewport, which lives in the latest open modal, stays out of the inert content and goes back above the hold's shield, so a toast the action raises takes a tap and is read during the fade (spec:AST-003 FR16).
  3. Width (FR9): on a phone the alert is 320 px wide in every theme, so its 1.2 start stays on screen.

Current specification

component:AlertDialog FR6, FR7 and FR9, with DEC-4: an alert's motion is not a gesture, so it plays the theme's tokens on every pointer (spec:AST-094 FR45; Specs decision, 2026-11-02). The touch exit plays even with the dialog exit switch off (spec:AST-090 FR14, spec:AST-094 FR7). An alert open in server markup opens at rest (spec:AST-013 FR13). FR6's turn-back from mid-transition is a recorded gap with its own pull request; With #8342 merged, Dialog holds pointer input while the alert enters, and the leaving alert holds its input with the same Layer hold, so FR7 ships whole. AlertDialog keeps no input gate of its own, apart from an Escape guard. The toast viewport stays visible and operable above the current native modal (spec:AST-003 FR16, FR18), also while the hold keeps a leaving modal's content inert; architecture:layer-runtime gains that sentence. It builds on #9706, #9771 and #8342, all merged.

Public delta and caller burden

No API change. Under a touch presentation the look, the held input while the alert leaves and the phone width change. A mouse keeps today's motion, and a caller's width still wins. The private modal-outlet registry gains a mark for global hosts; no public export changes.

Design and alternatives

AlertDialog gives Dialog its own keyframes under a touch presentation, so the browser plays them on the compositor. The dimming inherits the alert's timing (a backdrop inherits from its dialog), which also carries Reduce Motion's 200 ms. The hook closes the dialog at the fade's end, and holds the dialog's input with the Layer module's layer-scope hold until then. Inert content does not stop the dialog's own cancel event, so AlertDialog drops Escape while it leaves.

The shared hold keeps the toast viewport live, so every caller of a dismissal hold gets it, not only AlertDialog. ToastViewport marks the host it moves into the latest open modal. A dismissal hold leaves marked hosts out of the inert content, and its press hold and the gesture host's gate leave a press on them alone; a presentation's hold still holds a press anywhere, as on main. The hold's shield is a top-layer element shown after the toast viewport, so the hold then sends the top-layer entry signal that a non-modal sheet already sends, and the toast viewport re-enters the top layer above the shield. Holding a narrower element inside the dialog would not do it: the shield still covers the viewport from the top layer, and the press hold listens on the window. The hold's and the alert motion's node checks are aligned with #10224's realm-safe guards (isNode, isElement, isHTMLElement): a toast in a dismissing alert in a same-origin iframe stays live, and a reopen there gives the action its focus back. Same-document behavior is unchanged.

Evidence

  • Red first, on the previous head's code: Reduce Motion's dimming took 400 ms; an alert open in server markup replayed its entrance after hydration. The new case "while the alert fades out its content is inert: focus leaves the actions, Tab cannot reach them, and focus returns to the opener" fails on the head before the exit hold.
  • Red first for the toast: the browser case "while the alert fades out, a toast its action raised stays live above the hold: a tap on it runs" uses the new "Alert over a toast" story and a 2 s exit. Once the toast rests, it taps the toast's Undo during the fade, and expects the tap to count and the toast's click to come before the dialog's close event. It fails 5 of 5 on a build whose hold makes the toast inert, and 5 of 5 on a build whose hold leaves the toast live but still holds its press. It passes on this head. The three unit cases (a dismissal hold leaves a marked host live and announces the entry after its shield; it leaves a press on the host alone while a presentation's hold still holds it; a hosted toast viewport stays live and is shown again after the shield) fail with the previous input hold.
  • The 1 s main-thread block, judged against the harness's compositor control: 12 of 12 in Chromium pinned to 2 cores with 4 workers, where a frame-rate judgement failed 6 of 12. Motion moved onto the main thread fails it (a local probe).
  • iOS simulator (Safari, iOS 26.2), 1 s block during the entrance and the dismissal: the alert changed on 57–60 of 59–61 frames per block. The black square is the page's main-thread clock.
  • The spec owner's real-iPhone pass at 45e9de5 (the scale-in, the close fade, Reduce Motion and a tap during the fade) passed. The alert over a toast is being checked again on a phone at this head.
  • iOS 26.2 simulator (iPhone 17 Pro), Mobile Safari, at b86bb05, 60 fps recordings with one session per flow and no reload: Delete item, then Delete, five times (the toast updates in place): one slide-in, then the card in every one of 2,172 frames at the same place. A toast dismissed with its close button between alerts, three times: each new toast slides in once. Three plain toasts: each slides in once while the older ones move up. A toast raised inside a Dialog after the viewport emptied, twice: each slides in once inside the dialog. No frame lacks a shown toast in any of them.
  • Android: Chrome 133 with Gboard on an Android 16 emulator (Pixel, 1080×2400), recorded at 058b372. useAlertMotion.ts is byte-identical between 058b372 and this head. The input hold's inert set and the toast viewport's top-layer order changed since, and AlertDialog.tsx now passes onAction straight to the button, so a recording at this head, with the new story, is pending. The recording at 058b372 covers the normal profile, Remove animations and a slow profile (4× CPU throttling). Every frame was logged with the dialog's opacity, scale and position, the dimming's opacity, and each pointer event:
    • Entrance (6 takes): the alert starts at scale 1.2 and settles at 1 by 338–405 ms, its opacity and the dimming's rise together, and it never translates. A tap on Cancel's spot 62–112 ms into the entrance lands on the input hold's covering element, not on Cancel: no click reaches Cancel, and the alert stays open in 6 of 6.
    • Cancel (8 takes): the alert closes 179–277 ms after the tap, at scale 1 with no translate in every frame. Its opacity and the dimming's stay within 0.05 of each other in every sampled frame, and focus returns to "Delete item" in 8 of 8.
    • Remove animations (2 takes): scale 1 in every frame, so a fade alone; it closes 179–186 ms after the tap.
    • Taps on Delete and on Cancel again, right after Cancel (6 takes) or in the middle of the fade (2 takes): they land on the hold's covering element, no click reaches either button in 8 of 8, and the alert closes as cancelled.
    • Slow profile: two of the Cancel takes ran at 4× CPU throttling. Takes whose frame timing failed a validity gate (a frame gap over 100 ms) were not used.

Scope

  • One capability is added.
  • Unrelated fixes, cleanup, and policy changes were removed or split.
  • Consumer docs, focused tests, and migration evidence ship with the feature.
  • Public text and artifacts contain no internal Meta context.

Testing

This head (362ea7f) merges main into 773374e, with #10251, and adds two commits: this change's own node checks aligned with #10224's realm-safe guards, with same-document behavior unchanged, and a check that a toast the alert raises stays painted when the alert closes. The merge leaves this change's own lines in its 16 files as they were at 773374e. Gates at this head, on main 9a8d14d:

  • ESLint on the 16 files (no errors), Prettier, pnpm build, pnpm typecheck, Storybook, the Layer, AlertDialog, Toast, Dialog, gestures, hooks and ButtonGroup units (2,757) and AlertDialog's accessibility contract (5) pass.
  • Both AlertDialog interaction specs, the toast viewport's move spec, Dialog's touch entrance, the input hold's browser spec and the gesture spec: Chromium twice, 123 passed each (4 Chromium cases skipped by design); WebKit in CI's container twice, 221 passed with 32 skipped. One WebKit run of main's own case "a new toast plays its entrance once" failed under machine load. It fails on main at the same rate (5 of 40 runs there and 4 of 40 here, at the same moment), and test(Toast): the entrance cases hold on a busy runner #10268 fixes it.
  • A new case, "a toast the alert raises stays painted when the alert closes: it leaves the alert in the commit that closes it", counts each DOM change after which the toast's card sits in a dialog, and each one after which that dialog no longer displays, across three alerts. Without fix(Layer): toasts leave a closing modal in the commit that closes it #10251, a change leaves the card in a hidden dialog on every alert: it fails 3 of 3 runs in Chromium and in WebKit. At this head it passes 3 of 3 in each.
  • The new realm-safe checks have a case each, in a same-origin iframe: with either check reverted, its case fails and the other passes.
  • iOS 26.2 Simulator (iPhone 17 Pro), Mobile Safari, at this head, 60 fps recordings: three alerts over a toast, and three saves that close a Dialog after an awaited save. The toast is on every frame from its entrance on, with one entrance per take.

Local gates at c7f3449, on main b9e3dc2. That head merges main and takes its press-group marker (through the group's context) into AlertDialog.tsx; the other 15 files are unchanged:

  • CI=true pnpm lint (no errors), the build, pnpm typecheck, Storybook, the AlertDialog, Toast, Layer, Dialog, hooks and ButtonGroup units (1,876) and AlertDialog's accessibility contract (5) pass. Both AlertDialog interaction specs, the toast viewport's move spec and Dialog's touch entrance: Chromium twice, 33 of 33 each; WebKit in CI's container twice, 62 passed (the 4 skipped cases are Chromium-only).

The same change at dd1c162, on main 813c58c:

  • Both AlertDialog interaction specs, Dialog's touch entrance, the input hold's browser spec, the gesture and presented hit-testing specs, the pager spec and the toast viewport's move spec: Chromium twice and pinned to 2 cores three times, 145 of 145 each; WebKit in CI's container five times, 660 passed (the 95 skipped cases are Chromium-only). AlertDialog's accessibility contract and the bottom sheet's and the drawer's toast outlet contracts (11) and the hooks, AlertDialog, Dialog, Layer, Toast, BottomSheet, Drawer and gestures units (3,232) pass.
  • CI=true pnpm lint (no errors), the build, pnpm typecheck, Storybook, pnpm test:ui (21,300), pnpm test:node (16,742), check:knowledge, check:repo, specs:for --diff and review:pre pass. test:node ran with plain git first on the path, as CI's runners have it: with the devserver's git wrapper on a loaded box, three files this change does not touch timed out (a CLI text and JSON parity hook, the knowledge check's delegation case and the search ranker's whole-name case), and with plain git they pass on this head and on main alike, in three paired rounds on two cores and in a full run of both at once. api:report --check reports drift already on main, none of it from this change.
  • A frame without the toast can appear right after the alert closes. The cause is the modal outlet on main, not this change or the test environment. The commit that closes a Dialog stops displaying it, and the toast's viewport left the dialog only in a later effect, after the browser could paint. With Chromium's CPU throttled 4×, 9 of 10 fresh sessions show the frame; in Linux WebKit, 2 of 8 closes in a row do. fix(Layer): toasts leave a closing modal in the commit that closes it #10251 moves the viewport out in the commit that closes the dialog, and with it both counts are 0. This head includes it.
  • A new case, "a toast the alert raises enters once: repeated alerts move it in and out, and it starts no transition", records every transition or animation that starts on the toast's row or card across three alerts. The first alert's entrance shows up, and the later alerts start nothing. At d0f8d96 it passes three runs out of three in Chromium and in WebKit; on b86bb05, before refactor(Toast): a moved toast's card runs no hidden transition #10223, it fails three out of three in each (two card transform starts per alert). At d0f8d96 the AlertDialog, toast viewport and Dialog entrance specs pass (Chromium twice, 35 of 35; WebKit in CI's container twice, 66 passed), as do AlertDialog's accessibility contract, the build and pnpm typecheck.
  • Two tests follow main's input hold. Main's pen-selection case now waits for the alert to enter, because a press during the entrance is dropped (FR7). The motion spec's tap probe listens on the window before the page's scripts, because the hold stops a held touch there.
  • The inert case counts focus inside the alert's content, not on the dialog element: WebKit's Tab can focus the dialog element itself, which holds no action.
  • The case "a tap while the alert presents or leaves reaches nothing" checks first that its tap landed inside the entrance. It now holds the entrance's animation from its start, because a finished animation leaves the document's list and a slow runner read nothing there. It also gives the entrance 2 s, as it already gave the exit, so a tap can land inside it on any runner. On one CPU (4 workers, 10 runs a round, order-balanced): the previous version failed 16 of 20 at that check; with the hold alone the check failed 16 of 20 with real times (the tap reached the page 480–550 ms into a 400 ms entrance); with the 2 s entrance alone, and with both, 20 of 20 passed. On two CPUs the hold alone failed 1 of 24 (a tap at 470 ms). Both parts ship: the hold makes the check read real times, and the 2 s entrance makes the case possible on any runner. No assertion or timeout changed.

Writing rubric results

component:AlertDialog: FR6 plays the theme's tokens, with the exit fade on even while the dialog exit switch is off; its turn-back clause carries an implementation gap, listed under Current-state impact. FR7 ships whole: its gap sentence and its Current-state entry are gone, and its verification row names the entrance and exit cases with all five cells. DEC-4, readiness, system_specs, Governing rules and the FR6/PR2 verification row match what ships. No motion role is added.

  • R1 narration, R2 research, R4 rejected alternatives, R5 change references: none.
  • R3 evidence: the look's source is the measurement (UIAlertController, iOS 26.2); its timing's is DEC-4.
  • R6 open questions: none touched.

architecture:layer-runtime: one addition to the modal-outlet paragraph, in the present tense: an input hold can make a leaving modal's content inert, the host stays live, and the hold announces its shield's entry, so toasts stay operable above the shield. It names what the code does, with no change history.

  • R1 narration, R2 research, R4 rejected alternatives, R5 change references: none.
  • R3 evidence: the claim is verified by ToastViewport.modalHost.test.tsx, already in the record's verified_by.
  • R6 open questions: none touched.

@vercel

vercel Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Nov 6, 2026 12:48pm UTC

Request Review

@astracat-bot

astracat-bot Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

AI review status for this pull request.

Review status Updated
🟡 Reviewed (for maintainers only) Nov 6, 2026, 1:00 PM UTC

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 24, 2026

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 calls onOpenChange(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 adaptive threshold change contradicts component:Selector DEC-1; split it out or keep the current query.
  • The lint job 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}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This change comes from the touch presentation resolver (#8062), which this pull request reaches through #8076; it is not part of this pull request's own commit. The Selector DEC-1 question belongs to #8062 and waits in its reviewers' queue.

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

Current run: The current CI run concluded skipped. Current analysis is unavailable.


Generated by PR Enrichment workflow | View current CI run

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 onAction and Escape calls onOpenChange(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 adaptive threshold change contradicts component:Selector DEC-1; split it out or keep the current query.
  • The lint job runs out of memory on this head while the base passes.

[Automated review]

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 calls onOpenChange(false) again; make the leaving alert inert.
  • The adaptive threshold change contradicts component:Selector DEC-1; split it out or keep the current query.
  • The lint job runs out of memory on this head while main passes.
  • 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]

vjeux added a commit to vjeux/astryx that referenced this pull request Oct 24, 2026
@vjeux
vjeux force-pushed the uikit/c18-alerts-pr2 branch from b635c03 to 1c55503 Compare October 24, 2026 21:29
@vjeux vjeux changed the title feat(AlertDialog): on a touch screen the alert appears and leaves the way iOS's does feat(AlertDialog): on iPhone and iPad the alert appears and leaves the way iOS's does Oct 24, 2026

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 onAction and Escape calls onOpenChange(false) again; make the leaving alert inert.
  • Under Reduce Motion the iOS fades last 404 ms on a spring, but design:reduced-motion DR2 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-012 FR9 (the theme's md point or a height under 500), and the platform adaptation 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}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed at 7c77707: while the alert fades out, a click on an action or Cancel, Enter, an assistive-technology activation and Escape do nothing (isLeaving guards the three handlers). AlertDialog.test.tsx covers it.

const handle = panel.animate(
{opacity: 1, scale: 1},
{spring: springs.alertPresent},
{crossfade: springs.alertPresent},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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)';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@vjeux
vjeux force-pushed the uikit/c18-alerts-pr2 branch from 1c55503 to 6dca4bf Compare October 24, 2026 22:07
@vjeux

vjeux commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Addressed at 6dca4bf:

  • While the alert leaves, a click, Enter or assistive-technology activation on an action, Cancel and Escape do nothing (new failing-before case).
  • StrictMode mount-open case added; the presentation no longer waits for an animation frame.
  • lint passes on this head (the out-of-memory run was on b635c03).

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 adaptive query, which belongs to #8062. This pull request carries its stack until it lands; review the last commit.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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-motion DR2 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 onAction and onOpenChange; gate them until the presentation completes or narrow the text.
  • The compact test's 744 px width contradicts spec:AST-012 FR9 and the compact definition main now exports, and the platform adaptation 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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@vjeux
vjeux force-pushed the uikit/c18-alerts-pr2 branch from 6dca4bf to 3b4bd41 Compare October 24, 2026 23:31
@cixzhang

cixzhang commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 62c9b4a

1 similar comment
@cixzhang

cixzhang commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 62c9b4a

@vjeux vjeux mentioned this pull request Nov 5, 2026
4 tasks done
@cixzhang

cixzhang commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

/approve-spec dd1c162

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes: the touch entrance and fade-out, the inert leaving alert with toasts kept live above it, and the 320 px phone width match the AlertDialog record an engineering owner approved for this head, and a mouse on a desktop sees no change.

  • The owner's real-device iOS Safari pass and the Android Chrome emulator recording (Chrome with Gboard: default, Remove animations and a slow profile), which the FR6/PR2 verification row and spec:AST-094 Platform support and DEC-27 require at the exact head, are still missing; the iPhone pass and the Android recording linked in the description were taken on earlier commits.

Once both are posted for the head you intend to merge, nothing else blocks; CI has not run yet because the pull request is a draft.

[Automated review]

@cixzhang

cixzhang commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

/approve-spec c7f3449

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes: the touch entrance and fade-out, the inert leaving alert with toasts kept live above it, and the 320 px phone width match the AlertDialog record an engineering owner approved for this head, and a mouse on a desktop sees no change.

  • The owner's real-device iOS Safari pass and the Android Chrome emulator recording (Chrome with Gboard: default, Remove animations and a slow profile), which the FR6/PR2 verification row and spec:AST-094 Platform support and DEC-27 require at the exact head, are still missing; the runs in the description were taken on earlier commits.

Once both are posted for the head you intend to merge, nothing else blocks. CI has not run yet because the pull request is a draft.

[Automated review]

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approval waits on an engineering owner: component:AlertDialog and architecture:layer-runtime are edited at this head, and the spec approval given on the previous commit does not carry over to it. This head only merges main, and the record text is the same as the text approved before, so an engineering owner (the author included, as an engineering owner) records the decision by commenting /approve-spec <full head SHA> on this pull request.

  • The owner's real-device iOS Safari pass and the Android Chrome emulator recording (Chrome with Gboard: default, Remove animations and a slow profile), which the FR6/PR2 verification row and spec:AST-094 Platform support and DEC-27 require at the exact head, are still missing; the runs in the description were taken on earlier commits.

The touch entrance, the inert fade-out with toasts kept live above it, and the 320 px phone width behave as the record describes in Chromium, and a mouse on a desktop sees no change. CI has not run yet because the pull request is a draft.

[Automated review]

@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec b86bb05

A top-layer ToastViewport moves its host into the latest open modal and
back out. Each move renders the toast's card again, and the card's
starting style applied once more. So a toast already on screen started
a transform transition at every move: a Dialog, a modal Drawer or a
scrim sheet that opened or closed over it.

The card's starting style now applies only until the toast's row starts
to open, as the row's own does, and the row passes that state to the
card. A move then renders an entered card at rest and starts nothing.
This replaces the two private start properties the row set on the card,
so they leave the docs and the registry's list too.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes. The AlertDialog touch look, the inert fade-out with toasts kept live above it, and the 320 px phone width match the owner-approved component:AlertDialog record and behave as described in Chromium, but this head also merges #10223 and the exact-head device runs are still missing.

  • Drop the merge of #10223 from this branch and land that Toast fix through #10223: it is a separate change to every modal's toast host, and it takes this pull request from 1,993 to 2,124 authored lines, past the 2,000-line scope limit.
  • .changeset/toast-moved-card-still.md says a toast "no longer restarts its card's slide", but on main a moved card already stays still (offset 0 px, identity transform and full opacity on every frame; only a transition between identical values starts), so that wording should say what actually changes when it lands in #10223.
  • The owner's real-device iOS Safari pass and the Android Chrome emulator recording (Chrome with Gboard: default, Remove animations and a slow profile), which the FR6/PR2 verification row and spec:AST-094 Platform support and DEC-27 require at the exact head, are still missing.

CI has not run because the pull request is a draft.

[Automated review]

Comment thread .changeset/toast-moved-card-still.md Outdated
'@astryxdesign/core': patch
---

[fix] A toast already on screen no longer restarts its card's slide when a Dialog, a modal Drawer or a scrim sheet opens or closes over it. Its card now enters only once, as its row does.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On main nothing visibly restarts here: once a row has started to open it sets the card's starting values to opacity 1 and a 0 px slide, so a move starts a transform transition between identical values. A per-frame probe across a Dialog close on main keeps the card's offset at 0 px, its transform at identity and its opacity at 1, and the host-move spec run against main fails only on its list of started transitions. Suggest saying what changes, for example: "A toast's card no longer starts a CSS transition when a Dialog, a modal Drawer or a scrim sheet opens or closes over it; it already stayed still." This belongs with #10223 once its merge leaves this branch.

@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec d244534

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes. The AlertDialog touch look, the inert fade-out with toasts kept live above it, and the 320 px phone width match the owner-approved component:AlertDialog record and behave as described in Chromium and WebKit, and the Toast change from #10223 has left this pull request now that #10223 is on main.

  • The owner's real-device iOS Safari pass and the Android Chrome emulator recording (Chrome with Gboard: default, Remove animations and a slow profile), which the FR6/PR2 verification row and spec:AST-094 Platform support and DEC-27 require at the exact head, are still missing; post both for the head intended to merge.

CI has not run because the pull request is a draft.

[Automated review]

Repeating "Delete item", then "Delete", moves the toast the alert's
action raised into each alert and back out, and the action updates the
toast in place. The case records every transition or animation that
starts on the toast's row or card. The first alert's entrance shows up,
and the later alerts start nothing.
@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec d0f8d96

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes. The AlertDialog touch look, the inert fade-out with toasts kept live above it, and the 320 px phone width match the owner-approved component:AlertDialog record and pass their browser cases in Chromium and WebKit at this head, including the new case that a toast the alert raises enters only once.

  • The owner's real-device iOS Safari pass and the Android Chrome emulator recording (Chrome with Gboard: default, Remove animations and a slow profile), which the FR6/PR2 verification row and spec:AST-094 Platform support and DEC-27 require at the exact head, are still missing; post both for the head intended to merge.

CI has not run because the pull request is a draft.

[Automated review]

The header's second @input sentence ran to 26 words with no verb. It
now reads as two sentences, each with its verb.
@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 773374e

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes. The AlertDialog touch look, the inert fade-out with toasts kept live above it, and the 320 px phone width match the owner-approved component:AlertDialog record; the pull request is unchanged since the last review.

  • Regression: main moved Layer/inputHold.ts to realm-safe node checks (utils/nodeType) and this head conflicts with it, so merge main and write the new checks in isInGlobalHost (inputHold.ts:118, :122) and in useAlertMotion.ts:177 with isNode, isElement and isHTMLElement, so the toast-host exemption also holds for a toast in another window's document.
  • The owner's real-device iOS Safari pass and the Android Chrome emulator recording (Chrome with Gboard: default, Remove animations and a slow profile), which the FR6/PR2 verification row and spec:AST-094 Platform support and DEC-27 require at the exact head, are still missing; post both for the head intended to merge, which after the merge is a new head.

The non-blocking INV29 suggestion on the AlertDialog.tsx file header still stands. CI has not run because the pull request is a draft.

[Automated review]

Comment thread packages/core/src/Layer/inputHold.ts Outdated
Comment on lines +118 to +122
let node: Node | null = target instanceof Node ? target : null;
node != null;
node = parentAcrossShadow(node)
) {
if (node instanceof Element && isModalOutletHost(node)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Regression: since the last review, main moved this file's node checks to isNode and isElement from utils/nodeType, so a layer's input hold reads nodes from another window's document (a same-origin iframe or a Picture-in-Picture window), and this head now conflicts with that change at the imports. After the merge these two instanceof checks would be the hold's only realm-bound ones: a toast row created in another window's document fails target instanceof Node, so holdsTouch and holdsPressAt would hold it during the alert's fade (spec:AST-090 FR13). Merge main and write them as isNode(target) and isElement(node), and the refocus check in useAlertMotion.ts:177 as isHTMLElement(focused).

Comment on lines +7 to +12
* @input Uses React, StyleX, Dialog, Layout, Heading, Text, Button and the
* touch presentation resolver. It also uses the action-group tracking
* (hooks/useActionGroupTracking), the press-group context
* (hooks/pressGroupContext) and the group pointers
* (hooks/useGroupPressTracking). It takes its touch motion from
* useAlertMotion and the alert entries of the UIKit constants table

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking (INV29): with this rewrite the file header is 18 lines (15 on the base), past the record's "about 12 lines at most", and @input spans six lines. One sentence of names keeps it short:

 * @input React, StyleX, Dialog, Layout, Heading, Text, Button,
 *   useTouchPresentation, the action-group hooks, useAlertMotion and alertController

vjeux added 3 commits November 6, 2026 04:10
A leaving alert's hold left a press on a toast alone only when the
press landed on a node of this window, and the alert gave its blurred
action focus back only in the same way. In a same-origin iframe, an
instanceof check against this window's classes fails for the frame's
nodes. Both now use the realm-safe node checks from meta#10224, so a toast
in a leaving alert takes a tap in an iframe too, and a reopen gives
focus back there. Same-document behavior is unchanged.
The alert closes when its exit ends, outside the tap. A new case
counts each DOM change after which the toast's card sits in a dialog
that no longer displays, across three alerts. It must be none.
@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 362ea7f

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes. The AlertDialog touch look, the inert fade-out with toasts kept live above it, and the 320 px phone width match the owner-approved component:AlertDialog record, and the realm-safe node checks from main now reach the new code in Layer/inputHold.ts and useAlertMotion.ts.

  • The owner's real-device iOS Safari pass and the Android Chrome emulator recording (Chrome with Gboard: default, Remove animations and a slow profile), which the FR6/PR2 verification row (AlertDialog.spec.md:233) and spec:AST-094 Platform support and DEC-27 require at the exact head, are still missing for this head.

The non-blocking INV29 suggestion on the AlertDialog.tsx file header still stands. CI has not run because the pull request is a draft.

[Automated review]

This branch was successfully deployed

1 active deployment
Preview — 362ea7f4 Deployed Nov 6, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. needs:design-review Affects visuals — Design should review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants