diff --git a/.changeset/hover-card-trigger-behavior.md b/.changeset/hover-card-trigger-behavior.md new file mode 100644 index 0000000000000..566d6f00c72a6 --- /dev/null +++ b/.changeset/hover-card-trigger-behavior.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/core': patch +--- + +[feat] HoverCard (and `useHoverCard`) take `triggerBehavior` (type `HoverCardTriggerBehavior`): `adaptive`, the default and the released behavior, where hover and focus preview the card and a tap opens it unless the trigger performs an action of its own; `hover`, where hover and focus preview it and touch never opens it; or `toggle`, where hover and focus still preview it and a click, a tap, Enter, or Space on a button trigger pins it open, and the same press closes it. A pinned card stays until Escape, a second press, a press outside, or focus leaving; a press on its own content keeps it, and it takes Escape before the layer beneath it. On the `HoverCard` component the press stops at its trigger, so a clickable row, card, or label around it does nothing, and the trigger's own handlers still run. With `useHoverCard` and a caller's trigger, nothing is rendered around the trigger to stop it: the trigger's own `onClick` runs, Astryx clickable rows, cards, and containers ignore the press, and a plain `onClick` element around the trigger receives it unless the trigger's `onClick` stops it. The trigger reports `aria-expanded` and `aria-controls`, and the card's `label` (else its text) is announced once when it pins, inside a modal too (`component:HoverCard` FR9 and FR10). On a trigger that is not a button, a labelled card keeps its own popup state and tap-only opening; `touchTrigger` decides taps only under `adaptive`; `hover` and `toggle` ignore it. + +@vjeux diff --git a/.changeset/touch-trigger-tap-deprecated.md b/.changeset/touch-trigger-tap-deprecated.md new file mode 100644 index 0000000000000..1f25f6803493e --- /dev/null +++ b/.changeset/touch-trigger-tap-deprecated.md @@ -0,0 +1,8 @@ +--- +'@astryxdesign/core': patch +'@astryxdesign/cli': patch +--- + +[feat] `touchTrigger="tap"` on Tooltip, HoverCard, `useTooltip` and `useHoverCard` is deprecated in favor of `triggerBehavior="toggle"` (DEP-0016); `touchTrigger` remains supported until the next minor; only `"tap"` is deprecated now. It keeps working as released, and beside `triggerBehavior="hover"` or `"toggle"`, `touchTrigger` is ignored. A `tap` call resolves to a `@deprecated` signature, so editors mark it with its replacement, and the docs name it. `astryx upgrade`, run explicitly, moves `touchTrigger` to `triggerBehavior` ahead of the removal, each rewrite keeping its behavior: it rewrites static `touchTrigger="tap"` to `triggerBehavior="toggle"` on an icon or span trigger with no `role`, `tabIndex`, event handler, or spread props, rewrites static `touchTrigger="none"` to `triggerBehavior="hover"`, and removes static `touchTrigger="auto"`, the default, where no props are spread; a trigger rewritten to the toggle keeps its production behavior, tap-only opening, and in development it logs the warning that only a button trigger pins. Every other `tap` site keeps its prop with a TODO naming the manual migration, and the upgrade lists each one with its file and line among the manual migrations it cannot make: a button, a link, an element with a `role`, `tabIndex`, event handler, or spread props, a trigger whose layer repeats its own text, a `useTooltip` or `useHoverCard` options object, an element it cannot identify, a computed value, or spread props, even beside a static `triggerBehavior="toggle"`; a `touchTrigger="none"` beside spread props or a computed `triggerBehavior` is listed the same way. A core codemod reports such a site by passing a `ManualMigrationRequired` to `api.report`, and still keeps its changes to the file. The table-comparison page template now uses the toggle; its modality rail becomes a button so it can pin its explanation. + +@vjeux diff --git a/.changeset/trigger-behavior-toggle.md b/.changeset/trigger-behavior-toggle.md new file mode 100644 index 0000000000000..a62594c3e4365 --- /dev/null +++ b/.changeset/trigger-behavior-toggle.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/core': patch +--- + +[feat] Tooltip (and `useTooltip`) take `triggerBehavior` (type `TooltipTriggerBehavior`): `adaptive`, the default and the released behavior, where hover and focus preview the tooltip and a tap opens it unless the trigger performs an action of its own; `hover`, where hover and focus preview it and no touch or pen tap opens it; or `toggle`, where hover and focus still preview it and a click, a tap, Enter, or Space on a button trigger pins it open, and the same press closes it. A pinned tooltip stays until Escape, a second press, a press outside, or focus leaving, and it takes Escape before the layer beneath it. The press stops at its trigger, so a clickable row, card, or label around it does nothing, and the trigger's own handlers still run. With `anchorRef`, or `useTooltip` with a caller's trigger, nothing is rendered around the trigger to stop it: the trigger's own `onClick` runs, Astryx clickable rows, cards, and containers ignore the press, and a plain `onClick` element around the trigger receives it unless the trigger's `onClick` stops it. The trigger reports `aria-expanded` and `aria-controls`, and the tooltip's text is announced once when it pins, inside a modal too (`component:Tooltip` FR9 and FR10). A trigger that is not a button gets a development warning and keeps tap-only opening; `touchTrigger` decides taps only under `adaptive`; `hover` and `toggle` ignore it. + +@vjeux diff --git a/apps/storybook/stories/TooltipToggleBehaviorA11y.stories.tsx b/apps/storybook/stories/TooltipToggleBehaviorA11y.stories.tsx new file mode 100644 index 0000000000000..44af02f8fb498 --- /dev/null +++ b/apps/storybook/stories/TooltipToggleBehaviorA11y.stories.tsx @@ -0,0 +1,201 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file TooltipToggleBehaviorA11y.stories.tsx + * @input Uses Tooltip, HoverCard, Button, ClickableCard and Layout from core + * @output The fixtures the toggle-behavior proofs drive in a real browser: a + * button whose only job is its tooltip, the same trigger inside a clickable + * card, an anchored trigger with its own click handler inside a clickable + * card, a trigger whose tooltip opens on no focus, one trigger per + * `triggerBehavior` value, and an unlabelled HoverCard of two paragraphs on + * a button. + * @position The reproduction path for the `triggerBehavior="toggle"` cases in + * `packages/core/src/Layer/__tests__/TouchTrigger.a11y.chromium.spec.ts` + * (`component:Tooltip` FR9, FR10; `component:HoverCard` FR9, FR10; + * `docs/specs/AST-009/spec.md` FR30: a + * browser claim is checked against a checked-in story). + * + * SYNC: The spec navigates to these story ids by name. Nothing at compile + * time catches a renamed export — the Chromium spec does, when it finds no + * story. + */ + +import {useRef, useState} from 'react'; +import type {Meta, StoryObj} from '@storybook/react'; +import {Button} from '@astryxdesign/core/Button'; +import {ClickableCard} from '@astryxdesign/core/ClickableCard'; +import {HoverCard} from '@astryxdesign/core/HoverCard'; +import {VStack} from '@astryxdesign/core/Layout'; +import {Tooltip} from '@astryxdesign/core/Tooltip'; + +const meta: Meta = { + title: 'a11y/Tooltip toggle behavior', + // Driven by the toggle-behavior spec, not photographed: the states under test + // are pins and closes across presses, which no single frame shows. + tags: ['no-visual'], + parameters: { + docs: { + description: { + component: + 'A button whose only job is its tooltip takes `triggerBehavior="toggle"`: a click, a tap, Enter, or Space pins the tooltip, and the same press closes it. Inside a clickable card, the press stops at the trigger.', + }, + }, + }, +}; + +export default meta; + +type Story = StoryObj; + +/** A press pins the tooltip open; the same press, Escape, or a press outside closes it. */ +export const ButtonToggle: Story = { + render: () => ( +
+ + + +
+ ), +}; + +function InAClickableCardRender() { + const [opened, setOpened] = useState(0); + return ( +
+ + setOpened(n => n + 1)}> + + Icon usage + + + + + + Card opened {opened} times + +
+ ); +} + +/** A press on the trigger pins its tooltip and stops there: the card does not open. */ +export const InAClickableCard: Story = { + render: () => , +}; + +function AnchoredActionInACardRender() { + const [opened, setOpened] = useState(0); + const [pressed, setPressed] = useState(0); + const anchorRef = useRef(null); + return ( +
+ + setOpened(n => n + 1)}> + + Icon usage + + + + + Card opened {opened} times + Button pressed {pressed} times + +
+ ); +} + +/** + * An anchored trigger with its own click handler, in a clickable card: the + * pinning press runs the trigger's own handler, and the card ignores it. + */ +export const AnchoredActionInACard: Story = { + render: () => , +}; + +/** A tooltip that opens on no focus: focus moving to another control still closes a pin. */ +export const FocusTriggerNever: Story = { + render: () => ( +
+ + + + + + +
+ ), +}; + +/** One info-only trigger per `triggerBehavior` value: a tap opens, never opens, or pins. */ +export const TriggerBehaviors: Story = { + render: () => ( +
+ + + Adaptive term + + + Hover term + + + + + +
+ ), +}; + +/** + * An unlabelled card of two paragraphs on a button: a press pins it, its text + * is announced with the paragraphs apart, and a press on that text, which + * moves focus to no element, keeps it pinned. + */ +export const HoverCardToggle: Story = { + render: () => ( +
+ +

Ada Lovelace

+

Analyst, Payments

+ + } + placement="below" + triggerBehavior="toggle"> + + +
+ ), +}; diff --git a/docs/specs/components/HoverCard/HoverCard.spec.md b/docs/specs/components/HoverCard/HoverCard.spec.md index 7c76b3929de96..6210be64be6ea 100644 --- a/docs/specs/components/HoverCard/HoverCard.spec.md +++ b/docs/specs/components/HoverCard/HoverCard.spec.md @@ -12,6 +12,7 @@ owners: [cixzhang] review_triggers: [public-api, behavior, theming, accessibility] verified_by: [ + packages/core/src/Layer/usePressPin.test.tsx, packages/core/src/HoverCard/HoverCard.test.tsx, packages/core/src/Layer/layerDismissalFamilies.test.tsx, packages/core/src/Layer/useTouchTrigger.test.ts, @@ -43,7 +44,7 @@ system_specs: [spec:AST-003, spec:AST-017] | Behavior | A floating card of rich, possibly interactive content anchored to a trigger. It opens on hover after `delay`, on focus, and on a qualifying tap, stays open while the pointer is on the trigger or card, and closes on leave, Escape, or a tap outside. With `label` it is a named dialog the trigger advertises; without, a group that describes the trigger. With `triggerBehavior="hover"`, touch never opens it. With `triggerBehavior="toggle"`, hover and focus preview it, and a click, tap, Enter, or Space on a button trigger pins it open until Escape, a second press, a press outside, or focus leaving. | | End-user impact | Mouse, keyboard, and touch users can open the preview, move into it to use its actions, and dismiss it; screen-reader users hear it as a named popup dialog or as the trigger's description. With the toggle, a pinned card stays open while people read or use it, whatever the pointer does. | | Builder impact | A trigger whose information lives only in the card passes `triggerBehavior="toggle"` and a button trigger (DEC-1). `touchTrigger="tap"` callers migrate by trigger kind: the deprecating release's codemod, an opt-in early migration when run explicitly, rewrites icon and span triggers and reports the rest (`component:Tooltip` DEP-0016), and `component:Tooltip` CLN-0024 names each remaining kind's replacement in 0.8.0, whose upgrade runs the codemod again. | -| Compatibility/readiness | Additive `triggerBehavior`, default `adaptive`, the released behavior; `touchTrigger="tap"` deprecated and `touchTrigger` removed at the next minor, `triggerBehavior="adaptive"` keeping its `auto` behavior and `triggerBehavior="hover"` its `none` behavior (`component:Tooltip` DEP-0016, CLN-0024). FR9, FR10, and AR3 are not yet built; eight further implementation gaps (see Current-state impact). | +| Compatibility/readiness | Additive `triggerBehavior`, default `adaptive`, the released behavior; `touchTrigger="tap"` deprecated and `touchTrigger` removed at the next minor, `triggerBehavior="adaptive"` keeping its `auto` behavior and `triggerBehavior="hover"` its `none` behavior (`component:Tooltip` DEP-0016, CLN-0024). Eight implementation gaps (see Current-state impact). | | Review checks | Reject an unnamed `dialog`, a card that cannot be hovered or dismissed (WCAG 1.4.13), Escape in the page that also closes a layer underneath, or `aria-expanded` set on a trigger whose role does not support it, a pinned card that closes on pointer leave, or a non-button toggle trigger without the development warning. | | Governing rules | WCAG 1.4.13, 2.1.1; `family:overlay-dismissal`; `spec:AST-003` FR4; `spec:AST-017` (deprecation); `architecture:public-component-api` INV9, INV21; `architecture:layer-runtime`; `architecture:interaction-modality`; `architecture:component-theming-surface`. | @@ -93,10 +94,6 @@ Consumer migration instructions belong in consumer docs and release notes. so a name or ID inside a clickable card opens its card on the tap that opens the card behind it. The walk ships with `component:Tooltip` CLN-0024 (FR5, DEC-1). -- **Toggle pinning is not built.** `triggerBehavior` and its three values, the - toggle (FR9), and the trigger wiring and announcement (FR10) are specified - and not implemented. `touchTrigger="tap"` is the shipped way to open on every - tap, and `touchTrigger="none"` the shipped way to open on no tap. - **A held-closed card still opens on focus.** With `isOpen={false}`, the hover path stays shut but focus on the trigger opens the card, so a caller cannot hold it closed. Escape then only reports `onOpenChange(false)`, which the @@ -161,19 +158,19 @@ Consumer migration instructions belong in consumer docs and release notes. ## Behavioral and layout contract -| ID | Invariant | Basis | -| ---- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------ | -| FR1 | The open card is one surface carrying only the `hover-card` target, padded on every logical edge, with `content` inside and the theme's body font. `xstyle`, `className`, and `style` apply to the card. | Current source and tests | -| FR2 | Trigger wiring: a single string or number child is wrapped in a focusable inline span; element `children` attach to their first element inside an inline-safe wrapper. | Current source and tests | -| FR3 | With `label`, the card is `role="dialog"` named by `label`; the trigger gets `aria-haspopup="dialog"` and, only when its role supports it, `aria-expanded`, each replacing the trigger's own value while the card is wired, plus `aria-controls` naming the card while open, merged with the trigger's own; the trigger's own values return on cleanup. Without `label`, the card is `role="group"` and the trigger's `aria-describedby` gains the card's id. | Current source and tests | -| FR4 | On a hover-capable pointer, entering the trigger opens the card after `delay` ms; leaving the trigger or the card closes it after `hideDelay` ms, and moving between them keeps it open. Any focus on the trigger, keyboard, pointer, or programmatic, opens it per `focusTrigger`, except the focus a tap leaves behind, the refocus after an Escape dismiss (FR6), and the refocus after a controlled close (FR7). Implementation gap: under `auto` the tab order is read only when the trigger's listeners attach, on mount and on a re-render that re-attaches them, so a trigger rendered with `tabindex="-1"` does not open on focus until such a re-render follows its move into the tab order (Current-state impact). | WCAG 1.4.13; tests | -| FR5 | On touch, a completed tap opens it per `triggerBehavior` when the finger lifts; a touch that becomes a scroll does not. Under `adaptive`, a tap that already acts leaves it shut, decided as `component:Tooltip` DEC-1 decides it: the innermost control from the tapped element outward, the trigger included, decides, and an `aria-disabled` control, or a label for a disabled control, is not an action (DEC-1). Until `component:Tooltip` CLN-0024 removes `touchTrigger`, an explicit `touchTrigger` decides instead under `adaptive`: `tap` opens on every tap and `none` on no tap. Implementation gap: the walk ships with `component:Tooltip` CLN-0024 in the next minor; until then `adaptive`, like the shipped `touchTrigger="auto"`, asks only whether the trigger itself performs an action, apart from a disabled control (Compatibility and migration). A tap on the card's own content keeps it open; a second tap on the trigger, a tap outside, or a scroll starting outside closes it. Under `hover`, no tap opens it. Under `toggle`, FR9 decides what a tap does. `hover` and `toggle` ignore `touchTrigger`. | Current source and tests | -| FR6 | Escape closes a visible card through the shared dismissal stack when it is the topmost layer in the page, so a layer underneath stays open. Implementation gap: a card rendered into another document, such as an iframe or a Picture-in-Picture window, with no layer beneath it is never closed by Escape (Current-state impact). When focus was inside the card, it returns to the trigger, and that refocus does not reopen the card. | WCAG 1.4.13; tests | -| FR7 | `isOpen` forces the card shown or hidden, and `isOpen={true}` suppresses hover and focus hiding; a controlled card answers Escape by calling `onOpenChange(false)` without hiding, and setting `isOpen={false}` while focus is inside the card returns focus to the trigger. `isDefaultOpen` opens it on mount, still dismissible. Implementation gap: with `isOpen={false}`, focus on the trigger still opens the card, and Escape then only reports `onOpenChange(false)` (Current-state impact). `isEnabled={false}` disables the triggers. `onOpenChange` reports each show and hide. | Current source and tests | -| FR8 | Content renders only while open; server markup and the first client render match, including a default-open card. `hasHoverIndication` (`auto`: a single string or number child) adds a hover cue to the inline trigger. | Current source and tests | -| FR9 | With `triggerBehavior="toggle"` and a trigger whose role is `button` (a `; +const b = ;`); + + expect(output).not.toContain('touchTrigger'); + expect(output).not.toContain('triggerBehavior'); + }); + + it('rewrites a static touchTrigger="none" to triggerBehavior="hover", also in place of an explicit adaptive', async () => { + const output = + await applyTransform(`import {Tooltip, HoverCard} from '@astryxdesign/core'; +const a = t; +const b = ;`); + + expect(output.match(/triggerBehavior='hover'/g)).toHaveLength(2); + expect(output).not.toContain('touchTrigger'); + expect(output).not.toContain('adaptive'); + expect(output).toContain('placement="below"'); + expect(output).not.toContain('TODO'); + }); + + it('reports a tap on a trigger whose layer repeats its own text, and leaves it: only plain icons and spans are rewritten', async () => { + const reports = []; + const output = await applyTransform( + `import {Tooltip} from '@astryxdesign/core'; +const a = Quarterly revenue report; +const b = Quarterly revenue report;`, + reports, + ); + + expect(code(output).match(/touchTrigger="tap"/g)).toHaveLength(2); + expect(code(output)).not.toContain('triggerBehavior'); + expect(output.match(new RegExp(TRIGGER_TODO, 'g'))).toHaveLength(2); + expect(reports).toHaveLength(2); + }); + + it('leaves touchTrigger="none" beside spread props or a computed triggerBehavior, with a TODO', async () => { + const reports = []; + const output = await applyTransform( + `import {Tooltip} from '@astryxdesign/core'; +const a = t; +const b = t;`, + reports, + ); + + expect(code(output).match(/touchTrigger="none"/g)).toHaveLength(2); + expect(output).toContain('props are also spread here'); + expect(output).toContain('a computed triggerBehavior is also set here'); + expect(reports).toHaveLength(2); + }); + + it('removes touchTrigger beside a static triggerBehavior="hover" or "toggle", which ignore it', async () => { + const output = + await applyTransform(`import {Tooltip, HoverCard} from '@astryxdesign/core'; +const a = ; +const b = ;`); + + expect(output).toContain('triggerBehavior="toggle"'); + expect(output).toContain('triggerBehavior="hover"'); + expect(output).toContain('placement="below"'); + expect(output).not.toContain('touchTrigger'); + }); + + it('leaves a TODO for a computed value, a competing triggerBehavior, and spread props', async () => { + const output = + await applyTransform(`import {Tooltip} from '@astryxdesign/core'; +const a = ?; +const b = ?; +const c = ?;`); + + expect(output).toContain('touchTrigger={mode}'); + expect(output).toContain('This value is computed'); + expect(output).toContain('triggerBehavior="adaptive"'); + expect(output.match(/touchTrigger="tap">/g)).toHaveLength(2); + expect(output).toContain('an explicit triggerBehavior="adaptive"'); + expect(output).toContain('props are also spread here'); + expect(output.match(/TODO\(astryx upgrade\)/g)).toHaveLength(3); + }); + + it('reports tap beside spread props even next to a static toggle, which a spread triggerBehavior would override', async () => { + const reports = []; + const output = await applyTransform( + `import {Tooltip, useTooltip} from '@astryxdesign/core'; +const a = ; +function B() { + const tip = useTooltip({triggerBehavior: 'toggle', ...rest, touchTrigger: 'tap'}); +}`, + reports, + ); + + expect(code(output)).toContain('touchTrigger="tap"'); + expect(code(output)).toContain("touchTrigger: 'tap'"); + expect(output.match(/props are also spread here/g)).toHaveLength(2); + expect(reports).toHaveLength(2); + expect(reports.every(r => r instanceof ManualMigrationRequired)).toBe(true); + }); + + it('leaves touchTrigger="auto" beside spread props as it is, with no TODO and no listing', async () => { + const reports = []; + const source = `import {Tooltip, useTooltip} from '@astryxdesign/core'; +const a = ; +function B() { + const tip = useTooltip({...rest, touchTrigger: 'auto'}); +}`; + const output = await applyTransform(source, reports); + + expect(output).toBe(source); + expect(reports).toHaveLength(0); + }); + + it('reports tap in useTooltip and useHoverCard options, whose trigger it cannot see, removes auto, and maps none to hover', async () => { + const output = + await applyTransform(`import {useTooltip, useHoverCard} from '@astryxdesign/core'; +function A() { + const tip = useTooltip({placement: 'below', touchTrigger: 'tap'}); + const card = useHoverCard({touchTrigger: 'auto', delay: 100}); + const kept = useHoverCard({touchTrigger: 'none'}); + return null; +}`); + + expect(output).toContain("touchTrigger: 'tap'"); + expect(output).toContain('The trigger this hook wires is not visible here'); + expect(code(output)).toMatch( + /useTooltip\(\{placement: 'below',\s*touchTrigger: 'tap'\}\)/, + ); + expect(output).not.toContain("touchTrigger: 'auto'"); + expect(output).toContain('delay: 100'); + expect(output).toContain("useHoverCard({triggerBehavior: 'hover'})"); + expect(output).not.toContain("touchTrigger: 'none'"); + }); + + it('leaves components and hooks that are not Core’s unchanged', async () => { + const source = `import {Tooltip, useTooltip} from './local'; +const a = ; +const b = useTooltip({touchTrigger: 'tap'});`; + expect(await applyTransform(source)).toBe(source); + }); + + it('reports every site it leaves to the upgrade listing, with its line, on every run', async () => { + const source = `import {Tooltip, useTooltip, Icon} from '@astryxdesign/core'; +const a = ; +const b = ?; +const c = ; +const d = useTooltip({touchTrigger: 'tap'});`; + const first = []; + const once = await applyTransform(source, first); + expect(first.every(f => f instanceof ManualMigrationRequired)).toBe(true); + expect(first.map(f => f.line)).toEqual([2, 3, 5]); + expect(first[0].message).toContain('not a plain icon or span'); + expect(first[1].message).toContain('This value is computed'); + expect(first[2].message).toContain('not visible here'); + expect(first.some(f => f.message.startsWith('TODO'))).toBe(false); + // The rewritten icon site is not reported; the others are, again. + const second = []; + expect(await applyTransform(once, second)).toBe(once); + expect(second).toHaveLength(3); + }); + + it('changes nothing on a second run', async () => { + const once = + await applyTransform(`import {Tooltip, Icon} from '@astryxdesign/core'; +const a = ; +const b = ; +const c = ;`); + 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 2af3bdfcf09ad..9391099225306 100644 --- a/packages/cli/assets/codemods/transforms/next/index.mjs +++ b/packages/cli/assets/codemods/transforms/next/index.mjs @@ -7,4 +7,14 @@ * this file into the resolved version folder. */ -export default []; +import migrateTouchTriggerTapToTriggerBehavior, { + meta as migrateTouchTriggerTapToTriggerBehaviorMeta, +} from './migrate-touch-trigger-tap-to-trigger-behavior.mjs'; + +export default [ + { + name: 'migrate-touch-trigger-tap-to-trigger-behavior', + transform: migrateTouchTriggerTapToTriggerBehavior, + meta: migrateTouchTriggerTapToTriggerBehaviorMeta, + }, +]; diff --git a/packages/cli/assets/codemods/transforms/next/migrate-touch-trigger-tap-to-trigger-behavior.mjs b/packages/cli/assets/codemods/transforms/next/migrate-touch-trigger-tap-to-trigger-behavior.mjs new file mode 100644 index 0000000000000..f41526ac3d9b4 --- /dev/null +++ b/packages/cli/assets/codemods/transforms/next/migrate-touch-trigger-tap-to-trigger-behavior.mjs @@ -0,0 +1,649 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file Codemod: migrate touchTrigger="tap" to triggerBehavior="toggle" + * + * DEP-0016 (`component:Tooltip` and `component:HoverCard` DEC-1): the + * deprecated `touchTrigger="tap"` on `Tooltip` and `HoverCard` becomes + * `triggerBehavior="toggle"` only where the trigger is statically a + * non-interactive icon or span: an Astryx `Icon` or an ``, a `` of + * text and icons, or a lone string the layer wraps in a span, with no + * `role`, `tabIndex`, event handler, or spread props on it or inside it. There the result + * is observably the same, since FR9 keeps a non-button toggle trigger on + * tap-only opening, as `tap` does. Every other `tap` site keeps its prop and + * gets a TODO with the manual migration: a button or a link (where the toggle + * changes what a click, Enter, or Space does), a trigger whose layer repeats + * its own text, a `useTooltip` or `useHoverCard` options object (its trigger + * element is not visible here), and any element the codemod cannot identify. + * A static `touchTrigger="auto"`, the default, is removed, since the default + * `adaptive` reproduces it, and a static `touchTrigger="none"` becomes + * `triggerBehavior="hover"`, which keeps its behavior (beside an explicit + * `triggerBehavior="adaptive"` it takes that value's place). Beside a static + * `triggerBehavior="hover"` or `"toggle"`, which ignore it, any + * `touchTrigger` is removed. + * + * Built on the shared `transformProp` helper for JSX and the same import + * resolution for the hooks, so it does not follow variables, helpers, + * wrappers, or types. Where it cannot prove the result it keeps the old prop + * and leaves a TODO instead of guessing: a computed `touchTrigger`, a + * `touchTrigger` beside a `triggerBehavior` that is `adaptive` or computed, an + * element or options object that also spreads props, a static toggle there + * included, since a spread `triggerBehavior` would override it, and a + * `touchTrigger="none"` beside spread props or a computed `triggerBehavior`. + * Beside spread props a `touchTrigger="auto"`, the default, is left as it is. + * Each site it leaves is also reported + * through `api.report` as a `ManualMigrationRequired` with its line, so the + * upgrade lists it for a person on every run until it is migrated; the TODO + * is added once, so a second run changes nothing in the source. + */ + +import {ManualMigrationRequired} from '../../outcome.mjs'; +import { + importedComponents, + isModuleBinding, + transformProp, +} from '../../transform-prop.mjs'; + +export const meta = { + title: 'Migrate touchTrigger="tap" to triggerBehavior="toggle"', + description: + 'Replaces the deprecated `touchTrigger="tap"` on Tooltip and HoverCard ' + + 'with `triggerBehavior="toggle"` where the trigger is a plain icon or span ' + + '(no role, tabIndex, event handler, or spread props), ' + + 'removes a static `touchTrigger="auto"` and rewrites a static ' + + '`touchTrigger="none"` to `triggerBehavior="hover"` where no props are ' + + 'spread. ' + + 'Leaves a TODO with the ' + + 'manual migration on every other tap site (a button, a link, a trigger ' + + 'whose layer repeats its own text, a useTooltip or useHoverCard options ' + + 'object, an element it cannot identify), where the value is computed, ' + + 'where an explicit adaptive or computed triggerBehavior is also set, and where ' + + 'props are spread, a static toggle there included.', +}; + +const OLD_NAME = 'touchTrigger'; +const NEW_NAME = 'triggerBehavior'; +const CORE_IMPORT_RE = /^@(astryxdesign|xds)\/core(?:\/|$)/; +const COMPONENT_PROPS = new Map([ + ['Tooltip', new Set([OLD_NAME])], + ['HoverCard', new Set([OLD_NAME])], +]); +const HOOKS = new Map([ + ['useTooltip', new Set()], + ['useHoverCard', new Set()], +]); + +const VALUE_COMMENT = + ' TODO(astryx upgrade): touchTrigger="tap" is deprecated (DEP-0016). ' + + 'This value is computed, so it was left unchanged: pass ' + + 'triggerBehavior="toggle" where it is "tap", triggerBehavior="hover" where ' + + 'it is "none", and drop touchTrigger where it is "auto". '; +const BEHAVIOR_COMMENT = + ' TODO(astryx upgrade): touchTrigger="tap" is deprecated (DEP-0016), but ' + + 'an explicit triggerBehavior="adaptive" or a computed one is also set here, under which touchTrigger still decides taps, so it was ' + + 'left unchanged. Decide which one this layer needs by hand. '; +const NONE_BEHAVIOR_COMMENT = + ' TODO(astryx upgrade): touchTrigger="none" becomes ' + + 'triggerBehavior="hover" before touchTrigger is removed (DEP-0016), but a ' + + 'computed triggerBehavior is also set here, under which touchTrigger still ' + + 'decides taps where it is "adaptive", so it was left unchanged. Pass ' + + 'triggerBehavior="hover" where it is "adaptive", by hand. '; +const TRIGGER_COMMENT = + ' TODO(astryx upgrade): touchTrigger="tap" is deprecated (DEP-0016). This ' + + 'trigger is not a plain icon or span (a button, a link, an element with a ' + + 'role, tabIndex, event handler, or spread props, a trigger whose layer ' + + 'repeats its own text, or an element the codemod cannot identify), ' + + 'and there triggerBehavior="toggle" changes what a click, Enter, or Space ' + + 'does, so it was left unchanged. Migrate it by hand: triggerBehavior="toggle" ' + + 'on a button whose only job is this layer; otherwise keep tap until the ' + + 'next minor. '; +const HOOK_COMMENT = + " TODO(astryx upgrade): touchTrigger: 'tap' is deprecated (DEP-0016). The " + + 'trigger this hook wires is not visible here, so it was left unchanged. ' + + 'On an icon or span trigger, or a button whose only job is this layer, ' + + "pass triggerBehavior: 'toggle'; otherwise keep tap until the next minor. "; +const NONE_SPREAD_COMMENT = + ' TODO(astryx upgrade): touchTrigger="none" becomes ' + + 'triggerBehavior="hover" before touchTrigger is removed (DEP-0016), but ' + + 'props are also spread here and may set touchTrigger or triggerBehavior, so ' + + 'it was left unchanged. Pass triggerBehavior="hover" by hand. '; +const SPREAD_COMMENT = + ' TODO(astryx upgrade): touchTrigger="tap" is deprecated (DEP-0016), but ' + + 'props are also spread here and may set touchTrigger or triggerBehavior, so ' + + 'it was left unchanged. Migrate it by hand: triggerBehavior="toggle" on an ' + + 'icon or span trigger, or a button whose only job is this layer; ' + + 'otherwise keep tap until the next minor. '; + +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; +} + +/** The static string of a node, or null. @param {any} node */ +function staticString(node) { + const value = unwrap(node); + if ( + value != null && + (value.type === 'StringLiteral' || value.type === 'Literal') && + typeof value.value === 'string' + ) { + return value.value; + } + if ( + value?.type === 'TemplateLiteral' && + value.expressions.length === 0 && + value.quasis.length === 1 + ) { + return value.quasis[0].value.cooked; + } + return null; +} + +/** Plain markup that an icon or span trigger may hold. */ +const PLAIN_TAGS = new Set([ + 'span', + 'b', + 'i', + 'em', + 'strong', + 'small', + 'abbr', + 'svg', + 'path', + 'g', + 'circle', + 'rect', + 'use', +]); + +/** @param {string} text */ +function normalizeText(text) { + return text.replace(/\s+/g, ' ').trim(); +} + +/** + * Whether an element carries nothing that makes it act or take focus: no + * `role`, `tabIndex`, `contentEditable`, event handler, or spread props. FR9 + * pins a `role="button"` trigger, so such an element would not keep `tap`'s + * behavior under the toggle; it migrates by hand. + * @param {any} opening a JSXOpeningElement + */ +function isInert(opening) { + return opening.attributes.every((/** @type {any} */ a) => { + if (a.type !== 'JSXAttribute' || a.name?.type !== 'JSXIdentifier') { + return false; + } + const name = a.name.name; + return ( + name !== 'role' && + name !== 'tabIndex' && + name !== 'contentEditable' && + !/^on[A-Z]/.test(name) + ); + }); +} + +/** + * Whether an element and everything inside it are inert markup: elements that + * pass `isInert`, and whitespace or static text between them. + * @param {any} element a JSXElement + */ +function isInertTree(element) { + if (!isInert(element.openingElement)) { + return false; + } + return element.children.every((/** @type {any} */ child) => { + if (child.type === 'JSXText') { + return true; + } + if (child.type === 'JSXElement') { + return isInertTree(child); + } + return ( + child.type === 'JSXExpressionContainer' && + (child.expression.type === 'JSXEmptyExpression' || + staticString(child.expression) != null) + ); + }); +} + +/** + * The text a JSX child list shows, when every child is plain: text, a static + * string, a plain tag, or an Astryx `Icon` (no text), each inert. Null + * otherwise. + * @param {any[]} children + * @param {Set} icons local names of Core's `Icon` + * @returns {string | null} + */ +function plainText(children, icons) { + let text = ''; + for (const child of children) { + if (child.type === 'JSXText') { + text += child.value; + } else if (child.type === 'JSXExpressionContainer') { + if (child.expression.type === 'JSXEmptyExpression') { + continue; + } + const value = staticString(child.expression); + if (value == null) { + return null; + } + text += value; + } else if (child.type === 'JSXElement') { + const name = child.openingElement.name; + if (name.type !== 'JSXIdentifier' || !isInert(child.openingElement)) { + return null; + } + if (icons.has(name.name)) { + if (!isInertTree(child)) { + return null; + } + continue; + } + if (!PLAIN_TAGS.has(name.name)) { + return null; + } + const inner = plainText(child.children, icons); + if (inner == null) { + return null; + } + text += inner; + } else { + return null; + } + } + return text; +} + +/** + * Whether a `touchTrigger="tap"` on this element may become + * `triggerBehavior="toggle"`: its trigger is statically a non-interactive icon + * or span whose layer does not repeat its text. + * @param {any} element the Tooltip or HoverCard JSXElement + * @param {Set} icons local names of Core's `Icon` + */ +function isPlainIconOrSpan(element, icons) { + const attributes = element.openingElement.attributes; + if ( + attributes.some( + (/** @type {any} */ a) => + a.type === 'JSXAttribute' && a.name?.name === 'anchorRef', + ) + ) { + return false; + } + const children = element.children.filter( + (/** @type {any} */ c) => + !(c.type === 'JSXText' && c.value.trim() === '') && + !( + c.type === 'JSXExpressionContainer' && + c.expression.type === 'JSXEmptyExpression' + ), + ); + if (children.length !== 1) { + return false; + } + const [child] = children; + let text; + if (child.type === 'JSXElement') { + const name = child.openingElement.name; + if (name.type !== 'JSXIdentifier') { + return false; + } + if (icons.has(name.name) || name.name === 'svg') { + return isInertTree(child); + } + if (name.name !== 'span' || !isInert(child.openingElement)) { + return false; + } + text = plainText(child.children, icons); + } else { + // A lone string: the layer wraps it in a focusable span. + text = plainText([child], icons); + } + if (text == null) { + return false; + } + const content = attributes.find( + (/** @type {any} */ a) => + a.type === 'JSXAttribute' && a.name?.name === 'content', + ); + const contentText = + content == null ? null : staticString(attributeValue(content)); + // A trigger whose layer repeats its own visible text: the toggle would pin + // it on a click (FR9's exemption), so it migrates by hand. + return ( + contentText == null || + normalizeText(text) === '' || + normalizeText(contentText) !== normalizeText(text) + ); +} + +/** @param {any} attribute */ +function attributeValue(attribute) { + return attribute.value?.type === 'JSXExpressionContainer' + ? attribute.value.expression + : attribute.value; +} + +/** Whether a JSX attribute's static value is `none`. @param {any} attribute */ +function isNone(attribute) { + return staticString(attributeValue(attribute)) === 'none'; +} + +/** @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; +} + +/** + * Marks a site the codemod leaves for a person: a TODO on the node, added + * once, and the same note reported to the upgrade's manual-migration listing + * with its line, on every run until the site is migrated (`spec:AST-017` + * FR29c). Returns whether the source changed. + * @param {any} j @param {any} api @param {any} node @param {string} text + */ +function leaveForPerson(j, api, node, text) { + api.report?.( + new ManualMigrationRequired( + text.trim().replace(/^TODO\(astryx upgrade\): /, ''), + {line: node.loc?.start?.line ?? null}, + ), + ); + return addComment(j, node, text); +} + +/** + * What to do with one `touchTrigger`, given its static value, the static + * `triggerBehavior` beside it (undefined when absent, null when computed), and + * whether props are spread beside it. `hover` means: the layer takes + * `triggerBehavior="hover"` and loses `touchTrigger`. + * @param {string | null} value + * @param {string | null | undefined} behavior + * @param {boolean} spreads + * @returns {'rewrite' | 'hover' | 'remove' | 'keep' | 'value' | 'behavior' | 'spread'} + */ +function decide(value, behavior, spreads) { + // Spread props may set touchTrigger or triggerBehavior, even over a static + // toggle, so no rewrite or removal here is provably the same. `auto`, the + // default, is left as it is; anything else is left for a person. + if (spreads) { + return value === 'auto' ? 'keep' : 'spread'; + } + // `hover` and `toggle` ignore `touchTrigger`, so beside them it is dead. + if (behavior === 'hover' || behavior === 'toggle') { + return 'remove'; + } + if (value == null) { + return 'value'; + } + if (value === 'auto') { + return 'remove'; + } + // `none` decides taps only under `adaptive`, where `hover` keeps it: no + // touch or pen tap opens the layer, and no press pins it. + if (value === 'none') { + return behavior === undefined || behavior === 'adaptive' + ? 'hover' + : 'behavior'; + } + if (behavior !== undefined) { + return 'behavior'; + } + return value === 'tap' ? 'rewrite' : 'keep'; +} + +/** + * @param {import('../../../../authoring/codemod/type').AstryxCodemodFile} file + * @param {import('../../../../authoring/codemod/type').CodemodTransformApi} api + * @returns {string | null | undefined} + */ +export default function transformer(file, api) { + if (!file.source.includes(OLD_NAME)) { + return undefined; + } + const j = api.jscodeshift; + const root = j(file.source); + let hasChanges = false; + const matchesImport = (/** @type {string} */ source) => + CORE_IMPORT_RE.test(source); + + // Core's Icon, by local name: an icon trigger. + /** @type {Set} */ + const icons = new Set(); + root.find(j.ImportDeclaration).forEach((/** @type {any} */ importPath) => { + const source = importPath.node.source.value; + if (typeof source !== 'string' || !matchesImport(source)) { + return; + } + for (const specifier of importPath.node.specifiers ?? []) { + if ( + specifier.type === 'ImportSpecifier' && + specifier.imported?.name === 'Icon' + ) { + icons.add(specifier.local?.name ?? 'Icon'); + } + } + }); + + // JSX: removals wait until the walk is done, so no attribute is skipped. + /** @type {Array<{opening: any, attribute: any}>} */ + const removals = []; + transformProp( + root, + j, + {matchesImport, components: COMPONENT_PROPS}, + (/** @type {any} */ propPath) => { + const opening = propPath.parent?.node; + const attributes = opening?.attributes ?? []; + const attribute = propPath.node; + const spreads = attributes.some( + (/** @type {any} */ a) => a.type === 'JSXSpreadAttribute', + ); + const pressAttribute = attributes.find( + (/** @type {any} */ a) => + a.type === 'JSXAttribute' && a.name?.name === NEW_NAME, + ); + const behavior = + pressAttribute == null + ? undefined + : staticString(attributeValue(pressAttribute)); + switch ( + decide(staticString(attributeValue(attribute)), behavior, spreads) + ) { + case 'rewrite': { + const element = propPath.parent?.parent?.node; + if ( + element?.type !== 'JSXElement' || + !isPlainIconOrSpan(element, icons) + ) { + hasChanges = + leaveForPerson(j, api, attribute, TRIGGER_COMMENT) || hasChanges; + return; + } + attribute.name.name = NEW_NAME; + const value = attributeValue(attribute); + const literal = unwrap(value); + if (literal?.type === 'TemplateLiteral') { + attribute.value = j.stringLiteral('toggle'); + } else { + literal.value = 'toggle'; + if (literal.raw != null) { + literal.raw = undefined; + } + if (literal.extra != null) { + literal.extra = undefined; + } + } + hasChanges = true; + return; + } + case 'hover': + if (pressAttribute == null) { + attribute.name.name = NEW_NAME; + attribute.value = j.stringLiteral('hover'); + } else { + pressAttribute.value = j.stringLiteral('hover'); + removals.push({opening, attribute}); + } + hasChanges = true; + return; + case 'remove': + removals.push({opening, attribute}); + return; + case 'value': + hasChanges = + leaveForPerson(j, api, attribute, VALUE_COMMENT) || hasChanges; + return; + case 'behavior': + hasChanges = + leaveForPerson( + j, + api, + attribute, + isNone(attribute) ? NONE_BEHAVIOR_COMMENT : BEHAVIOR_COMMENT, + ) || hasChanges; + return; + case 'spread': + hasChanges = + leaveForPerson( + j, + api, + attribute, + isNone(attribute) ? NONE_SPREAD_COMMENT : SPREAD_COMMENT, + ) || hasChanges; + return; + default: + } + }, + ); + for (const {opening, attribute} of removals) { + opening.attributes = opening.attributes.filter( + (/** @type {any} */ a) => a !== attribute, + ); + hasChanges = true; + } + + // useTooltip and useHoverCard: the options object written at the call. + const hooks = importedComponents(root, j, { + matchesImport, + components: HOOKS, + }); + if (hooks.direct.size > 0 || hooks.namespaces.size > 0) { + root.find(j.CallExpression).forEach((/** @type {any} */ callPath) => { + const callee = callPath.node.callee; + const isHook = + (callee.type === 'Identifier' && + hooks.direct.has(callee.name) && + isModuleBinding(callPath, callee.name)) || + (callee.type === 'MemberExpression' && + !callee.computed && + callee.object.type === 'Identifier' && + hooks.namespaces.has(callee.object.name) && + isModuleBinding(callPath, callee.object.name) && + HOOKS.has(callee.property.name)); + const options = unwrap(callPath.node.arguments[0]); + if (!isHook || options?.type !== 'ObjectExpression') { + return; + } + const properties = options.properties; + const keyOf = (/** @type {any} */ p) => + (p.type === 'ObjectProperty' || p.type === 'Property') && !p.computed + ? p.key.type === 'Identifier' + ? p.key.name + : staticString(p.key) + : null; + const touch = properties.find( + (/** @type {any} */ p) => keyOf(p) === OLD_NAME, + ); + if (touch == null) { + return; + } + const pressProperty = properties.find( + (/** @type {any} */ p) => keyOf(p) === NEW_NAME, + ); + const behavior = + pressProperty == null ? undefined : staticString(pressProperty.value); + const spreads = properties.some( + (/** @type {any} */ p) => + p.type === 'SpreadElement' || p.type === 'SpreadProperty', + ); + switch (decide(staticString(touch.value), behavior, spreads)) { + case 'rewrite': + // The trigger element is not visible at the hook's call. + hasChanges = + leaveForPerson(j, api, touch, HOOK_COMMENT) || hasChanges; + return; + case 'hover': + if (pressProperty == null) { + touch.key = j.identifier(NEW_NAME); + touch.value = j.stringLiteral('hover'); + touch.shorthand = false; + } else { + pressProperty.value = j.stringLiteral('hover'); + options.properties = properties.filter( + (/** @type {any} */ p) => p !== touch, + ); + } + hasChanges = true; + return; + case 'remove': + options.properties = properties.filter( + (/** @type {any} */ p) => p !== touch, + ); + hasChanges = true; + return; + case 'value': + hasChanges = + leaveForPerson(j, api, touch, VALUE_COMMENT) || hasChanges; + return; + case 'behavior': + hasChanges = + leaveForPerson( + j, + api, + touch, + staticString(touch.value) === 'none' + ? NONE_BEHAVIOR_COMMENT + : BEHAVIOR_COMMENT, + ) || hasChanges; + return; + case 'spread': + hasChanges = + leaveForPerson( + j, + api, + touch, + staticString(touch.value) === 'none' + ? NONE_SPREAD_COMMENT + : SPREAD_COMMENT, + ) || hasChanges; + return; + default: + } + }); + } + + if (!hasChanges) { + return undefined; + } + return root.toSource({quote: 'single'}); +} diff --git a/packages/cli/assets/templates/pages/table-comparison/page.tsx b/packages/cli/assets/templates/pages/table-comparison/page.tsx index 2dd59d468f790..25a2defcc7d8e 100644 --- a/packages/cli/assets/templates/pages/table-comparison/page.tsx +++ b/packages/cli/assets/templates/pages/table-comparison/page.tsx @@ -692,6 +692,20 @@ const styles = stylex.create({ paddingBlock: spacingVars['--spacing-3'], paddingInline: spacingVars['--spacing-4'], }, + // The modality rail's trigger: a button with no chrome of its own, filling + // its cell so the icons keep their end alignment. + railButton: { + appearance: 'none', + backgroundColor: 'transparent', + borderWidth: 0, + color: 'inherit', + cursor: 'pointer', + display: 'block', + font: 'inherit', + margin: 0, + padding: 0, + width: '100%', + }, // Keep the card inside the content lane so Table's own wrapper, rather than // an outer page surface, owns horizontal overflow and sticky positioning. tableFrame: { @@ -807,33 +821,37 @@ function ModalityRail({supported}: {supported: Modality[]}) { - - {MODALITY_ORDER.map(modality => { - const has = supported.includes(modality); - return ( - - - - ); - })} - + triggerBehavior="toggle"> + {/* The rail's explanation lives only in the tooltip, so its trigger is a + button: a click, a tap, Enter, or Space pins the explanation. */} + ); } diff --git a/packages/cli/authoring/codemod/type.ts b/packages/cli/authoring/codemod/type.ts index 44bec2765b825..7c977d03a5f88 100644 --- a/packages/cli/authoring/codemod/type.ts +++ b/packages/cli/authoring/codemod/type.ts @@ -24,7 +24,12 @@ export interface AstryxCodemodApi { jscodeshift: unknown; /** Report a statistic (no-op-friendly; provided for jscodeshift parity). */ stats: (...args: unknown[]) => void; - /** Report progress (no-op-friendly; provided for jscodeshift parity). */ + /** + * Report progress (no-op-friendly; provided for jscodeshift parity). Under + * `astryx upgrade`, a core transform that passes a `ManualMigrationRequired` + * (codemods/outcome.mjs) reports a manual migration the upgrade lists, while + * the file keeps the transform's result. + */ report: (...args: unknown[]) => void; } diff --git a/packages/core/src/HoverCard/HoverCard.doc.mjs b/packages/core/src/HoverCard/HoverCard.doc.mjs index 8240f0faff85f..f926e10261fa5 100644 --- a/packages/core/src/HoverCard/HoverCard.doc.mjs +++ b/packages/core/src/HoverCard/HoverCard.doc.mjs @@ -104,9 +104,16 @@ export const docs = { name: 'touchTrigger', type: "'auto' | 'tap' | 'none'", description: - 'What a tap does where there is no hover. auto opens on tap unless the trigger performs an action of its own (a button, link, or form control), whose tap belongs to the control; an aria-disabled control performs none, so a tap on it opens its reason. tap always opens. none never opens on touch. A tap-opened card is dismissed by a tap outside it.', + 'What a tap does where there is no hover. auto opens on tap unless the trigger performs an action of its own (a button, link, or form control), whose tap belongs to the control; an aria-disabled control performs none, so a tap on it opens its reason. tap always opens; it is deprecated (DEP-0016): pass triggerBehavior="toggle" on a button trigger instead; `astryx upgrade` rewrites icon and span triggers and flags the rest. none never opens on touch. A tap-opened card is dismissed by a tap outside it. It decides only under triggerBehavior="adaptive", the default.', default: "'auto'", }, + { + name: 'triggerBehavior', + type: "'adaptive' | 'hover' | 'toggle'", + description: + "How the trigger opens the card. adaptive (the default): hover and focus preview it, and a tap opens it unless the trigger performs an action of its own, as touchTrigger decides while it remains. hover: hover and focus preview it, and touch never opens it. toggle, on a button trigger: a click, a tap, Enter, or Space pins the card and the same press closes it, while hover and focus still preview it. A pinned card stays until Escape, a second press on the trigger, a press outside the trigger and the card, or focus leaving them; a press on its own content keeps it. The press stops at the trigger, so a clickable row or card around it does not activate. The trigger reports aria-expanded and aria-controls, and the card's label (else its text) is announced once when it pins.", + default: "'adaptive'", + }, { name: 'isEnabled', type: 'boolean', @@ -167,7 +174,7 @@ export const docs = { { guidance: true, description: - 'Leave touchTrigger on auto so a tap opens the card on triggers that do nothing else, and stays out of the way on triggers that perform an action.', + 'Leave touchTrigger on auto so a tap opens the card on triggers that do nothing else, and stays out of the way on triggers that perform an action. When the trigger is a button whose only job is the card, set triggerBehavior to toggle: a press pins the card and stops at the trigger.', }, { guidance: false, @@ -289,9 +296,16 @@ export const docsZh = { name: 'touchTrigger', type: "'auto' | 'tap' | 'none'", description: - '在没有悬停的触摸设备上，轻点的行为。auto：轻点即打开，除非触发元素本身会执行操作（按钮、链接、表单控件），此时轻点归该控件所有；设置了 aria-disabled 的控件不执行操作，因此轻点它会显示其原因。tap：始终轻点打开。none：触摸永不打开。轻点打开的卡片可通过点击卡片外部关闭。', + '在没有悬停的触摸设备上，轻点的行为。auto：轻点即打开，除非触发元素本身会执行操作（按钮、链接、表单控件），此时轻点归该控件所有；设置了 aria-disabled 的控件不执行操作，因此轻点它会显示其原因。tap：始终轻点打开，已弃用（DEP-0016）：按钮触发元素请改用 triggerBehavior="toggle"；`astryx upgrade` 会改写图标和 span 触发元素，并标记其余用法。none：触摸永不打开。轻点打开的卡片可通过点击卡片外部关闭。仅在 triggerBehavior 为 adaptive（默认）时生效。', default: "'auto'", }, + { + name: 'triggerBehavior', + type: "'adaptive' | 'hover' | 'toggle'", + description: + '触发元素如何打开卡片。adaptive（默认）：悬停和聚焦会预览；轻点即打开，除非触发元素本身会执行操作（在 touchTrigger 仍存在期间由它决定）。hover：悬停和聚焦会预览，触摸永不打开。toggle（仅限按钮触发元素）：单击、轻点、Enter 或空格键固定打开，再次按下触发元素关闭；悬停和聚焦仍会预览。固定后，直到按 Escape、再次按下触发元素、在触发元素和卡片之外按下或焦点移出才关闭；在卡片内容上按下会保持固定。这次按下止于触发元素，外层可点击的行或卡片不会被激活。触发元素会报告 aria-expanded 和 aria-controls，固定时朗读一次卡片的 label（无 label 时朗读其文本）。', + default: "'adaptive'", + }, { name: 'isEnabled', type: 'boolean', @@ -346,7 +360,7 @@ export const docsZh = { { guidance: true, description: - 'Leave touchTrigger on auto so a tap opens the card on triggers that do nothing else, and stays out of the way on triggers that perform an action.', + 'Leave touchTrigger on auto so a tap opens the card on triggers that do nothing else, and stays out of the way on triggers that perform an action. When the trigger is a button whose only job is the card, set triggerBehavior to toggle: a press pins the card and stops at the trigger.', }, { guidance: false, @@ -398,7 +412,7 @@ export const docsDense = { { guidance: true, description: - 'Leave touchTrigger on auto so a tap opens the card on triggers that do nothing else, and stays out of the way on triggers that perform an action.', + 'Leave touchTrigger on auto so a tap opens the card on triggers that do nothing else, and stays out of the way on triggers that perform an action. When the trigger is a button whose only job is the card, set triggerBehavior to toggle: a press pins the card and stops at the trigger.', }, { guidance: false, @@ -439,7 +453,9 @@ export const docsDense = { hideDelay: 'Hide delay in ms.', focusTrigger: 'Controls when focus events trigger hover card.', touchTrigger: - 'Tap behavior where there is no hover. auto = tap opens unless the trigger acts (button/link/control; an aria-disabled control does not act); tap = always opens; none = never on touch. Tap-opened cards close on a tap outside.', + 'Tap behavior where there is no hover, under triggerBehavior="adaptive" only. auto = tap opens unless the trigger acts (button/link/control; an aria-disabled control does not act); tap = always opens (deprecated, DEP-0016: use triggerBehavior="toggle"); none = never on touch. Tap-opened cards close on a tap outside.', + triggerBehavior: + 'adaptive (default) = hover/focus preview, a tap opens unless the trigger acts; hover = hover/focus preview, touch never opens; toggle = a click/tap/Enter/Space on a button trigger pins the card (aria-expanded, label announced once), the same press closes it, and the press stops at the trigger.', isEnabled: 'Enable/disable hover + focus triggers.', label: 'Accessible name for the popup. With label: named role="dialog"; without: role="group".', diff --git a/packages/core/src/HoverCard/HoverCard.test.tsx b/packages/core/src/HoverCard/HoverCard.test.tsx index 745d10fbfb14b..7a2e8cb5d375a 100644 --- a/packages/core/src/HoverCard/HoverCard.test.tsx +++ b/packages/core/src/HoverCard/HoverCard.test.tsx @@ -28,7 +28,7 @@ import { } from '@testing-library/react'; import {renderToString} from 'react-dom/server'; import {hydrateRoot} from 'react-dom/client'; -import {StrictMode} from 'react'; +import {StrictMode, type MouseEvent as ReactMouseEvent} from 'react'; import {Button} from '../Button/Button'; import {ClickableCard} from '../ClickableCard/ClickableCard'; import {Theme, defineTheme} from '../theme'; @@ -1433,3 +1433,166 @@ describe('HoverCard surface press', () => { expect(screen.getByRole('checkbox')).not.toBeChecked(); }); }); + +describe('HoverCard triggerBehavior on every input (component:HoverCard FR5, FR9)', () => { + beforeEach(() => { + __resetInteractionModalityForTest(); + }); + + type Behavior = 'adaptive' | 'hover' | 'toggle'; + + /** An info-only trigger: a span, or under the toggle the button it pins. */ + function setup( + triggerBehavior?: Behavior, + touchTrigger?: 'auto' | 'tap' | 'none', + trigger = triggerBehavior === 'toggle' ? 'button' : 'span', + ) { + const onOpenChange = vi.fn<(open: boolean) => void>(); + render( + Ada Lovelace, analyst

} + delay={0} + onOpenChange={onOpenChange} + triggerBehavior={triggerBehavior} + touchTrigger={touchTrigger}> + {trigger === 'button' ? ( + + ) : ( + Ada + )} + , + ); + const opened = () => onOpenChange.mock.calls.some(([open]) => open); + return {trigger: screen.getByText('Ada'), opened}; + } + + /** A finger: it arrives, lands, lifts, then the click. */ + function tap(element: HTMLElement) { + fireEvent.pointerEnter(element, {pointerType: 'touch'}); + fireEvent.pointerDown(element, {pointerType: 'touch'}); + fireEvent.pointerUp(element, {pointerType: 'touch'}); + fireEvent.click(element); + } + + const settle = async () => + new Promise(resolve => { + setTimeout(resolve, 50); + }); + + describe.each([ + ['adaptive by default', undefined, true], + ['adaptive', 'adaptive', true], + ['hover', 'hover', false], + ['toggle', 'toggle', true], + ] as const)('%s', (_, behavior, tapOpens) => { + it('a mouse hover previews it', async () => { + const {trigger, opened} = setup(behavior); + fireEvent.pointerEnter(trigger, {pointerType: 'mouse'}); + fireEvent.mouseEnter(trigger); + await waitFor(() => expect(opened()).toBe(true)); + }); + + it('focus previews it', async () => { + const {trigger, opened} = setup(behavior); + fireEvent.focusIn(trigger); + await waitFor(() => expect(opened()).toBe(true)); + }); + + it(`a tap ${tapOpens ? 'opens' : 'never opens'} it`, async () => { + const {trigger, opened} = setup(behavior); + tap(trigger); + await settle(); + expect(opened()).toBe(tapOpens); + }); + + it.each(['mouse click', 'touch tap'] as const)( + `a %s on a button trigger ${behavior === 'toggle' ? 'pins it' : 'pins nothing'}`, + async press => { + const {trigger} = setup(behavior, undefined, 'button'); + if (press === 'mouse click') { + fireEvent.pointerDown(trigger, {pointerType: 'mouse'}); + fireEvent.pointerUp(trigger, {pointerType: 'mouse'}); + fireEvent.click(trigger); + } else { + tap(trigger); + } + await settle(); + expect(trigger.getAttribute('aria-expanded')).toBe( + behavior === 'toggle' ? 'true' : null, + ); + }, + ); + }); + + it.each([ + ['adaptive', 'none', 'span', 'decides: no tap opens it', false], + ['adaptive', 'tap', 'button', 'decides: every tap opens it', true], + ['hover', 'tap', 'button', 'is ignored: no tap opens it', false], + ['toggle', 'none', 'button', 'is ignored: a tap pins it', true], + ] as const)( + 'beside triggerBehavior="%s", touchTrigger="%s" on a %s %s', + async (behavior, touchTrigger, trigger, _, tapOpens) => { + const card = setup(behavior, touchTrigger, trigger); + tap(card.trigger); + await settle(); + expect(card.opened()).toBe(tapOpens); + }, + ); +}); + +describe('useHoverCard with a caller’s trigger under triggerBehavior="toggle" (component:HoverCard FR9)', () => { + function Card({ + onTriggerClick, + }: { + onTriggerClick?: (event: ReactMouseEvent) => void; + }) { + const hoverCard = useHoverCard({triggerBehavior: 'toggle', delay: 0}); + return ( + <> + + {hoverCard.renderHoverCard(

Ada Lovelace, analyst

)} + + ); + } + const trigger = () => screen.getByRole('button', {name: 'Ada'}); + + it('the trigger’s own onClick runs, and an Astryx ClickableCard around it does not activate', () => { + const onCard = vi.fn(); + const onTrigger = vi.fn(); + render( + + + , + ); + + fireEvent.click(trigger()); + + expect(onTrigger).toHaveBeenCalledTimes(1); + expect(onCard).not.toHaveBeenCalled(); + expect(trigger()).toHaveAttribute('aria-expanded', 'true'); + }); + + it('a plain onClick element around it receives the press, unless the trigger’s onClick stops it', () => { + const onRow = vi.fn(); + const {unmount} = render( +
+ +
, + ); + fireEvent.click(trigger()); + expect(onRow).toHaveBeenCalledTimes(1); + expect(trigger()).toHaveAttribute('aria-expanded', 'true'); + unmount(); + + onRow.mockClear(); + render( +
+ event.stopPropagation()} /> +
, + ); + fireEvent.click(trigger()); + expect(onRow).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/src/HoverCard/HoverCard.tsx b/packages/core/src/HoverCard/HoverCard.tsx index 4316875cdf889..77447ee859c4f 100644 --- a/packages/core/src/HoverCard/HoverCard.tsx +++ b/packages/core/src/HoverCard/HoverCard.tsx @@ -21,14 +21,17 @@ import * as stylex from '@stylexjs/stylex'; import { useHoverCard, type HoverCardFocusTrigger, + type HoverCardTriggerBehavior, type HoverCardTouchTrigger, } from './useHoverCard'; +import {isPressPinTrigger, stopPressPinClick} from '../Layer/usePressPin'; import type {LayerAlignment, LayerPlacement} from '../Layer/useLayer'; import type {BaseProps} from '../BaseProps'; import {colorVars, spacingVars} from '../theme/tokens.stylex'; export type { HoverCardFocusTrigger, + HoverCardTriggerBehavior, HoverCardTouchTrigger, } from './useHoverCard'; @@ -159,13 +162,37 @@ export interface HoverCardProps extends Pick< * What a tap does on a touch pointer, where there is no hover: * - `auto`: tap opens the card, unless the trigger performs an action of its * own (a button, a link, a form control) — that tap belongs to the control - * - `tap`: tap always opens the card, even on a trigger that acts + * - `tap`: tap always opens the card, even on a trigger that acts. + * Deprecated (DEP-0016): pass `triggerBehavior="toggle"` on a button trigger + * instead; `astryx upgrade` rewrites icon and span triggers and lists the + * rest * - `none`: touch never opens the card * + * It decides only under `triggerBehavior="adaptive"`, the default. + * * @default 'auto' */ touchTrigger?: HoverCardTouchTrigger; + /** + * How the trigger opens the card: + * - `adaptive` (the default): hover and focus preview it; a tap opens it + * unless the trigger performs an action of its own (`touchTrigger` + * decides while it remains) + * - `hover`: hover and focus preview it; touch never opens it + * - `toggle`: on a button trigger, a click, a tap, Enter, or Space pins the + * card open and the same press closes it; hover and focus still preview + * it. A pinned card stays until Escape, a second press on the trigger, a + * press outside the trigger and the card, or focus leaving them, and the + * press stops at the trigger: no clickable row or card around it + * activates. The trigger reports `aria-expanded` and `aria-controls`, and + * the card's `label` (else its text) is announced once when it pins. + * `touchTrigger` is ignored. + * + * @default 'adaptive' + */ + triggerBehavior?: HoverCardTriggerBehavior; + /** * Whether the hover card is enabled. * When false, hover/focus triggers are disabled. @@ -250,6 +277,17 @@ function mergeIds(...ids: (string | undefined | null)[]): string | undefined { * * ``` */ +export function HoverCard( + props: HoverCardProps & {touchTrigger?: 'auto' | 'none'}, +): ReactElement; +/** + * @deprecated `touchTrigger="tap"` is deprecated (DEP-0016): pass `triggerBehavior="toggle"` on a button trigger instead; `astryx upgrade` rewrites icon and span triggers and lists the rest. + */ +export function HoverCard( + props: HoverCardProps & {touchTrigger: 'tap'}, +): ReactElement; +/** Any `HoverCardProps`, such as props a wrapper forwards. */ +export function HoverCard(props: HoverCardProps): ReactElement; export function HoverCard({ children, content, @@ -259,6 +297,7 @@ export function HoverCard({ hideDelay = 200, focusTrigger = 'auto', touchTrigger = 'auto', + triggerBehavior = 'adaptive', isEnabled = true, label, onOpenChange, @@ -292,6 +331,7 @@ export function HoverCard({ hideDelay, focusTrigger, touchTrigger, + triggerBehavior, isEnabled, label, isOpen, @@ -335,14 +375,21 @@ export function HoverCard({ const existingHaspopup = firstChild.getAttribute('aria-haspopup'); const existingControls = firstChild.getAttribute('aria-controls'); const existingExpanded = firstChild.getAttribute('aria-expanded'); - const canExpand = supportsAriaExpanded(firstChild); - + // Under the toggle, the press pin owns aria-expanded (true only while + // pinned) and aria-controls on the button triggers it wires (FR10); the + // popup role stays here, and so does FR3's state on any other trigger. + const ownsToggleState = !( + triggerBehavior === 'toggle' && isPressPinTrigger(firstChild) + ); + const canExpand = ownsToggleState && supportsAriaExpanded(firstChild); firstChild.setAttribute('aria-haspopup', 'dialog'); // Only point aria-controls at the layer while it is open and in the DOM. // While closed, useLayer leaves only an inert