Repository navigation
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.
This pull request is on hold for a spec decision. Gestures for every pointer contradict spec:AST-094 FR19 and FR24 and architecture:react-component-runtime INV13 as they read on main, and the FR43 and DEC-21 amendment in #8646 that would allow them is not merged yet. Approval needs an engineering owner to settle that, either by landing #8646's amendment on main or by commenting /approve-spec <full head SHA> on this pull request; the author, as an engineering owner, may do so. Whatever the decision, these still need changes:
- Land #8360 first, so this diff carries only its own 14 files instead of 27,964 authored lines.
- Under a fine pointer, give a finger's claiming recognizer the same pre-registered blocking
touchmovelistener it gets under a coarse one (GestureHost.ts:825-831), with a test. - Apply a hold's selection lock to a finger or a pen whatever the primary pointer (
utils/selectionLock.ts:62-67), with a test. - Either serve hover under a fine pointer or say in the README that it stays coarse-only.
[Automated review]
| const needsListener = | ||
| this.isCoarse && | ||
| (registration.recognizers.some( | ||
| recognizer => recognizer.isEnabled && claimsWhileMoving(recognizer), | ||
| ) || | ||
| (registration.consumer?.keepsTouch != null && | ||
| registration.consumer.isEnabled?.() !== false)); |
There was a problem hiding this comment.
Under a fine pointer this leaves a finger's claiming recognizer (an axis-limited pan, a sheet's pull) without a pre-registered blocking touchmove listener. The only non-passive one left is added at touchstart by watchTarget, which spec:AST-094 FR19's known limit says "would hand diagonal drags to the scroll". #8646's FR24 counts "a claiming recognizer's touchmove listener for a finger" outside the one-listener budget, so dropping this.isCoarse && for claiming recognizers, plus a fine-pointer finger test that a claimed touchmove is cancelled, would close this.
| export function locksSelectionFor( | ||
| pointerType: string, | ||
| view: Window | null | undefined, | ||
| ): boolean { | ||
| return pointerType !== 'mouse' && isCoarsePointer(view); | ||
| } |
There was a problem hiding this comment.
With gestures attaching for every pointer, a finger's long press under a fine pointer (a touchscreen laptop) now runs the hold, but this gate skips the lock, so the browser's text-selection start and callout begin too, which spec:AST-094 FR41 rules out for an element whose hold does something. Locking for a finger or a pen whatever the primary pointer (a mouse stays exempt), and updating the comment above, would fix it.
| | `PinchGestureRecognizer` | `UIPinchGestureRecognizer` | continuous | `scale` (settable, to re-base it), `velocity` in scale per second, `focalPoint()` | | ||
| | `RotationGestureRecognizer` | `UIRotationGestureRecognizer` | continuous | `rotation` in radians, unwrapped, clockwise positive; `velocity` in rad/s | | ||
| | `ScreenEdgePanGestureRecognizer` | `UIScreenEdgePanGestureRecognizer` | continuous | `edges` including `leading` / `trailing` (resolved against the writing direction); fails as soon as the touch lands outside | | ||
| | `HoverGestureRecognizer` | `UIHoverGestureRecognizer` | continuous | a mouse or a hovering pen; silent for fingers | |
There was a problem hiding this comment.
The host serves hover only where the primary pointer is coarse (GestureHost.ts:1267, :1395-1397), so on a desktop a hover recognizer never begins. Either say here that hover stays coarse-only, or serve it under a fine pointer.
…) into the context request
There was a problem hiding this comment.
This pull request is on hold for a spec decision: gestures for every pointer contradict spec:AST-094 FR19 and FR24 and architecture:react-component-runtime INV13 as they read on main, the FR43 and DEC-21 amendment in #8646 is not merged, and clearing the selection of a mouse or pen drag a recognizer claims also departs from FR41's "a mouse keeps its released behavior", which #8646 leaves unchanged. Approval needs an engineering owner to settle both, either by landing the amendment on main with FR41 covering drag selection, or by commenting /approve-spec <full head SHA> on this pull request; the author, as an engineering owner, may do so. Whatever the decision, these still need changes:
- Land #8360 first, so this diff carries only its own 17 files instead of 28,460 authored lines.
- Under a fine pointer, make a kept press's blocking
touchmove(GestureHost.ts:832-833) actually keep the press, or stop registering it, and make the host header and README say which, with a test. - Apply a hold's selection lock to a finger whatever the primary pointer (
utils/selectionLock.ts:62-67), with a test. - Either serve hover under a fine pointer or say in the README that it stays coarse-only.
- Apply
isPrimaryClickto a pen as the README says (GestureHost.ts:1132,:2198), or correct the README.
[Automated review]
| (registration.consumer?.keepsTouch != null && | ||
| registration.consumer.isEnabled?.() !== false); |
There was a problem hiding this comment.
Under a fine pointer this now registers the blocking touchmove for a keepsTouch press tracker, but cancelIfClaimed still cancels a kept press's moves only where the pointer is coarse (:1771-1773), and the target listener's blocking flag is coarse-only too (:1605). On a touchscreen laptop the listener makes scrolls that start on the control wait for the main thread without ever keeping the press. Either drop those two coarse gates (and have #8646's FR24 budget the listener) or restore this.isCoarse && here; the header at :21-23 and README.md:320-322 should then say which, with a fine-pointer kept-press test.
| pointerType: string, | ||
| view: Window | null | undefined, | ||
| ): boolean { | ||
| return pointerType !== 'mouse' && isCoarsePointer(view); |
There was a problem hiding this comment.
With gestures attaching for every pointer, a finger's long press under a fine pointer (a touchscreen laptop) now runs the hold, but this gate skips the lock, so the browser's text-selection start and callout begin too, which spec:AST-094 FR41 rules out for an element whose hold does something. Locking for a finger whatever the primary pointer, keeping a mouse exempt, and updating the comment above and the README, would fix it.
| | `PinchGestureRecognizer` | `UIPinchGestureRecognizer` | continuous | `scale` (settable, to re-base it), `velocity` in scale per second, `focalPoint()` | | ||
| | `RotationGestureRecognizer` | `UIRotationGestureRecognizer` | continuous | `rotation` in radians, unwrapped, clockwise positive; `velocity` in rad/s | | ||
| | `ScreenEdgePanGestureRecognizer` | `UIScreenEdgePanGestureRecognizer` | continuous | `edges` including `leading` / `trailing` (resolved against the writing direction); fails as soon as the touch lands outside | | ||
| | `HoverGestureRecognizer` | `UIHoverGestureRecognizer` | continuous | a mouse or a hovering pen; silent for fingers | |
There was a problem hiding this comment.
The host serves hover only where the primary pointer is coarse (GestureHost.ts:1269, :1397-1399), so on a desktop a hover recognizer never begins, while this row and the host header now promise every pointer. Either say here that hover stays coarse-only, or serve it under a fine pointer.
| } | ||
| // A right click, or a ctrl-click on macOS (its context click), starts no | ||
| // press (FR43). | ||
| if (event.pointerType === 'mouse' && !isPrimaryClick(event)) { |
There was a problem hiding this comment.
The README (README.md:247-249) and isPrimaryClick's JSDoc say a pen's barrel button starts no press, but this check, like the one at :1132, applies to a mouse only, so a pen pointerdown with its barrel button (button 2) still begins a pan or a press. Applying isPrimaryClick to every pointer but a finger here and at :1132, with a pen case, would match the docs.
…ment-wide; a fine-pointer finger's action-group press ends) into the context request
…o the FR43 harness helper
|
Correction: since the move, pushes to pull requests from forks start no workflows at all. #8666's push at 23:22Z has 0 runs, while meta/astryx's own branches run normally. So there is nothing to re-run on those seven heads until fork pull requests can run workflows again. |
There was a problem hiding this comment.
This pull request is on hold for a spec decision: the rule that a claimed mouse or pen drag clears and refuses text selection changes spec:AST-094 FR41, which today keeps a mouse's released behavior, and the FR41 sentence added for it is not yet approved at this head. 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 FR41 and its verification row are amended on main, and the decision should also say whether the rule may clear a selection made elsewhere before the drag. Independent of that decision, these need changes:
- Land #8360 first (or rebase onto
mainonce it lands) and move the click-guard test-setup fix there, so this diff carries only its own change, today 27,728 authored lines with 2,401 of them its own. - DropdownMenu's trigger gives a touchscreen laptop's finger 10 px instead of FR29's drift allowance (
hooks/useMenuPress.ts:108-126). - Under a fine pointer, an armed press installs no cancelable
touchendorcontextmenulistener (GestureHost.ts:1271), a kept press never keeps its finger (GestureHost.ts:1920-1922), and a page hidden mid-press keeps the page listeners (GestureHost.ts:2736-2740).
[Automated review]
| } | ||
| this.isArmedPress = true; | ||
| this.syncKeptPresses(); | ||
| this.install(); |
There was a problem hiding this comment.
Under a fine pointer this installs the host's listeners without syncClaimingListeners(), which only syncListeners() calls (:1081-1082). So during the press the window's touchend is subscribed passively and no contextmenu canceller exists: a claimed finger's touchend is handled first by the passive window listener, whose preventDefault() is ignored, and the finger's cancelable target listener then skips it as already handled (:2084-2086). That leaves FR21's "its touchend MUST be cancelled, and no click or compatibility mouse event reaches the page" unmet for a touchscreen laptop's finger, and a claimed long press can also open the browser's menu. Calling this.syncClaimingListeners() after this.install() here would fix both; a fine-pointer test of a claimed lift and a mid-claim contextmenu would pin it.
| (this.arena.isClaimed(id) || | ||
| (this.isCoarse && | ||
| this.arena.consumerOf(id)?.keepsTouch?.(id) === true) || |
There was a problem hiding this comment.
The kept-press path still requires a coarse pointer, so the blocking touchmove that syncKeptPresses now registers during a fine-pointer press never cancels anything (also :1752-1753). A finger that moves on a held stepper on a touchscreen laptop is then panned by the browser, which cancels the press (FR20), against FR11's "A control whose own record keeps its press through a drag keeps it". Either drop this.isCoarse && here and at :1753, or stop registering the listener under a fine pointer; the README currently says both (README.md:248-251, :336-338).
|
|
||
| private readonly onVisibilityChange = (): void => { | ||
| if (this.doc.visibilityState === 'hidden') { | ||
| this.cancelEverything(this.arena.now()); |
There was a problem hiding this comment.
The blur path now ends the armed press, but a hidden page still only cancels the contacts: isArmedPress stays true, shouldListen() keeps returning true (:1421), and the window subscriptions stay until the next lift anywhere when the page is hidden without a window blur. FR24 and INV13's verification fail an idle document listener outside a press. Calling this.releaseArmedPress() here (or in cancelEverything) with a listener-count test would close it.
| function triggerTolerance(): LongPressTolerance { | ||
| return isCoarsePointer(typeof window === 'undefined' ? null : window) | ||
| ? {moveCancelPx: MENU_PRESS_TRIGGER_DRIFT_PX} | ||
| : {moveCancelPx: LONG_PRESS_ALLOWABLE_MOVEMENT_PX}; | ||
| } |
There was a problem hiding this comment.
Current spec:AST-094 applies FR29 to DropdownMenu's trigger under every pointer ("FR29 applies to DropdownMenu's trigger under the same condition"), and module:DropdownMenu/useMenuPress FR9 says the trigger's finger open follows FR29 "under every primary pointer". Gating the 13 px drift on (pointer: coarse) gives a touchscreen laptop's finger 10 px, so an 11 to 13 px drift cancels its held press where a phone's keeps holding. A finger or pen could take MENU_PRESS_TRIGGER_DRIFT_PX under every primary pointer, with the two tests at useMenuPress.test.tsx:487 and :537 updated.
| selection.removeAllRanges(); | ||
| } | ||
| }; | ||
| view?.getSelection()?.removeAllRanges(); |
There was a problem hiding this comment.
This removes whatever selection the page holds when a recognizer claims a mouse or pen contact, including one made elsewhere that the drag itself did not replace, and the document then refuses selectstart until a task after the lift. Current spec:AST-094 FR41 keeps a mouse's released behavior and its verification row fails "a change to native selection elsewhere or to mouse ... selection"; the FR41 sentence this pull request adds needs an engineering owner's approval at this head, and the open question asks for it.
There was a problem hiding this comment.
This pull request is on hold for a spec decision: the rule that a claimed mouse or pen drag clears and refuses text selection changes spec:AST-094 FR41, which today keeps a mouse's released behavior, and the FR41 sentence added for it is not yet approved at this head. 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 FR41 and its verification row are amended on main, and the decision should also say whether the rule may clear a selection made before the drag; the description should report the ## Writing rubric results for that edit, which approval reads it against. Independent of that decision, these need changes:
- Land #8360 first (or rebase onto
mainonce it lands) and move the click-guard test-setup fix there, so this diff carries only its own change, today 28,168 authored lines with 2,482 of them its own. - Under a fine pointer, an armed press installs no cancelable
touchendorcontextmenulistener (GestureHost.ts:1299), a kept press never keeps its finger (GestureHost.ts:1950-1953), and a page hidden mid-press keeps the page listeners (GestureHost.ts:2767-2771). - Several comments, README passages and one test still describe the coarse-only host, for example
GestureHost.ts:84-86,gestures/README.md:316-320andGestureHost.test.ts:2599.
[Automated review]
| } | ||
| this.isArmedPress = true; | ||
| this.syncKeptPresses(); | ||
| this.install(); |
There was a problem hiding this comment.
Under a fine pointer this installs the window listeners without syncClaimingListeners(), which only syncListeners() runs after install() (:1106-1110). For the whole press the window's touchend stays passive and no contextmenu canceller is subscribed, so a claimed finger's lift is not cancelled (spec:AST-094 FR21: no click or compatibility mouse event reaches the page) and a claimed long press can also open the browser's menu. Calling this.syncClaimingListeners() right after this.install(), with a fine-pointer test of a claimed finger's touchend and a mid-claim contextmenu, would close it.
| (this.isCoarse && | ||
| this.arena.consumerOf(id)?.keepsTouch?.(id) === true) || |
There was a problem hiding this comment.
A fine pointer's kept press gets its blocking touchmove during the armed press (:930, :1305-1311), but this coarse-only condition means its moves are never cancelled, so a finger that moves on a held stepper on a touchscreen laptop is not kept (spec:AST-094 FR11, DEC-21) while the listener still delays scrolls that start there. Dropping this.isCoarse && here and at :1783, or not registering the listener under a fine pointer, would make it consistent; the README says both things today (README.md:249-251, :338-340).
| private readonly onVisibilityChange = (): void => { | ||
| if (this.doc.visibilityState === 'hidden') { | ||
| this.cancelEverything(this.arena.now()); | ||
| } | ||
| }; |
There was a problem hiding this comment.
A hidden page cancels the press here but leaves isArmedPress set, so shouldListen() stays true and the page's listeners remain until the next lift anywhere (spec:AST-094 FR24; architecture:react-component-runtime INV13). onBlur calls releaseArmedPress() for the same case; this branch could too, with a listener-count test.
| expect(slider.style.getPropertyValue('user-select')).toBe(''); | ||
| }); | ||
|
|
||
| it("keeps selection for every contact where the primary pointer is fine: a touchscreen laptop's finger, a desktop pen and a mouse (spec:AST-094 FR19, spec:AST-017 FR59)", () => { |
There was a problem hiding this comment.
This title states the superseded rule: locksSelectionFor now locks a finger and a pen under any primary pointer (utils/selectionLock.ts:60-62, selectionLock.test.ts:97). The test passes only because it reads the iOS-only user-select write after a touchstart with no arming pointerdown; it could assert the lock a fine-pointer finger and pen now take.
| selection.removeAllRanges(); | ||
| } | ||
| }; | ||
| view?.getSelection()?.removeAllRanges(); |
There was a problem hiding this comment.
This removes whatever selection the page holds when a recognizer claims the drag, including one made before the drag that the drag did not replace. Whether FR41's proposed "for that drag only" covers that is part of the spec decision this pull request waits on.
|
Fixed in this push:
Whether FR41 may suppress selection for a claimed drag stays the owner's decision. |
There was a problem hiding this comment.
This pull request is on hold for a spec decision: the rule that a claimed mouse or pen drag clears the page's text selection and refuses a new one changes spec:AST-094 FR41, which today keeps a mouse's released behavior, and the FR41 amendment written for it is not yet approved at this head. 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 asks for the amendment to change; the description should report the ## Writing rubric results for that edit, which approval reads it against. Independent of that decision, these need changes:
- Land #8360 first (or rebase onto
mainonce it lands) and move the click-guard test-setup fix there, so this diff carries only its own change: today 28,406 authored lines, 2,788 of them its own. - Several comments and README passages still say mouse selection is unchanged or that content under a pan stays selectable, for example
GestureRecognizer.ts:71-73,gestures/README.md:316-320andPressTracker.ts:90-94.
[Automated review]
| * hold does not bundle it. Content under a sheet's or a pager's pan stays | ||
| * selectable, so there is none by default; a mouse keeps desktop selection | ||
| * either way. A long press brings its own when it has a hold (a |
There was a problem hiding this comment.
A claimed mouse pan now clears the page's selection and refuses a new one until after the lift (GestureHost.ts:1659-1690, utils/selectionLock.ts:147-213), so "stays selectable" and "a mouse keeps desktop selection either way" no longer hold for a mouse. Describe what a claimed mouse or pen drag does, or whatever the FR41 decision settles.
| finger or a pen takes the lock under any primary pointer, a touchscreen | ||
| laptop and a desktop pen tablet included; a mouse never does, so mouse | ||
| selection is unchanged (`spec:AST-017` FR59); content under a sheet's or a | ||
| pager's pan stays selectable. |
There was a problem hiding this comment.
A mouse never takes the hold lock, but a claimed mouse pan does change mouse selection (this README's own claimed-drag paragraph at :265-270), so "mouse selection is unchanged" and "content under a sheet's or a pager's pan stays selectable" contradict it for a mouse. The checklist's "content drags keep selection" (:574) has the same problem.
| * For a control whose own record defines a hold action (a stepper's | ||
| * hold-repeat): pass `lockSelection` (gestures/selectionLock.ts) to turn | ||
| * text selection and the callout off for each press, where the primary | ||
| * pointer is coarse (`spec:AST-094` FR41). A control whose hold does | ||
| * nothing passes none. @default undefined |
There was a problem hiding this comment.
locksSelectionFor now locks a finger or a pen under any primary pointer (utils/selectionLock.ts:60-62), so "where the primary pointer is coarse" no longer describes when this lock applies.
| */ | ||
| retain(): () => void { | ||
| this.retainers += 1; | ||
| // A watch is not an attach: no warning where the pointer is fine. |
There was a problem hiding this comment.
This change deleted the fine-pointer warning, so this comment explains the absence of something that no longer exists; it can go.
| * @position Internal util, the one primary-click rule: the gesture host | ||
| * starts a press only for one, and the press controller paints only one; |
There was a problem hiding this comment.
utils/pressFeedback.ts never imports isPrimaryClick: its mouse path checks event.button === 0 itself (pressFeedback.ts:492), so "the press controller paints only one" describes a rule the controller does not share.
meta#8360's change on the internal touch foundation: one click guard shared by the menu press and the gesture host (its click listener stays once the menu press arms it, as its record has it, and lasts only while armed for the gesture host, FR24), the host as the one touch owner (finger scroll, page holds, attachments, the press tracker hook and a held control's kept touch, the long press's single source, presented hit testing on iOS WebKit), its cases in the gesture browser proof (Chromium and WebKit) and the README. It carries meta#8074, meta#8270, meta#8293, meta#8315, meta#8320 and meta#8356, and folds in meta#8358 (the long press's tolerance comment and an unused import). Rebased onto main after section 2 as one commit; the touch-action rule came with meta#8071. Where fingers come from their pointer events, a claimed finger's touchend is cancelled from its element (FR21), under a fine primary pointer wherever the browser sends Touch Events and inside an open shadow root too; at most eight such lifts wait for their touchend, and each goes once taken or after 500 ms, kept across the host's uninstall, which can run in the microtask after the pointerup, before the touchend that comes later in the same task (FR24). An armed press's page listeners go once the arena has decided its last touch, a tap that waits on a double tap included, and after its last detach the host holds no listener, note, frame or timer (FR24). The animator's busy proof is judged against the harness's compositor-only control (meta#8906), with a main-thread mutant as its negative, and the spring's curve is placed against the control's own clock, so the late frames a loaded runner sends fail nothing. With the host serving every pointer (meta#8071), this change's own pieces follow it: a press tracker takes a mouse press as a press, painting at once and braking no scroll (FR11); a finger's or a pen's hold that does something keeps text unselected under any primary pointer, while a mouse keeps its released behavior (FR41); and `perEventPointer`, from before FR43, changes nothing. These come from meta#8665, which keeps only the undecided rule for text selection during a mouse or pen drag. The hover recognizer and the host's hover routing land with hover's first user. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…finger landed On iOS WebKit the host resolves moving content at touch-down and asks the engine what is under the finger. When a busy main thread hands it the touch later, the content has moved on: the host now asks at the point where what was under the finger at the touch's timestamp is drawn, so a tap on a moving sheet's Done still clicks Done (spec:AST-094 FR11). A presented source reports its offset at a given time for this. The presented browser proof drives the sheet with Core's animator and a critically damped spring, up into place and down off the screen over the scrim: a tap on the drawn Done clicks it once, and a drag on the drawn sheet catches it where it is drawn, in both directions.
…the finger (FR11, FR19); INV13 names the shared input listener Where the engine hit-tests an element a compositor animation moves at the motion's start (iOS WebKit), a touch on that content goes to what was drawn under the finger at touch-down, and a tap there activates that element with one click while the browser's own click is cancelled or swallowed (FR11). While such content around a recognizer that claims moving touches is moving, the document may carry a non-passive touchmove listener that cancels only owned moves and comes off when the motion ends; while it is on, a native scroll waits for a free main thread (FR19). The gesture host, press feedback, page hold, finger-scroll watch and click guard hear document-wide input through one capture-phase listener per event type on the window (architecture:react-component-runtime INV13).
…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.
…election and never clears the page's; a drag that starts on selectable text is not claimed (spec:AST-094 FR41) FR41, as approved: a mouse or pen drag that a recognizer claims takes no new text selection for that drag and never clears an existing selection; a drag that starts on selectable text is not claimed. - The host keeps the page's selection from a mouse or pen pointerdown, read before the arena admits the press. When a recognizer claims the contact, lockDragSelection refuses a new selection on the claiming elements and the document until a task after the lift (a cancel ends it at once), and puts the kept selection back over whatever the press's collapse or the drag made of it. - At the pointerdown the host also reads whether the press lands on selectable text: a character of text whose element allows a selection, or a text field. Once such a contact moves, the arena's refusesClaim fails a recognizer that claims touches instead of letting it recognize, so the browser's selection runs. A press claimed at its own pointerdown, before it moves, and a tap's claim at the lift are unaffected. - The last detach ends a lifted drag's hold at once, so nothing outlives the host; the nothing-left test lists the three new collections. - FR41's text and Verification row state both facts; the gestures README and the host's header say what the host does.
|
Rebuilt as option A, as one commit on #8360's head a57e57e: b848270. It replaces the earlier merge branch.
|
There was a problem hiding this comment.
This pull request is on hold for a spec decision: the sentence it adds to spec:AST-094 FR41, in particular "a drag that starts on selectable text is not claimed", goes beyond the earlier owner direction and narrows FR43's every-pointer attach, so an engineering owner needs to approve the sentence and its verification row by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or ask for the clause to change. Independent of that decision, a few things need fixing:
- A drag claimed by a swipe, or by a recognizer that waited for another to fail, never gets the selection hold (
GestureHost.ts:1900-1906), so those drags still select text; please lock the claiming recognizer's element whatever its state and add cases for both. - The title describes every-pointer attach, which
mainalready has, and.changeset/drag-selection.mdannounces a[fix]for sheets and pagers that no released component runs on this host; please retitle to the FR41 change and mark the changeset internal like the other gesture changesets. - The diff still carries #8360's 16,943 lines; landing #8360 first leaves this pull request's own 972-line change.
[Automated review]
| const elements = new Set<Element>(); | ||
| for (const recognizer of this.arena.participantsOf(id)) { | ||
| if ( | ||
| (recognizer.state === 'began' || recognizer.state === 'changed') && |
There was a problem hiding this comment.
Only began/changed participants get the drag hold, and the arena reports a touch's claim once. A swipe claims as ended while the button is still down, and a recognizer waiting on another's failure claims while possible and gets no second report when it begins, so neither drag gets the hold: the selection keeps running. Could the hold lock the claiming recognizer's element whatever its state (the arena knows it in claim), with cases for a swipe and a requireToFail pan?
| (`spec:AST-017` FR59; DEC-9). | ||
| (`spec:AST-017` FR59; DEC-9). A mouse or pen drag that a recognizer claims | ||
| takes no new text selection for that drag and never clears an existing | ||
| selection; a drag that starts on selectable text is not claimed. An |
There was a problem hiding this comment.
"a drag that starts on selectable text is not claimed" goes beyond the earlier owner direction (a claimed drag suppresses selection for that drag only): under a mouse or a pen, a sheet's, pager's or swipe's drag can no longer start from text, while FR43's verification row still fails "a gesture missing under a mouse or pen". This sentence needs an engineering owner's /approve-spec at this head, or a change.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] Gestures: a mouse or pen drag that a gesture claims, such as a sheet's or a pager's pan, no longer starts a text selection or clears the page's existing one, and a drag that starts on selectable text is left to the browser, which selects that text. |
There was a problem hiding this comment.
No released component runs the gesture host yet (BottomSheet drags through its own useSheetGestures), so a [fix] naming a sheet's or a pager's pan would send consumers looking for a change they cannot see. Suggest [chore] Internal: ... Not exported from the package entry point., like the other gesture changesets.
| return winner.canPrevent(other) && other.canBePrevented(winner); | ||
| } | ||
|
|
||
| /** UIControl's rule: a control may veto an overlapping ancestor recognizer. */ |
There was a problem hiding this comment.
This is isVetoedByConsumer's doc comment; it now sits on isRefusedByBrowser above that method's own doc block. Move it down onto isVetoedByConsumer.
Depends on #8360. This pull request's own commit: vjeux/astryx@a57e57e...b848270
User impact
With a mouse or pen, a drag that a gesture takes (a sheet's, a pager's) selected the page's text on its way and cleared the person's selection. A drag that started on text moved the element instead of selecting it.
Problem and solution fit
The host turned selection off only for a held finger or pen. It now keeps the page's selection at each mouse or pen press and, once a recognizer claims the drag, refuses new selections and restores the kept one. A press on selectable text stays the browser's, so it selects.
Expected behavior and authority
Cindy approved option A.
spec:AST-094FR41 records it in her words: "A mouse or pen drag that a recognizer claims takes no new text selection for that drag and never clears an existing selection; a drag that starts on selectable text is not claimed. An unclaimed mouse drag keeps its released behavior." Its Verification row lists both facts.Smallest restoration
pointerdown, the host reads the page's selection and whether the press lands on selectable text.lockDragSelectionrefuses a selection on the claiming elements and the document and restores the kept one, until a task after the lift or a cancel.refusesClaimfails a claim on a moved mouse or pen press over selectable text; claims atpointerdownand a tap's at the lift are unaffected.Evidence
Scope
Testing
Local gates at b848270 (9 files, +956/−16):
pnpm test:ui: 710 files, 17,673 tests passed.vitest --project node, after building Core: 831 files passed; onlyrelease/cache.test.mjs's two cases fail, as on main (fix(cli): a crashed run's release install is recognized by the Node it ran on, whatever its name #8520).Writing rubric results
R1 narration: none (FR41's sentences are in the present tense). R2 research: none. R3 evidence naming a change: none (the Verification row names cases). R4 rejected alternatives: none added. R5 pull request or issue references: none. R6 open questions listing options: none (FR41's question is ruled).
Notes
GestureHost.onPointerCancelmeets fix(gestures): pointer-read fingers get the blur refusal and the pinch rule of the Touch Events path #8309 in one line (unlockSelection(id, true)beside itspointerKinds.delete); whichever merges second keeps both.