Skip to content

Preserve focus across card presentation transition - #2799

Merged
Chris0Jeky merged 2 commits into
mainfrom
issue-2090/inspector-modal-focus
Sep 8, 2026
Merged

Preserve focus across card presentation transition#2799
Chris0Jeky merged 2 commits into
mainfrom
issue-2090/inspector-modal-focus

Conversation

@Chris0Jeky

Copy link
Copy Markdown
Owner

Summary

When an open desktop inspector changes to modal presentation, CardModal now moves focus into the modal when focus is outside the card and preserves focus already owned by the card or a nested dialog. Closing still restores the original invoking control.

Validation

  • npx vitest --run --maxWorkers=2 src/tests/components/CardModal.spec.ts (37 passed)
  • npm run typecheck (passed)
  • npm run lint (passed; 8 existing warnings, 0 errors)
  • The new external-focus regression was red against the pinned base before the component change.

Refs #2090

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-08T00:37:26.544184Z 9ce3794 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Chris0Jeky Chris0Jeky 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.

Independent review: no blocking findings.

The inspector-to-modal watcher only moves focus when it is outside CardModal and no nested modal owns it. It preserves editor focus and nested-dialog ownership, while the original opener remains the close-restoration target. Focused component tests (37), typecheck, and scoped lint pass at 163590a6303e1141fbefc914df3c0fc1b46b9a88.

The existing automated Codex comment only reports that its review is running; it is informational and has no finding to action. Browser evidence for the concrete transition remains pending the shared runtime lease.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 163590a630

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread frontend/taskdeck-web/src/components/board/CardModal.vue
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Session wrap-up: source and browser regression are pushed at 9ce3794. The worktree was removed after clean tracked/upstream checks; the full viewport PNG and trace are preserved at C:\Users\Public\codex-shell-home\taskdeck-artifacts\2090-inspector-modal-focus. Runtime ports 5025/4185 are stopped. Hosted checks remain to be refreshed; the tracked P2 has a resolved discussion and is retained on #2090. Handoff: PR #2802.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Independent Terra review, round 1 — no confirmed Critical/High findings at head 9ce3794 against base 8d7cd7d. The existing Codex P2 is correctly triaged as a separately tracked shell-palette return-focus lifecycle defect; it is not a merge blocker for this CardModal transition. Local evidence: npx vitest --run --maxWorkers=2 src/tests/components/CardModal.spec.ts (37 passed). No fix batch required; ci-required still had Windows API Integration running at review snapshot.

@Chris0Jeky
Chris0Jeky merged commit 0ffb958 into main Sep 8, 2026
35 checks passed
@Chris0Jeky
Chris0Jeky deleted the issue-2090/inspector-modal-focus branch September 8, 2026 01:04
@github-project-automation github-project-automation Bot moved this from Review to Done in Taskdeck Execution Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant