diff --git a/.changeset/create-menu-press.md b/.changeset/create-menu-press.md new file mode 100644 index 0000000000000..d917fdbd405ff --- /dev/null +++ b/.changeset/create-menu-press.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/core': patch +--- + +[feat] `createMenuPress(menu, options)` runs the menu press model on any element without React: the row under a release acts, the highlight follows a held pointer, a mouse press on `trigger` opens the menu, and a finger held on it opens it after the long-press delay. `update(options)` merges new options, which apply from the next event without ending a press in progress. `startBehavior: 'items'` starts a press only on an item, so other content in the menu keeps its own clicks. `destroy()` removes every listener it added. `useMenuPress(menuRef, options)` is its React form: `triggerRef` names the trigger, and there is nothing to spread. Its press exists only while the menu element is mounted, so a menu that opens on a press of its trigger keeps its element mounted while closed. Deprecated (DEP-0019), and removed in 0.8.0: the options form, `useMenuPress({menuRef, …})` with its `menuProps` and `triggerProps`, which still works as before. `DropdownMenuTriggerProps.onPointerDown` and `onContextMenu` stay: a custom trigger keeps spreading them as its forward, for exact mouse and pen parity, until 0.8.0 moves the press to the trigger's own listener. `astryx upgrade` moves calls to the ref form and drops redundant forwards of the two handlers. DropdownMenu, BreadcrumbItem, NavHeadingMenu, TopNavMenu and the SideNav and TopNav heading menus move to the ref form. One behavior changes for a finger: a finger's press-down that a page handler stops or cancels, inside or outside the menu, now starts no press, so on a page that cancels every press-down, touch menus lose press-drag-release; plain taps still work. A mouse or pen press behaves as before in Core's menus, which forward it from their React handlers through the new `handleMenuPointerDown`. In the ref form, a mouse's or pen's press-down acts at the menu's or trigger's own listener, before any React `onPointerDown` handler; `astryx upgrade` leaves a TODO where an element written inside the menu or the trigger sets `onPointerDown` or spreads props, or where they hold a component or a child it cannot read. + +@vjeux diff --git a/.changeset/menu-press-codemod.md b/.changeset/menu-press-codemod.md new file mode 100644 index 0000000000000..a7339306ba8ff --- /dev/null +++ b/.changeset/menu-press-codemod.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/cli': patch +--- + +[feat] `astryx upgrade` moves `useMenuPress({menuRef, …})` calls to the ref form, `useMenuPress(menuRef, options)`, and drops the spreads of its `menuProps` and `triggerProps`. It also drops a DropdownMenu trigger's redundant forwards of `onPointerDown` and `onContextMenu` beside a spread of the trigger props. Where it cannot prove the result, it leaves the code unchanged with a TODO. + +@vjeux diff --git a/docs/specs/AST-055-caller-rendered-trigger/spec.md b/docs/specs/AST-055-caller-rendered-trigger/spec.md index 6d931d9dd0b37..f9244c7e5eede 100644 --- a/docs/specs/AST-055-caller-rendered-trigger/spec.md +++ b/docs/specs/AST-055-caller-rendered-trigger/spec.md @@ -150,16 +150,16 @@ INV2), so the decision lives here and those records cite it. ### Behavioral contract -| ID | Invariant | Basis | Verification state | -| --- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------- | ---------------------------------------- | -| FR1 | A component that lets a caller supply the control which opens its overlay MUST take that control as a render prop named `renderTrigger` (DEC-5): a function the component calls with one props object, whose returned element the component renders as given. The component MUST NOT wrap the returned element in an element of its own, and MUST NOT require a particular tag or a particular Astryx component. | DEC-1; `spec:AST-002` FR4 | Accepted; jsdom evidence pending | -| FR2 | Spreading the handed-back props onto one element MUST be the whole integration. The object MUST carry the ref or anchor the overlay positions against, the `id` AR1 needs, every handler that opens, toggles, and closes the overlay from that control on the component's supported input paths, and the disclosure attributes of AR1. The object's type MUST be exported from the component's public entry point. | DEC-1; `architecture:public-component-api` INV1, INV5 | Accepted; jsdom evidence pending | -| FR3 | A caller-supplied control MUST NOT be taken as a `ReactNode` slot that the component renders inside an interactive element of its own. Callers routinely pass an interactive component, and that shape nests one interactive element inside another (AR2). A content slot rendered inside a component-owned control remains valid for non-interactive content and is unchanged. | DEC-1; AR2 | Accepted; jsdom evidence pending | -| FR4 | The caller's control MUST own a box. A component MUST NOT render it inside a wrapper with no box of its own (`display: contents`), and MUST NOT depend on such a wrapper where it paints or offsets a focus indicator on the trigger, or positions the overlay against the trigger's rect: there is no rect to anchor to and no box on which to paint. FR1 removes the need for any wrapper. | DEC-2; `architecture:layer-runtime` anchor contract | Accepted; real-browser evidence required | -| FR5 | Hover, pressed, and every other pointer paint on the caller's control MUST remain the caller's own CSS. The component MUST NOT write paint onto an element it does not render. The component MUST publish the overlay's open state on the handed-back props as `aria-expanded`, so a caller styles the open state from the rendered attribute and no additional public API is added for it. | DEC-3; `spec:AST-002` FR1, FR2 | Accepted; jsdom evidence pending | -| FR6 | The prop's consumer documentation MUST show styling the open state from `aria-expanded` on the caller's control, and MUST state that a pressed look keyed to `:active` is not a substitute, because `:active` does not behave the same under a finger, and press paint follows `spec:AST-094` FR11's profiles instead. | DEC-3; no current core component styles off `aria-expanded`, so callers have no example to copy | Accepted; documentation evidence pending | -| FR7 | Where a component offers both a built-in control and this prop, exactly one control MUST render. The caller's control MUST win, identically in every presentation the component supports, and a development warning MUST fire; the component's own chrome for the replaced control MUST NOT render. Silently ignoring a supplied prop, or rendering both, is rejected. | `spec:AST-002` FR15 (never silently render a broken state); `architecture:public-component-api` INV3 | Accepted; jsdom evidence pending | -| FR8 | A hook that returns props for a trigger — `triggerProps` and `getTriggerProps` on `useLightbox`, `usePopover`, `useMenuHover`, and `useHoverCard` — follows hook conventions and is unchanged by this record. Those return-value names are NOT precedent for a component prop's name or shape. This record does not require a hook to grow a render prop, nor a component to republish its trigger props as a hook return. | DEC-4 | Settled by DEC-4; no code change | +| ID | Invariant | Basis | Verification state | +| --- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------- | ---------------------------------------- | +| FR1 | A component that lets a caller supply the control which opens its overlay MUST take that control as a render prop named `renderTrigger` (DEC-5): a function the component calls with one props object, whose returned element the component renders as given. The component MUST NOT wrap the returned element in an element of its own, and MUST NOT require a particular tag or a particular Astryx component. | DEC-1; `spec:AST-002` FR4 | Accepted; jsdom evidence pending | +| FR2 | Spreading the handed-back props onto one element MUST be the whole integration. The object MUST carry the ref or anchor the overlay positions against, the `id` AR1 needs, every handler that opens, toggles, and closes the overlay from that control on the component's supported input paths (or the ref, where the component listens on the control itself), and the disclosure attributes of AR1. The object's type MUST be exported from the component's public entry point. | DEC-1; `architecture:public-component-api` INV1, INV5 | Accepted; jsdom evidence pending | +| FR3 | A caller-supplied control MUST NOT be taken as a `ReactNode` slot that the component renders inside an interactive element of its own. Callers routinely pass an interactive component, and that shape nests one interactive element inside another (AR2). A content slot rendered inside a component-owned control remains valid for non-interactive content and is unchanged. | DEC-1; AR2 | Accepted; jsdom evidence pending | +| FR4 | The caller's control MUST own a box. A component MUST NOT render it inside a wrapper with no box of its own (`display: contents`), and MUST NOT depend on such a wrapper where it paints or offsets a focus indicator on the trigger, or positions the overlay against the trigger's rect: there is no rect to anchor to and no box on which to paint. FR1 removes the need for any wrapper. | DEC-2; `architecture:layer-runtime` anchor contract | Accepted; real-browser evidence required | +| FR5 | Hover, pressed, and every other pointer paint on the caller's control MUST remain the caller's own CSS. The component MUST NOT write paint onto an element it does not render. The component MUST publish the overlay's open state on the handed-back props as `aria-expanded`, so a caller styles the open state from the rendered attribute and no additional public API is added for it. | DEC-3; `spec:AST-002` FR1, FR2 | Accepted; jsdom evidence pending | +| FR6 | The prop's consumer documentation MUST show styling the open state from `aria-expanded` on the caller's control, and MUST state that a pressed look keyed to `:active` is not a substitute, because `:active` does not behave the same under a finger, and press paint follows `spec:AST-094` FR11's profiles instead. | DEC-3; no current core component styles off `aria-expanded`, so callers have no example to copy | Accepted; documentation evidence pending | +| FR7 | Where a component offers both a built-in control and this prop, exactly one control MUST render. The caller's control MUST win, identically in every presentation the component supports, and a development warning MUST fire; the component's own chrome for the replaced control MUST NOT render. Silently ignoring a supplied prop, or rendering both, is rejected. | `spec:AST-002` FR15 (never silently render a broken state); `architecture:public-component-api` INV3 | Accepted; jsdom evidence pending | +| FR8 | A hook that returns props for a trigger — `triggerProps` and `getTriggerProps` on `useLightbox`, `usePopover`, `useMenuHover`, and `useHoverCard` — follows hook conventions and is unchanged by this record. Those return-value names are NOT precedent for a component prop's name or shape. This record does not require a hook to grow a render prop, nor a component to republish its trigger props as a hook return. | DEC-4 | Settled by DEC-4; no code change | ### Accessibility contract diff --git a/docs/specs/components/DropdownMenu/DropdownMenu.spec.md b/docs/specs/components/DropdownMenu/DropdownMenu.spec.md index 2eb77ab775fad..991211086e45e 100644 --- a/docs/specs/components/DropdownMenu/DropdownMenu.spec.md +++ b/docs/specs/components/DropdownMenu/DropdownMenu.spec.md @@ -129,7 +129,7 @@ component-local concepts; each keeps its released default. | FR6 | When the pointer menu closes, focus returns to the Trigger button with a visible ring only when the menu was driven by keyboard; a press outside that landed on a focusable control keeps focus there. | `architecture:interaction-modality` INV1; DEC-1 | | FR7 | `menuMaxHeight` replaces the 300px term of the menu's and its popover viewport's block-size cap while the viewport gutters still bound both; the touch sheet ignores it. | DEC-2 | | FR8 | The Trigger button opens the pointer menu on a mouse press-down, with a settle rule on the opening release, and pressing the trigger of an open menu closes it without reopening in the same gesture; a mouse click and the keyboard open as before. The mouse press-open is shipped behavior, not a requirement until `module:DropdownMenu/useMenuPress` DEC-4 is ruled. Under a finger the trigger MUST open the menu as `spec:AST-094` FR29 states: as the finger presses in a fixed bar, after the 150 ms content-touch delay or at a tap's lift inside a scroll view; `presentation="popover"` keeps the tap open. Implementation gap: a held finger opens it after a 500 ms hold, and a tap opens it at its click. | `module:DropdownMenu/useMenuPress` FR8, FR9; `spec:AST-094` FR29 | -| FR9 | With `renderTrigger`, the rendered control carries `aria-haspopup`, `aria-expanded`, `aria-controls` and the `id` the menu's `aria-labelledby` points at, and receives the same press-model, keyboard-open and toggle-click handlers as the Button path; the menu is named by that control. A disabled custom trigger never opens the menu or the sheet. `button` and `renderTrigger` are mutually exclusive: both given, `renderTrigger` wins and a dev warning fires, in both presentations. A drilled-in Back row (FR11) returns to the menu by that same name: the caller's `aria-label` when given, else the control's accessible name, read as the menu opens. | DEC-3 | +| FR9 | With `renderTrigger`, the rendered control carries `aria-haspopup`, `aria-expanded`, `aria-controls` and the `id` the menu's `aria-labelledby` points at, and receives the same keyboard-open and toggle-click handlers as the Button path; a finger's press reaches the menu through the trigger props' `ref`, and a mouse's or pen's through their `onPointerDown` and `onContextMenu`, which the control spreads (DEC-11). The menu is named by that control. A disabled custom trigger never opens the menu or the sheet. `button` and `renderTrigger` are mutually exclusive: both given, `renderTrigger` wins and a dev warning fires, in both presentations. A drilled-in Back row (FR11) returns to the menu by that same name: the caller's `aria-label` when given, else the control's accessible name, read as the menu opens. | DEC-3 | | FR10 | An action row with `href` renders as one anchor carrying `role="menuitem"` (the pointer menu) or the touch sheet's link row, routed through `LinkProvider`; a plain click runs `onClick`, then closes the menu (`hasCloseOnSelect`), then lets the browser navigate; a modified click does not run `onClick`, closes the menu, and is left to the browser; in the pointer menu a middle click closes the menu (`hasCloseOnSelect`), then runs the caller's `onAuxClick`, while the touch sheet leaves a middle click to the browser and stays open; a right click is left to the context menu; Enter and Space activate through a synthesized click that carries the key's modifiers. A disabled link row has no `href`; a blocked destination renders inertly. | Current behavior; `family:navigation-destinations` | | FR11 | When a compact touch display opened the pointer menu (the query the root presentation uses for its bottom sheet, sampled once as it opens), an `adaptive` sub-menu row drills in: its rows and a leading Back row named "Back to " replace the menu's rows in the same Pointer menu surface, no flyout opens, the drilled list is a `menu` named by the row's label, and Back, Escape and ArrowLeft return to the row with focus on it; a pick inside closes the whole menu, and closing the menu resets the drilled view. Roving focus and typeahead scope to the shown rows; sibling rows and dividers need no knowledge of it. `presentation="flyout"` keeps the flyout; `"drill-in"` drills in on any pointer. | DEC-4; `spec:AST-094` FR34 | | FR12 | In the pointer menu ArrowDown on the last enabled row wraps to the first and ArrowUp on the first to the last; PageDown and PageUp move to the last and first fully visible enabled row and, pressed there again, one viewport further without wrapping; ArrowUp on the trigger opens with the last enabled row highlighted; the key that opened the menu and its auto-repeats do not activate; typeahead matches the row's label element alone and ignores Control/Command chords and input-method composition. | Current source and tests | @@ -548,6 +548,20 @@ pattern requires; Escape and a press outside close it. Without either prop the component is unchanged: a Menu Button whose Tab closes the menu. Rejected: closing on Tab past either end, which leaves a modal dialog by Tab. +### DEC-11 — The trigger's press handlers stay until 0.8.0 + +**Reference:** `component:DropdownMenu/DEC-11` + +**Decider:** `cixzhang`, `2026-11-03` + +`DropdownMenuTriggerProps.onPointerDown` and `onContextMenu` are a +`renderTrigger` control's forward of a mouse or pen press, so it stays exactly +as released. The menu listens on the control itself for a finger, through its +`ref`. A caller that spreads them hands the same event to the press, which +takes it once. They are not deprecated now. At the next minor release, CLN-0030 +of `module:DropdownMenu/useMenuPress` moves a mouse's or pen's press to the +trigger's own listener and deprecates them. + ## Open questions - **OQ1 — Which shared owner should make disabled menu rows keyboard-discoverable diff --git a/docs/specs/components/DropdownMenu/modules/useMenuPress.spec.md b/docs/specs/components/DropdownMenu/modules/useMenuPress.spec.md index 6b613357e9174..036f029658310 100644 --- a/docs/specs/components/DropdownMenu/modules/useMenuPress.spec.md +++ b/docs/specs/components/DropdownMenu/modules/useMenuPress.spec.md @@ -88,14 +88,47 @@ adopter. `yes` for every consumer prop and every item, divider, section and presentation API - Compatibility class: additive public hook and types; behavior change inside - the menu family -- Migration decision: `module:DropdownMenu/useMenuPress/DEC-3`; proposed + the menu family; the options form is deprecated (DEP-0019) +- Migration decision: `module:DropdownMenu/useMenuPress/DEC-3` and + `module:DropdownMenu/useMenuPress/DEC-9`; proposed `module:DropdownMenu/useMenuPress/DEC-4` (pending owner review); `spec:AST-094` FR25 (a caller that needs the released 500 ms wait passes - `longPressDelayMs: 500`) + `longPressDelayMs: 500`); DEP-0019 and CLN-0030 below Consumer migration instructions belong in consumer docs and release notes. +### DEP-0019 — Deprecate the options form + +| Field | Value | +| ---------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| id | `DEP-0019` | +| cleanup | `CLN-0030` | +| package | `@astryxdesign/core` | +| surface | `useMenuPress`'s options form, `useMenuPress({menuRef, …})`, with the types `UseMenuPressOptions` and `UseMenuPressReturn` | +| old contract | `useMenuPress(options)` returns `menuProps` and `triggerProps` to spread on the menu root and the trigger, with `isTriggerClickFromPress` and `cancel` | +| replacement | The ref form, `useMenuPress(menuRef, {triggerRef, …})`, which listens on the menu and the trigger itself and returns `isTriggerClickFromPress`, `cancel`, `handleMenuPointerDown` and the two trigger handle methods (DEC-9) | +| direct authority | `module:DropdownMenu/useMenuPress` DEC-9 and `component:DropdownMenu` DEC-11. Owner: `cixzhang` | +| warning | Static `@deprecated` metadata on the options-form overload and its two types; consumer docs and the release notes name `DEP-0019`, the replacement and `astryx upgrade`. No runtime warning (`spec:AST-017` FR29, FR29a) | +| compatibility | The options form keeps its released behavior: the props it returns still claim the press. A trigger handler that a caller forwards reaches the press once. An ancestor that stops a finger's press-down hides it from the press; a mouse's or pen's that a caller forwards still reaches it, as in the options form | +| migration | Mechanical. `useMenuPress({menuRef: R, …rest})` becomes `useMenuPress(R, {…rest})`; the spreads of `menuProps` and `triggerProps` go, and a trigger that got the press only through its spread passes its `ref` as `triggerRef`. A `renderTrigger` control that spreads the trigger props drops its redundant forwards of the two handlers | +| codemod | An `astryx upgrade` codemod, `migrate-use-menu-press-to-ref-form.mjs`, staged in `packages/cli/assets/codemods/transforms/next/` with its tests, available in the release that deprecates the form. It leaves a TODO wherever it cannot prove the result: options that are not an object literal, a return read other than through the two spreads, no `menuProps` spread on the menu element, a spread on an element whose `ref` is not the menu's or the trigger's, a trigger passed without its props spread, a trigger reached only through its spread whose `ref` is not a `useRef` or `createRef` object of the component, a menu or trigger props spread on a component element, a menu spread not in one of the component's returns through DOM elements, fragments and conditions, a trigger whose menu is not in the component's one returned JSX through DOM elements and fragments, a press spread beside an `onPointerDown`, a trigger's `onContextMenu` or another spread, an element inside the menu or the trigger that sets `onPointerDown` or spreads props, a component or a child other than JSX or a literal inside them | +| downstream | Every maintained caller, DropdownMenu and the five navigation menus, moves to the ref form in the deprecating patch with the same behavior (`spec:AST-017` FR29c); Core's docs teach only the ref form (FR29b) | +| transition | no prior transition | +| state | `proposed` | +| target plan | `CLN-0030` in 0.8.0, the next minor release | + +### CLN-0030 — Remove the options form, and deprecate the trigger's press handlers + +| Field | Value | +| ----------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| id | `CLN-0030` | +| lifecycle | `DEP-0019` | +| delta | `useMenuPress`'s options form, `UseMenuPressOptions` and `UseMenuPressReturn` are removed; `DropdownMenuTriggerProps`'s `onPointerDown` and `onContextMenu` are deprecated, as the press moves to the trigger's own listener | +| replacement | `useMenuPress(menuRef, options)` (DEC-9); a `renderTrigger` control's press moves to the trigger's own listener, so its `ref` alone carries it (`component:DropdownMenu` DEC-11) | +| codemod | 0.8.0's `astryx upgrade` runs DEP-0019's rewrite, the same codemod callers could already run early, over any remaining site and reports what it cannot prove | +| rollback | Restore the overload and the two types from the final-patch baseline, and take the two handlers' deprecation back | +| state | `pending` — lands in 0.8.0, a declared minor, with its `[breaking]` Changeset | + ## Ownership boundary **Owns** @@ -326,6 +359,24 @@ the open, so a screen reader hears one open and one close. Rejected: opening a sub-menu when a finger rests on its row. +### DEC-9 — One factory, and every caller on the ref form + +**Reference:** `module:DropdownMenu/useMenuPress/DEC-9` +**Decider:** `cixzhang`, `2026-11-03` + +The ref form delegates to the factory, as `architecture:public-component-api` +INV22 asks. The factory takes new options in place: they merge with the ones it +has, apply from the next event, and never end a press in progress. A menu whose +other content keeps its own clicks starts a press only on an item. A menu or +trigger event that a caller forwards reaches the press once. In Core's menus, a mouse or pen press behaves exactly as in the released options form. For a finger's press-down (pointerType touch), one that a page handler stops or cancels starts no press; on a page that cancels every press-down, touch menus lose press-drag-release; plain taps still work. Without a forward, a mouse's or pen's press-down acts at the menu's or trigger's own listener. In the ref form, that listener runs before the page's React `onPointerDown` handlers, so a forward reaches the press once but does not order it. Every maintained +caller moves to the ref form, with the same behavior, in the patch that +deprecates the options form under INV9 (`spec:AST-017` FR29c). The options +form goes at the next minor release: DEP-0019 deprecates it, and CLN-0030 +removes it. + +Rejected: a new factory for each change of options, which the in-place update +replaces; moving the callers later, one at a time. + ## Open questions - **OQ1 — Is press-to-open the menu family's trigger model?** (`human-design`) diff --git a/internal/eslint-plugin-astryx/no-raw-private-attribute.baseline.json b/internal/eslint-plugin-astryx/no-raw-private-attribute.baseline.json index 673d42376cdae..543375d4133f4 100644 --- a/internal/eslint-plugin-astryx/no-raw-private-attribute.baseline.json +++ b/internal/eslint-plugin-astryx/no-raw-private-attribute.baseline.json @@ -114,11 +114,13 @@ "data-astryx-press" ], "packages/core/src/gestures/touchActionRule.ts": ["data-astryx-touch-action"], + "packages/core/src/hooks/createMenuPress.ts": [ + "data-astryx-menu-press-surface" + ], "packages/core/src/hooks/useAnnounce.ts": [ "data-astryx-live-region", "data-astryx-modal-live-region" ], - "packages/core/src/hooks/useMenuPress.ts": ["data-astryx-menu-press-surface"], "packages/core/src/theme/Theme.tsx": [ "data-astryx-id", "data-astryx-id", diff --git a/packages/cli/assets/codemods/transforms/next/__tests__/migrate-use-menu-press-to-ref-form.test.mjs b/packages/cli/assets/codemods/transforms/next/__tests__/migrate-use-menu-press-to-ref-form.test.mjs new file mode 100644 index 0000000000000..13302dc07f863 --- /dev/null +++ b/packages/cli/assets/codemods/transforms/next/__tests__/migrate-use-menu-press-to-ref-form.test.mjs @@ -0,0 +1,299 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +import {describe, expect, it} from 'vitest'; +import transform from '../migrate-use-menu-press-to-ref-form.mjs'; + +async function applyTransform(source) { + const jscodeshift = (await import('jscodeshift')).default; + const j = jscodeshift.withParser('tsx'); + const api = {jscodeshift: j, stats: () => {}, report: () => {}}; + return transform({source, path: 'test.tsx'}, api) ?? source; +} + +/** Collapse whitespace so assertions ignore the printer's line breaks. */ +const flat = text => text.replace(/\s+/g, ' '); + +const TODO = 'TODO(astryx upgrade): useMenuPress({menuRef, ...}) is deprecated'; + +describe('migrate-use-menu-press-to-ref-form', () => { + it('moves the menu ref first and drops the spreads the ref form no longer returns', async () => { + const output = flat( + await applyTransform(`import {useMenuPress} from '@astryxdesign/core'; +function Menu() { + const menuPress = useMenuPress({menuRef: listRef, triggerRef, itemSelector: '[role="menuitem"]', onActivate}); + return ( + <> +
    ;`, + // Rows the codemod cannot read: the caller's children, + `const press = useMenuPress({menuRef, itemSelector: ITEM}); +return
      {children}
    ;`, + // a row component, + `const press = useMenuPress({menuRef, itemSelector: ITEM}); +return
    ;`, + // and a row that spreads props. + `const press = useMenuPress({menuRef, itemSelector: ITEM}); +return
    • A
    ;`, + ]; + for (const body of cases) { + const source = `import {useMenuPress} from '@astryxdesign/core'; +function A() { +${body} +}`; + const output = await applyTransform(source); + expect(output).toContain(TODO); + // The call keeps its options form. + expect(output).toContain(body.split('\n')[0]); + const second = await applyTransform(output); + expect(second).toBe(output); + } + }); + + it('still rewrites a menu whose rows set other handlers', async () => { + const output = flat( + await applyTransform(`import {useMenuPress} from '@astryxdesign/core'; +function A() { + const press = useMenuPress({menuRef, itemSelector: ITEM}); + return
    • A
    ; +}`), + ); + expect(output).toContain('useMenuPress(menuRef, { itemSelector: ITEM })'); + expect(output).not.toContain('menuProps'); + expect(output).not.toContain('TODO'); + }); + + it('rewrites a menu without a trigger that its component renders only while open', async () => { + const output = flat( + await applyTransform(`import {useMenuPress} from '@astryxdesign/core'; +function Menu() { + const press = useMenuPress({menuRef, itemSelector: ITEM}); + return
    {open ?
      : null}{open && }
    ; +}`), + ); + expect(output).toContain('useMenuPress(menuRef, { itemSelector: ITEM });'); + expect(output).toContain('
      '); + expect(output).not.toContain('TODO'); + }); + + it('leaves the ref form, other hooks of the same name, and type-only imports alone', async () => { + const source = `import {useMenuPress} from '@astryxdesign/core'; +import {useMenuPress as localPress} from './local'; +import type {UseMenuPressOptions} from '@astryxdesign/core'; +function A() { + useMenuPress(menuRef, {itemSelector: ITEM}); + localPress({menuRef, itemSelector: ITEM}); +}`; + expect(await applyTransform(source)).toBe(source); + }); + + it("drops a redundant forward of a trigger's handlers only beside a spread of the trigger props", async () => { + const output = flat( + await applyTransform(`import {DropdownMenu, type DropdownMenuTriggerProps} from '@astryxdesign/core'; +const a = } />; +function Trigger(props: DropdownMenuTriggerProps) { + return ; +}`), + ); + expect(output).not.toContain('onPointerDown'); + expect(output).not.toContain('onContextMenu'); + expect(output).toContain( + 'renderTrigger={props => }', + ); + expect(output).toContain('return ;'); + expect(output).not.toContain('TODO'); + }); + + it('leaves every other forward of the trigger handlers as it is, with no TODO', async () => { + const cases = [ + // The element does not take the trigger's ref. + `const a = } />;`, + // A composed handler. + `const a = { p.onPointerDown(e); mine(e); }} />} />;`, + // A destructured handler passed elsewhere. + `const a = } />;`, + // A destructured handler: the rest does not carry it. + `const a = } />;`, + // The element takes the ref but spreads nothing. + `const a = } />;`, + ]; + for (const body of cases) { + const source = `import {DropdownMenu} from '@astryxdesign/core'; +${body}`; + expect(await applyTransform(source)).toBe(source); + } + }); + + it('changes nothing on a second run', async () => { + const once = + await applyTransform(`import {useMenuPress, DropdownMenu} from '@astryxdesign/core'; +function Menu() { + const press = useMenuPress({menuRef, itemSelector: ITEM}); + return <> } />; +}`); + expect(once).not.toContain('menuProps'); + expect(await applyTransform(once)).toBe(once); + }); +}); diff --git a/packages/cli/assets/codemods/transforms/next/index.mjs b/packages/cli/assets/codemods/transforms/next/index.mjs index 49c8efb0df30b..8c1b983bd4da6 100644 --- a/packages/cli/assets/codemods/transforms/next/index.mjs +++ b/packages/cli/assets/codemods/transforms/next/index.mjs @@ -1,5 +1,4 @@ // Copyright (c) Meta Platforms, Inc. and affiliates. - /** * @file next transform manifest * @@ -10,6 +9,9 @@ import promoteTokenizerPresentation, { meta as promoteTokenizerPresentationMeta, } from './promote-tokenizer-presentation.mjs'; +import migrateUseMenuPressToRefForm, { + meta as migrateUseMenuPressToRefFormMeta, +} from './migrate-use-menu-press-to-ref-form.mjs'; export default [ { @@ -17,4 +19,9 @@ export default [ transform: promoteTokenizerPresentation, meta: promoteTokenizerPresentationMeta, }, + { + name: 'migrate-use-menu-press-to-ref-form', + transform: migrateUseMenuPressToRefForm, + meta: migrateUseMenuPressToRefFormMeta, + }, ]; diff --git a/packages/cli/assets/codemods/transforms/next/migrate-use-menu-press-to-ref-form.mjs b/packages/cli/assets/codemods/transforms/next/migrate-use-menu-press-to-ref-form.mjs new file mode 100644 index 0000000000000..3c6a5af9b7074 --- /dev/null +++ b/packages/cli/assets/codemods/transforms/next/migrate-use-menu-press-to-ref-form.mjs @@ -0,0 +1,900 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file Codemod: move useMenuPress to its ref form + * + * `useMenuPress({menuRef, ...options})` becomes + * `useMenuPress(menuRef, {...options})`. The ref form takes the press-downs of + * the menu and the trigger itself. So the spreads of the old return's + * `menuProps` and `triggerProps` go. A trigger that got the press only + * through its spread gets `triggerRef` from its `ref`. In a DropdownMenu + * trigger render function, a redundant forward of `onPointerDown` or + * `onContextMenu` goes. It goes only where the same element spreads the + * trigger props, which still pass it. Any other forward stays as it is. + * + * Where it cannot prove the result, it leaves the code unchanged with a TODO. + * That covers options that are not an object literal or that spread, and a + * return used other than through those spreads, `isTriggerClickFromPress` + * and `cancel`. It also covers a call that spreads no `menuProps` on the menu + * element, since the old call then ran no press. So does a spread on an + * element whose `ref` is not the menu's or the trigger's. So does an element + * inside the menu or the trigger that sets `onPointerDown` or spreads props. + * So does a component, or a child other than JSX or a literal, inside them. + * With a + * trigger, the menu element must sit in the component's one returned JSX, + * through DOM elements and fragments only, with no function, prop value or + * early return in between. The ref form has + * no press while the menu is unmounted. The deprecated forms keep working + * there. Comments are the codemod's only report channel. Each is added once, so + * a second run changes nothing. + */ + +export const meta = { + title: 'Move useMenuPress to its ref form', + description: + 'Rewrites `useMenuPress({menuRef, ...options})` to ' + + '`useMenuPress(menuRef, {...options})` and removes the spreads of its ' + + '`menuProps` and `triggerProps`, which the ref form does not return. ' + + "Removes redundant forwards of a DropdownMenu trigger's " + + '`onPointerDown` and `onContextMenu`. Leaves a TODO where it cannot ' + + 'prove the result; the deprecated forms still work there.', +}; + +const HOOK = 'useMenuPress'; +const DROPDOWN = 'DropdownMenu'; +const TRIGGER_PROPS_TYPE = 'DropdownMenuTriggerProps'; +const CORE_IMPORT_RE = /^@(astryxdesign|xds)\/core(?:\/|$)/; +const TRIGGER_HANDLERS = new Set(['onPointerDown', 'onContextMenu']); +const KEPT_RETURN_KEYS = new Set(['isTriggerClickFromPress', 'cancel']); +const SPREAD_KEYS = new Set(['menuProps', 'triggerProps']); + +const CALL_COMMENT = + ' TODO(astryx upgrade): useMenuPress({menuRef, ...}) is deprecated. Call ' + + 'useMenuPress(menuRef, options) instead, pass triggerRef, and remove the ' + + 'menuProps and triggerProps spreads. With a trigger, keep the menu ' + + 'element mounted while closed. This call could not be rewritten safely, ' + + 'so it was left unchanged. '; + +const TYPE_WRAPPERS = new Set([ + 'TSAsExpression', + 'TSSatisfiesExpression', + 'TSTypeAssertion', + 'TSNonNullExpression', + 'TypeCastExpression', + 'ParenthesizedExpression', +]); + +/** @param {any} node */ +function unwrap(node) { + let current = node; + while (current && TYPE_WRAPPERS.has(current.type)) { + current = current.expression; + } + return current; +} + +/** @param {any} property */ +function isPlainProperty(property) { + return ( + (property.type === 'ObjectProperty' || property.type === 'Property') && + property.kind !== 'get' && + property.kind !== 'set' && + property.method !== true + ); +} + +/** The static name of a property's key, or null. @param {any} property */ +function keyName(property) { + if (property.computed) { + return null; + } + const key = property.key; + if (key.type === 'Identifier') { + return key.name; + } + if ( + (key.type === 'StringLiteral' || key.type === 'Literal') && + typeof key.value === 'string' + ) { + return key.value; + } + return null; +} + +/** The imported name of a specifier. @param {any} specifier */ +function importedName(specifier) { + return specifier.imported.name ?? specifier.imported.value; +} + +/** @param {any} attribute */ +function attributeExpression(attribute) { + return attribute?.value?.type === 'JSXExpressionContainer' + ? attribute.value.expression + : null; +} + +/** @param {any} opening @param {string} name */ +function findAttribute(opening, name) { + return ( + opening.attributes.find( + (/** @type {any} */ attribute) => + attribute.type === 'JSXAttribute' && + attribute.name.type === 'JSXIdentifier' && + attribute.name.name === name, + ) ?? null + ); +} + +/** Source text with whitespace removed, to compare two refs. */ +function sourceOf(/** @type {any} */ j, /** @type {any} */ node) { + return j(node).toSource().replace(/\s+/g, ''); +} + +/** A fresh copy of a call-free chain. @param {any} j @param {any} node */ +function cloneChain(j, node) { + const value = unwrap(node); + if (value.type === 'Identifier') { + return j.identifier(value.name); + } + if (value.type === 'ThisExpression') { + return j.thisExpression(); + } + const property = value.computed + ? j.literal(value.property.value) + : j.identifier(value.property.name); + return j.memberExpression( + cloneChain(j, value.object), + property, + value.computed, + ); +} + +/** @param {any} j @param {any} node @param {string} text */ +function addComment(j, node, text) { + node.comments ??= []; + if (node.comments.some((/** @type {any} */ c) => c.value === text)) { + return false; + } + node.comments.push(j.commentBlock(text, true, false)); + return true; +} + +/** + * Whether an identifier reads its binding, as opposed to naming a property + * or a type. + * @param {any} path + */ +function isReference(path) { + const parent = path.parent?.node; + const node = path.node; + if (parent == null) { + return true; + } + if (node.type === 'JSXIdentifier') { + // A JSX attribute's name is no read; a component's name is. + return parent.type === 'JSXMemberExpression' + ? parent.object === node + : parent.type === 'JSXOpeningElement'; + } + if ( + (parent.type === 'MemberExpression' || + parent.type === 'OptionalMemberExpression') && + parent.property === node && + !parent.computed + ) { + return false; + } + // A shorthand property reads its name in an object, but its key only names + // the property in a pattern. + if ( + (parent.type === 'ObjectProperty' || parent.type === 'Property') && + parent.key === node && + !parent.computed && + (parent.shorthand !== true || + path.parent.parent?.node.type === 'ObjectPattern') + ) { + return false; + } + if ( + (parent.type === 'ObjectMethod' || + parent.type === 'ClassProperty' || + parent.type === 'ClassMethod' || + parent.type === 'TSPropertySignature') && + parent.key === node && + !parent.computed + ) { + return false; + } + return !( + parent.type === 'TSQualifiedName' || + parent.type === 'TSTypeReference' || + parent.type === 'LabeledStatement' || + parent.type === 'BreakStatement' || + parent.type === 'ContinueStatement' + ); +} + +/** Whether an identifier declares a binding. @param {any} path */ +function isBinding(path) { + const parent = path.parent?.node; + const node = path.node; + if (parent == null) { + return false; + } + switch (parent.type) { + case 'VariableDeclarator': + return parent.id === node; + case 'FunctionDeclaration': + case 'FunctionExpression': + case 'ArrowFunctionExpression': + case 'ClassDeclaration': + return parent.id === node || parent.params?.includes(node) === true; + case 'AssignmentPattern': + return parent.left === node; + case 'RestElement': + return parent.argument === node; + case 'ArrayPattern': + return parent.elements.includes(node); + case 'CatchClause': + return parent.param === node; + case 'ObjectProperty': + case 'Property': + return ( + parent.value === node && + path.parent.parent?.node.type === 'ObjectPattern' + ); + case 'ImportSpecifier': + case 'ImportDefaultSpecifier': + case 'ImportNamespaceSpecifier': + return parent.local === node; + default: + return false; + } +} + +/** Whether `name` is bound again inside `scope`, besides `binding`. */ +function isRedeclared( + /** @type {any} */ j, + /** @type {any} */ scope, + /** @type {string} */ name, + /** @type {any} */ binding, +) { + return ( + j(scope) + .find(j.Identifier, {name}) + .filter( + (/** @type {any} */ path) => path.node !== binding && isBinding(path), + ) + .size() > 0 + ); +} + +/** + * The spreads of the old return's `menuProps` and `triggerProps`, and the + * edits that drop them, or null when another use keeps the call unproven. + * @param {any} j @param {any} root @param {any} callPath + * @param {any} menuRef @param {any} triggerRef + */ +function planReturnUses(j, root, callPath, menuRef, triggerRef) { + const parent = callPath.parent.node; + /** @type {{removals: Array<() => void>, triggerRef: any}} */ + const plan = {removals: [], triggerRef: null}; + if (parent.type === 'ExpressionStatement') { + // An unused result spreads no menuProps, so the old call ran no press. + return null; + } + if (parent.type !== 'VariableDeclarator' || parent.init !== callPath.node) { + return null; + } + const functions = j(callPath).closest(j.Function); + const scope = + functions.size() > 0 + ? functions.paths()[0] + : root.find(j.Program).paths()[0]; + /** @type {{menuProps: any[], triggerProps: any[]}} */ + const spreads = {menuProps: [], triggerProps: []}; + + if (parent.id.type === 'Identifier') { + const name = parent.id.name; + if (isRedeclared(j, scope, name, parent.id)) { + return null; + } + let isProven = true; + j(scope) + .find(j.Identifier, {name}) + .forEach((/** @type {any} */ path) => { + if (!isProven || path.node === parent.id || !isReference(path)) { + return; + } + const member = path.parent.node; + if ( + member.type !== 'MemberExpression' || + member.object !== path.node || + member.computed + ) { + isProven = false; + return; + } + const key = member.property.name; + if (KEPT_RETURN_KEYS.has(key)) { + return; + } + const spread = path.parent.parent; + if ( + !SPREAD_KEYS.has(key) || + spread?.node.type !== 'JSXSpreadAttribute' || + spread.node.argument !== member + ) { + isProven = false; + return; + } + spreads[/** @type {'menuProps' | 'triggerProps'} */ (key)].push(spread); + }); + if (!isProven) { + return null; + } + } else if (parent.id.type === 'ObjectPattern') { + const pattern = parent.id; + /** @type {any[]} */ + const kept = []; + for (const property of pattern.properties) { + if (!isPlainProperty(property)) { + return null; + } + const key = keyName(property); + if (key != null && KEPT_RETURN_KEYS.has(key)) { + kept.push(property); + continue; + } + if (key == null || !SPREAD_KEYS.has(key)) { + return null; + } + const local = property.value; + if ( + local.type !== 'Identifier' || + isRedeclared(j, scope, local.name, local) + ) { + return null; + } + let isProven = true; + j(scope) + .find(j.Identifier, {name: local.name}) + .forEach((/** @type {any} */ path) => { + if (!isProven || path.node === local || !isReference(path)) { + return; + } + if (path.parent.node.type !== 'JSXSpreadAttribute') { + isProven = false; + return; + } + spreads[/** @type {'menuProps' | 'triggerProps'} */ (key)].push( + path.parent, + ); + }); + if (!isProven) { + return null; + } + } + const declaration = callPath.parent.parent; + plan.removals.push(() => { + if (kept.length > 0 || declaration.node.declarations.length > 1) { + pattern.properties = kept; + return; + } + j(declaration).replaceWith(j.expressionStatement(callPath.node)); + }); + } else { + return null; + } + + // The old call ran a press only through a menuProps spread. + if (spreads.menuProps.length === 0) { + return null; + } + // A trigger opens a menu that must stay mounted: the ref form has no press + // while the menu is unmounted. + // A trigger the old call never reached keeps no press: the ref form would + // add one on its ref. + if (triggerRef != null && spreads.triggerProps.length === 0) { + return null; + } + const hasTrigger = triggerRef != null || spreads.triggerProps.length > 0; + if ( + hasTrigger && + !spreads.menuProps.every(spread => isAlwaysRendered(j, spread, scope)) + ) { + return null; + } + // The ref form attaches only on the calling component's commits. A menu + // that another component mounts in a commit of its own gets no press. + if ( + !spreads.menuProps.every(spread => rendersWithComponent(j, spread, scope)) + ) { + return null; + } + const menuSource = sourceOf(j, menuRef); + for (const spread of spreads.menuProps) { + const ref = attributeExpression(findAttribute(spread.parent.node, 'ref')); + if ( + ref == null || + sourceOf(j, ref) !== menuSource || + !isAloneWithPress(spread, ['onPointerDown']) + ) { + return null; + } + } + let triggerSource = triggerRef == null ? null : sourceOf(j, triggerRef); + for (const spread of spreads.triggerProps) { + const ref = attributeExpression(findAttribute(spread.parent.node, 'ref')); + if ( + ref == null || + // A component may drop the two handlers, or give its ref to an + // imperative handle that takes no listeners. + !isDomElement(spread.parent.parent.node) || + !isAloneWithPress(spread, ['onPointerDown', 'onContextMenu']) + ) { + return null; + } + if (triggerSource == null) { + if (!isLocalRefObject(ref, spread)) { + return null; + } + plan.triggerRef = cloneChain(j, ref); + triggerSource = sourceOf(j, ref); + } else if (sourceOf(j, ref) !== triggerSource) { + return null; + } + } + const pressSpreads = [...spreads.menuProps, ...spreads.triggerProps]; + if (pressSpreads.some(spread => mayHandleInnerPressDown(j, spread))) { + return null; + } + for (const spread of pressSpreads) { + plan.removals.push(() => j(spread).remove()); + } + return plan; +} + +/** + * Whether something inside the spread's element may handle a press-down: an + * `onPointerDown`, a spread, a component, or a child the codemod cannot read. + * Such a handler ran before the old spread's, so its stop kept the press from + * starting. The ref form's own listener runs before every React + * `onPointerDown` handler. + * @param {any} j @param {any} spread + */ +function mayHandleInnerPressDown(j, spread) { + const element = spread.parent.parent; + const inside = (/** @type {any} */ path) => + path.parent.node !== element.node.openingElement; + const scope = j(element); + return ( + scope.find(j.JSXAttribute, {name: {name: 'onPointerDown'}}).some(inside) || + scope.find(j.JSXSpreadAttribute).some(inside) || + scope + .find(j.JSXElement) + .some( + (/** @type {any} */ path) => + path.node !== element.node && !isDomElement(path.node), + ) || + scope + .find(j.JSXExpressionContainer) + .some( + (/** @type {any} */ path) => + path.parent.node.type !== 'JSXAttribute' && + !isReadableChild(path.node.expression), + ) + ); +} + +/** + * Whether an expression child is one the codemod can read: JSX, a comment, or + * a literal. Anything else may render rows with their own handlers. + * @param {any} expression + */ +function isReadableChild(expression) { + return [ + 'JSXElement', + 'JSXFragment', + 'JSXEmptyExpression', + 'StringLiteral', + 'NumericLiteral', + 'Literal', + ].includes(expression.type); +} + +/** + * Whether `ref` names a ref object that the component makes with `useRef` or + * `createRef`. A callback ref or a merged ref has no `current` to read. + * @param {any} ref @param {any} path + */ +function isLocalRefObject(ref, path) { + if (ref.type !== 'Identifier') { + return false; + } + const binding = path.scope?.lookup(ref.name)?.getBindings()[ref.name]?.[0]; + const declarator = binding?.parent?.node; + if ( + declarator?.type !== 'VariableDeclarator' || + declarator.id !== binding.node || + declarator.init?.type !== 'CallExpression' + ) { + return false; + } + const callee = declarator.init.callee; + const name = + callee.type === 'Identifier' + ? callee.name + : callee.type === 'MemberExpression' && !callee.computed + ? callee.property.name + : null; + return name === 'useRef' || name === 'createRef'; +} + +/** + * Whether a press spread is the only thing on its element that meets the + * press: no `handlers` attribute and no other spread. The old spread and such a + * handler override each other by order, and the ref form runs both. + * @param {any} spread @param {string[]} handlers + */ +function isAloneWithPress(spread, handlers) { + return (spread.parent.node.attributes ?? []).every( + (/** @type {any} */ attribute) => + attribute === spread.node || + (attribute.type === 'JSXAttribute' && + !handlers.includes(attribute.name?.name)), + ); +} + +/** + * Whether a JSX element is a DOM element. A component element carrying the + * spread may mount its own DOM later, or only while open. + * @param {any} node + */ +function isDomElement(node) { + const name = node.openingElement.name; + return name.type === 'JSXIdentifier' && /^[a-z]/.test(name.name); +} + +/** + * Whether a JSX node renders its children in the same commit: a DOM element or + * a fragment. A component element may mount them later. + * @param {any} node + */ +function passesChildren(node) { + if (node?.type === 'JSXFragment') { + return true; + } + const name = node?.type === 'JSXElement' ? node.openingElement.name : null; + return ( + (name?.type === 'JSXIdentifier' && + (/^[a-z]/.test(name.name) || name.name === 'Fragment')) || + (name?.type === 'JSXMemberExpression' && + name.object.name === 'React' && + name.property.name === 'Fragment') + ); +} + +/** + * Whether a JSX attribute's element mounts in its component's own commits. It + * must sit in one of the component's returns, reached only through conditions + * and the children of DOM elements and fragments. A function, a prop value or + * a component element stops it. + * @param {any} j @param {any} path @param {any} fnPath + */ +function rendersWithComponent(j, path, fnPath) { + let current = path.parent?.parent; + if (current?.node.type !== 'JSXElement' || !isDomElement(current.node)) { + return false; + } + for (;;) { + const parent = current.parent; + const node = parent?.node; + const isChild = + (passesChildren(node) && (node.children ?? []).includes(current.node)) || + (node?.type === 'JSXExpressionContainer' && + passesChildren(parent.parent?.node) && + (parent.parent.node.children ?? []).includes(node)); + const isBranch = + (node?.type === 'LogicalExpression' && node.right === current.node) || + (node?.type === 'ConditionalExpression' && node.test !== current.node); + if (isChild || isBranch) { + current = parent; + continue; + } + if (node?.type !== 'ReturnStatement') { + return false; + } + const owner = j(parent).closest(j.Function); + return owner.size() > 0 && owner.paths()[0].node === fnPath.node; + } +} + +/** + * Whether a JSX attribute's element always renders with its component: it + * sits in the component's one top-level return, reached only through the + * children of DOM elements and fragments. A function, a prop value, a + * component element (which may hold its children back), a condition or + * another return stops it. + * @param {any} j @param {any} path @param {any} fnPath + */ +function isAlwaysRendered(j, path, fnPath) { + let current = path.parent?.parent; + if (current?.node.type !== 'JSXElement' || !isDomElement(current.node)) { + return false; + } + for (;;) { + const parent = current.parent; + const node = parent?.node; + if (passesChildren(node) && (node.children ?? []).includes(current.node)) { + current = parent; + continue; + } + if ( + node?.type === 'JSXExpressionContainer' && + passesChildren(parent.parent?.node) && + (parent.parent.node.children ?? []).includes(node) + ) { + current = parent; + continue; + } + if (node?.type !== 'ReturnStatement') { + return false; + } + const fn = fnPath.node; + if (fn.type === 'Program' || parent.parent?.node !== fn.body) { + return false; + } + const returns = j(fnPath) + .find(j.ReturnStatement) + .filter((/** @type {any} */ each) => { + const owner = j(each).closest(j.Function); + return owner.size() > 0 && owner.paths()[0].node === fn; + }); + return returns.size() === 1; + } +} + +/** + * Whether a JSX element takes the trigger props' ref: it spreads one of + * `spreadNames`, or its `ref` reads one of `refSources`. + * @param {any} j @param {any} opening + * @param {string[]} spreadNames @param {string[]} refSources + */ +function takesTriggerRef(j, opening, spreadNames, refSources) { + return opening.attributes.some((/** @type {any} */ attribute) => { + if (attribute.type === 'JSXSpreadAttribute') { + const argument = unwrap(attribute.argument); + return ( + argument?.type === 'Identifier' && spreadNames.includes(argument.name) + ); + } + if (attribute.name?.name !== 'ref') { + return false; + } + const ref = attributeExpression(attribute); + return ref != null && refSources.includes(sourceOf(j, ref)); + }); +} + +/** + * The JSX attribute a reference is the whole value of, when its name is + * `name`, or null. + * @param {any} path @param {string} name + */ +function forwardingAttribute(path, name) { + const container = path.parent; + const attribute = container?.parent; + if ( + container?.node.type !== 'JSXExpressionContainer' || + attribute?.node.type !== 'JSXAttribute' || + attribute.node.name.name !== name + ) { + return null; + } + return attribute; +} + +/** + * Drops redundant forwards of the trigger handlers beside a spread of the + * trigger props in one trigger render function. Returns 'changed', + * 'unproven' or 'none'. + * @param {any} j @param {any} fnPath + */ +function migrateTriggerFunction(j, fnPath) { + const param = fnPath.node.params[0]; + const body = j(fnPath.get('body')); + /** @type {any[]} */ + const forwards = []; + if (param?.type === 'Identifier') { + let isProven = true; + body + .find(j.MemberExpression, { + object: {type: 'Identifier', name: param.name}, + }) + .forEach((/** @type {any} */ path) => { + const member = path.node; + if ( + !isProven || + member.computed || + !TRIGGER_HANDLERS.has(member.property.name) + ) { + return; + } + const attribute = forwardingAttribute(path, member.property.name); + // Only a forward beside a spread of the props is redundant: the spread + // still passes the handler, under any ancestor. + if ( + attribute == null || + !takesTriggerRef(j, attribute.parent.node, [param.name], []) + ) { + isProven = false; + return; + } + forwards.push(attribute); + }); + if (!isProven) { + return 'unproven'; + } + forwards.forEach(attribute => j(attribute).remove()); + return forwards.length > 0 ? 'changed' : 'none'; + } + if (param?.type !== 'ObjectPattern') { + return 'none'; + } + // A destructured handler is not in the rest the element spreads. Its + // forward is then the control's only path to the press under an ancestor + // that stops the press-down. + const hasHandler = param.properties.some( + (/** @type {any} */ property) => + property.type !== 'RestElement' && + isPlainProperty(property) && + TRIGGER_HANDLERS.has(keyName(property) ?? ''), + ); + return hasHandler ? 'unproven' : 'none'; +} + +/** @param {any} file @param {any} api */ +export default function transform(file, api) { + const j = api.jscodeshift; + const root = j(file.source); + /** @type {Set} */ + const hookNames = new Set(); + /** @type {Set} */ + const dropdownNames = new Set(); + /** @type {Set} */ + const triggerTypeNames = new Set(); + /** @type {Set} */ + const namespaces = new Set(); + root.find(j.ImportDeclaration).forEach((/** @type {any} */ path) => { + const source = path.node.source.value; + if (typeof source !== 'string' || !CORE_IMPORT_RE.test(source)) { + return; + } + for (const specifier of path.node.specifiers ?? []) { + if (specifier.type === 'ImportNamespaceSpecifier') { + namespaces.add(specifier.local.name); + } else if (specifier.type === 'ImportSpecifier') { + const imported = importedName(specifier); + const isTypeOnly = + path.node.importKind === 'type' || specifier.importKind === 'type'; + if (imported === TRIGGER_PROPS_TYPE) { + triggerTypeNames.add(specifier.local.name); + } else if (!isTypeOnly && imported === HOOK) { + hookNames.add(specifier.local.name); + } else if (!isTypeOnly && imported === DROPDOWN) { + dropdownNames.add(specifier.local.name); + } + } + } + }); + /** @param {any} node @param {Set} names @param {string} exported */ + const resolves = (node, names, exported) => + (node.type === 'Identifier' && names.has(node.name)) || + (node.type === 'JSXIdentifier' && names.has(node.name)) || + ((node.type === 'MemberExpression' || + node.type === 'JSXMemberExpression') && + !node.computed && + (node.object.type === 'Identifier' || + node.object.type === 'JSXIdentifier') && + namespaces.has(node.object.name) && + node.property.name === exported); + + let changed = false; + /** @param {any} path @param {string} text */ + const report = (path, text) => { + const statement = j(path).closest(j.Statement); + const target = statement.size() > 0 ? statement.paths()[0].node : null; + if (target != null && addComment(j, target, text)) { + changed = true; + } + }; + + root + .find(j.CallExpression) + .filter( + (/** @type {any} */ path) => + path.node.arguments.length === 1 && + resolves(path.node.callee, hookNames, HOOK), + ) + .forEach((/** @type {any} */ path) => { + const options = unwrap(path.node.arguments[0]); + if (options?.type !== 'ObjectExpression') { + report(path, CALL_COMMENT); + return; + } + let menuRef = null; + let triggerRef = null; + /** @type {any[]} */ + const rest = []; + for (const property of options.properties) { + const key = isPlainProperty(property) ? keyName(property) : null; + if (key == null) { + report(path, CALL_COMMENT); + return; + } + if (key === 'menuRef') { + menuRef = property.value; + continue; + } + if (key === 'triggerRef') { + triggerRef = property.value; + } + rest.push(property); + } + const plan = + menuRef == null + ? null + : planReturnUses(j, root, path, menuRef, triggerRef); + if (plan == null) { + report(path, CALL_COMMENT); + return; + } + if (plan.triggerRef != null) { + rest.push( + j.objectProperty(j.identifier('triggerRef'), plan.triggerRef), + ); + } + path.node.arguments = [menuRef, j.objectExpression(rest)]; + plan.removals.forEach(remove => remove()); + changed = true; + }); + + /** @type {Set} */ + const triggerFunctions = new Set(); + root.find(j.JSXAttribute).forEach((/** @type {any} */ path) => { + if ( + path.node.name.name !== 'renderTrigger' || + !resolves(path.parent.node.name, dropdownNames, DROPDOWN) + ) { + return; + } + const value = unwrap(attributeExpression(path.node)); + if ( + value?.type === 'ArrowFunctionExpression' || + value?.type === 'FunctionExpression' + ) { + triggerFunctions.add(value); + } + }); + root.find(j.Function).forEach((/** @type {any} */ path) => { + const annotation = path.node.params[0]?.typeAnnotation?.typeAnnotation; + if ( + annotation?.type === 'TSTypeReference' && + annotation.typeName.type === 'Identifier' && + triggerTypeNames.has(annotation.typeName.name) + ) { + triggerFunctions.add(path.node); + } + }); + root + .find(j.Function) + .filter((/** @type {any} */ path) => triggerFunctions.has(path.node)) + .forEach((/** @type {any} */ path) => { + if (migrateTriggerFunction(j, path) === 'changed') { + changed = true; + } + }); + + return changed ? root.toSource() : null; +} diff --git a/packages/core/api-report.md b/packages/core/api-report.md index ac2d19dd74387..1ee09ce1c6b53 100644 --- a/packages/core/api-report.md +++ b/packages/core/api-report.md @@ -2914,7 +2914,9 @@ hook useListFocus: (options?: UseListFocusO hook useLocale: () => Locale hook useLongPress: (options: UseLongPressOptions) => UseLongPressHandlers hook useMediaQuery: (query: string, serverDefault?: boolean) => boolean -hook useMenuPress: (options: UseMenuPressOptions) => UseMenuPressReturn +hook useMenuPress + overload (menuRef: RefObject, options: UseMenuPressRefOptions) => UseMenuPressRefReturn + overload (options: UseMenuPressOptions) => UseMenuPressReturn hook useMergedRefs: (refA?: Ref, refB?: Ref, refC?: Ref, refD?: Ref, refE?: Ref, refF?: Ref) => RefCallback hook useMultiCombobox: ({…}: UseMultiComboboxOptions) => UseMultiComboboxResult hook useNavHeadingCloseContext: () => NavHeadingCloseContextValue | null @@ -2987,6 +2989,7 @@ function createAnimator: (element: HTMLElement) => Animator function createCodeLanguage: ({…}: CreateCodeLanguageOptions) => CodeLanguage function createISOTimeString: (value: string) => ISOTimeString | null function createIncrementalState: () => IncrementalState +function createMenuPress: (menu: HTMLElement, options: CreateMenuPressOptions) => CreateMenuPressReturn function createPowerSearchConfig: >(definitions: D, configName?: string) => { config: PowerSearchConfig; applyFilters: >(filters: ReadonlyArray, data: ReadonlyArray) => T[] } function createStaticSource: (items: T[], options?: CreateStaticSourceOptions) => SearchSource function dateToISO: (pd: PlainDate) => ISODateString @@ -4085,6 +4088,26 @@ interface ContextRenderProps type CoreIndicatorName = 'check' | 'checkbox' | 'radio' type CoreTokenName = keyof typeof colorDefaults | keyof typeof spacingDefaults | keyof typeof sizeDefaults | keyof typeof borderDefaults | keyof typeof focusDefaults | keyof typeof … #4209c9b3 type CreateCodeLanguageOptions = CodeLanguageMeta & { tokenize: CodeLanguageTokenize } +interface CreateMenuPressOptions + getScroller?: () => HTMLElement | null + hitTest?: (x: number, y: number) => Element | null + isEnabled?: boolean = true + itemSelector: string + longPressDelayMs?: number = 500 + onActivate?: (row: HTMLElement, release: PointerEvent) => void + onDismiss?: () => void + onHighlight?: (row: HTMLElement | null) => void + onTriggerPress?: (pointerType: MenuPressPointerType) => boolean + startBehavior?: 'menu' | 'items' = 'menu' + trigger?: HTMLElement | null +interface CreateMenuPressReturn + cancel: () => void + destroy: () => void + handleMenuPointerDown: (event: MenuPressDownEvent) => void + handleTriggerContextMenu: (event: { preventDefault: () => void; nativeEvent?: Event }) => void + handleTriggerPointerDown: (event: MenuPressDownEvent) => void + isTriggerClickFromPress: () => boolean + update: (options: Partial) => void interface CreateStaticSourceOptions keywords?: (item: T) => string[] interface CustomOperatorValue @@ -4794,6 +4817,16 @@ type MathParseOptions = reado type MediaThemeMode = 'dark' | 'light' | 'auto' | 'off' interface MediaThemeProps (props of MediaTheme) type MenuPresentation = 'popover' | 'bottom-sheet' | 'adaptive' +interface MenuPressDownEvent + button: number + ctrlKey: boolean + metaKey?: boolean + nativeEvent?: PointerEvent + pointerId: number + pointerType: string + preventDefault: () => void + shiftKey?: boolean + target: EventTarget | null type MenuPressEffect = { type: 'none' } | { type: 'open' } | { type: 'highlight'; row: T } | { type: 'clear' } | { type: 'act'; row: T } | { type: 'settle'; stray: boolean; dismiss: boolean } type MenuPressEvent = { type: 'down'; target: 'trigger' | 'menu'; pointerType: MenuPressPointerType; row: T | null; time: number } | { type: 'opened'; time: number } | { type: 'move' … #b8bb8361 type MenuPressGesture = { phase: 'idle' } | { phase: 'triggerPress'; pointerType: MenuPressPointerType; startedAt: number } | { phase: 'open'; pointerType: MenuPressPointerType; opened … #8aa37415 @@ -5990,7 +6023,7 @@ interface UseLongPressOptions disabled?: boolean moveCancelPx?: number onLongPress: (point: { x: number; y: number }) => void -interface UseMenuPressOptions +interface UseMenuPressOptions @deprecated getScroller?: () => HTMLElement | null hitTest?: (x: number, y: number) => Element | null isEnabled?: boolean = true @@ -6002,7 +6035,20 @@ interface UseMenuPressOptions onHighlight?: (row: HTMLElement | null) => void onTriggerPress?: (pointerType: MenuPressPointerType) => boolean triggerRef?: RefObject -interface UseMenuPressReturn +interface UseMenuPressRefOptions extends Omit + getScroller?: () => HTMLElement | null + hitTest?: (x: number, y: number) => Element | null + isEnabled?: boolean = true + itemSelector: string + longPressDelayMs?: number = 500 + onActivate?: (row: HTMLElement, release: PointerEvent) => void + onDismiss?: () => void + onHighlight?: (row: HTMLElement | null) => void + onTriggerPress?: (pointerType: MenuPressPointerType) => boolean + startBehavior?: 'menu' | 'items' = 'menu' + triggerRef?: RefObject +type UseMenuPressRefReturn = Pick +interface UseMenuPressReturn @deprecated cancel: () => void isTriggerClickFromPress: () => boolean menuProps: { onPointerDown: (event: React.PointerEvent) => void; [MENU_PRESS_MARKER]: '' } @@ -9067,6 +9113,8 @@ AnnouncePoliteness (.) ClickableContainerResult (.) ContainerRevealOptions (.) ContentRevealOptions (.) +CreateMenuPressOptions (.) +CreateMenuPressReturn (.) EntryAnimationPreset (.) Hotkey (.) INTERACTIVE_SELECTORS (.) @@ -9076,6 +9124,7 @@ KeyboardHintOrientation (.) ListFocusOrientation (.) MENU_PRESS_MARKER (.) MENU_PRESS_SETTLE_MS (.) +MenuPressDownEvent (.) MenuPressEffect (.) MenuPressEvent (.) MenuPressGesture (.) @@ -9113,6 +9162,8 @@ UseListFocusReturn (.) UseLongPressHandlers (.) UseLongPressOptions (.) UseMenuPressOptions (.) +UseMenuPressRefOptions (.) +UseMenuPressRefReturn (.) UseMenuPressReturn (.) UseOverflowOptions (.) UseOverflowReturn (.) @@ -9123,6 +9174,7 @@ UseTreeFocusOptions (.) UseTreeFocusReturn (.) UseTypeaheadOptions (.) UseTypeaheadReturn (.) +createMenuPress (.) hasActiveFocusTrapEscape (.) hasInteractiveAncestor (.) hasTextSelection (.) diff --git a/packages/core/src/Breadcrumbs/BreadcrumbItem.tsx b/packages/core/src/Breadcrumbs/BreadcrumbItem.tsx index ebd6dac4e00f4..3cbb018bce87c 100644 --- a/packages/core/src/Breadcrumbs/BreadcrumbItem.tsx +++ b/packages/core/src/Breadcrumbs/BreadcrumbItem.tsx @@ -52,8 +52,7 @@ import {focusOutlineProps} from '../utils/focusOutline.stylex'; import {Icon} from '../Icon'; import {usePopover} from '../Popover/usePopover'; import {useListFocus} from '../hooks/useListFocus'; -import {useMenuPress} from '../hooks/useMenuPress'; -import {pressOnItemsOnly} from '../hooks/menuPressOnItems'; +import {useCoreMenuPress} from '../hooks/useMenuPress'; import {menuTouchActionStyles} from '../DropdownMenu/menuTouchAction.stylex'; import {useMenuOverflow} from '../DropdownMenu/useMenuOverflow'; import {useTypeahead} from '../hooks/useTypeahead'; @@ -572,10 +571,10 @@ function BreadcrumbMenuTrigger({ // A press that slides across the menu's items acts on the item it lifts on, // the highlight following the pointer, as DropdownMenu's does // (`family:action-groups` FR1-FR3, FR5); a mouse released outside closes it. - const menuPress = useMenuPress({ - menuRef: listRef, + const menuPress = useCoreMenuPress(listRef, { itemSelector: MENU_ITEM_SELECTOR, onDismiss: closeMenu, + startBehavior: 'items', }); const typeahead = useTypeahead({ @@ -701,11 +700,7 @@ function BreadcrumbMenuTrigger({ id={menuId} role="menu" aria-labelledby={triggerId} - {...menuPress.menuProps} - onPointerDown={pressOnItemsOnly( - menuPress.menuProps.onPointerDown, - MENU_ITEM_SELECTOR, - )} + onPointerDown={menuPress.handleMenuPointerDown} onKeyDown={listKeyDown} {...mergeProps( themeProps('breadcrumb-menu'), diff --git a/packages/core/src/ContextMenu/ContextMenu.test.tsx b/packages/core/src/ContextMenu/ContextMenu.test.tsx index e14cd2eeda51f..e687e6199b189 100644 --- a/packages/core/src/ContextMenu/ContextMenu.test.tsx +++ b/packages/core/src/ContextMenu/ContextMenu.test.tsx @@ -23,6 +23,7 @@ import { import userEvent from '@testing-library/user-event'; import {readFileSync} from 'node:fs'; import {ContextMenu} from './ContextMenu'; +import {MENU_PRESS_MARKER} from '../hooks/menuPressMarker'; import {ContextMenuGroup} from './index'; import { ContextMenuItem, @@ -293,6 +294,40 @@ describe('ContextMenu', () => { expect(HTMLElement.prototype.showPopover).not.toHaveBeenCalled(); }); + it('keeps a menuContent touch sheet as the press root of its rows', async () => { + render( + }> +
      Right-click me
      +       , + ); + fireEvent.contextMenu(screen.getByText('Right-click me')); + const row = await screen.findByRole('menuitem', {name: 'Rename'}); + // The rows leave their press paint to the press root they sit in. + expect(row.closest(`[${MENU_PRESS_MARKER}]`)).not.toBeNull(); + }); + + it('keeps the items sheet out of the press model, so its Back button keeps its press paint', async () => { + const user = userEvent.setup(); + render( + +
      Right-click me
      +       , + ); + fireEvent.contextMenu(screen.getByText('Right-click me')); + await user.click( + await screen.findByRole('button', {name: 'Move to project'}), + ); + const back = await screen.findByRole('button', {name: 'Back'}); + // The press feedback skips any element inside a press root. + expect(back.closest(`[${MENU_PRESS_MARKER}]`)).toBeNull(); + }); + it('drills into nested data items inside the BottomSheet', async () => { const user = userEvent.setup(); const onMove = vi.fn(); diff --git a/packages/core/src/ContextMenu/ContextMenu.tsx b/packages/core/src/ContextMenu/ContextMenu.tsx index ffc0999b774fe..402752628b1c1 100644 --- a/packages/core/src/ContextMenu/ContextMenu.tsx +++ b/packages/core/src/ContextMenu/ContextMenu.tsx @@ -76,7 +76,8 @@ import { import {useMenuOverflow} from '../DropdownMenu/useMenuOverflow'; import {useMenuDrillIn} from '../DropdownMenu/useMenuDrillIn'; import {useInternalListFocus} from '../hooks/useListFocus'; -import {useMenuPress} from '../hooks/useMenuPress'; +import {useCoreMenuPress} from '../hooks/useMenuPress'; +import {MENU_PRESS_MARKER} from '../hooks/menuPressMarker'; import {useTypeahead} from '../hooks/useTypeahead'; import {useLongPress} from '../hooks/useLongPress'; import {layerAnimations} from '../Layer/layerAnimations.stylex'; @@ -749,9 +750,16 @@ export function ContextMenu({ // The press model: the row under a release acts, the highlight follows a // held pointer, a mouse released outside dismisses and a finger leaves the - // menu open. - const menuPress = useMenuPress({ - menuRef: listRef, + // menu open. It runs on the menu element only, never on the items sheet's + // container. That container's Back keeps its press paint, which skips + // anything inside a press root. + const pressMenuRef = useRef(null); + const menuElementRef = useMergedRefs(listRef, pressMenuRef); + // The touch sheet mounts the menu in its lazy boundary's own commit, before + // the press attaches. The marker is in the markup, so the menu carries it + // from its first paint. + const pressMarker = {[MENU_PRESS_MARKER]: ''}; + const menuPress = useCoreMenuPress(pressMenuRef, { itemSelector: MENU_ITEM_SELECTOR, onDismiss: closeMenu, isEnabled: !usesBottomSheet, @@ -759,7 +767,8 @@ export function ContextMenu({ const renderedMenu = (