fix!: scrollToElement succeeds only when the element is in the viewport

CHANGELOG.md

@@ -9,10 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- `ScrollToElementError` (`ELEMENT_NOT_FOUND` / `ELEMENT_NOT_IN_VIEWPORT`, with `swipes` and the last `frame`) and the `ScrollDirection` / `ScrollToElementResult` types ([#50](https://github.com/MetaMask/device-mcp/pull/50))
 - `tapElement(query, { element })` taps an element the caller already resolved (for example from `waitForElement`) without taking a second snapshot
 
 ### Changed
 
+- **BREAKING:** fix: `scrollToElement` succeeds only when the element is in the viewport ([#50](https://github.com/MetaMask/device-mcp/pull/50))
+  - It returns `{ element, swipes, inViewport: true }` instead of the element. Before, it returned as soon as the element was in the UI tree; React Native keeps off-screen rows in the tree, so it often returned after zero swipes with the element below the fold.
+  - After a swipe it reads the screen until it stops moving. It fails with `ScrollToElementError` after `maxAttempts` swipes, or when the screen stops changing.
+  - New default `direction: 'auto'`: the finger direction comes from the element's position (searching toward content below first while it is not in the tree). Explicit `up` / `down` stay finger directions. Before, the default `down` revealed content above.
+  - Android and Appium swipe from the middle of the real screen (40% of its height), not from fixed coordinates.
 - **BREAKING:** fix: iOS key presses, `back` and text clearing ([#48](https://github.com/MetaMask/device-mcp/pull/48))
   - iOS `pressButton('enter')` and `dismissKeyboard()` send the integer HID key code (`idb ui key 40`); `home` and `lock` use `idb ui button`. Both failed before because `idb ui key` takes only integer codes.
   - iOS `pressButton('back')` now throws `UNSUPPORTED_ON_IOS` instead of pressing Home (which put the app in the background).

README.md

@@ -176,15 +176,16 @@ The `.device-session` file is typically written by the test runner when it creat
 
 ### Interaction
 
-| Tool                     | Description                                                        |
-| ------------------------ | ------------------------------------------------------------------ |
-| `device_tap_element`     | Find an element by label/identifier/text/type and tap its center.  |
-| `device_tap_coordinates` | Tap at exact screen coordinates. Last resort when queries fail.    |
-| `device_type`            | Type text into the currently focused input field.                  |
-| `device_swipe`           | Swipe in a direction with optional start coordinates and distance. |
-| `device_long_press`      | Long press an element for context menus or drag initiation.        |
-| `device_wait_for`        | Poll until an element matching a query appears.                    |
-| `device_press_button`    | Press a device button (home/back/enter/lock; no back on iOS).      |
+| Tool                       | Description                                                                                                                                                                                                              |
+| -------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
+| `device_tap_element`       | Find an element by label/identifier/text/type and tap its center.                                                                                                                                                        |
+| `device_tap_coordinates`   | Tap at exact screen coordinates. Last resort when queries fail.                                                                                                                                                          |
+| `device_type`              | Type text into the currently focused input field.                                                                                                                                                                        |
+| `device_swipe`             | Swipe in a direction with optional start coordinates and distance.                                                                                                                                                       |
+| `device_long_press`        | Long press an element for context menus or drag initiation.                                                                                                                                                              |
+| `device_scroll_to_element` | Swipe until an element is inside the viewport (not only in the UI tree); reports the swipe count. `direction` is the finger direction (`up` reveals content below); default `auto` picks it from the element's position. |
+| `device_wait_for`          | Poll until an element matching a query appears.                                                                                                                                                                          |
+| `device_press_button`      | Press a device button (home/back/enter/lock; no back on iOS).                                                                                                                                                            |
 
 ### App & Device Control
 

src/backends/adb-backend.ts

@@ -43,6 +43,8 @@ import type {
   ScreenshotFileResult,
   ScreenshotOptions,
   ScreenshotResult,
+  ScrollDirection,
+  ScrollToElementResult,
   SnapshotResult,
   TapElementOptions,
   TapResult,
@@ -63,6 +65,7 @@ import {
   hardenArtifactFile,
   resolveArtifactPath,
 } from '../utils/output-path.js';
+import { scrollUntilInViewport } from '../utils/scroll.js';
 
 // `uiautomator dump` exits 0 and prints its success banner even when it fails, so
 // the XML payload is the only trustworthy signal.
@@ -900,26 +903,40 @@ export class AdbBackend implements DeviceBackend {
 
   async scrollToElement(
     query: ElementQuery,
-    direction: 'up' | 'down' = 'down',
+    direction: ScrollDirection = 'auto',
     maxAttempts = 10,
-  ): Promise<UIElement> {
-    let previousRaw = '';
+  ): Promise<ScrollToElementResult> {
+    const windowSize = await this.getWindowSize();
+    return scrollUntilInViewport(
+      {
+        snapshot: async () => this.snapshot(),
+        windowSize,
+        swipe: async (finger) => this.#scrollSwipe(finger, windowSize),
+      },
+      query,
+      direction,
+      maxAttempts,
+    );
+  }
 
-    for (let i = 0; i < maxAttempts; i++) {
-      const snap = await this.snapshot();
-      const element = findElement(snap.hierarchy, query);
-      if (element) {
-        return element;
-      }
-      if (snap.raw === previousRaw) {
-        break;
-      }
-      previousRaw = snap.raw;
-      await this.swipe(direction);
-    }
-    throw new Error(
-      `Element not found after scrolling: ${JSON.stringify(query)}\n` +
-        'Use device_snapshot to inspect the current UI hierarchy.',
+  /**
+   * One scroll gesture from the middle of the screen over 40% of its height
+   * (the iOS backend's default), so the swipe matches the real screen size.
+   *
+   * @param finger - Finger direction.
+   * @param windowSize - The screen size, in the frame units.
+   * @param windowSize.width - Viewport width.
+   * @param windowSize.height - Viewport height.
+   */
+  async #scrollSwipe(
+    finger: 'up' | 'down',
+    { width, height }: WindowSize,
+  ): Promise<void> {
+    await this.swipe(
+      finger,
+      Math.round(width / 2),
+      Math.round(height / 2),
+      Math.round(height * 0.4),
     );
   }
 

src/backends/appium-backend.ts

@@ -10,6 +10,8 @@ import type {
   ScreenshotFileResult,
   ScreenshotOptions,
   ScreenshotResult,
+  ScrollDirection,
+  ScrollToElementResult,
   SnapshotResult,
   TapElementOptions,
   TapResult,
@@ -26,6 +28,7 @@ import {
   resolveArtifactPath,
   writeArtifactFile,
 } from '../utils/output-path.js';
+import { scrollUntilInViewport } from '../utils/scroll.js';
 
 export class AppiumBackend implements DeviceBackend {
   readonly kind = 'appium' as const;
@@ -425,26 +428,40 @@ export class AppiumBackend implements DeviceBackend {
 
   async scrollToElement(
     query: ElementQuery,
-    direction: 'up' | 'down' = 'down',
+    direction: ScrollDirection = 'auto',
     maxAttempts = 10,
-  ): Promise<UIElement> {
-    let previousRaw = '';
+  ): Promise<ScrollToElementResult> {
+    const windowSize = await this.getWindowSize();
+    return scrollUntilInViewport(
+      {
+        snapshot: async () => this.snapshot(),
+        windowSize,
+        swipe: async (finger) => this.#scrollSwipe(finger, windowSize),
+      },
+      query,
+      direction,
+      maxAttempts,
+    );
+  }
 
-    for (let i = 0; i < maxAttempts; i++) {
-      const snap = await this.snapshot();
-      const element = findElement(snap.hierarchy, query);
-      if (element) {
-        return element;
-      }
-      if (snap.raw === previousRaw) {
-        break;
-      }
-      previousRaw = snap.raw;
-      await this.swipe(direction);
-    }
-    throw new Error(
-      `Element not found after scrolling: ${JSON.stringify(query)}\n` +
-        'Use device_snapshot to inspect the current UI hierarchy.',
+  /**
+   * One scroll gesture from the middle of the screen over 40% of its height
+   * (the iOS backend's default), so the swipe matches the real screen size.
+   *
+   * @param finger - Finger direction.
+   * @param windowSize - The screen size, in the frame units.
+   * @param windowSize.width - Viewport width.
+   * @param windowSize.height - Viewport height.
+   */
+  async #scrollSwipe(
+    finger: 'up' | 'down',
+    { width, height }: WindowSize,
+  ): Promise<void> {
+    await this.swipe(
+      finger,
+      Math.round(width / 2),
+      Math.round(height / 2),
+      Math.round(height * 0.4),
     );
   }
 

src/backends/idb-backend.test.ts

@@ -701,9 +701,9 @@ describe('IdbBackend.swipe and getWindowSize', () => {
         snapshotCount += 1;
         return JSON.stringify([
           {
-            type: snapshotCount === 3 ? 'Button' : 'StaticText',
-            AXLabel: snapshotCount === 3 ? 'Settings' : `Page ${snapshotCount}`,
-            frame: { x: 10, y: 20, width: 100, height: 44 },
+            type: snapshotCount >= 3 ? 'Button' : 'StaticText',
+            AXLabel: snapshotCount >= 3 ? 'Settings' : `Page ${snapshotCount}`,
+            frame: { x: 10, y: 200, width: 100, height: 44 },
             enabled: true,
           },
         ]);

src/backends/idb-backend.ts

@@ -14,6 +14,8 @@ import type {
   ScreenshotFileResult,
   ScreenshotOptions,
   ScreenshotResult,
+  ScrollDirection,
+  ScrollToElementResult,
   SnapshotResult,
   TapElementOptions,
   TapResult,
@@ -38,6 +40,7 @@ import {
   idbSupportsNestedDescribe,
   resolveIdbPath,
 } from '../utils/platform.js';
+import { scrollUntilInViewport } from '../utils/scroll.js';
 import { stopChildProcess } from '../utils/stop-child-process.js';
 
 /**
@@ -909,28 +912,20 @@ export class IdbBackend implements DeviceBackend {
 
   async scrollToElement(
     query: ElementQuery,
-    direction: 'up' | 'down' = 'down',
+    direction: ScrollDirection = 'auto',
     maxAttempts = 10,
-  ): Promise<UIElement> {
+  ): Promise<ScrollToElementResult> {
     await this.ensureConnected();
     const windowSize = await this.getWindowSize();
-    let previousRaw = '';
-
-    for (let i = 0; i < maxAttempts; i++) {
-      const snap = await this.snapshot();
-      const element = findElement(snap.hierarchy, query);
-      if (element) {
-        return element;
-      }
-      if (snap.raw === previousRaw) {
-        break;
-      }
-      previousRaw = snap.raw;
-      await this.#swipe(direction, windowSize);
-    }
-    throw new Error(
-      `Element not found after scrolling: ${JSON.stringify(query)}\n` +
-        'Use device_snapshot to inspect the current UI hierarchy.',
+    return scrollUntilInViewport(
+      {
+        snapshot: async () => this.snapshot(),
+        windowSize,
+        swipe: async (finger) => this.#swipe(finger, windowSize),
+      },
+      query,
+      direction,
+      maxAttempts,
     );
   }
 

src/backends/index.ts

@@ -15,6 +15,8 @@ import type {
   ScreenshotFileResult,
   ScreenshotOptions,
   ScreenshotResult,
+  ScrollDirection,
+  ScrollToElementResult,
   SnapshotResult,
   TapElementOptions,
   TapResult,
@@ -304,9 +306,9 @@ export function createLazyBackend(
 
     async scrollToElement(
       query: ElementQuery,
-      direction?: 'up' | 'down',
+      direction?: ScrollDirection,
       maxAttempts?: number,
-    ): Promise<UIElement> {
+    ): Promise<ScrollToElementResult> {
       return (await resolve()).scrollToElement(query, direction, maxAttempts);
     },
 

src/backends/types.ts

@@ -100,6 +100,17 @@ export type ElementQuery = {
 
 export type DeviceButton = 'home' | 'back' | 'enter' | 'lock';
 
+/** Finger direction of a scroll gesture, or `auto` (from the target's position). */
+export type ScrollDirection = 'up' | 'down' | 'auto';
+
+export type ScrollToElementResult = {
+  /** The element as last read, with its in-view frame. */
+  element: UIElement;
+  /** Swipes performed (0 when the element was already in view). */
+  swipes: number;
+  inViewport: true;
+};
+
 export type WindowSize = {
   width: number;
   height: number;
@@ -191,11 +202,23 @@ export type DeviceBackend = {
 
   longPress(query: ElementQuery, durationMs?: number): Promise<TapResult>;
 
+  /**
+   * Swipe until the element is in the viewport (not only in the UI tree).
+   *
+   * @param query - The element to bring into view.
+   * @param direction - Finger direction (`up` reveals content below), or
+   * `auto` (default): from the element's position, searching downward first
+   * when it is not in the tree yet.
+   * @param maxAttempts - Maximum number of swipes (default 10).
+   * @returns The element as last read and the number of swipes.
+   * @throws {ScrollToElementError} `ELEMENT_NOT_FOUND` or
+   * `ELEMENT_NOT_IN_VIEWPORT` (with the last frame).
+   */
   scrollToElement(
     query: ElementQuery,
-    direction?: 'up' | 'down',
+    direction?: ScrollDirection,
     maxAttempts?: number,
-  ): Promise<UIElement>;
+  ): Promise<ScrollToElementResult>;
 
   getAlertText(): Promise<string>;
 

src/index.ts

@@ -9,6 +9,8 @@ export { createMcpServer } from './server.js';
 
 export { detectAllDevices, MultipleDevicesError } from './utils/platform.js';
 export { IdbUnavailableError } from './backends/idb-errors.js';
+export { ScrollToElementError } from './utils/scroll.js';
+export type { ScrollToElementErrorCode } from './utils/scroll.js';
 export type { IdbErrorCode } from './backends/idb-errors.js';
 export type { DetectedDevice } from './utils/platform.js';
 
@@ -23,6 +25,8 @@ export type {
   ScreenshotResult,
   ScreenshotFileResult,
   ScreenshotOptions,
+  ScrollDirection,
+  ScrollToElementResult,
   LogsResult,
   TapElementOptions,
   TapResult,

src/tools/scroll-to-element.ts

@@ -13,8 +13,10 @@ export function registerScrollToElementTool(
     {
       title: 'Scroll To Element',
       description:
-        'Scroll the screen until an element matching the query becomes visible. ' +
-        'Combines scrolling and element search into one atomic operation. ' +
+        'Swipe until an element matching the query is inside the viewport ' +
+        '(being in the UI tree is not enough), and report how many swipes ' +
+        'it took. Fails when the element is not in view after maxAttempts ' +
+        'swipes or the screen stops moving. ' +
         'Specify at least one of: label, identifier, text, type.',
       inputSchema: {
         label: z.string().optional().describe('Accessibility label to match'),
@@ -25,18 +27,22 @@ export function registerScrollToElementTool(
         text: z.string().optional().describe('Text content to match'),
         type: z.string().optional().describe('Element type to match'),
         direction: z
-          .enum(['up', 'down'])
+          .enum(['auto', 'up', 'down'])
           .optional()
-          .describe('Scroll direction (default: down)'),
+          .describe(
+            'Finger direction: "up" reveals content below, "down" content ' +
+              'above. Default "auto": from where the element is, searching ' +
+              'downward first when it is not in the UI tree yet.',
+          ),
         maxAttempts: z
           .number()
           .optional()
-          .describe('Maximum scroll attempts (default: 10)'),
+          .describe('Maximum number of swipes (default: 10)'),
       },
     },
     async ({ label, identifier, text, type, direction, maxAttempts }) => {
       try {
-        const element = await backend.scrollToElement(
+        const { element, swipes } = await backend.scrollToElement(
           { label, identifier, text, type },
           direction,
           maxAttempts,
@@ -46,7 +52,7 @@ export function registerScrollToElementTool(
             {
               type: 'text' as const,
               text: [
-                `Found element after scrolling:`,
+                `Element in viewport after ${swipes} swipe(s):`,
                 `  type: ${element.type}`,
                 element.label ? `  label: "${element.label}"` : null,
                 element.identifier ? `  id: "${element.identifier}"` : null,