detached 9e90b30

Stabilize popup restore followups

commit
9e90b30dfbcbb6d5265e155aa6b852754ae937c3
author
brice · · 2026-05-11 18:16 UTC
parent
18f35c0
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +85 -4
Unified
UnifiedSplit
M internal/ui/app.go +1 -1
@@ -3687,7 +3687,7 @@ func (a *App) createWorkspaceViewWithoutAttach(ctx context.Context, tab *entity.
36873687 }
36883688 a.installFloatingOverlayPositioning(tab.ID, wsView.WorkspaceOverlayWidget())
36893689 if a.contentCoord != nil {
3690- syncCtx := ctx
3690+ syncCtx := context.Background()
36913691 if a.deps != nil && a.deps.Ctx != nil {
36923692 syncCtx = a.deps.Ctx
36933693 }
@@ -3687,7 +3687,7 @@ func (a *App) createWorkspaceViewWithoutAttach(ctx context.Context, tab *entity.
3687 }3687 }
3688 a.installFloatingOverlayPositioning(tab.ID, wsView.WorkspaceOverlayWidget())3688 a.installFloatingOverlayPositioning(tab.ID, wsView.WorkspaceOverlayWidget())
3689 if a.contentCoord != nil {3689 if a.contentCoord != nil {
3690- syncCtx := ctx3690+ syncCtx := context.Background()
3691 if a.deps != nil && a.deps.Ctx != nil {3691 if a.deps != nil && a.deps.Ctx != nil {
3692 syncCtx = a.deps.Ctx3692 syncCtx = a.deps.Ctx
3693 }3693 }
M internal/ui/app_browser_launch_test.go +5 -1
@@ -1913,12 +1913,16 @@ func TestApp_AttachPopupToTabReleasesRegistrationWhenWrapFails(t *testing.T) {
19131913 tabID: wsView,
19141914 },
19151915 }
1916+ popupWV := &fakeRecordingWebView{id: 1}
19161917
1917- app.attachPopupToTab(ctx, tabID, pane, &fakeRecordingWebView{id: 1})
1918+ app.attachPopupToTab(ctx, tabID, pane, popupWV)
19181919
19191920 if got := contentCoord.GetWebView(pane.ID); got != nil {
19201921 t.Fatalf("popup webview registration remained after wrap failure: %v", got)
19211922 }
1923+ if popupWV.destroyCalls != 1 {
1924+ t.Fatalf("popup webview destroy calls = %d, want 1", popupWV.destroyCalls)
1925+ }
19221926 }
19231927
19241928 type fakeZoomRepo struct{}
@@ -1913,12 +1913,16 @@ func TestApp_AttachPopupToTabReleasesRegistrationWhenWrapFails(t *testing.T) {
1913 tabID: wsView,1913 tabID: wsView,
1914 },1914 },
1915 }1915 }
1916+ popupWV := &fakeRecordingWebView{id: 1}
1916 1917
1917- app.attachPopupToTab(ctx, tabID, pane, &fakeRecordingWebView{id: 1})1918+ app.attachPopupToTab(ctx, tabID, pane, popupWV)
1918 1919
1919 if got := contentCoord.GetWebView(pane.ID); got != nil {1920 if got := contentCoord.GetWebView(pane.ID); got != nil {
1920 t.Fatalf("popup webview registration remained after wrap failure: %v", got)1921 t.Fatalf("popup webview registration remained after wrap failure: %v", got)
1921 }1922 }
1923+ if popupWV.destroyCalls != 1 {
1924+ t.Fatalf("popup webview destroy calls = %d, want 1", popupWV.destroyCalls)
1925+ }
1922 }1926 }
1923 1927
1924 type fakeZoomRepo struct{}1928 type fakeZoomRepo struct{}
M internal/ui/component/workspace_view.go +25 -2
@@ -260,12 +260,35 @@ func (wv *WorkspaceView) restoreActivePaneInternal() {
260260 wv.workspace.ActivePaneID = ""
261261 }
262262
263- for fallbackID := range wv.paneViews {
263+ if fallbackID, ok := wv.firstPaneIDInTreeInternal(); ok {
264264 _ = wv.setActivePaneIDInternal(fallbackID)
265- return
266265 }
267266 }
268267
268+func (wv *WorkspaceView) firstPaneIDInTreeInternal() (entity.PaneID, bool) {
269+ if wv.workspace == nil {
270+ return "", false
271+ }
272+ return wv.firstPaneIDInNodeInternal(wv.workspace.Root)
273+}
274+
275+func (wv *WorkspaceView) firstPaneIDInNodeInternal(node *entity.PaneNode) (entity.PaneID, bool) {
276+ if node == nil {
277+ return "", false
278+ }
279+ if node.Pane != nil {
280+ paneID := node.Pane.ID
281+ _, ok := wv.paneViews[paneID]
282+ return paneID, ok
283+ }
284+ for _, child := range node.Children {
285+ if paneID, ok := wv.firstPaneIDInNodeInternal(child); ok {
286+ return paneID, true
287+ }
288+ }
289+ return "", false
290+}
291+
269292 // setActivePaneIDInternal updates active pane without locking.
270293 // Updates the domain model as the single source of truth.
271294 func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error {
@@ -260,12 +260,35 @@ func (wv *WorkspaceView) restoreActivePaneInternal() {
260 wv.workspace.ActivePaneID = ""260 wv.workspace.ActivePaneID = ""
261 }261 }
262 262
263- for fallbackID := range wv.paneViews {263+ if fallbackID, ok := wv.firstPaneIDInTreeInternal(); ok {
264 _ = wv.setActivePaneIDInternal(fallbackID)264 _ = wv.setActivePaneIDInternal(fallbackID)
265- return
266 }265 }
267 }266 }
268 267
268+func (wv *WorkspaceView) firstPaneIDInTreeInternal() (entity.PaneID, bool) {
269+ if wv.workspace == nil {
270+ return "", false
271+ }
272+ return wv.firstPaneIDInNodeInternal(wv.workspace.Root)
273+}
274+
275+func (wv *WorkspaceView) firstPaneIDInNodeInternal(node *entity.PaneNode) (entity.PaneID, bool) {
276+ if node == nil {
277+ return "", false
278+ }
279+ if node.Pane != nil {
280+ paneID := node.Pane.ID
281+ _, ok := wv.paneViews[paneID]
282+ return paneID, ok
283+ }
284+ for _, child := range node.Children {
285+ if paneID, ok := wv.firstPaneIDInNodeInternal(child); ok {
286+ return paneID, true
287+ }
288+ }
289+ return "", false
290+}
291+
269 // setActivePaneIDInternal updates active pane without locking.292 // setActivePaneIDInternal updates active pane without locking.
270 // Updates the domain model as the single source of truth.293 // Updates the domain model as the single source of truth.
271 func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error {294 func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error {
M internal/ui/component/workspace_view_test.go +54 -0
@@ -292,6 +292,60 @@ func TestSetWorkspace_StaleActivePaneID_DoesNotFailInitialization(t *testing.T)
292292 assert.True(t, wv.GetPaneView(pane.ID).IsActive())
293293 }
294294
295+func TestSetWorkspace_StaleActivePaneID_UsesFirstPaneInTree(t *testing.T) {
296+ // Arrange
297+ mockFactory := mocks.NewMockWidgetFactory(t)
298+ ctx, mockBox := setupWorkspaceViewBase(t, mockFactory)
299+ wv := component.NewWorkspaceView(ctx, mockFactory)
300+
301+ pane1 := entity.NewPane(entity.PaneID("pane-1"))
302+ pane2 := entity.NewPane(entity.PaneID("pane-2"))
303+ node1 := &entity.PaneNode{ID: "node-1", Pane: pane1}
304+ node2 := &entity.PaneNode{ID: "node-2", Pane: pane2}
305+
306+ mockPaned := mocks.NewMockPanedWidget(t)
307+ mockOverlay1, mockBorderBox1 := setupWorkspacePaneViewMocks(t, mockFactory)
308+ mockStackBox1 := setupStackedLeafMocks(t, mockFactory, mockOverlay1)
309+ mockOverlay2, _ := setupWorkspacePaneViewMocks(t, mockFactory)
310+ mockStackBox2 := setupStackedLeafMocks(t, mockFactory, mockOverlay2)
311+
312+ mockFactory.EXPECT().NewPaned(layout.OrientationHorizontal).Return(mockPaned).Once()
313+ mockPaned.EXPECT().SetResizeStartChild(true).Once()
314+ mockPaned.EXPECT().SetResizeEndChild(true).Once()
315+ mockPaned.EXPECT().SetVisible(true).Twice()
316+ mockStackBox1.EXPECT().SetVisible(true).Once()
317+ mockStackBox2.EXPECT().SetVisible(true).Once()
318+ mockPaned.EXPECT().SetStartChild(mockStackBox1).Once()
319+ mockPaned.EXPECT().SetEndChild(mockStackBox2).Once()
320+ mockPaned.EXPECT().ConnectNotifyPosition(mock.Anything).Return(uint(0)).Once()
321+ mockPaned.EXPECT().GetAllocatedWidth().Return(0).Once()
322+ mockPaned.EXPECT().ConnectMap(mock.Anything).Return(uint(0)).Once()
323+ mockPaned.EXPECT().AddTickCallback(mock.Anything).Return(uint(0)).Once()
324+ mockBox.EXPECT().Append(mockPaned).Once()
325+ mockBox.EXPECT().RemoveCssClass("single-pane").Once()
326+ mockBorderBox1.EXPECT().AddCssClass("pane-active").Once()
327+
328+ ws := &entity.Workspace{
329+ ID: "ws-1",
330+ Root: &entity.PaneNode{
331+ ID: "split",
332+ SplitDir: entity.SplitHorizontal,
333+ SplitRatio: 0.5,
334+ Children: []*entity.PaneNode{node1, node2},
335+ },
336+ ActivePaneID: entity.PaneID("stale-pane"),
337+ }
338+
339+ // Act
340+ err := wv.SetWorkspace(ctx, ws)
341+
342+ // Assert
343+ require.NoError(t, err)
344+ assert.Equal(t, pane1.ID, wv.GetActivePaneID())
345+ assert.True(t, wv.GetPaneView(pane1.ID).IsActive())
346+ assert.False(t, wv.GetPaneView(pane2.ID).IsActive())
347+}
348+
295349 func TestWorkspaceView_HoverFocusLockState(t *testing.T) {
296350 // Intentional zero-value check: hoverFocusLock is an atomic.Bool and should
297351 // be safe with its language-level zero value before constructor wiring.
@@ -292,6 +292,60 @@ func TestSetWorkspace_StaleActivePaneID_DoesNotFailInitialization(t *testing.T)
292 assert.True(t, wv.GetPaneView(pane.ID).IsActive())292 assert.True(t, wv.GetPaneView(pane.ID).IsActive())
293 }293 }
294 294
295+func TestSetWorkspace_StaleActivePaneID_UsesFirstPaneInTree(t *testing.T) {
296+ // Arrange
297+ mockFactory := mocks.NewMockWidgetFactory(t)
298+ ctx, mockBox := setupWorkspaceViewBase(t, mockFactory)
299+ wv := component.NewWorkspaceView(ctx, mockFactory)
300+
301+ pane1 := entity.NewPane(entity.PaneID("pane-1"))
302+ pane2 := entity.NewPane(entity.PaneID("pane-2"))
303+ node1 := &entity.PaneNode{ID: "node-1", Pane: pane1}
304+ node2 := &entity.PaneNode{ID: "node-2", Pane: pane2}
305+
306+ mockPaned := mocks.NewMockPanedWidget(t)
307+ mockOverlay1, mockBorderBox1 := setupWorkspacePaneViewMocks(t, mockFactory)
308+ mockStackBox1 := setupStackedLeafMocks(t, mockFactory, mockOverlay1)
309+ mockOverlay2, _ := setupWorkspacePaneViewMocks(t, mockFactory)
310+ mockStackBox2 := setupStackedLeafMocks(t, mockFactory, mockOverlay2)
311+
312+ mockFactory.EXPECT().NewPaned(layout.OrientationHorizontal).Return(mockPaned).Once()
313+ mockPaned.EXPECT().SetResizeStartChild(true).Once()
314+ mockPaned.EXPECT().SetResizeEndChild(true).Once()
315+ mockPaned.EXPECT().SetVisible(true).Twice()
316+ mockStackBox1.EXPECT().SetVisible(true).Once()
317+ mockStackBox2.EXPECT().SetVisible(true).Once()
318+ mockPaned.EXPECT().SetStartChild(mockStackBox1).Once()
319+ mockPaned.EXPECT().SetEndChild(mockStackBox2).Once()
320+ mockPaned.EXPECT().ConnectNotifyPosition(mock.Anything).Return(uint(0)).Once()
321+ mockPaned.EXPECT().GetAllocatedWidth().Return(0).Once()
322+ mockPaned.EXPECT().ConnectMap(mock.Anything).Return(uint(0)).Once()
323+ mockPaned.EXPECT().AddTickCallback(mock.Anything).Return(uint(0)).Once()
324+ mockBox.EXPECT().Append(mockPaned).Once()
325+ mockBox.EXPECT().RemoveCssClass("single-pane").Once()
326+ mockBorderBox1.EXPECT().AddCssClass("pane-active").Once()
327+
328+ ws := &entity.Workspace{
329+ ID: "ws-1",
330+ Root: &entity.PaneNode{
331+ ID: "split",
332+ SplitDir: entity.SplitHorizontal,
333+ SplitRatio: 0.5,
334+ Children: []*entity.PaneNode{node1, node2},
335+ },
336+ ActivePaneID: entity.PaneID("stale-pane"),
337+ }
338+
339+ // Act
340+ err := wv.SetWorkspace(ctx, ws)
341+
342+ // Assert
343+ require.NoError(t, err)
344+ assert.Equal(t, pane1.ID, wv.GetActivePaneID())
345+ assert.True(t, wv.GetPaneView(pane1.ID).IsActive())
346+ assert.False(t, wv.GetPaneView(pane2.ID).IsActive())
347+}
348+
295 func TestWorkspaceView_HoverFocusLockState(t *testing.T) {349 func TestWorkspaceView_HoverFocusLockState(t *testing.T) {
296 // Intentional zero-value check: hoverFocusLock is an atomic.Bool and should350 // Intentional zero-value check: hoverFocusLock is an atomic.Bool and should
297 // be safe with its language-level zero value before constructor wiring.351 // be safe with its language-level zero value before constructor wiring.

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