Skip to content

Commit 41d6bd9

Browse files
Lykhoydaclaude
andauthored
fix(story-06): union-bbox screen rect so Android direction scroll actually moves the list (#387) (#488)
Device-proven on CI: with amount 1 the Android golden-set list stayed on rows 1-13 after 30 scrolls. The scroll gesture used a 52px drag at y=38-90 because the screen rect resolved to a 1080x128 top-chrome node — no node in that snapshot spanned the full window, so the largest-(0,0)-anchored heuristic picked the chrome strip. Compute the screen rect as the union bounding box of all node rects (max x+width, max y+height) instead; iOS is unchanged (its Application node already spans full-screen). Applied to both updateRefMap (Android) and updateRefMapFromFlat (iOS XCUITree). Unit-tested against the exact CI-failing snapshot shape. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 4b12a33 commit 41d6bd9

4 files changed

Lines changed: 89 additions & 39 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'rn-dev-agent-plugin': patch
3+
---
4+
5+
Fix direction `device_scroll`/`device_swipe` computing a no-op gesture on Android when no snapshot node spans the full window. The screen rect (used to size direction gestures) was picked as the largest `(0,0)`-anchored node; on some Android snapshots that is a ~128px top-chrome strip while the scrollable content sits below it, so scrolls dragged ~50px in the status bar and never moved the list. The screen rect is now the union bounding box of all node rects (max extent), recovering the true viewport on both platforms.

scripts/cdp-bridge/dist/fast-runner-ref-map.js

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,28 @@ let lastSnapshotHash = null;
77
export function updateRefMap(nodes) {
88
refMap.clear();
99
screenRect = null;
10+
// Screen rect = the union bounding box of all node rects. A (0,0)-anchored
11+
// heuristic is fragile: on some Android snapshots (#387 Phase B device-proven)
12+
// NO node spans the full window — the tallest (0,0) node is a ~128px top-chrome
13+
// strip, so a "largest (0,0) rect" pick yields a tiny viewport and direction
14+
// scrolls compute a ~50px drag in the status bar that never moves the list.
15+
// The max extent across all nodes recovers the true viewport on both platforms.
16+
let maxRight = 0;
17+
let maxBottom = 0;
1018
for (const node of nodes) {
1119
if (!node.ref || !node.rect)
1220
continue;
1321
refMap.set(node.ref, node.rect);
14-
// Largest (0,0)-anchored rect wins: with interactive windows in the
15-
// Android snapshot (#370), the status bar (0,0,w,~156) precedes the app
16-
// window, and first-match sent direction gestures into the status bar
17-
// (#387 Phase B device-proven).
18-
if (node.rect.x === 0 && node.rect.y === 0 && node.rect.width > 300) {
19-
if (!screenRect ||
20-
node.rect.width * node.rect.height > screenRect.width * screenRect.height) {
21-
screenRect = node.rect;
22-
}
22+
const { x, y, width, height } = node.rect;
23+
if (width > 0 && height > 0) {
24+
maxRight = Math.max(maxRight, x + width);
25+
maxBottom = Math.max(maxBottom, y + height);
2326
}
2427
}
28+
// width > 300 keeps the same sanity floor the old heuristic used (ignore
29+
// degenerate all-tiny-node snapshots rather than emit a bogus rect).
30+
screenRect =
31+
maxRight > 300 && maxBottom > 0 ? { x: 0, y: 0, width: maxRight, height: maxBottom } : null;
2532
lastUpdated = Date.now();
2633
}
2734
export function lookupRef(ref) {
@@ -113,6 +120,11 @@ export function updateRefMapFromFlat(nodes) {
113120
refMap.clear();
114121
screenRect = null;
115122
const hashed = [];
123+
// Screen rect = union bounding box of all node rects (same rationale as
124+
// updateRefMap — a (0,0)-anchored pick is fragile when no node spans the
125+
// full window).
126+
let maxRight = 0;
127+
let maxBottom = 0;
116128
for (let i = 0; i < nodes.length; i++) {
117129
const node = nodes[i];
118130
if (!node.ref || !node.rect)
@@ -126,10 +138,14 @@ export function updateRefMapFromFlat(nodes) {
126138
meta.identifier = node.identifier;
127139
metadataMap.set(key, meta);
128140
hashed.push(node);
129-
if (!screenRect && node.rect.x === 0 && node.rect.y === 0 && node.rect.width > 300) {
130-
screenRect = node.rect;
141+
const { x, y, width, height } = node.rect;
142+
if (width > 0 && height > 0) {
143+
maxRight = Math.max(maxRight, x + width);
144+
maxBottom = Math.max(maxBottom, y + height);
131145
}
132146
}
147+
screenRect =
148+
maxRight > 300 && maxBottom > 0 ? { x: 0, y: 0, width: maxRight, height: maxBottom } : null;
133149
// Hash only the nodes that passed the ref/rect filter: hashSnapshotNodes
134150
// dereferences node.rect.* unconditionally, and a malformed entry must not
135151
// throw mid-update. For real runner data the filtered subset equals the full

scripts/cdp-bridge/src/fast-runner-ref-map.ts

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -66,23 +66,28 @@ export function updateRefMap(nodes: SnapshotNode[]): void {
6666
refMap.clear();
6767
screenRect = null;
6868

69+
// Screen rect = the union bounding box of all node rects. A (0,0)-anchored
70+
// heuristic is fragile: on some Android snapshots (#387 Phase B device-proven)
71+
// NO node spans the full window — the tallest (0,0) node is a ~128px top-chrome
72+
// strip, so a "largest (0,0) rect" pick yields a tiny viewport and direction
73+
// scrolls compute a ~50px drag in the status bar that never moves the list.
74+
// The max extent across all nodes recovers the true viewport on both platforms.
75+
let maxRight = 0;
76+
let maxBottom = 0;
6977
for (const node of nodes) {
7078
if (!node.ref || !node.rect) continue;
7179
refMap.set(node.ref, node.rect);
7280

73-
// Largest (0,0)-anchored rect wins: with interactive windows in the
74-
// Android snapshot (#370), the status bar (0,0,w,~156) precedes the app
75-
// window, and first-match sent direction gestures into the status bar
76-
// (#387 Phase B device-proven).
77-
if (node.rect.x === 0 && node.rect.y === 0 && node.rect.width > 300) {
78-
if (
79-
!screenRect ||
80-
node.rect.width * node.rect.height > screenRect.width * screenRect.height
81-
) {
82-
screenRect = node.rect;
83-
}
81+
const { x, y, width, height } = node.rect;
82+
if (width > 0 && height > 0) {
83+
maxRight = Math.max(maxRight, x + width);
84+
maxBottom = Math.max(maxBottom, y + height);
8485
}
8586
}
87+
// width > 300 keeps the same sanity floor the old heuristic used (ignore
88+
// degenerate all-tiny-node snapshots rather than emit a bogus rect).
89+
screenRect =
90+
maxRight > 300 && maxBottom > 0 ? { x: 0, y: 0, width: maxRight, height: maxBottom } : null;
8691

8792
lastUpdated = Date.now();
8893
}
@@ -184,6 +189,11 @@ export function updateRefMapFromFlat(nodes: FlatNode[]): void {
184189
screenRect = null;
185190

186191
const hashed: FlatNode[] = [];
192+
// Screen rect = union bounding box of all node rects (same rationale as
193+
// updateRefMap — a (0,0)-anchored pick is fragile when no node spans the
194+
// full window).
195+
let maxRight = 0;
196+
let maxBottom = 0;
187197
for (let i = 0; i < nodes.length; i++) {
188198
const node = nodes[i];
189199
if (!node.ref || !node.rect) continue;
@@ -196,10 +206,14 @@ export function updateRefMapFromFlat(nodes: FlatNode[]): void {
196206
metadataMap.set(key, meta);
197207
hashed.push(node);
198208

199-
if (!screenRect && node.rect.x === 0 && node.rect.y === 0 && node.rect.width > 300) {
200-
screenRect = node.rect;
209+
const { x, y, width, height } = node.rect;
210+
if (width > 0 && height > 0) {
211+
maxRight = Math.max(maxRight, x + width);
212+
maxBottom = Math.max(maxBottom, y + height);
201213
}
202214
}
215+
screenRect =
216+
maxRight > 300 && maxBottom > 0 ? { x: 0, y: 0, width: maxRight, height: maxBottom } : null;
203217

204218
// Hash only the nodes that passed the ref/rect filter: hashSnapshotNodes
205219
// dereferences node.rect.* unconditionally, and a malformed entry must not
Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,45 @@
1-
// Story 06 Phase B (#387): with FLAG_RETRIEVE_INTERACTIVE_WINDOWS (#370),
2-
// Android snapshots include SYSTEM windows, and the status bar
3-
// (0,0,1280,156) precedes the app window in node order. updateRefMap's
4-
// screen-rect heuristic took the FIRST (0,0)-anchored wide node, so
5-
// direction-based device_scroll/device_swipe computed gestures inside the
6-
// status bar (device-proven: a 62px drag at y 47-109 opened the shade and
7-
// knocked the fixture out of the foreground). The heuristic must pick the
8-
// LARGEST full-bleed rect.
1+
// Story 06 Phase B (#387): the screen rect that direction scroll/swipe compute
2+
// their gesture from is the UNION BOUNDING BOX of all snapshot node rects. A
3+
// (0,0)-anchored heuristic was fragile — device-proven on CI Android, where NO
4+
// node spans the full window (the tallest (0,0) node is a ~128px top-chrome
5+
// strip while the scrollable list sits at y=570,h=1590), so a "largest (0,0)
6+
// rect" pick produced a 128px-tall viewport and every direction scroll dragged
7+
// ~50px in the status bar, never moving the list.
98
import { test } from 'node:test';
109
import assert from 'node:assert/strict';
1110
import { updateRefMap, getScreenRect } from '../../dist/fast-runner-ref-map.js';
1211

13-
test('screen rect: largest (0,0)-anchored rect wins over a leading status-bar window', () => {
12+
test('screen rect: CI-Android shape — no full-window (0,0) node, list below the chrome', () => {
13+
// Exactly the failing snapshot shape (app=0,0,1080,128; list at y=570,h=1590).
1414
updateRefMap([
15-
{ ref: 'e0', rect: { x: 0, y: 0, width: 1280, height: 156 } },
16-
{ ref: 'e1', rect: { x: 0, y: 0, width: 1280, height: 156 } },
17-
{ ref: 'e2', rect: { x: 0, y: 0, width: 1280, height: 2856 } },
18-
{ ref: 'e3', rect: { x: 24, y: 24, width: 264, height: 144 } },
15+
{ ref: 'e0', rect: { x: 0, y: 0, width: 1080, height: 128 } },
16+
{ ref: 'e1', rect: { x: 0, y: 570, width: 1080, height: 1590 } },
17+
{ ref: 'e2', rect: { x: 16, y: 2170, width: 300, height: 60 } },
1918
] as never);
20-
assert.deepEqual(getScreenRect(), { x: 0, y: 0, width: 1280, height: 2856 });
19+
// Union bbox height reaches the bottom-most node (2170+60), NOT the 128 chrome.
20+
assert.deepEqual(getScreenRect(), { x: 0, y: 0, width: 1080, height: 2230 });
2121
});
2222

23-
test('screen rect: unchanged behavior when the app window comes first (iOS shape)', () => {
23+
test('screen rect: iOS shape — full-window Application node sets the extent', () => {
2424
updateRefMap([
2525
{ ref: 'e0', rect: { x: 0, y: 0, width: 402, height: 874 } },
2626
{ ref: 'e1', rect: { x: 0, y: 0, width: 402, height: 874 } },
2727
{ ref: 'e2', rect: { x: 16, y: 134, width: 370, height: 34 } },
2828
] as never);
2929
assert.deepEqual(getScreenRect(), { x: 0, y: 0, width: 402, height: 874 });
3030
});
31+
32+
test('screen rect: a leading status-bar node does not cap the extent', () => {
33+
updateRefMap([
34+
{ ref: 'e0', rect: { x: 0, y: 0, width: 1280, height: 156 } },
35+
{ ref: 'e1', rect: { x: 0, y: 0, width: 1280, height: 156 } },
36+
{ ref: 'e2', rect: { x: 0, y: 0, width: 1280, height: 2856 } },
37+
{ ref: 'e3', rect: { x: 24, y: 24, width: 264, height: 144 } },
38+
] as never);
39+
assert.deepEqual(getScreenRect(), { x: 0, y: 0, width: 1280, height: 2856 });
40+
});
41+
42+
test('screen rect: degenerate all-tiny-node snapshot yields null (sanity floor)', () => {
43+
updateRefMap([{ ref: 'e0', rect: { x: 0, y: 0, width: 40, height: 40 } }] as never);
44+
assert.equal(getScreenRect(), null);
45+
});

0 commit comments

Comments
 (0)