Skip to content

fix(tui): prevent shortcut fall-through when modal dialogs are open - #30000

Closed
leo0o7 wants to merge 1 commit into
anomalyco:devfrom
leo0o7:fix/tui-dialog-shortcut-fallthrough
Closed

leo0o7 wants to merge 1 commit into
anomalyco:devfrom
leo0o7:fix/tui-dialog-shortcut-fallthrough

Conversation

@leo0o7

@leo0o7 leo0o7 commented May 30, 2026

Copy link
Copy Markdown

Issue for this PR

Closes #29999

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Open dialogs could lose keyboard input when the TUI entered question/autocomplete mode. Keys intended for the dialog could be handled by the prompt state instead.

This PR gives dialog selection bindings higher priority so the active dialog gets first chance to handle its own keyboard input.

Global app/session bindings were also active while dialogs were open. This PR disables those global layers while dialog.stack is non-empty, so leader shortcuts and session-level shortcuts do not run from inside an active modal.

How did you verify your code works?

  • Extended the existing mode-less bindings test to cover COMMAND_PALETTE_COMMAND.
  • Added a modal keybinding test that checks active command counts and key resolution before/after opening a modal.
  • Ran bun test test/cli/tui/keymap.test.tsx — 3/3 pass.
  • Ran bun typecheck — clean.

Screenshots / recordings

Not included. This is a keyboard-state fix covered by keymap tests.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@kommander

kommander commented May 30, 2026 •

Copy link
Copy Markdown
Contributor

Keymap layers can have a priority and dispatch order should be last added first, so the reverse order of when layers were added, then by priority, so higher priority should win. The dialog layers registered via useBindings should get a high priority, default is just 1 or 0 or something, so they can take 1000. It's like a zindex basically.

Edit: I meant sort is by priority first, then newer registration first. So priority takes precedence.

@leo0o7
leo0o7 force-pushed the fix/tui-dialog-shortcut-fallthrough branch from 9d0f9b6 to 295f73d Compare May 31, 2026 08:16
@leo0o7

leo0o7 commented May 31, 2026

Copy link
Copy Markdown
Author

Updated, thanks.

I switched this to a shared DIALOG_LAYER_PRIORITY = 1000, applied it to dialog-owned binding layers, removed the dialog.stack global-layer gates, and added a regression test for dialog priority winning over a newer default-priority binding.

@leo0o7

leo0o7 commented Jun 5, 2026

Copy link
Copy Markdown
Author

@kommander just checking if there's anything else needed here now that this uses the priority-based approach.

@github-actions

Copy link
Copy Markdown
Contributor

Automated PR Cleanup

Thank you for contributing to opencode.

Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions.

This PR was closed because it matched the following cleanup criteria:

  • The PR was created more than 1 month ago
  • The PR had fewer than 2 positive reactions
  • Positive reactions are counted as thumbs-up, heart, celebration, or rocket reactions on the PR

PRs created within the last month are not affected by this cleanup.

If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate.

Thanks again for taking the time to contribute.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dialogs should keep keyboard focus when a question prompt appears

2 participants