refactor(MobileNav): keep the closing dialog rendered through the layer runtime's presentation hold by cixzhang · Pull Request #8590 · meta/astryx

cixzhang · 2026-10-26T20:47:55Z

Follow-up to #8587 (the layer runtime's presentation hold, merged). The base is now main, so this diff is MobileNav's adoption alone.

What it does

MobileNav keeps its dialog rendered from the commit that closes it until its delayed close() has run (WebKit drops a closing modal <dialog> to display: none at once otherwise, leaving the page inert with nothing visible). That close hold (isCloseHeld with a wasOpen latch) now uses the layer runtime's presentation hold (Layer/presentationHold.ts, architecture:layer-runtime INV14). The hold is taken during render as MobileNav opens, so it's already held in the commit that closes. It's released after close() runs, or when nothing is open, exactly where the old hold was cleared. The drawer's transform and backdrop stay keyed to isOpen.

No behavior change

MobileNav's tests are the oracle and pass unchanged (78), including the WebKit close-hold cases. The real-browser check is MobileNav's specs, which this PR's CI runs because the change touches the MobileNav folder.

Size (gzip KB, minified)

The same method as #8587: a fresh packages/core dist at the base and at this head; esbuild bundle, minify, splitting, ESM, packages external.

Component	Desktop	Phone
MobileNav	35.24 → 35.33 (+0.09)	36.11 → 36.20 (+0.09)

The shared hold is about 0.1 KB, loaded once with Popover's.

Overlap

#8374 (MobileNav's touch close timing) adds a second close timer that clears the old hold. It lands first; this pull request then merges it up, and its extra release point becomes releasePresentation(). #8374 needs no change for this one.

Testing

pnpm vitest run --project ui packages/core/src/MobileNav: 78 pass.
tsc and eslint clean.
[chore] changeset.

Retarget (@cixzhang)

#8587 merged, so the base moved to main and main is merged in at b6f4a72 (no force-push; the earlier stack history, including @vjeux's commits, stays in the branch). Outside MobileNav the merge takes main. MobileNav takes main's version, including the shared scroll lock, with its close hold moved onto usePresentationHold. MobileNav, AppShell and presentation-hold suites pass (190 tests).

The host read getComputedStyle(element).touchAction at a recognizer's first attach, to keep a caller's own touch-action in force; inside a mount that forces a style recalc over everything just mounted (C21's 200-row year wheel: 9.2 ms at 1x, rows written afterward recalc again). Failing on this head: attaching calls getComputedStyle, and a caller's stylesheet value is intersected at once. A caller's inline value applies at once with no read; a stylesheet's is checked after the frame has painted.

A caller's inline touch-action is the style's own, read with no recalc, and applies at once. A stylesheet's needs a computed read, which now waits until the next frame has painted, when styles are clean, instead of running inside the attach, where it forced a style recalc over whatever had just mounted. The check is batched: the host's own values come off every pending element, one read serves them all, and the writes follow. Until it runs (a frame or two after the element mounts) the host's own value stands, so a caller's narrower stylesheet value can read wider for that long; no touch starts on an element in the frame it mounts in.

…n attach) into #8315

…on clock and Reduce Motion plan The motion README takes main's internal-roles sentence with this layer's animator; uikit.test.ts takes main's version (rows, docAbove) and keeps the check of values bound from an exported group, which timing.ts's Reduce Motion fade bindings need.

architecture:react-component-runtime INV3 asks every Effect to name the external system it synchronizes: the listener ref, the recognizer's target-action listener and isEnabled, the host attachment, the tracker's options, tracking expansion, press consumer and enabled state, the unmount flag, and the attachment's node.

… proof describe the roles' table values (DEC-11)

…reader architecture:react-component-runtime INV11 allows the keyboard module at most one layout read per keyboard change. `getMobileKeyboardHeight` measured again on every call, and the layout guide, each component's `useKeyboardHeight` and ChatLayout's commit all call it after the module has already measured the report: one show read the visual viewport's height 6 times in Full AI Chat in Chromium, 13 times in the module's test with the guide, a listener and two hooks. While the module listens, `getMobileKeyboardHeight` returns the height it measured at the last report. Every change of the visual viewport or the window is a report, so a reader between two reports still sees the current height; with nothing listening yet, it measures. Tests: the module's test counts one read for a report with the guide, a listener and two hooks reading the height (red before: 13), and an interaction test counts one read for a show in Full AI Chat (red on the previous build: 6).

…is focused without the reveal spec:AST-099 FR4 says every route into the composer's text, a tap included, focuses it without scrolling the page. The tap route claimed only contenteditable text: a `<textarea>` or `<input>` passed as ChatComposer's input (its documented plain textarea) was left to the browser, and with nothing focused no blur reaches the focus takeover, so Safari revealed the field by scrolling the document. The tap route now claims an unfocused text control in the composer that brings up a keyboard, and delivers it as it delivers contenteditable text: focused with `preventScroll`, with the caret where the standard `caretPositionFromPoint` maps the point inside the control, or after its text where the browser maps none. A focused control keeps its own taps (the browser moves the caret); a checkbox or an `inputmode="none"` field is never claimed. Tests (useChatKeyboard.test.tsx): ChatLayout with a textarea composer, under a coarse pointer, cancels the tap's default and focuses the textarea with `preventScroll` (red before: the browser's own focus); the claim takes an unfocused text control and leaves a focused one, a checkbox and an `inputmode="none"` field; the caret lands at the mapped offset, or after the text.

…d Chrome it moves once reported spec:AST-099 FR8 asks a ride within one frame of the keyboard on the measured keyboards, with Android emulator evidence against Gboard; Firefox for Android has FR2–FR5 only, and reports the keyboard once it is up. This branch plays UIKit's keyboard spring from the browser's report there, and times no ride to Gboard. The docs said the composer rides in step in Firefox for Android and rides the keyboard in Chrome on Android. ChatLayout's docs (English, Chinese, dense), the browser-support guide and the changeset now say: in Safari on iOS the composer rides on top of the keyboard in step with it; in Firefox for Android and Chrome on Android it moves above the keyboard once the browser reports it; under Chrome's `interactive-widget=resizes-content` it stays above it without moving.

…es a dip lands its move with it Under Reduce Motion on a touch screen: - A move over a stylesheet opacity of 0 dips nothing (it lands at once), but kept 0 as a borrow that outlived it, so a later move once the element showed read it as invisible and landed in view. A borrow now exists only while its dip runs. - stop() on a dip's opacity finished the fade but left the move's pending jump to land later, at opacity 1. A finished fade now lands the travel waiting for it at the same instant.

…ll request This pull request keeps curve sampling (hold, seek, sample, release, the curve end); the recorder, waitForAnimation, watchAnimations and waitForRest, with their self-tests, land separately on top. The holding self-tests that waited with them now poll the page for a running or idle animation. Chromium: 19 tests, 5x repeat, pass.

The harness's animation-state instruments, split out of the curve-sampling pull request so each stays reviewable: - a page-side recorder that sees every animation from its first frame or creation, so one that starts and ends before a test looks is still reported, with its finish time and start (kept when the page cancels it); - waitForAnimation: the running animations on an element past a minimum progress, or the ones that finished since the last wait, with their curve interval through the playback rate (a run going backward is dated from when it began); - watchAnimations: a watch taken before an action, for an entrance on a page's first contact; - waitForRest: rest judged by presented transform, translate, rotate, scale and opacity, not playState, so an end value held by the keyframes is not waited out, and a backward run's end delay and curve are. Self-tests cover each in real Chromium. Co-authored-by: Christopher Chedeau <vjeuxx@gmail.com>

# Conflicts: # packages/core/src/motion/README.md # packages/core/src/motion/uikit/uikit.test.ts

…inishes a dip lands its move); a pseudo-element's own opacity hold is owned Conflicts in animator.ts take this layer's Reduce Motion plan, which gains #8055's rule in its own terms: a stylesheet that hides the element lends nothing, so the move lands at once with no dip and no borrow. #8055's stop rule already holds under the plan (the opacity pass lands the waiting move); its test passes unchanged. uikit.test.ts takes #8055's ordering. Also fixed (CodeBunny on 3d201a0): a pseudo-element shown with set({opacity: 1}) over a stylesheet that hides it holds that value with an animation, which now counts as an owned opacity (writesOwnOpacity), so a move or a fade by role returns to it instead of borrowing the hiding 0.

… a dependency

# Conflicts: # packages/core/src/BottomSheet/BottomSheetPanel.tsx

…e is not a motion role

… Effects; #8072 61e891d) into #8076 Both of cixzhang's rules kept: #8073's useNodeAttachment (a replaced node re-attaches, deps re-attach, each Effect names what it syncs) and #8076's guards, so neither the isEnabled sync's cancel nor a press tracker's release calls a caller back from an Effect. Both sides' tests stay.

… from its trigger A semantic merge: - Popover.tsx: the grow-from-trigger motion (usePopoverSurfaceMotion) sits beside main's bottom-sheet presentation (usePresentationSurface and the controlled-surface latch). It runs only when the anchored layer shows; a render-prop trigger records its anchor through setRenderTrigger. - motion/uikit/springs.ts takes main's per-spring constants and adds POPOVER_PRESENT_SPRING and POPOVER_DISMISS_SPRING with their rows; index.ts and the motion README list both keyboard and popover. - layer-runtime.md takes main's INV14/INV15 status paragraph.

…ole animator

Her af953bd merges #8076's bd9246d (main 2af4d30, with #8229's Popover presentation) into this branch, with her resolution of BottomSheetPanel.tsx against #8229. No conflicts with this branch's commits since 2d520a6: the result is her head plus those commits (the keyboard-height Effect's INV3 comment, one layout read per keyboard change, the text control tap route, and the Safari/Firefox/Chrome docs). `timing.ts` checked: #8076's with exactly #8484's change.

# Conflicts: # packages/core/src/gestures/GestureHost.ts

#8176's local head reads the layout once per keyboard change, focuses a text control composer tapped with nothing focused without the reveal, says the composer moves above the keyboard once reported in Firefox and Chrome for Android, and merges the maintainer's af953bd (#8076's bd9246d, main 2af4d30 with #8229). No conflicts; `timing.ts` is #8176's.

…longer has delegated consumers #8076 carries the host without delegated consumers, which nothing used, so #8309's FR41 case for a delegated pressable has no subject left. The press-tracked and attached cases still cover native selection on a hold that does nothing (spec:AST-094 FR41).

…into the gesture hook fixes

…ides in step #8176 says the composer moves above the keyboard once Chrome reports it, since it times no ride to Gboard (spec:AST-099 FR8 asks a ride within one frame of the keyboard, with Android emulator evidence). This branch times the ride to Gboard's measured slides, with that evidence: ChatLayout's docs (English, Chinese, dense) and the browser-support guide say the composer rides Gboard's slides in step in Chrome on Android, while a Tall BottomSheet makes its room once Chrome reports the keyboard. Firefox for Android keeps #8176's wording: it moves once Firefox reports the keyboard.

# Conflicts: # docs/architecture/layer-runtime.md

…ation until its close settles architecture:layer-runtime INV14 gives the layer runtime the presentation hold: keeping a layer's chosen presentation from the open until the close settles. Layer/presentationHold.ts (internal) is that one job: createPresentationHold for code that chooses at an open event, and usePresentationHold for a choice taken during render; the component releases it when its close settles. Popover adopts it for its three latches, with no behavior change: the controlled surface (taken as isOpen turns on, let go as it turns off), the sheet kept mounted until its dialog has closed, and the touch-or-pointer presentation its close plays in (usePopoverSurfaceMotion). Not the input hold (inputHold.ts). Not exported; a test locks that. Co-authored-by: Christopher Chedeau <vjeuxx@gmail.com>

…the record names the shared hold usePopoverSurfaceMotion's returned callback reads touchHold, so it is in its dependency list (lint). architecture:layer-runtime's current-gaps paragraph no longer says no shared presentation hold exists or that Popover builds its own: the runtime owns it (Layer/presentationHold.ts) and Popover uses it; Owning code and verified_by name it.

…er runtime's presentation hold MobileNav's own close hold (isCloseHeld with a wasOpen latch) kept the dialog rendered from the commit that closes it until its delayed close() ran. It now takes the layer runtime's presentation hold (Layer/presentationHold): held from the open, released after close() runs or when nothing is open, as before. No behavior change; MobileNav's tests pass unchanged.

vercel · 2026-10-26T20:47:56Z

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

Project	Deployment	Actions	Updated
astryx	Ready	Preview	Nov 9, 2026 11:34pm UTC

astracat-bot · 2026-10-26T20:53:15Z

AI review status for this pull request.

Review status	Updated
🟢 Reviewed (for maintainers only)	Nov 10, 2026, 5:07 AM UTC

github-actions · 2026-10-26T21:05:49Z

PR Analysis Report

🔎 Preview · `aee5cb6`

Storybook · Sandbox · built from this PR's head

Modified components

Component	Size	LoC	Visual · A11y · RTL
MobileNavToggle	60.7 kB (0) · 10.0 kB CSS (0)	—	✓ · ✓ · ✗
AppShell	91.1 kB (+92 B) · 13.5 kB CSS (0)	—	— · — · —
TopNav	72.5 kB (+84 B) · 10.9 kB CSS (0)	—	— · — · —

2 more components

Component	Size	LoC	Visual · A11y · RTL
SideNav	114.3 kB (+63 B) · 11.4 kB CSS (0)	—	— · — · —
MobileNav	70.8 kB (+46 B) · 10.6 kB CSS (0)	—	✓ · ✓ · ✓

Package size: @astryxdesign/core 7.22 MB packed (−603 B); no new dependencies.

API changes: none in this pull request

Committed API report behind this build: cli 29 rows, core 36 rows; run pnpm api:report.

Touched components checked; add ci:run-visual or ci:run-rtl to run the full suites.

_{Generated by PR Enrichment workflow | View full report}

astracat-bot

Looks good to merge once #8587 lands: MobileNav's close hold now comes from the layer runtime's presentation hold with no change in behavior, and the close test that guards the hold still passes. Two optional follow-ups: MobileNav.spec.md's Current-state impact could say the close hold is now the runtime's, and the size table could note that CSS is unchanged. Test and lint CI don't run for pull requests into feat/presentation-hold, so they will first run when this targets main.

[Automated review]

#8587 (the layer runtime's presentation hold) merged as a squash, so this takes main for everything outside MobileNav and keeps only MobileNav's adoption: main's MobileNav, including the shared scroll lock, with its close hold moved onto usePresentationHold.

astracat-bot

Approved pending CI: the CI run on this head ended without reporting the required docsite-test check, although every job it started passed, so CI needs a re-run before this merges.

MobileNav's close hold now comes from the layer runtime's presentation hold with the same open-styling sequence as before, and the close test that guards the hold fails without it. Optional follow-ups: shorten the hold comment (see the existing inline suggestion), add a regression test for the release when nothing is open (a native close, or an <Activity> hidden mid-close and shown closed), and update the MobileNav and layer-runtime records' gap text now that the presentation hold is the runtime's.

[Automated review]

astracat-bot · 2026-11-09T15:57:23Z

+  // Keeps the dialog rendered from the open until the delayed `close()` below
+  // has run: the layer runtime's presentation hold. The `display` transition
+  // on the dialog is meant to do this, but a browser that does not hold a
+  // discrete `display` transition (WebKit) drops the dialog to
+  // `display: none` at once while it is still an open modal: nothing is
+  // visible, yet the whole page stays inert until the timer fires. Held during
+  // render, so it is already held in the commit that closes. The drawer's
+  // transform and the backdrop stay keyed to `isOpen`, so the slide-out is
+  // unchanged.


Non-blocking: the second sentence of this comment runs about 50 words over several topics, and "so the slide-out is unchanged" compares with past code. A shorter form that keeps only the reason:

// Keeps the dialog rendered until the delayed `close()` below runs. // WebKit does not hold a discrete `display` transition. It drops a closing // modal dialog to `display: none` at once. The page then stays inert with // nothing visible. The hook takes the hold during render, so the closing // commit already has it.

astracat-bot

Approved pending CI: CI has not finished on this head yet. The first run stopped before its required jobs because CI was full, and the re-run now in progress has to pass before this merges.

MobileNav's close hold now comes from the layer runtime's presentation hold, with the same open-styling sequence as before on every close path checked, and the close test that guards the hold fails without it. Optional follow-ups: shorten the hold comment (see the existing inline suggestion), add a regression test for the release when nothing is open (a native close, or an <Activity> shown again after the close), and update MobileNav's record now that the presentation hold is the runtime's.

[Automated review]

cixzhang · 2026-11-10T22:12:37Z

/approve-spec aee5cb6

cixzhang · 2026-11-10T22:53:34Z

(Posted by Cindy's agent.) Machine evidence (main a370b5e vs this head aee5cb6, MobileNav default story, phone viewports):

Close timing, Android Chrome emulator, 6 interleaved rounds x 2 closes per side, normal motion: median close gap (tap -> dialog gone), close button 801 ms (main) vs 884 ms (head); scrim tap 704 vs 797 ms. Both within main + 100 ms (PASS). Machine was heavily loaded; desktop runs below show identical close times.
Chromium (touch, 412x915) and WebKit (touch, 390x664), normal and reduced motion, before vs after: identical in all 8 cells.
Normal motion: the drawer slide-out and scrim fade play on both main and head (Chromium ~13 frames, dialog gone ~0.5 s; WebKit dialog removed ~0.2 s with the drawer almost off-screen).
Reduced motion: no slide; the drawer leaves in one frame on both.
After every close (button, scrim tap, Escape): focus returns to the menu trigger.
After every close the page is usable: a button underneath takes taps, nothing is left inert or aria-hidden, and no open dialog or backdrop remains.
No behavior change, as intended: the close hold moved onto the shared presentation hold.

vjeux and others added 30 commits October 26, 2026 01:44

Merge #8309 (#8076 4adaf8b; the lock after admission; no style read o…

3a8ee21

…n attach) into #8315

Merge 3d201a0 (#8056 with main through #8257's landing) as a dependency

f581948

docs(motion): the animator, README, reducedMotionFade and the browser…

8cd46df

… proof describe the roles' table values (DEC-11)

Merge #8067 (f581948) into the motion-role animator

54561a1

Merge main (2af4d30: #8257, the motion roles internal) into the animator

89614c7

# Conflicts: # packages/core/src/motion/README.md # packages/core/src/motion/uikit/uikit.test.ts

Merge main (2af4d30) into the experimental touch layer

bd9246d

Merge 5d9e4ec (#8056 on #8055's 89614c7 with a pseudo-element fix) as…

7b26ef2

… a dependency

Merge #8076 (bd9246d: main 2af4d30 with #8257) into the keyboard module

af953bd

# Conflicts: # packages/core/src/BottomSheet/BottomSheetPanel.tsx

docs(motion): coast's edge spring plays its table value; scroll.bounc…

b61b264

…e is not a motion role

Merge #8067 (7b26ef2: #8056 5d9e4ec, #8055 89614c7) into the motion-r…

370c4d9

…ole animator

Merge bd9246d (#8076 with main) as a dependency

e084460

# Conflicts: # packages/core/src/gestures/GestureHost.ts

Merge #8309 (3ccbd7a: #8076 bd9246d, main, the host's injected lock) …

2a0d065

…into the gesture hook fixes

Merge #8076 bd9246d (main 2af4d30)

999ef1b

# Conflicts: # docs/architecture/layer-runtime.md

cixzhang and others added 4 commits October 26, 2026 12:30

Merge #8342 (2ebecf5: the input hold) into the Popover presentation base

b795951

cixzhang requested review from imdreamrunner and josephfarina as code owners October 26, 2026 20:47

cixzhang mentioned this pull request Oct 26, 2026

refactor(Layer): one presentation hold keeps a layer's chosen presentation until its close settles #8587

Merged

vercel Bot deployed to Preview October 26, 2026 21:04

View deployment

astracat-bot Bot approved these changes Oct 26, 2026

View reviewed changes

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

cixzhang force-pushed the feat/presentation-hold branch from 99b0ba8 to cf409fc Compare November 7, 2026 15:54

github-actions Bot deleted the branch feat/presentation-hold November 8, 2026 06:04

cixzhang changed the base branch from feat/presentation-hold to main November 9, 2026 15:15

cixzhang marked this pull request as draft November 9, 2026 15:39

cixzhang added ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) ci:run-a11y Run the accessibility suite on this PR; write access, next push (AST-030 FR12) labels Nov 9, 2026

vercel Bot deployed to Preview November 9, 2026 15:52

View deployment

astracat-bot Bot approved these changes Nov 9, 2026

View reviewed changes

cixzhang marked this pull request as ready for review November 9, 2026 17:58

Merge branch 'main' into feat/presentation-hold-mobilenav

aee5cb6

vercel Bot deployed to Preview November 9, 2026 23:34

View deployment

astracat-bot Bot approved these changes Nov 9, 2026

View reviewed changes

cixzhang merged commit 77aa2b9 into main Nov 10, 2026
114 of 120 checks passed

github-actions Bot deleted the feat/presentation-hold-mobilenav branch November 11, 2026 06:05

Navigation Menu

Repository navigation

refactor(MobileNav): keep the closing dialog rendered through the layer runtime's presentation hold - #8590

refactor(MobileNav): keep the closing dialog rendered through the layer runtime's presentation hold#8590
cixzhang merged 1193 commits into
mainfrom
feat/presentation-hold-mobilenav

cixzhang commented Oct 26, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Oct 26, 2026 •

edited

Loading

Uh oh!

astracat-bot Bot commented Oct 26, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 26, 2026 •

edited

Loading

Uh oh!

astracat-bot Bot left a comment

Uh oh!

astracat-bot Bot left a comment •

edited

Loading

Uh oh!

astracat-bot Bot Nov 9, 2026

Uh oh!

astracat-bot Bot left a comment •

edited

Loading

Uh oh!

cixzhang commented Nov 10, 2026

Uh oh!

cixzhang commented Nov 10, 2026

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cixzhang commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What it does

No behavior change

Size (gzip KB, minified)

Overlap

Testing

Retarget (@cixzhang)

Uh oh!

vercel Bot commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

astracat-bot Bot commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Analysis Report

🔎 Preview · aee5cb6

Modified components

Uh oh!

astracat-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

astracat-bot Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astracat-bot Bot Nov 9, 2026

Choose a reason for hiding this comment

Uh oh!

astracat-bot Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cixzhang commented Nov 10, 2026

Uh oh!

cixzhang commented Nov 10, 2026

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cixzhang commented Oct 26, 2026 •

edited

Loading

vercel Bot commented Oct 26, 2026 •

edited

Loading

astracat-bot Bot commented Oct 26, 2026 •

edited

Loading

github-actions Bot commented Oct 26, 2026 •

edited

Loading

🔎 Preview · `aee5cb6`

astracat-bot Bot left a comment •

edited

Loading

astracat-bot Bot left a comment •

edited

Loading