Skip to content

feat(Tooltip): pressTrigger="toggle" pins the tooltip on a press - #9232

Open
vjeux wants to merge 1 commit into
meta:mainfrom
vjeux:feat/tooltip-press-trigger
Open

vjeux wants to merge 1 commit into
meta:mainfrom
vjeux:feat/tooltip-press-trigger

Conversation

@vjeux

@vjeux vjeux commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Status: green at 26d7af9 (on main a55352c): every gate under Testing finished at this head.

User need

A tooltip that is the only home of its information (an info icon, a "What is this?" button) can't be kept open by touch, keyboard or screen-reader users. Hover opens it and leaving closes it. touchTrigger="tap" opens it on a tap, but a mouse click, Enter or Space doesn't keep it, and nothing pins it. So the information is a hover-only path, which architecture:interaction-modality rules out for content that has no other home.

Problem and solution fit

  • pressTrigger="toggle" makes the trigger a toggle. On a button trigger, a click, a tap, Enter or Space pins the tooltip, and the same press closes it. Hover and focus still preview it. That gives every modality one way to keep the tooltip.
  • A pinned tooltip is the person's own interaction, so it takes Escape before the layer beneath it (spec:AST-003 FR4, DEC-6).
  • The press stops at its trigger, so a toggle inside a clickable row, card or label doesn't also activate the container.

Current specification

component:Tooltip FR9, FR10, AR6 and DEC-1; spec:AST-003 FR4 and DEC-6. All are current at 1d8696a (#9175).

Public delta and caller burden

  • pressTrigger: 'none' | 'toggle' on Tooltip and useTooltip, with the type TooltipPressTrigger. The default none changes nothing.
    • With toggle on a button trigger (a <button> or role="button"), a click, a tap, Enter or Space pins the tooltip, and the same press closes it.
    • A pinned tooltip stays when the pointer leaves, and a press on its own text keeps it. It closes on Escape, a second press, a press outside, a scroll that starts outside, or focus moving onto an element outside the trigger and the tooltip, whatever focusTrigger is.
    • A controlled tooltip reports the toggled value through onOpenChange and doesn't change itself.
    • The trigger carries aria-expanded (true only while pinned) and aria-controls while the tooltip shows. When it pins, the tooltip's rendered text is announced once by the shared rendered-text announcer (spec:AST-086 FR12), which keeps blocks apart and leaves hidden content out. It reaches a polite live region through useAnnounce, so inside a modal it's heard in the modal's own region (spec:AST-086 FR14).
    • The press stops at its trigger: a clickable row, card or container around it doesn't activate, and a <label> around it doesn't operate its control. The trigger's own handlers still run. With anchorRef, the caller's own handlers still receive the press: the anchor's onClick runs, and a plain onClick element around the anchor receives it unless the anchor's onClick stops it; Astryx clickable rows, cards and containers ignore it (FR9, by the spec owners' ruling: a tap that already acts stays with its control, DEC-1). With the bare hook, Astryx clickable rows and cards ignore the pinning press, but a plain onClick ancestor still receives it; the component stops it.
    • A trigger that is not a button gets one development warning and keeps tap-only opening.
    • A pressTrigger that changes after the trigger mounted wires or unwires it then, and a tooltip whose pressTrigger turns to none while pinned closes on leave again.
    • Beside pressTrigger="toggle", touchTrigger is ignored.
  • Exemption (FR9): a trigger whose tooltip repeats its own visible text, such as a truncated Text that is its own tab stop, pins without being a button, and gets no warning, no toggle state and no announcement. It's an internal option for the components that own such triggers (useTooltipInternal({repeatsTriggerText: true})), not public API.

Before and after:

// before: a tap opens it; nothing keeps it
<Tooltip content="Rows are ranked by how many places import the icon" touchTrigger="tap">
  <Button label="What is this?">?</Button>
</Tooltip>
// after: a click, tap, Enter or Space pins it; the same press closes it
<Tooltip content="Rows are ranked by how many places import the icon" pressTrigger="toggle">
  <Button label="What is this?">?</Button>
</Tooltip>

Not in this pull request:

API changes

Added public rows, decided by component:Tooltip FR9 and FR10 (#9175). They go to human review under architecture:public-component-api INV25.

Change Package Signature Usage
added @astryxdesign/core TooltipProps.pressTrigger?: TooltipPressTrigger (default 'none') <Tooltip content="…" pressTrigger="toggle"><Button label="What is this?">?</Button></Tooltip>
added @astryxdesign/core TooltipOptions.pressTrigger?: TooltipPressTrigger useTooltip({pressTrigger: 'toggle'})
added @astryxdesign/core type TooltipPressTrigger = 'none' | 'toggle' const mode: TooltipPressTrigger = 'toggle';

Design and alternatives

  • One press-pin hook for the hover layers, Layer/usePressPin.ts. It owns the layer's touch trigger (useTouchTriggerInternal), so useTooltip makes one usePressPin call (architecture:react-component-runtime INV16).
  • A pin closes through the hover layers' existing owners: the touch trigger's outside dismissal, which the pin arms, for a press or scroll outside, and the pin's own focus-out listener on the trigger it wires, whatever focus listeners the host attaches, for focus moving onto an element outside. The hover and focus hides ask the pin's own isHeldOpen(), and a pin counts only while its layer shows.
  • The stop is built once in the shared press handler: the toggle's click listener prevents the default action and marks the event, the wrapper stops propagation after the trigger's own handlers ran, and useClickableContainer ignores a marked click.
  • Internal, by the spec owners' ruling: the shared LayerPressTrigger type and the touch trigger's armOutsideDismiss (on useTouchTriggerInternal) are not exported from the Layer entry point; the public useTouchTrigger return is unchanged.
  • The toggle owns aria-expanded and aria-controls while it wires a button trigger, and restores the trigger's own values when it unwires. The trigger element itself stays in a ref, never in React state.
  • The records reject a toggle on every trigger: a non-button can't announce the toggle state, so it keeps tap-only opening. They also reject removing touchTrigger in a patch, which is CLN-0024 at the next minor.

Evidence

  • Red first:
    • With main's sources, the pinned Tooltip case in layerDismissalFamilies.test.tsx fails.
    • With this pull request's first head, a5250c6, the first review's Tooltip cases fail: the pin announced through useAnnounce (inside a modal Dialog too), and a scroll that starts outside closing a pin.
    • With its second head, c543d57, a press on a pinned tooltip's own text (focus moving to no element) and a pressTrigger that turns to toggle after mount fail, in jsdom and, for the press, in Chromium (3 of 3 runs).
    • With its third head, 90ae917, the single-call case fails: the host called the touch trigger beside usePressPin.
    • With its fourth head, 40c79f0, a pinned tooltip whose pressTrigger turns to none never closes on leave, and a non-button trigger warns four times across a mount and two re-renders.
    • With its fifth head, fc05aff, a focusTrigger="never" tooltip stays pinned when focus moves away: in jsdom, and in Chromium (3 of 3 runs).
    • The anchorRef cases state FR9's anchored clause, which fc05aff already meets. A stop at the anchored trigger itself fails them: the action anchor's onClick and the plain row lose the press (2 of 5), and the Chromium case fails (3 of 3 runs).
  • Layer/usePressPin.test.tsx (31 tests):
    • Tooltip press toggle: click, tap, Enter and Space pin; pointer leave and a press on its own text keep the pin; Escape, a second press, a press outside, a scroll starting outside and focus leaving close it; controlled; a non-button trigger, warned once across re-renders; focus moving away under focusTrigger="never"; pressTrigger turning to toggle after mount, and to none while pinned.
    • Trigger state: aria-expanded, aria-controls, restore on cleanup, one announcement, including inside a modal Dialog.
    • Stop at the trigger: a clickable row, a ClickableCard, an Item row and a <label>, plus the released behavior without the toggle.
    • With anchorRef: the action anchor's own onClick fires; an Astryx ClickableCard, Item row and click-expanding Table row don't activate; a plain row receives the press unless the anchor's onClick stops it.
    • The repeated-text exemption.
  • layerDismissalFamilies.test.tsx: a pinned Tooltip over a Dialog takes Escape first, and the Dialog stays open.
  • Layer/usePressPin.calls.test.tsx: the host makes one usePressPin call per render, and every touch-trigger call comes from inside it.
  • hooks/useAnnounceRenderedText.adopters.test.tsx: a Tooltip pinned by a press hands its text to the shared rendered-text announcer and announces none of it itself; the registry check lists useTooltip as an adopter (architecture:react-component-runtime INV10).
  • TouchTrigger.a11y.chromium.spec.ts: real-touch and keyboard cases on the new a11y/Tooltip press toggle stories: a tap pins and the next tap closes; a tap outside closes; Enter pins and Space closes; inside a ClickableCard the tap pins and the card doesn't open. Under a mouse: a click on a pinned tooltip's own text, which moves focus to the body, keeps the pin; a click on an anchored trigger runs its own click handler and pins it, and the ClickableCard around it ignores the press; with focusTrigger="never", Tab to another control closes a pin.
  • Records:
    • Tooltip drops the "not yet built" notes on FR9, FR10 and AR6, and the "Press-to-pin is not built" current-state item.
    • FR9 gains a clause for anchorRef, by the spec owners' ruling: the caller's own handlers still receive the press (the anchor's onClick runs; a plain onClick element around the anchor receives it unless that onClick stops it; Astryx clickable rows, cards and containers ignore it). Its verification row names the anchored cases. FR9 keeps a gap note for its own-text clause, which no component reaches yet (only an internal option marks such a trigger), with its current-state item; its verification rows cite the tests above (added to verified_by). Its current state keeps one item, "DEP-0016 is not built", which feat(Tooltip, HoverCard, cli): deprecate touchTrigger="tap" (DEP-0016), with its upgrade codemod #9246 removes.
    • HoverCard's record keeps FR9, FR10 and AR3 marked not yet built here; the HoverCard pull request removes those notes.
  • Overlaps with open pull requests: fix(ChatMessageList, Tooltip, HoverCard): the chat log and text trigger spans draw the shared focus ring #8876 also edits Tooltip.tsx (trigger spans).

Split

The HoverCard wiring (about 580 lines, under component:HoverCard FR9, FR10 and AR3) is its own pull request, stacked between this one and #9246, by the maintainers' ruling. All three ship in the same release. This one is 16 files, +2,008/−66: the shared press pin and touch-trigger wiring, Tooltip's wiring, its tests and story, its record and docs, and its changeset.

Scope

  • One capability is added.
  • Unrelated fixes, cleanup, and policy changes were removed or split.
  • Consumer docs, focused tests, and migration evidence ship with the feature.
  • Public text and artifacts contain no internal Meta context.

Writing rubric results

The rubric in docs/contributing/spec-writing.md, over the changed parts of Tooltip.spec.md:

  • R1 narration: none; the rows state what the tooltip does.
  • R2 research in the record: none.
  • R3 evidence naming a change: none; the verification cells name test files and cases.
  • R4 rejected alternatives over one line or outside a decision: none.
  • R5 pull request or issue references remaining: none.
  • R6 ruled open questions still listing options: none.

Testing

Local gates at 26d7af9 (on main a55352c):

  • These pass: pnpm check:repo, pnpm check:knowledge, pnpm check:changesets, pnpm typecheck (0 errors), pnpm lint (0 errors), CI-strict ESLint on the changed files, the private-attribute baseline check, the root build, and the Storybook build.
  • Prettier passes on the touched files except two .doc.mjs files, which are unformatted on main too and outside the repository's formatting scope.
  • pnpm test:ui: 17,426 tests pass.
  • pnpm test:node: 15,102 pass. Its 2 failures, in the release cache.test.mjs, are independent of this change (fix(cli): a crashed run's release install is recognized by the Node it ran on, whatever its name #8520).
  • TouchTrigger.a11y.chromium.spec.ts passes in Chromium, twice (38 of 38), the three mouse cases included.
  • Stress for the press-toggle and mouse cases in Chromium: 42 of 42 on two cores with 4 workers, and 70 of 70 with 16 workers.
  • In WebKit, Tooltip's two .a11y.browser.spec.ts specs pass ten times each on one worker with 4 CPUs: 220 of 220.
  • The codemod suite: 1,038 tests pass.

@vercel

vercel Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Nov 1, 2026 1:20pm UTC

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 1, 2026
@astracat-bot

astracat-bot Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

AI review status for this pull request.

Review status Updated
🔄 In progress (for maintainers only) Nov 1, 2026, 1:20 PM UTC

@github-actions github-actions Bot added the needs:design-review Affects visuals — Design should review label Nov 1, 2026
@github-actions github-actions Bot added the needs:spec-owner-review Current knowledge records await owner approval label Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

🔎 Preview · fc05aff

Storybook · Sandbox · built from this PR's head

Modified components

Component Size LoC Visual · A11y · RTL
LayerProvider 68.8 kB (+1.7 kB) · 10.4 kB CSS (0) — ✓ · ✓ · ✗
lab/DiffViewer 🔴 50.6 kB (+3.4 kB) · 2.1 kB CSS (0) — — · — · —
lab/Schedule 🔴 67.1 kB (+3.3 kB) · 3.9 kB CSS (0) — — · — · —
122 more components
Component Size LoC Visual · A11y · RTL
lab/TourStep 🔴 61.2 kB (+3.2 kB) · 590 B CSS (0) — — · — · —
Popover 🔴 55.8 kB (+3.2 kB) · 12.4 kB CSS (0) — — · — · —
Carousel 🔴 56.7 kB (+3.2 kB) · 10.5 kB CSS (0) — — · — · —
DialogHeader 🔴 54.5 kB (+2.7 kB) · 10.1 kB CSS (0) — — · — · —
DrawerHeader 🔴 54.6 kB (+2.7 kB) · 11.5 kB CSS (0) — — · — · —
TableSelectionToolbar 🔴 57.9 kB (+2.6 kB) · 11.4 kB CSS (0) — — · — · —
ChatLayout 🔴 54.6 kB (+2.6 kB) · 10.2 kB CSS (0) — — · — · —
Stepper 🔴 53.1 kB (+2.6 kB) · 9.9 kB CSS (0) — — · — · —
ChatDictationButton 🔴 52.0 kB (+2.6 kB) · 9.8 kB CSS (0) — — · — · —
MobileNavToggle 🔴 51.8 kB (+2.6 kB) · 9.7 kB CSS (0) — — · — · —
Banner 🔴 53.3 kB (+2.6 kB) · 10.2 kB CSS (0) — — · — · —
TopNavMegaMenu 🔴 60.0 kB (+2.6 kB) · 11.1 kB CSS (0) — — · — · —
ChatSendButton 🔴 51.3 kB (+2.6 kB) · 9.4 kB CSS (0) — — · — · —
TopNavMenu 🔴 63.6 kB (+2.6 kB) · 10.5 kB CSS (0) — — · — · —
ToggleButton 🔴 49.4 kB (+2.5 kB) · 9.5 kB CSS (0) — — · — · —
ChatLayoutScrollButton 🔴 52.0 kB (+2.5 kB) · 9.9 kB CSS (0) — — · — · —
SideNavCollapseButton 🔴 52.2 kB (+2.5 kB) · 9.8 kB CSS (0) — — · — · —
SideNavHeading 🔴 64.1 kB (+2.5 kB) · 10.5 kB CSS (0) — — · — · —
lab/InfoTip 🔴 25.5 kB (+2.5 kB) · 458 B CSS (0) — — · — · —
SideNavItem 🔴 60.3 kB (+2.5 kB) · 10.4 kB CSS (0) — — · — · —
Toast 🔴 65.5 kB (+2.5 kB) · 10.0 kB CSS (0) — — · — · —
TopNavHeading 🔴 63.5 kB (+2.5 kB) · 10.3 kB CSS (0) — — · — · —
lab/CommentPin 🔴 52.9 kB (+2.5 kB) · 1.2 kB CSS (0) — — · — · —
StatusDot 🔴 22.6 kB (+2.5 kB) · 6.3 kB CSS (0) — — · — · —
Tooltip · Storybook 🔴 20.4 kB (+2.5 kB) · 5.0 kB CSS (0) 627 (+50) ✓ · ✓ · ✓
AvatarGroupOverflow 🔴 45.0 kB (+2.5 kB) · 6.5 kB CSS (0) — — · — · —
Button 🔴 48.5 kB (+2.5 kB) · 9.4 kB CSS (0) — — · — · —
IconButton 🔴 48.6 kB (+2.5 kB) · 9.4 kB CSS (0) — — · — · —
Link 🔴 49.0 kB (+2.5 kB) · 7.9 kB CSS (0) — — · — · —
TabMenu 🔴 60.8 kB (+2.5 kB) · 10.4 kB CSS (0) — — · — · —
FieldLabel 🔴 42.5 kB (+2.5 kB) · 5.9 kB CSS (0) — — · — · —
Avatar 🔴 41.6 kB (+2.5 kB) · 6.5 kB CSS (0) — — · — · —
AvatarGroup 🔴 44.0 kB (+2.5 kB) · 6.6 kB CSS (0) — — · — · —
SegmentedControl 🔴 43.0 kB (+2.4 kB) · 6.5 kB CSS (0) — — · — · —
AppShell 🔴 73.9 kB (+2.2 kB) · 12.0 kB CSS (0) — — · — · —
AlertDialog 🔴 62.0 kB (+2.2 kB) · 12.7 kB CSS (0) — — · — · —
SideNav 🔴 69.9 kB (+2.1 kB) · 10.9 kB CSS (0) — — · — · —
ContextMenuItem 🔴 58.5 kB (+2.0 kB) · 9.9 kB CSS (0) — — · — · —
DropdownMenuSubMenu 🔴 63.8 kB (+2.0 kB) · 9.9 kB CSS (0) — — · — · —
MobileNav 🔴 56.3 kB (+2.0 kB) · 10.4 kB CSS (0) — — · — · —
TopNav 🔴 57.2 kB (+2.0 kB) · 10.7 kB CSS (0) — — · — · —
Lightbox 56.5 kB (+1.9 kB) · 10.1 kB CSS (0) — — · — · —
CodeBlock 65.1 kB (+1.9 kB) · 11.0 kB CSS (0) — — · — · —
TimeInput 94.1 kB (+1.9 kB) · 14.1 kB CSS (0) — — · — · —
charts/ChartLegend 58.4 kB (+1.9 kB) · 20 B CSS (0) — — · — · —
ChatComposerInput 60.6 kB (+1.8 kB) · 10.3 kB CSS (0) — — · — · —
ChatComposerTokenElement 60.6 kB (+1.8 kB) · 10.3 kB CSS (0) — — · — · —
charts/ChartTooltip 58.3 kB (+1.8 kB) · 20 B CSS (0) — — · — · —
ContextMenu 80.3 kB (+1.8 kB) · 15.2 kB CSS (0) — — · — · —
charts/Chart 69.0 kB (+1.8 kB) · 20 B CSS (0) — — · — · —
TableCell 81.5 kB (+1.8 kB) · 15.7 kB CSS (0) — — · — · —
DateTimeInput 117.3 kB (+1.8 kB) · 15.6 kB CSS (0) — — · — · —
DateInput 105.7 kB (+1.8 kB) · 15.1 kB CSS (0) — — · — · —
richtext/RichTextView 255.4 kB (+1.8 kB) · 2.3 kB CSS (0) — — · — · —
TableHeaderCell 81.1 kB (+1.8 kB) · 15.4 kB CSS (0) — — · — · —
Calendar 60.0 kB (+1.8 kB) · 10.3 kB CSS (0) — — · — · —
BreadcrumbItem 72.5 kB (+1.8 kB) · 11.4 kB CSS (0) — — · — · —
CheckboxList 49.9 kB (+1.8 kB) · 8.5 kB CSS (0) — — · — · —
DropdownMenu 78.3 kB (+1.8 kB) · 15.4 kB CSS (0) — — · — · —
TextArea 53.4 kB (+1.8 kB) · 9.7 kB CSS (0) — — · — · —
Field 48.1 kB (+1.8 kB) · 8.3 kB CSS (0) — — · — · —
Thumbnail 55.5 kB (+1.8 kB) · 11.3 kB CSS (0) — — · — · —
lab/TransferListSelector 77.9 kB (+1.8 kB) · 1.9 kB CSS (0) — — · — · —
FileInput 59.3 kB (+1.8 kB) · 10.7 kB CSS (0) — — · — · —
lab/TransferList 72.3 kB (+1.8 kB) · 1.7 kB CSS (0) — — · — · —
Switch 53.1 kB (+1.8 kB) · 9.7 kB CSS (0) — — · — · —
richtext/RichTextEditor 274.9 kB (+1.8 kB) · 2.6 kB CSS (0) — — · — · —
Slider 52.8 kB (+1.7 kB) · 8.8 kB CSS (0) — — · — · —
InputGroup 48.7 kB (+1.7 kB) · 8.3 kB CSS (0) — — · — · —
TextInput 58.5 kB (+1.7 kB) · 10.8 kB CSS (0) — — · — · —
MoreMenu 78.5 kB (+1.7 kB) · 15.4 kB CSS (0) — — · — · —
RadioList 49.5 kB (+1.7 kB) · 8.3 kB CSS (0) — — · — · —
DateRangeInput 74.8 kB (+1.7 kB) · 11.5 kB CSS (0) — — · — · —
NumberInput 60.7 kB (+1.7 kB) · 11.0 kB CSS (0) — — · — · —
ComplexSelector 61.2 kB (+1.7 kB) · 11.0 kB CSS (0) — — · — · —
lab/ChatEmojiPicker 70.4 kB (+1.7 kB) · 773 B CSS (0) — — · — · —
lab/ChatReactionBar 70.9 kB (+1.7 kB) · 1.1 kB CSS (0) — — · — · —
RadioListItem 57.3 kB (+1.7 kB) · 10.0 kB CSS (0) — — · — · —
ListItem 58.8 kB (+1.7 kB) · 11.0 kB CSS (0) — — · — · —
lab/EmojiPicker 68.2 kB (+1.7 kB) · 722 B CSS (0) — — · — · —
CheckboxListItem 59.3 kB (+1.7 kB) · 11.0 kB CSS (0) — — · — · —
CheckboxInput 54.7 kB (+1.7 kB) · 9.6 kB CSS (0) — — · — · —
richtext/RichTextEditorToolbar 218.6 kB (+1.5 kB) · 454 B CSS (0) — — · — · —
PowerSearch 176.6 kB (+1.5 kB) · 20.3 kB CSS (0) — — · — · —
lab/MobileTokenizer 97.8 kB (+1.4 kB) · 880 B CSS (0) — — · — · —
lab/CommentThread 222.8 kB (+1.4 kB) · 2.4 kB CSS (0) — — · — · —
lab/MobilePowerSearch 162.0 kB (+1.4 kB) · 1.2 kB CSS (0) — — · — · —
ChatComposer 65.5 kB (+1.4 kB) · 11.1 kB CSS (0) — — · — · —
lab/Comments 227.4 kB (+1.4 kB) · 2.5 kB CSS (0) — — · — · —
BaseTypeahead 59.9 kB (+1.4 kB) · 10.2 kB CSS (0) — — · — · —
Tokenizer 74.5 kB (+1.3 kB) · 16.9 kB CSS (0) — — · — · —
Pagination 90.4 kB (+1.3 kB) · 15.7 kB CSS (0) — — · — · —
lab/ListInput 79.8 kB (+1.3 kB) · 1.6 kB CSS (0) — — · — · —
Selector 81.9 kB (+1.3 kB) · 15.4 kB CSS (0) — — · — · —
Markdown 150.2 kB (+1.3 kB) · 18.2 kB CSS (0) — — · — · —
Table 89.7 kB (+1.3 kB) · 15.9 kB CSS (0) — — · — · —
lab/CommentComposer 99.1 kB (+1.3 kB) · 1.9 kB CSS (0) — — · — · —
MultiSelector 77.8 kB (+1.2 kB) · 15.0 kB CSS (0) — — · — · —
Typeahead 67.3 kB (+1.2 kB) · 11.6 kB CSS (0) — — · — · —
CommandPalette 54.3 kB (+185 B) · 12.1 kB CSS (0) — — · — · —
CommandPaletteInput 33.8 kB (+101 B) · 8.4 kB CSS (0) — — · — · —
ChatMessageList 29.9 kB (+68 B) · 7.9 kB CSS (0) — — · — · —
HoverCard 18.2 kB (+58 B) · 5.0 kB CSS (0) — — · — · —
DropdownMenuItem 16.9 kB (+49 B) · 5.5 kB CSS (0) — — · — · —
DropdownMenuCheckboxItem 17.7 kB (+45 B) · 5.9 kB CSS (0) — — · — · —
SelectorOption 16.0 kB (+43 B) · 5.4 kB CSS (0) — — · — · —
TreeList 43.2 kB (+38 B) · 5.0 kB CSS (0) — — · — · —
ClickableCard 14.0 kB (+36 B) · 7.1 kB CSS (0) — — · — · —
Item 12.7 kB (+36 B) · 4.9 kB CSS (0) — — · — · —
DropdownMenuRadioItem 17.7 kB (+35 B) · 5.9 kB CSS (0) — — · — · —
SelectableCard 13.8 kB (+33 B) · 7.3 kB CSS (0) — — · — · —
Overlay 19.8 kB (+30 B) · 4.0 kB CSS (0) — — · — · —
Timer 9.5 kB (+30 B) · 6.9 kB CSS (0) — — · — · —
ProgressBar 8.0 kB (+28 B) · 6.9 kB CSS (0) — — · — · —
Token 31.5 kB (+22 B) · 4.8 kB CSS (0) — — · — · —
ChatToolCalls 32.9 kB (+19 B) · 9.2 kB CSS (0) — — · — · —
Spinner 25.7 kB (+19 B) · 6.7 kB CSS (0) — — · — · —
Text 7.4 kB (+1 B) · 6.0 kB CSS (0) — — · — · —
Heading 7.9 kB (−3 B) · 6.0 kB CSS (0) — — · — · —
Timestamp 28.4 kB (−7 B) · 9.9 kB CSS (0) — — · — · —
layer — — — · ✓ · —
Layer — — ✓ · ✓ · —

Package size: @astryxdesign/core 5.67 MB packed (+14.3 kB); no new dependencies.

API changes: core +12 · ~20 · −0 (report stale: run pnpm api:report); richtext +0 · ~1 · −0 (report stale: run pnpm api:report)

33 API changes (advisory)
Package Where Signature Usage
+ core . · Components · component Tooltip prop pressTrigger?: TooltipPressTrigger = 'none' <Tooltip pressTrigger="none" />
+ core . · Types · interface FieldStatusProps message: string import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {message: …}
+ core . · Types · interface FieldStatusProps ref?: React.Ref<HTMLDivElement> import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {ref: …}
+ core . · Types · interface FieldStatusProps type: InputStatusType import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {type: …}
+ core . · Types · interface FieldStatusProps variant?: FieldStatusVariant = 'attached' import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {variant: …}
+ core . · Types · interface FieldStatusProps xstyle?: StyleXStyles import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {xstyle: …}
+ core . · Types · interface FieldStatusProps inherits AriaAttributes, DOMAttributes, HTMLAttributes import type {FieldStatusProps} from '@astryxdesign/core'
+ core . · Types · interface SearchContext interface SearchContext<T extends SearchableItem = SearchableItem> import type {SearchContext} from '@astryxdesign/core'
+ core . · Types · interface TooltipOptions pressTrigger?: TooltipPressTrigger = 'none' import type {TooltipOptions} from '@astryxdesign/core'; const x: TooltipOptions = {pressTrigger: …}
+ core . · Types · type TooltipPressTrigger type TooltipPressTrigger = LayerPressTrigger import type {TooltipPressTrigger} from '@astryxdesign/core'
+ core ./Tooltip · Also exported from an earlier entry point · TooltipPressTrigger TooltipPressTrigger (.) import {TooltipPressTrigger} from '@astryxdesign/core/Tooltip'
+ core ./Typeahead · Also exported from an earlier entry point · SearchContext SearchContext (.) import {SearchContext} from '@astryxdesign/core/Typeahead'
~ core . · Components · component ComplexSelector prop statusVariant?: FieldStatusVariant = 'attached' for input selectors; 'detached' for secondary and ghost selectors → prop statusVariant?: FieldStatusPlacement = 'attached' for input selectors; 'detached' for secondary and ghost selectors <ComplexSelector statusVariant="attached" />
~ core . · Components · component DateInput prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' <DateInput statusVariant="attached" />
~ core . · Components · component DateRangeInput prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' <DateRangeInput statusVariant="attached" />
~ core . · Components · component Field prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' <Field statusVariant="attached" />
~ core . · Components · component FieldStatus component FieldStatus(props: FieldStatusProps) → component FieldStatus @deprecated <FieldStatus />
~ core . · Components · component FieldStatus prop variant?: FieldStatusVariant = 'attached' → prop variant: FieldStatusVariant = 'attached' <FieldStatus variant="attached" />
~ core . · Components · component FileInput prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' <FileInput statusVariant="attached" />
~ core . · Components · component MultiSelector prop statusVariant?: FieldStatusVariant = 'attached' for input selectors; 'detached' for secondary and ghost selectors → prop statusVariant?: FieldStatusPlacement = 'attached' for input selectors; 'detached' for secondary and ghost selectors <MultiSelector statusVariant="attached" />
~ core . · Components · component NumberInput prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' <NumberInput statusVariant="attached" />
~ core . · Components · component PowerSearch prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' <PowerSearch statusVariant="attached" />
~ core . · Components · component Selector prop statusVariant?: FieldStatusVariant = 'attached' for input selectors; 'detached' for secondary/ghost selectors → prop statusVariant?: FieldStatusPlacement = 'attached' for input selectors; 'detached' for secondary/ghost selectors <Selector statusVariant="attached" />
~ core . · Components · component TextArea prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' <TextArea statusVariant="attached" />
~ core . · Components · component TextInput prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' <TextInput statusVariant="attached" />
~ core . · Components · component TimeInput prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' <TimeInput statusVariant="attached" />
~ core . · Components · component Tokenizer prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' <Tokenizer statusVariant="attached" />
~ core . · Components · component Typeahead prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' <Typeahead statusVariant="attached" />
~ core . · Types · interface FieldStatusProps interface FieldStatusProps extends BaseProps<HTMLDivElement> (props of FieldStatus) → interface FieldStatusProps extends BaseProps<HTMLDivElement> import type {FieldStatusProps} from '@astryxdesign/core'
~ core . · Types · interface FieldStatusVariantMap tooltip: true → tooltip: true @deprecated import type {FieldStatusVariantMap} from '@astryxdesign/core'; const x: FieldStatusVariantMap = {tooltip: …}
~ core . · Types · interface SearchSource search: (query: string) => Promise<T[]> | T[] → search: (query: string, context?: SearchContext<T>) => Promise<T[]> | T[] import type {SearchSource} from '@astryxdesign/core'; const x: SearchSource = {search: …}
~ core . · Types · interface UseInputStatusIconOptions statusVariant?: FieldStatusVariant → statusVariant?: FieldStatusPlacement import type {UseInputStatusIconOptions} from '@astryxdesign/core'; const x: UseInputStatusIconOptions = {statusVariant: …}
~ richtext . · Components · component RichTextEditor prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusVariant | 'tooltip' = 'attached' <RichTextEditor statusVariant="tooltip" />

Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.


Generated by PR Enrichment workflow | View full report

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested: the toggle behaves as component:Tooltip FR9 describes for button triggers, but a few gaps need fixing first.

  • A pin inside a modal Dialog is announced with announceMessage into the page-body live region, which the modal hides, so announce through useAnnounce or useAnnounceRenderedText to get spec:AST-086 FR14 routing.
  • A labelled HoverCard on a non-button trigger loses aria-controls/aria-expanded under pressTrigger="toggle" (HoverCard.tsx:380), which also makes the codemod's span rewrite change behavior; hand that state to the pin only when it wires the trigger, and test it.
  • The outside-press Effect in usePressPin duplicates useTouchTrigger's outside dismissal; please extend that owner instead.
  • The hook docs promise the press stops at the trigger (it does not without the component wrapper), touchTrigger="tap" has no @deprecated tag, and TooltipPressTrigger/HoverCardPressTrigger are missing from the entry-point exports.
  • At 2,307 authored lines, please split the DEP-0016 codemod and deprecation into a follow-up in the same release.

[Automated review]

Comment thread packages/core/src/Layer/usePressPin.ts Outdated
announcedRef.current = true;
const text = optionsRef.current.announcement()?.trim();
if (text != null && text !== '') {
announceMessage(text, 'polite');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

announceMessage always writes to the page-body live region. Inside an open modal Dialog, that region is inert, so the pin is never heard; useAnnounce() routes to the modal's own region (spec:AST-086 FR14). The shared useAnnounceRenderedText already reads a layer's rendered text and announces it through useAnnounce, so gating it on the pin would replace this Effect.


// Under the toggle, the press pin owns aria-expanded (true only while
// pinned) and aria-controls (FR10); the popup role stays here.
const ownsToggleState = pressTrigger !== 'toggle';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hands aria-controls/aria-expanded to the press pin for every trigger, but usePressPin wires only button triggers. A labelled HoverCard on a span or link trigger therefore loses HoverCard FR3's popup state, and the codemod rewrites such spans. Something like pressTrigger === 'toggle' && isPressPinTrigger(firstChild) keeps FR3 for the rest.

Comment thread packages/core/src/Layer/usePressPin.ts Outdated
Comment on lines +287 to +319
// While pinned, a press or focus outside the trigger and the layer closes it.
useEffect(() => {
if (!isToggleMode || trigger == null || !isExpanded) {
return;
}
const ownerDocument = trigger.ownerDocument;
const isInside = (target: EventTarget | null) => {
if (!(target instanceof Node)) {
return false;
}
const surface = ownerDocument.getElementById(layerId);
return (
trigger.contains(target) ||
(surface != null && surface.contains(target))
);
};
const onPointerDown = (event: PointerEvent) => {
if (!isInside(event.target)) {
close();
}
};
const onFocusIn = (event: FocusEvent) => {
if (!isInside(event.target)) {
close();
}
};
ownerDocument.addEventListener('pointerdown', onPointerDown, true);
ownerDocument.addEventListener('focusin', onFocusIn, true);
return () => {
ownerDocument.removeEventListener('pointerdown', onPointerDown, true);
ownerDocument.removeEventListener('focusin', onFocusIn, true);
};
}, [isToggleMode, trigger, isExpanded, layerId, close]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useTouchTrigger (armOutsideDismiss), which both hooks already call, owns outside dismissal for hover layers, armed from the opening handler. This Effect adds a second path with its own containment test. Could the pin arm that owner from toggle, and could focus leaving close a pinned layer through the existing focus-leave path?

{name: 'focusTrigger', type: "'auto' | 'always' | 'never'", description: 'When focus should open the tooltip. auto only attaches focus listeners to naturally focusable elements.', default: "'auto'"},
{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; tap always opens (an info icon rendered as a button); none never opens on touch.', default: "'auto'"},
{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; tap always opens (deprecated, DEP-0016: use pressTrigger="toggle"); none never opens on touch. Ignored under pressTrigger="toggle".', default: "'auto'"},
{name: 'pressTrigger', type: "'none' | 'toggle'", description: 'Whether a press on the trigger pins the tooltip open. none leaves taps to touchTrigger; toggle, on a button trigger, pins on a click, a tap, Enter, or Space and closes on the same press, Escape, a press outside, or focus leaving. The trigger carries aria-expanded and aria-controls, the tooltip is announced once when it pins, and the press stops at the trigger.', default: "'none'"},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The press stops at the trigger" holds only for the Tooltip/HoverCard components, whose wrapper calls stopPropagation. With the hook alone, a button inside <div onClick> pins the tooltip and also fires the row's handler. The same wording is in useHoverCard.doc.mjs and both hooks' JSDoc.

Comment thread packages/core/src/Tooltip/Tooltip.tsx Outdated
* as a button wants: it looks like an action to the DOM, but revealing the
* tooltip is the only thing it does
* - `tap`: tap always opens the tooltip, even on a trigger that acts.
* Deprecated (DEP-0016): pass `pressTrigger="toggle"` instead; `astryx

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is prose only. DEP-0016's warning field and spec:AST-017 FR29a ask for static @deprecated metadata on tap, and the description says it is present, but no @deprecated tag exists for it.

export type {TooltipFocusTrigger, TooltipTouchTrigger} from './useTooltip';
export type {
TooltipFocusTrigger,
TooltipPressTrigger,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TooltipPressTrigger (and HoverCardPressTrigger) is not added to Tooltip/index.ts, HoverCard/index.ts or Layer/index.ts. Those files export the FocusTrigger/TouchTrigger siblings, so the new types are unreachable from @astryxdesign/core (architecture:public-component-api INV1).

@vjeux
vjeux force-pushed the feat/tooltip-press-trigger branch from a5250c6 to c543d57 Compare November 1, 2026 02:06
@github-actions github-actions Bot removed the needs:design-review Affects visuals — Design should review label Nov 1, 2026
@vjeux vjeux changed the title feat(Tooltip, HoverCard): pressTrigger="toggle" pins the layer on a press; deprecate touchTrigger="tap" feat(Tooltip, HoverCard): pressTrigger="toggle" pins the layer on a press Nov 1, 2026
@vjeux

vjeux commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Per the owner's ruling, DEP-0016 (the @deprecated metadata, docs and codemod) moved to a stacked follow-up that lands in the same release; the other four points are fixed in c543d57.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested: the toggle now pins and closes as component:Tooltip FR9 describes and the earlier findings are fixed, but a few gaps remain.

  • In Chromium, a press on a pinned HoverCard's (or Tooltip's) own text closes it, because the focus-leave check treats focus moving to the page body as leaving (usePressPin.ts:353-359), while HoverCard FR9 says such a press keeps the pin.
  • The pin reads textContent and announces it from its own Effect, but architecture:react-component-runtime INV10 puts that in useAnnounceRenderedText, which also keeps an unlabelled card's paragraphs apart (today it says "Ada LovelaceAnalyst, Payments").
  • The HoverCard verification row cites Enter, Space, press-outside, focus-leave, pointer-leave and unlabelled cases that no HoverCard test covers yet.
  • Smaller: the touchDismissRef bridge Effect and the trigger element kept in state in usePressPin, the duplicated stopPressPinClick, and the changeset's forward-looking DEP-0016 sentence.

[Automated review]

Comment thread packages/core/src/Layer/usePressPin.ts Outdated
Comment on lines +353 to +359
if (
next instanceof Node &&
(trigger?.contains(next) === true || surface?.contains(next) === true)
) {
return true;
}
close();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the person presses non-focusable text inside the pinned layer, Chromium moves focus from the trigger to the body, so relatedTarget is null and this closes the pin. A pinned HoverCard then closes on a press on its own content, against HoverCard FR9 ("a press on its own content keeps it pinned"), and a pinned Tooltip closes the same way. Could a focus move caused by a press inside the trigger or the layer be left to useTouchTrigger's outside dismissal, with this closing only when focus lands outside both?

Comment thread packages/core/src/Layer/usePressPin.ts Outdated
Comment on lines +399 to +412
useEffect(() => {
if (!isToggleMode || trigger == null || !isExpanded || !wiresState) {
announcedRef.current = false;
return;
}
if (announcedRef.current) {
return;
}
announcedRef.current = true;
const text = optionsRef.current.announcement()?.trim();
if (text != null && text !== '') {
announce(text, 'polite');
}
}, [isToggleMode, trigger, isExpanded, wiresState, announce]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Effect announces text the hosts read with textContent. architecture:react-component-runtime INV10 keeps reading and announcing a component's rendered text in hooks/useAnnounceRenderedText.ts, which skips aria-hidden content and keeps blocks apart; an unlabelled HoverCard with two paragraphs is announced here as "Ada LovelaceAnalyst, Payments".

Comment thread packages/core/src/Layer/usePressPin.ts Outdated
}, []);

// The wired trigger, as state: its ARIA follows the pin through effects.
const [trigger, setTrigger] = useState<HTMLElement | null>(null);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

architecture:react-component-runtime INV1: DOM nodes must not be stored in React state. The trigger can live in triggerRef alone, with attach/detach setting and restoring its aria-expanded/aria-controls, which would also replace the restore Effect below.

Comment on lines +440 to +442
useEffect(() => {
touchDismissRef.current = {arm: armOutsideDismiss, disarm: clearTapOpen};
}, [armOutsideDismiss, clearTapOpen]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Effect only copies useTouchTrigger's callbacks into a ref, so it synchronizes no external system (INV3), and the same bridge and hideFromTouch are copied in useHoverCard.tsx:450-452. Letting usePressPin own the touch-trigger wiring (or useTouchTrigger take the pin's close) would leave each hook one call.

Comment thread .changeset/press-trigger-toggle.md Outdated
'@astryxdesign/core': patch
---

[feat] Tooltip and HoverCard (and `useTooltip`, `useHoverCard`) take `pressTrigger` (types `TooltipPressTrigger`, `HoverCardPressTrigger`): `none` (the default, unchanged) or `toggle`. With `toggle` on a button trigger, a click, a tap, Enter, or Space pins the layer open and the same press closes it, while hover and focus still preview it. A pinned layer 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. The trigger reports `aria-expanded` and `aria-controls`, and the layer is announced once when it pins, inside a modal too (`component:Tooltip` FR9 and FR10, `component:HoverCard` FR9 and FR10). A trigger that is not a button gets a development warning and keeps tap-only opening; beside `pressTrigger="toggle"`, `touchTrigger` is ignored. `touchTrigger="tap"` is deprecated in favor of `pressTrigger="toggle"` in the next patch (DEP-0016).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last sentence announces the DEP-0016 deprecation, which now ships in the stacked follow-up. Could it move to that follow-up's changeset so this release note describes only what this change does?

@cixzhang cixzhang added the ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) label Nov 1, 2026
@cixzhang
cixzhang marked this pull request as ready for review November 1, 2026 03:13
@vjeux
vjeux force-pushed the feat/tooltip-press-trigger branch from c543d57 to 90ae917 Compare November 1, 2026 05:18
@vjeux

vjeux commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in 90ae917, per the spec owners' ruling:

  • A layer's rendered text is announced through useAnnounceRenderedText (spec:AST-086 FR12), with no raw textContent and no Effect of its own. A labelled HoverCard keeps FR10's label: a string, said once from the press handler through useAnnounce (FR14), again with no Effect.
  • A press on a pinned layer's own text keeps it. Focus that moves to no element is left to the outside-press dismissal; only focus landing on an element outside the trigger and the layer closes the pin. Chromium cases cover a click on a pinned tooltip's and card's own text.
  • touchDismissRef is assigned during render, and the copy Effects are gone.
  • A pressTrigger that changes after mount re-wires the trigger, with a rerender test.
  • The trigger stays in a ref, with no DOM node in state, and stopPressPinClick is shared.
  • Each case the HoverCard verification row names now has a HoverCard test. An unlabelled card's paragraphs come out apart ("Ada Lovelace Analyst, Payments"), asserted in jsdom and in Chromium.
  • LayerPressTrigger and armOutsideDismiss stay internal: neither is exported from the Layer entry point, and the public useTouchTrigger return is unchanged (useTouchTriggerInternal carries the arm).

Kept, per the spec owners' ruling: the changeset says tap is deprecated in the next patch, since #9246, which carries the deprecation in its own changeset, merges separately. The Tooltip.tsx:38 exports comment was already fixed in c543d57.

@github-actions github-actions Bot added the needs:design-review Affects visuals — Design should review label Nov 1, 2026

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested: the toggle now behaves as component:Tooltip and component:HoverCard FR9 and FR10 describe, and the earlier findings are fixed, but three items remain.

  • The touch-trigger bridge (touchDismissRef, hideFromTouch, the usePressPin call and the touchTrigger override) is copied line for line into useTooltip.tsx:375-439 and useHoverCard.tsx:384-448; moving it into usePressPin, so each hook makes one call, keeps the two layers from drifting apart.
  • At 2,218 authored lines the change is over the 2,000-line threshold; the HoverCard wiring (about 600 lines) could land as a follow-up, or the description could show why the two are inseparable.
  • The changeset's last sentence announces DEP-0016, which ships in #9246, so it belongs in that pull request's changeset.

[Automated review]

Comment on lines +384 to +448
// A press pins the card under `pressTrigger="toggle"` (FR9, FR10). The pin
// arms and drops the touch trigger's outside dismissal, the one owner of a
// hover layer's dismissal by a press outside. That hook takes the pin's
// close as part of its hide, so the pin reaches it through a ref, assigned
// during render below and read only from handlers.
const touchDismissRef = useRef<{arm: () => void; disarm: () => void} | null>(
null,
);
const armTouchDismiss = useCallback(() => touchDismissRef.current?.arm(), []);
const disarmTouchDismiss = useCallback(
() => touchDismissRef.current?.disarm(),
[],
);

const {
isToggleTrigger,
isPinnedRef: isPressPinnedRef,
attach: attachPressPin,
unpin: unpinPress,
closeIfPinned: closePressPin,
handleFocusLeave: handlePressFocusLeave,
isPinShown,
} = usePressPin({
pressTrigger,
isEnabled,
isOpen,
isLayerOpen,
layerId,
component: 'HoverCard',
show: showNow,
hide: hideNow,
report: next => (next ? onShow?.() : onHide?.()),
label,
armOutsideDismiss: armTouchDismiss,
disarmOutsideDismiss: disarmTouchDismiss,
});

// An outside press or scroll closes a pinned layer through the pin, so the
// pin drops with it; otherwise it hides as a tap-open does.
const hideFromTouch = useCallback(() => {
if (closePressPin()) {
return;
}
hideNow();
}, [closePressPin, hideNow]);

const {
isTouchPointerRef,
isTouchInteraction,
handlePointerEnter,
handlePointerDown: handleTouchPointerDown,
clearTapOpen,
} = useTouchTrigger({
touchTrigger,
armOutsideDismiss,
} = useTouchTriggerInternal({
// Under the toggle, a trigger no press can pin keeps tap-only opening.
touchTrigger: pressTrigger === 'toggle' ? 'tap' : touchTrigger,
isEnabled,
isControlled: isOpen !== undefined,
isOpen: isLayerOpen,
layerId: layerId,
triggerRef,
show: showNow,
hide: hideNow,
hide: hideFromTouch,
});
touchDismissRef.current = {arm: armOutsideDismiss, disarm: clearTapOpen};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block is the same as useTooltip.tsx:375-439 apart from the component name and one option: the touchDismissRef bridge, hideFromTouch, the usePressPin call and the touchTrigger override. A fix to how a pin closes on an outside press would then have to land twice. Could usePressPin (or one Layer hook) call useTouchTriggerInternal itself and hand back the touch handles, so each hook makes one call?

Comment thread .changeset/press-trigger-toggle.md Outdated
'@astryxdesign/core': patch
---

[feat] Tooltip and HoverCard (and `useTooltip`, `useHoverCard`) take `pressTrigger` (types `TooltipPressTrigger`, `HoverCardPressTrigger`): `none` (the default, unchanged) or `toggle`. With `toggle` on a button trigger, a click, a tap, Enter, or Space pins the layer open and the same press closes it, while hover and focus still preview it. A pinned layer 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. The trigger reports `aria-expanded` and `aria-controls`, and the layer is announced once when it pins, inside a modal too (`component:Tooltip` FR9 and FR10, `component:HoverCard` FR9 and FR10). A trigger that is not a button gets a development warning and keeps tap-only opening; beside `pressTrigger="toggle"`, `touchTrigger` is ignored. `touchTrigger="tap"` is deprecated in favor of `pressTrigger="toggle"` in the next patch (DEP-0016).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last sentence announces DEP-0016, which ships in #9246 with its own changeset. If both land together the notes say it twice, and if #9246 slips they promise a deprecation that has not happened. Could it move to #9246?

@vjeux
vjeux force-pushed the feat/tooltip-press-trigger branch from 90ae917 to 40c79f0 Compare November 1, 2026 06:30
@vjeux

vjeux commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in 40c79f0:

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested: the toggle behaves as component:Tooltip and component:HoverCard FR9 and FR10 describe, and the earlier touch-bridge and changeset points are fixed, but three items remain.

  • Switching pressTrigger from toggle to none while the layer is pinned leaves the pin set, so that tooltip or card never closes on pointer or focus leave again (usePressPin.ts:421-425 with each host's scheduleHide); having scheduleHide ask the pin's own isHeldOpen(), plus a rerender test that leaves after the switch, would cover it.
  • At 2,348 authored lines the change is over the 2,000-line threshold, and the HoverCard wiring (about 580 lines under HoverCard FR9, FR10 and AR3) could land as a stacked pull request ahead of #9246.
  • The unmount Effect at usePressPin.ts:535 repeats the cleanup attach(null) already runs on every host and names no external system, so it can be removed.

[Automated review]

Comment on lines +421 to +425
unwire();
const current = optionsRef.current;
if (element == null || current.pressTrigger !== 'toggle') {
return;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When pressTrigger turns from toggle to none while the layer is pinned, this unwires the trigger but leaves isPinnedRef true, and scheduleHide in both hosts (useTooltip.tsx:437, useHoverCard.tsx:437) still returns early on it. The tooltip or card then never closes on pointer or focus leave again, even after a trigger press hides it and a new hover preview opens it, until Escape (reproduced in jsdom for both). Could scheduleHide ask the pin's own isHeldOpen() (already false once pressTrigger is not toggle), or could the pin drop when the toggle unwires?

Comment thread packages/core/src/Layer/usePressPin.ts Outdated
writeAttribute(trigger, 'aria-controls', isLayerOpen ? layerId : null);
});

useEffect(() => unwire, [unwire]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cleanup repeats what each host's callback ref already does on detach: attach(null) runs wire(null), which unwires (useTooltip.tsx:592, useHoverCard.tsx:630, and the wrappers' ref(null) cleanups in Tooltip.tsx and HoverCard.tsx). It also names no external system (architecture:react-component-runtime INV3, INV5). Could it be removed?

@vjeux
vjeux force-pushed the feat/tooltip-press-trigger branch from 40c79f0 to fc05aff Compare November 1, 2026 09:06
@vjeux vjeux changed the title feat(Tooltip, HoverCard): pressTrigger="toggle" pins the layer on a press feat(Tooltip): pressTrigger="toggle" pins the tooltip on a press Nov 1, 2026
@vjeux

vjeux commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in fc05aff:

  • useTooltip's scheduleHide asks the pin's own isHeldOpen(), which is false once pressTrigger isn't toggle, and a pin counts only while its layer shows. A rerender test pins the tooltip, switches pressTrigger to none and leaves; it fails on 40c79f0.
  • The unmount Effect is gone: each host's attach(null) already unwires.
  • The pressTrigger re-wire no longer repeats on mount: the pin skips an element and mode it already settled, and a non-button trigger warns once (it warned four times across a mount and two re-renders on 40c79f0).
  • FR9 keeps an implementation-gap note for its own-text clause, which no component reaches yet (raised on feat(Tooltip, HoverCard, cli): deprecate touchTrigger="tap" (DEP-0016), with its upgrade codemod #9246).
  • Split, by the maintainers' ruling: the HoverCard wiring (component:HoverCard FR9, FR10 and AR3) is its own pull request (branch feat/hover-card-press-trigger), stacked between this one and feat(Tooltip, HoverCard, cli): deprecate touchTrigger="tap" (DEP-0016), with its upgrade codemod #9246, and all three ship in the same release. HoverCard's record keeps FR9 and FR10 marked not yet built here.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested: pressTrigger="toggle" now behaves as component:Tooltip FR9 and FR10 describe for a child trigger, and the earlier stale-pin, unmount-Effect and size points are fixed, but two FR9 gaps remain. Please fix both and add a test for each.

  • With anchorRef, the pinning click still reaches a plain onClick row around the anchored button, because only the child wrapper stops it (Tooltip.tsx:263-312).
  • With focusTrigger="never", focus moving to another control never closes a pinned tooltip, because the close rides on focus listeners that option skips (useTooltip.tsx:581-588); the pin listening for focusout on the trigger it wires would cover it.

[Automated review]

<div ref={wrapperRef} {...stylex.props(styles.wrapperContents)}>
<div
ref={wrapperRef}
onClick={stopPressPinClick}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only this wrapper (and the inline span's) stops a pinning press. The anchorRef path above (lines 263-312) wires an external element with no wrapper, so with pressTrigger="toggle" a click on an anchored button inside a plain <div onClick> both pins the tooltip and fires the row (reproduced in jsdom: one row call; zero with a child trigger). FR9 says the press "does not reach a clickable row, card, or container around it". Stop the marked click for anchorRef triggers too, or record the case as an FR9 gap beside the hook caveat, with a test.

const handleFocusOut = useCallback(
(event: FocusEvent) => {
// A pinned tooltip closes when focus leaves the trigger and the layer.
if (handlePressFocusLeave(event)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the pin's only focus-leave close, and handleFocusOut is attached only when focusTrigger is always or auto on a focusable trigger (lines 581-588). Under focusTrigger="never", a pinned tooltip stays open with aria-expanded="true" after Tab moves focus away (reproduced in jsdom), against FR9's "focus moving outside them closes it". Having usePressPin listen for focusout on the trigger it wires, whatever focusTrigger is, would cover it; please add a focusTrigger="never" case.

`component:Tooltip` FR9 and FR10, `spec:AST-003` FR4:

- `pressTrigger` (`none`, the default, or `toggle`) on Tooltip and
  useTooltip, with the type TooltipPressTrigger. With `toggle` on a button
  trigger, a click, a tap, Enter, or Space pins the tooltip and the same
  press closes it; hover and focus still preview it.
- A pinned tooltip stays until Escape, a second press, a press or scroll
  outside (the touch trigger's outside dismissal, which the pin arms: the pin
  owns the layer's touch trigger, so useTooltip makes one usePressPin call),
  or focus moving onto an element outside the trigger and the tooltip (the
  pin listens for it on the trigger, whatever `focusTrigger` is); a press on
  the tooltip's own text keeps it. It takes Escape before the layer beneath
  it. A `pressTrigger` that changes after mount wires or unwires the
  trigger, and a tooltip whose `pressTrigger` turns to `none` while pinned
  closes on leave again.
- The press stops at its trigger. With `anchorRef`, the caller's own
  handlers still receive the press: the anchor's `onClick` runs, and a plain
  `onClick` element around the anchor receives it unless the anchor's
  `onClick` stops it; Astryx clickable rows, cards, and containers ignore it
  (FR9). The trigger carries aria-expanded and aria-controls on a button
  trigger, and the tooltip's rendered text is announced once when it pins,
  through useAnnounceRenderedText (`spec:AST-086` FR12).
- A non-button trigger gets one development warning and keeps tap-only
  opening; a trigger whose tooltip repeats its own text pins without toggle
  state (`Layer/usePressPin.ts`, the hover layers' shared press pin).
- LayerPressTrigger and the touch trigger's armOutsideDismiss
  (useTouchTriggerInternal) stay internal: neither is exported from the Layer
  entry point, and the public useTouchTrigger return is unchanged.
@vjeux
vjeux force-pushed the feat/tooltip-press-trigger branch from fc05aff to 26d7af9 Compare November 1, 2026 13:12
@vjeux

vjeux commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in 26d7af9, per the maintainers' ruling:

  • anchorRef: by the spec owners' ruling, which settles FR9's stop at the trigger in favor of DEC-1 (a tap that already acts stays with its control), the caller's own handlers still receive the press. The anchor's onClick runs, and Astryx clickable rows, cards and containers ignore it. A plain onClick element around the anchor receives it unless the anchor's onClick stops it. FR9 now says so, and its verification row names the cases. Tests cover all three: an action anchor's onClick fires; an Astryx ClickableCard, Item row and click-expanding Table row don't activate; a plain row receives the press unless the anchor stops it. The changeset and docs state it too.
  • focusTrigger="never": the pin listens for focusout on the trigger it wires, whatever focus listeners the host attaches, so a pinned tooltip closes when focus moves to another control. It has unit and Chromium tests, both failing on fc05aff.

HoverCard has no anchorRef; its focusTrigger="never" case is in the stacked #9291.

This branch was successfully deployed

1 active deployment
Preview — 26d7af94 Deployed Nov 1, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) CLA Signed This label is managed by the Meta Open Source bot. needs:design-review Affects visuals — Design should review needs:spec-owner-review Current knowledge records await owner approval

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants