Skip to content

Keep pending approvals visible and keyboard accessible - #78

Open
ZedingZhang wants to merge 2 commits into
mainfrom
codex/issue-64-approval-accessibility
Open

ZedingZhang wants to merge 2 commits into
mainfrom
codex/issue-64-approval-accessibility

Conversation

@ZedingZhang

@ZedingZhang ZedingZhang commented Oct 28, 2026 •

Copy link
Copy Markdown
Owner

A pending local action now opens a labelled approval region, announces the pending count in English and Chinese, and moves keyboard focus to Deny. Escape closes Task options while keeping the approval visible. The Activity toggle exposes the pending state in its accessible name; resolving or expiring an approval restores usable focus without taking it away from someone who has navigated elsewhere.

Closes #64.

Validation:

  • npm run check: 665 tests, typecheck, and production build passed.
  • npm run smoke:gui: passed, including both locales, the real Electron accessibility tree, Escape, disabled response controls, expiration, and focus restoration.
  • Native macOS keyboard checks exercised the real local-write approval through the main process and renderer. Review found document-body focus could be restored after disabling the composer; a regression failed before the fix, passes afterward, and the native Deny path now returns focus to Activity.
  • VoiceOver was enabled for the native keyboard session and restored to its original disabled state afterward. Actual spoken output could not be verified: the native UI/read-only phrase interface timed out. A human VoiceOver/NVDA reading check remains outstanding.

@ZedingZhang ZedingZhang left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Reviewed the full approval accessibility diff and exercised a real local-write request with native keyboard input. The region naming, pending-state label, Escape behavior, and disabled response target are covered in both locales. One focus-restoration defect remains below. Actual VoiceOver speech is still unverified because the local reader interface is unavailable; this limitation is recorded in the PR.

Comment thread src/gui/renderer.ts Resolved

@ZedingZhang ZedingZhang left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Re-reviewed the complete current diff at 73c122a after the P2 focus finding was fixed. The renderer now excludes the document body and verifies that a candidate actually takes focus before using it; otherwise it returns to Activity. The added Electron regression failed before the fix and passes afterward in English and Chinese. Valid prior focus restoration and preserving focus outside the panel remain covered.

npm run check passed all 661 tests, typechecking, and build. npm run smoke:gui passed, including actual accessibility-tree region naming, focused button descriptions, live status, Escape, response/expiry transitions, and both restoration paths. No further code findings. Actual VoiceOver spoken output remains a manual validation gap as recorded in the PR; native keyboard interaction was exercised with VoiceOver enabled and the original disabled state was restored afterward.

@ZedingZhang
ZedingZhang force-pushed the codex/issue-64-approval-accessibility branch from 73c122a to 4082657 Compare October 28, 2026 12:56

@ZedingZhang ZedingZhang left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Re-reviewed 4082657 after rebasing onto main 0c63d29, which includes the merged Keychain UTF-8 limit fix (#79). All three GUI files retain identical Git blob hashes, so the approval behavior and the native keyboard verification remain applicable. No conflicts or further code findings. The previous P2 focus thread is resolved.

Re-ran the complete check on this integrated baseline: 665 tests, typechecking, production build, and the actual Electron smoke suite all pass. Actual VoiceOver/NVDA spoken output remains pending the human check required by #64; the isolated native fixture remains open for that check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Accessibility] Keep pending approvals visible on Escape and announce them to assistive technology

1 participant