Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/gesture-host-own-scroller.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@astryxdesign/core': patch
---

[fix] Gestures: a pan on its own scroller takes a trackpad's scroll on the axis its touch-action keeps, as it takes a finger's; a scroller inside it still takes the wheel first. A recognizer attached with `manageStyle: false` leaves the element's styles to its caller for that recognizer only.

@vjeux
80 changes: 70 additions & 10 deletions packages/core/src/gestures/GestureHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ import {
canElementScroll,
createNativeScrollProbe,
parentAcrossShadow,
parseTouchAction,
} from './nativeScroll';
import {containsAlongFlatTree} from '../utils/flatTree';
import {isIosWebKit} from '../utils/touchQueries';
Expand Down Expand Up @@ -214,6 +215,11 @@ interface Registration {
*/
authorTouchAction: {derived: string; author: string | null} | null;
manageStyle: boolean;
/**
* The recognizers attached with `manageStyle: false`. The host styles the
* element for its other recognizers only.
*/
readonly unstyled: Set<GestureRecognizer>;
/** An action group (`family:action-groups` FR5). */
actionGroup: boolean;
/** The attachments that asked for the action group; it lasts while one does. */
Expand Down Expand Up @@ -252,7 +258,8 @@ export interface AttachOptions {
/**
* Let the host set the element's styles: its `touch-action`, and the
* selection and callout lock a hold that does something keeps while it is
* held (`spec:AST-094` FR41). With false the caller keeps both.
* held (`spec:AST-094` FR41). With false the caller keeps both for this
* recognizer, and the host still styles the element for its others.
* @default true
*/
manageStyle?: boolean;
Expand Down Expand Up @@ -925,7 +932,16 @@ export class GestureHost {
options: AttachOptions = {},
): () => void {
const registration = this.registrationFor(element);
registration.manageStyle = options.manageStyle ?? registration.manageStyle;
// Each attachment says whether the host styles the element for its
// recognizer. The element is styled while an attached recognizer asks.
if (options.manageStyle === false) {
registration.unstyled.add(recognizer);
if (registration.recognizers.length === 0) {
registration.manageStyle = false;
}
} else {
registration.manageStyle = true;
}
// An action group while any attachment asks, whatever their order.
const asksForGroup = options.actionGroup === true;
if (asksForGroup) {
Expand Down Expand Up @@ -963,6 +979,7 @@ export class GestureHost {
if (index >= 0) {
registration.recognizers.splice(index, 1);
}
registration.unstyled.delete(recognizer);
// A recognizer whose view goes away mid-gesture is cancelled, as in
// UIKit when a view leaves the window.
if (recognizer.state !== 'possible' || recognizer.isInvolved()) {
Expand Down Expand Up @@ -1394,6 +1411,7 @@ export class GestureHost {
savedStyle: new Map(),
authorTouchAction: null,
manageStyle: true,
unstyled: new Set(),
actionGroup: false,
actionGroupAsks: 0,
};
Expand Down Expand Up @@ -1497,7 +1515,8 @@ export class GestureHost {
// A disabled recognizer leaves the browser everything (FR24).
// Every pointer gets it: a touchscreen laptop's finger as a phone's (FR43).
const active = registration.recognizers.filter(
recognizer => recognizer.isEnabled,
recognizer =>
recognizer.isEnabled && !registration.unstyled.has(recognizer),
);
let style: GestureHostStyle =
active.length > 0 ? gestureHostStyle(active) : {};
Expand Down Expand Up @@ -2191,17 +2210,22 @@ export class GestureHost {
// it). Only one that took this contact locks for it: a recognizer that
// declined it (its pointer, or its own shouldReceiveTouch) has no hold
// here.
// An element attached with manageStyle: false keeps its own styles, the
// lock included (AttachOptions.manageStyle).
const managed = (element: Element) =>
this.registry.get(element)?.manageStyle !== false;
// A recognizer attached with manageStyle: false leaves the element's
// styles to the caller, the lock included (AttachOptions.manageStyle).
const managed = (element: Element, recognizer?: GestureRecognizer) => {
const registration = this.registry.get(element);
return (
registration?.manageStyle !== false &&
(recognizer == null || registration?.unstyled.has(recognizer) !== true)
);
};
const byLock = new Map<SelectionLock, Set<Element>>();
for (const recognizer of recognizers) {
const lock = recognizer.selectionLock;
if (
lock != null &&
recognizer.view instanceof Element &&
managed(recognizer.view)
managed(recognizer.view, recognizer)
) {
const elements = byLock.get(lock) ?? new Set<Element>();
elements.add(recognizer.view);
Expand Down Expand Up @@ -2520,7 +2544,14 @@ export class GestureHost {
if (
kind === 'pan' &&
(!panTakesMove(recognizer, -dx, -dy) ||
this.scrollerTakes(target, node, dx, dy, isHorizontal) ||
this.scrollerTakes(
target,
node,
dx,
dy,
isHorizontal,
this.keepsAxis(node, isHorizontal),
) ||
// A scroll-edge pan only past every scroller's edge.
(isScrollEdgePan(recognizer) && scroll.canScroll(-dx, -dy)))
) {
Expand Down Expand Up @@ -2583,20 +2614,25 @@ export class GestureHost {
/**
* Whether a scroller between `target` and `boundary` (both included)
* scrolls this wheel now, along the wheel's major axis. It uses the edge
* test a finger's probe uses (canElementScroll).
* test a finger's probe uses (canElementScroll). With `keepsBoundary`, the
* boundary's own scroll is its recognizers', as it is for a finger.
*/
private scrollerTakes(
target: unknown,
boundary: Element,
dx: number,
dy: number,
isHorizontal: boolean,
keepsBoundary: boolean,
): boolean {
for (
let node: Node | null = target instanceof Node ? target : null;
node != null;
node = parentAcrossShadow(node)
) {
if (node === boundary && keepsBoundary) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This skips the pan's own scroller when its touch-action keeps the axis, but README.md's "Wheel and trackpad" paragraph and the allowedScrollTypes JSDoc in GestureRecognizer.ts and PanGestureRecognizer.ts still say a pan moves only after every scroller under the pointer. Please state the exception there, and that an inner scroller still goes first; the SYNC line at the top of this file names the README.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 581f5e9: the README's "Wheel and trackpad" paragraph and the allowedScrollTypes JSDoc in GestureRecognizer.ts and PanGestureRecognizer.ts state the own-scroller exception, and that an inner scroller still goes first.

break;
}
if (
node instanceof Element &&
canElementScroll(node, isHorizontal ? -dx : 0, isHorizontal ? 0 : -dy)
Expand All @@ -2610,6 +2646,30 @@ export class GestureHost {
return false;
}

/**
* Whether the touch-action the host derives for `element` keeps the
* wheel's axis from the browser. A finger's pan along that axis is then
* the element's recognizers', and so is a trackpad's scroll of the element
* itself.
*/
private keepsAxis(element: Element, isHorizontal: boolean): boolean {
const registration = this.registry.get(element);
if (registration == null || !registration.manageStyle) {
return false;
}
const active = registration.recognizers.filter(
recognizer =>
recognizer.isEnabled && !registration.unstyled.has(recognizer),
);
const touchAction =
active.length > 0 ? gestureHostStyle(active).touchAction : undefined;
if (touchAction == null) {
return false;
}
const allowed = parseTouchAction(touchAction);
return isHorizontal ? !allowed.x : !allowed.y;
}

/** A ctrl-wheel pinch's two contacts, `span` apart around its center. */
private pinchContacts(
gesture: WheelGesture,
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/gestures/GestureRecognizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,9 @@ export interface GestureRecognizerOptions<R = GestureRecognizer> {
* Scroll input this recognizer takes, as UIKit's `allowedScrollTypesMask`.
* A pan follows a trackpad's two-finger scroll (`continuous`), never a
* mouse wheel's notches. It moves as a contact with the content, along its
* own axis, after the scrollers under the pointer. A pinch takes a
* ctrl-wheel of either, as a trackpad pinches.
* own axis, after the scrollers under the pointer. Its own element is the
* pan's on the axis its `touch-action` keeps, as for a finger. A pinch takes
* a ctrl-wheel of either, as a trackpad pinches.
* @default none; a pinch takes both, the screen-edge pan none whatever a
* caller passes
*/
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/gestures/PanGestureRecognizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,9 @@ export interface PanGestureRecognizerOptions extends GestureRecognizerOptions<Pa
/**
* A trackpad's two-finger scroll (`continuous`) drives the pan where its
* owner allows it. It moves as a contact with the content, along its own
* axis, after the scrollers under the pointer. A mouse wheel's notches
* never drive a pan; they scroll what is under the pointer.
* axis, after the scrollers under the pointer. Its own element is the pan's
* on the axis its `touch-action` keeps, as for a finger. A mouse wheel's
* notches never drive a pan; they scroll what is under the pointer.
* @default [] (none)
*/
allowedScrollTypes?: ReadonlyArray<'continuous'>;
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/gestures/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,10 @@ tick over a 40 px delta. A trackpad's precise deltas, a third of their legacy
A wheel gesture drives a pan as a contact moving with the content, along the
pan's own axis, and only once every scroller between the pointer and the
pan's element is at its edge that way, decided at the gesture's first wheel:
a wheel the pan does not take is never prevented. A watcher
a wheel the pan does not take is never prevented. The pan's own element is
the exception on the axis that its derived `touch-action` keeps from the
browser: a pan on its own scroller takes that scroller's trackpad scroll, as
it takes a finger's pan there, and a scroller inside it still goes first. A watcher
(`claimsTouches: false`) never takes one, and a gesture whose first wheel
another handler already prevented, or that cannot be cancelled, stays the
page's. A scroll-edge pan (a pull to
Expand Down
31 changes: 31 additions & 0 deletions packages/core/src/gestures/__tests__/GestureHost.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3148,6 +3148,37 @@ describe('GestureHost', () => {
expect(lock).not.toHaveBeenCalled();
});

for (const holdFirst of [false, true]) {
it(`styles an element for its other recognizers when one attaches with manageStyle: false ${holdFirst ? 'before' : 'after'} them, and restores it when both leave`, () => {
detachers.push(coarsePrimaryPointer());
const {row} = mount('<div id="row">A row a pan and a catch share</div>');
const lock = vi.fn(() => () => {});
const attachHold = () =>
host.attach(
row,
new LongPressGestureRecognizer({name: 'hold', selectionLock: lock}),
{manageStyle: false},
);
const detachEarly = holdFirst ? attachHold() : null;
const detachPan = host.attach(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: these attachments sit outside detachers, so a failed assertion above the explicit detaches leaves the row attached. With main's GestureHost.ts, the two cases fail and seven unrelated listener-count cases fail with them. Pushing detachPan and detachHold onto detachers, as the neighbouring cases do, keeps a future failure to this case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. It is test-only, so it rides this pull request's next push rather than moving the stamped head.

row,
new PanGestureRecognizer({name: 'pan', direction: 'horizontal'}),
);
const detachHold = detachEarly ?? attachHold();
// The pan's touch-action, and none of the hold's styles or its lock.
expect(row.getAttribute('data-astryx-touch-action')).toBe(
'pan-y pinch-zoom',
);
const finger = {id: 1, x: 5, y: 5};
touch('touchstart', row, [finger], [finger], 0);
touch('touchend', row, [finger], [], 600);
expect(lock).not.toHaveBeenCalled();
detachHold();
detachPan();
expect(row.hasAttribute('data-astryx-touch-action')).toBe(false);
});
}

it('a long press brings lockSelection by default', () => {
expect(new LongPressGestureRecognizer().selectionLock).toBe(lockSelection);
expect(new TapGestureRecognizer().selectionLock).toBeNull();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1903,6 +1903,60 @@ test.describe('under a fine primary pointer (spec:AST-094 FR43, FR24)', () => {

test.describe('where the primary pointer is fine', () => {
test.use({hasTouch: false, isMobile: false});
test('a pan on its own scroller takes the trackpad scroll a finger would pan, a scroller inside it takes the wheel first, and a mouse wheel scrolls natively (spec:AST-094 FR43)', async ({
page,
browserName,
}) => {
await page.goto('about:blank');
await page.setContent(
'<!doctype html><meta name="viewport" content="width=device-width, initial-scale=1"><body style="margin: 0"></body>',
);
await page.addScriptTag({content: bareBundle});
await page.evaluate(() => {
const {host, PanGestureRecognizer} = window.__api;
document.body.innerHTML =
'<div id="row" style="width: 300px; overflow-x: auto; white-space: nowrap"><div id="inner" style="display: inline-block; width: 150px; overflow-x: auto"><div style="width: 600px; height: 60px">inner</div></div><div style="display: inline-block; width: 900px; height: 60px">row</div></div>';
const pan = new PanGestureRecognizer({
direction: 'horizontal',
allowedScrollTypes: ['continuous'],
});
pan.addTransitionListener((_recognizer, _from, to) =>
window.__wheelLog.push(`pan:${to}`),
);
host.attach(document.getElementById('row') as HTMLElement, pan);
});
const driver = await webInputDriver(page);
test.skip(!driver.wheels, WHEEL_SKIP);
const read = async () =>
page.evaluate(() => ({
row: (document.getElementById('row') as HTMLElement).scrollLeft,
inner: (document.getElementById('inner') as HTMLElement).scrollLeft,
log: window.__wheelLog.splice(0),
}));
const swipe = async (x: number) => {
for (let step = 0; step < 4; step += 1) {
await driver.wheel({x, y: 30, dx: 16});
}
await page.waitForTimeout(250);
return read();
};
if (browserName === 'chromium') {
// Chromium's driver wheels read as a mouse's: the row scrolls natively.
const mouse = await swipe(250);
expect(mouse.row).toBeGreaterThan(0);
expect(mouse.log).toEqual([]);
return;
}
// On the row itself, a trackpad scroll is the pan's, and the row holds.
const own = await swipe(250);
expect(own.log[0]).toBe('pan:began');
expect(own.row).toBe(0);
// Over the scroller inside it, the inner scroller scrolls first.
const inner = await swipe(50);
expect(inner.inner).toBeGreaterThan(0);
expect(inner.log).toEqual([]);
});

test('a scroll-edge pan takes a trackpad scroll only at the top of the page that scrolls it, pulling the content down; mid-page the page scrolls (spec:AST-094 FR43, FR19)', async ({
page,
browserName,
Expand Down
28 changes: 28 additions & 0 deletions packages/core/src/gestures/__tests__/wheel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,34 @@ describe('wheel and trackpad input (spec:AST-094 FR43)', () => {
expect(states[0]).toBe('began');
});

it('takes the trackpad scroll of a pan on its own scroller, whose derived touch-action keeps that axis from the browser; a scroller inside it still takes the wheel first', () => {
const {row, inner} = mount(
'<div id="row"><div id="inner"><div id="cell">cell</div></div></div>',
);
scrollableX(row, {scrollLeft: 0, scrollWidth: 600, clientWidth: 200});
const pan = new PanGestureRecognizer({
direction: 'horizontal',
allowedScrollTypes: ['continuous'],
});
const states = statesOf(pan);
detachers.push(host.attach(row, pan));
const cell = document.getElementById('cell') as HTMLElement;
// The row scrolls, but a finger's horizontal pan on it is the pan's: so
// is the trackpad's.
const taken = [0, 16, 32].map(time =>
wheel(cell, {trackpad: true, dx: 8, time}),
);
expect(taken.every(event => event.defaultPrevented)).toBe(true);
expect(states[0]).toBe('began');
clock.runDue(32 + 300);
// A scroller inside the row that can scroll that way takes the wheel.
scrollableX(inner, {scrollLeft: 0, scrollWidth: 600, clientWidth: 100});
const native = [500, 516].map(time =>
wheel(cell, {trackpad: true, dx: 8, time}),
);
expect(native.some(event => event.defaultPrevented)).toBe(false);
});

it("tells a mouse wheel's notches from a trackpad: a pan leaves lines and whole notches to the page and takes a trackpad's scroll (spec:AST-094 FR43, DEC-24)", () => {
const {smooth} = mount('<div id="smooth"></div>');
const pan = new PanGestureRecognizer({
Expand Down
Loading