Repository navigation
feat(core): swipe, pinch and screen-edge pan recognizers - #8061
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
needs:design-review
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval is waiting on an engineering owner's decision about the draft spec spec:AST-094, which governs every recognizer in this PR. Any engineering owner, including you, records that decision by commenting /approve-spec <full head SHA> here. What needs settling is FR1–FR23 and DEC-1/DEC-2, including the proposed amendments to architecture:theme-tokens INV3, design:user-states DR1/DR3, family:buttons, spec:AST-025 FR14 and the guessed 5° rotation threshold. Whatever that decision is, these changes are still needed:
- Land the five predecessor PRs first and rebase, so this PR's diff against
mainis its own 955-line commit rather than 7,610 lines. SwipeGestureRecognizer.ts:126-128fails off-axis swipes depending on event spacing: a 20° drag fails at 4 px per event and is recognized at 9 px.HoverGestureRecognizernever recovers aftersetEnabled(false)orcancel()while hovering, because it stayscancelledand refuses the nextbegan.- Add the
## Writing rubric resultsfor the new AST-094 record to the description.
[Automated review]
| if (Math.hypot(dx, dy) >= this.minimumDistance) { | ||
| this.setState('failed'); | ||
| } |
There was a problem hiding this comment.
This fails any drag whose total travel reaches minimumDistance before its along-axis travel does, so the same off-axis swipe is recognized or failed depending on event spacing: a 20° drag fails at 4 px per event and is recognized at 9 px. Under the measured thresholds alone, a 20° drift recognizes at 50 px along with about 18 px across. Consider failing only when no allowed direction is still reachable, meaning its across travel exceeds maximumSecondaryMovement or the drag runs against it, and add a 20° drag in small steps to the tests.
| } | ||
| this.point = point; | ||
| this.pointerType = pointerType; | ||
| this.applyState('began'); |
There was a problem hiding this comment.
If the recognizer was disabled or cancelled while hovering, its state is cancelled here, and applyState('began') is refused because cancelled may only move to possible. hoverExited also returns early because the recognizer is not active, so it never fires again. Resetting to possible after a cancel outside an arena, or here when the state is terminal, would fix it. A test that disables it mid-hover and re-enters would cover it.
| override get nativeTouchBehavior(): NativeTouchBehavior { | ||
| const horizontal = this.edges.some( | ||
| edge => edge !== 'top' && edge !== 'bottom', | ||
| ); | ||
| const vertical = this.edges.some( | ||
| edge => edge === 'top' || edge === 'bottom', | ||
| ); | ||
| return { | ||
| panX: !horizontal, | ||
| panY: !vertical, | ||
| pinchZoom: true, | ||
| doubleTapZoom: false, | ||
| }; | ||
| } |
There was a problem hiding this comment.
This drops pan-x from the element's touch-action everywhere on the element, not only in the edge band. A one-way PanGestureRecognizer leaves the browser everything, and proposed FR18 asks for that. On a page-level view it would stop a zoomed page from panning sideways. Keeping pan-x collides with FR19's 10 px decision limit, given the 15 px hysteresis, so this needs a decision on the spec side. At the least, the PR text should say that horizontal native panning is given up across the element.
|
/approve-spec 3d85c30 |
There was a problem hiding this comment.
Approval is waiting on an engineering owner to provide or identify the governing spec for one addition: a top or bottom ScreenEdgePanGestureRecognizer on a thin strip removes pan-y and decides at 15 px, while current spec:AST-094 FR18, DEC-13 and DEC-22 state the edge-band rule for pan-x only, and the approval recorded for the previous head does not carry to this commit. Any engineering owner, including you, can record that decision by commenting /approve-spec <full head SHA> on this pull request, or say that the top and bottom edges should leave the browser everything and decide at 10 px. Two fixes are needed either way:
- The edge zone is re-measured at every touch-down while
touch-actionis derived once at attach, so a strip laid out or resized later keeps a staletouch-action; letting the host re-derive it when the zone answer changes, as the size-observer offer on this pull request does, would fix it. isEdgeZonechecks only the view's extent, so a thin view that does not lie within the band (offset from its edge, straddling the band, or aleadingstrip on the wrong side under RTL) losespan-xwhile every touch on it fails at touch-down, although FR18 now ties the claim to an element that lies within the band.
[Automated review]
| // On an element within the band, which takes the axis across the edge, | ||
| // a touch decides at UIKit's 15 px; on one that reaches beyond it, | ||
| // which leaves the browser that axis, at FR19's 10 px, before the | ||
| // browser commits to a scroll (spec:AST-094 FR18, DEC-22). | ||
| const offZone = !this.isEdgeZone( | ||
| edge === 'left' || edge === 'right' ? 'x' : 'y', | ||
| ); | ||
| this.hysteresis = offZone | ||
| ? Math.min(this.edgeHysteresis, UIKIT_PAN.hysteresis.value) | ||
| : this.edgeHysteresis; |
There was a problem hiding this comment.
This touch-down read picks 10 or 15 px from the view's live size, but the host derives touch-action from nativeTouchBehavior once, at attach (PanGestureRecognizer.ts:137-142), and nothing tells it the answer changed. A strip attached before layout keeps pan-x for the browser yet decides here at 15 px, past the 10 px at which the browser commits (spec:AST-094 FR19); a strip widened past the band after attach keeps pan-x removed across the wider element, DEC-22's rejected case. Letting the host re-derive touch-action when the zone answer changes, as the size-observer offer on this pull request does, and deciding from that same answer would close it; a test that resizes the view across the band after attach would cover it.
|
/approve-spec d9a1e1a |
Three recognizers beside main's tap, long press and pan, each reading its own UIKit table group: a swipe recognized from the path between events, a pinch whose velocity settles, and a screen-edge pan that takes the axis across its edge only in the edge zone. meta#8061's change, rebased onto main as one commit; its rotation and hover recognizers, which no component uses yet (and rotation's threshold is unmeasured), land with their first users. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
d9a1e1a to
69abdb3
Compare
… the gesture host The browser negotiation and the gesture host on top of the recognizers: touch-action derived from what is attached, a native scroll probe that follows a slotted node into its shadow tree, the tap haptic, and the host that claims a finger, suppresses the click a claimed touch would send, and hands a touch to the browser when it takes over. meta#8071's and meta#8072's changes, rebased onto main after meta#8061 as one commit; the host's hover routing lands with the hover recognizer's first user. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
vjeux
changed the title
…or, and Lab's route through core's exports Core's internal touch foundation on the gesture recognizers (meta#8061) and the gesture host (meta#8071 + meta#8072): UIControl press tracking with its press profiles, paint and hooks; the animator, which starts a motion from rest at its commit, holds the value on screen inline while a curve runs, and lets a call wait for a presented frame; and Lab reaching Core only through its exports, internal entries included. It absorbs meta#8073 (closed: the press tracking this rests on), meta#8562 (the internal entries) and meta#8340 (the animator's start and hold fixes), with the held dip's load fixes, and keeps meta#8641's rule: a gesture that cancels itself while its finger moves keeps that finger until it lifts. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
There was a problem hiding this comment.
Approval is waiting on an engineering owner to provide or identify the governing spec for one addition: a top or bottom ScreenEdgePanGestureRecognizer on a thin strip removes pan-y and decides at 15 px, while current spec:AST-094 FR18, DEC-13 and DEC-22 state the edge-band rule for pan-x only, and the approval recorded for the previous head does not carry to this commit. Any engineering owner, including you, can record that decision by commenting /approve-spec <full head SHA> on this pull request, land the top and bottom edges in FR18, or say that they should leave the browser everything and decide at 10 px. Three edge-pan fixes are needed either way:
- Compute the edge-zone answer once and share it between
nativeTouchBehaviorand the touch-down decision, with a change signal sotouch-actionfollows a resize (FR19, DEC-22). - Count a view as the edge zone only when it lies within the band at its edge, not merely when it is thin (FR18).
- Keep the edge pan off under a mouse or a pen, for example by defaulting
allowedPointerTypesto['touch'](FR43).
[Automated review]
| super({ | ||
| maximumNumberOfTouches: 1, | ||
| hysteresis: UIKIT_SCREEN_EDGE_PAN.hysteresis.value, | ||
| ...options, | ||
| }); |
There was a problem hiding this comment.
With no allowedPointerTypes default, a mouse or pen drag from the edge begins this pan once the host attaches for every pointer (architecture:react-component-runtime INV13), while spec:AST-094 FR43 keeps the screen-edge pan off under a mouse, a trackpad, or a pen. Defaulting allowedPointerTypes: ['touch'] here, with a test for a mouse and a pen drag, would close it.
| const takesY = | ||
| this.edges.some(edge => !isAcrossHorizontal(edge)) && | ||
| this.isEdgeZone('y'); |
There was a problem hiding this comment.
For a top or bottom edge this takes pan-y on a view no taller than the band and then decides at 15 px; current spec:AST-094 FR18, DEC-13 and DEC-22 state the band claim for pan-x only, so this waits on an engineering owner's decision for this head.
| const offZone = !this.isEdgeZone( | ||
| edge === 'left' || edge === 'right' ? 'x' : 'y', | ||
| ); | ||
| this.hysteresis = offZone | ||
| ? Math.min(this.edgeHysteresis, UIKIT_PAN.hysteresis.value) | ||
| : this.edgeHysteresis; |
There was a problem hiding this comment.
This picks 10 or 15 px from a fresh size read at touch-down, while the host derives touch-action from nativeTouchBehavior once at attach (PanGestureRecognizer.ts:139-140), and before the first touch that read goes through DEFAULT_ENVIRONMENT rather than the arena's environment. A strip unmeasured at attach then leaves pan-x to the browser yet decides at 15 px (FR19), and one that grows past the band keeps pan-x removed (DEC-22). Computing the zone once, sharing it with nativeTouchBehavior, and signalling the host when it changes would fix both.
| private isEdgeZone(across: 'x' | 'y'): boolean { | ||
| const size = this.environment.viewSize?.(this.view); | ||
| if (size == null) { | ||
| return false; | ||
| } | ||
| const extent = across === 'x' ? size.width : size.height; | ||
| return extent > 0 && extent <= this.edgeRegion + EDGE_ZONE_TOLERANCE; | ||
| } |
There was a problem hiding this comment.
Only the extent is checked, so a thin view away from its edge (or a leading strip on the left under RTL) loses pan-x while edgeAt fails every touch on it; FR18 ties the claim to "an element that lies within the band" and keeps the author's scrolling outside it. Checking the view's position against the edge it watches would fix it.
Three recognizers beside main's tap, long press and pan, each reading its own UIKit table group: a swipe recognized from the path between events, a pinch whose velocity settles, and a screen-edge pan that takes the axis across its edge only in the edge zone and is a finger's only (a mouse, a trackpad or a pen never starts it). meta#8061's change, rebased onto main as one commit; its rotation and hover recognizers, which no component uses yet (and rotation's threshold is unmeasured), land with their first users. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
69abdb3 to
65d81b5
Compare
… the gesture host The browser negotiation and the gesture host on top of the recognizers: touch-action derived from what is attached and never wider than the element's own (spec:AST-094 FR18, DEC-8), a native scroll probe that follows a slotted node into its shadow tree, the tap haptic, and the host that claims a finger, suppresses the click a claimed touch would send, and hands a touch to the browser when it takes over. The host measures a screen-edge pan's view, so an edge-zone strip decides at UIKit's 15 px (FR18), and recognizers report their kind, which the host and the touch-action rules read instead of checking a class (architecture:react-component-runtime INV13). meta#8071's and meta#8072's changes, rebased onto main after meta#8061 as one commit; the host's hover routing lands with the hover recognizer's first user. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
There was a problem hiding this comment.
Approval is waiting on an engineering owner to provide or identify the governing spec for one addition: a top or bottom ScreenEdgePanGestureRecognizer on a thin strip removes pan-y and decides at 15 px, while current spec:AST-094 FR18, DEC-13 and DEC-22 state the edge-band rule for pan-x only, and the approvals recorded for earlier heads do not carry to this commit. Any engineering owner, including you, can record that decision by commenting /approve-spec <full head SHA> on this pull request, land the top and bottom edges in FR18, or say that they should leave the browser everything and decide at 10 px. The edge pan now stays off under a mouse or a pen as FR43 asks; two edge-pan fixes are still needed either way:
- Compute the edge-zone answer once and share it between
nativeTouchBehaviorand the touch-down decision, with a change signal sotouch-actionfollows a resize (FR19, DEC-22). - Count a view as the edge zone only when it lies within the band at its edge, not merely when it is thin (FR18).
[Automated review]
|
Thanks. Both points are addressed in 08a1683:
|
There was a problem hiding this comment.
Changes requested: the swipe, pinch and screen-edge pan recognizers match spec:AST-094, but under RTL the screen-edge pan resolves the leading edge differently for its touch-action and for its touch-down decision. Before any touch it reads the writing direction through DEFAULT_ENVIRONMENT, whose isRtl() always returns false (ScreenEdgePanGestureRecognizer.ts:192-197, types.ts:143-145), so a leading strip at the right edge keeps pan-x for the browser while it waits for 15 px, which FR18, FR19 and DEC-22 allow only where the strip takes pan-x. Please resolve the direction once together with the edge-zone box (and give DEFAULT_ENVIRONMENT.isRtl the view's computed direction), then add an RTL test that reads nativeTouchBehavior before a touch and checks it against the touch-down decision.
[Automated review]
| private physicalEdges(): PhysicalEdge[] { | ||
| const isRtl = this.environment.isRtl(this.view); | ||
| return this.edges.map(edge => | ||
| (edge === 'leading') !== isRtl ? 'left' : 'right', | ||
| ); | ||
| } |
There was a problem hiding this comment.
This mapping is not part of the cached edge-zone answer. Before a touch reaches the recognizer it has no arena (GestureArena.ts:208), so this.environment is DEFAULT_ENVIRONMENT, whose isRtl() always returns false (types.ts:143-145); the touch-down read goes through the arena's environment. Under RTL a leading strip at the right edge therefore declares every native behavior when the host derives touch-action, yet touchesBegan picks the 15 px start for it, and a leading strip on the left declares panX: false while every touch on it fails. layoutChanged() compares only the box answer, so nothing announces the switch after the first touch. Resolving the direction once with the box (and re-checking it in layoutChanged()), plus a real isRtl in DEFAULT_ENVIRONMENT, keeps both answers on one direction (spec:AST-094 FR18, FR19, DEC-22).
|
Thanks, fixed in 7e1cc39. The edge pan now resolves its edges' sides from the view's writing direction together with the edge-zone box, once, so touch-action and the touch-down decision read the same answer, and the default environment reads the view's computed direction. A test covers LTR and RTL, leading and trailing, each before a touch, at touch-down and after a resize. |
There was a problem hiding this comment.
Recommending approval: the swipe, pinch and screen-edge pan recognizers match spec:AST-094, and the screen-edge pan resolves its edges from the view's writing direction together with its edge zone, so under RTL its touch-action and its touch-down decision agree. The new RTL tests fail against an earlier revision and pass with this patch, which the rebase left unchanged. Nothing else is needed here; the real-touch browser tests for FR18–FR22 and FR43 belong with the gesture host that applies touch-action.
[Automated review]
There was a problem hiding this comment.
Changes requested: the swipe, pinch and screen-edge pan recognizers match spec:AST-094, but the new touch-down direction re-read lets one touch decide at 15 px while the browser still holds pan-x. After an in-place switch to RTL, the touch that remeasures decides from the new answer (ScreenEdgePanGestureRecognizer.ts:184-187). The touch-action in force for it was derived from the old one, so a right-edge strip that just became the edge zone waits until 15 px instead of FR19's 10. Deciding that touch at 10 px unless the layout in force claimed pan-x, and adding that case to the in-place test, would resolve it.
[Automated review]
| // browser that axis, at FR19's 10 px, before the browser commits to a | ||
| // scroll (spec:AST-094 FR18, DEC-22). The answer is the one the | ||
| // element's touch-action was derived from. | ||
| const offZone = !this.measured().zone.has(edge); |
There was a problem hiding this comment.
After the in-place remeasure at lines 170-172, this reads the new answer, but the touch-action the browser applied when this touch landed was derived from the old one. For a right-edge strip that the switch to RTL just made the edge zone, this touch keeps pan-x for the browser yet waits until 15 px, past spec:AST-094 FR19's 10 px. Consider recording whether the layout in force claimed pan-x before remeasuring, and deciding this touch at 10 px unless it did.
|
Fixed in 31ce964: the touch that measures again decides as the |
There was a problem hiding this comment.
Recommending approval: the swipe, pinch and screen-edge pan recognizers match spec:AST-094, and the first touch after an in-place switch to RTL now decides at 10 px unless the touch-action in force claimed pan-x, as FR19 and DEC-22 require. The new in-place test fails against the previous decision rule and passes with this change. Nothing else is needed here; FR18–FR22's browser checks and FR43's wheel and ctrl-wheel mappings belong with the gesture host and the first component that attaches these recognizers.
[Automated review]
Three recognizers beside main's tap, long press and pan, each reading its own UIKit table group: a swipe recognized from the path between events, a pinch whose velocity settles, and a screen-edge pan from the leading or trailing edge. The edge pan takes the horizontal axis only on a view that lies within that edge's band. The sides its edges resolve to under the view's writing direction and the edge zone are measured once, together, for touch-action and the touch-down decision alike, and again when the host reports a layout change or a touch-down finds the view's direction switched in place; and it is a finger's only (a mouse, a trackpad or a pen never starts it). meta#8061's change, rebased onto main as one commit; its rotation and hover recognizers and its top and bottom edges, which no component uses yet (and rotation's threshold is unmeasured), land with their first users. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
There was a problem hiding this comment.
Recommending approval: the swipe, pinch and screen-edge pan recognizers match spec:AST-094 (FR15–FR19, FR43, DEC-13 and DEC-22) and its related records, and the review found no defect or missing evidence. The edge pan's 10 px and 15 px decisions are pinned by tests that fail if the band claim or its start distance is loosened. The browser-level checks AST-094 lists for these recognizers belong with the gesture host that attaches them.
[Automated review]
Section 2 of the merge plan: #8061 → #8071 → #8076. One commit on main holding only this change.
Primary intent
Complete the internal gesture system's recognizer set with UIKit's other recognizers that a component uses: swipe (Slider), pinch (Lightbox's zoom) and screen-edge pan (NavigationStack's back swipe). Rotation and hover, which no component uses yet (and rotation's threshold is unmeasured), land with their first users.
User or maintainer impact
Sliders, lightboxes and back-swipe edges each need a recognizer, and today they would hand-roll one. These follow UIKit's classes, so each component gets the same thresholds and conflict rules. For example, an edge pan that the inner pan requires to fail delays nothing outside the edge band.
Problem and solution fit
Deltas:
SwipeGestureRecognizer: recognized during the motion; one or more directions; fingers.PinchGestureRecognizer: scale measured from the starting span moved by the hysteresis, so it does not jump; settable scale; velocity in scale per second; focal point. The page keeps pinch-zoom wherever no pinch recognizer is attached.ScreenEdgePanGestureRecognizer: the leading or trailing edge, resolved against the writing direction; a touch outside every band fails at touch-down, so nothing waits on it there. Attached to an element that lies within the band at its edge, a pan takes the horizontal axis and an in-band touch decides at UIKit's 15 px; on any other element it leaves the browser every touch behavior and decides at FR19's 10 px (spec:AST-094FR18, DEC-22). The sides its edges resolve to under the view's writing direction and that answer are measured together, once, for the element'stouch-actionand the touch-down decision alike, and again when the host reports a layout change, which re-derivestouch-action(FR19), or when a touch-down finds the view's writing direction switched in place, so aleadingback swipe follows a switch to RTL (design:bidirectionalityDR13); that touch landed under thetouch-actionfrom before, so it decides at FR19's 10 px unless that one claimed the axis. It is a finger's only: a mouse, a trackpad or a pen never starts it (FR43). Top and bottom edges, which no component uses yet, land with their first user.Authority
spec:AST-094(current). Thresholds come frommotion/uikit/gestures.tswith their sources.Change and evidence
Read from UIKit's recognizers in a reference app on the iOS 26.2 simulator and confirmed with HID touches:
.began= span ÷ (initial span ± 8), for example 144 / 128 = 1.125 from 120;Scope
Testing
Local gates at ab509ed, one commit on main 257380d holding only this change (12 files, +1,522/−18):
pnpm test:ui(core UI): 658 files, 16,506 tests passed.pnpm exec vitest run --project node: 806 files passed; the only failures are main's own story-tree and release-cache reds, tracked in test(storybook): run the story-tree check where Storybook is built, and bring the tree back to its shape #8509 and fix(cli): a crashed run's release install is recognized by the Node it ran on, whatever its name #8520, and its single-bundle case, which bundles Core's build and passes once that build is fresh.tsc --project packages/core/tsconfig.json --noEmit, strict ESLint and Prettier on every changed file: clean.pnpm check:repo: clean.