Repository navigation
Conversation
The drawer closed its modal dialog 60% of the way into the slide-out (at most 250 ms in), which cut the scrim's fade off partway, left the leaving drawer's links live until then, and, inside AppShell, let the Activity boundary cut the slide-out off at 300 ms. The dialog now stays open, modal and rendered until the slide-out ends, and the drawer panel is inert meanwhile: nothing takes a tap during the slide-out, as in a UIKit dismissal (taps 0.1 s and 0.25 s into an iOS 26.2 sheet dismissal are dropped). When it ends, the dialog closes, the scroll lock is released and focus returns to the opener. The scrim fades over the whole slide-out, and AppShell holds its Activity boundary for MobileNav's longest slide-out (MAX_EXIT_MS) plus a buffer.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
needs:design-review
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsAppShell (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Visual Regression46 of 494 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 26 more. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
![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: no current spec decides how MobileNav treats input and focus while it closes, so blocking every tap until the slide-out ends and returning focus only at its end needs an engineering owner to approve or reject that behavior, or to point to the spec that governs it. An engineering owner records approval by commenting /approve-spec <full head SHA> on this pull request, and as an engineering owner you may record it yourself. Independently, one fix is needed: when the dialog is closed natively (a <form method="dialog"> submit or ref.current.close()) before isOpen turns false, the effect's already-closed branch in MobileNav.tsx never releases the scroll lock, so overflow: clip and the scrollbar gutter stay on <html>; calling releaseScrollLock() there, with a regression test, restores the cleanup main has.
[Automated review]
| setIsCloseHeld(false); | ||
| // Nothing is open, so there is no slide-out to hold the dialog for, and | ||
| // no exit timer is left to release the hold. | ||
| setIsExiting(false); |
There was a problem hiding this comment.
When the dialog was already closed natively (a <form method="dialog"> submit, or ref.current.close() through the forwarded ref) before isOpen turned false, this branch is the only one that runs, and nothing releases the scroll lock any more: the effect cleanup no longer resets overflow or the gutter. A jsdom probe (open, native close(), isOpen={false}, 2 s) leaves overflow: clip and scrollbar-gutter: stable on <html> here and clears both on the base. Calling releaseScrollLock() in this branch restores that cleanup (architecture:react-component-runtime INV3).
|
Posted by Cindy's agent on her behalf. Parts of this PR re-implement behavior that already exists, or is being specified, as a shared part of core. Converging keeps one implementation per behavior:
This is about where the behavior lives, not about the PR's intent. |
|
Agreed: one implementation.
|
…8063) ## User impact On a phone narrower than the drawer, `MobileNav` covered the whole screen: the default 320 px drawer on a 320 px phone left no backdrop to tap, although the `width` prop promises "Capped at 85vw to prevent overflow on small screens". The page the drawer navigates was hidden entirely. ## Problem and solution fit The drawer's width was `width: 100vw; max-width: <width>px`, which is min(100vw, width): the only cap was the viewport itself. A drawer that covers the screen hides its context and takes away the usual way out, a tap beside it; UIKit's overlay sidebar never covers the whole screen. Small phones, zoomed-in text and split-screen windows reach these widths. The fix swaps the two: `width: <width>px; max-width: 85vw`, which is min(width, 85vw). On a 320 px phone the drawer is 272 px wide, and a 48 px strip of the dimmed page stays beside it as a target for the tap that closes it. ## Expected behavior and authority - `MobileNav.doc.mjs`, `width`: "Drawer width in pixels. Capped at 85vw to prevent overflow on small screens." ## Smallest restoration One style function in `MobileNav.tsx`. No API, token or default changes. From 377 px up, 85vw is at least 320 px, so the default drawer is unchanged there. Siblings: #8042, #8046 and #8057 also add `ci.yml` steps, and #7358, #7356 and the close-path fix #8064 touch `MobileNav.tsx`; whichever lands second rebases. ## Evidence Before (`main`) and after, 320 × 568, drawer open:   iOS 26.2 simulator, real taps (402 px: the drawer keeps 320 px): a tap on the strip closes it:  - Reproduction before the change: on a 320 px phone the drawer is 320 px wide. The new Chromium contract fails against a `main` Storybook (expected 272, received 320). - Result after the change: a 272 px drawer and a 48 px strip; the strip is backdrop (`elementFromPoint` is the dialog), and a real tap on it closes the drawer. - Representative unchanged path: on a 402 px phone the drawer keeps its 320 px width and the 82 px strip. - Regression test or other mutation-sensitive proof: `MobileNavWidth.a11y.chromium.spec.ts` (real Chromium, touch), plus a jsdom test that the drawer's own class declares `max-width: 85vw` and takes its width from the prop. ## Scope - [x] One defect is restored. - [x] Separable contradictory or unsettled tagalongs were removed or split. - [x] No new public API, visual representation, default, or interaction model is hidden under the bug-fix label. - [x] Public text and artifacts contain no internal Meta context. ## Testing - `pnpm exec vitest run packages/core/src/MobileNav packages/core/src/AppShell`: 136 passed. - `pnpm -F @astryxdesign/core typecheck`; eslint and prettier on touched files; `pnpm check:repo` (pre-commit). - `pnpm exec playwright test packages/core/src/MobileNav/__tests__/MobileNavWidth.a11y.chromium.spec.ts --project chromium` against a fresh Storybook build: 2 passed. Against `main`: the 320 px test fails and the 402 px test passes. - CI: a new "Run MobileNav width contract" step in the a11y contract job, after the Token remove target contract.
|
Posted by Cindy's agent on her behalf. Under the desktop rule (#8347, |
User impact
Closing the phone navigation drawer let input through before the drawer was gone. For the first 60% of the slide-out, the leaving drawer's links stayed live. After that, taps reached the page while the drawer was still visibly leaving. Focus returned mid-slide, and inside
AppShellthe drawer was unmounted 300 ms into its 410 ms slide-out.Evidence
iOS 26.2 simulator, real taps (Chromium where noted):
On
mainthese look the same; the difference is when input returns, which the contract asserts. Focus return is test-only.main(not modal midway, a link tap on the leaving drawer registers, a late tap reaches the page); on the simulator the dialog closes about 238 ms after the touch.Problem and solution fit
The close was timed to a CSS
displayhold, not the slide-out. UIKit (iOS 26.2) drops taps during a sheet dismissal; input returns when it completes.The dialog now stays open, modal and rendered until the slide-out ends. The drawer panel is
inertmeanwhile (not the dialog: an inert modal dialog stops blocking the page in Chromium). When the slide-out ends, the dialog closes, the scroll lock is released and focus returns to the opener.Expected behavior and authority
Smallest restoration
MAX_EXIT_MS(550 ms; 0 under Reduce Motion); thedisplaytransition is removed ([Bug] MobileNav close freezes the page in Safari 26.1 #4290).MAX_EXIT_MS+ 50 ms (was 300).Owner decision (proposed; pending owner review): input stays blocked for the whole slide-out, and focus returns when it ends, as in UIKit.
Siblings: #8042, #8046, #8057 (
ci.yml); #7356, #7358, #8063 (MobileNav.tsx); whichever lands second rebases.Scope
Testing
pnpm exec vitest run packages/core/src/MobileNav packages/core/src/AppShell packages/core/src/SideNav packages/core/src/TopNav packages/core/src/Layer: 699 passed.pnpm -F @astryxdesign/core typecheck; eslint and prettier on touched files;pnpm check:repo(pre-commit).pnpm exec playwright test packages/core/src/MobileNav/__tests__/MobileNavClose.a11y.chromium.spec.ts --project chromium: 4 passed; againstmain, 3 failed and 1 passed.