Skip to content

Commit dc8f367

Browse files
committed
fix(daemon): a surface entry carries a state-free content key, so an anonymous toggle is not movement
Review follow-up. The within-container movement check fell back to `key` for an entry without an identity, and `key` carries the checked state, so an unlabelled switch a swipe brushed still read as content moving. Every entry now carries `content`: the identity where the node has one, else its type and role; the check compares on that alone.
1 parent f1f571f commit dc8f367

4 files changed

Lines changed: 49 additions & 26 deletions

File tree

‎src/daemon/__tests__/interaction-outcome-policy.test.ts‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -478,22 +478,30 @@ test('classifyInteractionSurfaceChange reads a checked-only flip as a change', (
478478
assert.equal(classifyInteractionSurfaceChange(before, after), 'changed');
479479
});
480480

481-
test('discriminatingSurfaceChangedWithinRect reads a flip at the same rect as no movement, and a moved row as movement', () => {
482-
const rect = { x: 0, y: 0, width: 390, height: 844 };
483-
const signature = (checked: boolean, y?: number) =>
484-
buildInteractionSurfaceSignature(makeToggleSnapshot(checked, y).nodes);
485-
486-
assert.equal(
487-
discriminatingSurfaceChangedWithinRect(signature(false), signature(true), rect),
488-
false,
489-
);
490-
assert.equal(
491-
discriminatingSurfaceChangedWithinRect(signature(false, 300), signature(false, 200), rect),
492-
true,
493-
);
494-
});
495-
496-
function makeToggleSnapshot(checked: boolean, y = 300): SnapshotState {
481+
test.each([
482+
{ anonymous: false, label: 'a labelled switch' },
483+
{ anonymous: true, label: 'an anonymous switch' },
484+
])(
485+
'discriminatingSurfaceChangedWithinRect reads a flip of $label at the same rect as no movement, and a moved one as movement',
486+
({ anonymous }) => {
487+
const rect = { x: 0, y: 0, width: 390, height: 844 };
488+
const signature = (checked: boolean, y?: number) =>
489+
buildInteractionSurfaceSignature(makeToggleSnapshot(checked, y, anonymous).nodes);
490+
491+
assert.equal(
492+
discriminatingSurfaceChangedWithinRect(signature(false), signature(true), rect),
493+
false,
494+
);
495+
assert.equal(
496+
discriminatingSurfaceChangedWithinRect(signature(false, 300), signature(false, 200), rect),
497+
true,
498+
);
499+
},
500+
);
501+
502+
// An anonymous switch has no identity, so its content is its type: the flip still changes only the
503+
// key, and a swipe that brushed it must not read as the list moving.
504+
function makeToggleSnapshot(checked: boolean, y = 300, anonymous = false): SnapshotState {
497505
const base = makeSnapshot('Inbox');
498506
return {
499507
...base,
@@ -504,8 +512,7 @@ function makeToggleSnapshot(checked: boolean, y = 300): SnapshotState {
504512
index: 2,
505513
parentIndex: 0,
506514
type: 'android.widget.Switch',
507-
identifier: 'wifi-switch',
508-
label: 'Wi-Fi switch',
515+
...(anonymous ? {} : { identifier: 'wifi-switch', label: 'Wi-Fi switch' }),
509516
checked,
510517
rect: { x: 300, y, width: 60, height: 40 },
511518
},

‎src/daemon/handlers/__tests__/snapshot-handler-capture-retry.test.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ test('captureSnapshot lazily retries pending no-change touch before returning fr
9797
preSignature: [
9898
{
9999
key: 'open-feed|Open feed||Button||hittable|#0',
100+
identity: 'open-feed|Open feed||Button',
101+
content: 'open-feed|Open feed||Button',
100102
x: 20,
101103
y: 120,
102104
width: 160,
@@ -240,6 +242,8 @@ test('captureSnapshot retries pending tap outcome before post-gesture stabilizat
240242
preSignature: [
241243
{
242244
key: '|Navigate to Third||android.widget.Button||hittable|#0',
245+
identity: '|Navigate to Third||android.widget.Button',
246+
content: '|Navigate to Third||android.widget.Button',
243247
x: 302,
244248
y: 1301,
245249
width: 476,

‎src/daemon/interaction-outcome-policy.ts‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -467,10 +467,10 @@ export function summarizeDiscriminatingSurfaceDivergence(
467467

468468
/**
469469
* Whether the DISCRIMINATING entries inside `rect` moved across a gesture: one left or entered the
470-
* region, or its rect moved beyond tolerance. Entries match on the flip-tolerant `identity` where
471-
* they have one, told apart by document order when repeated, and on `key` otherwise. A scroll moves
472-
* content, while a state flip inside the container (a switch the swipe brushed, a row it selected)
473-
* changes the key at the same rect and is not movement.
470+
* region, or its rect moved beyond tolerance. Entries match on `content`: the flip-tolerant
471+
* `identity` where they have one, the type and role of an anonymous node otherwise, told apart by
472+
* document order when repeated. A scroll moves content, while a state flip inside the container (a
473+
* switch the swipe brushed, a row it selected) changes the key at the same rect and is not movement.
474474
*
475475
* A whole-surface difference is not automatically the gesture's doing. A captured tree carries system
476476
* chrome with it, and on Android the status bar clocks and icons change on their own while the app's
@@ -500,10 +500,9 @@ function contentKeyed(
500500
const occurrences = new Map<string, number>();
501501
const keyed = new Map<string, InteractionSurfaceSignature[number]>();
502502
for (const entry of entries) {
503-
const content = entry.identity ?? entry.key;
504-
const occurrence = occurrences.get(content) ?? 0;
505-
occurrences.set(content, occurrence + 1);
506-
keyed.set(`${content}|#${occurrence}`, entry);
503+
const occurrence = occurrences.get(entry.content) ?? 0;
504+
occurrences.set(entry.content, occurrence + 1);
505+
keyed.set(`${entry.content}|#${occurrence}`, entry);
507506
}
508507
return keyed;
509508
}
@@ -558,6 +557,7 @@ function buildInteractionSurfaceEntry(
558557
return {
559558
key: `${semanticKey}|#${occurrence}`,
560559
...(identity ? { identity } : {}),
560+
content: interactionSurfaceContent(node, identity),
561561
x: Math.round(node.rect.x),
562562
y: Math.round(node.rect.y),
563563
width: Math.round(node.rect.width),
@@ -566,6 +566,11 @@ function buildInteractionSurfaceEntry(
566566
};
567567
}
568568

569+
/** What the element is without the state it is in: its identity, else the type and role of an anonymous node. */
570+
function interactionSurfaceContent(node: SnapshotNode, identity: string | undefined): string {
571+
return identity ?? `${node.type ?? ''}|${node.role ?? ''}`;
572+
}
573+
569574
/**
570575
* What the element IS — never where it sits, and never volatile state a gesture
571576
* is expected to change. `interactionSurfaceSemanticKey` deliberately folds in

‎src/daemon/session-state.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,13 @@ export type InteractionSurfaceEntry = {
5353
* comparison entirely.
5454
*/
5555
identity?: string;
56+
/**
57+
* What the element is without the state it is in: `identity` where the node
58+
* has one, else its type and role. A within-container movement check compares
59+
* entries on this, in document order when repeated, because a state flip
60+
* changes `key` at the same rect and is not movement.
61+
*/
62+
content: string;
5663
x: number;
5764
y: number;
5865
width: number;

0 commit comments

Comments
 (0)