fix: address per-window tab review edge cases
- prefer window-scoped previous tab for Alt+Tab - use tab creation target for new-tab ownership - count live tabs for default window-scoped names - prevent unowned restored tabs from leaking across windows - reset restored window active state - add defensive close-path logging - split tab coordinator wiring to satisfy lint
- commit
- 5509537e709c9f1c0146410a47ac144cfc52bb2a
- author
- brice · · 2026-04-28 17:55 UTC
- parent
- 4ea1430
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +121 -36
M internal/ui/app.go +91 -31
| @@ -511,6 +511,22 @@ func (a *App) browserWindowForTab(tabID entity.TabID) *browserWindow { | |||
| 511 | 511 | return a.browserWindowForMainWindow(a.mainWindow) | |
| 512 | 512 | } | |
| 513 | 513 | ||
| 514 | + | func (a *App) tabCountForBrowserWindow(bw *browserWindow) int { | |
| 515 | + | if a.tabs == nil || bw == nil { | |
| 516 | + | return 0 | |
| 517 | + | } | |
| 518 | + | count := 0 | |
| 519 | + | for _, tab := range a.tabs.Tabs { | |
| 520 | + | if tab == nil { | |
| 521 | + | continue | |
| 522 | + | } | |
| 523 | + | if owner := a.windowForTab[tab.ID]; owner == bw { | |
| 524 | + | count++ | |
| 525 | + | } | |
| 526 | + | } | |
| 527 | + | return count | |
| 528 | + | } | |
| 529 | + | ||
| 514 | 530 | func (a *App) browserWindowForPane(paneID entity.PaneID) *browserWindow { | |
| 515 | 531 | if paneID == "" || a.tabs == nil { | |
| 516 | 532 | return nil | |
| @@ -1867,6 +1883,8 @@ func (a *App) restoreSession(ctx context.Context, sessionID entity.SessionID) er | |||
| 1867 | 1883 | // Replace tabs in-place to preserve references held by TabCoordinator | |
| 1868 | 1884 | a.tabs.ReplaceFrom(restoredTabs) | |
| 1869 | 1885 | ||
| 1886 | + | a.assignRestoredTabsToCurrentWindow() | |
| 1887 | + | ||
| 1870 | 1888 | // Create UI for each restored tab and add to tab bar | |
| 1871 | 1889 | // Don't attach to content area yet - only the active tab should be attached | |
| 1872 | 1890 | tabBar := a.mainWindow.TabBar() | |
| @@ -1898,6 +1916,34 @@ func (a *App) restoreSession(ctx context.Context, sessionID entity.SessionID) er | |||
| 1898 | 1916 | return nil | |
| 1899 | 1917 | } | |
| 1900 | 1918 | ||
| 1919 | + | func (a *App) assignRestoredTabsToCurrentWindow() { | |
| 1920 | + | if a == nil || a.tabs == nil { | |
| 1921 | + | return | |
| 1922 | + | } | |
| 1923 | + | ||
| 1924 | + | restoreWindow := a.browserWindowForMainWindow(a.mainWindow) | |
| 1925 | + | if restoreWindow == nil { | |
| 1926 | + | restoreWindow = a.lastFocusedBrowserWindow() | |
| 1927 | + | } | |
| 1928 | + | if restoreWindow == nil { | |
| 1929 | + | return | |
| 1930 | + | } | |
| 1931 | + | ||
| 1932 | + | for _, tab := range a.tabs.Tabs { | |
| 1933 | + | if tab != nil { | |
| 1934 | + | a.setBrowserWindowForTab(tab.ID, restoreWindow) | |
| 1935 | + | } | |
| 1936 | + | } | |
| 1937 | + | ||
| 1938 | + | restoreWindow.activeTabID = a.tabs.ActiveTabID | |
| 1939 | + | previousID := a.tabs.PreviousActiveTabID | |
| 1940 | + | if previousID != "" && previousID != a.tabs.ActiveTabID && a.tabs.Find(previousID) != nil { | |
| 1941 | + | restoreWindow.prevActiveTabID = previousID | |
| 1942 | + | return | |
| 1943 | + | } | |
| 1944 | + | restoreWindow.prevActiveTabID = "" | |
| 1945 | + | } | |
| 1946 | + | ||
| 1901 | 1947 | func (a *App) finalizeActivation(ctx context.Context) { | |
| 1902 | 1948 | log := logging.FromContext(ctx) | |
| 1903 | 1949 | ||
| @@ -2063,26 +2109,8 @@ func (a *App) initContentCoordinator( | |||
| 2063 | 2109 | }) | |
| 2064 | 2110 | } | |
| 2065 | 2111 | ||
| 2066 | - | // initCoordinators initializes all coordinators and wires their callbacks. | |
| 2067 | - | func (a *App) initCoordinators(ctx context.Context) { | |
| 2068 | - | log := logging.FromContext(ctx) | |
| 2069 | - | log.Debug().Msg("initializing coordinators") | |
| 2070 | - | ||
| 2071 | - | // Helper to get active workspace and view | |
| 2072 | - | getActiveWS := func() (*entity.Workspace, *component.WorkspaceView) { | |
| 2073 | - | return a.activeWorkspace(), a.activeWorkspaceView() | |
| 2074 | - | } | |
| 2075 | - | ||
| 2076 | - | // Create FaviconAdapter with service and engine FaviconDatabase. | |
| 2077 | - | // Skip if FaviconService is nil (e.g. in tests or when favicon support is disabled). | |
| 2078 | - | if a.deps.FaviconService != nil { | |
| 2079 | - | a.faviconAdapter = adapter.NewFaviconAdapter(a.deps.FaviconService, a.engine.FaviconDatabase(), a.deps.FaviconAdapterConfig) | |
| 2080 | - | } | |
| 2081 | - | ||
| 2082 | - | // 1. Content Coordinator (no dependencies on other coordinators) | |
| 2083 | - | a.initContentCoordinator(ctx, getActiveWS) | |
| 2084 | - | ||
| 2085 | - | // 2. Tab Coordinator | |
| 2112 | + | // initTabCoordinator creates the TabCoordinator and wires all its callbacks. | |
| 2113 | + | func (a *App) initTabCoordinator(ctx context.Context) { | |
| 2086 | 2114 | a.tabCoord = coordinator.NewTabCoordinator(ctx, coordinator.TabCoordinatorConfig{ | |
| 2087 | 2115 | TabsUC: a.tabsUC, | |
| 2088 | 2116 | Tabs: a.tabs, | |
| @@ -2092,25 +2120,27 @@ func (a *App) initCoordinators(ctx context.Context) { | |||
| 2092 | 2120 | a.tabCoord.SetOnTabCreated(func(ctx context.Context, tab *entity.Tab) { | |
| 2093 | 2121 | // Assign ownership BEFORE creating workspace view so windowForTab is set | |
| 2094 | 2122 | // for scope filtering and browserWindowForTab resolution. | |
| 2095 | - | bw := a.lastFocusedBrowserWindow() | |
| 2123 | + | bw := a.browserWindowForTab(tab.ID) | |
| 2096 | 2124 | if bw != nil { | |
| 2097 | 2125 | a.setBrowserWindowForTab(tab.ID, bw) | |
| 2098 | 2126 | } | |
| 2099 | 2127 | // Set a window-scoped default title so "Tab N" doesn't use global position. | |
| 2100 | - | // Count tabs owned by this window. The new tab was just assigned above. | |
| 2101 | - | windowTabCount := 0 | |
| 2102 | - | for _, owner := range a.windowForTab { | |
| 2103 | - | if owner == bw { | |
| 2104 | - | windowTabCount++ | |
| 2105 | - | } | |
| 2128 | + | // Count only live tabs owned by this window via the helper. | |
| 2129 | + | count := a.tabCountForBrowserWindow(bw) | |
| 2130 | + | if count <= 0 { | |
| 2131 | + | count = 1 | |
| 2106 | 2132 | } | |
| 2107 | - | // windowTabCount includes the tab just added, so it equals the 1-based index. | |
| 2108 | - | tab.Name = defaultTabName(windowTabCount) | |
| 2133 | + | tab.Name = defaultTabName(count) | |
| 2109 | 2134 | a.createWorkspaceView(ctx, tab) | |
| 2110 | 2135 | }) | |
| 2111 | 2136 | a.tabCoord.SetOnTabSwitched(func(ctx context.Context, tab *entity.Tab) { | |
| 2112 | 2137 | if bw := a.browserWindowForTab(tab.ID); bw != nil { | |
| 2113 | - | bw.prevActiveTabID = bw.activeTabID | |
| 2138 | + | oldActiveID := bw.activeTabID | |
| 2139 | + | if oldActiveID != "" && oldActiveID != tab.ID && a.tabs.Find(oldActiveID) != nil { | |
| 2140 | + | bw.prevActiveTabID = oldActiveID | |
| 2141 | + | } else if bw.prevActiveTabID == tab.ID || a.tabs.Find(bw.prevActiveTabID) == nil { | |
| 2142 | + | bw.prevActiveTabID = "" | |
| 2143 | + | } | |
| 2114 | 2144 | bw.activeTabID = tab.ID | |
| 2115 | 2145 | a.activateBrowserWindow(bw) | |
| 2116 | 2146 | } | |
| @@ -2129,10 +2159,17 @@ func (a *App) initCoordinators(ctx context.Context) { | |||
| 2129 | 2159 | } | |
| 2130 | 2160 | owner := a.windowForTab[tab.ID] | |
| 2131 | 2161 | if owner == nil { | |
| 2132 | - | return true // backward-compatible fallback for restored/unowned tabs | |
| 2162 | + | return false // unowned tabs are out of scope when a real window exists | |
| 2133 | 2163 | } | |
| 2134 | 2164 | return owner == bw | |
| 2135 | 2165 | }) | |
| 2166 | + | // Wire per-window previous active tab ID provider for scoped Alt+Tab switching | |
| 2167 | + | a.tabCoord.SetPreviousActiveTabIDProvider(func(mainWindow *window.MainWindow) entity.TabID { | |
| 2168 | + | if bw := a.browserWindowForMainWindow(mainWindow); bw != nil { | |
| 2169 | + | return bw.prevActiveTabID | |
| 2170 | + | } | |
| 2171 | + | return "" | |
| 2172 | + | }) | |
| 2136 | 2173 | a.tabCoord.SetOnCurrentWindowEmpty(func(ctx context.Context, mainWindow *window.MainWindow) { | |
| 2137 | 2174 | bw := a.browserWindowForMainWindow(mainWindow) | |
| 2138 | 2175 | if bw != nil { | |
| @@ -2150,6 +2187,29 @@ func (a *App) initCoordinators(ctx context.Context) { | |||
| 2150 | 2187 | for _, bw := range a.browserWindows { | |
| 2151 | 2188 | a.wireBrowserWindowTabBar(ctx, bw) | |
| 2152 | 2189 | } | |
| 2190 | + | } | |
| 2191 | + | ||
| 2192 | + | // initCoordinators initializes all coordinators and wires their callbacks. | |
| 2193 | + | func (a *App) initCoordinators(ctx context.Context) { | |
| 2194 | + | log := logging.FromContext(ctx) | |
| 2195 | + | log.Debug().Msg("initializing coordinators") | |
| 2196 | + | ||
| 2197 | + | // Helper to get active workspace and view | |
| 2198 | + | getActiveWS := func() (*entity.Workspace, *component.WorkspaceView) { | |
| 2199 | + | return a.activeWorkspace(), a.activeWorkspaceView() | |
| 2200 | + | } | |
| 2201 | + | ||
| 2202 | + | // Create FaviconAdapter with service and engine FaviconDatabase. | |
| 2203 | + | // Skip if FaviconService is nil (e.g. in tests or when favicon support is disabled). | |
| 2204 | + | if a.deps.FaviconService != nil { | |
| 2205 | + | a.faviconAdapter = adapter.NewFaviconAdapter(a.deps.FaviconService, a.engine.FaviconDatabase(), a.deps.FaviconAdapterConfig) | |
| 2206 | + | } | |
| 2207 | + | ||
| 2208 | + | // 1. Content Coordinator (no dependencies on other coordinators) | |
| 2209 | + | a.initContentCoordinator(ctx, getActiveWS) | |
| 2210 | + | ||
| 2211 | + | // 2. Tab Coordinator | |
| 2212 | + | a.initTabCoordinator(ctx) | |
| 2153 | 2213 | ||
| 2154 | 2214 | // Set fullscreen callback to hide/show tab bar (after tabCoord is initialized) | |
| 2155 | 2215 | a.contentCoord.SetOnFullscreenChanged(func(paneID entity.PaneID, entering bool) { | |
| @@ -511,6 +511,22 @@ func (a *App) browserWindowForTab(tabID entity.TabID) *browserWindow { | |||||
| 511 | return a.browserWindowForMainWindow(a.mainWindow) | 511 | return a.browserWindowForMainWindow(a.mainWindow) | ||
| 512 | } | 512 | } | ||
| 513 | 513 | ||||
| 514 | + | func (a *App) tabCountForBrowserWindow(bw *browserWindow) int { | |||
| 515 | + | if a.tabs == nil || bw == nil { | |||
| 516 | + | return 0 | |||
| 517 | + | } | |||
| 518 | + | count := 0 | |||
| 519 | + | for _, tab := range a.tabs.Tabs { | |||
| 520 | + | if tab == nil { | |||
| 521 | + | continue | |||
| 522 | + | } | |||
| 523 | + | if owner := a.windowForTab[tab.ID]; owner == bw { | |||
| 524 | + | count++ | |||
| 525 | + | } | |||
| 526 | + | } | |||
| 527 | + | return count | |||
| 528 | + | } | |||
| 529 | + | ||||
| 514 | func (a *App) browserWindowForPane(paneID entity.PaneID) *browserWindow { | 530 | func (a *App) browserWindowForPane(paneID entity.PaneID) *browserWindow { | ||
| 515 | if paneID == "" || a.tabs == nil { | 531 | if paneID == "" || a.tabs == nil { | ||
| 516 | return nil | 532 | return nil | ||
| @@ -1867,6 +1883,8 @@ func (a *App) restoreSession(ctx context.Context, sessionID entity.SessionID) er | |||||
| 1867 | // Replace tabs in-place to preserve references held by TabCoordinator | 1883 | // Replace tabs in-place to preserve references held by TabCoordinator | ||
| 1868 | a.tabs.ReplaceFrom(restoredTabs) | 1884 | a.tabs.ReplaceFrom(restoredTabs) | ||
| 1869 | 1885 | ||||
| 1886 | + | a.assignRestoredTabsToCurrentWindow() | |||
| 1887 | + | ||||
| 1870 | // Create UI for each restored tab and add to tab bar | 1888 | // Create UI for each restored tab and add to tab bar | ||
| 1871 | // Don't attach to content area yet - only the active tab should be attached | 1889 | // Don't attach to content area yet - only the active tab should be attached | ||
| 1872 | tabBar := a.mainWindow.TabBar() | 1890 | tabBar := a.mainWindow.TabBar() | ||
| @@ -1898,6 +1916,34 @@ func (a *App) restoreSession(ctx context.Context, sessionID entity.SessionID) er | |||||
| 1898 | return nil | 1916 | return nil | ||
| 1899 | } | 1917 | } | ||
| 1900 | 1918 | ||||
| 1919 | + | func (a *App) assignRestoredTabsToCurrentWindow() { | |||
| 1920 | + | if a == nil || a.tabs == nil { | |||
| 1921 | + | return | |||
| 1922 | + | } | |||
| 1923 | + | ||||
| 1924 | + | restoreWindow := a.browserWindowForMainWindow(a.mainWindow) | |||
| 1925 | + | if restoreWindow == nil { | |||
| 1926 | + | restoreWindow = a.lastFocusedBrowserWindow() | |||
| 1927 | + | } | |||
| 1928 | + | if restoreWindow == nil { | |||
| 1929 | + | return | |||
| 1930 | + | } | |||
| 1931 | + | ||||
| 1932 | + | for _, tab := range a.tabs.Tabs { | |||
| 1933 | + | if tab != nil { | |||
| 1934 | + | a.setBrowserWindowForTab(tab.ID, restoreWindow) | |||
| 1935 | + | } | |||
| 1936 | + | } | |||
| 1937 | + | ||||
| 1938 | + | restoreWindow.activeTabID = a.tabs.ActiveTabID | |||
| 1939 | + | previousID := a.tabs.PreviousActiveTabID | |||
| 1940 | + | if previousID != "" && previousID != a.tabs.ActiveTabID && a.tabs.Find(previousID) != nil { | |||
| 1941 | + | restoreWindow.prevActiveTabID = previousID | |||
| 1942 | + | return | |||
| 1943 | + | } | |||
| 1944 | + | restoreWindow.prevActiveTabID = "" | |||
| 1945 | + | } | |||
| 1946 | + | ||||
| 1901 | func (a *App) finalizeActivation(ctx context.Context) { | 1947 | func (a *App) finalizeActivation(ctx context.Context) { | ||
| 1902 | log := logging.FromContext(ctx) | 1948 | log := logging.FromContext(ctx) | ||
| 1903 | 1949 | ||||
| @@ -2063,26 +2109,8 @@ func (a *App) initContentCoordinator( | |||||
| 2063 | }) | 2109 | }) | ||
| 2064 | } | 2110 | } | ||
| 2065 | 2111 | ||||
| 2066 | - | // initCoordinators initializes all coordinators and wires their callbacks. | 2112 | + | // initTabCoordinator creates the TabCoordinator and wires all its callbacks. |
| 2067 | - | func (a *App) initCoordinators(ctx context.Context) { | 2113 | + | func (a *App) initTabCoordinator(ctx context.Context) { |
| 2068 | - | log := logging.FromContext(ctx) | |||
| 2069 | - | log.Debug().Msg("initializing coordinators") | |||
| 2070 | - | ||||
| 2071 | - | // Helper to get active workspace and view | |||
| 2072 | - | getActiveWS := func() (*entity.Workspace, *component.WorkspaceView) { | |||
| 2073 | - | return a.activeWorkspace(), a.activeWorkspaceView() | |||
| 2074 | - | } | |||
| 2075 | - | ||||
| 2076 | - | // Create FaviconAdapter with service and engine FaviconDatabase. | |||
| 2077 | - | // Skip if FaviconService is nil (e.g. in tests or when favicon support is disabled). | |||
| 2078 | - | if a.deps.FaviconService != nil { | |||
| 2079 | - | a.faviconAdapter = adapter.NewFaviconAdapter(a.deps.FaviconService, a.engine.FaviconDatabase(), a.deps.FaviconAdapterConfig) | |||
| 2080 | - | } | |||
| 2081 | - | ||||
| 2082 | - | // 1. Content Coordinator (no dependencies on other coordinators) | |||
| 2083 | - | a.initContentCoordinator(ctx, getActiveWS) | |||
| 2084 | - | ||||
| 2085 | - | // 2. Tab Coordinator | |||
| 2086 | a.tabCoord = coordinator.NewTabCoordinator(ctx, coordinator.TabCoordinatorConfig{ | 2114 | a.tabCoord = coordinator.NewTabCoordinator(ctx, coordinator.TabCoordinatorConfig{ | ||
| 2087 | TabsUC: a.tabsUC, | 2115 | TabsUC: a.tabsUC, | ||
| 2088 | Tabs: a.tabs, | 2116 | Tabs: a.tabs, | ||
| @@ -2092,25 +2120,27 @@ func (a *App) initCoordinators(ctx context.Context) { | |||||
| 2092 | a.tabCoord.SetOnTabCreated(func(ctx context.Context, tab *entity.Tab) { | 2120 | a.tabCoord.SetOnTabCreated(func(ctx context.Context, tab *entity.Tab) { | ||
| 2093 | // Assign ownership BEFORE creating workspace view so windowForTab is set | 2121 | // Assign ownership BEFORE creating workspace view so windowForTab is set | ||
| 2094 | // for scope filtering and browserWindowForTab resolution. | 2122 | // for scope filtering and browserWindowForTab resolution. | ||
| 2095 | - | bw := a.lastFocusedBrowserWindow() | 2123 | + | bw := a.browserWindowForTab(tab.ID) |
| 2096 | if bw != nil { | 2124 | if bw != nil { | ||
| 2097 | a.setBrowserWindowForTab(tab.ID, bw) | 2125 | a.setBrowserWindowForTab(tab.ID, bw) | ||
| 2098 | } | 2126 | } | ||
| 2099 | // Set a window-scoped default title so "Tab N" doesn't use global position. | 2127 | // Set a window-scoped default title so "Tab N" doesn't use global position. | ||
| 2100 | - | // Count tabs owned by this window. The new tab was just assigned above. | 2128 | + | // Count only live tabs owned by this window via the helper. |
| 2101 | - | windowTabCount := 0 | 2129 | + | count := a.tabCountForBrowserWindow(bw) |
| 2102 | - | for _, owner := range a.windowForTab { | 2130 | + | if count <= 0 { |
| 2103 | - | if owner == bw { | 2131 | + | count = 1 |
| 2104 | - | windowTabCount++ | |||
| 2105 | - | } | |||
| 2106 | } | 2132 | } | ||
| 2107 | - | // windowTabCount includes the tab just added, so it equals the 1-based index. | 2133 | + | tab.Name = defaultTabName(count) |
| 2108 | - | tab.Name = defaultTabName(windowTabCount) | |||
| 2109 | a.createWorkspaceView(ctx, tab) | 2134 | a.createWorkspaceView(ctx, tab) | ||
| 2110 | }) | 2135 | }) | ||
| 2111 | a.tabCoord.SetOnTabSwitched(func(ctx context.Context, tab *entity.Tab) { | 2136 | a.tabCoord.SetOnTabSwitched(func(ctx context.Context, tab *entity.Tab) { | ||
| 2112 | if bw := a.browserWindowForTab(tab.ID); bw != nil { | 2137 | if bw := a.browserWindowForTab(tab.ID); bw != nil { | ||
| 2113 | - | bw.prevActiveTabID = bw.activeTabID | 2138 | + | oldActiveID := bw.activeTabID |
| 2139 | + | if oldActiveID != "" && oldActiveID != tab.ID && a.tabs.Find(oldActiveID) != nil { | |||
| 2140 | + | bw.prevActiveTabID = oldActiveID | |||
| 2141 | + | } else if bw.prevActiveTabID == tab.ID || a.tabs.Find(bw.prevActiveTabID) == nil { | |||
| 2142 | + | bw.prevActiveTabID = "" | |||
| 2143 | + | } | |||
| 2114 | bw.activeTabID = tab.ID | 2144 | bw.activeTabID = tab.ID | ||
| 2115 | a.activateBrowserWindow(bw) | 2145 | a.activateBrowserWindow(bw) | ||
| 2116 | } | 2146 | } | ||
| @@ -2129,10 +2159,17 @@ func (a *App) initCoordinators(ctx context.Context) { | |||||
| 2129 | } | 2159 | } | ||
| 2130 | owner := a.windowForTab[tab.ID] | 2160 | owner := a.windowForTab[tab.ID] | ||
| 2131 | if owner == nil { | 2161 | if owner == nil { | ||
| 2132 | - | return true // backward-compatible fallback for restored/unowned tabs | 2162 | + | return false // unowned tabs are out of scope when a real window exists |
| 2133 | } | 2163 | } | ||
| 2134 | return owner == bw | 2164 | return owner == bw | ||
| 2135 | }) | 2165 | }) | ||
| 2166 | + | // Wire per-window previous active tab ID provider for scoped Alt+Tab switching | |||
| 2167 | + | a.tabCoord.SetPreviousActiveTabIDProvider(func(mainWindow *window.MainWindow) entity.TabID { | |||
| 2168 | + | if bw := a.browserWindowForMainWindow(mainWindow); bw != nil { | |||
| 2169 | + | return bw.prevActiveTabID | |||
| 2170 | + | } | |||
| 2171 | + | return "" | |||
| 2172 | + | }) | |||
| 2136 | a.tabCoord.SetOnCurrentWindowEmpty(func(ctx context.Context, mainWindow *window.MainWindow) { | 2173 | a.tabCoord.SetOnCurrentWindowEmpty(func(ctx context.Context, mainWindow *window.MainWindow) { | ||
| 2137 | bw := a.browserWindowForMainWindow(mainWindow) | 2174 | bw := a.browserWindowForMainWindow(mainWindow) | ||
| 2138 | if bw != nil { | 2175 | if bw != nil { | ||
| @@ -2150,6 +2187,29 @@ func (a *App) initCoordinators(ctx context.Context) { | |||||
| 2150 | for _, bw := range a.browserWindows { | 2187 | for _, bw := range a.browserWindows { | ||
| 2151 | a.wireBrowserWindowTabBar(ctx, bw) | 2188 | a.wireBrowserWindowTabBar(ctx, bw) | ||
| 2152 | } | 2189 | } | ||
| 2190 | + | } | |||
| 2191 | + | ||||
| 2192 | + | // initCoordinators initializes all coordinators and wires their callbacks. | |||
| 2193 | + | func (a *App) initCoordinators(ctx context.Context) { | |||
| 2194 | + | log := logging.FromContext(ctx) | |||
| 2195 | + | log.Debug().Msg("initializing coordinators") | |||
| 2196 | + | ||||
| 2197 | + | // Helper to get active workspace and view | |||
| 2198 | + | getActiveWS := func() (*entity.Workspace, *component.WorkspaceView) { | |||
| 2199 | + | return a.activeWorkspace(), a.activeWorkspaceView() | |||
| 2200 | + | } | |||
| 2201 | + | ||||
| 2202 | + | // Create FaviconAdapter with service and engine FaviconDatabase. | |||
| 2203 | + | // Skip if FaviconService is nil (e.g. in tests or when favicon support is disabled). | |||
| 2204 | + | if a.deps.FaviconService != nil { | |||
| 2205 | + | a.faviconAdapter = adapter.NewFaviconAdapter(a.deps.FaviconService, a.engine.FaviconDatabase(), a.deps.FaviconAdapterConfig) | |||
| 2206 | + | } | |||
| 2207 | + | ||||
| 2208 | + | // 1. Content Coordinator (no dependencies on other coordinators) | |||
| 2209 | + | a.initContentCoordinator(ctx, getActiveWS) | |||
| 2210 | + | ||||
| 2211 | + | // 2. Tab Coordinator | |||
| 2212 | + | a.initTabCoordinator(ctx) | |||
| 2153 | 2213 | ||||
| 2154 | // Set fullscreen callback to hide/show tab bar (after tabCoord is initialized) | 2214 | // Set fullscreen callback to hide/show tab bar (after tabCoord is initialized) | ||
| 2155 | a.contentCoord.SetOnFullscreenChanged(func(paneID entity.PaneID, entering bool) { | 2215 | a.contentCoord.SetOnFullscreenChanged(func(paneID entity.PaneID, entering bool) { | ||
M internal/ui/coordinator/tab.go +30 -5
| @@ -33,6 +33,10 @@ type TabCoordinator struct { | |||
| 33 | 33 | onCurrentWindowEmpty func(ctx context.Context, mainWindow *window.MainWindow) | |
| 34 | 34 | onAttachPopupToTab func(ctx context.Context, tabID entity.TabID, pane *entity.Pane, wv port.WebView) // For popup tabs | |
| 35 | 35 | onStateChanged func() // For session snapshots | |
| 36 | + | ||
| 37 | + | // previousActiveTabID provides the per-window previous active tab ID for Alt+Tab style switching. | |
| 38 | + | // When set and tabScope is active, SwitchToLastActive prefers this over the global PreviousActiveTabID. | |
| 39 | + | previousActiveTabID func(mainWindow *window.MainWindow) entity.TabID | |
| 36 | 40 | } | |
| 37 | 41 | ||
| 38 | 42 | // TabCoordinatorConfig holds configuration for TabCoordinator. | |
| @@ -77,6 +81,13 @@ func (c *TabCoordinator) SetOnStateChanged(fn func()) { | |||
| 77 | 81 | c.onStateChanged = fn | |
| 78 | 82 | } | |
| 79 | 83 | ||
| 84 | + | // SetPreviousActiveTabIDProvider sets the callback for retrieving the per-window | |
| 85 | + | // previous active tab ID. When set and tabScope is active, SwitchToLastActive | |
| 86 | + | // prefers this provider over the global PreviousActiveTabID. | |
| 87 | + | func (c *TabCoordinator) SetPreviousActiveTabIDProvider(fn func(mainWindow *window.MainWindow) entity.TabID) { | |
| 88 | + | c.previousActiveTabID = fn | |
| 89 | + | } | |
| 90 | + | ||
| 80 | 91 | // SetTabScope sets the function used to filter tabs to the current window. | |
| 81 | 92 | // When set, tab operations (switch by index, next/prev, close) only consider | |
| 82 | 93 | // tabs for which the scope function returns true. | |
| @@ -202,6 +213,9 @@ func (c *TabCoordinator) Close(ctx context.Context) error { | |||
| 202 | 213 | // Compute the next scoped tab to activate BEFORE closing | |
| 203 | 214 | scoped := c.scopedTabs() | |
| 204 | 215 | activeIdx := indexOfTab(scoped, activeID) | |
| 216 | + | if activeIdx < 0 && len(scoped) > 0 { | |
| 217 | + | log.Debug().Str("active_tab_id", string(activeID)).Int("scoped_count", len(scoped)).Msg("active tab not in scoped list") | |
| 218 | + | } | |
| 205 | 219 | ||
| 206 | 220 | var nextScopedID entity.TabID | |
| 207 | 221 | if activeIdx >= 0 && len(scoped) > 1 { | |
| @@ -384,24 +398,35 @@ func (c *TabCoordinator) EnsureTabByIndex(ctx context.Context, index int, initia | |||
| 384 | 398 | } | |
| 385 | 399 | ||
| 386 | 400 | // SwitchToLastActive switches to the previously active tab (Alt+Tab style). | |
| 387 | - | // Uses per-window prevActiveTabID when scope is active; falls back to global otherwise. | |
| 401 | + | // When window-scoped (tabScope is set and previousActiveTabID provider is available), | |
| 402 | + | // it uses the per-window previous active tab ID. Falls back to the global | |
| 403 | + | // PreviousActiveTabID otherwise. | |
| 388 | 404 | func (c *TabCoordinator) SwitchToLastActive(ctx context.Context) error { | |
| 389 | 405 | log := logging.FromContext(ctx) | |
| 390 | 406 | ||
| 391 | - | // Use the global PreviousActiveTabID directly when there's no window scope | |
| 392 | - | prevID := c.tabs.PreviousActiveTabID | |
| 407 | + | var prevID entity.TabID | |
| 408 | + | if c.tabScope != nil && c.previousActiveTabID != nil { | |
| 409 | + | // Use per-window previous active tab ID when scoped | |
| 410 | + | prevID = c.previousActiveTabID(c.mainWindow) | |
| 411 | + | } else { | |
| 412 | + | // Fall back to global PreviousActiveTabID when not scoped or no provider | |
| 413 | + | prevID = c.tabs.PreviousActiveTabID | |
| 414 | + | } | |
| 415 | + | ||
| 393 | 416 | if prevID == "" || prevID == c.tabs.ActiveTabID { | |
| 394 | 417 | return nil | |
| 395 | 418 | } | |
| 396 | 419 | ||
| 397 | 420 | tab := c.tabs.Find(prevID) | |
| 398 | 421 | if tab == nil { | |
| 399 | - | c.tabs.PreviousActiveTabID = "" | |
| 422 | + | // Only clear the global previous active tab ID if that's what we used | |
| 423 | + | if c.tabScope == nil || c.previousActiveTabID == nil { | |
| 424 | + | c.tabs.PreviousActiveTabID = "" | |
| 425 | + | } | |
| 400 | 426 | return nil | |
| 401 | 427 | } | |
| 402 | 428 | ||
| 403 | 429 | // With a scope function, only allow switching to the previous tab if it's in scope. | |
| 404 | - | // Per-window prevActiveTabID is tracked by the App layer and always in-scope. | |
| 405 | 430 | if c.tabScope != nil && !c.tabScope(tab, c.mainWindow) { | |
| 406 | 431 | log.Debug().Str("tab_id", string(prevID)).Msg("previous active tab outside current window scope") | |
| 407 | 432 | return nil | |
| @@ -33,6 +33,10 @@ type TabCoordinator struct { | |||||
| 33 | onCurrentWindowEmpty func(ctx context.Context, mainWindow *window.MainWindow) | 33 | onCurrentWindowEmpty func(ctx context.Context, mainWindow *window.MainWindow) | ||
| 34 | onAttachPopupToTab func(ctx context.Context, tabID entity.TabID, pane *entity.Pane, wv port.WebView) // For popup tabs | 34 | onAttachPopupToTab func(ctx context.Context, tabID entity.TabID, pane *entity.Pane, wv port.WebView) // For popup tabs | ||
| 35 | onStateChanged func() // For session snapshots | 35 | onStateChanged func() // For session snapshots | ||
| 36 | + | ||||
| 37 | + | // previousActiveTabID provides the per-window previous active tab ID for Alt+Tab style switching. | |||
| 38 | + | // When set and tabScope is active, SwitchToLastActive prefers this over the global PreviousActiveTabID. | |||
| 39 | + | previousActiveTabID func(mainWindow *window.MainWindow) entity.TabID | |||
| 36 | } | 40 | } | ||
| 37 | 41 | ||||
| 38 | // TabCoordinatorConfig holds configuration for TabCoordinator. | 42 | // TabCoordinatorConfig holds configuration for TabCoordinator. | ||
| @@ -77,6 +81,13 @@ func (c *TabCoordinator) SetOnStateChanged(fn func()) { | |||||
| 77 | c.onStateChanged = fn | 81 | c.onStateChanged = fn | ||
| 78 | } | 82 | } | ||
| 79 | 83 | ||||
| 84 | + | // SetPreviousActiveTabIDProvider sets the callback for retrieving the per-window | |||
| 85 | + | // previous active tab ID. When set and tabScope is active, SwitchToLastActive | |||
| 86 | + | // prefers this provider over the global PreviousActiveTabID. | |||
| 87 | + | func (c *TabCoordinator) SetPreviousActiveTabIDProvider(fn func(mainWindow *window.MainWindow) entity.TabID) { | |||
| 88 | + | c.previousActiveTabID = fn | |||
| 89 | + | } | |||
| 90 | + | ||||
| 80 | // SetTabScope sets the function used to filter tabs to the current window. | 91 | // SetTabScope sets the function used to filter tabs to the current window. | ||
| 81 | // When set, tab operations (switch by index, next/prev, close) only consider | 92 | // When set, tab operations (switch by index, next/prev, close) only consider | ||
| 82 | // tabs for which the scope function returns true. | 93 | // tabs for which the scope function returns true. | ||
| @@ -202,6 +213,9 @@ func (c *TabCoordinator) Close(ctx context.Context) error { | |||||
| 202 | // Compute the next scoped tab to activate BEFORE closing | 213 | // Compute the next scoped tab to activate BEFORE closing | ||
| 203 | scoped := c.scopedTabs() | 214 | scoped := c.scopedTabs() | ||
| 204 | activeIdx := indexOfTab(scoped, activeID) | 215 | activeIdx := indexOfTab(scoped, activeID) | ||
| 216 | + | if activeIdx < 0 && len(scoped) > 0 { | |||
| 217 | + | log.Debug().Str("active_tab_id", string(activeID)).Int("scoped_count", len(scoped)).Msg("active tab not in scoped list") | |||
| 218 | + | } | |||
| 205 | 219 | ||||
| 206 | var nextScopedID entity.TabID | 220 | var nextScopedID entity.TabID | ||
| 207 | if activeIdx >= 0 && len(scoped) > 1 { | 221 | if activeIdx >= 0 && len(scoped) > 1 { | ||
| @@ -384,24 +398,35 @@ func (c *TabCoordinator) EnsureTabByIndex(ctx context.Context, index int, initia | |||||
| 384 | } | 398 | } | ||
| 385 | 399 | ||||
| 386 | // SwitchToLastActive switches to the previously active tab (Alt+Tab style). | 400 | // SwitchToLastActive switches to the previously active tab (Alt+Tab style). | ||
| 387 | - | // Uses per-window prevActiveTabID when scope is active; falls back to global otherwise. | 401 | + | // When window-scoped (tabScope is set and previousActiveTabID provider is available), |
| 402 | + | // it uses the per-window previous active tab ID. Falls back to the global | |||
| 403 | + | // PreviousActiveTabID otherwise. | |||
| 388 | func (c *TabCoordinator) SwitchToLastActive(ctx context.Context) error { | 404 | func (c *TabCoordinator) SwitchToLastActive(ctx context.Context) error { | ||
| 389 | log := logging.FromContext(ctx) | 405 | log := logging.FromContext(ctx) | ||
| 390 | 406 | ||||
| 391 | - | // Use the global PreviousActiveTabID directly when there's no window scope | 407 | + | var prevID entity.TabID |
| 392 | - | prevID := c.tabs.PreviousActiveTabID | 408 | + | if c.tabScope != nil && c.previousActiveTabID != nil { |
| 409 | + | // Use per-window previous active tab ID when scoped | |||
| 410 | + | prevID = c.previousActiveTabID(c.mainWindow) | |||
| 411 | + | } else { | |||
| 412 | + | // Fall back to global PreviousActiveTabID when not scoped or no provider | |||
| 413 | + | prevID = c.tabs.PreviousActiveTabID | |||
| 414 | + | } | |||
| 415 | + | ||||
| 393 | if prevID == "" || prevID == c.tabs.ActiveTabID { | 416 | if prevID == "" || prevID == c.tabs.ActiveTabID { | ||
| 394 | return nil | 417 | return nil | ||
| 395 | } | 418 | } | ||
| 396 | 419 | ||||
| 397 | tab := c.tabs.Find(prevID) | 420 | tab := c.tabs.Find(prevID) | ||
| 398 | if tab == nil { | 421 | if tab == nil { | ||
| 399 | - | c.tabs.PreviousActiveTabID = "" | 422 | + | // Only clear the global previous active tab ID if that's what we used |
| 423 | + | if c.tabScope == nil || c.previousActiveTabID == nil { | |||
| 424 | + | c.tabs.PreviousActiveTabID = "" | |||
| 425 | + | } | |||
| 400 | return nil | 426 | return nil | ||
| 401 | } | 427 | } | ||
| 402 | 428 | ||||
| 403 | // With a scope function, only allow switching to the previous tab if it's in scope. | 429 | // With a scope function, only allow switching to the previous tab if it's in scope. | ||
| 404 | - | // Per-window prevActiveTabID is tracked by the App layer and always in-scope. | |||
| 405 | if c.tabScope != nil && !c.tabScope(tab, c.mainWindow) { | 430 | if c.tabScope != nil && !c.tabScope(tab, c.mainWindow) { | ||
| 406 | log.Debug().Str("tab_id", string(prevID)).Msg("previous active tab outside current window scope") | 431 | log.Debug().Str("tab_id", string(prevID)).Msg("previous active tab outside current window scope") | ||
| 407 | return nil | 432 | return nil | ||