Repository navigation
DropdownMenu: under a finger, the opening finger picks only after UIMenu's handoff (AST-094 FR29) - #8627
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| BreadcrumbItem | 68.1 kB (+610 B) · 10.9 kB CSS (+81 B) | — | — · — · — |
| PowerSearch | 236.7 kB (+585 B) · 21.8 kB CSS (+51 B) | — | — · — · — |
| Pagination | 87.2 kB (+547 B) · 16.1 kB CSS (+56 B) | — | — · — · — |
31 more components
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| DropdownMenuSubMenu | 57.2 kB (+510 B) · 8.9 kB CSS (+80 B) | — | ✓ · ✓ · ✓ |
| Selector | 78.3 kB (+519 B) · 15.8 kB CSS (+69 B) | — | — · — · — |
| DropdownMenu | 73.2 kB (+478 B) · 15.6 kB CSS (+86 B) | — | ✓ · ✓ · ✓ |
| ContextMenu | 74.6 kB (+479 B) · 15.5 kB CSS (+83 B) | — | — · — · — |
| Markdown | 145.2 kB (+488 B) · 18.4 kB CSS (+74 B) | — | — · — · — |
| MoreMenu · Storybook | 73.5 kB (+475 B) · 15.6 kB CSS (+86 B) | 92 (0) | ✓ · ✓ · ✓ |
| Table | 84.2 kB (+452 B) · 16.2 kB CSS (+91 B) | — | — · — · — |
| TableHeaderCell | 75.4 kB (+456 B) · 15.7 kB CSS (+86 B) | — | — · — · — |
| TableCell | 75.8 kB (+437 B) · 16.0 kB CSS (+86 B) | — | — · — · — |
| lab/Comments | 258.9 kB (+518 B) · 2.5 kB CSS (0) | — | — · — · — |
| SideNavHeading | 59.4 kB (+461 B) · 10.0 kB CSS (+48 B) | — | — · — · — |
| richtext/RichTextEditorToolbar | 225.5 kB (+507 B) · 454 B CSS (0) | — | — · — · — |
| TopNavHeading | 58.8 kB (+454 B) · 9.8 kB CSS (+52 B) | — | — · — · — |
| TopNavMenu | 58.6 kB (+451 B) · 9.9 kB CSS (+51 B) | — | — · — · — |
| NavHeadingMenu | 15.2 kB (+438 B) · 3.5 kB CSS (+46 B) | — | — · — · — |
| TabMenu | 56.0 kB (+432 B) · 9.8 kB CSS (+51 B) | — | — · — · — |
| lab/CommentThread | 253.6 kB (+463 B) · 2.4 kB CSS (0) | — | — · — · — |
| ContextMenuItem | 20.2 kB (+160 B) · 5.7 kB CSS (+73 B) | — | — · — · — |
| DropdownMenuItem | 17.9 kB (+93 B) · 5.6 kB CSS (+75 B) | — | ✓ · ✓ · — |
| DropdownMenuCheckboxItem | 18.7 kB (+91 B) · 6.0 kB CSS (+54 B) | — | ✓ · ✓ · ✓ |
| DropdownMenuRadioItem | 18.7 kB (+81 B) · 6.0 kB CSS (+54 B) | — | ✓ · ✓ · — |
| Switch | 86.9 kB (+22 B) · 9.5 kB CSS (−2 B) | — | — · — · — |
| SegmentedControl | 81.6 kB (+4 B) · 6.7 kB CSS (0) | — | — · — · — |
| DropdownMenuRadioGroup | 6.1 kB (0) · 3.4 kB CSS (+2 B) | — | ✓ · ✓ · — |
| DateInput | 147.2 kB (0) · 15.5 kB CSS (+1 B) | — | — · — · — |
| DropdownMenuGroup | 6.2 kB (0) · 3.4 kB CSS (+1 B) | — | ✓ · ✓ · — |
| Slider | 88.6 kB (+1 B) · 8.3 kB CSS (0) | — | — · — · — |
| AlertDialog | 83.2 kB (0) · 13.1 kB CSS (0) | — | — · — · — |
| DateTimeInput | 167.9 kB (0) · 17.2 kB CSS (0) | — | — · — · — |
| DropdownMenuDivider | 5.2 kB (0) · 548 B CSS (0) | — | ✓ · ✓ · — |
| TimeInput | 131.6 kB (−1 B) · 14.5 kB CSS (+1 B) | — | — · — · — |
Package size: @astryxdesign/core 6.33 MB packed (+10.3 kB); no new dependencies.
API changes: core +0 · ~2 · −0
Committed API report behind this build: cli 8 rows, core 2 rows, theme-butter 1 row, theme-chocolate 1 row, theme-gothic 1 row, theme-matcha 1 row, theme-neutral 1 row, theme-stone 1 row, theme-y2k 1 row; run pnpm api:report.
2 API changes (advisory)
| Package | Where | Signature | Usage | |
|---|---|---|---|---|
| ~ | core |
. · Components · component DropdownMenu |
prop presentation?: DropdownMenuPresentation = 'popover' → prop presentation?: DropdownMenuPresentation |
<DropdownMenu presentation={…} /> |
| ~ | core |
. · Components · component MoreMenu |
prop presentation?: DropdownMenuPresentation = 'popover' → prop presentation?: DropdownMenuPresentation |
<MoreMenu presentation={…} /> |
Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.
Generated by PR Enrichment workflow | View full report
meta-cla
Bot
added
the
CLA Signed
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested. The menu handoff itself matches AST-094 FR29: the finger that opened the menu picks only after moving 10 px, and presentation="popover" keeps the released rule. But this branch also carries the still-open #8076, #8298 and #8446, so the diff is about 21,400 lines for a roughly 1,100-line change. Please land those pull requests first, or wait for them, and rebase onto main so this one carries only its handoff change; if the resolver ships in the same release, also drop "No component reads it yet; behavior is unchanged" from .changeset/presentation-resolver.md.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] An internal `usePresentation` / `readPresentation` resolves a component's `presentation` prop into the surface to render and whether its touch input modes apply, keeping an omitted value distinct from an explicit `'popover'` so a caller's choice always wins over a touch default. No component reads it yet; behavior is unchanged. |
There was a problem hiding this comment.
At this head DropdownMenu reads usePresentation (packages/core/src/DropdownMenu/DropdownMenu.tsx:1784) and its behaviour changes under a finger (.changeset/menu-press-handoff.md), so "No component reads it yet; behavior is unchanged" is no longer true when both ship in one release. Ship this resolver in an earlier release, or drop that sentence.
There was a problem hiding this comment.
Changes requested. The menu handoff itself matches AST-094 FR29: under a finger, the finger that opened the menu picks a row only after moving 10 px, and presentation="popover" keeps the released rule. But this branch still carries the open #8076, #8298 and #8446 (17,553 authored lines for a 1,094-line change), and it now conflicts with main in packages/core/src/motion/animator.ts and packages/core/src/motion/README.md. Please land those pull requests first, or wait for them, then rebase onto main so this one carries only the handoff change.
[Automated review]
github-actions
Bot
added
the
needs:design-review
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved: the DropdownMenu handoff matches spec:AST-094 FR29, but the new lit-row paint has no settled spec. The row a held finger, pen or mouse lights now takes --color-overlay-pressed while keyboard focus keeps the hover overlay; FR27 says which row is lit but names no paint, and the current component:DropdownMenuItem FR8 ("After pointer input the row shows only its hover background") and module:DropdownMenu/useMenuPress AR1 ("share one highlight and one paint") describe the hover overlay. An engineering owner either approves the pressed paint, amending those clauses, or asks for it to leave this pull request, and records the decision by commenting /approve-spec <full head SHA> on the pull request; the author may do so as an engineering owner.
Required fix, whatever the decision, unless the paint is removed:
- With a mouse, a press that begins on Edit and drags onto Duplicate leaves both rows at the pressed overlay (the start row's
:activeand the new lit paint), so the row the release acts on cannot be told apart; limit the lit paint to a finger and a pen, or stop the row's:activepaint while a menu press tracks, and add a mouse case that starts on a row.
[Automated review]
| { | ||
| '--_menu-row-focus-paint': { | ||
| default: null, | ||
| '[data-astryx-menu-press-lit]': colorVars['--color-overlay-pressed'], |
There was a problem hiding this comment.
With a mouse, a press that begins on one row keeps that row's :active pressed paint (DropdownMenuItem.tsx, the @media (hover: hover) arm), so once the drag reaches another row this arm paints a second row at the same --color-overlay-pressed. In Chromium, pressing Edit and dragging onto Duplicate gives both rows alpha 0.1, and the release picks Duplicate. Limiting the mark's paint to a finger and a pen, or dropping the row's :active paint while a menu press tracks, keeps one lit row; a mouse case that starts on a row in DropdownMenuDragHighlight would pin it.
3d7fb45 to
6adca52
Compare
|
/approve-spec 2ce2374 |
There was a problem hiding this comment.
Changes requested. The DropdownMenu handoff matches spec:AST-094 FR29: under a finger or pen, the finger that opened the menu picks only after moving 10 px from the first position the open menu sees, and presentation="popover" keeps the released rule. The pressed lit-row paint now has an engineering owner's approval. One fix remains:
- With a pen on a device whose primary pointer hovers, a press that begins on Edit and drags onto Duplicate leaves both rows at the pressed overlay (Edit through the
@media (hover: hover):activearm inDropdownMenuItem.tsx, Duplicate through the new lit mark), so the row the release picks cannot be told apart, againstspec:AST-094FR27 andmodule:DropdownMenu/useMenuPressFR6; stop the start row's:activepaint under a pen or finger, for example by gating it on the press's pointer type rather than(hover: hover), and add a desktop pen case that begins on a row.
Non-blocking: the MENU_PRESS_SETTLE_MS JSDoc now calls the settle rule a mouse's, but the public hook, menuPressStep and presentation="popover" keep it for a finger too.
[Automated review]
| // paint. A held mouse keeps the hover tint, as on a desktop. | ||
| const gesture = gestureRef.current; | ||
| const lit = | ||
| gesture.phase !== 'idle' && gesture.pointerType !== 'mouse' |
There was a problem hiding this comment.
With a pen on a desktop ((hover: hover) true), a press that begins on one row keeps that row's :active pressed paint (DropdownMenuItem.tsx, the @media (hover: hover) arm), so once the drag reaches another row this mark paints a second row at the same --color-overlay-pressed. In Chromium, a pen pressed on Edit and dragged onto Duplicate gives both rows alpha 0.1, and the release picks Duplicate (spec:AST-094 FR27). Gating that :active arm on the press's pointer type, rather than on hover, would keep one pressed row; a desktop pen case that starts on a row would pin it.
| /** How long the opening press must last before its release may act. */ | ||
| import {MENU_PRESS_HANDOFF_PX as UIKIT_MENU_PRESS_HANDOFF_PX} from './menuPressValues'; | ||
|
|
||
| /** How long a mouse's opening press must last before its release may act. */ |
There was a problem hiding this comment.
Non-blocking: the public menuPressStep, useMenuPress and presentation="popover" keep this rule for a finger too. Suggest: /** How long the opening press must last before its release may act, unless a finger's handoff applies. */
There was a problem hiding this comment.
This looks ready to approve. Under a finger or pen, the finger that opened a DropdownMenu or MoreMenu now picks a row only after moving 10 px, as spec:AST-094 FR29 requires, with presentation="popover" as the way back; the pressed lit-row paint matches the owner-approved component:DropdownMenuItem FR8 and module:DropdownMenu/useMenuPress AR1. The earlier desktop pen case is fixed: a pen pressed on one row and dragged onto another now leaves only the row under it at the pressed overlay.
Optional, non-blocking:
useMenuPress.spec.md's Design relationships and DEC-6 still describe the highlight paint as the focus overlay alone; a short amendment would match the new AR1.- The
presentationJSDoc inDropdownMenu.tsxends with "as before", which will read oddly after release. - The
menuPressLit.stylex.tsheader's@outputline restates the export names and could shrink to their purpose.
[Automated review]
| one highlight and one paint; in Core's menus, a held finger or pen paints its | ||
| lit row with the pressed overlay (a public `useMenuPress` caller gets the | ||
| highlight, the lit row's focus, and styles it itself); a row whose root takes | ||
| no focus is highlighted through the control inside it. |
There was a problem hiding this comment.
Non-blocking: this record still says, under Design relationships, "the highlight paint itself is the existing focus overlay owned by the item components", and DEC-6 says keyboard, mouse and touch share "one state and one paint". Both now disagree with this AR1. Suggest for Design relationships: "The highlight paint is the item components' focus overlay, or their pressed overlay while a held finger or pen lights the row (AR1)."
| * Omitted, the menu is the anchored popover. Under a finger, on every | ||
| * device, the finger that opened it picks as UIMenu's does, once it has | ||
| * moved. `'popover'` pins the anchored popover with the pointer behavior on | ||
| * every device (the opening press follows the settle rule, as before). |
There was a problem hiding this comment.
Non-blocking: "as before" narrates the change and means nothing once this ships (the same words are in the compound JSDoc below). Suggest: "'popover' pins the anchored popover and the settle rule on every device."
| * @output markMenuPressLit and clearMenuPressLit, which mark that row, | ||
| * isMenuPressLit, which reads the mark, menuPressLitRowStyle, which | ||
| * paints the marked row, holdMenuPressPaint and releaseMenuPressPaint, | ||
| * which keep the other rows off their pressed paint, and | ||
| * isMenuPressPaintHeld, which reads that hold |
There was a problem hiding this comment.
Non-blocking: the header runs 21 lines and this @output restates the six export names. Suggest: @output The private menu-press-lit attribute: the row mark, the menu's paint hold, and the row style that reads them.
|
/approve-spec ddc4575 |
|
/approve-spec 99489b4 |
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet, because the pull request is a draft on a fork branch. Once it is marked ready for review, a maintainer adds the ci:run label so CI runs.
Under a finger or pen, the finger that opened a DropdownMenu or MoreMenu now picks a row only after it travels 10 px, as spec:AST-094 FR29 requires, with presentation="popover" as the way back; the pressed lit-row paint matches the owner-approved component:DropdownMenuItem FR8 and module:DropdownMenu/useMenuPress AR1. A mouse and the keyboard paint exactly as before.
Optional, non-blocking:
useMenuPress.spec.md's Design relationships and DEC-6 still describe one shared focus paint; a one-line amendment of each would match the new AR1.- The
presentationJSDoc inDropdownMenu.tsxends with "as before", which will read oddly after release. - The
menuPressLit.stylex.tsheader's@outputrestates the export names and could shrink to their purpose.
[Automated review]
|
/approve-spec 2161faa |
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet, because the pull request is a draft on a fork branch. Once it is marked ready for review, a maintainer adds the ci:run label so CI runs.
Under a finger or pen, the finger that opened a DropdownMenu or MoreMenu now picks a row only after it travels 10 px, as spec:AST-094 FR29 requires, with presentation="popover" as the way back; the pressed lit-row paint matches the approved component:DropdownMenuItem FR8 and module:DropdownMenu/useMenuPress AR1. The new unit tests and both new Chromium interaction specs pass, and a held mouse renders exactly as before.
Optional, non-blocking:
useMenuPress.spec.mdstill describes one shared focus paint under Design relationships and in DEC-6; a one-line amendment of each would match the new AR1.- The
presentationJSDoc inDropdownMenu.tsx(lines 466 and 476) ends with "as before", which will read oddly once this ships.
[Automated review]
| * Omitted, the menu is the anchored popover. Under a finger, on every | ||
| * device, the finger that opened it picks as UIMenu's does, once it has | ||
| * moved. `'popover'` pins the anchored popover with the pointer behavior on | ||
| * every device (the opening press follows the settle rule, as before). |
There was a problem hiding this comment.
Non-blocking: "as before" narrates the change and means nothing once this ships (the compound JSDoc at line 476 has the same words). Suggest: "'popover' pins the anchored popover and the settle rule on every device."
There was a problem hiding this comment.
Changes requested: one formatting fix stands between this pull request and approval.
- Found by CI: the Lint check fails because
DropdownMenuTouchHandoff.interaction.browser.spec.ts(line 18) andDropdownMenuDragHighlight.interaction.browser.spec.ts(line 20) keep their new@astryxdesign/a11y-spec/playwrightimport on one 89-character line; runpnpm exec prettier --writeon both files and push.
Otherwise the change holds up: under a finger or pen, the finger that opened a DropdownMenu or MoreMenu picks a row only after it travels 10 px, as spec:AST-094 FR29 requires, with presentation="popover" as the way back, and the pressed lit-row paint matches the approved component:DropdownMenuItem FR8 and module:DropdownMenu/useMenuPress AR1. The unit tests and both new Chromium interaction specs pass, and a held mouse renders exactly as before.
Optional, non-blocking:
useMenuPress.spec.mdstill describes one shared focus paint under Design relationships and in DEC-6; a one-line amendment of each would match the new AR1.- The
presentationJSDoc inDropdownMenu.tsx(lines 466 and 476) ends with "as before", which will read oddly once this ships.
[Automated review]
| * and useMenuPress.test.tsx | ||
| */ | ||
|
|
||
| import {expect, test, type Locator, type Page} from '@astryxdesign/a11y-spec/playwright'; |
There was a problem hiding this comment.
Found by CI: Lint's Prettier step fails on this 89-character line. pnpm exec prettier --write wraps it:
import {
expect,
test,
type Locator,
type Page,
} from '@astryxdesign/a11y-spec/playwright';| * press model's mark on that row is unit-tested in useMenuPress.test.tsx | ||
| */ | ||
|
|
||
| import {expect, test, type Locator, type Page} from '@astryxdesign/a11y-spec/playwright'; |
There was a problem hiding this comment.
Found by CI: Lint's Prettier step fails on this 89-character line. pnpm exec prettier --write wraps it:
import {
expect,
test,
type Locator,
type Page,
} from '@astryxdesign/a11y-spec/playwright';| * Omitted, the menu is the anchored popover. Under a finger, on every | ||
| * device, the finger that opened it picks as UIMenu's does, once it has | ||
| * moved. `'popover'` pins the anchored popover with the pointer behavior on | ||
| * every device (the opening press follows the settle rule, as before). |
There was a problem hiding this comment.
Non-blocking: "as before" narrates the change and means nothing once this ships (the compound JSDoc at line 476 has the same words). Suggest: "'popover' pins the anchored popover and the settle rule on every device."
… moves (spec:AST-094 FR29, FR43) Under a finger or pen, on every device, a DropdownMenu or MoreMenu opened under a resting finger picked the row beneath it once the press had lasted 300 ms. The press machine's open phase now takes UIMenu's handoff: until the finger is 10 px, in a straight line, from the first position the open menu sees, nothing lights and a lift acts on nothing; then it slides and picks as usual. A mouse keeps the settle rule, and presentation="popover" pins the pointer model. - menuPressGesture: the handoff state and transitions, on internal types; the released menuPressStep keeps its public shapes. - useMenuPress: the internal useDropdownMenuPress entry takes a touch model; the public hook is unchanged. - menuPressValues: UIKit's handoff distance, copied from the constants table so a menu bundles the number, not the table. - DropdownMenu and MoreMenu run the touch model unless presentation is "popover"; docs and a changeset say so. - A real-browser spec (DropdownMenuTouchHandoff); core declares the interaction harness it imports.
… carry positions The released menuPressStep hands its events to the internal transition, which starts the finger's handoff when a move carries x and y. The public gesture drops the handoff origin, so a caller whose events carried positions got no pick from the opening finger. menuPressStep now strips the positions first, and a test whose events carry them pins it. Three comments are reworded.
…sed overlay (spec:AST-094 FR27, FR43) Under a finger, the hover overlay that marks a focused row is too faint to see, so a held finger sliding across the rows showed no highlight. The press model now marks the row a held finger or pen lights, through a private attribute owned by privateAttributes/menuPressLit.stylex.ts, and DropdownMenu's rows paint that row with the pressed overlay. A held mouse, the keyboard and a hover keep the hover overlay.
…finger or pen paints component:DropdownMenuItem FR8 and AR4, and module:DropdownMenu/useMenuPress AR1, described only the hover overlay after pointer input. A held finger or pen now paints its lit row with the pressed overlay (spec:AST-094 FR27); a held mouse, the keyboard and a hover keep the hover overlay.
…R4 cites FR8 module:DropdownMenu/useMenuPress AR1: in Core's menus, a held finger or pen paints its lit row with the pressed overlay; a public useMenuPress caller gets the highlight mark and paints it itself. component:DropdownMenuItem AR4 cites FR8 for the background after pointer input instead of repeating it.
…hlight it styles module:DropdownMenu/useMenuPress AR1: a public useMenuPress caller gets the highlight, the lit row's focus, and styles it itself. The private lit-row attribute stays out of the caller's surface.
…at the pressed overlay On a desktop the row a press began on stays :active while the press moves, and DropdownMenuItem paints :active where a pointer hovers. So a pen pressed on one row and dragged onto another left both at the pressed overlay, and the row the release picks could not be told apart (spec:AST-094 FR27). While a finger or pen is held, the press model sets the rows' :active paint to none on the menu; the lit row keeps the pressed overlay through its own mark. A held mouse keeps every row's own :active look. Also: MENU_PRESS_SETTLE_MS's JSDoc no longer calls the settle rule a mouse's.
|
/approve-spec 5523889 |
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet, because its required checks wait for a maintainer's ci:run label on this fork branch. A maintainer adds the ci:run label so CI runs.
The earlier Lint failure on the two new browser specs is fixed, and under a finger or pen the finger that opened a DropdownMenu or MoreMenu now picks a row only after travelling 10 px (spec:AST-094 FR29), with presentation="popover" as the way back and the pressed lit-row paint an engineering owner approved in component:DropdownMenuItem FR8 and module:DropdownMenu/useMenuPress AR1.
Optional, non-blocking:
useMenuPress.spec.mdstill describes one shared focus paint under Design relationships and in DEC-6; a one-line amendment of each would match the new AR1.- The
presentationJSDoc inDropdownMenu.tsx(lines 466 and 476) ends with "as before", which will read oddly once this ships.
[Automated review]
|
/approve-spec c97a5d3 |
There was a problem hiding this comment.
Changes requested: one CI failure stands between this pull request and approval.
- Found by CI:
test-node (3/4)failsscripts/build-css.test.mjs"emits background-color pressed after hover at equal specificity", because itsstateRulematcher now takes DropdownMenuItem's new:activevalue,var(--_menu-row-active-paint, var(--color-overlay-pressed))(DropdownMenuItem.tsxline 79), for the shared pressed rule, and that rule sits before the shared hover rule inastryx.css; skipping declarations whose token is another variable's fallback (for example!/var\(--[\w-]+,var\(--color-overlay-/.test(line)) makes both of that file's ordering cases pass on this head, and the shared pressed rule still follows the hover rule.
The handoff itself matches spec:AST-094 FR29 and FR25, the pressed lit-row paint has an engineering owner's spec approval on this head, and the unit tests and both Chromium interaction specs pass after the merge of main.
Optional, non-blocking: useMenuPress.spec.md still describes one shared focus paint under Design relationships and in DEC-6, and the presentation JSDoc in DropdownMenu.tsx (lines 466 and 476) ends with "as before".
[Automated review]
| default: null, | ||
| ':active:where(:not(:disabled,[aria-disabled="true"]))': | ||
| colorVars['--color-overlay-pressed'], | ||
| ':active:where(:not(:disabled,[aria-disabled="true"]))': `var(--_menu-row-active-paint, ${colorVars['--color-overlay-pressed']})`, |
There was a problem hiding this comment.
Found by CI: test-node (3/4) fails scripts/build-css.test.mjs "emits background-color pressed after hover at equal specificity" (expected 161681 to be greater than 162059). Its stateRule takes the first @media (hover: hover) :active line containing var(--color-overlay-pressed), which is now this arm's var(--_menu-row-active-paint,var(--color-overlay-pressed)), emitted before the shared hover rule. The shared pressed rule still follows the hover rule, so the fix belongs in the test: skip a declaration whose token is another variable's fallback, for example:
line.includes(`var(--color-overlay-${token})`) &&
!/var\(--[\w-]+,var\(--color-overlay-/.test(line)With that filter both the background-color and background-image cases pass on this head.
…paint The pressed-after-hover guard took the first :active rule that names the pressed overlay. DropdownMenuItem's pressed paint names it only as the fallback of its lit-row variable. That rule never meets the shared hover overlay on one element, and its place in astryx.css follows the build's file walk. The guard now skips a token used as another variable's fallback, so it orders the shared pair alone.
|
/approve-spec bd9134c |
There was a problem hiding this comment.
Approved pending CI: CI is running again on this head, so the required checks have not finished yet.
The earlier scripts/build-css.test.mjs failure is fixed: the ordering guard now skips a row's own fallback paint, passes on this head (the CI shard that failed before passed in the first run here), and still catches the shared pressed rule landing before the shared hover rule. The finger handoff and the pressed lit-row paint match spec:AST-094 FR29, FR25 and FR27 and the record amendments an engineering owner approved for this head.
Optional follow-ups:
useMenuPress.spec.mdstill says the highlight paint is "the existing focus overlay" (Design relationships) and that keyboard, mouse and touch share "one state and one paint" (DEC-6); a one-line edit to each would match the amended AR1.- The
presentationJSDoc inDropdownMenu.tsx(lines 466 and 476) ends with "as before"; "'popover'pins the anchored popover and the settle rule on every device." says the same without the history.
[Automated review]
| * Omitted, the menu is the anchored popover. Under a finger, on every | ||
| * device, the finger that opened it picks as UIMenu's does, once it has | ||
| * moved. `'popover'` pins the anchored popover with the pointer behavior on | ||
| * every device (the opening press follows the settle rule, as before). |
There was a problem hiding this comment.
Non-blocking (INV29): "as before" narrates the change and means nothing once this ships; the compound JSDoc at line 476 has the same words. Suggest: "'popover' pins the anchored popover and the settle rule on every device."
It also fixes a phone-check defect: a held finger's lit row was too faint (
spec:AST-094FR27).User need
A finger held on a DropdownMenu or MoreMenu trigger opens the menu under it, and after 300 ms, lifting picks the row beneath. On iOS, the opening finger picks only after it moves.
What led to it: UIMenu on the iOS 26.2 simulator (
spec:AST-094FR29), under every pointer (FR43).Problem and solution fit
For a finger or pen, the opening press counts distance, not time: until it is 10 px, in a straight line, from the open menu's first sample, nothing lights and lifting picks nothing. A mouse is unchanged. A held finger or pen lights only its row with the pressed overlay; a held mouse, keyboard and hover keep the hover overlay.
Current specification
spec:AST-094FR27, FR29, FR43 and FR25;module:DropdownMenu/useMenuPressFR9 (current). Record lines (stamped):component:DropdownMenuItemFR8 (AR4 cites it) andmodule:DropdownMenu/useMenuPressAR1 name the pressed overlay a held finger or pen paints.Public delta and caller burden
The opening finger moves 10 px before picking (
presentation="popover"opts out). No new option or type; the handoff and the lit-row mark are internal.Design and alternatives
Rejected: a longer wait (a resting finger still picks); a per-axis distance (iOS picks (9, 9), not (7, 7)); counting from the open's position. Chosen: 10 px in a straight line from the first sample.
Evidence
iOS 26.2 simulator, after a 1 pt first step: (0, 9) and (7, 7) picked nothing; (0, 10), (10, 0), (9, 9) and (10, 10) picked.
menuPressStep's settle rule for events with positions.Since the touch re-check at 3d7fb45
Rebased onto main (range-diff
=). CodeBunny's desktop pen case adds a touch-path change beyond the mouse branch: a held finger or pen keeps the other rows off their:activepaint. Card 7b needs a re-tap.Writing rubric results
R1 narration, R2 research, R4 rejected alternatives, R5 change references and R6 open questions: none. R3 evidence: the new clause cites
spec:AST-094FR27; FR8's basis cell is unchanged.Scope
Testing
This head adds one test-only commit on the merge of main (c97a5d3).
scripts/build-css.test.mjs's pressed-after-hover guard took DropdownMenuItem's own row paint,var(--_menu-row-active-paint, var(--color-overlay-pressed)), for the shared pressed overlay, since it now comes first in astryx.css; that class never shares an element with the shared hover class. The guard now skips a token used as another variable's fallback. At c97a5d3 the guard fails; at this head it passes (12 of 12) on the same build. A NOT-FOR-LANDING control (290b738) that moves the shared pressed rule before the shared hover rule fails it. On a merge of this head into main b93ee91:pnpm test:node --changed=HEAD^1(12 of 12) and CI's source-reading guard list for shard 3 of 4 (3,258 passed); across all four shards in one run, five CLI search and doctor files timed out on the loaded box and pass on their own here and on main.On the DropdownMenu "Default" story, with touch emulation:
With main's press model, DropdownMenu and MoreMenu restored at cf176dc,
DropdownMenuTouchHandofffailed in Chromium: the 9 px lift picked "Edit". It passes here in Chromium.With the press model and rows as at 6d0389c (NOT-FOR-LANDING arm bcaf523), the four new finger and pen
useMenuPresscases fail; with the press model as at this PR's earlier head 3d7fb45 (arm 7759ac2), which marked a mouse row too, the mouse case fails. Against the storybook built at 6d0389c,DropdownMenuDragHighlightfails for the finger and the pen; against the one built at 3d7fb45, its two mouse cases fail (the drag from the trigger, and the desktop press that begins on a row).This head is 99489b4's seven commits replayed on main 217fc46 with no conflict. Main now bars
@playwright/testin browser specs, so the two specs this pull request adds,DropdownMenuTouchHandoffandDropdownMenuDragHighlight, import from@astryxdesign/a11y-spec/playwright, in Prettier's multi-line form; every other patch matches 99489b4's (range-diff). Local gates at 2161faa, the same tree but for that wrap: units across DropdownMenu, hooks and the menus (2,410 of 2,410), the core type check, and lint on the two specs (a copy with the old import trips the rule); in Chromium, on a fresh Storybook build, the DropdownMenu a11y specs (166 of 166) and the two interaction specs (7 of 7); in WebKit, through the container recipe, 5 passed and 2 skipped (the desktop-pointer cases). Local gates at a016e38d90, which differs from ddc4575 in one header comment: the DropdownMenu, ContextMenu, Selector, MoreMenu, hooks, utils, private-attribute and theme units (4,303 of 4,303), the core type check, strict lint andreview:pre(no findings); in Chromium the menu interaction specs (7 of 7,DropdownMenuDragHighlight's five included). With the press model and row as at 2ce2374 (NOT-FOR-LANDING arm b6388181ae), the three new hold cases fail; against a storybook built from arm eccba8bb64, the desktop pen case fails (Edit at alpha 0.1). Earlier, at d456749: the menu a11y set (300 of 300) and the handoff drill (5 runs); at 6adca52,DropdownMenuDragHighlightin WebKit (the pen cases skip: Playwright has no pen there).Stack: this PR → #8078.