From e0060b12e5e96b9806d166f29ddef36a6ff0bb7d Mon Sep 17 00:00:00 2001 From: Jordi Bunster Date: Sun, 18 Apr 2021 17:48:39 -0700 Subject: [PATCH] Keep active tab state in one place (manager) (#683) With this change TabsBase objects will retain a reference to the TabsManager they belong to, so they can ask it if they're the active tab or not. This removes the possibility for bugs like activating an unmanaged tab, or having more than one active tab, etc. --- src/Contracts/ViewModels.ts | 1 - .../Settings/SettingsComponent.test.tsx | 1 - src/Explorer/Explorer.tsx | 8 +- .../CommandBar/CommandBarComponentAdapter.tsx | 4 +- src/Explorer/Tabs/DocumentsTab.test.ts | 12 --- src/Explorer/Tabs/NotebookV2Tab.ts | 1 - src/Explorer/Tabs/QueryTab.test.ts | 1 - src/Explorer/Tabs/TabsBase.ts | 41 ++++------ src/Explorer/Tabs/TabsManager.html | 27 ++++--- src/Explorer/Tabs/TabsManager.test.ts | 17 ++-- src/Explorer/Tabs/TabsManager.ts | 77 ++++++------------- src/Explorer/Tree/Collection.ts | 12 --- src/Explorer/Tree/Database.ts | 1 - src/Explorer/Tree/ResourceTokenCollection.ts | 2 - src/Explorer/Tree/StoredProcedure.ts | 4 +- src/Explorer/Tree/Trigger.ts | 4 +- src/Explorer/Tree/UserDefinedFunction.ts | 4 +- 17 files changed, 74 insertions(+), 143 deletions(-) diff --git a/src/Contracts/ViewModels.ts b/src/Contracts/ViewModels.ts index 71d76624b..dc14bb86c 100644 --- a/src/Contracts/ViewModels.ts +++ b/src/Contracts/ViewModels.ts @@ -276,7 +276,6 @@ export interface TabOptions { tabKind: CollectionTabKind; title: string; tabPath: string; - isActive: ko.Observable; hashLocation: string; onUpdateTabsButtons: (buttons: CommandButtonComponentProps[]) => void; isTabsContentExpanded?: ko.Observable; diff --git a/src/Explorer/Controls/Settings/SettingsComponent.test.tsx b/src/Explorer/Controls/Settings/SettingsComponent.test.tsx index 2db5adf5f..69ee13439 100644 --- a/src/Explorer/Controls/Settings/SettingsComponent.test.tsx +++ b/src/Explorer/Controls/Settings/SettingsComponent.test.tsx @@ -39,7 +39,6 @@ describe("SettingsComponent", () => { tabPath: "", node: undefined, hashLocation: "settings", - isActive: ko.observable(false), onUpdateTabsButtons: undefined, }), }; diff --git a/src/Explorer/Explorer.tsx b/src/Explorer/Explorer.tsx index 474efdff2..19a559458 100644 --- a/src/Explorer/Explorer.tsx +++ b/src/Explorer/Explorer.tsx @@ -559,6 +559,12 @@ export default class Explorer { }); this.tabsManager = params?.tabsManager ?? new TabsManager(); + this.tabsManager.openedTabs.subscribe((tabs) => { + if (tabs.length === 0) { + this.selectedNode(undefined); + this.onUpdateTabsButtons([]); + } + }); this._panes = [ this.addDatabasePane, @@ -1570,7 +1576,6 @@ export default class Explorer { collection: null, masterKey: userContext.masterKey || "", hashLocation: "notebooks", - isActive: ko.observable(false), isTabsContentExpanded: ko.observable(true), onLoadStartKey: null, onUpdateTabsButtons: this.onUpdateTabsButtons, @@ -1976,7 +1981,6 @@ export default class Explorer { tabPath: title, collection: null, hashLocation: hashLocation, - isActive: ko.observable(false), isTabsContentExpanded: ko.observable(true), onLoadStartKey: null, onUpdateTabsButtons: this.onUpdateTabsButtons, diff --git a/src/Explorer/Menus/CommandBar/CommandBarComponentAdapter.tsx b/src/Explorer/Menus/CommandBar/CommandBarComponentAdapter.tsx index 65ff5e176..d93407c22 100644 --- a/src/Explorer/Menus/CommandBar/CommandBarComponentAdapter.tsx +++ b/src/Explorer/Menus/CommandBar/CommandBarComponentAdapter.tsx @@ -23,8 +23,8 @@ export class CommandBarComponentAdapter implements ReactAdapter { constructor(container: Explorer) { this.container = container; this.tabsButtons = []; - this.isNotebookTabActive = ko.computed(() => - container.tabsManager.isTabActive(ViewModels.CollectionTabKind.NotebookV2) + this.isNotebookTabActive = ko.computed( + () => container.tabsManager.activeTab()?.tabKind === ViewModels.CollectionTabKind.NotebookV2 ); // These are the parameters watched by the react binding that will trigger a renderComponent() if one of the ko mutates diff --git a/src/Explorer/Tabs/DocumentsTab.test.ts b/src/Explorer/Tabs/DocumentsTab.test.ts index df46d171a..1d470f23a 100644 --- a/src/Explorer/Tabs/DocumentsTab.test.ts +++ b/src/Explorer/Tabs/DocumentsTab.test.ts @@ -16,8 +16,6 @@ describe("Documents tab", () => { title: "", tabPath: "", hashLocation: "", - isActive: ko.observable(false), - onUpdateTabsButtons: (buttons: CommandButtonComponentProps[]): void => {}, }); @@ -89,8 +87,6 @@ describe("Documents tab", () => { title: "", tabPath: "", hashLocation: "", - isActive: ko.observable(false), - onUpdateTabsButtons: (buttons: CommandButtonComponentProps[]): void => {}, }); @@ -106,8 +102,6 @@ describe("Documents tab", () => { title: "", tabPath: "", hashLocation: "", - isActive: ko.observable(false), - onUpdateTabsButtons: (buttons: CommandButtonComponentProps[]): void => {}, }); @@ -123,8 +117,6 @@ describe("Documents tab", () => { title: "", tabPath: "", hashLocation: "", - isActive: ko.observable(false), - onUpdateTabsButtons: (buttons: CommandButtonComponentProps[]): void => {}, }); @@ -140,8 +132,6 @@ describe("Documents tab", () => { title: "", tabPath: "", hashLocation: "", - isActive: ko.observable(false), - onUpdateTabsButtons: (buttons: CommandButtonComponentProps[]): void => {}, }); @@ -157,8 +147,6 @@ describe("Documents tab", () => { title: "", tabPath: "", hashLocation: "", - isActive: ko.observable(false), - onUpdateTabsButtons: (buttons: CommandButtonComponentProps[]): void => {}, }); diff --git a/src/Explorer/Tabs/NotebookV2Tab.ts b/src/Explorer/Tabs/NotebookV2Tab.ts index a7c2242c2..c3f3817be 100644 --- a/src/Explorer/Tabs/NotebookV2Tab.ts +++ b/src/Explorer/Tabs/NotebookV2Tab.ts @@ -88,7 +88,6 @@ export default class NotebookTabV2 extends TabsBase { public onCloseTabButtonClick(): Q.Promise { const cleanup = () => { this.notebookComponentAdapter.notebookShutdown(); - this.isActive(false); super.onCloseTabButtonClick(); }; diff --git a/src/Explorer/Tabs/QueryTab.test.ts b/src/Explorer/Tabs/QueryTab.test.ts index 2e588a99d..c146f6d6f 100644 --- a/src/Explorer/Tabs/QueryTab.test.ts +++ b/src/Explorer/Tabs/QueryTab.test.ts @@ -25,7 +25,6 @@ describe("Query Tab", () => { database: database, title: "", tabPath: "", - isActive: ko.observable(false), hashLocation: "", onUpdateTabsButtons: (buttons: CommandButtonComponentProps[]): void => {}, }); diff --git a/src/Explorer/Tabs/TabsBase.ts b/src/Explorer/Tabs/TabsBase.ts index 23dbde0a4..3addaba4f 100644 --- a/src/Explorer/Tabs/TabsBase.ts +++ b/src/Explorer/Tabs/TabsBase.ts @@ -1,15 +1,16 @@ import * as ko from "knockout"; import Q from "q"; import * as Constants from "../../Common/Constants"; -import * as ViewModels from "../../Contracts/ViewModels"; -import * as DataModels from "../../Contracts/DataModels"; -import { Action, ActionModifiers } from "../../Shared/Telemetry/TelemetryConstants"; -import { RouteHandler } from "../../RouteHandlers/RouteHandler"; -import { WaitsForTemplateViewModel } from "../WaitsForTemplateViewModel"; -import * as TelemetryProcessor from "../../Shared/Telemetry/TelemetryProcessor"; import * as ThemeUtility from "../../Common/ThemeUtility"; -import Explorer from "../Explorer"; +import * as DataModels from "../../Contracts/DataModels"; +import * as ViewModels from "../../Contracts/ViewModels"; +import { RouteHandler } from "../../RouteHandlers/RouteHandler"; +import { Action, ActionModifiers } from "../../Shared/Telemetry/TelemetryConstants"; +import * as TelemetryProcessor from "../../Shared/Telemetry/TelemetryProcessor"; import { CommandButtonComponentProps } from "../Controls/CommandButton/CommandButtonComponent"; +import Explorer from "../Explorer"; +import { WaitsForTemplateViewModel } from "../WaitsForTemplateViewModel"; +import { TabsManager } from "./TabsManager"; // TODO: Use specific actions for logging telemetry data export default class TabsBase extends WaitsForTemplateViewModel { @@ -20,18 +21,16 @@ export default class TabsBase extends WaitsForTemplateViewModel { public database: ViewModels.Database; public rid: string; public hasFocus: ko.Observable; - public isActive: ko.Observable; public isMouseOver: ko.Observable; public tabId: string; public tabKind: ViewModels.CollectionTabKind; public tabTitle: ko.Observable; public tabPath: ko.Observable; - public closeButtonTabIndex: ko.Computed; - public errorDetailsTabIndex: ko.Computed; public hashLocation: ko.Observable; public isExecutionError: ko.Observable; public isExecuting: ko.Observable; public pendingNotification?: ko.Observable; + public manager?: TabsManager; protected _theme: string; public onLoadStartKey: number; @@ -46,7 +45,6 @@ export default class TabsBase extends WaitsForTemplateViewModel { this.database = options.database; this.rid = options.rid || (this.collection && this.collection.rid) || ""; this.hasFocus = ko.observable(false); - this.isActive = options.isActive || ko.observable(false); this.isMouseOver = ko.observable(false); this.tabId = `tab${id}`; this.tabKind = options.tabKind; @@ -55,21 +53,12 @@ export default class TabsBase extends WaitsForTemplateViewModel { (options.tabPath && ko.observable(options.tabPath)) || (this.collection && ko.observable(`${this.collection.databaseId}>${this.collection.id()}>${this.tabTitle()}`)); - this.closeButtonTabIndex = ko.computed(() => (this.isActive() ? 0 : null)); - this.errorDetailsTabIndex = ko.computed(() => (this.isActive() ? 0 : null)); this.isExecutionError = ko.observable(false); this.isExecuting = ko.observable(false); this.pendingNotification = ko.observable(undefined); this.onLoadStartKey = options.onLoadStartKey; this.hashLocation = ko.observable(options.hashLocation || ""); this.hashLocation.subscribe((newLocation: string) => this.updateGlobalHash(newLocation)); - - this.isActive.subscribe((isActive: boolean) => { - if (isActive) { - this.onActivate(); - } - }); - this.closeTabButton = { enabled: ko.computed(() => { return true; @@ -82,12 +71,9 @@ export default class TabsBase extends WaitsForTemplateViewModel { } public onCloseTabButtonClick(): void { - const explorer = this.getContainer(); - explorer.tabsManager.closeTab(this.tabId, explorer); - + this.manager?.closeTab(this); TelemetryProcessor.trace(Action.Tab, ActionModifiers.Close, { tabName: this.constructor.name, - dataExplorerArea: Constants.Areas.Tab, tabTitle: this.tabTitle(), tabId: this.tabId, @@ -95,7 +81,7 @@ export default class TabsBase extends WaitsForTemplateViewModel { } public onTabClick(): void { - this.getContainer().tabsManager.activateTab(this); + this.manager?.activateTab(this); } protected updateSelectedNode(): void { @@ -127,6 +113,11 @@ export default class TabsBase extends WaitsForTemplateViewModel { return this.onSpaceOrEnterKeyPress(event, () => this.onCloseTabButtonClick()); }; + /** @deprecated this is no longer observable, bind to comparisons with manager.activeTab() instead */ + public isActive() { + return this === this.manager?.activeTab(); + } + public onActivate(): void { this.updateSelectedNode(); if (!!this.collection) { diff --git a/src/Explorer/Tabs/TabsManager.html b/src/Explorer/Tabs/TabsManager.html index d66a2ba33..a876cf38d 100644 --- a/src/Explorer/Tabs/TabsManager.html +++ b/src/Explorer/Tabs/TabsManager.html @@ -1,4 +1,8 @@ -
+