Fix close active tab behavior (#1367)

This fixes a bug where closing the active tab would clean up the closed
tab view before switching to an un-closed tab view, putting the window
into an unrecoverable state. This also moves around some of the CloseTab
logic so that it's more standardized and reduces unnecessary
frontend-backend comms and DB writes

emain/emain-window.ts

@@ -282,10 +282,11 @@ export class WaveBrowserWindow extends BaseWindow {
         await this.queueTabSwitch(tabView, tabInitialized);
     }
 
-    async setActiveTab(tabId: string) {
+    async setActiveTab(tabId: string, setInBackend: boolean) {
-        console.log("setActiveTab", this);
+        console.log("setActiveTab", tabId, this.waveWindowId, this.workspaceId, setInBackend);
-        const workspace = await WorkspaceService.GetWorkspace(this.workspaceId);
+        if (setInBackend) {
-        await WorkspaceService.SetActiveTab(workspace.oid, tabId);
+            await WorkspaceService.SetActiveTab(this.workspaceId, tabId);
+        }
         const fullConfig = await FileService.GetFullConfig();
         const [tabView, tabInitialized] = getOrCreateWebViewForTab(fullConfig, tabId);
         await this.queueTabSwitch(tabView, tabInitialized);
@@ -293,19 +294,20 @@ export class WaveBrowserWindow extends BaseWindow {
 
     async createTab() {
         const tabId = await WorkspaceService.CreateTab(this.workspaceId, null, true);
-        this.setActiveTab(tabId);
+        await this.setActiveTab(tabId, false);
     }
 
     async closeTab(tabId: string) {
+        console.log("closeTab", tabId, this.waveWindowId, this.workspaceId);
         const tabView = this.allTabViews.get(tabId);
         if (tabView) {
             const rtn = await WorkspaceService.CloseTab(this.workspaceId, tabId, true);
-            this.allTabViews.delete(tabId);
             if (rtn?.closewindow) {
                 this.close();
             } else if (rtn?.newactivetabid) {
-                this.setActiveTab(rtn.newactivetabid);
+                await this.setActiveTab(rtn.newactivetabid, false);
             }
+            this.allTabViews.delete(tabId);
         }
     }
 
@@ -519,7 +521,7 @@ export async function createBrowserWindow(
     console.log("createBrowserWindow", waveWindow.oid, workspace.oid, workspace);
     const bwin = new WaveBrowserWindow(waveWindow, fullConfig, opts);
     if (workspace.activetabid) {
-        await bwin.setActiveTab(workspace.activetabid);
+        await bwin.setActiveTab(workspace.activetabid, false);
     }
     return bwin;
 }
@@ -527,7 +529,7 @@ export async function createBrowserWindow(
 ipcMain.on("set-active-tab", async (event, tabId) => {
     const ww = getWaveWindowByWebContentsId(event.sender.id);
     console.log("set-active-tab", tabId, ww?.waveWindowId);
-    await ww?.setActiveTab(tabId);
+    await ww?.setActiveTab(tabId, true);
 });
 
 ipcMain.on("create-tab", async (event, opts) => {

pkg/service/workspaceservice/workspaceservice.go

@@ -164,28 +164,17 @@ func (svc *WorkspaceService) CloseTab(ctx context.Context, workspaceId string, t
 	if err != nil {
 		return nil, nil, fmt.Errorf("error getting tab: %w", err)
 	}
-	ws, err := wstore.DBMustGet[*waveobj.Workspace](ctx, workspaceId)
-	if err != nil {
-		return nil, nil, fmt.Errorf("error getting workspace: %w", err)
-	}
-	tabIndex := -1
-	for i, id := range ws.TabIds {
-		if id == tabId {
-			tabIndex = i
-			break
-		}
-	}
 	go func() {
 		for _, blockId := range tab.BlockIds {
 			blockcontroller.StopBlockController(blockId)
 		}
 	}()
-	if err := wcore.DeleteTab(ctx, workspaceId, tabId); err != nil {
+	newActiveTabId, err := wcore.DeleteTab(ctx, workspaceId, tabId)
+	if err != nil {
 		return nil, nil, fmt.Errorf("error closing tab: %w", err)
 	}
 	rtn := &CloseTabRtnType{}
-	if ws.ActiveTabId == tabId && tabIndex != -1 {
+	if newActiveTabId == "" {
-		if len(ws.TabIds) == 1 {
 		windowId, err := wstore.DBFindWindowForWorkspaceId(ctx, workspaceId)
 		if err != nil {
 			return rtn, nil, fmt.Errorf("unable to find window for workspace id %v: %w", workspaceId, err)
@@ -196,22 +185,7 @@ func (svc *WorkspaceService) CloseTab(ctx context.Context, workspaceId string, t
 			return rtn, nil, err
 		}
 	} else {
-			if tabIndex < len(ws.TabIds)-1 {
-				newActiveTabId := ws.TabIds[tabIndex+1]
-				err := wcore.SetActiveTab(ctx, ws.OID, newActiveTabId)
-				if err != nil {
-					return rtn, nil, err
-				}
 		rtn.NewActiveTabId = newActiveTabId
-			} else {
-				newActiveTabId := ws.TabIds[tabIndex-1]
-				err := wcore.SetActiveTab(ctx, ws.OID, newActiveTabId)
-				if err != nil {
-					return rtn, nil, err
-				}
-				rtn.NewActiveTabId = newActiveTabId
-			}
-		}
 	}
 	updates := waveobj.ContextGetUpdatesRtn(ctx)
 	go func() {

pkg/wcore/window.go

@@ -108,7 +108,7 @@ func CreateWindow(ctx context.Context, winSize *waveobj.WinSize, workspaceId str
 	if err != nil {
 		return nil, fmt.Errorf("error inserting window: %w", err)
 	}
-	client, err := wstore.DBGetSingleton[*waveobj.Client](ctx)
+	client, err := GetClientData(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("error getting client: %w", err)
 	}
@@ -117,12 +117,12 @@ func CreateWindow(ctx context.Context, winSize *waveobj.WinSize, workspaceId str
 	if err != nil {
 		return nil, fmt.Errorf("error updating client: %w", err)
 	}
-	return wstore.DBMustGet[*waveobj.Window](ctx, windowId)
+	return GetWindow(ctx, windowId)
 }
 
 func CloseWindow(ctx context.Context, windowId string, fromElectron bool) error {
 	log.Printf("CloseWindow %s\n", windowId)
-	window, err := wstore.DBMustGet[*waveobj.Window](ctx, windowId)
+	window, err := GetWindow(ctx, windowId)
 	if err == nil {
 		log.Printf("got window %s\n", windowId)
 		deleted, err := DeleteWorkspace(ctx, window.WorkspaceId, false)

pkg/wcore/workspace.go

@@ -39,7 +39,7 @@ func DeleteWorkspace(ctx context.Context, workspaceId string, force bool) (bool,
 	}
 	for _, tabId := range workspace.TabIds {
 		log.Printf("deleting tab %s\n", tabId)
-		err := DeleteTab(ctx, workspaceId, tabId)
+		_, err := DeleteTab(ctx, workspaceId, tabId)
 		if err != nil {
 			return false, fmt.Errorf("error closing tab: %w", err)
 		}
@@ -57,9 +57,9 @@ func GetWorkspace(ctx context.Context, wsID string) (*waveobj.Workspace, error)
 }
 
 func createTabObj(ctx context.Context, workspaceId string, name string) (*waveobj.Tab, error) {
-	ws, _ := wstore.DBGet[*waveobj.Workspace](ctx, workspaceId)
+	ws, err := GetWorkspace(ctx, workspaceId)
-	if ws == nil {
+	if err != nil {
-		return nil, fmt.Errorf("workspace not found: %q", workspaceId)
+		return nil, fmt.Errorf("workspace %s not found: %w", workspaceId, err)
 	}
 	layoutStateId := uuid.NewString()
 	tab := &waveobj.Tab{
@@ -81,19 +81,11 @@ func createTabObj(ctx context.Context, workspaceId string, name string) (*waveob
 // returns tabid
 func CreateTab(ctx context.Context, workspaceId string, tabName string, activateTab bool) (string, error) {
 	if tabName == "" {
-		client, err := wstore.DBGetSingleton[*waveobj.Client](ctx)
+		ws, err := GetWorkspace(ctx, workspaceId)
 		if err != nil {
-			return "", fmt.Errorf("error getting client: %w", err)
+			return "", fmt.Errorf("workspace %s not found: %w", workspaceId, err)
-		}
-		ws, _ := wstore.DBGet[*waveobj.Workspace](ctx, workspaceId)
-		if ws == nil {
-			return "", fmt.Errorf("workspace not found: %q", workspaceId)
 		}
 		tabName = "T" + fmt.Sprint(len(ws.TabIds)+1)
-		err = wstore.DBUpdate(ctx, client)
-		if err != nil {
-			return "", fmt.Errorf("error updating client: %w", err)
-		}
 	}
 	tab, err := createTabObj(ctx, workspaceId, tabName)
 	if err != nil {
@@ -109,49 +101,60 @@ func CreateTab(ctx context.Context, workspaceId string, tabName string, activate
 	return tab.OID, nil
 }
 
-// must delete all blocks individually first
+// Must delete all blocks individually first.
-// also deletes LayoutState
+// Also deletes LayoutState.
-func DeleteTab(ctx context.Context, workspaceId string, tabId string) error {
+// Returns new active tab id, error.
+func DeleteTab(ctx context.Context, workspaceId string, tabId string) (string, error) {
 	ws, _ := wstore.DBGet[*waveobj.Workspace](ctx, workspaceId)
 	if ws == nil {
-		return fmt.Errorf("workspace not found: %q", workspaceId)
+		return "", fmt.Errorf("workspace not found: %q", workspaceId)
 	}
 	tab, _ := wstore.DBGet[*waveobj.Tab](ctx, tabId)
 	if tab == nil {
-		return fmt.Errorf("tab not found: %q", tabId)
+		return "", fmt.Errorf("tab not found: %q", tabId)
 	}
 
 	// close blocks (sends events + stops block controllers)
 	for _, blockId := range tab.BlockIds {
 		err := DeleteBlock(ctx, blockId)
 		if err != nil {
-			return fmt.Errorf("error deleting block %s: %w", blockId, err)
+			return "", fmt.Errorf("error deleting block %s: %w", blockId, err)
 		}
 	}
 	tabIdx := utilfn.FindStringInSlice(ws.TabIds, tabId)
 	if tabIdx == -1 {
-		return nil
+		return "", nil
 	}
 	ws.TabIds = append(ws.TabIds[:tabIdx], ws.TabIds[tabIdx+1:]...)
+	newActiveTabId := ws.ActiveTabId
+	if len(ws.TabIds) > 0 {
+		if ws.ActiveTabId == tabId {
+			newActiveTabId = ws.TabIds[max(0, min(tabIdx-1, len(ws.TabIds)-1))]
+		}
+	} else {
+		newActiveTabId = ""
+	}
+	ws.ActiveTabId = newActiveTabId
+
 	wstore.DBUpdate(ctx, ws)
 	wstore.DBDelete(ctx, waveobj.OType_Tab, tabId)
 	wstore.DBDelete(ctx, waveobj.OType_LayoutState, tab.LayoutState)
-	return nil
+	return newActiveTabId, nil
 }
 
 func SetActiveTab(ctx context.Context, workspaceId string, tabId string) error {
-	workspace, _ := wstore.DBGet[*waveobj.Workspace](ctx, workspaceId)
-	if workspace == nil {
-		return fmt.Errorf("workspace not found: %q", workspaceId)
-	}
 	if tabId != "" {
+		workspace, err := GetWorkspace(ctx, workspaceId)
+		if err != nil {
+			return fmt.Errorf("workspace %s not found: %w", workspaceId, err)
+		}
 		tab, _ := wstore.DBGet[*waveobj.Tab](ctx, tabId)
 		if tab == nil {
 			return fmt.Errorf("tab not found: %q", tabId)
 		}
-	}
 		workspace.ActiveTabId = tabId
 		wstore.DBUpdate(ctx, workspace)
+	}
 	return nil
 }