Repository navigation
chore(lab): Lab's animator hook, over Core's internal touch entry - #9754
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| BottomSheet | 56.4 kB (0) · 7.5 kB CSS (0) | — | — · — · — |
| ContextMenu | 82.9 kB (0) · 16.3 kB CSS (0) | — | — · — · — |
| DateInput · Storybook | 163.5 kB (0) · 15.5 kB CSS (0) | 5,613 (0) | ✓ · ✓ · ✓ |
25 more components
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| DateTimeInput | 185.4 kB (0) · 17.2 kB CSS (0) | — | — · — · — |
| DropdownMenu | 77.8 kB (0) · 16.4 kB CSS (0) | — | — · — · — |
| Markdown | 153.6 kB (0) · 19.2 kB CSS (0) | — | — · — · — |
| MoreMenu | 78.0 kB (0) · 16.4 kB CSS (0) | — | — · — · — |
| MultiSelector | 80.2 kB (0) · 15.6 kB CSS (0) | — | — · — · — |
| Pagination | 93.1 kB (0) · 16.6 kB CSS (0) | — | — · — · — |
| Popover | 55.3 kB (0) · 12.8 kB CSS (0) | — | — · — · — |
| PowerSearch | 260.0 kB (0) · 22.6 kB CSS (0) | — | — · — · — |
| SegmentedControlItem | 12.1 kB (0) · 4.0 kB CSS (0) | — | ✓ · ✓ · ✓ |
| Selector | 83.1 kB (0) · 16.2 kB CSS (0) | — | — · — · — |
| Slider · Storybook | 92.5 kB (0) · 8.3 kB CSS (0) | 2,333 (0) | ✓ · ✓ · ✓ |
| Table | 92.3 kB (0) · 16.9 kB CSS (0) | — | — · — · — |
| TableCell | 84.1 kB (0) · 16.7 kB CSS (0) | — | — · — · — |
| TableHeaderCell | 83.6 kB (0) · 16.5 kB CSS (0) | — | — · — · — |
| TimeInput | 140.6 kB (0) · 14.5 kB CSS (0) | — | — · — · — |
| Tokenizer | 74.7 kB (0) · 17.1 kB CSS (0) | — | — · — · — |
| lab/ChatEmojiPicker | 69.0 kB (0) · 773 B CSS (0) | — | — · — · — |
| lab/ChatReactionBar | 69.5 kB (0) · 1.1 kB CSS (0) | — | — · — · — |
| lab/CommentComposer | 108.9 kB (0) · 1.9 kB CSS (0) | — | — · — · — |
| lab/CommentThread | 268.5 kB (0) · 2.4 kB CSS (0) | — | — · — · — |
| lab/TourStep | 61.2 kB (0) · 590 B CSS (0) | — | — · — · — |
| Switch · Storybook | 90.2 kB (−1 B) · 9.5 kB CSS (0) | 1,717 (0) | ✓ · ✓ · ✓ |
| lab/Comments | 273.6 kB (−1 B) · 2.5 kB CSS (0) | — | — · — · — |
| richtext/RichTextEditorToolbar | 234.0 kB (−1 B) · 454 B CSS (0) | — | — · — · — |
| SegmentedControl | 83.9 kB (−2 B) · 6.7 kB CSS (0) | — | ✓ · ✓ · ✓ |
Package size: @astryxdesign/core 6.56 MB packed (+245 B); no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 9 rows, core 4 rows; run pnpm api:report.
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
0cf6dce to
e8cb6cb
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested. The new internal-entry names and the Lab hook fit the runtime and release rules, but two things need fixing before this can merge:
packages/core/src/internal/touch.ts:48re-exportscreateAnimatorfrom../motion/animator, whichmainhas since renamed tocreateMotionAnimator, so once merged Core's typecheck and build fail (TS2305) anduseMotionAnimatorthrowscreateMotionAnimator is not a function; rebase, exportcreateMotionAnimatordirectly, and compare againstanimator.createMotionAnimatorintouch.test.ts:44.- The changeset files this internal-entry change under
[experimental], which Core's changelog lists as an Experimental API even though the entry is outside the public API and no Lab component uses these names yet; use[chore] Internal: …as earlier internal-entry changesets do.
The identity check at touch.test.ts:44 also passes when both sides are undefined, so an added typeof entry.createMotionAnimator === 'function' assertion would catch this kind of break.
[Automated review]
| export {isRtlElement} from '../hooks/isRtlElement'; | ||
| // The animator's internal factory, under a name no public factory has: the | ||
| // public animator factory is `createAnimator`. | ||
| export {createAnimator as createMotionAnimator} from '../motion/animator'; |
There was a problem hiding this comment.
main has renamed this factory to createMotionAnimator and no longer exports createAnimator from ../motion/animator, so after a merge this line fails with TS2305 and the Lab hook gets undefined. After rebasing:
export {createMotionAnimator} from '../motion/animator';The alias comment above can then go.
There was a problem hiding this comment.
Fixed in 4e363d8: rebased onto main, and the engine's factory is now createMotionEngine at its source and in every internal caller, so the entry exports it directly with no alias.
| expect(entry.useGestureRecognizer).toBe(hook.useGestureRecognizer); | ||
| expect(entry.useTouchPresentation).toBe(presentation.useTouchPresentation); | ||
| expect(entry.isRtlElement).toBe(direction.isRtlElement); | ||
| expect(entry.createMotionAnimator).toBe(animator.createAnimator); |
There was a problem hiding this comment.
Once rebased, compare against animator.createMotionAnimator. Also assert expect(typeof entry.createMotionAnimator).toBe('function'): merged with main today, both sides are undefined, so this check passes while the build fails.
There was a problem hiding this comment.
Done in 4e363d8: the identity check compares animator.createMotionEngine, and a loop asserts every re-exported function is a function and every table is defined.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [experimental] Lab components reach Core's animator, cubic-bézier solver, UIKit timing and gesture tables, gesture hook, pan recognizer, rubber band, touch query, touch-presentation hook and writing-direction read through Core's internal `@astryxdesign/core/internal/touch` entry, which only Astryx's own packages may import and which stays outside the public API and its compatibility promise. |
There was a problem hiding this comment.
[experimental] puts this under "Experimental APIs" in Core's changelog, but the internal entry is outside the public API and no Lab component imports these names yet. Suggested:
[chore] Internal: Core's internal touch entry re-exports the animator, curve and gesture helpers Lab's touch components use. Apps import nothing new.
There was a problem hiding this comment.
Done in 4e363d8: the changeset is now [chore] Internal: …, with "Apps import nothing new."
e8cb6cb to
4e363d8
Compare
This head's touch.test.ts against the base's touch.ts (meta#9754's entry): the identity and defined checks for the new names fail here and pass on the parent.
There was a problem hiding this comment.
Recommending approval. Each new name on Core's internal touch entry fills a known internal-API gap, the createMotionEngine rename is complete and changes no behavior, and the earlier alias break and changeset category are fixed.
Two optional follow-ups:
packages/core/src/internal/touch.ts: the header's@outputstill says "the gesture core's primitives"; it could name the gesture, motion, touch-presentation and direction modules the entry now carries.- The description says the
useMotionEnginetests bind, rebind and release the element, but none replaces the node; a re-keyed child case would back that.
[Automated review]
| * @input The gesture core: the arena, the recognizers, and their geometry | ||
| * @input The gesture core and the motion, touch and direction modules Lab | ||
| * uses | ||
| * @output `@astryxdesign/core/internal/touch`: the gesture core's primitives |
There was a problem hiding this comment.
Optional: @input now names the motion, touch and direction modules, but @output still says "the gesture core's primitives". Suggested:
* @output `@astryxdesign/core/internal/touch`: the gesture, motion,
* touch-presentation and direction modules Lab uses, re-exported by nameThere was a problem hiding this comment.
Done in 3946ad2: @output now reads "the gesture, motion, touch-presentation and direction modules Lab uses, re-exported by name".
This head's touch.test.ts against the base's touch.ts (meta#9754's entry): the identity and defined checks for the new names fail here and pass on the parent.
4e363d8 to
3946ad2
Compare
There was a problem hiding this comment.
Recommending approval, pending CI: CI has not run on this head yet. It skips drafts, and because the branch is in a fork, a maintainer adds the ci:run label so CI runs once this is ready for review.
Every new name on @astryxdesign/core/internal/touch is one the internal-entry rules allow, the createMotionEngine rename is complete and merges cleanly with main, and nothing public or user-visible changes.
Optional:
- #9669, next in the declared stack, is built on this branch's previous commit, so it needs a rebase onto this head before the stack lands together.
useMotionEngine.test.tsxcovers mount, unmount and a stable ref but no node replacement, which the description lists as "rebind".- The header of
packages/core/src/internal/touch.tsruns 20 lines, past the source-comment guide of about 12 (see the inline note).
[Automated review]
| * @input The gesture core and the motion, touch and direction modules Lab | ||
| * uses | ||
| * @output `@astryxdesign/core/internal/touch`: the gesture, motion, | ||
| * touch-presentation and direction modules Lab uses, re-exported by name |
There was a problem hiding this comment.
Optional (source-comment guide): this header now runs 20 lines, past the "about 12 lines at most" the guide sets for @input, @output, @position and SYNC:. These two tags fit on one line each:
* @input The gesture core and Core's motion, touch and direction modules
* @output `@astryxdesign/core/internal/touch`: those modules for Lab, by name@position can also drop what the code already shows (named re-exports, Core's relative imports); keep its record citations until a record links this file.
|
/approve-spec 3946ad2 |
3946ad2 to
687c610
Compare
|
/approve-spec 687c610 |
There was a problem hiding this comment.
Recommending approval, pending CI: CI has not run on this head yet. It skips drafts, and because the branch is in a fork, a maintainer adds the ci:run label so CI runs once this is ready for review.
Every new name on @astryxdesign/core/internal/touch is one the internal-entry rules allow, the createMotionEngine rename is complete and merges cleanly with main, and nothing public or user-visible changes.
Optional:
- #9699 and #9669, next in the two declared stacks, are built on this branch's previous commit, so each needs a rebase onto this head before its stack lands.
useMotionEngine.test.tsxcovers mount, unmount and a stable ref but no node replacement, which the description lists as "rebind".- The header of
packages/core/src/internal/touch.tsnow runs 22 lines, past the source-comment guide of about 12, and two comments in that file restate the export names (see the inline notes).
[Automated review]
| * @input The gesture core: the arena, the recognizers, and their geometry; | ||
| * the pan, the motion animator and the motion math that Lab's | ||
| * useRefreshControl reads | ||
| * the pan, the motion engine and math, and the touch-presentation and | ||
| * direction helpers that Lab components read | ||
| * @output `@astryxdesign/core/internal/touch`: the gesture core's primitives | ||
| * for Lab components, re-exported by name | ||
| * and the motion, touch-presentation and direction modules for Lab | ||
| * components, re-exported by name |
There was a problem hiding this comment.
Optional (source-comment guide): this header now runs 22 lines, past the guide's "about 12 lines at most". @input and @output fit on one line each:
* @input The gesture core and Core's motion, touch and direction modules
* @output `@astryxdesign/core/internal/touch`: those modules for Lab, by name| // The motion engine's factory. The public one, `createAnimator`, plays | ||
| // only the theme's state-change motion. |
There was a problem hiding this comment.
Optional (source-comment guide): "The motion engine's factory." restates the export below. The second sentence is the reason, so it can stand alone:
// The public `createAnimator` plays only the theme's state-change motion.| // Lab's PageControl, NavigationStack and TabBar also read the UIKit gesture | ||
| // table, the touch presentation and its query, and an element's direction. |
There was a problem hiding this comment.
Optional (source-comment guide): the second half restates the four names below. This keeps the reason:
// Lab's PageControl, NavigationStack and TabBar read these.
cixzhang
added
the
ci:run
Internal entries land with their first user, per meta#8562. Lab components, TabBar first, animate with Core's motion engine and read the touch presentation and the writing direction as Core does. `@astryxdesign/core/internal/touch`, which already carries the pan recognizer, its hook, the engine and the rubber band for Lab's refresh control (meta#9807), gains by name: - `useTouchPresentation` and `TOUCH_QUERY`, the finger-or-mouse decision in render and outside it; - `isRtlElement`, the writing direction a pan mirrors by; - UIKit's `UIKIT_GESTURES` table. The engine's factory is renamed from `createMotionAnimator` to `createMotionEngine` at its source and in every internal caller, with its types `MotionEngine` and `MotionEngineOptions` (the engine's `Animator` and `AnimatorOptions`), so the engine and the public `createAnimator` differ by more than one word (architecture:react-component-runtime INV13). Named re-exports of code already on main, plus that rename: no behavior changes, no public docs, no root re-export, and the `@astryx/no-core-internal-import` lint rule already limits the entry to Core and Lab. The entry's tests check it re-exports the same values Core imports by path and carries no public factory's name; the touch-support bundle test keeps every Lab-only name out of the root entry. One Lab module those components share comes with them: `internal/useMotionEngine`, a stable motion engine bound to an element through a callback ref. Lab's refresh control now uses it in place of its own identical `RefreshControl/useMotionAnimator.ts`.
687c610 to
f3e826a
Compare
|
/approve-spec f3e826a |
There was a problem hiding this comment.
This pull request is reviewed as part of its stack; see the review on #9966.
There was a problem hiding this comment.
This pull request is reviewed as part of its stack; see the review on #9966.
…n a phone (#9400) * chore(docsite): a lab page to check presented hit testing on a phone /lab/presented-hit-testing, not linked and not indexed, springs a sheet up over a full-page scrim on a spring slowed to a 3 s response, with Done in the sheet and a page behind it that scrolls. It reads out what the engine hit-tested at each touch-down, Done's clicks, the scrim's taps, the sheet pan's translation and how far the page scrolled during the last touch on the sheet, and a switch turns the gesture host's correction off as the control: a phone opening a preview can now exercise the presented hit testing of spec:AST-094 FR11 and FR19. The page drives Core's gesture host and animator through @astryxdesign/core/internal/touch, which now also re-exports GestureHost, PanGestureRecognizer, createAnimator and isIosWebKit by name (no root or public export changes); the entry's test checks each is the value Core imports by path, and failed before them. eslint.config.js lets that one route import the internal entry; any other docsite file still cannot. * Merge remote-tracking branch 'origin/main' into takeover/9400 * chore(gestures): the presented hit-testing check is a Core harness that Storybook renders Per review: a docsite route may not import Core's internal entries (AST-017 FR21, react-component-runtime INV13), so the device check moves into Core as a harness that a Storybook story renders, and a phone opens it on the preview's Storybook. The harness imports the gesture host, the motion animator and the engine check by relative path, so internal/touch, its test, eslint.config.js and the changeset are back to main's. Also from review: - Every touch is measured at the page root in the capture phase: on iOS WebKit a touch on the moving sheet targets the scrim, so the sheet's own handlers missed it. - The engine and Reduce Motion are read on the first render, and the animator lives in the sheet's ref callback, instead of three Effects. - The status line says when Reduce Motion is on, since the sheet then does not move. - A test checks the scrim-targeted touch is logged and measured, and the Reduce Motion line. Co-authored-by: vjeux <vjeux@users.noreply.github.com> * fix(gestures harness): the correction control is a native checkbox Per review: Core's Switch attaches to the document's shared gesture host, which on iOS WebKit resolves the moving sheet before the harness's own host, so the correction never showed. A native checkbox keeps the harness's host the only one on the page. A test stubs the engine check to iOS WebKit, sends a scrim-targeted touch at Done's drawn point, and expects Done to click (it fails with the Switch). A comment says the root logs the engine's own target. Co-authored-by: vjeux <vjeux@users.noreply.github.com> * fix(gestures harness): a two-segment story title Per review: the story tree allows <Package>/<Component> only, so the story is "Presented hit testing" under "Core/Gestures" (core-gestures--presented-hit-testing). The story-tree test passes on a Storybook built from this head. Co-authored-by: vjeux <vjeux@users.noreply.github.com> * Merge branch 'main' into uikit/f2-presented-lab * Merge main into the presented hit-testing lab * chore(gestures): the lab calls createMotionEngine, the animator's new name #9754 renamed Core's internal createMotionAnimator to createMotionEngine, with no alias. The presented hit-testing lab still called the old name, so main's type check would fail once it merged. * Merge branch 'main' into uikit/f2-presented-lab Co-authored-by: Cindy Zhang <cindyxz@meta.com>
Stack: #9754 → #9699 → #9700
Maintainer impact
Lab's TabBar (#9699, #9700) animates with Core's motion engine and reads the touch presentation and the writing direction as Core does; PageControl (#9669) and NavigationStack (#9696) follow it. Lab reaches Core's internals only through the internal
@astryxdesign/core/internal/touchentry, which since #9807 carries the pan, its hook, the engine and the rubber band, but not the rest. Internal entries land with their first user: per the maintainers' Nov 4 ruling, this PR merges only with #9700 (TabBar), which reads all four new names (useTouchPresentationinTabBar.tsx,isRtlElementandUIKIT_GESTURESinuseTabBarLens.ts,TOUCH_QUERYin its test; NavigationStack, #9697, reads it in product code). PageControl's pair (#9669, #9684) restacks on #9754 after it merges.Intended invariant
architecture:react-component-runtimeINV13: Core and Lab components reach the shared touch modules directly only for its known public-API gaps, through@astryxdesign/core/internal/touch(spec:AST-017FR21), and no internal factory shares a public factory's name (spec:AST-107FR6).Change
The engine's factory is renamed from
createMotionAnimatortocreateMotionEngine, and its types fromMotionAnimator/MotionAnimatorOptionstoMotionEngine/MotionEngineOptions, at the source and in every internal caller, so it differs from the publiccreateAnimatorby more than one word. The entry also re-exports, by name, code already on main, each filling an INV13 gap:useTouchPresentationandTOUCH_QUERY: the touch presentation, in render and outside it;UIKIT_GESTURES: UIKit's measured gesture tables;isRtlElement: the direction read Core's own gestures use, which has no public equivalent.Lab gains
internal/useMotionEngine, a stable motion engine bound to an element through a callback ref, which those components share. Lab's refresh control (#9865) now uses it, and its identicalRefreshControl/useMotionAnimator.tsis deleted. No root re-export, no docs, and the@astryx/no-core-internal-importlint rule already limits the entry to Core and Lab.Evidence
createAnimator,createPanGesture,createDragToSnapPoints) is on the entry; the bundle test keeps every Lab-only name out of the root entry.useMotionEngine's tests bind, rebind and release its element.Scope
Testing
At f3e826a, on main 9d068fc (with #9807 and #9865), with node 24.22.1 from
.nvmrc, afterpnpm install --frozen-lockfile. The full gates ran at 1447488, which differs from this head only in text: two comments and a test name in test files, one source comment, and the changeset's wording (no Lab component this stack doesn't ship is named). The touched files' tests (the internal entry's 4, the touch-support bundle's 11), Prettier, ESLint, the changeset checks andpnpm review:prere-ran here. The unit tests of every area the rename and the shared hook touch (motion, animator, gestures, Switch, SegmentedControl, Slider, usePager, DateInput, the internal entry, Lab's hook and the refresh control: 100 files, 2,794 tests) pass; strict ESLint over the whole repo (0 errors); the full build, then the whole typecheck (0 errors); the repository checks (knowledge records, export sync, changesets and their coverage, the i18n catalog and runtime checks, CLI API types, token docs, Lab readiness) pass.pnpm api:reportshows nothing from this PR (its lines are main's own: the CLI report, DropdownMenu's and MoreMenu'spresentationdefault and two style hashes). The UI tests for the changed set (231 files, 6,907 tests) and the node tests (motion-node-loadand the touch-support bundle: 2 files, 13 tests) pass. In Chromium the animator's and the refresh control's interaction specs pass (41; 1 WebKit-only case skips); in WebKit, in the Playwright container, the refresh control's interaction spec passes (18, its end cases included; 6 Chromium-only cases skip).pnpm review:pre: 0 findings. A whole-repogit grep -E 'createMotionAnimator|MotionAnimator|useMotionAnimator'at this head finds only the changeset's rename note (88 occurrences on main), against 62createMotionEnginehits.