Repository navigation
chore(core): Lab's PageControl reaches the shared animator, the Reduce Motion cross-fade and the hit-area watcher through Core's internal touch entry - #9669
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
Generated by PR Enrichment workflow | View current CI run |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the PanGestureRecognizer re-export: architecture:react-component-runtime INV13 opens @astryxdesign/core/internal/touch only for its listed public-API gaps, which name no pan, and spec:AST-107 FR7 sends a drag to the public usePanGesture/createPanGesture, which is not built yet. An engineering owner needs to provide or identify the governing spec for reaching PanGestureRecognizer through the internal entry until then (for example an amended INV13 gap list), or decide that the re-export waits for the public pan factory; they record the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Independently, these changes are needed:
- The diff against
mainis 7,234 authored lines because it carries all of #9399; land #9399 first and rebase so this pull request holds only its own 26-line change. packages/lab/src/SearchField/SearchFieldTouch.ts(lines 495-547) adds its own document-level stray-click swallower after a claimed tap instead of extending Core's click guard (armClickGuardinutils/clickGuard.ts), which the gesture host already arms for claimed touches.packages/lab/src/SearchField/SearchFieldTouch.ts(lines 604-633) creates aResizeObserverper field instead of using Core's sharedobserveResizefrom@astryxdesign/core/utils.
[Automated review]
| unwrapAngle, | ||
| } from '../gestures/geometry'; | ||
| export {LongPressGestureRecognizer} from '../gestures/LongPressGestureRecognizer'; | ||
| export {PanGestureRecognizer} from '../gestures/PanGestureRecognizer'; |
There was a problem hiding this comment.
PanGestureRecognizer serves PageControl's scrub, a drag that spec:AST-107 FR1, FR6 and FR7 assign to the public usePanGesture/createPanGesture; INV13's list of known public-API gaps names no pan. This needs an engineering owner's decision: amend INV13's gap list for the interim, or wait for the public pan factory.
| /** | ||
| * @file touch.ts | ||
| * @input The gesture core: the arena, the recognizers, and their geometry | ||
| * @input The gesture core: the arena, the recognizers, their geometry and |
There was a problem hiding this comment.
Non-blocking: the header is now 23 lines (file headers stay near 12), and @output still says "the gesture core's primitives" although the entry now carries the motion core too. Suggested: @input Core's gesture, motion and touch-presentation modules and @output @astryxdesign/core/internal/touch: those modules for Lab, by name.
| // lets go (its click, the next press, or its window), the late events are | ||
| // over, and this swallow lets go too. | ||
| const disarmRef = useRef<(() => void) | null>(null); | ||
| const armSwallow = useCallback((doc: Document) => { |
There was a problem hiding this comment.
This adds a second stray-click swallower: document capture listeners for mousedown, mouseup and click that stay armed until the next click, touch, pointer or key, with no time limit. Core's armClickGuard (utils/clickGuard.ts) already owns the click after a claimed touch, and the gesture host arms it for claimed contacts. Suggest extending the guard with an option that also swallows the compatibility mousedown/mouseup, re-exporting it through internal/touch, arming it here, and deleting armSwallow and its unmount Effect.
| if (field == null || Observer == null || placement === 'tab') { | ||
| return; | ||
| } | ||
| const observer = new Observer(() => { |
There was a problem hiding this comment.
This creates one ResizeObserver per field. Core's shared observeResize (from @astryxdesign/core/utils, which this component already imports from) provides the same fact: return observeResize(field, onResize);. Its first synchronous call only re-places the settled pose.
0a0243a to
e740f3b
Compare
vjeux
changed the title
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the PanGestureRecognizer re-export: architecture:react-component-runtime INV13 opens @astryxdesign/core/internal/touch only for its listed public-API gaps, which name no pan, and spec:AST-107 FR7 sends a drag to the public usePanGesture/createPanGesture, which is not built yet. An engineering owner needs to provide or identify the governing spec for reaching PanGestureRecognizer through the internal entry until then (for example an amended INV13 gap list), or decide that the re-export waits for the public pan factory; they record the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. The animatorFor, reducedMotionFade and watchHitArea re-exports match INV13's listed gaps. Independently, these changes are needed:
- The diff against
mainis 7,237 authored lines because it carries all of #9399; land #9399 first and rebase so this pull request holds only its own 29-line change. packages/lab/src/SearchField/SearchFieldTouch.ts(lines 495-547) adds its own document-level stray-click swallower after a claimed tap instead of extending Core's click guard (armClickGuardinutils/clickGuard.ts), which the gesture host already arms for claimed touches.packages/lab/src/SearchField/SearchFieldTouch.ts(lines 604-633) creates aResizeObserverper field instead of using Core's sharedobserveResizefrom@astryxdesign/core/utils.
[Automated review]
| unwrapAngle, | ||
| } from '../gestures/geometry'; | ||
| export {LongPressGestureRecognizer} from '../gestures/LongPressGestureRecognizer'; | ||
| export {PanGestureRecognizer} from '../gestures/PanGestureRecognizer'; |
There was a problem hiding this comment.
PanGestureRecognizer serves PageControl's scrub, a drag that spec:AST-107 FR1, FR6 and FR7 assign to the public usePanGesture/createPanGesture, which is not built yet. INV13's list of known public-API gaps names no pan, so this re-export waits on an engineering owner's decision (for example an amended INV13 gap list).
| // An element's one shared animator, as Core's components reach it (no public | ||
| // API has the name), and the Reduce Motion cross-fade's timing. |
There was a problem hiding this comment.
Non-blocking: this comment restates the names below it. Delete it.
| // lets go (its click, the next press, or its window), the late events are | ||
| // over, and this swallow lets go too. | ||
| const disarmRef = useRef<(() => void) | null>(null); | ||
| const armSwallow = useCallback((doc: Document) => { |
There was a problem hiding this comment.
This adds a second stray-click swallower: document capture listeners for mousedown, mouseup and click after a claimed tap, with no time limit. Core's click guard (armClickGuard, utils/clickGuard.ts) already owns the click after a claimed touch, and the gesture host arms it. Extend the guard to also swallow the compatibility mousedown/mouseup, arm it here, and delete armSwallow and its unmount Effect.
|
|
||
| // A resize while settled (the window turning, a host relaying out) moves | ||
| // the narrowed end to the new width at once. | ||
| useLayoutEffect(() => { |
There was a problem hiding this comment.
This creates one ResizeObserver per field. Core's shared observeResize (exported from @astryxdesign/core/utils, which SearchField.tsx already imports) owns this: return observeResize(field, onResize); with the same callback.
e740f3b to
769ed35
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the PanGestureRecognizer re-export: architecture:react-component-runtime INV13 opens @astryxdesign/core/internal/touch only for its listed public-API gaps, which name no pan, and spec:AST-107 FR7 sends a drag to the public usePanGesture/createPanGesture, which is not built yet. An engineering owner needs to provide or identify the governing spec for reaching PanGestureRecognizer through the internal entry until then (for example an amended INV13 gap list), or decide that the re-export waits for the public pan factory; they record the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. The animatorFor, reducedMotionFade and watchHitArea re-exports match INV13's listed gaps, and the earlier SearchField resize-observer and stray-click findings are resolved. Independently, this change is needed:
- The diff against
mainis 7,362 authored lines because it carries all of #9399; land #9399 first and rebase so this pull request holds only its own 22-line change.
[Automated review]
Internal entries land with their first user, per meta#8562. Lab's NavigationStack, PageControl and TabBar animate with Core's animator, solve UIKit's timing curves with Core's cubic-bézier solver, listen for taps and pans with Core's gesture hook and pan recognizer, stretch past their ends with Core's rubber band and read the writing direction as Core does, so `@astryxdesign/core/internal/touch` re-exports, by name: - the motion engine, `createMotionEngine`, for its jobs beyond the public `createAnimator` (architecture:react-component-runtime INV13), with its types `MotionEngine` and `MotionEngineOptions` (the engine's `Animator` and `AnimatorOptions`), `AnimateOptions`, `MotionCurve` and `MotionHandle`. The engine's factory is renamed from `createMotionAnimator` at its source and in every internal caller, so the engine and the public animator differ by more than one word; - `cubicBezier` and UIKit's `timing` table; - `useGestureRecognizer`, `useTouchPresentation` and `isRtlElement`; - `PanGestureRecognizer`, `rubberBand`, UIKit's `UIKIT_GESTURES` table and `TOUCH_QUERY`, which more than one of those components reads. Named re-exports of code already on main, plus that rename: no behavior changes, no 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 test checks it re-exports the same values Core imports by path, and that it carries no public factory's name. One Lab module that those components share comes with them: `internal/useMotionEngine`, a stable motion engine bound to an element through a callback ref.
…e Motion cross-fade and the hit-area watcher through Core's internal touch entry Internal entries land with their first user. Lab's PageControl animates its strip and dots with each element's one shared animator, where the strip's slide and a dot's size and fade compose. It reads the Reduce Motion cross-fade's timing, and it keeps its coarse-pointer hit area live with the hit-area watcher. So `@astryxdesign/core/internal/touch` also re-exports, by name: - `animatorFor`, an element's shared animator, as Core's components reach it (no public API has the name); - `reducedMotionFade`; - `watchHitArea`. Named re-exports only, of code already on main: no behavior change, no docs, no root re-export, no new entry, and the `@astryx/no-core-internal-import` lint rule already limits the entry to Core and Lab. The entry's test checks it re-exports the same values Core imports by path.
769ed35 to
1ea5d3e
Compare
vjeux
changed the title
There was a problem hiding this comment.
This looks ready to approve. Each new name on @astryxdesign/core/internal/touch fills a gap that architecture:react-component-runtime INV13 lists, or, for PanGestureRecognizer, the engineering owners' interim decision recorded on this pull request, and the engine rename leaves no old reference on current main.
Two non-blocking comment suggestions:
packages/core/src/motion/animator.ts: the "Options ofcreateMotionEngine." JSDoc sits aboveSetOptions's own JSDoc, so it documents nothing; move it ontoAnimatorOptionsor drop it.packages/core/src/internal/touch.ts: the header's@outputstill says "the gesture core's primitives"; a shorter header that names the gesture, motion, touch and direction modules would match the entry.
Because the branch is stacked, it should merge after #9754.
[Automated review]
|
|
||
| /** | ||
| * Options of `createMotionAnimator`. | ||
| * Options of `createMotionEngine`. |
There was a problem hiding this comment.
Non-blocking: this JSDoc sits above SetOptions's own JSDoc, so it documents nothing, and AnimatorOptions (line 318) has none. Move it onto AnimatorOptions, or delete it.
| * @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.
Non-blocking: @output still says "the gesture core's primitives", but the entry now also carries the motion engine, curve helpers, hit-area watcher and direction read, and the header is 20 lines (file headers stay near 12). Suggested: @input Core's gesture, motion, touch and direction modules and @output @astryxdesign/core/internal/touch: those modules for Lab, by name.
Stacked on #9754. Review only this PR's change: vjeux/astryx@4e363d8...1ea5d3e
Batch: L1 · Lab: iOS chrome (TabBar, SearchField, NavigationStack, PageControl)
Primary intent
Core's internal touch entry gains three named re-exports for Lab's PageControl, their first user: an element's shared animator, the Reduce Motion cross-fade and the hit-area watcher. The pan recognizer PageControl also reads comes with #9754, which several Lab components share. It is its own small Core PR so that the PageControl batch stays Lab-only. No other template fits: it adds no public capability and changes no behavior.
User or maintainer impact
None for product builders.
@astryxdesign/core/internal/touchis reserved for Astryx's own packages, outside the public API. Lab's PageControl imports these three names.Problem and solution fit
Authority
architecture:react-component-runtimeINV13: Core and Lab reach the internal touch modules only for its known public-API gaps, through this entry, which the@astryx/no-core-internal-importlint rule limits to Core and Lab.spec:AST-017FR21: outside the public API's compatibility promise. Each re-export's gap:reducedMotionFade: "the Reduce Motion cross-fade constants".animatorFor: "the animator's jobs beyond the public handle". The strip's slide and a dot's size and fade compose on one element's shared animator: a companion on its clock.watchHitArea: the coarse-pointer hit-area watcher, which the public gesture layer doesn't cover ("the touch hit-area expansion").Internal entries land with their first user: the PageControl Lab batch, #9684. This PR merges right before it.
Change and evidence
internal/touch.tsre-exportsanimatorFor,reducedMotionFadeandwatchHitAreaby name. Its header's@input("the gesture core and the motion, touch and direction modules Lab uses") already covers them.internal/touch.test.tschecks that each is the same value Core imports by path.[chore]changeset.No behavior change.
Scope
Testing
internal/touch.test.ts: 2 passed.scripts/touch-support-bundle.test.mjs: 8 passed.pnpm linton the changed files: no warnings. Core tsc: 0 errors.check:changesetsandcheck:knowledgepass.architecture:react-component-runtimeINV13;spec:AST-017FR21;architecture:knowledge-contractsINV29 (comments); and the global recordspnpm specs:for --difflists. No record changes, no public API change, no owner-gated paths.Stack: #9754 → #9669 → #9684