fix(vim): keep input activation and reverse focus
- commit
- 8c7ae2b9917642c2d2e39194ee9ae57b4e1e2909
- author
- brice · · 2026-08-14 13:30 UTC
- parent
- 55482b4
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +55 -12
M internal/application/usecase/vim_mode_policy.go +3 -3
| @@ -46,9 +46,9 @@ func NewVimModePolicyUseCase() *VimModePolicyUseCase { | |||
| 46 | 46 | func (*VimModePolicyUseCase) Evaluate(input VimModePolicyInput) VimModePolicyTransition { | |
| 47 | 47 | switch input.Trigger { | |
| 48 | 48 | case VimModePolicyTriggerActivationAttempt: | |
| 49 | - | if input.PageEditableFocused { | |
| 50 | - | return VimModePolicyTransitionBlockActivation | |
| 51 | - | } | |
| 49 | + | // Ctrl+Y is the explicit Vim Mode activation command. It must remain | |
| 50 | + | // available while a page input is focused so users can leave typing mode | |
| 51 | + | // without first moving focus elsewhere. | |
| 52 | 52 | return VimModePolicyTransitionStay | |
| 53 | 53 | case VimModePolicyTriggerOmniboxFocus, | |
| 54 | 54 | VimModePolicyTriggerFindBarFocus, | |
| @@ -46,9 +46,9 @@ func NewVimModePolicyUseCase() *VimModePolicyUseCase { | |||||
| 46 | func (*VimModePolicyUseCase) Evaluate(input VimModePolicyInput) VimModePolicyTransition { | 46 | func (*VimModePolicyUseCase) Evaluate(input VimModePolicyInput) VimModePolicyTransition { | ||
| 47 | switch input.Trigger { | 47 | switch input.Trigger { | ||
| 48 | case VimModePolicyTriggerActivationAttempt: | 48 | case VimModePolicyTriggerActivationAttempt: | ||
| 49 | - | if input.PageEditableFocused { | 49 | + | // Ctrl+Y is the explicit Vim Mode activation command. It must remain |
| 50 | - | return VimModePolicyTransitionBlockActivation | 50 | + | // available while a page input is focused so users can leave typing mode |
| 51 | - | } | 51 | + | // without first moving focus elsewhere. |
| 52 | return VimModePolicyTransitionStay | 52 | return VimModePolicyTransitionStay | ||
| 53 | case VimModePolicyTriggerOmniboxFocus, | 53 | case VimModePolicyTriggerOmniboxFocus, | ||
| 54 | VimModePolicyTriggerFindBarFocus, | 54 | VimModePolicyTriggerFindBarFocus, | ||
M internal/application/usecase/vim_mode_policy_test.go +2 -2
| @@ -13,12 +13,12 @@ func TestVimModePolicyEvaluate(t *testing.T) { | |||
| 13 | 13 | want VimModePolicyTransition | |
| 14 | 14 | }{ | |
| 15 | 15 | { | |
| 16 | - | name: "activation blocked while page editable focused", | |
| 16 | + | name: "activation stays allowed while page editable focused", | |
| 17 | 17 | input: VimModePolicyInput{ | |
| 18 | 18 | Trigger: VimModePolicyTriggerActivationAttempt, | |
| 19 | 19 | PageEditableFocused: true, | |
| 20 | 20 | }, | |
| 21 | - | want: VimModePolicyTransitionBlockActivation, | |
| 21 | + | want: VimModePolicyTransitionStay, | |
| 22 | 22 | }, | |
| 23 | 23 | { | |
| 24 | 24 | name: "activation stays allowed when page is not editable", | |
| @@ -13,12 +13,12 @@ func TestVimModePolicyEvaluate(t *testing.T) { | |||||
| 13 | want VimModePolicyTransition | 13 | want VimModePolicyTransition | ||
| 14 | }{ | 14 | }{ | ||
| 15 | { | 15 | { | ||
| 16 | - | name: "activation blocked while page editable focused", | 16 | + | name: "activation stays allowed while page editable focused", |
| 17 | input: VimModePolicyInput{ | 17 | input: VimModePolicyInput{ | ||
| 18 | Trigger: VimModePolicyTriggerActivationAttempt, | 18 | Trigger: VimModePolicyTriggerActivationAttempt, | ||
| 19 | PageEditableFocused: true, | 19 | PageEditableFocused: true, | ||
| 20 | }, | 20 | }, | ||
| 21 | - | want: VimModePolicyTransitionBlockActivation, | 21 | + | want: VimModePolicyTransitionStay, |
| 22 | }, | 22 | }, | ||
| 23 | { | 23 | { | ||
| 24 | name: "activation stays allowed when page is not editable", | 24 | name: "activation stays allowed when page is not editable", | ||
M internal/ui/app_vim_mode_test.go +4 -4
| @@ -879,20 +879,20 @@ func TestVimMode_TabSwitchExitsAndClearsOldAccent(t *testing.T) { | |||
| 879 | 879 | assert.False(t, f.pv1A.IsVimMode()) | |
| 880 | 880 | } | |
| 881 | 881 | ||
| 882 | - | func TestVimMode_ActivationBypassWhenActivePageIsEditable(t *testing.T) { | |
| 882 | + | func TestVimMode_ActivationRemainsAvailableWhenPageIsEditable(t *testing.T) { | |
| 883 | 883 | f := newSingleWindowVimModeFixture(t) | |
| 884 | 884 | f.app.pageEditableFocusByPane = map[entity.PaneID]bool{ | |
| 885 | 885 | "pane-a": true, | |
| 886 | 886 | } | |
| 887 | 887 | ||
| 888 | - | assert.True(t, f.app.shouldBypassVimModeActivation(f.bw1)) | |
| 888 | + | assert.False(t, f.app.shouldBypassVimModeActivation(f.bw1)) | |
| 889 | 889 | } | |
| 890 | 890 | ||
| 891 | - | func TestVimMode_ActivationBypassClearsWhenEditableFocusLeaves(t *testing.T) { | |
| 891 | + | func TestVimMode_ActivationAvailabilityDoesNotDependOnEditableFocus(t *testing.T) { | |
| 892 | 892 | f := newSingleWindowVimModeFixture(t) | |
| 893 | 893 | ||
| 894 | 894 | f.app.handlePageEditableFocusChanged(context.Background(), entity.PaneID("pane-a"), true) | |
| 895 | - | assert.True(t, f.app.shouldBypassVimModeActivation(f.bw1)) | |
| 895 | + | assert.False(t, f.app.shouldBypassVimModeActivation(f.bw1)) | |
| 896 | 896 | ||
| 897 | 897 | f.app.handlePageEditableFocusChanged(context.Background(), entity.PaneID("pane-a"), false) | |
| 898 | 898 | assert.False(t, f.app.shouldBypassVimModeActivation(f.bw1)) | |
| @@ -879,20 +879,20 @@ func TestVimMode_TabSwitchExitsAndClearsOldAccent(t *testing.T) { | |||||
| 879 | assert.False(t, f.pv1A.IsVimMode()) | 879 | assert.False(t, f.pv1A.IsVimMode()) | ||
| 880 | } | 880 | } | ||
| 881 | 881 | ||||
| 882 | - | func TestVimMode_ActivationBypassWhenActivePageIsEditable(t *testing.T) { | 882 | + | func TestVimMode_ActivationRemainsAvailableWhenPageIsEditable(t *testing.T) { |
| 883 | f := newSingleWindowVimModeFixture(t) | 883 | f := newSingleWindowVimModeFixture(t) | ||
| 884 | f.app.pageEditableFocusByPane = map[entity.PaneID]bool{ | 884 | f.app.pageEditableFocusByPane = map[entity.PaneID]bool{ | ||
| 885 | "pane-a": true, | 885 | "pane-a": true, | ||
| 886 | } | 886 | } | ||
| 887 | 887 | ||||
| 888 | - | assert.True(t, f.app.shouldBypassVimModeActivation(f.bw1)) | 888 | + | assert.False(t, f.app.shouldBypassVimModeActivation(f.bw1)) |
| 889 | } | 889 | } | ||
| 890 | 890 | ||||
| 891 | - | func TestVimMode_ActivationBypassClearsWhenEditableFocusLeaves(t *testing.T) { | 891 | + | func TestVimMode_ActivationAvailabilityDoesNotDependOnEditableFocus(t *testing.T) { |
| 892 | f := newSingleWindowVimModeFixture(t) | 892 | f := newSingleWindowVimModeFixture(t) | ||
| 893 | 893 | ||||
| 894 | f.app.handlePageEditableFocusChanged(context.Background(), entity.PaneID("pane-a"), true) | 894 | f.app.handlePageEditableFocusChanged(context.Background(), entity.PaneID("pane-a"), true) | ||
| 895 | - | assert.True(t, f.app.shouldBypassVimModeActivation(f.bw1)) | 895 | + | assert.False(t, f.app.shouldBypassVimModeActivation(f.bw1)) |
| 896 | 896 | ||||
| 897 | f.app.handlePageEditableFocusChanged(context.Background(), entity.PaneID("pane-a"), false) | 897 | f.app.handlePageEditableFocusChanged(context.Background(), entity.PaneID("pane-a"), false) | ||
| 898 | assert.False(t, f.app.shouldBypassVimModeActivation(f.bw1)) | 898 | assert.False(t, f.app.shouldBypassVimModeActivation(f.bw1)) | ||
M internal/ui/input/keyboard.go +20 -3
| @@ -407,6 +407,13 @@ func (h *KeyboardHandler) handleKeyPress(keyval, keycode uint, state gdk.Modifie | |||
| 407 | 407 | } | |
| 408 | 408 | } | |
| 409 | 409 | ||
| 410 | + | // Page focus traversal remains native in normal mode. Keep Tab and | |
| 411 | + | // Shift+Tab out of the shortcut lookup so a focused CEF page control can | |
| 412 | + | // handle both directions consistently. | |
| 413 | + | if shouldPassthroughNativePageFocusNavigation(mode, keyval, modifiers) { | |
| 414 | + | return false | |
| 415 | + | } | |
| 416 | + | ||
| 410 | 417 | // Determine routing for this key event | |
| 411 | 418 | route := RouteHandleShortcuts // default: process through shortcut system | |
| 412 | 419 | if routeKey != nil && mode == ModeNormal { | |
| @@ -612,6 +619,18 @@ func normalizeKeyval(keyval uint) uint { | |||
| 612 | 619 | return keyval | |
| 613 | 620 | } | |
| 614 | 621 | ||
| 622 | + | func shouldPassthroughNativePageFocusNavigation(mode Mode, keyval uint, modifiers Modifier) bool { | |
| 623 | + | if mode != ModeNormal { | |
| 624 | + | return false | |
| 625 | + | } | |
| 626 | + | switch keyval { | |
| 627 | + | case uint(gdk.KEY_Tab), uint(gdk.KEY_ISO_Left_Tab): | |
| 628 | + | return modifiers == ModNone || modifiers == ModShift | |
| 629 | + | default: | |
| 630 | + | return false | |
| 631 | + | } | |
| 632 | + | } | |
| 633 | + | ||
| 615 | 634 | func shouldPassthroughNativeVimModeNavigation(mode Mode, keyval uint, modifiers Modifier) bool { | |
| 616 | 635 | if mode != ModeVim { | |
| 617 | 636 | return false | |
| @@ -619,9 +638,7 @@ func shouldPassthroughNativeVimModeNavigation(mode Mode, keyval uint, modifiers | |||
| 619 | 638 | switch keyval { | |
| 620 | 639 | case uint(gdk.KEY_Left), uint(gdk.KEY_Right), uint(gdk.KEY_Up), uint(gdk.KEY_Down): | |
| 621 | 640 | return modifiers == ModNone | |
| 622 | - | case uint(gdk.KEY_Tab): | |
| 623 | - | return modifiers == ModNone || modifiers == ModShift | |
| 624 | - | case uint(gdk.KEY_ISO_Left_Tab): | |
| 641 | + | case uint(gdk.KEY_Tab), uint(gdk.KEY_ISO_Left_Tab): | |
| 625 | 642 | return modifiers == ModNone || modifiers == ModShift | |
| 626 | 643 | default: | |
| 627 | 644 | return false | |
| @@ -407,6 +407,13 @@ func (h *KeyboardHandler) handleKeyPress(keyval, keycode uint, state gdk.Modifie | |||||
| 407 | } | 407 | } | ||
| 408 | } | 408 | } | ||
| 409 | 409 | ||||
| 410 | + | // Page focus traversal remains native in normal mode. Keep Tab and | |||
| 411 | + | // Shift+Tab out of the shortcut lookup so a focused CEF page control can | |||
| 412 | + | // handle both directions consistently. | |||
| 413 | + | if shouldPassthroughNativePageFocusNavigation(mode, keyval, modifiers) { | |||
| 414 | + | return false | |||
| 415 | + | } | |||
| 416 | + | ||||
| 410 | // Determine routing for this key event | 417 | // Determine routing for this key event | ||
| 411 | route := RouteHandleShortcuts // default: process through shortcut system | 418 | route := RouteHandleShortcuts // default: process through shortcut system | ||
| 412 | if routeKey != nil && mode == ModeNormal { | 419 | if routeKey != nil && mode == ModeNormal { | ||
| @@ -612,6 +619,18 @@ func normalizeKeyval(keyval uint) uint { | |||||
| 612 | return keyval | 619 | return keyval | ||
| 613 | } | 620 | } | ||
| 614 | 621 | ||||
| 622 | + | func shouldPassthroughNativePageFocusNavigation(mode Mode, keyval uint, modifiers Modifier) bool { | |||
| 623 | + | if mode != ModeNormal { | |||
| 624 | + | return false | |||
| 625 | + | } | |||
| 626 | + | switch keyval { | |||
| 627 | + | case uint(gdk.KEY_Tab), uint(gdk.KEY_ISO_Left_Tab): | |||
| 628 | + | return modifiers == ModNone || modifiers == ModShift | |||
| 629 | + | default: | |||
| 630 | + | return false | |||
| 631 | + | } | |||
| 632 | + | } | |||
| 633 | + | ||||
| 615 | func shouldPassthroughNativeVimModeNavigation(mode Mode, keyval uint, modifiers Modifier) bool { | 634 | func shouldPassthroughNativeVimModeNavigation(mode Mode, keyval uint, modifiers Modifier) bool { | ||
| 616 | if mode != ModeVim { | 635 | if mode != ModeVim { | ||
| 617 | return false | 636 | return false | ||
| @@ -619,9 +638,7 @@ func shouldPassthroughNativeVimModeNavigation(mode Mode, keyval uint, modifiers | |||||
| 619 | switch keyval { | 638 | switch keyval { | ||
| 620 | case uint(gdk.KEY_Left), uint(gdk.KEY_Right), uint(gdk.KEY_Up), uint(gdk.KEY_Down): | 639 | case uint(gdk.KEY_Left), uint(gdk.KEY_Right), uint(gdk.KEY_Up), uint(gdk.KEY_Down): | ||
| 621 | return modifiers == ModNone | 640 | return modifiers == ModNone | ||
| 622 | - | case uint(gdk.KEY_Tab): | 641 | + | case uint(gdk.KEY_Tab), uint(gdk.KEY_ISO_Left_Tab): |
| 623 | - | return modifiers == ModNone || modifiers == ModShift | |||
| 624 | - | case uint(gdk.KEY_ISO_Left_Tab): | |||
| 625 | return modifiers == ModNone || modifiers == ModShift | 642 | return modifiers == ModNone || modifiers == ModShift | ||
| 626 | default: | 643 | default: | ||
| 627 | return false | 644 | return false | ||
M internal/ui/input/keyboard_test.go +26 -0
| @@ -916,6 +916,32 @@ func TestHandleKeyPress_VimModeTabKeysPassThroughNatively(t *testing.T) { | |||
| 916 | 916 | } | |
| 917 | 917 | } | |
| 918 | 918 | ||
| 919 | + | func TestHandleKeyPress_NormalModeTabKeysPassThroughNatively(t *testing.T) { | |
| 920 | + | ctx := context.Background() | |
| 921 | + | workspace := newTestWorkspace() | |
| 922 | + | ||
| 923 | + | h := NewKeyboardHandler(ctx, workspace, newTestSession()) | |
| 924 | + | for _, test := range []struct { | |
| 925 | + | name string | |
| 926 | + | keyval uint | |
| 927 | + | modifiers gdk.ModifierType | |
| 928 | + | }{ | |
| 929 | + | {name: "tab", keyval: uint(gdk.KEY_Tab)}, | |
| 930 | + | {name: "shift tab", keyval: uint(gdk.KEY_Tab), modifiers: gdk.ShiftMaskValue}, | |
| 931 | + | {name: "iso left tab", keyval: uint(gdk.KEY_ISO_Left_Tab), modifiers: gdk.ShiftMaskValue}, | |
| 932 | + | } { | |
| 933 | + | t.Run(test.name, func(t *testing.T) { | |
| 934 | + | consumed := h.handleKeyPress(test.keyval, 0, test.modifiers) | |
| 935 | + | if consumed { | |
| 936 | + | t.Fatal("tab key should pass through to native page focus navigation in normal mode") | |
| 937 | + | } | |
| 938 | + | if h.Mode() != ModeNormal { | |
| 939 | + | t.Fatal("tab key should keep normal mode active") | |
| 940 | + | } | |
| 941 | + | }) | |
| 942 | + | } | |
| 943 | + | } | |
| 944 | + | ||
| 919 | 945 | func TestHandleKeyPress_VimModeBlocksGlobalShortcutFallback(t *testing.T) { | |
| 920 | 946 | ctx := context.Background() | |
| 921 | 947 | workspace := newTestWorkspace() | |
| @@ -916,6 +916,32 @@ func TestHandleKeyPress_VimModeTabKeysPassThroughNatively(t *testing.T) { | |||||
| 916 | } | 916 | } | ||
| 917 | } | 917 | } | ||
| 918 | 918 | ||||
| 919 | + | func TestHandleKeyPress_NormalModeTabKeysPassThroughNatively(t *testing.T) { | |||
| 920 | + | ctx := context.Background() | |||
| 921 | + | workspace := newTestWorkspace() | |||
| 922 | + | ||||
| 923 | + | h := NewKeyboardHandler(ctx, workspace, newTestSession()) | |||
| 924 | + | for _, test := range []struct { | |||
| 925 | + | name string | |||
| 926 | + | keyval uint | |||
| 927 | + | modifiers gdk.ModifierType | |||
| 928 | + | }{ | |||
| 929 | + | {name: "tab", keyval: uint(gdk.KEY_Tab)}, | |||
| 930 | + | {name: "shift tab", keyval: uint(gdk.KEY_Tab), modifiers: gdk.ShiftMaskValue}, | |||
| 931 | + | {name: "iso left tab", keyval: uint(gdk.KEY_ISO_Left_Tab), modifiers: gdk.ShiftMaskValue}, | |||
| 932 | + | } { | |||
| 933 | + | t.Run(test.name, func(t *testing.T) { | |||
| 934 | + | consumed := h.handleKeyPress(test.keyval, 0, test.modifiers) | |||
| 935 | + | if consumed { | |||
| 936 | + | t.Fatal("tab key should pass through to native page focus navigation in normal mode") | |||
| 937 | + | } | |||
| 938 | + | if h.Mode() != ModeNormal { | |||
| 939 | + | t.Fatal("tab key should keep normal mode active") | |||
| 940 | + | } | |||
| 941 | + | }) | |||
| 942 | + | } | |||
| 943 | + | } | |||
| 944 | + | ||||
| 919 | func TestHandleKeyPress_VimModeBlocksGlobalShortcutFallback(t *testing.T) { | 945 | func TestHandleKeyPress_VimModeBlocksGlobalShortcutFallback(t *testing.T) { | ||
| 920 | ctx := context.Background() | 946 | ctx := context.Background() | ||
| 921 | workspace := newTestWorkspace() | 947 | workspace := newTestWorkspace() | ||