Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ Call `item.measure()` after scrolling if the order should follow the new positio

`ScanProvider` also takes `policy` (`Policy.KEYBOARD`, the default, restarts from the top; `Policy.MENU` stays where it was), `afterSelection` (`continue` or `stop`), `onSelect` and `onError`. If `onActivate` returns a promise, scanning holds until it settles.

Pass `exclusive` to `useScanGroup` or `<ScanGroup>` for a dialog: while it is mounted, scanning is confined to it. The most recently mounted exclusive group wins.

Ids must be unique across items and groups.

## License
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@switchify/scanning",
"version": "0.1.0",
"version": "0.2.0",
"description": "Switch scanning for React and React Native, ported from Switchify PC.",
"license": "AGPL-3.0-only",
"type": "module",
Expand Down
23 changes: 22 additions & 1 deletion src/core/controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ export interface ItemRegistration {
export interface GroupRegistration {
parentId?: string;
order?: number;
/**
* While registered, scanning is confined to this group, as for an open dialog.
* The most recently registered exclusive group wins.
*/
exclusive?: boolean;
}

export type Highlight =
Expand Down Expand Up @@ -133,6 +138,8 @@ export class ScanController {
Pick<ScanControllerOptions, "onSelect" | "onError">;
private scanner: ItemScanner<string>;
private readonly sequences = new Map<string, number>();
private readonly exclusive = new Map<string, number>();
private exclusiveOrder = 0;
private sequence = 0;
private active = false;
private paused = false;
Expand Down Expand Up @@ -214,10 +221,16 @@ export class ScanController {
sequence: this.sequenceOf(id),
};
this.groups.set(id, entry);
if (registration.exclusive) {
if (!this.exclusive.has(id)) this.exclusive.set(id, ++this.exclusiveOrder);
} else {
this.exclusive.delete(id);
}
this.contentChanged();
return () => {
if (this.groups.get(id) === entry) {
this.groups.delete(id);
this.exclusive.delete(id);
this.contentChanged();
}
};
Expand Down Expand Up @@ -386,7 +399,15 @@ export class ScanController {
if (seen.has(entry.id)) return [];
return [group(entry.id, build(entry.id, new Set(seen).add(entry.id)))];
});
return build(undefined, new Set());
let scope: string | undefined;
let latest = -1;
for (const [id, order] of this.exclusive) {
if (this.groups.has(id) && order > latest) {
scope = id;
latest = order;
}
}
return build(scope, scope === undefined ? new Set() : new Set([scope]));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Dialog reports wrong entered state

The exclusive group's ID disappears from ScanSnapshot.path because the new tree starts at its children. useScanGroup therefore reports entered: false even while scanning highlights and successfully activates an item inside that group. This is a non-blocking state-reporting defect: callers using entered for styling or feedback do not show that scanning is inside the dialog.

Include the exclusive scope in the reported path without adding a way to escape it.

Artifacts

Evidence from the check

  • This executed test mounts the real React scanning hooks, highlights and selects a group item, and checks snapshot and rendered state, demonstrating the exclusive-group reporting defect.

Command output from the check

  • The same test ran against isolated PR base source and passed both cases after entering the group and activating its item, establishing the pre-change comparison.

Command output from the check

  • The test ran against actual HEAD source and passed the normal baseline but failed the exclusive entered assertion after successful child activation, confirming candidate 1.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/core/controller.ts
Line: 410

Comment:
**Dialog reports wrong entered state**

The exclusive group's ID disappears from `ScanSnapshot.path` because the new tree starts at its children. `useScanGroup` therefore reports `entered: false` even while scanning highlights and successfully activates an item inside that group. This is a non-blocking state-reporting defect: callers using `entered` for styling or feedback do not show that scanning is inside the dialog.

Include the exclusive scope in the reported path without adding a way to escape it.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

}

private moving(): boolean {
Expand Down
12 changes: 7 additions & 5 deletions src/react/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ export function useScanItem(

export interface ScanGroupOptions {
order?: number;
/** Confine scanning to this group while it is mounted, as for an open dialog. */
exclusive?: boolean;
}

export interface ScanGroupState {
Expand Down Expand Up @@ -224,14 +226,14 @@ function groupState(key: GroupKey): ScanGroupState {
/** Registers a group in the current scope. Wrap its children in ScanGroupScope. */
export function useScanGroup(
id: string,
{ order }: ScanGroupOptions = {},
{ order, exclusive }: ScanGroupOptions = {},
anchor?: unknown,
): ScanGroupState {
const controller = useScanController();
const scope = useContext(ScopeContext);
useEffect(
() => controller.registerGroup(id, { parentId: scope.id, order }, anchor),
[controller, id, scope.id, order, anchor],
() => controller.registerGroup(id, { parentId: scope.id, order, exclusive }, anchor),
[controller, id, scope.id, order, exclusive, anchor],
Comment on lines +235 to +236

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Older dialog steals scanning

Changing order on an older exclusive ScanGroup takes scanning away from a newer, still-open dialog. The registration effect cleans up the older group, deleting its exclusive priority, then registers it with a higher priority. The next Select activates a control in the older dialog even though the newer dialog remains mounted and exclusive.

Preserve exclusive priority across option updates and add a regression test for an older dialog updating while a newer one remains open. This must be fixed before merging.

Artifacts

Evidence from the check

  • The executed test mounts two exclusive groups and compares stable-order and changed-order rerenders against actual source, proving the candidate through snapshots and handler calls.

Command output from the check

  • The baseline command captures snapshots and actual selections before and after an unchanged-order rerender, showing that the newer dialog retains priority.

Command output from the check

  • The update command captures real effect cleanup and re-registration followed by snapshots and actual selections, showing that the older dialog steals priority.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/react/index.tsx
Line: 235-236

Comment:
**Older dialog steals scanning**

Changing `order` on an older exclusive `ScanGroup` takes scanning away from a newer, still-open dialog. The registration effect cleans up the older group, deleting its exclusive priority, then registers it with a higher priority. The next Select activates a control in the older dialog even though the newer dialog remains mounted and exclusive.

Preserve exclusive priority across option updates and add a regression test for an older dialog updating while a newer one remains open. This must be fixed before merging.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

);
const read = useCallback(
(snapshot: ScanSnapshot): GroupKey => {
Expand Down Expand Up @@ -261,8 +263,8 @@ export interface ScanGroupProps extends ScanGroupOptions {
}

/** A group with no element of its own. Use the dom or native wrappers to order groups by layout. */
export function ScanGroup({ id, order, children }: ScanGroupProps) {
const state = useScanGroup(id, { order });
export function ScanGroup({ id, order, exclusive, children }: ScanGroupProps) {
const state = useScanGroup(id, { order, exclusive });
return (
<ScanGroupScope id={id}>{typeof children === "function" ? children(state) : children}</ScanGroupScope>
);
Expand Down
28 changes: 28 additions & 0 deletions test/controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,3 +261,31 @@ test("a held switch pauses automatic movement until release", () => {
clock.advance(500);
expect(highlight(controller)).toEqual({ kind: "item", id: "b" });
});

test("an exclusive group confines scanning until it is removed", async () => {
const { controller } = setup({ options: { automatic: false } });
controller.registerItem("page-a");
controller.registerItem("page-b");
controller.flush();
controller.start();
controller.dispatch("next");
const closeDialog = controller.registerGroup("dialog", { exclusive: true });
const dialogItems = [controller.registerItem("ok", { parentId: "dialog" }), controller.registerItem("cancel", { parentId: "dialog" })];
controller.flush();
expect(highlight(controller)).toEqual({ kind: "item", id: "ok" });
controller.dispatch("next");
controller.dispatch("next");
expect(highlight(controller)).toEqual({ kind: "item", id: "ok" });
const closeNested = controller.registerGroup("nested", { exclusive: true });
const removeInner = controller.registerItem("inner", { parentId: "nested" });
controller.flush();
expect(highlight(controller)).toEqual({ kind: "item", id: "inner" });
removeInner();
closeNested();
await Promise.resolve();
expect(highlight(controller)).toEqual({ kind: "item", id: "ok" });
dialogItems.forEach((remove) => remove());
closeDialog();
await Promise.resolve();
expect(highlight(controller)).toEqual({ kind: "item", id: "page-a" });
});
Loading