Skip to content

chore(core): Core's internal touch entry re-exports the action-group tracking hook, extended for Lab's TabBar, and a tab-bar string joins the catalog - #9699

Open
vjeux wants to merge 11 commits into
meta:mainfrom
vjeux:tabbar/core-touch-entry
Open

vjeux wants to merge 11 commits into
meta:mainfrom
vjeux:tabbar/core-touch-entry

Conversation

@vjeux

@vjeux vjeux commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #9754 (Core's internal touch entry and Lab's motion-engine hook). Review only this PR's change: vjeux/astryx@f3e826a...7a64fbf

Maintainer impact

Lab's TabBar lands as two Lab-only batches, drafts that follow this one. Its press needs Core's action-group tracking hook, which Lab can't reach or extend itself, and its badged tabs need one catalog string. A Lab-only batch carries no Core code, so those parts land here first, on their own.

Intended invariant

No behavior change for today's caller: the hook's new options are optional, and AlertDialog passes none. Core's internal touch entry stays internal: named re-exports only, outside the public API, and limited to Core and Lab by @astryx/no-core-internal-import. No Core module reads the new catalog key.

Change

  • useActionGroupTracking gains options for a group that draws its own highlight. paintsHighlight: false writes no press attribute. Press callbacks report the press: onPressBegin and onPressMove with the action under it, onPressEnd before the lift's action runs, and onPressCancel when it ends with nothing acting: a cancel, a second finger, or tracking turned off mid-press.
  • @astryxdesign/core/internal/touch re-exports the hook and its options type. TabBar's press goes through it, so a second finger ends a press with nothing chosen (family:action-groups FR3) and focus follows the lift (FR1), as in AlertDialog.
  • packages/core/locales/en.json gains @astryx.tabBar.badgedItem, a badged tab's name.
  • A Core changeset ([chore]) names the re-export, the options and the string.

Evidence

  • Failure before the change: the hook's four new cases fail on the current hook (the press attribute is painted, expected 'on' to be null; no callback runs, also when tracking turns off mid-press). The entry's new case fails on the current entry (useActionGroupTracking: expected undefined).
  • Success after the change: they pass, and the hook's existing cases pass unchanged.
  • Product/runtime behavior verified unchanged: AlertDialog's unit and interaction tests pass at this head.

Scope

  • No intended public API, product behavior, visual direction, or policy change.
  • Product changes discovered during the work were removed or split.
  • Public text and artifacts contain no internal Meta context.

Testing

Local gates at 7a64fbf, on #9754's f3e826a, node 24.22.1 from .nvmrc. Rebased from aa755bc with no change to this PR's own lines; the one conflict, in the internal entry's export list, is resolved by adding the hook's two lines after #9754's new sections. Core typecheck; 146 unit tests (the hook's 30 cases, the entry, AlertDialog, the shared tracker, the bundle test); pnpm lint, 0 errors; review:pre. On #9700's build: AlertDialog's interaction spec, 12/12 in each engine. Red-first at the previous head: the hook's four new cases on the current hook, and the entry's new case.

Stack: #9754 → #9699 → #9700
Merge group: #9699, #9700

TabBar's second Lab batch, #9701, also builds on #9700.

@vercel

vercel Bot commented Nov 3, 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 6, 2026 11:50am UTC

Request Review

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

astracat-bot Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

AI review status for this pull request.

Review status Updated
🟢 Reviewed (for maintainers only) Nov 6, 2026, 11:57 AM UTC

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

🔎 Preview · 5a5ebab

Storybook · Sandbox · built from this PR's head

Modified components

Component Size LoC
lab/CommentThread 296.2 kB (+94 B) · 2.4 kB CSS (0) —
lab/Comments 301.8 kB (+72 B) · 2.5 kB CSS (0) —
lab/Schedule 99.3 kB (+64 B) · 3.9 kB CSS (0) —
5 more components
Component Size LoC
ButtonGroup 40.1 kB (+60 B) · 3.7 kB CSS (0) —
AlertDialog 95.8 kB (+59 B) · 13.7 kB CSS (0) —
lab/PageControl 71.7 kB (+15 B) · 709 B CSS (0) —
lab/NavigationScreen 78.4 kB (+10 B) · 1.5 kB CSS (0) —
lab/NavigationStack 78.4 kB (+10 B) · 1.5 kB CSS (0) —

Package size: @astryxdesign/core 6.75 MB packed (+897 B); no new dependencies.

API changes: none in this pull request

Committed API report behind this build: cli 19 rows, core 24 rows; run pnpm api:report.

Not run on this PR: add ci:run-visual, ci:run-a11y or ci:run-rtl to run them.

Current run: CI concluded cancelled; parts of this report may be missing.


Generated by PR Enrichment workflow | View full report

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes. The Core part itself, seven named re-exports on the internal touch entry and three catalog strings, follows the current internal-entry and catalog rules, and its tests pass. What blocks is what the diff against main carries.

  • The diff includes #9399's two commits, which make it 7,261 changed lines (51 of them this pull request's own); land #9399 first and rebase, so this pull request contains only its own commit.
  • SearchField's armSwallow (packages/lab/src/SearchField/SearchFieldTouch.ts) adds its own document listeners to swallow a claimed tap's late mouse events, a second stray-click swallower beside Core's click guard, which INV13 in docs/architecture/react-component-runtime.md allows only once; extend utils/clickGuard.ts to cover the compatibility mousedown and mouseup and call it instead.
  • .changeset/core-internal-entries-lab-tabbar.md says Lab's TabBar reaches these modules and that TabBar's strings join the catalog, but no TabBar exists at this head; describe the re-exports and the strings themselves.

Two non-blocking comment suggestions are inline.

[Automated review]

'@astryxdesign/core': patch
---

[experimental] Lab's experimental TabBar reaches Core's pan recognizer, press tracker, finger-scroll reader, selection lock, touch query, rubber band and UIKit gesture values through Core's internal `@astryxdesign/core/internal/touch` entry. That entry stays outside the public API and its compatibility promise. TabBar's strings join Core's message catalog.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No TabBar exists at this head (it arrives in #9700 and #9701), so this release note announces a consumer the release may not contain. Suggested wording:

[experimental] Core's internal `@astryxdesign/core/internal/touch` entry re-exports the pan recognizer, press tracker, finger-scroll reader, selection lock, touch query, rubber band and UIKit gesture values for Lab components. Three tab-bar strings join Core's message catalog.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in b5b0aaa with your wording: the note describes the entry's re-exports and the three strings, and names no consumer.

Comment on lines +506 to +547
const armSwallow = useCallback((doc: Document) => {
disarmRef.current?.();
let sawGuard = false;
const swallow = (event: Event) => {
if (event.type === 'click' && (event as MouseEvent).detail === 0) {
return;
}
if (isClickGuardArmed(doc)) {
sawGuard = true;
} else if (sawGuard) {
disarm();
return;
}
event.preventDefault();
event.stopPropagation();
if (event.type === 'click') {
disarm();
}
};
const types = ['mousedown', 'mouseup', 'click'] as const;
const ends = ['touchstart', 'pointerdown', 'keydown'] as const;
const disarm = () => {
for (const type of types) {
doc.removeEventListener(type, swallow, true);
}
for (const type of ends) {
doc.removeEventListener(type, disarm, true);
}
if (disarmRef.current === disarm) {
disarmRef.current = null;
}
};
for (const type of types) {
doc.addEventListener(type, swallow, true);
}
for (const type of ends) {
doc.addEventListener(type, disarm, true);
}
disarmRef.current = disarm;
}, []);
// Unmount releases a pending swallow's document listeners.
useEffect(() => () => disarmRef.current?.(), []);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These document listeners are a second stray-click swallower beside Core's click guard (packages/core/src/utils/clickGuard.ts), and INV13 in docs/architecture/react-component-runtime.md allows only one. Extending the click guard to cover a claimed tap's compatibility mousedown and mouseup, and arming it from this tap through the internal entry, would let armSwallow, disarmRef and the unmount Effect go.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This swallower is #9399's code (SearchField), which this branch carries only because it is stacked on #9399. #9623 extends Core's click guard to cover the compatibility mouse events, and armSwallow goes there. This PR's own commit adds no listener.

Comment thread packages/core/src/internal/touch.ts Outdated
Comment on lines +5 to +9
* @input The gesture core: the arena, the recognizers and their geometry,
* React hook, press tracker, finger-scroll reader and text-selection lock.
* The animator, the cubic-bézier solver, the rubber band and UIKit's gesture
* and timing tables. The touch-presentation hook and its query, the
* writing-direction read, and whether the click guard is armed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking: INV29 keeps a file header to about 12 lines, and this one is now 23. A shorter @input:

 * @input Core's gesture, motion and touch modules that Lab components use

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Taken in b5b0aaa: @input is now your one line, and the header is down from 23 to 19 lines. The rest is the @position block's record citations, which stay until a record links the file.

Comment on lines +468 to +488
// A tap on the text of a field at the bottom of the screen (`bottom`,
// `tab`) focuses it without the browser's reveal (`preventScroll`). The
// browser would otherwise pan the whole page up to show a field the
// keyboard is about to cover. The tap is the gesture system's: recognizing
// it cancels the browser's own focus for that touch. Taps on the buttons,
// and taps once the text has focus, stay the browser's.
// The claim applies only where the keyboard covers the page, Chrome on
// Android included. There the browser's own tap scrolls the document once
// its keyboard is up. The claimed tap's focus brings the keyboard up and
// pans nothing (measured on the Android emulator).
// The recognizer attaches only where it claims: any other field keeps the
// browser's own touch-action and tap.
// It takes a finger and a pen, because Safari reveals a field for both. A
// mouse press stays the browser's, which places the caret and selects text
// where it lands. The claim is touch work, so it attaches only where the
// primary pointer is coarse. Under a fine one, as in desktop Safari or
// Firefox, no software keyboard rises. There a mouse or a pen tablet keeps
// the browser's own focus and click.
// The keyboard check reads the window that the touch presentation reads.
// `isTouch` is false on the server and while hydrating, so no markup
// differs.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking: this comment covers seven topics in 21 lines (INV29: one topic per sentence). A shorter version that keeps the reasons:

// A tap on the text focuses the field with preventScroll. The browser's own
// focus would pan the page under the keyboard. The claim attaches only under
// a coarse pointer where the keyboard covers the page. It takes touch and pen.
// A mouse keeps the browser's focus and caret.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This comment is #9399's (SearchField), carried here only by the stack. I've passed your shorter version to that PR's thread rather than change its code here.

@vjeux
vjeux force-pushed the tabbar/core-touch-entry branch from b86bd24 to b5b0aaa Compare November 3, 2026 05:08
@vjeux

vjeux commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

On the stack: this PR's own commit is the 51-line change at vjeux/astryx@1748182...b5b0aaa. The rest of the diff is #9399, which this branch sits on because both change internal/touch.ts. Putting it straight on main would make #9399, which is ready, conflict. It rebases onto main once #9399 lands, and then the diff is just its own commit.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes. The Core part itself, seven named re-exports on the internal touch entry and three catalog strings, follows the internal-entry and catalog rules, its tests pass, and the TabBar changeset now describes exactly what the head contains. What blocks is what the diff against main carries from #9399.

  • The diff includes #9399's two commits, which make it 7,257 changed lines (47 of them this pull request's own); land #9399 first and rebase, so this pull request contains only its own commit.
  • SearchField's armSwallow (packages/lab/src/SearchField/SearchFieldTouch.ts) adds its own document listeners to swallow a claimed tap's late mouse events, a second stray-click swallower beside Core's click guard, which INV13 in docs/architecture/react-component-runtime.md allows only once; extend utils/clickGuard.ts to cover the compatibility mousedown and mouseup and call it instead.
  • SearchField's resize handling in the same file creates its own ResizeObserver per field instead of calling observeResize from @astryxdesign/core/utils; subscribe through the shared observer, and extend it if the field's own window is the reason.

Both SearchField items live in #9399's code and leave this diff with the rebase.

[Automated review]

Comment on lines +606 to +624
useLayoutEffect(() => {
const field = fieldRef.current;
const Observer = field?.ownerDocument.defaultView?.ResizeObserver;
// The tab bar lays out and moves a tab field's box; it has no parts.
if (field == null || Observer == null || placement === 'tab') {
return;
}
const observer = new Observer(() => {
if (isAnimating.current || settledRef.current === null) {
return;
}
if (isBottom) {
placeBottom(bottomPoseFor(settledRef.current, bottomShape().width));
} else {
place(poseFor(settledRef.current, geometry()));
}
});
observer.observe(field);
return () => observer.disconnect();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This creates one ResizeObserver per field. observeResize in @astryxdesign/core/utils (packages/core/src/utils/sharedResizeObserver.ts) already owns resize observation, and SearchField.tsx already imports from that entry. Subscribing with observeResize(field, …) and returning its unsubscribe would drop the per-instance observer; if the field's own window is the reason, extending observeResize for it keeps one path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is #9399's code too (SearchField), carried here only by the stack, and I've passed it to that PR's author. This PR's own commit has no observer.

@vjeux
vjeux force-pushed the tabbar/core-touch-entry branch from b5b0aaa to 62b989d Compare November 3, 2026 07:42
@vjeux

vjeux commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Restacked on #9399's ee42864, so this diff now holds only this PR's commit (+73/−3). For TabBar's round 1 it also re-exports Core's action-group tracker and widens useFingerScroll to take a ref that holds a document (type only). Both SearchField points left with #9399's new head.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes. The Core part of this pull request (eight named re-exports on the internal touch entry, the widened useFingerScroll type and three catalog strings) follows the internal-entry and catalog rules, and its tests pass. What blocks is what the diff against main still carries from #9399.

  • The diff includes #9399's two commits, which make it 7,416 changed lines (76 of them this pull request's own); land #9399 first and rebase, so this pull request contains only its own commit.
  • Those commits bring Lab's SearchField with its strings, astryx discover entry, stories and tests, which this pull request's title and body do not cover and which belong in #9399, so they need to leave this diff.

The two earlier SearchField findings (the second stray-click swallower and the per-field ResizeObserver) are fixed at this head. Two non-blocking notes are inline.

[Automated review]

* the document (or a ref that holds it) for the page's own scroll. Returns a
* reader, stable across renders: `true` while the scroller's scroll is a
* finger's.
* Internal: not exported from the package.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking: this commit re-exports the hook from @astryxdesign/core/internal/touch, so "not exported from the package" no longer holds. Suggested:

 * Internal: outside Core, only `@astryxdesign/core/internal/touch` reaches it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I'll take your wording in the push that rebases this onto main once #9399 lands, so it rides with the change this round can't make yet.

return {duration, at, velocity: t => slope(at, t)};
}

/** A curve times a constant (a direction). */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking (INV29, #9399's code): this JSDoc restates scaled's one-line body. Dropping it loses nothing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is #9399's code, carried here only by the stack. I've passed it to that PR's author.

@vjeux

vjeux commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

The remaining blocker is the stack. #9399's commits leave this diff once #9399 lands and this branch rebases onto main; until then the compare link at the top of the body shows this PR's own 76 lines. The useFingerScroll comment fix rides with that rebase.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes. This pull request's own commit (eight named re-exports on the internal touch entry, the widened useFingerScroll type and three catalog strings) follows the internal-entry and catalog rules, and its new test and type cases fail before the change and pass after it. What blocks is what the diff against main still carries from #9399.

  • The diff includes #9399's two commits, 7,371 of its 7,449 changed lines, and this copy is already behind #9399's own head; land #9399 first and rebase onto main, so this pull request holds only its own 78-line commit.
  • Lab's SearchField, its stories, strings, discover entry and registry route belong to #9399 and have no place in a pull request about the touch entry's re-exports; they leave with that rebase.

Non-blocking, for #9399: the local translate(...) helper in SearchFieldBottom.interaction.browser.spec.ts makes the i18n catalog check list nine unverifiable key references; renaming it removes the noise.

[Automated review]

@vjeux

vjeux commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Stack only, as before: #9399's commits leave this diff when #9399 lands and this branch rebases onto main. Until then the body's compare link shows this PR's own 78-line commit. The SearchField note is passed to #9399's author.

@vjeux

vjeux commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Restacked on #9754 instead of #9399, since #9399 won't merge. This PR's own commit is now only what TabBar adds to Core's internal touch entry: the press tracker, the action-group tracker, the finger-scroll reader and the selection lock, plus the widened useFingerScroll type and the three catalog strings. The names other Lab components share come from #9754, so nothing is exported twice. The compare link at the top of the body shows this PR's 66 lines.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes for one small fix. The new internal re-exports, the widened useFingerScroll type and the three catalog strings follow the internal-entry and catalog rules.

  • .changeset/core-internal-entries-lab-tabbar.md is tagged [experimental], which lists this internal-entry change under "Experimental APIs" in Core's release notes, while AST-017 FR21 and FR22 keep ./internal/* outside experimental status; please retag it [chore], as the stacked changeset does.

Non-blocking: the @output header in packages/core/src/internal/touch.ts and the @input header in packages/core/src/gestures/useFingerScroll.ts no longer describe everything they now cover.

[Automated review]

'@astryxdesign/core': patch
---

[experimental] Core's internal `@astryxdesign/core/internal/touch` entry re-exports the press tracker, action-group tracker, finger-scroll reader and selection lock for Lab components. The finger-scroll reader also takes a ref that holds a document. Three tab-bar strings join Core's message catalog.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[experimental] files this under "Experimental APIs" in Core's release notes, but an ./internal/* entry is outside experimental status (AST-017 FR21 and FR22). Please retag it [chore], as core-internal-touch-entry-lab-animator.md does; the patch bump stays.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Retagged [chore], as #9754's changeset is; the patch bump stays.

@vjeux
vjeux force-pushed the tabbar/core-touch-entry branch from 918491f to a88504b Compare November 3, 2026 18:36
@vjeux

vjeux commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Changes in this round:

@cixzhang

cixzhang commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

/approve-spec a88504b

@cixzhang

cixzhang commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 4bbb9ca

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved pending CI: CI has not run on this head yet because the pull request is a draft, and it runs once the pull request is marked ready for review.

The hook's new paintsHighlight option and press callbacks, their re-export from Core's internal touch entry and the badged-tab string follow the action-group, internal-entry and catalog rules, and AlertDialog, the hook's only Core caller, is unchanged. #9700 still sits on this branch's previous head, so it needs a restack before the two line up.

  • Miss: (non-blocking) onPressCancel's doc comment can also name the group unmounting mid-press, which the cleanup reports as a cancel (see the inline note).
  • Miss: (non-blocking) LAB_ONLY_NAMES in scripts/touch-support-bundle.test.mjs can list useActionGroupTracking, as the entry's sync note asks (see the inline note).

[Automated review]

Comment on lines +65 to +66
* The press ended with nothing acting: a cancel, a second finger, or
* tracking turned off.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking: the Effect cleanup (lines 132-139) also calls onPressCancel when the group unmounts mid-press. Naming that case keeps a caller from assuming it never runs during its own unmount:

   * The press ended with nothing acting: a cancel, a second finger,
   * tracking turned off, or the group unmounting.

export {TOUCH_QUERY} from '../utils/touchQueries';

// Lab's TabBar tracks its press with the shared action-group tracker.
export {useActionGroupTracking} from '../hooks/useActionGroupTracking';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking: the header's SYNC note also names scripts/touch-support-bundle.test.mjs. Its LAB_ONLY_NAMES list does not include this name yet, so the check that Lab's names stay off the root entry does not cover it:

  'TOUCH_QUERY',
  'useActionGroupTracking',
];

@vjeux
vjeux marked this pull request as ready for review November 5, 2026 14:29
# Conflicts:
#	packages/core/src/hooks/useActionGroupTracking.ts
@cixzhang

cixzhang commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 1f67124

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved pending CI: CI is still running on this head.

The hook's new paintsHighlight option and press callbacks, their re-export from Core's internal touch entry and the badged-tab string follow the action-group, internal-entry and catalog rules, and neither Core caller (AlertDialog, and ButtonGroup through useGroupPressTracking) passes the new options, so shipped behavior is unchanged. The branch now conflicts with main in packages/core/src/internal/touch.ts and its test, where #9696 appended its own section; a merge that keeps both sections resolves it, and #9700 still needs a restack onto this branch.

  • Miss: (non-blocking) onPressCancel's doc comment can also name the group unmounting mid-press, which the Effect cleanup reports as a cancel.
  • Miss: (non-blocking) LAB_ONLY_NAMES in scripts/touch-support-bundle.test.mjs can list useActionGroupTracking, as the internal entry's sync note asks.

[Automated review]

# Conflicts:
#	packages/core/src/internal/touch.test.ts
#	packages/core/src/internal/touch.ts
@cixzhang

cixzhang commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 4c9c002

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This pull request is reviewed as part of its stack; see the review on #9700.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved pending CI: CI is still running on this head.

The new paintsHighlight option and press callbacks extend the shared action-group tracker rather than copying it, the re-export stays on the internal entry outside the public API, and AlertDialog and ButtonGroup pass none of the new options, so they behave as before. Two earlier optional notes still apply: name unmounting in onPressCancel's doc, and add useActionGroupTracking to LAB_ONLY_NAMES in scripts/touch-support-bundle.test.mjs. #9700 does not contain this head yet; merge this branch into it to keep the stack intact.

[Automated review]

@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 9fc9bde

# Conflicts:
#	packages/core/src/internal/touch.ts
@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec a236965

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved pending CI: CI is still running on this head.

The new paintsHighlight option and press callbacks extend the shared action-group tracker rather than copying it, the re-export stays on the internal entry outside the public API, and AlertDialog and ButtonGroup pass none of the new options, so they behave as before. Two earlier optional notes still apply: name unmounting in onPressCancel's doc, and add useActionGroupTracking to LAB_ONLY_NAMES in scripts/touch-support-bundle.test.mjs. #9700 does not contain this head yet; merge this branch into it to keep the stack intact.

[Automated review]

# Conflicts:
#	packages/core/src/internal/touch.test.ts
#	packages/core/src/internal/touch.ts

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved pending CI: CI is still running on this head.

The new paintsHighlight option and press callbacks extend the shared action-group tracker rather than copying it, the re-export stays on the internal entry outside the public API, and AlertDialog and ButtonGroup pass none of the new options, so they behave as before. Two earlier optional notes still apply: name unmounting in onPressCancel's doc, and add useActionGroupTracking to LAB_ONLY_NAMES in scripts/touch-support-bundle.test.mjs. #9700 does not contain this head yet; merge this branch into it to keep the stack intact.

[Automated review]

@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 92659fe

# Conflicts:
#	packages/core/src/internal/touch.ts

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved pending CI: CI has not run its tests on this head, because the CI workflow stopped at admission with "CI is full". Re-run all jobs, or push again later, so the required checks run.

The new paintsHighlight option and press callbacks extend the shared action-group tracker rather than copying it, the re-export stays on the internal entry outside the public API, and AlertDialog and ButtonGroup pass none of the new options, so they behave as before. The latest merge of main keeps both re-export sections in the internal entry. Two earlier optional notes still apply: name unmounting in onPressCancel's doc, and add useActionGroupTracking to LAB_ONLY_NAMES in scripts/touch-support-bundle.test.mjs. #9700 does not contain this head yet; merge this branch into it to keep the stack intact.

[Automated review]

@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 5a5ebab

This branch was successfully deployed

1 active deployment
Preview — 5a5ebab2 Deployed Nov 6, 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants