detached 7bf171b

Preserve UI state for stale pane IDs

commit
7bf171bdfc53a1cd3c1c7fbac76da6212eccc298
author
brice · · 2026-05-11 17:26 UTC
parent
e79a598
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +94 -6
Split
UnifiedSplit
M internal/ui/app.go +8 -0
@@ -3810,6 +3810,14 @@ func (a *App) attachPopupToTab(ctx context.Context, tabID entity.TabID, pane *en
38103810 log.Warn().Str("tab_id", string(tabID)).Msg("workspace view not found for popup tab")
38113811 return
38123812 }
3813+ if pane == nil {
3814+ log.Warn().Str("tab_id", string(tabID)).Msg("popup pane is nil")
3815+ return
3816+ }
3817+ if wsView.GetPaneView(pane.ID) == nil {
3818+ log.Warn().Str("pane_id", string(pane.ID)).Msg("pane view not found for popup")
3819+ return
3820+ }
38133821
38143822 // Register WebView with content coordinator
38153823 if a.contentCoord != nil {
@@ -3810,6 +3810,14 @@ func (a *App) attachPopupToTab(ctx context.Context, tabID entity.TabID, pane *en
3810 log.Warn().Str("tab_id", string(tabID)).Msg("workspace view not found for popup tab")3810 log.Warn().Str("tab_id", string(tabID)).Msg("workspace view not found for popup tab")
3811 return3811 return
3812 }3812 }
3813+ if pane == nil {
3814+ log.Warn().Str("tab_id", string(tabID)).Msg("popup pane is nil")
3815+ return
3816+ }
3817+ if wsView.GetPaneView(pane.ID) == nil {
3818+ log.Warn().Str("pane_id", string(pane.ID)).Msg("pane view not found for popup")
3819+ return
3820+ }
3813 3821
3814 // Register WebView with content coordinator3822 // Register WebView with content coordinator
3815 if a.contentCoord != nil {3823 if a.contentCoord != nil {
M internal/ui/app_browser_launch_test.go +19 -0
@@ -1835,6 +1835,25 @@ func (f *fakeRecordingWebView) IsPlayingAudio() bool { retu
18351835 func (f *fakeRecordingWebView) IsDestroyed() bool { return false }
18361836 func (f *fakeRecordingWebView) Destroy() {}
18371837
1838+func TestApp_AttachPopupToTabSkipsRegistrationWhenPaneViewMissing(t *testing.T) {
1839+ ctx := context.Background()
1840+ contentCoord := contentcoord.NewCoordinator(ctx, nil, nil, nil, nil, nil, nil, nil)
1841+ tabID := entity.TabID("tab-1")
1842+ pane := entity.NewPane(entity.PaneID("missing-pane"))
1843+ app := &App{
1844+ contentCoord: contentCoord,
1845+ workspaceViews: map[entity.TabID]*component.WorkspaceView{
1846+ tabID: &component.WorkspaceView{},
1847+ },
1848+ }
1849+
1850+ app.attachPopupToTab(ctx, tabID, pane, &fakeRecordingWebView{id: 1})
1851+
1852+ if got := contentCoord.GetWebView(pane.ID); got != nil {
1853+ t.Fatalf("popup webview was registered for missing pane view: %v", got)
1854+ }
1855+}
1856+
18381857 type fakeZoomRepo struct{}
18391858
18401859 func (f *fakeZoomRepo) Get(context.Context, string) (*entity.ZoomLevel, error) { return nil, nil }
@@ -1835,6 +1835,25 @@ func (f *fakeRecordingWebView) IsPlayingAudio() bool { retu
1835 func (f *fakeRecordingWebView) IsDestroyed() bool { return false }1835 func (f *fakeRecordingWebView) IsDestroyed() bool { return false }
1836 func (f *fakeRecordingWebView) Destroy() {}1836 func (f *fakeRecordingWebView) Destroy() {}
1837 1837
1838+func TestApp_AttachPopupToTabSkipsRegistrationWhenPaneViewMissing(t *testing.T) {
1839+ ctx := context.Background()
1840+ contentCoord := contentcoord.NewCoordinator(ctx, nil, nil, nil, nil, nil, nil, nil)
1841+ tabID := entity.TabID("tab-1")
1842+ pane := entity.NewPane(entity.PaneID("missing-pane"))
1843+ app := &App{
1844+ contentCoord: contentCoord,
1845+ workspaceViews: map[entity.TabID]*component.WorkspaceView{
1846+ tabID: &component.WorkspaceView{},
1847+ },
1848+ }
1849+
1850+ app.attachPopupToTab(ctx, tabID, pane, &fakeRecordingWebView{id: 1})
1851+
1852+ if got := contentCoord.GetWebView(pane.ID); got != nil {
1853+ t.Fatalf("popup webview was registered for missing pane view: %v", got)
1854+ }
1855+}
1856+
1838 type fakeZoomRepo struct{}1857 type fakeZoomRepo struct{}
1839 1858
1840 func (f *fakeZoomRepo) Get(context.Context, string) (*entity.ZoomLevel, error) { return nil, nil }1859 func (f *fakeZoomRepo) Get(context.Context, string) (*entity.ZoomLevel, error) { return nil, nil }
M internal/ui/component/workspace_view.go +7 -6
@@ -256,6 +256,13 @@ func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error {
256256 Str("to_pane", string(paneID)).
257257 Msg("active pane changing")
258258
259+ // Validate the target pane before mutating UI state. Invalid/stale IDs
260+ // should leave the current active pane styling and overlays untouched.
261+ newPV, ok := wv.paneViews[paneID]
262+ if !ok {
263+ return ErrPaneNotFound
264+ }
265+
259266 // Destroy find bar if active pane is changing
260267 if currentActiveID != paneID && wv.findBar != nil {
261268 wv.hideFindBarInternal()
@@ -273,12 +280,6 @@ func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error {
273280 }
274281 }
275282
276- // Activate new pane
277- newPV, ok := wv.paneViews[paneID]
278- if !ok {
279- return ErrPaneNotFound
280- }
281-
282283 newPV.SetActive(true)
283284
284285 // Update domain model - single source of truth
@@ -256,6 +256,13 @@ func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error {
256 Str("to_pane", string(paneID)).256 Str("to_pane", string(paneID)).
257 Msg("active pane changing")257 Msg("active pane changing")
258 258
259+ // Validate the target pane before mutating UI state. Invalid/stale IDs
260+ // should leave the current active pane styling and overlays untouched.
261+ newPV, ok := wv.paneViews[paneID]
262+ if !ok {
263+ return ErrPaneNotFound
264+ }
265+
259 // Destroy find bar if active pane is changing266 // Destroy find bar if active pane is changing
260 if currentActiveID != paneID && wv.findBar != nil {267 if currentActiveID != paneID && wv.findBar != nil {
261 wv.hideFindBarInternal()268 wv.hideFindBarInternal()
@@ -273,12 +280,6 @@ func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error {
273 }280 }
274 }281 }
275 282
276- // Activate new pane
277- newPV, ok := wv.paneViews[paneID]
278- if !ok {
279- return ErrPaneNotFound
280- }
281-
282 newPV.SetActive(true)283 newPV.SetActive(true)
283 284
284 // Update domain model - single source of truth285 // Update domain model - single source of truth
M internal/ui/component/workspace_view_test.go +60 -0
@@ -443,6 +443,66 @@ func TestSetActivePaneID_InvalidID_ReturnsError(t *testing.T) {
443443 assert.ErrorIs(t, err, component.ErrPaneNotFound)
444444 }
445445
446+func TestSetActivePaneID_InvalidID_PreservesCurrentActivePane(t *testing.T) {
447+ // Arrange
448+ mockFactory := mocks.NewMockWidgetFactory(t)
449+ ctx, mockBox := setupWorkspaceViewBase(t, mockFactory)
450+
451+ wv := component.NewWorkspaceView(ctx, mockFactory)
452+
453+ pane1 := entity.NewPane(entity.PaneID("pane-1"))
454+ pane2 := entity.NewPane(entity.PaneID("pane-2"))
455+ node1 := &entity.PaneNode{ID: "node-1", Pane: pane1}
456+ node2 := &entity.PaneNode{ID: "node-2", Pane: pane2}
457+
458+ mockPaned := mocks.NewMockPanedWidget(t)
459+ mockOverlay1, mockBorderBox1 := setupWorkspacePaneViewMocks(t, mockFactory)
460+ mockStackBox1 := setupStackedLeafMocks(t, mockFactory, mockOverlay1)
461+ mockOverlay2, _ := setupWorkspacePaneViewMocks(t, mockFactory)
462+ mockStackBox2 := setupStackedLeafMocks(t, mockFactory, mockOverlay2)
463+
464+ mockFactory.EXPECT().NewPaned(layout.OrientationHorizontal).Return(mockPaned).Once()
465+ mockPaned.EXPECT().SetResizeStartChild(true).Once()
466+ mockPaned.EXPECT().SetResizeEndChild(true).Once()
467+ mockPaned.EXPECT().SetVisible(true).Twice()
468+ mockStackBox1.EXPECT().SetVisible(true).Once()
469+ mockStackBox2.EXPECT().SetVisible(true).Once()
470+ mockPaned.EXPECT().SetStartChild(mockStackBox1).Once()
471+ mockPaned.EXPECT().SetEndChild(mockStackBox2).Once()
472+ mockPaned.EXPECT().ConnectNotifyPosition(mock.Anything).Return(uint(0)).Once()
473+ mockPaned.EXPECT().GetAllocatedWidth().Return(0).Once()
474+ mockPaned.EXPECT().ConnectMap(mock.Anything).Return(uint(0)).Once()
475+ mockPaned.EXPECT().AddTickCallback(mock.Anything).Return(uint(0)).Once()
476+
477+ mockBox.EXPECT().Append(mockPaned).Once()
478+ mockBox.EXPECT().RemoveCssClass("single-pane").Once()
479+ mockBorderBox1.EXPECT().AddCssClass("pane-active").Once()
480+
481+ splitNode := &entity.PaneNode{
482+ ID: "split",
483+ SplitDir: entity.SplitHorizontal,
484+ SplitRatio: 0.5,
485+ Children: []*entity.PaneNode{node1, node2},
486+ }
487+ ws := &entity.Workspace{
488+ ID: "ws-1",
489+ Root: splitNode,
490+ ActivePaneID: pane1.ID,
491+ }
492+
493+ err := wv.SetWorkspace(ctx, ws)
494+ require.NoError(t, err)
495+ require.True(t, wv.GetPaneView(pane1.ID).IsActive())
496+
497+ // Act
498+ err = wv.SetActivePaneID(entity.PaneID("non-existent"))
499+
500+ // Assert
501+ require.ErrorIs(t, err, component.ErrPaneNotFound)
502+ assert.Equal(t, pane1.ID, wv.GetActivePaneID())
503+ assert.True(t, wv.GetPaneView(pane1.ID).IsActive())
504+}
505+
446506 func TestSetWebViewWidget_InvokesAttachedCallback(t *testing.T) {
447507 mockFactory := mocks.NewMockWidgetFactory(t)
448508 ctx, mockBox := setupWorkspaceViewBase(t, mockFactory)
@@ -443,6 +443,66 @@ func TestSetActivePaneID_InvalidID_ReturnsError(t *testing.T) {
443 assert.ErrorIs(t, err, component.ErrPaneNotFound)443 assert.ErrorIs(t, err, component.ErrPaneNotFound)
444 }444 }
445 445
446+func TestSetActivePaneID_InvalidID_PreservesCurrentActivePane(t *testing.T) {
447+ // Arrange
448+ mockFactory := mocks.NewMockWidgetFactory(t)
449+ ctx, mockBox := setupWorkspaceViewBase(t, mockFactory)
450+
451+ wv := component.NewWorkspaceView(ctx, mockFactory)
452+
453+ pane1 := entity.NewPane(entity.PaneID("pane-1"))
454+ pane2 := entity.NewPane(entity.PaneID("pane-2"))
455+ node1 := &entity.PaneNode{ID: "node-1", Pane: pane1}
456+ node2 := &entity.PaneNode{ID: "node-2", Pane: pane2}
457+
458+ mockPaned := mocks.NewMockPanedWidget(t)
459+ mockOverlay1, mockBorderBox1 := setupWorkspacePaneViewMocks(t, mockFactory)
460+ mockStackBox1 := setupStackedLeafMocks(t, mockFactory, mockOverlay1)
461+ mockOverlay2, _ := setupWorkspacePaneViewMocks(t, mockFactory)
462+ mockStackBox2 := setupStackedLeafMocks(t, mockFactory, mockOverlay2)
463+
464+ mockFactory.EXPECT().NewPaned(layout.OrientationHorizontal).Return(mockPaned).Once()
465+ mockPaned.EXPECT().SetResizeStartChild(true).Once()
466+ mockPaned.EXPECT().SetResizeEndChild(true).Once()
467+ mockPaned.EXPECT().SetVisible(true).Twice()
468+ mockStackBox1.EXPECT().SetVisible(true).Once()
469+ mockStackBox2.EXPECT().SetVisible(true).Once()
470+ mockPaned.EXPECT().SetStartChild(mockStackBox1).Once()
471+ mockPaned.EXPECT().SetEndChild(mockStackBox2).Once()
472+ mockPaned.EXPECT().ConnectNotifyPosition(mock.Anything).Return(uint(0)).Once()
473+ mockPaned.EXPECT().GetAllocatedWidth().Return(0).Once()
474+ mockPaned.EXPECT().ConnectMap(mock.Anything).Return(uint(0)).Once()
475+ mockPaned.EXPECT().AddTickCallback(mock.Anything).Return(uint(0)).Once()
476+
477+ mockBox.EXPECT().Append(mockPaned).Once()
478+ mockBox.EXPECT().RemoveCssClass("single-pane").Once()
479+ mockBorderBox1.EXPECT().AddCssClass("pane-active").Once()
480+
481+ splitNode := &entity.PaneNode{
482+ ID: "split",
483+ SplitDir: entity.SplitHorizontal,
484+ SplitRatio: 0.5,
485+ Children: []*entity.PaneNode{node1, node2},
486+ }
487+ ws := &entity.Workspace{
488+ ID: "ws-1",
489+ Root: splitNode,
490+ ActivePaneID: pane1.ID,
491+ }
492+
493+ err := wv.SetWorkspace(ctx, ws)
494+ require.NoError(t, err)
495+ require.True(t, wv.GetPaneView(pane1.ID).IsActive())
496+
497+ // Act
498+ err = wv.SetActivePaneID(entity.PaneID("non-existent"))
499+
500+ // Assert
501+ require.ErrorIs(t, err, component.ErrPaneNotFound)
502+ assert.Equal(t, pane1.ID, wv.GetActivePaneID())
503+ assert.True(t, wv.GetPaneView(pane1.ID).IsActive())
504+}
505+
446 func TestSetWebViewWidget_InvokesAttachedCallback(t *testing.T) {506 func TestSetWebViewWidget_InvokesAttachedCallback(t *testing.T) {
447 mockFactory := mocks.NewMockWidgetFactory(t)507 mockFactory := mocks.NewMockWidgetFactory(t)
448 ctx, mockBox := setupWorkspaceViewBase(t, mockFactory)508 ctx, mockBox := setupWorkspaceViewBase(t, mockFactory)

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 8

Tags 42

↑↓ moveenter openesc close