Repository navigation
fix(scroll): keep adb-backed Android directional swipes inside the resolved scrollable #3746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |||||
|
|
||||||
| ## Unreleased | ||||||
|
|
||||||
| - On adb-backed Android, a directional `scroll` keeps both swipe endpoints inside the scroll container the current snapshot resolves, inset from its edges and within the 10% edge padding, so a list under a header or above a footer no longer receives a drag that starts outside it and ends in `scroll_no_progress`. A swipe that already fits is unchanged; one longer than the container is shortened, and the response reports the dispatched `pixels` beside `requestedPixels`. Horizontal scrolls, scrolls after a mutation that has not been re-snapshotted, iOS, and WebDriver (Appium) sessions keep the unclamped swipe for now (#2714). | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: This bullet overstates the guarantee. When the 10% padding leaves none of the container on the scrolled axis, the swipe keeps the padded-frame lane, so endpoints can fall outside the container. The cross-axis fallback can also leave the padding. Qualify the sentence so users don't rely on containment in those cases. Prompt for AI agents
Suggested change
|
||||||
| - Add `uninstall <app>` and `client.apps.uninstall({ app })` to remove an installed app on adb-backed Android targets, iOS simulators, and CoreDevice iOS devices. Both report `wasInstalled`. A package name or bundle id that is not installed succeeds with `wasInstalled: false`. Other targets, Appium and Limrun sessions included, refuse it before anything runs. | ||||||
| - Add `settings mobile-data on|off` for adb-backed Android targets. It switches mobile data through `svc data` and reports the `mobileData` state stored in the global `mobile_data` row; multi-SIM read-back is not covered yet. Every other target refuses it before anything runs, and `--app` is refused because the switch is device-wide. | ||||||
| - On iOS, a `fill` that runs under a degraded XCTest snapshot channel now aims at a point, including ref, selector and replay fills. When the runner identifies no text input under that point, it refuses with `NO_TEXT_INPUT_AT_POINT` (`error.details.reason: no_text_input_at_point`, `dispatched: no`) before dispatching its focus tap, and no field changes; when the input lookup itself cannot answer, it refuses with `TEXT_INPUT_PROBE_UNAVAILABLE` (`error.details.reason: text_input_probe_unavailable`). Previously such a fill cleared and typed through the app-level first responder, so it could rewrite a field the caller never named, or report a clear failure after emptying it. A ref or selector fill whose rect centre misses the real field (an outer container, a non-input node) now needs a point inside the input's own rect. | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,179 @@ | ||
| import { test } from 'vitest'; | ||
| import assert from 'node:assert/strict'; | ||
| import type { Rect } from '@agent-device/kernel/snapshot'; | ||
| import { buildScrollGesturePlan, type ScrollDirection } from './scroll-gesture.ts'; | ||
|
|
||
| // The iPhone geometry #2714 was reported on: a 402x874 frame, the default 10% edge padding leaving | ||
| // x 40..362 and y 87..787 for a swipe. | ||
| const FRAME = { referenceWidth: 402, referenceHeight: 874 }; | ||
| const PADDED_LEFT = 40; | ||
| const PADDED_TOP = 87; | ||
| const PADDED_BOTTOM = 787; | ||
| // Inset under a navigation bar and running to the bottom of the frame. | ||
| const INSET_CONTAINER: Rect = { x: 18, y: 178, width: 366, height: 662 }; | ||
| // Inset at both ends: a footer sits below it, well above the padded band's bottom. | ||
| const FOOTERED_CONTAINER: Rect = { x: 18, y: 178, width: 366, height: 500 }; | ||
| const FITTING_CONTAINER: Rect = { x: 18, y: 62, width: 366, height: 729 }; | ||
|
|
||
| type Endpoints = { x1: number; y1: number; x2: number; y2: number }; | ||
|
|
||
| /** | ||
| * A container rect is half-open, as Android bounds and View hit-testing are: `y + height` is the | ||
| * first row outside it. Strictly inside on both sides also keeps the drag off the edge row. | ||
| */ | ||
| function strictlyInside(plan: Endpoints, rect: Rect): boolean { | ||
| const inside = (x: number, y: number) => | ||
| x > rect.x && x < rect.x + rect.width && y > rect.y && y < rect.y + rect.height; | ||
| return inside(plan.x1, plan.y1) && inside(plan.x2, plan.y2); | ||
| } | ||
|
|
||
| test.each<[ScrollDirection, string, Rect]>([ | ||
| ['up', 'inset', INSET_CONTAINER], | ||
| ['down', 'inset', INSET_CONTAINER], | ||
| ['up', 'footered', FOOTERED_CONTAINER], | ||
| ['down', 'footered', FOOTERED_CONTAINER], | ||
| ])( | ||
| 'a %s swipe longer than an %s container shrinks inside it and reports both spans', | ||
| (direction, _, container) => { | ||
| const unclamped = buildScrollGesturePlan({ direction, amount: 0.75, ...FRAME }); | ||
| const plan = buildScrollGesturePlan({ direction, amount: 0.75, ...FRAME, container }); | ||
|
|
||
| assert.ok(strictlyInside(plan, container), JSON.stringify(plan)); | ||
| assert.ok(Math.max(plan.y1, plan.y2) <= PADDED_BOTTOM, JSON.stringify(plan)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: The shrink test no longer pins the 8 px inset. Prompt for AI agents |
||
| assert.equal(plan.pixels, Math.abs(plan.y2 - plan.y1)); | ||
| assert.ok(plan.pixels < unclamped.pixels); | ||
| assert.equal(plan.requestedPixels, unclamped.pixels); | ||
| assert.equal(Math.sign(plan.y2 - plan.y1), Math.sign(unclamped.y2 - unclamped.y1)); | ||
| }, | ||
| ); | ||
|
|
||
| test.each<ScrollDirection>(['left', 'right'])( | ||
| 'a %s swipe longer than a narrow container shrinks inside it along x', | ||
| (direction) => { | ||
| const column: Rect = { x: 100, y: 0, width: 200, height: 874 }; | ||
| const unclamped = buildScrollGesturePlan({ direction, ...FRAME }); | ||
| const plan = buildScrollGesturePlan({ direction, ...FRAME, container: column }); | ||
|
|
||
| assert.ok(strictlyInside(plan, column), JSON.stringify(plan)); | ||
| assert.deepEqual([plan.y1, plan.y2], [unclamped.y1, unclamped.y2]); | ||
| assert.equal(plan.pixels, Math.abs(plan.x2 - plan.x1)); | ||
| assert.ok(plan.pixels < unclamped.pixels); | ||
| assert.equal(plan.requestedPixels, unclamped.pixels); | ||
| assert.equal(Math.sign(plan.x2 - plan.x1), Math.sign(unclamped.x2 - unclamped.x1)); | ||
| }, | ||
| ); | ||
|
|
||
| test('a horizontal swipe that fits an offset container moves into it without shortening', () => { | ||
| const rightPane: Rect = { x: 180, y: 0, width: 222, height: 874 }; | ||
| const unclamped = buildScrollGesturePlan({ direction: 'left', pixels: 100, ...FRAME }); | ||
| const plan = buildScrollGesturePlan({ | ||
| direction: 'left', | ||
| pixels: 100, | ||
| ...FRAME, | ||
| container: rightPane, | ||
| }); | ||
|
|
||
| assert.ok(strictlyInside(plan, rightPane), JSON.stringify(plan)); | ||
| assert.equal(Math.abs(plan.x2 - plan.x1), Math.abs(unclamped.x2 - unclamped.x1)); | ||
| assert.equal(plan.pixels, unclamped.pixels); | ||
| assert.equal('requestedPixels' in plan, false); | ||
| }); | ||
|
|
||
| test.each<ScrollDirection>(['up', 'down', 'left', 'right'])( | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
|
||
| 'a %s swipe already inside its container keeps today’s endpoints and distance', | ||
| (direction) => { | ||
| const unclamped = buildScrollGesturePlan({ direction, amount: 0.75, ...FRAME }); | ||
| const plan = buildScrollGesturePlan({ | ||
| direction, | ||
| amount: 0.75, | ||
| ...FRAME, | ||
| container: FITTING_CONTAINER, | ||
| }); | ||
|
|
||
| assert.deepEqual(plan, unclamped); | ||
| assert.equal('requestedPixels' in plan, false); | ||
| }, | ||
| ); | ||
|
|
||
| test('a swipe that fits an offset container moves into it without shortening', () => { | ||
| const lowerContainer: Rect = { x: 0, y: 300, width: 402, height: 574 }; | ||
| const unclamped = buildScrollGesturePlan({ direction: 'down', amount: 0.5, ...FRAME }); | ||
| const plan = buildScrollGesturePlan({ | ||
| direction: 'down', | ||
| amount: 0.5, | ||
| ...FRAME, | ||
| container: lowerContainer, | ||
| }); | ||
|
|
||
| assert.ok(strictlyInside(plan, lowerContainer), JSON.stringify(plan)); | ||
| assert.equal(Math.abs(plan.y1 - plan.y2), Math.abs(unclamped.y1 - unclamped.y2)); | ||
| assert.equal(plan.pixels, unclamped.pixels); | ||
| assert.equal('requestedPixels' in plan, false); | ||
| }); | ||
|
|
||
| test('an odd span that only loses its rounding pixel is not reported as shortened', () => { | ||
| // On an 875-point axis the padded band is 88..787 (699 points), and a 699-point request centred | ||
| // on 438 rounds its halves to a 700-point lane. Fitting it into the band drops that rounding | ||
| // pixel, not any of the travel asked for. | ||
| const frame = { referenceWidth: 402, referenceHeight: 875 }; | ||
| const wholeFrame: Rect = { x: 0, y: 0, width: 402, height: 875 }; | ||
| const plan = buildScrollGesturePlan({ | ||
| direction: 'up', | ||
| pixels: 699, | ||
| ...frame, | ||
| container: wholeFrame, | ||
| }); | ||
|
|
||
| assert.equal(plan.pixels, 699); | ||
| assert.equal(Math.abs(plan.y2 - plan.y1), 699); | ||
| assert.equal('requestedPixels' in plan, false); | ||
| }); | ||
|
|
||
| test('a vertical swipe beside a narrow container moves across into it', () => { | ||
| const rightColumn: Rect = { x: 250, y: 0, width: 152, height: 874 }; | ||
| const unclamped = buildScrollGesturePlan({ direction: 'up', ...FRAME }); | ||
| const plan = buildScrollGesturePlan({ direction: 'up', ...FRAME, container: rightColumn }); | ||
|
|
||
| assert.ok(strictlyInside(plan, rightColumn), JSON.stringify(plan)); | ||
| assert.ok(plan.x1 <= FRAME.referenceWidth - PADDED_LEFT, `swipe x ${plan.x1} is in the padding`); | ||
| assert.equal(plan.x2, plan.x1); | ||
| assert.deepEqual([plan.y1, plan.y2, plan.pixels], [unclamped.y1, unclamped.y2, unclamped.pixels]); | ||
| }); | ||
|
|
||
| test('a container reaching into the side padding is swiped in the part the padding leaves', () => { | ||
| const edgeColumn: Rect = { x: 0, y: 0, width: 60, height: 874 }; | ||
| const plan = buildScrollGesturePlan({ direction: 'up', ...FRAME, container: edgeColumn }); | ||
|
|
||
| assert.ok(strictlyInside(plan, edgeColumn), JSON.stringify(plan)); | ||
| assert.ok(plan.x1 >= PADDED_LEFT, `swipe x ${plan.x1} is inside the side padding`); | ||
| }); | ||
|
|
||
| test.each<[ScrollDirection, string, Rect]>([ | ||
| [ | ||
| 'up', | ||
| 'a navigation rail along the left edge', | ||
| { x: 0, y: 0, width: PADDED_LEFT - 1, height: 874 }, | ||
| ], | ||
| [ | ||
| 'left', | ||
| 'a chip row just above the padded band', | ||
| { x: 0, y: 60, width: 402, height: PADDED_TOP - 61 }, | ||
| ], | ||
| ])( | ||
| 'a %s swipe aimed at %s runs inside it when the cross-axis padding leaves none of it', | ||
| (direction, _, container) => { | ||
| const unclamped = buildScrollGesturePlan({ direction, ...FRAME }); | ||
| const plan = buildScrollGesturePlan({ direction, ...FRAME, container }); | ||
|
|
||
| assert.ok(strictlyInside(plan, container), JSON.stringify(plan)); | ||
| assert.equal(plan.pixels, unclamped.pixels); | ||
| }, | ||
| ); | ||
|
|
||
| test('a container lying wholly in the padding of the scrolled axis leaves the padded swipe as it was', () => { | ||
| const statusBand: Rect = { x: 0, y: 0, width: 402, height: PADDED_TOP - 1 }; | ||
| const unclamped = buildScrollGesturePlan({ direction: 'up', ...FRAME }); | ||
| const plan = buildScrollGesturePlan({ direction: 'up', ...FRAME, container: statusBand }); | ||
|
|
||
| assert.deepEqual(plan, unclamped); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3:
scroll_no_progressis documented as a consequence of endpoint placement alone. State that it requires an unchanged surface, an inside-container swipe, and hidden content remaining in that direction.Prompt for AI agents