Skip to content

Commit 95031db

Browse files
committed
refactor(interaction): say which tap point a path has in its resolver's type
The stage door carried two overload declarations and two parameter aliases to say that a path handing in a resolver returning `Point` gets a `Point` back and a path without one gets `null`. One constrained generic — `TPoint extends Point | null` on a resolver every path must pass — says exactly that, with the native-ref fast path writing its own nullability into its resolver instead of into a signature above the function it calls. No cast, no branch that cannot be reached. Also removes ceremony the previous commit left behind: the guard rebound `params.tapPoint` to a local for one use, and the runtime cases lifted the keyboard off the bottom edge twice, once through a translation knob with one caller and once inline with the offset spelled out again.
1 parent 280ebec commit 95031db

3 files changed

Lines changed: 54 additions & 71 deletions

File tree

src/commands/interaction/runtime/keyboard-occlusion.test.ts

Lines changed: 39 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,26 @@ import { createInteractionDevice } from './__tests__/test-utils/index.ts';
1414

1515
const TAB_BAR_RECT: Rect = { x: 148, y: 791, width: 104, height: 83 };
1616

17-
/** The contract fixture with its app-owned rects moved: the variants stay one tree, not copies. */
18-
function keyboardTree(params: { tabRect?: Rect; keyboardOwnedDy?: number } = {}): SnapshotState {
19-
if (!params.tabRect && params.keyboardOwnedDy === undefined)
20-
return keyboardCoveredTabBarSnapshot();
17+
/** The contract fixture with its app-owned button moved, so the variants stay one tree, not copies. */
18+
function keyboardTree(params: { tabRect?: Rect } = {}): SnapshotState {
19+
const tabRect = params.tabRect;
20+
if (!tabRect) return keyboardCoveredTabBarSnapshot();
2121
return makeSnapshotState(
22-
keyboardCoveredTabBarSnapshot().nodes.map((node) => {
23-
if (params.tabRect && node.index === 1) return { ...node, rect: params.tabRect };
24-
const keyboardOwned = node.type === 'Keyboard' || node.type === 'Key';
25-
if (params.keyboardOwnedDy !== undefined && keyboardOwned && node.rect) {
26-
return { ...node, rect: { ...node.rect, y: node.rect.y + params.keyboardOwnedDy } };
27-
}
28-
return node;
29-
}),
22+
keyboardCoveredTabBarSnapshot().nodes.map((node) =>
23+
node.index === 1 ? { ...node, rect: tabRect } : node,
24+
),
25+
);
26+
}
27+
28+
/**
29+
* Keyboard-owned rects hauled above the docking budget: the tree stops describing a keyboard the
30+
* bottom of the screen belongs to, which is what the app-drawn-keypad and aim cases measure against.
31+
*/
32+
function liftKeyboardOffBottomEdge(nodes: SnapshotState['nodes']): SnapshotState['nodes'] {
33+
return nodes.map((node) =>
34+
node.rect && (node.type === 'Keyboard' || node.type === 'Key')
35+
? { ...node, rect: { ...node.rect, y: node.rect.y - 560 } }
36+
: node,
3037
);
3138
}
3239

@@ -119,7 +126,8 @@ test('no keyboard in the tree means nothing to refuse', async () => {
119126

120127
test('an app-drawn keypad that stops short of the bottom edge is not the system keyboard', async () => {
121128
const calls: Point[] = [];
122-
const device = tappedDevice(keyboardTree({ keyboardOwnedDy: -560 }), calls);
129+
const appOwnedKeypad = makeSnapshotState(liftKeyboardOffBottomEdge(keyboardTree().nodes));
130+
const device = tappedDevice(appOwnedKeypad, calls);
123131

124132
await device.interactions.click(ref('@e2'), { session: 'default' });
125133

@@ -143,9 +151,8 @@ test('a coordinate behind the keyboard taps anyway and discloses the reason', as
143151
});
144152

145153
test('the guard reads the point the interaction dispatches, not the rect center', async () => {
146-
const calls: Point[] = [];
147-
// The Form button's own center sits 3 pt above the key plane, but its interactive child owns that
148-
// upper region, so the aim point the tap dispatches is pushed down into the keyboard's band.
154+
// The Form button's own center sits 3 pt above the key plane, and its interactive child owns that
155+
// upper region, so the point the tap dispatches is pushed down into the keyboard's band.
149156
const aimShifted = makeSnapshotState([
150157
...keyboardCoveredTabBarSnapshot().nodes,
151158
{
@@ -157,41 +164,29 @@ test('the guard reads the point the interaction dispatches, not the rect center'
157164
rect: { x: 148, y: 500, width: 104, height: 83 },
158165
hittable: true,
159166
},
160-
]);
161-
const shifted = aimShifted.nodes.map((node) =>
167+
]).nodes.map((node) =>
162168
node.index === 1 ? { ...node, rect: { x: 148, y: 500, width: 104, height: 160 } } : node,
163169
);
164-
const device = tappedDevice(makeSnapshotState(shifted), calls);
165170

171+
const calls: Point[] = [];
166172
await assert.rejects(
167-
() => device.interactions.click(ref('@e2'), { session: 'default' }),
168-
(error: unknown) => {
169-
assert.ok(error instanceof Error);
170-
assert.match(error.message, /Ref @e2 is behind the visible keyboard/);
171-
return true;
172-
},
173+
() =>
174+
tappedDevice(makeSnapshotState(aimShifted), calls).interactions.click(ref('@e2'), {
175+
session: 'default',
176+
}),
177+
/Ref @e2 is behind the visible keyboard/,
173178
);
174179
assert.deepEqual(calls, []);
175180

176-
// The same tree with its keypad lifted off the bottom edge shows where that tap was aiming: below
177-
// the key plane, which is what makes this the aim point's refusal rather than the center's.
178-
const aimCalls: Point[] = [];
179-
const clearDevice = tappedDevice(
180-
makeSnapshotState(
181-
shifted.map((node) =>
182-
node.type === 'Keyboard' || node.type === 'Key'
183-
? { ...node, rect: { ...node.rect!, y: node.rect!.y - 560 } }
184-
: node,
185-
),
186-
),
187-
aimCalls,
188-
);
189-
await clearDevice.interactions.click(ref('@e2'), { session: 'default' });
190-
assert.equal(aimCalls.length, 1);
191-
assert.ok(
192-
(aimCalls[0]?.y ?? 0) > 583,
193-
`expected the dispatched aim below the key plane at 583, got ${aimCalls[0]?.y}`,
194-
);
181+
// The same tree with its keyboard hauled off the bottom edge shows where that tap was aiming: below
182+
// the key plane, which is what makes this the dispatched point's refusal rather than the center's.
183+
const aim: Point[] = [];
184+
await tappedDevice(
185+
makeSnapshotState(liftKeyboardOffBottomEdge(aimShifted)),
186+
aim,
187+
).interactions.click(ref('@e2'), { session: 'default' });
188+
assert.equal(aim.length, 1);
189+
assert.ok((aim[0]?.y ?? 0) > 583, `expected the dispatched point below 583, got ${aim[0]?.y}`);
195190
});
196191

197192
test('a coordinate on a reported key is the keyboard the caller asked for', async () => {

src/commands/interaction/runtime/keyboard-occlusion.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,18 @@ export function assertTapTargetClearOfVisibleKeyboard(params: {
3131
/** How the caller named the target, e.g. `Ref @e40` or `Selector text=Form`. */
3232
label: string;
3333
/**
34-
* Where this interaction aims: the point a point-dispatching path resolved through the same
35-
* resolver it dispatches with, or the rect center for the native-ref fast path, which hands the
36-
* element to the platform and lets it pick. Null when the node has no measurable aim.
34+
* The point this interaction taps with: the one a coordinate-dispatching path resolved through the
35+
* same resolver it dispatches with, or the rect center for the native-ref fast path, which hands the
36+
* element to the platform and lets it choose. Null when the node has no measurable point.
3737
*/
3838
tapPoint: Point | null;
3939
}): void {
4040
const targetRect = params.node.rect;
41-
const tapPoint = params.tapPoint;
42-
if (!targetRect || !tapPoint) return;
41+
if (!targetRect || !params.tapPoint) return;
4342
const occlusion = resolveKeyboardTapOcclusion({
4443
nodes: params.nodes,
4544
viewport: createSnapshotVisibility(params.nodes).resolveViewport(targetRect),
46-
point: tapPoint,
45+
point: params.tapPoint,
4746
node: params.node,
4847
});
4948
if (occlusion.kind !== 'occluded') return;

src/commands/interaction/runtime/resolution.ts

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -654,30 +654,20 @@ function describeNonHittableTarget(
654654
* ref, and the native-ref preflight — enters them here, which is what keeps the native-ref fast
655655
* path from succeeding on a target the shared rules would refuse.
656656
*
657-
* A path that dispatches a coordinate hands in its own aim resolver and gets the measured point
658-
* back, so the keyboard guard reads the coordinate the interaction will actually send and the
659-
* dispatch does not derive a second one. The native-ref fast path dispatches by ref and has no
660-
* point to measure: its guard reads the rect center, which is the aim the platform picks.
657+
* Each path hands in the resolver that produces the point it taps with, and taps the point that
658+
* comes back, so the keyboard guard measures the coordinate the interaction is actually made of and no
659+
* path derives a second one. A path whose point can fail to exist — the native-ref fast path taps by
660+
* ref, reading the rect center the platform aims at — says so in its resolver's return type.
661661
*/
662-
type InteractionStageParams = {
662+
async function runInteractionPipelineStages<TPoint extends Point | null>(params: {
663663
policy: SelectorPipelinePolicy;
664664
nodes: SnapshotState['nodes'];
665665
node: SnapshotNode;
666666
action: InteractionAction;
667667
label: string;
668668
hooks: SelectorPipelineHooks;
669-
};
670-
type InteractionStageOutcome = { node: SnapshotNode };
671-
672-
async function runInteractionPipelineStages(
673-
params: InteractionStageParams & { resolveTapPoint: (node: SnapshotNode) => Point },
674-
): Promise<InteractionStageOutcome & { tapPoint: Point }>;
675-
async function runInteractionPipelineStages(
676-
params: InteractionStageParams & { resolveTapPoint?: undefined },
677-
): Promise<InteractionStageOutcome & { tapPoint: null }>;
678-
async function runInteractionPipelineStages(
679-
params: InteractionStageParams & { resolveTapPoint?: (node: SnapshotNode) => Point },
680-
): Promise<InteractionStageOutcome & { tapPoint: Point | null }> {
669+
resolveTapPoint: (node: SnapshotNode) => TPoint;
670+
}): Promise<{ node: SnapshotNode; tapPoint: TPoint }> {
681671
const target = await runNodePipelineStages(
682672
params.policy,
683673
params.nodes,
@@ -691,9 +681,7 @@ async function runInteractionPipelineStages(
691681
action: params.action,
692682
});
693683
}
694-
const tapPoint = params.resolveTapPoint
695-
? params.resolveTapPoint(target.node)
696-
: (resolveRectCenter(target.node.rect) ?? null);
684+
const tapPoint = params.resolveTapPoint(target.node);
697685
assertTapTargetClearOfVisibleKeyboard({
698686
nodes: params.nodes,
699687
node: target.node,
@@ -1021,7 +1009,7 @@ export async function preflightNativeRefInteraction(
10211009
// `resolvedTarget` whatever the command: its `none` promotion is what holds
10221010
// ADR 0011's "the preflight never changes which element the backend acts on".
10231011
const pipeline = SELECTOR_PIPELINE_POLICIES.resolvedTarget;
1024-
// #1542: dispatches by REF, not coordinate, so no point to re-derive — but
1012+
// #1542: dispatches by REF, not coordinate, so no point is re-derived for the dispatch — but
10251013
// evidence/annotation below still describes the returned (visible) node.
10261014
const { node: visibleNode } = await runInteractionPipelineStages({
10271015
policy: pipeline,
@@ -1036,6 +1024,7 @@ export async function preflightNativeRefInteraction(
10361024
pipeline,
10371025
}),
10381026
},
1027+
resolveTapPoint: (node) => resolveRectCenter(node.rect) ?? null,
10391028
});
10401029
return {
10411030
...describeNonHittableTarget(visibleNode, action),

0 commit comments

Comments
 (0)