Skip to content

Commit 8369106

Browse files
airoweairowe
authored
fix(scroll): keep adb-backed Android directional swipes inside the resolved scrollable (#3746)
* fix(scroll): keep Android directional swipe endpoints inside the resolved scrollable A directional scroll centred its swipe on the window, so on an inset scrollable the drag could start on a header above the list and the command failed with scroll_no_progress. The daemon now resolves the scroll container from the current stored tree before the gesture and hands its rect to the owner. The shared planner keeps both endpoints inside the container's intersection with the padded frame: a span that fits is unchanged, one that leaves the band moves into it, and one longer than the band shrinks to it and reports `requestedPixels` beside the dispatched `pixels`. Android applies it after the keyboard clip, so the two compose. iOS plans in the runner and follows separately. Refs #2714 * fix(scroll): keep clamped swipe endpoints strictly inside the container The lane clamp treated the container's bottom edge as reachable, but Android bounds are half-open, so a shortened `scroll down` above a footer could start one pixel below the list. The usable band is now the container's half-open range inset by 8 px on both sides, on both axes. Along the scrolled axis it is intersected with the padded frame, which still wins when it leaves nothing of the container. Across it, the padded range is preferred, but a rail or chip row lying wholly in that padding is swiped along its own inset range rather than missed. A lane that only loses the rounding pixel of an odd span is no longer reported as shortened. Refs #2714
1 parent 8f3eb3d commit 8369106

8 files changed

Lines changed: 492 additions & 37 deletions

File tree

‎packages/contracts/src/scroll-command.ts‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { AppError } from '@agent-device/kernel/errors';
22
import type { SettleObservation } from './interaction.ts';
33
import type { ScrollDirection } from './scroll-gesture.ts';
4+
import type { Rect } from '@agent-device/kernel/snapshot';
45

56
export const SCROLL_DURATION_MAX_MS = 10_000;
67
export const DEFAULT_MOBILE_SCROLL_DURATION_MS = 300;
@@ -38,13 +39,24 @@ export type ScrollTimingOptions = {
3839

3940
export type ScrollCommandOptions = ScrollDistanceOptions & ScrollTimingOptions;
4041

41-
export type ScrollExecutionOptions = ScrollCommandOptions & {
42-
releaseBehavior?: ScrollReleaseBehavior;
42+
/**
43+
* The scroll container the caller resolved from the current tree, in the snapshot's absolute space.
44+
* An owner that plans its own swipe keeps both endpoints inside it (#2714); an owner without that
45+
* planner ignores it.
46+
*/
47+
export type ScrollContainerTarget = {
48+
containerRect?: Rect;
4349
};
4450

45-
export type ResolvedScrollExecutionOptions = ScrollCommandOptions & {
46-
releaseBehavior: ScrollReleaseBehavior;
47-
};
51+
export type ScrollExecutionOptions = ScrollCommandOptions &
52+
ScrollContainerTarget & {
53+
releaseBehavior?: ScrollReleaseBehavior;
54+
};
55+
56+
export type ResolvedScrollExecutionOptions = ScrollCommandOptions &
57+
ScrollContainerTarget & {
58+
releaseBehavior: ScrollReleaseBehavior;
59+
};
4860

4961
export function resolveScrollExecutionOptions(
5062
options: ScrollCommandOptions,

‎packages/contracts/src/scroll-gesture-container.test.ts‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
import { test } from 'vitest';
2+
import assert from 'node:assert/strict';
3+
import type { Rect } from '@agent-device/kernel/snapshot';
4+
import { buildScrollGesturePlan, type ScrollDirection } from './scroll-gesture.ts';
5+
6+
// The iPhone geometry #2714 was reported on: a 402x874 frame, the default 10% edge padding leaving
7+
// x 40..362 and y 87..787 for a swipe.
8+
const FRAME = { referenceWidth: 402, referenceHeight: 874 };
9+
const PADDED_LEFT = 40;
10+
const PADDED_TOP = 87;
11+
const PADDED_BOTTOM = 787;
12+
// Inset under a navigation bar and running to the bottom of the frame.
13+
const INSET_CONTAINER: Rect = { x: 18, y: 178, width: 366, height: 662 };
14+
// Inset at both ends: a footer sits below it, well above the padded band's bottom.
15+
const FOOTERED_CONTAINER: Rect = { x: 18, y: 178, width: 366, height: 500 };
16+
const FITTING_CONTAINER: Rect = { x: 18, y: 62, width: 366, height: 729 };
17+
18+
type Endpoints = { x1: number; y1: number; x2: number; y2: number };
19+
20+
/**
21+
* A container rect is half-open, as Android bounds and View hit-testing are: `y + height` is the
22+
* first row outside it. Strictly inside on both sides also keeps the drag off the edge row.
23+
*/
24+
function strictlyInside(plan: Endpoints, rect: Rect): boolean {
25+
const inside = (x: number, y: number) =>
26+
x > rect.x && x < rect.x + rect.width && y > rect.y && y < rect.y + rect.height;
27+
return inside(plan.x1, plan.y1) && inside(plan.x2, plan.y2);
28+
}
29+
30+
test.each<[ScrollDirection, string, Rect]>([
31+
['up', 'inset', INSET_CONTAINER],
32+
['down', 'inset', INSET_CONTAINER],
33+
['up', 'footered', FOOTERED_CONTAINER],
34+
['down', 'footered', FOOTERED_CONTAINER],
35+
])(
36+
'a %s swipe longer than an %s container shrinks inside it and reports both spans',
37+
(direction, _, container) => {
38+
const unclamped = buildScrollGesturePlan({ direction, amount: 0.75, ...FRAME });
39+
const plan = buildScrollGesturePlan({ direction, amount: 0.75, ...FRAME, container });
40+
41+
assert.ok(strictlyInside(plan, container), JSON.stringify(plan));
42+
assert.ok(Math.max(plan.y1, plan.y2) <= PADDED_BOTTOM, JSON.stringify(plan));
43+
assert.equal(plan.pixels, Math.abs(plan.y2 - plan.y1));
44+
assert.ok(plan.pixels < unclamped.pixels);
45+
assert.equal(plan.requestedPixels, unclamped.pixels);
46+
assert.equal(Math.sign(plan.y2 - plan.y1), Math.sign(unclamped.y2 - unclamped.y1));
47+
},
48+
);
49+
50+
test.each<ScrollDirection>(['left', 'right'])(
51+
'a %s swipe longer than a narrow container shrinks inside it along x',
52+
(direction) => {
53+
const column: Rect = { x: 100, y: 0, width: 200, height: 874 };
54+
const unclamped = buildScrollGesturePlan({ direction, ...FRAME });
55+
const plan = buildScrollGesturePlan({ direction, ...FRAME, container: column });
56+
57+
assert.ok(strictlyInside(plan, column), JSON.stringify(plan));
58+
assert.deepEqual([plan.y1, plan.y2], [unclamped.y1, unclamped.y2]);
59+
assert.equal(plan.pixels, Math.abs(plan.x2 - plan.x1));
60+
assert.ok(plan.pixels < unclamped.pixels);
61+
assert.equal(plan.requestedPixels, unclamped.pixels);
62+
assert.equal(Math.sign(plan.x2 - plan.x1), Math.sign(unclamped.x2 - unclamped.x1));
63+
},
64+
);
65+
66+
test('a horizontal swipe that fits an offset container moves into it without shortening', () => {
67+
const rightPane: Rect = { x: 180, y: 0, width: 222, height: 874 };
68+
const unclamped = buildScrollGesturePlan({ direction: 'left', pixels: 100, ...FRAME });
69+
const plan = buildScrollGesturePlan({
70+
direction: 'left',
71+
pixels: 100,
72+
...FRAME,
73+
container: rightPane,
74+
});
75+
76+
assert.ok(strictlyInside(plan, rightPane), JSON.stringify(plan));
77+
assert.equal(Math.abs(plan.x2 - plan.x1), Math.abs(unclamped.x2 - unclamped.x1));
78+
assert.equal(plan.pixels, unclamped.pixels);
79+
assert.equal('requestedPixels' in plan, false);
80+
});
81+
82+
test.each<ScrollDirection>(['up', 'down', 'left', 'right'])(
83+
'a %s swipe already inside its container keeps today’s endpoints and distance',
84+
(direction) => {
85+
const unclamped = buildScrollGesturePlan({ direction, amount: 0.75, ...FRAME });
86+
const plan = buildScrollGesturePlan({
87+
direction,
88+
amount: 0.75,
89+
...FRAME,
90+
container: FITTING_CONTAINER,
91+
});
92+
93+
assert.deepEqual(plan, unclamped);
94+
assert.equal('requestedPixels' in plan, false);
95+
},
96+
);
97+
98+
test('a swipe that fits an offset container moves into it without shortening', () => {
99+
const lowerContainer: Rect = { x: 0, y: 300, width: 402, height: 574 };
100+
const unclamped = buildScrollGesturePlan({ direction: 'down', amount: 0.5, ...FRAME });
101+
const plan = buildScrollGesturePlan({
102+
direction: 'down',
103+
amount: 0.5,
104+
...FRAME,
105+
container: lowerContainer,
106+
});
107+
108+
assert.ok(strictlyInside(plan, lowerContainer), JSON.stringify(plan));
109+
assert.equal(Math.abs(plan.y1 - plan.y2), Math.abs(unclamped.y1 - unclamped.y2));
110+
assert.equal(plan.pixels, unclamped.pixels);
111+
assert.equal('requestedPixels' in plan, false);
112+
});
113+
114+
test('an odd span that only loses its rounding pixel is not reported as shortened', () => {
115+
// On an 875-point axis the padded band is 88..787 (699 points), and a 699-point request centred
116+
// on 438 rounds its halves to a 700-point lane. Fitting it into the band drops that rounding
117+
// pixel, not any of the travel asked for.
118+
const frame = { referenceWidth: 402, referenceHeight: 875 };
119+
const wholeFrame: Rect = { x: 0, y: 0, width: 402, height: 875 };
120+
const plan = buildScrollGesturePlan({
121+
direction: 'up',
122+
pixels: 699,
123+
...frame,
124+
container: wholeFrame,
125+
});
126+
127+
assert.equal(plan.pixels, 699);
128+
assert.equal(Math.abs(plan.y2 - plan.y1), 699);
129+
assert.equal('requestedPixels' in plan, false);
130+
});
131+
132+
test('a vertical swipe beside a narrow container moves across into it', () => {
133+
const rightColumn: Rect = { x: 250, y: 0, width: 152, height: 874 };
134+
const unclamped = buildScrollGesturePlan({ direction: 'up', ...FRAME });
135+
const plan = buildScrollGesturePlan({ direction: 'up', ...FRAME, container: rightColumn });
136+
137+
assert.ok(strictlyInside(plan, rightColumn), JSON.stringify(plan));
138+
assert.ok(plan.x1 <= FRAME.referenceWidth - PADDED_LEFT, `swipe x ${plan.x1} is in the padding`);
139+
assert.equal(plan.x2, plan.x1);
140+
assert.deepEqual([plan.y1, plan.y2, plan.pixels], [unclamped.y1, unclamped.y2, unclamped.pixels]);
141+
});
142+
143+
test('a container reaching into the side padding is swiped in the part the padding leaves', () => {
144+
const edgeColumn: Rect = { x: 0, y: 0, width: 60, height: 874 };
145+
const plan = buildScrollGesturePlan({ direction: 'up', ...FRAME, container: edgeColumn });
146+
147+
assert.ok(strictlyInside(plan, edgeColumn), JSON.stringify(plan));
148+
assert.ok(plan.x1 >= PADDED_LEFT, `swipe x ${plan.x1} is inside the side padding`);
149+
});
150+
151+
test.each<[ScrollDirection, string, Rect]>([
152+
[
153+
'up',
154+
'a navigation rail along the left edge',
155+
{ x: 0, y: 0, width: PADDED_LEFT - 1, height: 874 },
156+
],
157+
[
158+
'left',
159+
'a chip row just above the padded band',
160+
{ x: 0, y: 60, width: 402, height: PADDED_TOP - 61 },
161+
],
162+
])(
163+
'a %s swipe aimed at %s runs inside it when the cross-axis padding leaves none of it',
164+
(direction, _, container) => {
165+
const unclamped = buildScrollGesturePlan({ direction, ...FRAME });
166+
const plan = buildScrollGesturePlan({ direction, ...FRAME, container });
167+
168+
assert.ok(strictlyInside(plan, container), JSON.stringify(plan));
169+
assert.equal(plan.pixels, unclamped.pixels);
170+
},
171+
);
172+
173+
test('a container lying wholly in the padding of the scrolled axis leaves the padded swipe as it was', () => {
174+
const statusBand: Rect = { x: 0, y: 0, width: 402, height: PADDED_TOP - 1 };
175+
const unclamped = buildScrollGesturePlan({ direction: 'up', ...FRAME });
176+
const plan = buildScrollGesturePlan({ direction: 'up', ...FRAME, container: statusBand });
177+
178+
assert.deepEqual(plan, unclamped);
179+
});

‎packages/contracts/src/scroll-gesture.ts‎

Lines changed: 111 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@ export type ScrollGestureOptions = {
4949
pixels?: number;
5050
referenceWidth: number;
5151
referenceHeight: number;
52+
/**
53+
* The scroll container the swipe is meant for, in the reference frame's own zero-origin space.
54+
* Both endpoints stay strictly inside its intersection with the padded frame (#2714): a span
55+
* that already fits is unchanged, one that leaves it is moved in, and one longer than it is
56+
* shortened. A container the padding leaves nothing of along the scrolled axis gets the padded
57+
* swipe unchanged, since the safe bounds outrank the container.
58+
*/
59+
container?: Rect;
5260
};
5361

5462
export type ScrollGesturePlan = {
@@ -60,7 +68,10 @@ export type ScrollGesturePlan = {
6068
referenceWidth: number;
6169
referenceHeight: number;
6270
amount?: number;
71+
/** The span dispatched, which is less than `requestedPixels` when the container shortened it. */
6372
pixels: number;
73+
/** Present only when the container shortened the swipe: the span the padded frame would carry. */
74+
requestedPixels?: number;
6475
};
6576

6677
export type SwipePresetGesturePlan = {
@@ -97,44 +108,122 @@ export const DEFAULT_SCROLL_AMOUNT = 0.6;
97108
const DEFAULT_EDGE_PADDING_FRACTION = 0.1;
98109
// Edge presets stay close to the system gesture boundary without emitting edge coordinates.
99110
const SWIPE_PRESET_EDGE_MARGIN_PX = 8;
111+
// A swipe kept inside a container stays this far off its edges, so the drag never starts on the
112+
// container's boundary row.
113+
const SCROLL_CONTAINER_EDGE_INSET = 8;
100114

101115
export function buildScrollGesturePlan(options: ScrollGestureOptions): ScrollGesturePlan {
102116
const direction = options.direction;
103-
const axisLength =
104-
direction === 'up' || direction === 'down' ? options.referenceHeight : options.referenceWidth;
117+
const vertical = direction === 'up' || direction === 'down';
118+
const axisLength = vertical ? options.referenceHeight : options.referenceWidth;
119+
const crossLength = vertical ? options.referenceWidth : options.referenceHeight;
105120
const requestedAmount = resolveRequestedAmount(options.amount);
106121
const requestedPixels =
107122
options.pixels !== undefined
108123
? normalizeRequestedPixels(options.pixels)
109124
: Math.round(axisLength * requestedAmount);
110-
const edgePadding = Math.max(1, Math.round(axisLength * DEFAULT_EDGE_PADDING_FRACTION));
125+
const edgePadding = scrollEdgePadding(axisLength);
111126
const maxTravelPixels = Math.max(1, axisLength - edgePadding * 2);
112127
const travelPixels = Math.max(1, Math.min(requestedPixels, maxTravelPixels));
113128
const halfTravel = Math.round(travelPixels / 2);
114-
const centerX = Math.round(options.referenceWidth / 2);
115-
const centerY = Math.round(options.referenceHeight / 2);
116-
const buildPlan = (x1: number, y1: number, x2: number, y2: number): ScrollGesturePlan => ({
129+
const axisCenter = Math.round(axisLength / 2);
130+
const unclamped = {
131+
axisStart: axisCenter - halfTravel,
132+
axisEnd: axisCenter + halfTravel,
133+
cross: Math.round(crossLength / 2),
134+
};
135+
const crossPadding = scrollEdgePadding(crossLength);
136+
const lane = options.container
137+
? keepSwipeInContainer(unclamped, options.container, vertical, {
138+
axis: { min: edgePadding, max: axisLength - edgePadding },
139+
cross: { min: crossPadding, max: crossLength - crossPadding },
140+
crossFrame: { min: 0, max: crossLength },
141+
})
142+
: unclamped;
143+
const laneLength = lane.axisEnd - lane.axisStart;
144+
// Centring rounds an odd span up by one, so only a lane shorter than the travel itself is short.
145+
const shortened = laneLength < travelPixels;
146+
return {
117147
direction,
118-
x1,
119-
y1,
120-
x2,
121-
y2,
148+
...laneEndpoints(direction, lane),
122149
referenceWidth: options.referenceWidth,
123150
referenceHeight: options.referenceHeight,
124151
amount: options.amount,
125-
pixels: travelPixels,
126-
});
152+
pixels: shortened ? laneLength : travelPixels,
153+
...(shortened ? { requestedPixels: travelPixels } : {}),
154+
};
155+
}
127156

128-
switch (direction) {
129-
case 'up':
130-
return buildPlan(centerX, centerY - halfTravel, centerX, centerY + halfTravel);
131-
case 'down':
132-
return buildPlan(centerX, centerY + halfTravel, centerX, centerY - halfTravel);
133-
case 'left':
134-
return buildPlan(centerX - halfTravel, centerY, centerX + halfTravel, centerY);
135-
case 'right':
136-
return buildPlan(centerX + halfTravel, centerY, centerX - halfTravel, centerY);
137-
}
157+
/** `up`/`left` drag from the lane's low end toward its high end; `down`/`right` drag the other way. */
158+
function laneEndpoints(
159+
direction: ScrollDirection,
160+
lane: SwipeLane,
161+
): Pick<ScrollGesturePlan, 'x1' | 'y1' | 'x2' | 'y2'> {
162+
const vertical = direction === 'up' || direction === 'down';
163+
const low = vertical
164+
? { x: lane.cross, y: lane.axisStart }
165+
: { x: lane.axisStart, y: lane.cross };
166+
const high = vertical ? { x: lane.cross, y: lane.axisEnd } : { x: lane.axisEnd, y: lane.cross };
167+
const [start, end] = direction === 'up' || direction === 'left' ? [low, high] : [high, low];
168+
return { x1: start.x, y1: start.y, x2: end.x, y2: end.y };
169+
}
170+
171+
type SwipeLane = { axisStart: number; axisEnd: number; cross: number };
172+
173+
type CoordinateRange = { min: number; max: number };
174+
175+
function scrollEdgePadding(length: number): number {
176+
return Math.max(1, Math.round(length * DEFAULT_EDGE_PADDING_FRACTION));
177+
}
178+
179+
/**
180+
* Moves a centred swipe lane inside the container's intersection with the padded frame,
181+
* shortening it only when that intersection is shorter than the lane. The container is half-open,
182+
* as Android bounds and View hit-testing are, and inset from its edges, so every endpoint lands on
183+
* a row the container owns. A container the padding leaves nothing of on the scrolled axis keeps
184+
* the lane the padded frame planned: the safe bounds outrank the container, and the movement read
185+
* reports the miss as `container-outside-swipe` when nothing else moved.
186+
*/
187+
function keepSwipeInContainer(
188+
lane: SwipeLane,
189+
container: Rect,
190+
vertical: boolean,
191+
padded: { axis: CoordinateRange; cross: CoordinateRange; crossFrame: CoordinateRange },
192+
): SwipeLane {
193+
const band = insetContainerRange(
194+
vertical ? container.y : container.x,
195+
vertical ? container.height : container.width,
196+
padded.axis,
197+
);
198+
const crossStart = vertical ? container.x : container.y;
199+
const crossSize = vertical ? container.width : container.height;
200+
// The padding guards the scrolled axis against system bars. Across it, a rail or chip row that
201+
// sits wholly in the padding is still the surface asked for, so the swipe runs along it there.
202+
const crossBand =
203+
insetContainerRange(crossStart, crossSize, padded.cross) ??
204+
insetContainerRange(crossStart, crossSize, padded.crossFrame);
205+
if (!band || !crossBand) return lane;
206+
207+
const cross =
208+
lane.cross >= crossBand.min && lane.cross <= crossBand.max
209+
? lane.cross
210+
: Math.round((crossBand.min + crossBand.max) / 2);
211+
const length = lane.axisEnd - lane.axisStart;
212+
if (length >= band.max - band.min) return { axisStart: band.min, axisEnd: band.max, cross };
213+
const shift =
214+
lane.axisStart < band.min ? band.min - lane.axisStart : Math.min(0, band.max - lane.axisEnd);
215+
return { axisStart: lane.axisStart + shift, axisEnd: lane.axisEnd + shift, cross };
216+
}
217+
218+
/** The coordinates of `[start, start + size)` an inset swipe may use inside `padded`, if any. */
219+
function insetContainerRange(
220+
start: number,
221+
size: number,
222+
padded: CoordinateRange,
223+
): CoordinateRange | undefined {
224+
const min = Math.max(padded.min, Math.ceil(start) + SCROLL_CONTAINER_EDGE_INSET);
225+
const max = Math.min(padded.max, Math.floor(start + size) - 1 - SCROLL_CONTAINER_EDGE_INSET);
226+
return max > min ? { min, max } : undefined;
138227
}
139228

140229
/**

0 commit comments

Comments
 (0)