Repository navigation
Add exclusive groups for dialogs - #1
Conversation
- While an exclusive group is registered, scanning is confined to it; the most recently registered one wins, and closing it returns to the page - Pass exclusive through useScanGroup and ScanGroup, document it, and bump to 0.2.0 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
| () => controller.registerGroup(id, { parentId: scope.id, order, exclusive }, anchor), | ||
| [controller, id, scope.id, order, exclusive, anchor], |
There was a problem hiding this comment.
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
- 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.
- The baseline command captures snapshots and actual selections before and after an unchanged-order rerender, showing that the newer dialog retains priority.
- The update command captures real effect cleanup and re-registration followed by snapshots and actual selections, showing that the older dialog steals priority.
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.| latest = order; | ||
| } | ||
| } | ||
| return build(scope, scope === undefined ? new Set() : new Set([scope])); |
There was a problem hiding this 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.
Artifacts
- 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.
- 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.
- 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.
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.
Lets an open dialog confine scanning to itself. Switchify Remote needs this for its selector and PC switcher dialogs.
registerGroup(id, { exclusive: true }): while the group is registered, the scan tree starts at it. The most recently registered exclusive group wins, so nested dialogs work, and removing it returns to the page.useScanGroupand<ScanGroup>acceptexclusive.Validation
npm test: 60 tests pass, including a new nested-dialog test.npm run lintandnpm run buildare clean.🤖 Generated with Claude Code