Repository navigation
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC |
|---|---|---|
| lab/CommentThread | 317.3 kB (+92 B) · 2.4 kB CSS (0) | — |
| lab/Comments | 322.2 kB (+85 B) · 2.5 kB CSS (0) | — |
| lab/Schedule | 103.0 kB (+66 B) · 3.9 kB CSS (0) | — |
5 more components
| Component | Size | LoC |
|---|---|---|
| ButtonGroup | 40.6 kB (+62 B) · 3.7 kB CSS (0) | — |
| AlertDialog | 103.1 kB (+58 B) · 14.2 kB CSS (0) | — |
| lab/NavigationStack | 79.0 kB (+18 B) · 1.5 kB CSS (0) | — |
| lab/NavigationScreen | 79.0 kB (+17 B) · 1.5 kB CSS (0) | — |
| lab/PageControl | 72.3 kB (+13 B) · 709 B CSS (0) | — |
Package size: @astryxdesign/core 6.89 MB packed (+819 B); no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 19 rows, core 25 rows; run pnpm api:report.
Not run on this PR: add ci:run-visual, ci:run-a11y or ci:run-rtl to run them.
Generated by PR Enrichment workflow | View full report
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 indocs/architecture/react-component-runtime.mdallows only once; extendutils/clickGuard.tsto cover the compatibilitymousedownandmouseupand call it instead. .changeset/core-internal-entries-lab-tabbar.mdsays 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]
b86bd24 to
b5b0aaa
Compare
|
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 |
There was a problem hiding this comment.
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 indocs/architecture/react-component-runtime.mdallows only once; extendutils/clickGuard.tsto cover the compatibilitymousedownandmouseupand call it instead. - SearchField's resize handling in the same file creates its own
ResizeObserverper field instead of callingobserveResizefrom@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]
b5b0aaa to
62b989d
Compare
There was a problem hiding this comment.
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 discoverentry, 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]
62b989d to
53c7f49
Compare
There was a problem hiding this comment.
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]
53c7f49 to
918491f
Compare
|
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 |
There was a problem hiding this comment.
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.mdis 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]
918491f to
a88504b
Compare
|
Changes in this round:
|
|
/approve-spec a88504b |
|
/approve-spec 9fc9bde |
# Conflicts: # packages/core/src/internal/touch.ts
|
/approve-spec a236965 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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]
|
/approve-spec 92659fe |
# Conflicts: # packages/core/src/internal/touch.ts
There was a problem hiding this comment.
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]
|
/approve-spec 5a5ebab |
|
/approve-spec d24b6d4 |
There was a problem hiding this comment.
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 Core's internal touch 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]
|
/approve-spec 82971fa |
There was a problem hiding this comment.
Approved pending CI: CI is still running on this head.
The new paintsHighlight option and the four press callbacks extend the shared action-group tracking hook instead of copying it. The re-export stays on Core's internal touch entry, outside the public API. AlertDialog and ButtonGroup pass none of the new options, so they behave as before. The five new cases fail on the base and pass here.
Two optional notes from the earlier review still apply:
- Mention unmounting in
onPressCancel's doc, since the cleanup also reports a press dropped at unmount. - Add
useActionGroupTrackingtoLAB_ONLY_NAMESinscripts/touch-support-bundle.test.mjs, as the entry's sync note asks.
#9700 does not contain this head yet; merge this branch into it to keep the stack intact.
[Automated review]
|
/approve-spec ae94e02 |
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
useActionGroupTrackinggains options for a group that draws its own highlight.paintsHighlight: falsewrites no press attribute. Press callbacks report the press:onPressBeginandonPressMovewith the action under it,onPressEndbefore the lift's action runs, andonPressCancelwhen it ends with nothing acting: a cancel, a second finger, or tracking turned off mid-press.@astryxdesign/core/internal/touchre-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-groupsFR3) and focus follows the lift (FR1), as in AlertDialog.packages/core/locales/en.jsongains@astryx.tabBar.badgedItem, a badged tab's name.[chore]) names the re-export, the options and the string.Evidence
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).Scope
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: #9699 → #9700
Merge group: #9699, #9700
TabBar's second Lab batch, #9701, also builds on #9700.