fix(vim): keep sub-mode indicator and hide legend during page interactions
Starting link hints or visual selection reset the pending key sequence with notification, so the pending-sequence listener repainted the mode toast back to plain VIM MODE in the same instant the VISUAL or HINTS label was shown. The interaction itself kept running in the page, but the indicator told the user it had ended, and the full Vim Mode legend stayed on screen listing bindings that no longer applied. SetPageKeyCapture now resets the sequence silently, pending updates are ignored while a page interaction owns the keys, and the legend is suspended for the duration of the interaction and restored when it ends. Both release paths share one helper and log why the keys were released.
- commit
- 560f676bbce840b1cda9aa5dbd9b70b211bf82a6
- author
- brice · · 2026-10-04 10:31 UTC
- committer
- brice · · 2026-10-10 08:27 UTC
- parent
- 7db1f11
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +191 -35
M internal/ui/app.go +37 -16
| @@ -2253,13 +2253,47 @@ func (a *App) captureVimPageKeys(ctx context.Context, bw *browserWindow, wv port | |||
| 2253 | 2253 | return | |
| 2254 | 2254 | } | |
| 2255 | 2255 | bw.vimPageInteractionWebView = wv | |
| 2256 | - | a.showVimModeToast(ctx, bw, label) | |
| 2256 | + | // Capture first: it resets the pending sequence, and the toast and legend | |
| 2257 | + | // below must reflect the sub-mode, not a cleared sequence. | |
| 2257 | 2258 | bw.keyboardHandler.SetPageKeyCapture(func(key string) { | |
| 2258 | 2259 | if err := a.vimNavigationUseCase().SendPageKey(ctx, wv, key); err != nil { | |
| 2259 | 2260 | logging.FromContext(ctx).Debug().Err(err).Msg("vim page key forwarding failed") | |
| 2260 | 2261 | a.endVimPageInteraction(ctx, bw) | |
| 2261 | 2262 | } | |
| 2262 | 2263 | }) | |
| 2264 | + | a.showVimModeToast(ctx, bw, label) | |
| 2265 | + | if bw.modeFrame != nil { | |
| 2266 | + | bw.modeFrame.setSuspended(true) | |
| 2267 | + | } | |
| 2268 | + | logging.FromContext(ctx).Debug(). | |
| 2269 | + | Str("window_id", bw.id). | |
| 2270 | + | Str("sub_mode", label). | |
| 2271 | + | Msg("vim page interaction capturing keys") | |
| 2272 | + | } | |
| 2273 | + | ||
| 2274 | + | // releaseVimPageKeys returns this window's keys to Vim Mode bindings and | |
| 2275 | + | // restores the plain indicator and legend when the window is still in Vim | |
| 2276 | + | // Mode. It reports the WebView that owned the interaction, if any. | |
| 2277 | + | func (a *App) releaseVimPageKeys(ctx context.Context, bw *browserWindow, reason string) port.WebView { | |
| 2278 | + | wv := bw.vimPageInteractionWebView | |
| 2279 | + | bw.vimPageInteractionWebView = nil | |
| 2280 | + | if bw.keyboardHandler != nil { | |
| 2281 | + | bw.keyboardHandler.ClearPageKeyCapture() | |
| 2282 | + | } | |
| 2283 | + | if wv == nil { | |
| 2284 | + | return nil | |
| 2285 | + | } | |
| 2286 | + | if bw.keyboardHandler != nil && bw.keyboardHandler.Mode() == input.ModeVim { | |
| 2287 | + | a.showVimModeToast(ctx, bw, "") | |
| 2288 | + | if bw.modeFrame != nil { | |
| 2289 | + | bw.modeFrame.setSuspended(false) | |
| 2290 | + | } | |
| 2291 | + | } | |
| 2292 | + | logging.FromContext(ctx).Debug(). | |
| 2293 | + | Str("window_id", bw.id). | |
| 2294 | + | Str("reason", reason). | |
| 2295 | + | Msg("vim page interaction released keys") | |
| 2296 | + | return wv | |
| 2263 | 2297 | } | |
| 2264 | 2298 | ||
| 2265 | 2299 | // handleVimPageInteractionEnded releases key capture when the page reports | |
| @@ -2272,13 +2306,7 @@ func (a *App) handleVimPageInteractionEnded(ctx context.Context, paneID entity.P | |||
| 2272 | 2306 | if a.contentCoord.GetWebView(paneID) != bw.vimPageInteractionWebView { | |
| 2273 | 2307 | return | |
| 2274 | 2308 | } | |
| 2275 | - | bw.vimPageInteractionWebView = nil | |
| 2276 | - | if bw.keyboardHandler != nil { | |
| 2277 | - | bw.keyboardHandler.ClearPageKeyCapture() | |
| 2278 | - | if bw.keyboardHandler.Mode() == input.ModeVim { | |
| 2279 | - | a.showVimModeToast(ctx, bw, "") | |
| 2280 | - | } | |
| 2281 | - | } | |
| 2309 | + | a.releaseVimPageKeys(ctx, bw, "page-reported-end") | |
| 2282 | 2310 | } | |
| 2283 | 2311 | ||
| 2284 | 2312 | // vimPageInteractionLabel names the key-capturing sub-mode shown in the Vim | |
| @@ -2301,17 +2329,10 @@ func (a *App) endVimPageInteraction(ctx context.Context, bw *browserWindow) { | |||
| 2301 | 2329 | if bw == nil { | |
| 2302 | 2330 | return | |
| 2303 | 2331 | } | |
| 2304 | - | wv := bw.vimPageInteractionWebView | |
| 2305 | - | bw.vimPageInteractionWebView = nil | |
| 2306 | - | if bw.keyboardHandler != nil { | |
| 2307 | - | bw.keyboardHandler.ClearPageKeyCapture() | |
| 2308 | - | } | |
| 2332 | + | wv := a.releaseVimPageKeys(ctx, bw, "canceled") | |
| 2309 | 2333 | if wv == nil { | |
| 2310 | 2334 | return | |
| 2311 | 2335 | } | |
| 2312 | - | if bw.keyboardHandler != nil && bw.keyboardHandler.Mode() == input.ModeVim { | |
| 2313 | - | a.showVimModeToast(ctx, bw, "") | |
| 2314 | - | } | |
| 2315 | 2336 | if err := a.vimNavigationUseCase().CancelPageInteraction(ctx, wv); err != nil { | |
| 2316 | 2337 | logging.FromContext(ctx).Debug().Err(err).Msg("failed to cancel vim page interaction") | |
| 2317 | 2338 | } | |
| @@ -2253,13 +2253,47 @@ func (a *App) captureVimPageKeys(ctx context.Context, bw *browserWindow, wv port | |||||
| 2253 | return | 2253 | return | ||
| 2254 | } | 2254 | } | ||
| 2255 | bw.vimPageInteractionWebView = wv | 2255 | bw.vimPageInteractionWebView = wv | ||
| 2256 | - | a.showVimModeToast(ctx, bw, label) | 2256 | + | // Capture first: it resets the pending sequence, and the toast and legend |
| 2257 | + | // below must reflect the sub-mode, not a cleared sequence. | |||
| 2257 | bw.keyboardHandler.SetPageKeyCapture(func(key string) { | 2258 | bw.keyboardHandler.SetPageKeyCapture(func(key string) { | ||
| 2258 | if err := a.vimNavigationUseCase().SendPageKey(ctx, wv, key); err != nil { | 2259 | if err := a.vimNavigationUseCase().SendPageKey(ctx, wv, key); err != nil { | ||
| 2259 | logging.FromContext(ctx).Debug().Err(err).Msg("vim page key forwarding failed") | 2260 | logging.FromContext(ctx).Debug().Err(err).Msg("vim page key forwarding failed") | ||
| 2260 | a.endVimPageInteraction(ctx, bw) | 2261 | a.endVimPageInteraction(ctx, bw) | ||
| 2261 | } | 2262 | } | ||
| 2262 | }) | 2263 | }) | ||
| 2264 | + | a.showVimModeToast(ctx, bw, label) | |||
| 2265 | + | if bw.modeFrame != nil { | |||
| 2266 | + | bw.modeFrame.setSuspended(true) | |||
| 2267 | + | } | |||
| 2268 | + | logging.FromContext(ctx).Debug(). | |||
| 2269 | + | Str("window_id", bw.id). | |||
| 2270 | + | Str("sub_mode", label). | |||
| 2271 | + | Msg("vim page interaction capturing keys") | |||
| 2272 | + | } | |||
| 2273 | + | ||||
| 2274 | + | // releaseVimPageKeys returns this window's keys to Vim Mode bindings and | |||
| 2275 | + | // restores the plain indicator and legend when the window is still in Vim | |||
| 2276 | + | // Mode. It reports the WebView that owned the interaction, if any. | |||
| 2277 | + | func (a *App) releaseVimPageKeys(ctx context.Context, bw *browserWindow, reason string) port.WebView { | |||
| 2278 | + | wv := bw.vimPageInteractionWebView | |||
| 2279 | + | bw.vimPageInteractionWebView = nil | |||
| 2280 | + | if bw.keyboardHandler != nil { | |||
| 2281 | + | bw.keyboardHandler.ClearPageKeyCapture() | |||
| 2282 | + | } | |||
| 2283 | + | if wv == nil { | |||
| 2284 | + | return nil | |||
| 2285 | + | } | |||
| 2286 | + | if bw.keyboardHandler != nil && bw.keyboardHandler.Mode() == input.ModeVim { | |||
| 2287 | + | a.showVimModeToast(ctx, bw, "") | |||
| 2288 | + | if bw.modeFrame != nil { | |||
| 2289 | + | bw.modeFrame.setSuspended(false) | |||
| 2290 | + | } | |||
| 2291 | + | } | |||
| 2292 | + | logging.FromContext(ctx).Debug(). | |||
| 2293 | + | Str("window_id", bw.id). | |||
| 2294 | + | Str("reason", reason). | |||
| 2295 | + | Msg("vim page interaction released keys") | |||
| 2296 | + | return wv | |||
| 2263 | } | 2297 | } | ||
| 2264 | 2298 | ||||
| 2265 | // handleVimPageInteractionEnded releases key capture when the page reports | 2299 | // handleVimPageInteractionEnded releases key capture when the page reports | ||
| @@ -2272,13 +2306,7 @@ func (a *App) handleVimPageInteractionEnded(ctx context.Context, paneID entity.P | |||||
| 2272 | if a.contentCoord.GetWebView(paneID) != bw.vimPageInteractionWebView { | 2306 | if a.contentCoord.GetWebView(paneID) != bw.vimPageInteractionWebView { | ||
| 2273 | return | 2307 | return | ||
| 2274 | } | 2308 | } | ||
| 2275 | - | bw.vimPageInteractionWebView = nil | 2309 | + | a.releaseVimPageKeys(ctx, bw, "page-reported-end") |
| 2276 | - | if bw.keyboardHandler != nil { | |||
| 2277 | - | bw.keyboardHandler.ClearPageKeyCapture() | |||
| 2278 | - | if bw.keyboardHandler.Mode() == input.ModeVim { | |||
| 2279 | - | a.showVimModeToast(ctx, bw, "") | |||
| 2280 | - | } | |||
| 2281 | - | } | |||
| 2282 | } | 2310 | } | ||
| 2283 | 2311 | ||||
| 2284 | // vimPageInteractionLabel names the key-capturing sub-mode shown in the Vim | 2312 | // vimPageInteractionLabel names the key-capturing sub-mode shown in the Vim | ||
| @@ -2301,17 +2329,10 @@ func (a *App) endVimPageInteraction(ctx context.Context, bw *browserWindow) { | |||||
| 2301 | if bw == nil { | 2329 | if bw == nil { | ||
| 2302 | return | 2330 | return | ||
| 2303 | } | 2331 | } | ||
| 2304 | - | wv := bw.vimPageInteractionWebView | 2332 | + | wv := a.releaseVimPageKeys(ctx, bw, "canceled") |
| 2305 | - | bw.vimPageInteractionWebView = nil | |||
| 2306 | - | if bw.keyboardHandler != nil { | |||
| 2307 | - | bw.keyboardHandler.ClearPageKeyCapture() | |||
| 2308 | - | } | |||
| 2309 | if wv == nil { | 2333 | if wv == nil { | ||
| 2310 | return | 2334 | return | ||
| 2311 | } | 2335 | } | ||
| 2312 | - | if bw.keyboardHandler != nil && bw.keyboardHandler.Mode() == input.ModeVim { | |||
| 2313 | - | a.showVimModeToast(ctx, bw, "") | |||
| 2314 | - | } | |||
| 2315 | if err := a.vimNavigationUseCase().CancelPageInteraction(ctx, wv); err != nil { | 2336 | if err := a.vimNavigationUseCase().CancelPageInteraction(ctx, wv); err != nil { | ||
| 2316 | logging.FromContext(ctx).Debug().Err(err).Msg("failed to cancel vim page interaction") | 2337 | logging.FromContext(ctx).Debug().Err(err).Msg("failed to cancel vim page interaction") | ||
| 2317 | } | 2338 | } | ||
A internal/ui/app_vim_page_test.go +89 -0
| @@ -0,0 +1,89 @@ | |||
| 1 | + | package ui | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | "github.com/stretchr/testify/mock" | |
| 8 | + | "github.com/stretchr/testify/require" | |
| 9 | + | ||
| 10 | + | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |
| 11 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 12 | + | "github.com/bnema/dumber/internal/ui/input" | |
| 13 | + | "github.com/bnema/dumber/internal/ui/layout/mocks" | |
| 14 | + | ) | |
| 15 | + | ||
| 16 | + | // newVimPageTestWindow builds a window in Vim Mode with a real mode toaster | |
| 17 | + | // bound to the pending-sequence callback, as initChrome does. It records every | |
| 18 | + | // text shown on the toast so tests can assert what the user sees. | |
| 19 | + | func newVimPageTestWindow(t *testing.T) (*App, *browserWindow, *[]string) { | |
| 20 | + | t.Helper() | |
| 21 | + | factory := mocks.NewMockWidgetFactory(t) | |
| 22 | + | toaster, container, label := newModeToasterForVimModeTest(t, factory) | |
| 23 | + | shown := &[]string{} | |
| 24 | + | container.EXPECT().RemoveCssClass(mock.Anything).Maybe() | |
| 25 | + | container.EXPECT().AddCssClass(mock.Anything).Maybe() | |
| 26 | + | container.EXPECT().SetHalign(mock.Anything).Maybe() | |
| 27 | + | container.EXPECT().SetValign(mock.Anything).Maybe() | |
| 28 | + | container.EXPECT().SetVisible(mock.Anything).Maybe() | |
| 29 | + | label.EXPECT().SetText(mock.Anything).Run(func(text string) { *shown = append(*shown, text) }).Maybe() | |
| 30 | + | ||
| 31 | + | app := &App{runtimeConfig: runtimeConfigStateFromSnapshotForTest(entity.RuntimeConfigSnapshot{ | |
| 32 | + | UI: entity.RuntimeUIConfig{Workspace: entity.WorkspaceConfig{Styling: entity.WorkspaceStylingConfig{ | |
| 33 | + | ModeIndicatorToasterEnabled: true, | |
| 34 | + | }}}, | |
| 35 | + | })} | |
| 36 | + | bw := &browserWindow{id: "win-1", modeToaster: toaster} | |
| 37 | + | app.browserWindows = map[string]*browserWindow{bw.id: bw} | |
| 38 | + | bw.keyboardHandler = newKeyboardHandlerInVimMode(t) | |
| 39 | + | app.bindVimModeSequenceToaster(context.Background(), bw) | |
| 40 | + | return app, bw, shown | |
| 41 | + | } | |
| 42 | + | ||
| 43 | + | // Starting a key-capturing interaction must leave the sub-mode label on the | |
| 44 | + | // toast. The capture path used to reset the pending sequence with | |
| 45 | + | // notification, which repainted the toast back to plain "VIM MODE" in the | |
| 46 | + | // same instant and made the interaction look as if it had ended. | |
| 47 | + | func TestCaptureVimPageKeysKeepsSubModeLabelOnToast(t *testing.T) { | |
| 48 | + | app, bw, shown := newVimPageTestWindow(t) | |
| 49 | + | wv := portmocks.NewMockWebView(t) | |
| 50 | + | ||
| 51 | + | app.captureVimPageKeys(context.Background(), bw, wv, "VISUAL") | |
| 52 | + | ||
| 53 | + | require.NotEmpty(t, *shown) | |
| 54 | + | require.Equal(t, "VIM MODE · VISUAL", (*shown)[len(*shown)-1]) | |
| 55 | + | require.True(t, bw.keyboardHandler.PageKeyCaptureActive()) | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | // A completed sequence is reported with an empty pending string before its | |
| 59 | + | // action runs. When that action starts a page interaction, the stale pending | |
| 60 | + | // notification must not reach the toast after the sub-mode label. | |
| 61 | + | func TestVimPageSubModeLabelSurvivesSequenceCompletion(t *testing.T) { | |
| 62 | + | app, bw, shown := newVimPageTestWindow(t) | |
| 63 | + | wv := portmocks.NewMockWebView(t) | |
| 64 | + | ||
| 65 | + | app.showPendingSequence(context.Background(), bw, "") | |
| 66 | + | app.captureVimPageKeys(context.Background(), bw, wv, "HINTS") | |
| 67 | + | // Any pending notification emitted while the page owns the keys is noise. | |
| 68 | + | app.showPendingSequence(context.Background(), bw, "") | |
| 69 | + | ||
| 70 | + | require.Equal(t, "VIM MODE · HINTS", (*shown)[len(*shown)-1]) | |
| 71 | + | } | |
| 72 | + | ||
| 73 | + | // The legend is a key reference for Vim Mode bindings; while the page owns the | |
| 74 | + | // keys it would describe keys that do not apply, so it must be hidden and | |
| 75 | + | // restored when the interaction ends. | |
| 76 | + | func TestVimPageInteractionHidesLegendWhileCaptured(t *testing.T) { | |
| 77 | + | app, bw, _ := newVimPageTestWindow(t) | |
| 78 | + | frame := &modeFrame{mode: input.ModeVim, visible: true} | |
| 79 | + | bw.modeFrame = frame | |
| 80 | + | wv := portmocks.NewMockWebView(t) | |
| 81 | + | ||
| 82 | + | app.captureVimPageKeys(context.Background(), bw, wv, "VISUAL") | |
| 83 | + | require.True(t, frame.suspended, "legend must be suspended while the page captures keys") | |
| 84 | + | ||
| 85 | + | wv.EXPECT().ID().Return(1).Maybe() | |
| 86 | + | app.endVimPageInteraction(context.Background(), bw) | |
| 87 | + | require.False(t, frame.suspended, "legend must be restored when the interaction ends") | |
| 88 | + | require.False(t, bw.keyboardHandler.PageKeyCaptureActive()) | |
| 89 | + | } | |
| @@ -0,0 +1,89 @@ | |||||
| 1 | + | package ui | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | "github.com/stretchr/testify/mock" | |||
| 8 | + | "github.com/stretchr/testify/require" | |||
| 9 | + | ||||
| 10 | + | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |||
| 11 | + | "github.com/bnema/dumber/internal/domain/entity" | |||
| 12 | + | "github.com/bnema/dumber/internal/ui/input" | |||
| 13 | + | "github.com/bnema/dumber/internal/ui/layout/mocks" | |||
| 14 | + | ) | |||
| 15 | + | ||||
| 16 | + | // newVimPageTestWindow builds a window in Vim Mode with a real mode toaster | |||
| 17 | + | // bound to the pending-sequence callback, as initChrome does. It records every | |||
| 18 | + | // text shown on the toast so tests can assert what the user sees. | |||
| 19 | + | func newVimPageTestWindow(t *testing.T) (*App, *browserWindow, *[]string) { | |||
| 20 | + | t.Helper() | |||
| 21 | + | factory := mocks.NewMockWidgetFactory(t) | |||
| 22 | + | toaster, container, label := newModeToasterForVimModeTest(t, factory) | |||
| 23 | + | shown := &[]string{} | |||
| 24 | + | container.EXPECT().RemoveCssClass(mock.Anything).Maybe() | |||
| 25 | + | container.EXPECT().AddCssClass(mock.Anything).Maybe() | |||
| 26 | + | container.EXPECT().SetHalign(mock.Anything).Maybe() | |||
| 27 | + | container.EXPECT().SetValign(mock.Anything).Maybe() | |||
| 28 | + | container.EXPECT().SetVisible(mock.Anything).Maybe() | |||
| 29 | + | label.EXPECT().SetText(mock.Anything).Run(func(text string) { *shown = append(*shown, text) }).Maybe() | |||
| 30 | + | ||||
| 31 | + | app := &App{runtimeConfig: runtimeConfigStateFromSnapshotForTest(entity.RuntimeConfigSnapshot{ | |||
| 32 | + | UI: entity.RuntimeUIConfig{Workspace: entity.WorkspaceConfig{Styling: entity.WorkspaceStylingConfig{ | |||
| 33 | + | ModeIndicatorToasterEnabled: true, | |||
| 34 | + | }}}, | |||
| 35 | + | })} | |||
| 36 | + | bw := &browserWindow{id: "win-1", modeToaster: toaster} | |||
| 37 | + | app.browserWindows = map[string]*browserWindow{bw.id: bw} | |||
| 38 | + | bw.keyboardHandler = newKeyboardHandlerInVimMode(t) | |||
| 39 | + | app.bindVimModeSequenceToaster(context.Background(), bw) | |||
| 40 | + | return app, bw, shown | |||
| 41 | + | } | |||
| 42 | + | ||||
| 43 | + | // Starting a key-capturing interaction must leave the sub-mode label on the | |||
| 44 | + | // toast. The capture path used to reset the pending sequence with | |||
| 45 | + | // notification, which repainted the toast back to plain "VIM MODE" in the | |||
| 46 | + | // same instant and made the interaction look as if it had ended. | |||
| 47 | + | func TestCaptureVimPageKeysKeepsSubModeLabelOnToast(t *testing.T) { | |||
| 48 | + | app, bw, shown := newVimPageTestWindow(t) | |||
| 49 | + | wv := portmocks.NewMockWebView(t) | |||
| 50 | + | ||||
| 51 | + | app.captureVimPageKeys(context.Background(), bw, wv, "VISUAL") | |||
| 52 | + | ||||
| 53 | + | require.NotEmpty(t, *shown) | |||
| 54 | + | require.Equal(t, "VIM MODE · VISUAL", (*shown)[len(*shown)-1]) | |||
| 55 | + | require.True(t, bw.keyboardHandler.PageKeyCaptureActive()) | |||
| 56 | + | } | |||
| 57 | + | ||||
| 58 | + | // A completed sequence is reported with an empty pending string before its | |||
| 59 | + | // action runs. When that action starts a page interaction, the stale pending | |||
| 60 | + | // notification must not reach the toast after the sub-mode label. | |||
| 61 | + | func TestVimPageSubModeLabelSurvivesSequenceCompletion(t *testing.T) { | |||
| 62 | + | app, bw, shown := newVimPageTestWindow(t) | |||
| 63 | + | wv := portmocks.NewMockWebView(t) | |||
| 64 | + | ||||
| 65 | + | app.showPendingSequence(context.Background(), bw, "") | |||
| 66 | + | app.captureVimPageKeys(context.Background(), bw, wv, "HINTS") | |||
| 67 | + | // Any pending notification emitted while the page owns the keys is noise. | |||
| 68 | + | app.showPendingSequence(context.Background(), bw, "") | |||
| 69 | + | ||||
| 70 | + | require.Equal(t, "VIM MODE · HINTS", (*shown)[len(*shown)-1]) | |||
| 71 | + | } | |||
| 72 | + | ||||
| 73 | + | // The legend is a key reference for Vim Mode bindings; while the page owns the | |||
| 74 | + | // keys it would describe keys that do not apply, so it must be hidden and | |||
| 75 | + | // restored when the interaction ends. | |||
| 76 | + | func TestVimPageInteractionHidesLegendWhileCaptured(t *testing.T) { | |||
| 77 | + | app, bw, _ := newVimPageTestWindow(t) | |||
| 78 | + | frame := &modeFrame{mode: input.ModeVim, visible: true} | |||
| 79 | + | bw.modeFrame = frame | |||
| 80 | + | wv := portmocks.NewMockWebView(t) | |||
| 81 | + | ||||
| 82 | + | app.captureVimPageKeys(context.Background(), bw, wv, "VISUAL") | |||
| 83 | + | require.True(t, frame.suspended, "legend must be suspended while the page captures keys") | |||
| 84 | + | ||||
| 85 | + | wv.EXPECT().ID().Return(1).Maybe() | |||
| 86 | + | app.endVimPageInteraction(context.Background(), bw) | |||
| 87 | + | require.False(t, frame.suspended, "legend must be restored when the interaction ends") | |||
| 88 | + | require.False(t, bw.keyboardHandler.PageKeyCaptureActive()) | |||
| 89 | + | } | |||
M internal/ui/input/vim_page_capture.go +4 -2
| @@ -11,9 +11,11 @@ type PageKeyForwarder func(key string) | |||
| 11 | 11 | ||
| 12 | 12 | // SetPageKeyCapture routes Vim Mode keys to fn until ClearPageKeyCapture or | |
| 13 | 13 | // Vim Mode exits. Pending sequences are discarded so they cannot complete | |
| 14 | - | // against keys meant for the page. | |
| 14 | + | // against keys meant for the page. The reset is silent: the pending listener | |
| 15 | + | // repaints the mode indicator, and a notification here would overwrite the | |
| 16 | + | // sub-mode label the caller shows for the interaction. | |
| 15 | 17 | func (h *KeyboardHandler) SetPageKeyCapture(fn PageKeyForwarder) { | |
| 16 | - | h.ResetPendingSequence() | |
| 18 | + | h.resetPendingSequence(false) | |
| 17 | 19 | h.mu.Lock() | |
| 18 | 20 | h.pageKeyCapture = fn | |
| 19 | 21 | h.mu.Unlock() | |
| @@ -11,9 +11,11 @@ type PageKeyForwarder func(key string) | |||||
| 11 | 11 | ||||
| 12 | // SetPageKeyCapture routes Vim Mode keys to fn until ClearPageKeyCapture or | 12 | // SetPageKeyCapture routes Vim Mode keys to fn until ClearPageKeyCapture or | ||
| 13 | // Vim Mode exits. Pending sequences are discarded so they cannot complete | 13 | // Vim Mode exits. Pending sequences are discarded so they cannot complete | ||
| 14 | - | // against keys meant for the page. | 14 | + | // against keys meant for the page. The reset is silent: the pending listener |
| 15 | + | // repaints the mode indicator, and a notification here would overwrite the | |||
| 16 | + | // sub-mode label the caller shows for the interaction. | |||
| 15 | func (h *KeyboardHandler) SetPageKeyCapture(fn PageKeyForwarder) { | 17 | func (h *KeyboardHandler) SetPageKeyCapture(fn PageKeyForwarder) { | ||
| 16 | - | h.ResetPendingSequence() | 18 | + | h.resetPendingSequence(false) |
| 17 | h.mu.Lock() | 19 | h.mu.Lock() | ||
| 18 | h.pageKeyCapture = fn | 20 | h.pageKeyCapture = fn | ||
| 19 | h.mu.Unlock() | 21 | h.mu.Unlock() | ||
M internal/ui/input/vim_page_capture_test.go +14 -0
| @@ -68,3 +68,17 @@ func TestPageKeyCaptureModifiedEscapeForwardsPlainEscape(t *testing.T) { | |||
| 68 | 68 | assert.Equal(t, []string{"<Escape>"}, forwarded) | |
| 69 | 69 | assert.False(t, h.PageKeyCaptureActive()) | |
| 70 | 70 | } | |
| 71 | + | ||
| 72 | + | // Capturing keys for the page must not report a cleared pending sequence: | |
| 73 | + | // the pending callback repaints the mode toast and would erase the sub-mode | |
| 74 | + | // label shown an instant earlier. | |
| 75 | + | func TestSetPageKeyCaptureDoesNotNotifyPendingListeners(t *testing.T) { | |
| 76 | + | h := NewKeyboardHandler(context.Background(), vimModeSequenceWorkspace(nil), newTestSession()) | |
| 77 | + | enterVimMode(t, h) | |
| 78 | + | pending := capturePending(h) | |
| 79 | + | ||
| 80 | + | h.SetPageKeyCapture(func(string) {}) | |
| 81 | + | ||
| 82 | + | assert.Empty(t, *pending) | |
| 83 | + | assert.Empty(t, h.PendingSequence()) | |
| 84 | + | } | |
| @@ -68,3 +68,17 @@ func TestPageKeyCaptureModifiedEscapeForwardsPlainEscape(t *testing.T) { | |||||
| 68 | assert.Equal(t, []string{"<Escape>"}, forwarded) | 68 | assert.Equal(t, []string{"<Escape>"}, forwarded) | ||
| 69 | assert.False(t, h.PageKeyCaptureActive()) | 69 | assert.False(t, h.PageKeyCaptureActive()) | ||
| 70 | } | 70 | } | ||
| 71 | + | ||||
| 72 | + | // Capturing keys for the page must not report a cleared pending sequence: | |||
| 73 | + | // the pending callback repaints the mode toast and would erase the sub-mode | |||
| 74 | + | // label shown an instant earlier. | |||
| 75 | + | func TestSetPageKeyCaptureDoesNotNotifyPendingListeners(t *testing.T) { | |||
| 76 | + | h := NewKeyboardHandler(context.Background(), vimModeSequenceWorkspace(nil), newTestSession()) | |||
| 77 | + | enterVimMode(t, h) | |||
| 78 | + | pending := capturePending(h) | |||
| 79 | + | ||||
| 80 | + | h.SetPageKeyCapture(func(string) {}) | |||
| 81 | + | ||||
| 82 | + | assert.Empty(t, *pending) | |||
| 83 | + | assert.Empty(t, h.PendingSequence()) | |||
| 84 | + | } | |||
M internal/ui/mode_frame.go +42 -17
| @@ -26,22 +26,25 @@ const ( | |||
| 26 | 26 | // modeFrame owns the per-window mode border and its non-interactive legend. | |
| 27 | 27 | // GTK access is restricted to the main thread, as with the rest of the UI. | |
| 28 | 28 | type modeFrame struct { | |
| 29 | - | mode input.Mode | |
| 30 | - | tabID entity.TabID | |
| 31 | - | target layout.Widget | |
| 32 | - | frameClass string | |
| 33 | - | root *gtk.Box | |
| 34 | - | border *gtk.Box | |
| 35 | - | borderStyle cssClassTarget // border's CSS classes; a seam for tests | |
| 36 | - | borderRect func() (x, y, width, height int, ok bool) | |
| 37 | - | panel *gtk.Box | |
| 38 | - | content *gtk.FlowBox | |
| 39 | - | scroller *gtk.ScrolledWindow | |
| 40 | - | heading *gtk.Label | |
| 41 | - | rows map[string][]*gtk.Widget | |
| 42 | - | keycaps map[string][]*gtk.Widget | |
| 43 | - | pending string | |
| 44 | - | visible bool | |
| 29 | + | mode input.Mode | |
| 30 | + | tabID entity.TabID | |
| 31 | + | target layout.Widget | |
| 32 | + | frameClass string | |
| 33 | + | root *gtk.Box | |
| 34 | + | border *gtk.Box | |
| 35 | + | borderStyle cssClassTarget // border's CSS classes; a seam for tests | |
| 36 | + | borderRect func() (x, y, width, height int, ok bool) | |
| 37 | + | panel *gtk.Box | |
| 38 | + | content *gtk.FlowBox | |
| 39 | + | scroller *gtk.ScrolledWindow | |
| 40 | + | heading *gtk.Label | |
| 41 | + | rows map[string][]*gtk.Widget | |
| 42 | + | keycaps map[string][]*gtk.Widget | |
| 43 | + | pending string | |
| 44 | + | visible bool | |
| 45 | + | // suspended hides the legend while an in-page interaction (link hints, | |
| 46 | + | // visual selection) owns the keys; the border keeps tracking its target. | |
| 47 | + | suspended bool | |
| 45 | 48 | style string | |
| 46 | 49 | modeClass string | |
| 47 | 50 | config entity.WorkspaceStylingConfig | |
| @@ -330,6 +333,7 @@ func (f *modeFrame) setMode( | |||
| 330 | 333 | f.mode, f.config = mode, cfg | |
| 331 | 334 | f.pending = "" | |
| 332 | 335 | f.pulseCycle = false | |
| 336 | + | f.suspended = false | |
| 333 | 337 | if linger { | |
| 334 | 338 | f.lingering = true | |
| 335 | 339 | f.visible = false | |
| @@ -431,6 +435,27 @@ func shouldStartLinger(exiting, pressInFlight, enabled, inside bool, x, y float6 | |||
| 431 | 435 | return exiting && !pressInFlight && shouldLinger(enabled, inside, x, y, placement) | |
| 432 | 436 | } | |
| 433 | 437 | ||
| 438 | + | // setSuspended hides the legend while suspended is true and brings it back | |
| 439 | + | // otherwise, without touching the mode, its border, or a pending show timer. | |
| 440 | + | // Used while an in-page interaction owns the keys, when the legend would list | |
| 441 | + | // bindings that do not apply. | |
| 442 | + | func (f *modeFrame) setSuspended(suspended bool) { | |
| 443 | + | if f == nil || f.suspended == suspended { | |
| 444 | + | return | |
| 445 | + | } | |
| 446 | + | f.suspended = suspended | |
| 447 | + | if f.root == nil || f.lingering { | |
| 448 | + | return | |
| 449 | + | } | |
| 450 | + | if suspended { | |
| 451 | + | f.clearLegend() | |
| 452 | + | return | |
| 453 | + | } | |
| 454 | + | if f.visible { | |
| 455 | + | f.refreshGeometry() | |
| 456 | + | } | |
| 457 | + | } | |
| 458 | + | ||
| 434 | 459 | func (f *modeFrame) dismissLinger() { | |
| 435 | 460 | if f == nil || !f.lingering { | |
| 436 | 461 | return | |
| @@ -554,7 +579,7 @@ func (f *modeFrame) refreshGeometry() { | |||
| 554 | 579 | } | |
| 555 | 580 | x, bottom := int(out.X), int(out.Y)+height | |
| 556 | 581 | f.placeBorder(x, int(out.Y), width, height) | |
| 557 | - | if !f.visible { | |
| 582 | + | if !f.visible || f.suspended { | |
| 558 | 583 | f.clearLegend() | |
| 559 | 584 | return | |
| 560 | 585 | } | |
| @@ -26,22 +26,25 @@ const ( | |||||
| 26 | // modeFrame owns the per-window mode border and its non-interactive legend. | 26 | // modeFrame owns the per-window mode border and its non-interactive legend. | ||
| 27 | // GTK access is restricted to the main thread, as with the rest of the UI. | 27 | // GTK access is restricted to the main thread, as with the rest of the UI. | ||
| 28 | type modeFrame struct { | 28 | type modeFrame struct { | ||
| 29 | - | mode input.Mode | 29 | + | mode input.Mode |
| 30 | - | tabID entity.TabID | 30 | + | tabID entity.TabID |
| 31 | - | target layout.Widget | 31 | + | target layout.Widget |
| 32 | - | frameClass string | 32 | + | frameClass string |
| 33 | - | root *gtk.Box | 33 | + | root *gtk.Box |
| 34 | - | border *gtk.Box | 34 | + | border *gtk.Box |
| 35 | - | borderStyle cssClassTarget // border's CSS classes; a seam for tests | 35 | + | borderStyle cssClassTarget // border's CSS classes; a seam for tests |
| 36 | - | borderRect func() (x, y, width, height int, ok bool) | 36 | + | borderRect func() (x, y, width, height int, ok bool) |
| 37 | - | panel *gtk.Box | 37 | + | panel *gtk.Box |
| 38 | - | content *gtk.FlowBox | 38 | + | content *gtk.FlowBox |
| 39 | - | scroller *gtk.ScrolledWindow | 39 | + | scroller *gtk.ScrolledWindow |
| 40 | - | heading *gtk.Label | 40 | + | heading *gtk.Label |
| 41 | - | rows map[string][]*gtk.Widget | 41 | + | rows map[string][]*gtk.Widget |
| 42 | - | keycaps map[string][]*gtk.Widget | 42 | + | keycaps map[string][]*gtk.Widget |
| 43 | - | pending string | 43 | + | pending string |
| 44 | - | visible bool | 44 | + | visible bool |
| 45 | + | // suspended hides the legend while an in-page interaction (link hints, | |||
| 46 | + | // visual selection) owns the keys; the border keeps tracking its target. | |||
| 47 | + | suspended bool | |||
| 45 | style string | 48 | style string | ||
| 46 | modeClass string | 49 | modeClass string | ||
| 47 | config entity.WorkspaceStylingConfig | 50 | config entity.WorkspaceStylingConfig | ||
| @@ -330,6 +333,7 @@ func (f *modeFrame) setMode( | |||||
| 330 | f.mode, f.config = mode, cfg | 333 | f.mode, f.config = mode, cfg | ||
| 331 | f.pending = "" | 334 | f.pending = "" | ||
| 332 | f.pulseCycle = false | 335 | f.pulseCycle = false | ||
| 336 | + | f.suspended = false | |||
| 333 | if linger { | 337 | if linger { | ||
| 334 | f.lingering = true | 338 | f.lingering = true | ||
| 335 | f.visible = false | 339 | f.visible = false | ||
| @@ -431,6 +435,27 @@ func shouldStartLinger(exiting, pressInFlight, enabled, inside bool, x, y float6 | |||||
| 431 | return exiting && !pressInFlight && shouldLinger(enabled, inside, x, y, placement) | 435 | return exiting && !pressInFlight && shouldLinger(enabled, inside, x, y, placement) | ||
| 432 | } | 436 | } | ||
| 433 | 437 | ||||
| 438 | + | // setSuspended hides the legend while suspended is true and brings it back | |||
| 439 | + | // otherwise, without touching the mode, its border, or a pending show timer. | |||
| 440 | + | // Used while an in-page interaction owns the keys, when the legend would list | |||
| 441 | + | // bindings that do not apply. | |||
| 442 | + | func (f *modeFrame) setSuspended(suspended bool) { | |||
| 443 | + | if f == nil || f.suspended == suspended { | |||
| 444 | + | return | |||
| 445 | + | } | |||
| 446 | + | f.suspended = suspended | |||
| 447 | + | if f.root == nil || f.lingering { | |||
| 448 | + | return | |||
| 449 | + | } | |||
| 450 | + | if suspended { | |||
| 451 | + | f.clearLegend() | |||
| 452 | + | return | |||
| 453 | + | } | |||
| 454 | + | if f.visible { | |||
| 455 | + | f.refreshGeometry() | |||
| 456 | + | } | |||
| 457 | + | } | |||
| 458 | + | ||||
| 434 | func (f *modeFrame) dismissLinger() { | 459 | func (f *modeFrame) dismissLinger() { | ||
| 435 | if f == nil || !f.lingering { | 460 | if f == nil || !f.lingering { | ||
| 436 | return | 461 | return | ||
| @@ -554,7 +579,7 @@ func (f *modeFrame) refreshGeometry() { | |||||
| 554 | } | 579 | } | ||
| 555 | x, bottom := int(out.X), int(out.Y)+height | 580 | x, bottom := int(out.X), int(out.Y)+height | ||
| 556 | f.placeBorder(x, int(out.Y), width, height) | 581 | f.placeBorder(x, int(out.Y), width, height) | ||
| 557 | - | if !f.visible { | 582 | + | if !f.visible || f.suspended { |
| 558 | f.clearLegend() | 583 | f.clearLegend() | ||
| 559 | return | 584 | return | ||
| 560 | } | 585 | } | ||
M internal/ui/vim_mode.go +5 -0
| @@ -316,6 +316,11 @@ func (a *App) showPendingSequence(ctx context.Context, bw *browserWindow, pendin | |||
| 316 | 316 | if a == nil || bw == nil { | |
| 317 | 317 | return | |
| 318 | 318 | } | |
| 319 | + | if bw.vimPageInteractionWebView != nil { | |
| 320 | + | // The page owns the keys: the indicator shows the interaction's | |
| 321 | + | // sub-mode, and the legend is suspended. Nothing to repaint. | |
| 322 | + | return | |
| 323 | + | } | |
| 319 | 324 | if bw.modeFrame != nil { | |
| 320 | 325 | cfg := a.runtimeConfigSnapshot().UI.Workspace | |
| 321 | 326 | bw.modeFrame.setPending(pending, cfg.VimMode.Actions) | |
| @@ -316,6 +316,11 @@ func (a *App) showPendingSequence(ctx context.Context, bw *browserWindow, pendin | |||||
| 316 | if a == nil || bw == nil { | 316 | if a == nil || bw == nil { | ||
| 317 | return | 317 | return | ||
| 318 | } | 318 | } | ||
| 319 | + | if bw.vimPageInteractionWebView != nil { | |||
| 320 | + | // The page owns the keys: the indicator shows the interaction's | |||
| 321 | + | // sub-mode, and the legend is suspended. Nothing to repaint. | |||
| 322 | + | return | |||
| 323 | + | } | |||
| 319 | if bw.modeFrame != nil { | 324 | if bw.modeFrame != nil { | ||
| 320 | cfg := a.runtimeConfigSnapshot().UI.Workspace | 325 | cfg := a.runtimeConfigSnapshot().UI.Workspace | ||
| 321 | bw.modeFrame.setPending(pending, cfg.VimMode.Actions) | 326 | bw.modeFrame.setPending(pending, cfg.VimMode.Actions) | ||