Defer Ctrl+P/Ctrl+N to an open modal before app-level keybindings#44
Open
LukasIngemarsson wants to merge 3 commits into
Open
Defer Ctrl+P/Ctrl+N to an open modal before app-level keybindings#44LukasIngemarsson wants to merge 3 commits into
LukasIngemarsson wants to merge 3 commits into
Conversation
app.Update matched ActionTabPrev/ActionTabNext and returned before delegating to the active tab, with no gate for an open modal. When tab.prev/tab.next are rebound onto Ctrl+P/Ctrl+N -- the same keys list_nav.go uses for in-list navigation -- the app layer intercepted them before /config, the ask-question modal, and inline pickers could move their cursors, so those keys silently switched tabs instead of navigating the open list. The existing model-layer deferral in update.go never ran because dispatchActive was skipped. Gate the switch on the active tab: when isCtrlListNav(msg) and the active tab reports modalOpen() || popoverOpen(), dispatch the key to the tab instead of switching. Outside a modal/popover the keys fall through to normal tab navigation, so the rebind keeps working everywhere else. Tests: TestApp_TabNavDefersToModalListNav (modal open -> list cursor advances, active tab unchanged) and TestApp_TabNavSwitchesWithoutModal (no modal -> tab still switches), both with tab.prev/next rebound onto Ctrl+P/N. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
LukasIngemarsson
changed the title
The deferral gate sat after ActionAppSuspend/ActionTabNew, so it only beat tab-switch rebinds. Binding any other app-level action (suspend, new tab) onto the list-nav keys would still shadow in-modal navigation -- the same bug, one rebind away. Move the gate to the front of the app.Update key switch so that whenever the active tab's modal/popover owns list navigation, Ctrl+P/Ctrl+N navigate the list regardless of which action is bound to them. Retargets the test (now TestApp_ModalListNavWinsOverReboundAppActions): with suspend or tab.new rebound onto Ctrl+N and a modal open, Ctrl+N now advances the modal cursor instead of suspending / opening a tab. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
LukasIngemarsson
changed the title
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
App-level key dispatch in
app.Update(tabs.go) matches keymap actions (tab.prev/tab.next/tab.new/app.suspend) and returns before delegating to the active tab — so it runs before any modal in that tab can see the key.Ctrl+P/Ctrl+N(list_nav.go) are alternates to the arrow keys for navigating in-modal lists —/config, the ask-question modal, inline pickers. The keybindings config lets you rebind any app-level action onto other keys, includingCtrl+P/Ctrl+N. When you do, the app layer consumes the keypress for that action and the modal never sees it — so the keys switch tabs, open a tab, or suspend instead of navigating the open list. The model-layer popover deferral inupdate.gonever runs becausedispatchActiveis skipped.(By default, tab nav is
Ctrl+Left/Ctrl+RightandCtrl+P/Ctrl+Naren't bound to any app action, so they already reach the modal — this only bites once an app action is rebound onto the list-nav keys.)Fix
When the active tab's modal/popover owns list navigation (
ownsCtrlListNav()) and the key is a Ctrl list-nav key, dispatch it to the tab before any app-level action — so in a modal,Ctrl+P/Ctrl+Nalways navigate the list regardless of what's bound to them. Outside such a modal the keys fall through to normal app handling, so a rebind keeps working everywhere else.ownsCtrlListNav()is intentionally narrow: confirm/approval overlays and field-editing sub-states do not consume the keys, so they keep their normal app behavior rather than swallowing the keypress. The predicate mirrors each modal's own list-nav guards (e.g. the ask modal'saskConfirmingCancel/askOllamaActive/askEditNoteearly-returns).Tests
keymap_dispatch_test.go: withtab.prev/tab.next,tab.new, orapp.suspendrebound ontoCtrl+P/Ctrl+N, those keys navigate the/configlist without switching/opening tabs or suspending; confirm/approval modals fall through to normal handling; and with no modal open the rebind still performs its app action.🤖 Generated with Claude Code