Repository navigation
Conversation
vjeux
requested review from
cixzhang,
imdreamrunner and
josephfarina
as code owners
October 27, 2026 14:09
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
needs:design-review
github-actions
Bot
requested review from
cvkxx,
ernestt,
kentonquatman and
rubyycheung
October 27, 2026 14:10
Contributor
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.
This pull request is on hold for a spec decision: the BottomSheet wheel and trackpad pull relies on spec:AST-094 FR43, which is only proposed in #8646, while FR19 on main still keeps gesture recognizers off a fine primary pointer and spec:AST-017 FR59 asks for an owner decision before touch work adds new mouse or trackpad behaviour. An engineering owner needs to land FR43 or an equivalent amendment on main, or identify the existing claim that governs this addition, and records that decision by commenting /approve-spec <full head SHA> on this pull request; the author may do so if they are an engineering owner. Separately, the diff against main is 234 files and 47,300 changed lines because it carries the unmerged #8386 and #8676 stack, so please merge those through their own reviews first, or rebase this branch, so that it contains only its own four files.
[Automated review]
meta-cla
Bot
added
the
CLA Signed
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 27, 2026
… (left: the base; right: meta#8704), Chromium desktop
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 27, 2026
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 27, 2026
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 27, 2026
There was a problem hiding this comment.
Changes requested: the BottomSheet wheel and trackpad pull now has spec authority (spec:AST-094 FR43 is on main), but this branch's diff against main is 236 files and 47,976 changed lines because it carries the unmerged #8386 and #8676 stack under a 292-line change. Please merge those through their own reviews and rebase so the diff is this pull request's own change, and drop the unused sampleCurve pseudo-element helper or move it to the pull request that uses it. Please also update the description, its recording and table, and the comments at useSheetGestures.ts:2133-2135 and :2233-2235: they still say the handle takes no wheel, while the head's tests and changeset make the handle pull the sheet.
[Automated review]
Comment on lines
+2233
to
+2235
| // The handle: a vertical pan, for a finger or a pen where the primary | ||
| // pointer is coarse (the pointer path above serves the rest). A control's | ||
| // drag, so text selection and the touch callout are off under the finger. |
There was a problem hiding this comment.
This still describes the handle's pan as serving "a finger or a pen where the primary pointer is coarse". At this head the pan also takes wheel and trackpad scrolling on every primary pointer (the pan default plus the scroll branch in notePointer), so the comment, like the description, says the opposite of what ships for the handle.
Comment on lines
+2133
to
+2135
| // The recognizers serve a finger, and a pen, where the primary pointer | ||
| // is coarse (spec:AST-094 FR19); a mouse, and anything where it is | ||
| // fine, take the pointer path below (spec:AST-017 FR59). |
There was a problem hiding this comment.
This cites spec:AST-094 FR19 for a coarse-pointer-only rule, but FR19 on main now reads "Recognizers attach for every pointer, with FR43's exceptions", and the branch just above already serves wheel and trackpad input on every primary pointer. Worth rewording with the description.
…view fixes A touch the host handles late now also leaves alone content that slid under the finger after it landed: the touch goes to what was beneath it then (spec:AST-094 FR11). The input-stream changeset says what moved: the gesture host's listeners, and the press feedback's scroll and visibility listeners, since its pointer listeners were already on the window's capture phase. The coarse-pointer gate reads TOUCH_QUERY from touchQueries.ts instead of spelling it again. The WebKit interaction job's scope covers every module the gesture host now bundles: hooks/isRtlElement and utils' flatTree, hapticSwitch, inputStream, clickGuard, selectionLock and coarsePointer. The scope test's example of a util outside it moves from clickGuard, which the gesture fixture now bundles, to focusReturn. The presented proof checks that the sheet moves on a spring (its speed changes along the animator's curve) and no longer compares the drawn box with the animator's offset: under a 3-CPU cap the two disagree by 3 to 27 px in about half the frames whatever reference is used, while the tap and drag cases go through the host's real path.
…e origin most frames agree with Under the CI lane's WebKit profile (four CPUs, one worker), "a spring keeps advancing every frame through a 1 s main-thread block" failed in two ways, each about once in a few hundred runs, and a failing run's frames show each cause: - The spring drifted 138.6 ms against the control. The control's clock took its origin from the first usable frame alone, reading that frame's direction from the next usable one: the frame sat at x 210 on its way out, the next usable one at x 210 after the turn, and the tie read as "back". The clock now lives in controlClock.ts and takes the origin most frames agree with. On that run's own frames it reads 6.5 ms (bound 12), and its unit case for that layout fails on the old reading (150.7 ms astray). - The spring "moved on 61 of the 62 frames the control moved". At the block's end it was in its tail, under two of the cast's 1 px steps a frame, where a frame delivered early shows no step and the next catches up two. The harness now reads such a tail at its own pace (busyWindowProof), and the proof judges the whole block: on that run's own frames the earlier proof fails it and this one passes. No bound changes.
… (architecture:public-component-api INV23) Main's lint rule now keeps every private data attribute in its owner module. This change had moved `data-astryx-haptic-switch` from the haptics module to utils/hapticSwitch.ts, so the click guard could skip the hidden switch without importing haptics (architecture:react-component-runtime INV13). The name now lives in privateAttributes/hapticSwitch.stylex.ts, modelled on segmentDisabled.stylex.ts: it exports markHapticSwitch, which the haptics module uses on its hidden switch, and isHapticSwitch, which the click guard reads. utils/hapticSwitch.ts and the deprecated gestures/hapticSwitch.ts re-export go, the haptics module's baseline entry goes with its raw name, and the WebKit interaction job's scope covers the new module. No behavior changes.
…y the gesture host and the selection lock The selection lock and the gesture host each kept a copy of two helpers: the iOS WebKit engine check, and the walk that asks whether a node is inside an element along the flat tree. The engine check now lives in utils/touchQueries.ts, the React-free half of touchPresentation.ts, as IOS_SUPPORTS_QUERY with one isIosWebKit(view) reader; touchPresentation.ts re-exports the query, so its export is unchanged. The walk is containsAlongFlatTree in utils/flatTree.ts, beside parentAcrossShadow. Unit cases cover both in their new homes. No behavior changes.
No Core or Lab component attaches a gesture recognizer yet, so the input-stream changeset's fix entry told consumers about a change they cannot meet. It now keeps `[fix]` for what ships, touch press feedback hearing scrolls and page hides on the window's capture phase, and a new `[chore] Internal:` changeset carries the recognizers' shared stream and its passive-unless-cancelled listeners.
…trl-wheel (spec:AST-094 FR43) meta#8676's reviewed change, rebuilt as one commit on meta#8360's head: a wheel gesture drives the recognizers that allow its scroll type as a contact moving with the content, a pan along its own axis once every scroller between it and the page is at its edge, and a ctrl-wheel drives a pinch. A screen-edge pan stays a finger's.
…ain input reads as a mouse wheel, and a pan can take a swipe as one step (spec:AST-094 FR43) The round-4 trackpad behaviors, as a draft for the owner to try each one: - wheelSegmenter.ts splits the wheel stream into gestures and reads a trackpad's lift: once a run of shrinking deltas has lost a fifth of its size, momentum began. A pan the host follows ends there at the fingers' travel, released at their last move with their speed, and the momentum after it never moves the pan. Fingers that stop release still, 180 ms (vertical) or 220 ms (horizontal) after their last wheel. - Input the deltas cannot place reads as a mouse wheel: a delta a hair off a whole pixel, a scroll with no legacy wheelDelta (an engine that sends none), a legacy wheelDelta that is not three times the delta, or a 120 tick over a 40 px delta. - A pan created with wheelStep takes a trackpad swipe as one step: a short flick once the swipe travels 24 px, a slow swipe's travel carried across pauses, its slow tail absorbed, and one undo back to rest. A mouse wheel's gesture and a pinch keep their 100 ms end. In Chromium the harness's wheels arrive as notches (wheelDeltaY -120 whatever the delta), which now read as a mouse wheel, so the two trackpad-only browser cases run in WebKit, whose driver sends a trackpad's precise deltas. The lift's release is stamped at the fingers' last move: stamped at the lift, three held wheels later, it read as a stop and lost the fling.
…speed until the splitter decides, so it never stalls before a lift (spec:AST-094 FR43) The splitter reads a lift only after four shrinking deltas, so a pan that held them stood still for that stretch (about 64 to 130 ms) and then let go at the fingers' speed: a stop, then a go. Through the undecided stretch the pan now moves on from where it last was at the fingers' last speed, never past what that speed projects for the time since. At the lift it keeps that travel and lets go there, still moving at that speed, and nothing more moves. When the fingers carry on, their moves absorb what the stretch showed past their travel or short of it, half a move at a time, so the pan never steps back and ends on their travel; fingers that stop without a lift end on their travel where it runs ahead.
…t, and never a mouse wheel (spec:AST-094 FR43, DEC-24) The merged record decides it: a trackpad's two-finger scroll is direct manipulation, so a pan follows it where its record allows it; a mouse wheel's notches are a scrolling control, so a pan takes none, and a scroll without the trackpad signature, Shift+wheel included, counts as a wheel's. - A pan's allowedScrollTypes takes only 'continuous', and none by default, as UIKit's allowedScrollTypesMask: the sheet, the pager and swipe rows opt in. The host never feeds a wheel's notches to a pan, whatever a caller passes. ScrollType keeps 'discrete' for the pinch, which takes a ctrl-wheel from a trackpad or a mouse. - A scroll with Shift held reads as a mouse wheel's. - wheelStep goes: DEC-24 rejects a discrete step per trackpad swipe for rows and paging, and nothing used it. - The splitter's verification streams: ten swipes that each start in the last one's momentum, ten whose momentum runs out, and a slow drag with 150 ms pauses count ten, ten and one gestures.
The measured release table for UISheetPresentationController (motion/uikit/sheet.ts) and the rule that reads it (sheetRelease.ts). From meta#8068.
The detent spec: drags, flicks and throws onto the sheet's detents, the rubber band, the dim, and the settle under a blocked main thread, in Chromium and WebKit. From meta#8423.
A touch release reads UIKit's sheet rule and settles on its springs, from where the sheet is drawn; a press catches a settling sheet where it is; a drag above the base detent restores the full height. From meta#8105.
…does A finger moves the sheet 1:1 between detents and stretches it on the record's rubber band past fully open; a mouse or a pen keeps today's magnet and stretch. From meta#8109.
…'s does The dim tracks the sheet under the finger and moves in lockstep with the release, on the compositor. From meta#8128.
motion/interactive.ts drives a transition a finger scrubs and a release settles: one track rule and one release rule, with layers (the dim) on the driver's clock. BottomSheet's release runs on it. From meta#8341. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
The handle and the body's two scroll-edge pans run on the gesture host for every pointer, and a press catches a settling sheet; whether the content or the sheet takes a drag is decided once, when it begins. From meta#8386. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…gaps after a stall, and a proof can check its pacing WebKit's injected touches carry only real dispatch time, so a stall before a move changes the speed a release reads. The driver paces each move against its script and can check the received gaps (checkPacing); a proof whose gesture arrived off its pacing re-takes it once on a fresh page (retakeOnTouchPacing), and a page's own hold counts as its stall only past the pacing tolerance. Internal harness code landing with its first user, the sheet's velocity proofs (from meta#8910).
emulateIosWebKit skips only stylesheets from another origin, and the scrim-catch proof's touch probe treats only WebKit's TypeError as no touch constructor; anything else throws. The detent spec's header says other components' specs copy its shape.
…e only The cast filmstrip (castStoryboard) belongs to meta#8549, which this base does not carry; the proof's verdict and its frame data stand without it.
…ayer's touches to it, on the recognizers Main's meta#8820 kept a host sheet's pointer path off a gesture that starts inside a <dialog> rendered in its body (a sheet or a Dialog opened from its content). With the sheet's drag on the gesture recognizers, the body's pans refuse such a touch (isInsideNestedLayer, now scoped to the body); the handle's touches are the sheet's own. The nested-sheet test drives touches on measured boxes; a pull from a nested <dialog> fails without the guard. Also moves a release test off a tie: 150 px in 100 ms projected exactly onto the midpoint between the 200 px detent and gone, where float rounding of the event times decided the outcome; 160 px projects clearly past it.
…d the recognizers Main's meta#9193 writes the height and end inset a drag across its base detent restores in the move's own task. On this branch the drag moves the sheet on the travel channel and every pointer's pan begins past its threshold, so its two tests read the sheet's translation (resting transform plus travel) and press one pan threshold behind the drag's start, with pointer events of a mouse; their expectations are unchanged. The hook test that pinned the earlier pairing (the travel waited for the host's commit) now checks that the move writes the full height with its travel. All three fail without the move's layout write. Main's meta#8742 swipes a MobileTokenizer sheet with pointer events alone on a zero-height jsdom sheet, which never reach the recognizers: the swipe now sends a finger's touch and pointer events on a phone-sized box, as the MobilePowerSearch swipe does; its expectation is unchanged.
…es; a sheet a swipe can't close stretches on one rubber band for a finger and a trackpad On the web the sheet's three pans take a trackpad's scroll only (spec:AST-094 FR43, DEC-24): a mouse wheel's notches, ticks and accelerated deltas stay with the page and scroll the content. A trackpad's pull follows the fingers, lets go where they lift, at their speed (or still, 180 ms after its last wheel, when they stop first), and carries the sheet at most its height below its base. A sheet a swipe can't close (a form or required sheet) stretches below its lowest rest on FR3's rubber band for a finger and a trackpad alike, the same travel giving the same stretch, and springs back: a 120 px finger drag that moved such a sheet 110 px now moves a 500 px sheet 54 px. A mouse drag and a pen drag keep their 1:1 tracking (spec:AST-017 FR59). Co-authored-by: Cindy Zhang <cindyxz@meta.com>
vjeux
force-pushed
the
uikit/c17-sheet-wheel
branch
from
November 1, 2026 16:35
a7a090a to
0a6ac57
Compare
There was a problem hiding this comment.
This is on hold for a spec decision: the branch still carries three amendments main does not state, spec:AST-094 FR11 (a tap on content the compositor is moving activates what was drawn under the finger), FR19 (a document-level blocking touchmove listener while that motion runs) and architecture:react-component-runtime INV13 (one shared capture-phase window listener per event type). An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may, as an engineering owner), or the amendments can move to the stacked pull request that owns them. The BottomSheet trackpad pull and form-sheet band themselves follow FR43 and FR3, but these still need changes after that decision:
- Land #8676 and the BottomSheet combined branch first, so this diff is its own ~790 lines instead of ~32,650.
WheelSegmenteris a class outside the recognizer family (INV13); acreateWheelSegmenter()factory would conform.sheetSelectionLock.tsrepeatslockSelectionand the new sharedisIosWebKit, anduseFingerScrollrepeatsuseAttachment; calling the shared helpers keeps one path.- The description's "three wheel notches: sheet pulled 299 px" row describes neither
mainnor the parent commit, where a mouse wheel never moved the sheet; please mark the mouse wheel as unchanged there and in the changeset.
[Automated review]
| } | ||
|
|
||
| /** Whether the engine is iOS WebKit (the only one with the touch callout). */ | ||
| function isIosWebKit(): boolean { |
There was a problem hiding this comment.
This keeps its own iOS WebKit check beside the shared isIosWebKit in packages/core/src/utils/touchQueries.ts:31, which this branch's refactor commit added so the selection lock and the gesture host would stop carrying copies. With TEXT_ENTRY_TYPES, isTextEntry and the selectstart refusal above it, the whole file repeats lockSelection (packages/core/src/utils/selectionLock.ts:79); calling lockSelection here and as the handle's selectionLock keeps one path.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #8676 and the BottomSheet combined branch (fork branch
uikit/sheet-combined-bank, combining #8068, #8423, #8105, #8109, #8128, #8341 and #8386). Review only this PR's change: https://github.com/vjeux/astryx/compare/018b4a07bf6b87f47bc3b1d1b1751d7f68621329..0a6ac57aeb8307bc13cdf5086a5cd2e829ad81cdPrimary intent
On the web a trackpad pulls a BottomSheet, a mouse wheel never moves it, and a sheet a swipe can't close rubber-bands below rest for a finger and a trackpad alike.
User or maintainer impact
formandrequiredsheets stretch below rest on the rubber band and spring back, for a finger and a trackpad: a 120 px finger drag that moved such a sheet 110 px moves a 500 px sheet 54 px. A mouse or pen drag stays one-to-one.Problem and solution fit
spec:AST-094FR43 (#8905) asks for this; #8676 lets a pan take the trackpad when its owner opts in. Chosen: opt the three pans in, cap the pull, share one band.Authority
spec:AST-094FR43, DEC-24, FR3, FR36, FR37;spec:AST-017FR59.Change and evidence
Red first: 14 cases fail without this commit.
Scope
Testing
How to check: on a phone, drag the Form purpose sheet down 120 px; it moves about half and springs back. On a laptop, two fingers up at Snap points' content top pull the sheet; wheel clicks scroll the list.
Trackpad browser cases run in WebKit only (no Chromium driver sends a trackpad wheel).
Local gates at 0a6ac57: root build, typecheck, lint clean; core 18,245, Lab 926 pass; node (but
cache.test.mjs, this machine); Storybook; touchscreen-laptop 2/2; sheet specs (Chromium 24, WebKit 21). Chromium 2-core pin: busy proofs 10/10 at 1 worker; at 4, the rest pass (69), the dim proof starved 3/3. WebKit CI profile ×10: 209 pass; once a WebKit touch burst read as a flick and closed the sheet (dim proof). 3 CPUs: 20/21 at 1 and 16 workers (busy-settle: a repaint at the lift stalls frames). Both come from the combined branch; fixes await BottomSheet's section.