Repository navigation
chore(gestures): a pan's begin threshold can be read from each press - #9953
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC |
|---|---|---|
| TimeInput | 136.9 kB (+56 B) · 14.5 kB CSS (0) | — |
| Slider | 89.8 kB (+52 B) · 8.3 kB CSS (0) | — |
| DateTimeInput | 172.8 kB (+50 B) · 17.2 kB CSS (0) | — |
3 more components
| Component | Size | LoC |
|---|---|---|
| DateInput | 150.7 kB (+47 B) · 15.5 kB CSS (0) | — |
| PowerSearch | 241.0 kB (+47 B) · 21.9 kB CSS (0) | — |
| SegmentedControl | 81.7 kB (+26 B) · 6.7 kB CSS (0) | — |
Package size: @astryxdesign/core 6.39 MB packed (+501 B); no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 8 rows, core 2 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
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. The pull request is a draft from a fork, so CI starts once it is marked ready for review and a maintainer adds the ci:run label.
The function form of hysteresis stays internal, every recognizer that passes a number or nothing keeps UIKit's 10 px, and the new unit fails without the change and passes with it. Two non-blocking suggestions, also left inline:
ScreenEdgePanGestureRecognizerOptionsnow inherits the function form, and given one the screen-edge pan overwrites its edge-zone threshold and loses its 10 px cap off the zone, so omittinghysteresisthere and adding backhysteresis?: numberwould close it.- The new
hysteresisForfield comment restates the field's name and type; keeping only "Read once per press. A finger that lands later keeps the press's threshold." would carry the one constraint the code does not show.
[Automated review]
| * threshold: it reads the press's first touch when that touch lands. | ||
| * @default UIKit's 10 | ||
| */ | ||
| hysteresis?: number | ((touch: GestureTouch) => number); |
There was a problem hiding this comment.
Non-blocking: ScreenEdgePanGestureRecognizerOptions extends Omit<PanGestureRecognizerOptions, …>, so it inherits this function form. Given one, ScreenEdgePanGestureRecognizer.touchesBegan sets the edge-zone threshold and then super.touchesBegan overwrites it, so a touch on an element wider than the band no longer decides by 10 px, and edgeHysteresis falls from 15 to 10. Omitting hysteresis from the screen-edge options and adding back hysteresis?: number keeps the function form to the plain pan.
| travelAtBegan: Point = ZERO; | ||
|
|
||
| private readonly tracker: VelocityTracker; | ||
| /** Each press's threshold, read from its first touch, when given. */ |
There was a problem hiding this comment.
Non-blocking (comment style): this restates the field's name and its | null. Keeping only the constraint the code does not show reads better:
/** Read once per press. A finger that lands later keeps the press's threshold. */PanGestureRecognizer's `hysteresis` may be a function. When a press's first touches land, the pan reads that press's threshold from its first touch. So a recognizer can give a mouse or a pen its own threshold and keep UIKit's 10 px for a finger. A number keeps today's behavior. Lightbox's zoom pan is the first user: a mouse's or a pen's drag on a zoomed photo pans from its first move, while a finger keeps the 10 px that tells a tap from a pan. Without this, it sets the threshold from its own pointerdown listener.
afd73a1 to
2909ed4
Compare
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. The pull request is a draft from a fork, so CI starts once it is marked ready for review and a maintainer adds the ci:run label.
The function form of hysteresis stays internal, a number or no option keeps one threshold for every press, and the new unit fails without the change and passes with it. Both earlier suggestions are addressed: the screen-edge pan's options take a number only, and the hysteresisFor comment keeps just its constraint.
[Automated review]
First users: Lightbox's zoom pan in #8148 and #8197, which adopt this after it merges. One commit on
mainfbb81c6.Maintainer impact
A recognizer user that needs a different begin threshold per press can only mutate the pan's
hysteresisfield from outside the recognizer. Lightbox needs one: a mouse's or a pen's drag on a zoomed photo pans from its first move, while a finger keeps UIKit's 10 px, which tells a tap from a pan. #8148 sets the field from its touch-down recognizer, and #8197, whose touch-down no longer begins on every press, sets it from its own capturepointerdownlistener. Review flagged that listener: it keeps a gesture listener outside the shared modules (architecture:react-component-runtimeINV16,component:LightboxFR14).Intended invariant
A pan begins once its centroid has moved the threshold along either axis (
packages/core/src/gestures/README.md, "What the pan reports"). A number keeps that one threshold for every press, as today.Change
PanGestureRecognizer'shysteresisoption also takes a function of the press's first touch. When a press's first touches land, the pan reads that press's threshold from the function. A second finger landing later does not read it again. The README says so in one sentence.The option is internal: the gestures barrel is not a package export, and the API report does not list it. The changeset is
[chore] Internal:. The screen-edge pan keeps a number only: its edge zone sets the threshold each press, so its options type omits the function form.Two open pull requests sit near it. #8678 (Toast, every pointer) edits
PanGestureRecognizer.tsin other hunks, so the two merge cleanly in either order. #8655 adds a publiccreatePanGesture: if its options passhysteresisthrough, the function form becomes public there, and that review decides it.Evidence
main'sPanGestureRecognizer.ts, which takes the function as the threshold itself, so the finger's press begins at 5 px.Scope
Testing
At 2909ed4, on main fbb81c6:
pnpm install --frozen-lockfileexits 0; coretscis clean; strict ESLint on the changed files reports nothing;CI=true pnpm lintshows no error; Prettier passes;main'sPanGestureRecognizer.ts("expected 'began' to be 'possible'"), and passes here;pnpm test:uipassed at afd73a1, 800 files and 19,988 tests; 2909ed4 then narrows the screen-edge pan's option type and trims a comment, and the gestures, hooks, Lightbox and BottomSheet unit files pass, 2,249 tests;pnpm check:knowledgeandpnpm check:changesetspass.