Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:spec-owner-review
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval is on hold until an engineering owner decides on the proposed touch direction. The phone action-sheet card has no governing spec yet: the DropdownMenu record is draft and does not describe it. This pull request also adds draft requirements in spec:AST-094, component:DropdownMenu (FR8, FR20, FR23) and module:DropdownMenu/useMenuPress (FR9, DEC-8), which an engineering owner approves by commenting /approve-spec <full head SHA> (the author may do so as an engineering owner) or rejects by naming the parts to change. Independently of that decision, these need changes:
- The card drops each item's
descriptionandendContent(isPlainActionandrenderActioninMenuActionSheetCard.tsx); keep the sheet for such items or render them. - When the trigger was not focused at open, closing the card leaves focus on the page body; return it to the trigger as the bottom sheet does with
finalFocusRef. - The branch carries the 25 dependency pull requests (about 31,100 authored lines); rebase once they land so this pull request carries only the card commit.
- The
useMenuPressrecord still lists a 500 ms long-press delay while the code uses 150 ms, andspec:AST-094FR7 amends aspec:AST-090that is not in the repository.
[Automated review]
| function isPlainAction(item: DropdownMenuItemData): boolean { | ||
| return ( | ||
| typeof item.label === 'string' && | ||
| item.href == null && | ||
| !opensSubMenu(item, {showsRegions: true}) && | ||
| item.header == null && | ||
| item.footer == null | ||
| ); | ||
| } |
There was a problem hiding this comment.
isPlainAction admits rows with description or endContent, but renderAction (lines 183–199) passes only label, icon, variant and isDisabled to Button. On a phone those rows lose their description and trailing content, which the bottom sheet renders (the repository's Presentation / action sheet story shows all four descriptions disappear). Treat either field as ineligible here, or render them in the card, and add the case to the eligibility tests.
There was a problem hiding this comment.
Fixed at d2c3d6f: a row with a description, endContent or hasCloseOnSelect: false makes the menu ineligible, so it keeps the bottom sheet. Eligibility cases added.
| onClick={() => setSubmenuPath(path => path.slice(0, -1))} | ||
| /> | ||
| {presentsAsCard ? ( | ||
| <MenuActionSheetCard |
There was a problem hiding this comment.
The sheet branch passes finalFocusRef={buttonRef}, but the card gets no return target, so Dialog restores whatever document.activeElement was at open. After a programmatic open, a controlled isMenuOpen, or a browser that does not focus a tapped button, Escape, Cancel or a tap outside leaves focus on the page body instead of the trigger, which spec:AST-055 AR3 requires for a caller-rendered trigger. Pass the trigger to the card and focus it after the exit.
There was a problem hiding this comment.
Fixed at d2c3d6f: the card takes the trigger as its final focus and focuses it once the dialog has closed, as the sheet's finalFocusRef does. New jsdom case: a controlled open from another button, then Cancel, puts focus on the trigger. It fails on the old code.
| @@ -140,6 +140,7 @@ Consumer migration instructions belong in consumer docs and release notes. | |||
| | stray-click window | 400 ms, or the next `pointerdown` | how long after a tracked release the browser's click is still swallowed | `MENU_PRESS_STRAY_CLICK_MS` | `module:DropdownMenu/useMenuPress` | stable | | |||
| | settle time | 300 ms | proposed: how long an opening press must last before its release may act | `MENU_PRESS_SETTLE_MS` | `module:DropdownMenu/useMenuPress` | proposed | | |||
| | long-press delay | 500 ms | proposed: how long a finger rests on the trigger before the menu opens under it | `longPressDelayMs` | `module:DropdownMenu/useMenuPress` | proposed | | |||
There was a problem hiding this comment.
This row still says 500 ms, but MENU_PRESS_LONG_PRESS_MS is now menu.holdOpenMs (150 ms, plus 150 ms inside a scroller), and the hook's doc says 150. Update the row with its UIKit source, or keep 500 ms in code, so the owner decision on OQ1 covers the number that ships.
| type: "'popover' | 'bottom-sheet' | 'adaptive'", | ||
| description: | ||
| "Presentation surface for data-driven items. 'popover' stays anchored, 'bottom-sheet' always renders the actions in a modal BottomSheet, and 'adaptive' uses a BottomSheet on compact coarse-pointer layouts while remaining anchored elsewhere. Compound children currently support popover only.", | ||
| "Presentation surface for data-driven items. 'popover' stays anchored, 'bottom-sheet' always renders the actions in a modal BottomSheet, and 'adaptive' uses a BottomSheet on compact coarse-pointer layouts while remaining anchored elsewhere. On a phone, a 'bottom-sheet' of plain actions (no sub-menus, links, rich labels, header or footer) presents the way iOS 26 presents an action sheet: a centered alert-style card with Cancel last, which a tap outside dismisses. Compound children currently support popover only.", |
There was a problem hiding this comment.
This still says 'bottom-sheet' always renders a BottomSheet and 'adaptive' uses one on compact coarse-pointer layouts. On a phone, both values present plain actions as the card (adaptive resolves to the same branch). Drop "always" and name both values.
There was a problem hiding this comment.
Fixed at d2c3d6f: the presentation doc drops "always" and says that on a compact touch screen either sheet of plain actions presents as the card.
| follows. | ||
| - **FR7 — Exits animate as UIKit's do.** A touch presentation MUST play the | ||
| exit its UIKit counterpart plays. **Proposed; pending owner review:** this | ||
| departs from the draft `spec:AST-090` FR1 (exits off by default) and FR10 |
There was a problem hiding this comment.
spec:AST-090 does not exist on main or at this head, so there is no FR1 or FR10 for an owner to amend. Point this, and the matching current-state impact line, at an existing record, or drop the amendment.
There was a problem hiding this comment.
AST-094 is now the maintainer’s record in #8316 (one touch model on every touch device); the body cites it there.
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsAlertDialog (@astryxdesign/core) · View in Storybook
ContextMenu (@astryxdesign/core) · View in Storybook
MoreMenu (@astryxdesign/core) · View in Storybook
Component Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included.
|
| Component | JavaScript | CSS |
|---|---|---|
| core/AlertDialog | +31.8KB |
+74B |
| core/ContextMenu | +2.1KB |
+145B |
| core/DropdownMenu | +49.8KB |
+2.2KB |
| core/Markdown | +2.0KB |
+119B |
| core/MoreMenu | +49.7KB |
+2.2KB |
| core/Table | +2.1KB |
+135B |
| core/TableCell | +2.1KB |
+147B |
| core/TableHeaderCell | +2.2KB |
+160B |
Bundle Size Summary
Gzip figures measure each package's entry file, not the code an import pulls in.
| Package | Entry (ESM) | Entry gzip before | Entry gzip after | Delta |
|---|---|---|---|---|
| @astryxdesign/core | 4.9KB | 1.2KB | 1.2KB | 0B |
| Package stylesheet | Gzip before | Gzip after | Delta |
|---|---|---|---|
| @astryxdesign/core | 34.6KB | 35.2KB | +604B |
Accessibility Audit
Status: No accessibility violations detected.
Visual Regression
55 of 502 shot(s) changed.
A change here is a question, not a failure: check whether the after is the
picture you intended. Record that review on the PR. Baseline maintenance is an
explicit dispatch of CI; this report never rewrites the baseline or adds a release gate.
| component | story | theme | mode | pixels |
|---|---|---|---|---|
| Selector | Indicator Space Bottom Sheet Narrow End RTL | neutral | light | 753,933 |
| Selector | Indicator Space Bottom Sheet Wide Start | neutral | light | 740,072 |
| RichTextEditor | Markdown parity: side by side | probe | dark | 558,840 |
| RichTextEditor | Markdown parity: side by side | probe | light | 540,756 |
| Chat | Full AI Chat | probe | light | 230,823 |
| Chat | Full AI Chat | probe | dark | 229,805 |
| AppShell | With Mobile Nav | probe | light | 217,010 |
| AppShell | With Mobile Nav | probe | dark | 216,960 |
| Collapsible | Dividers | probe | dark | 66,445 |
| Collapsible | Dividers | probe | light | 66,445 |
| AppShell | Playground | neutral | dark | 65,198 |
| AppShell | Playground | neutral | light | 63,788 |
| Chat | Default | neutral | dark | 36,304 |
| Chat | Default | neutral | light | 35,767 |
| Chat | Empty State | probe | light | 23,531 |
| Chat | Empty State | probe | dark | 22,622 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | dark | 15,378 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | light | 14,107 |
| Chat | With Attachments | probe | light | 13,406 |
| Chat | With Attachments | probe | dark | 11,232 |
and 35 more.
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report
a2f4a7c to
f8738f3
Compare
There was a problem hiding this comment.
Approval is on hold until an engineering owner decides on the proposed touch direction: the phone action-sheet card has no governing spec yet, and this pull request adds draft requirements in spec:AST-094, component:DropdownMenu (FR8, FR20, FR23) and module:DropdownMenu/useMenuPress (FR9, DEC-8). An engineering owner records that decision by commenting /approve-spec with the full head SHA (the author may do so as an engineering owner), or names the parts to change. Independently of that decision, these still need changes:
- The card drops each item's
descriptionandendContent(isPlainActionandrenderActioninMenuActionSheetCard.tsx); keep the sheet for such items or render them. - Closing the card returns focus to whatever held it at open rather than the trigger; pass the trigger to the card and focus it after the exit, as the bottom sheet does with
finalFocusRef. - The branch carries the 25 dependency pull requests (about 33,400 authored lines); rebase once they land so this pull request carries only the card commit.
- The
useMenuPressrecord still lists a 500 ms long-press delay while the code uses 150 ms,spec:AST-094FR7 amends aspec:AST-090that is not in the repository, and thepresentationdocs still saybottom-sheetalways renders a BottomSheet.
[Automated review]
| return; | ||
| } | ||
| const item = option as DropdownMenuItemData; | ||
| rows.push(renderAction(item, `item-${item.id ?? index}`)); |
There was a problem hiding this comment.
Eligibility is decided once at open (DropdownMenu.tsx:748-757), but the card renders the live items, and every row becomes a Button whose click only runs onClick and closes. A row that gains an href or nested items while the card shows (an asynchronous load, say) loses its navigation or its sub-menu. Keep the card on the rows it opened with, or render a link row as a link and leave sub-menu rows out while the card shows.
There was a problem hiding this comment.
Fixed at d2c3d6f: the card follows its live rows only while they stay plain actions. If rows gain a link, a sub-menu or rich content while it shows, it keeps the last rows it could present, and they get the sheet next time. New jsdom case; it fails on the old code.
… platform (#8274) ## Exact decision needed What "compact" means when a component picks a presentation by screen size, and whether it differs by platform. ## Existing authority searched - `spec:AST-012` FR1–FR2: width points (`md` is 768) and a closed `when` vocabulary with no height field. - `architecture:public-component-api` INV14 and change coupling: components keep no parallel hardcoded breakpoint table. - `component:Selector` FR4: `adaptive` uses the sheet on "compact coarse-pointer screens", with no number. - `design:mobile-typography` DR3 uses `(width < 1024px) and (pointer: coarse)` for text sizing, a separate concern this PR leaves alone. - No current record defines compact. Code uses `(max-width: 768px) and (pointer: coarse)`. - Search terms: compact, 744, 768, height 500, size class, adaptive, coarse pointer, breakpoint. Overlapping open pull requests: #8062 (shared touch resolver and its compact query), #8052 (AST-094 resolver), #8015 (NumberInput steppers on coarse pointers; not a size test), #6817 (compact density, a different meaning), #8089, #8104, #8095 (phone presentations that consume the resolver). ## Decision - `spec:AST-012` FR9 / DEC-6: a screen is compact while its width is below the effective `md` point or its height is below 500, with the same numbers on every platform (CSS px on the web, pt on iOS, dp on Android); native does not map compact to platform size classes. The theme exports this one definition; the web touch presentation resolver, Selector's `adaptive` presentation, and `@astryxdesign/native` all use it. - `spec:AST-012` FR2: `when` gains `height`, accepting only `{below: 500}`. - `architecture:public-component-api` INV17: a component that chooses a presentation by screen size uses that definition and keeps no numbers of its own. - `spec:AST-082` FR18: `PortableTheme` carries the compact definition, so native reads the same numbers; FR11 and `spec:AST-093` FR3: a native choice between presentations by screen size uses it, not size classes, while the platform's own control still presents the surface. - Compact governs presentation choice only; a layout width a caller picks, such as AppShell's `mobileNav.breakpoint`, is not compact. - `component:Selector` records the `adaptive` edge change as pending and compatible under `spec:AST-017` FR1. ## Rejected alternatives - Mapping compact to each platform's size classes: the same screen would be compact on one platform and not another. ## Claim scope and non-goals The compact definition and the height condition only. Text sizing under `design:mobile-typography`, which surfaces adopt a touch presentation, and resolver code are outside this PR. ## Evidence and implementation relationship The touch presentation resolver contract lives only in the open `spec:AST-094` revision (#8052), which this PR does not edit; it must use FR9. Native adopts FR9 when it gains a compact resolver. Behavior delta when the code conforms: a coarse-pointer screen exactly 768 wide and at least 500 tall opens the popover, and a landscape phone wider than 768 but under 500 tall opens the bottom sheet. Code follow-up belongs to separate implementation PRs. INV16 is the caller-control invariant from #8273 (merged); this PR adds INV17. No implementation is included. ## Writing rubric results - R1 narration: none. - R2 research in the record: none. - R3 evidence naming a change: none; verification names edge states. - R4 rejected alternatives over one line or outside a decision: one, inside DEC-6, one line. - R5 pull request or issue references remaining: none added. - R6 ruled open questions still listing options: none. ## Scope - [x] This PR contains specification records only. - [x] It records observable behavior and preserves equivalent internal implementations. - [x] Any named internal mechanism is intentionally public, names its dependent caller/system, and explains why an equivalent implementation would not satisfy the contract. - [x] Verification states the evidence layer and failure signal without prescribing CI job/workflow topology. - [x] It records one intentional durable decision, not a rescue for a separable tagalong. - [x] The canonical knowledge owner, affected current claims, and overlapping open work are named; any ownership collision is resolved or called out for human decision. - [x] Adjacent API, visual, accessibility, composition, and implementation facts remain outside scope unless this exact decision depends on them. - [x] Public text and artifacts contain no internal Meta context. ## Validation - `pnpm check:knowledge` (CI lint) - Prettier - `git diff --check`
|
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.
|
…ry pair, meta#8195 re-takes
…capture, and blur-on stills
f8738f3 to
d2c3d6f
Compare
github-actions
Bot
added
the
needs:design-review
There was a problem hiding this comment.
Approval is on hold until an engineering owner decides on the proposed touch direction: the phone action-sheet card has no governing spec yet, and this pull request adds draft requirements in spec:AST-094, component:DropdownMenu, component:DropdownMenuSubMenu and module:DropdownMenu/useMenuPress. An engineering owner records that decision by commenting /approve-spec with the full head SHA (the author may do so as an engineering owner), or names the parts to change. Independently of that decision, these still need changes:
- An explicit
presentation="bottom-sheet"now shows the card on a phone with no value that keeps the sheet, againstarchitecture:public-component-apiINV16 andspec:AST-017DEC-23; route the choice through the shared presentation resolver so the caller's choice wins, and name the restoring value in the changeset. MenuActionSheetCardkeeps the rows it shows in a ref written during render (architecture:react-component-runtimeINV4); hold them in state instead.- The usage guidance in
DropdownMenu.doc.mjsandMoreMenu.doc.mjsstill promises a BottomSheet for these menus, andspec:AST-094FR7 amends aspec:AST-090that does not exist. - The branch carries its dependency pull requests (about 40,800 authored lines), including an AlertDialog touch mode and a
platformtheme condition that contradict current claims; resolve those there and rebase once they land, so this pull request carries only the card commit.
[Automated review]
| setCardDecidedFor(isOpen); | ||
| if (isOpen) { | ||
| setPresentsAsCard( | ||
| isCompactTouch && |
There was a problem hiding this comment.
This swaps the card in for an explicit presentation="bottom-sheet" too, from isCompactTouch and the rows alone, and no presentation value keeps the sheet or reaches the card. architecture:public-component-api INV16 has the caller's explicit choice win over a touch default, and spec:AST-017 DEC-23 rejects a touch presentation callers cannot pin. Decide the card through the shared presentation resolver so an explicit value wins, and name the restoring value in the changeset.
| // present (they get the sheet the next time the menu opens). | ||
| const shownItemsRef = useRef(items); | ||
| if (isActionSheetCardEligible(items, {hasRegions: false})) { | ||
| shownItemsRef.current = items; |
There was a problem hiding this comment.
The ref is written during render and is the only source of the rows once the live rows stop being plain actions; architecture:react-component-runtime INV4 says a ref mutation must not be the sole source of rendered output, and an interrupted render can leave rows that never committed. Keep the last presentable rows in state adjusted during render, as presentsAsCard does in DropdownMenu.tsx.
d2c3d6f to
7f38a12
Compare
e7b0676 moved a call's start from rest to its first read whenever WebKit had dated it more than two frames before that read. Under load from other tasks the first read comes late with nothing in the call's task delaying it, so the start moved about 95 ms on a loaded box, and whatever chained on the call's end ran that late: the second half of a Reduce Motion cross-fade, so SearchField's cancel missed DR2's 200 ms. The call's task now records when it ended, at its first microtask (or a read in those microtasks is that time), and only a call whose own task ran past the call by more than two frames, the block e7b0676 fixed, moves its start; a read other tasks delay keeps the browser's start.
…s its start past the block Core Animation dates an animation at the commit, when the calling task ends, however short the block before it. A 20 ms block in the call's task, with WebKit's start dated at the call, left the start at the call: the two-frame bound skipped it. Fails now.
… task, with no threshold Core Animation dates an animation at the commit, when the calling task ends, whatever the block's length. The call's task already records when it ended, at its first microtask, so a start the browser dated before that (WebKit's last frame time before the task) now moves to it: a task that ended at once leaves the start at the call, a blocked one moves it past the block, however short, and a read other tasks delay changes nothing. The two-frame bound, which told a read's own frame from a block before the task's end was known, goes, and the start no longer moves to the read.
meta#8089 now takes meta#8080, which takes meta#8078 (with meta#8627, meta#8360 and meta#8298) and meta#8186. DropdownMenuTouch.ts registers UIKit's two menu springs for the grow, from meta#8080, and keeps the springs table for the in-place sub-menu; its header says so. The usage text takes meta#8080's wording, meta#8089's popover sentence and this PR's in-place sub-menu sentence.
…rings and no other (red) architecture:component-style-authoring INV6: a touch layer ships the springs it plays. The grow and collapse already carry UIKit's two menu springs and no other row. The in-place sub-menu reads the whole springs table and registers all of it, so its bundle carries every row: the keyboard's spring among them. This case bundles createMenuDrillInMotion alone and fails here.
…prings and no other The in-place sub-menu plays springs.subMenuOpen and springs.subMenuBack. It now imports those two springs instead of the whole table, and registers them alone through registerSubMenuSprings, as the grow does with registerMenuSprings. Its bundle carries those two springs and no other row (architecture:component-style-authoring INV6).
Once a component in the root entry holds its layer's input (a popover's presentation hold, meta#8223), Layer/inputHold's static imports of gestures/GestureHost and gestures/nativeScroll put eight gesture modules (about 47 KB) in the root entry, against architecture:react-component-runtime INV13. Bundle a component that imports the input hold and require no gesture bytes. Fails while the input hold imports from gestures/.
…e host asks it Layer/inputHold registered a touch gate with the gesture host (getGestureHost(doc).addTouchGate(isInHeldLayer)) and walked the flat tree through gestures/nativeScroll, so any component that holds its layer statically carried the gesture system into its bundle and the root entry (architecture:react-component-runtime INV13). Invert it: the host asks isInHeldLayer for every touch itself (gestures may import Layer), the hold registers nothing, and it walks the flat tree through utils/flatTree, where parentAcrossShadow already lives. addTouchGate stays for other gates. A host test holds a held layer's touches with no gate (it fails if the host stops asking the hold).
…few ms, and one task's calls start together Browser start times are frame times, while a task's end is a live clock, so on every call they differ by up to a frame plus the task's own length. Moving every start from rest to its task's end shifted each one by that length, about 5 ms in a component's effect after a tap, and the shift added up along a chained sequence: SearchField's Reduce Motion cancel measured 204-207 ms against design:reduced-motion DR2's 200. The first case fails now (the start moves to 1010). The second keeps one motion's parts together when they are separate calls in one task: a block measured per call would split them by a frame.
…n on more than two frames, and all the task's calls move together fc42d64 moved every start from rest to the end of the call's task. Browser start times are frame times while that end is a live clock, so on every call they differ by up to a frame plus the task's own length: each start slid by that length, and a chained sequence added it up, about 5 ms per link in SearchField. The rule from e6876ca is back, with its bound now named for that quantization: only a task that ran on more than LATE_START_MS (two frames) past its first call blocked anything, and its calls' starts move to their first read. The block is now measured per task, not per call, and every call of a blocked task takes the same read, so one motion's parts that are separate calls in one task (a component's effect) start together instead of splitting by a frame. The 20 ms case keeps the browser's start, as no clock here tells it from quantization.
…andoned meta#7839 entered this branch's dependencies in six merges, the last of meta#7839 53e8deb. Its whole diff against the main it was built on (d5cd551), 14 files, comes off in this one commit, so the branch ships none of its code. The history stays. This PR's own change needs none of it: no file here imports menuSheetPath.ts or opensSubMenu.ts, and the core typecheck and the DropdownMenu, ContextMenu and MoreMenu suites pass without it. The DropdownMenu spec loses meta#7839's rows (FR23, its FR20 and FR15 text, the sub-menu regions row), so the FR21-before-FR23 order that merge kept no longer applies.
…sub-menu springs [skip ci] createMenuDrillInMotion's comment still said it registers the springs table; since the sub-menu ships only its two springs, it registers those (registerSubMenuSprings), as createMenuSurfaceMotion's comment says of the menu springs.
… as a dependency [skip ci] meta#8220 no longer carries the withdrawn meta#7839. This PR's action-sheet card used two things only meta#7839 provided: its opensSubMenu helper, and header and footer on a data row. isPlainAction now checks for a nested level itself, by main's rule (a non-empty items), and drops the per-row header and footer checks, since rows have neither. The level's own header and footer still keep the sheet (hasRegions). The DropdownMenu bottom sheet takes main's regions again, inside this PR's card-or-sheet branch. DropdownMenuTouch.ts names both register functions it calls, and says the card plays the alert's two springs through useAlertMotion (meta#8373). The presentation description keeps this PR's card sentence and meta#8080's wording.
There was a problem hiding this comment.
Approval is on hold until an engineering owner decides the phone action-sheet card, which no spec on main describes and which replaces the List touch rows that component:MoreMenu FR1 and FR3 record for MoreMenu's bottom sheet, and approves or rejects the requirements this pull request writes into spec:AST-094 (FR4's alert springs and FR41's selection rule), spec:AST-057 and architecture:layer-runtime (the exit copy and INV13), along with the in-place sub-menu that spec:AST-094 FR34 and OQ10 leave open. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or names the parts to change. Independently of that decision, these need changes:
- An explicit
presentation="bottom-sheet"now shows the card on a phone with no value that keeps the sheet, againstarchitecture:public-component-apiINV16 andspec:AST-017DEC-23, so the card should be decided in the shared presentation resolver and the changeset should name the restoring value. - Main's
touch-support-bundlecheck fails for the root and hooks entries until #8671 or an equivalent lands. - The card keeps its rows in a ref written during render (
MenuActionSheetCard.tsx:178-182), and the DropdownMenu and MoreMenu usage docs still promise a BottomSheet for menus that now open the card. - The flagged +2.2 KB touch-only CSS needs a justification, and the branch's 45,507 authored lines should shrink to the card commits once the dependencies land.
[Automated review]
| if (cardDecidedFor !== isOpen) { | ||
| setCardDecidedFor(isOpen); | ||
| if (isOpen) { | ||
| setPresentsAsCard( | ||
| isCompactTouch && | ||
| isActionSheetCardEligible(items, { | ||
| hasRegions: isRenderable(header) || isRenderable(footer), | ||
| }), | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
On a compact touch screen this swaps the card in for an explicit presentation="bottom-sheet" too, so a caller who chose the sheet has no value that keeps it (architecture:public-component-api INV16, spec:AST-017 DEC-23). Deciding the card in the shared resolver (usePresentation) lets an explicit value win and gives the card a value of its own; the changeset then names the restoring value (spec:AST-017 FR3).
| const shownItemsRef = useRef(items); | ||
| if (isActionSheetCardEligible(items, {hasRegions: false})) { | ||
| shownItemsRef.current = items; | ||
| } | ||
| const shownItems = shownItemsRef.current; |
There was a problem hiding this comment.
Writing shownItemsRef.current during render makes the ref the only source of the rendered rows once they stop being plain actions, which architecture:react-component-runtime INV4 rules out, and a render React discards can still overwrite it. State adjusted during render, as presentsAsCard is in DropdownMenu.tsx, keeps the last presentable rows instead.
| type: "'popover' | 'bottom-sheet' | 'adaptive'", | ||
| description: 'Presentation policy forwarded to DropdownMenu. `adaptive` keeps the anchored popover on pointer-based layouts and uses a BottomSheet on a compact touch screen: a coarse primary pointer, and a viewport narrower than the theme `md` breakpoint (768px by default) or shorter than 500px.', | ||
| default: "'popover'", | ||
| description: 'Presentation policy forwarded to DropdownMenu. `adaptive` keeps the anchored popover on pointer-based layouts and uses a BottomSheet on a compact touch screen: a coarse primary pointer, and a viewport narrower than the theme `md` breakpoint (768px by default) or shorter than 500px. Omitted, the menu is the anchored popover, and under a finger, on every device, its trigger opens as the finger presses (inside a scroll view, after the 150 ms content-touch delay). Pass \'popover\' to keep the pointer behavior on every device, a 500 ms hold included.', |
There was a problem hiding this comment.
This still says adaptive uses a BottomSheet on a compact touch screen, but with plain actions MoreMenu now shows the alert-style card there; this line and the guidance at lines 142, 226 and 243 need the card (architecture:public-component-api Change coupling).
…wins over touch defaults (#8298) <!-- Template: maintenance.md (an internal resolver; no behavior change until a component reads it). --> Small batch of the merge plan: one commit on main holding only this change. Its paragraph for the gestures README waits for that README, which comes with #8076; the resolver's file header carries the same contract. ## Maintainer impact A component that behaves differently under a finger, such as a menu that tracks the finger or becomes a sheet, must still let a caller pin the desktop behavior. Today components read `presentation ?? 'popover'`, which cannot tell an omitted prop from an explicit `'popover'`, so a component that adopts a touch default would override a caller who asked for the popover. The menu, context menu and picker work that adopts touch modes builds on this resolver (#8104, #8627, #8080, #8125, #8201, #8159, #8162). ## Intended invariant A caller's explicit `presentation` always wins over the touch default (`architecture:public-component-api` INV16), and the surface equals `useAdaptivePresentation`'s for every value on every device, so no caller sees a change: | `presentation` | surface | touch input modes | | ------------------------------- | --------------------------------------------- | ----------------- | | `'popover'` | popover | never | | `'bottom-sheet'` | sheet | under a finger | | `'adaptive'` | sheet on a compact touch screen, else popover | under a finger | | omitted, `whenOmitted: 'touch'` | popover | under a finger | The modes follow the pointer, never the platform. The server and hydrating render never present touch modes, and no hint changes that (`spec:AST-094` FR13). The hook listens only to the queries a request reads (`architecture:react-component-runtime` INV5): none for a pinned `'popover'`, the pointer for `'bottom-sheet'` and the omitted `'touch'` default, and the pointer and compact size for `'adaptive'`. A touch-mode style is applied through `touchModes`, never nested under the iOS-only `@supports` gate. ## Change - `utils/presentation.ts`: `usePresentation` and `readPresentation` (and the pure `resolvePresentation`) return the surface to render and `touchModes`, whether the component's touch input modes apply; `usePresentation` subscribes to each media query only while the request reads it. Internal: no component reads it yet. - `hooks/useAdaptivePresentation.ts`: its header points a component that also switches input modes to `usePresentation`, whose surface is its own. ## Evidence - Failure before the change: an omitted `presentation` and an explicit `'popover'` resolve the same, so a touch default cannot tell them apart; a server render given a touch hint must still answer the popover with no touch modes, a case that failed first. On an iPhone, a resolver that seeded its server snapshot from the live window fails both the server-render case and the `hydrateRoot` case; listening to both queries for every value fails the subscription case. A resolver that gives `'adaptive'` the sheet without the coarse pointer, or reads compact by width only, fails the surface matrix; one that ignores the theme's `md` fails the themed case. - Success after the change: the unit matrix (iPhone, iPad, Android, desktop, Mac Safari, a fine-pointer narrow window, a phone in landscape, and the 767/768 and 499/500 compact edges, plus `'adaptive'` under a theme whose `md` is 1000) covers each row above, a live pointer change and the one-moment read; on an iPhone, the server render and the hydrating render answer the popover with no touch modes, and after hydration the phone's sheet with touch modes; each request holds only the subscriptions it reads. - Product/runtime behavior verified unchanged: every value renders `useAdaptivePresentation`'s surface on every device, and no component reads the resolver yet. ## Scope - [x] No intended public API, product behavior, visual direction, or policy change. - [x] Product changes discovered during the work were removed or split. - [x] Public text and artifacts contain no internal Meta context. ## Testing Local gates at 9f0595f, one commit on main d73908f holding only this change (4 files, +571/−1): - `pnpm test:ui` (core UI): 654 files, 16,459 tests passed. - `pnpm exec vitest run --project node`: 804 files passed; the only failures are main's own story-tree and release-cache reds, tracked in #8509 and #8520. - `tsc --project packages/core/tsconfig.json --noEmit`, strict ESLint and Prettier on every changed file: clean. - `pnpm check:repo`: clean.
Depends on #8220, #8373, #8340. Review only this PR's change: vjeux/astryx@4428def...c5e07b0
User need
On a phone, DropdownMenu's action sheet (
presentation="bottom-sheet") still slides up from the bottom, as on iOS 18. Since iOS 26, one without a source view appears as an alert: a centered card over a dimmed page, Cancel in the same stack, and a finger can slide between actions.What led to it: comparing our menus with UIKit's on the iOS 26.2 simulator.
Problem and solution fit
On a phone, a bottom-sheet menu of plain actions renders that card: it scales in on the alert's spring, ignores taps that land while it enters (keys stay live), tracks a finger across its actions, and closes on a tap outside. Sub-menus, links, rich rows, a header or a footer keep the sheet.
Current specification
Proposed, pending owner review (
spec:AST-094): on a phone,bottom-sheetwith plain actions presents as iOS 26's card, Android included. The sheet is today's design.Public delta and caller burden
No API change. Phones with plain actions get the card; focus returns to the trigger. Larger screens and a mouse are unchanged.
Design and alternatives
Evidence
iOS 26.2 simulator, Safari: a finger slides from "New file" to "New folder". Main's sheet is dragged away; the card runs "New folder" once.
Android Chromium 158 emulator (blur off in the clips, on in the stills): main drags the sheet back; the card runs "New folder".
Scope
Testing
On the "Presentation / action sheet card" story, phone-sized, with touch emulation:
Automated: the card's unit tests and
DropdownMenuActionSheet(Chromium, WebKit), whose entrance taps fail without #8373. The card decides a row opens a level by main's rule (non-emptyitems). Unrelated reds:touch-support-bundle(#8671);DropdownMenuLazySheetsetup timeout (#8353).The local gates are the evidence; no CI run is expected for this push.
Stack (merge in this order): #8360, #8298 and #8627 (any order) → #8671 → #8078 and #8186 (any order) → #8080 → #8089 → #8220; #8084 and #8342 → #8373; #8340; then this PR.