diff --git a/app/common/config-schemata.ts b/app/common/config-schemata.ts index 8d74f2583..87a7757a5 100644 --- a/app/common/config-schemata.ts +++ b/app/common/config-schemata.ts @@ -21,7 +21,7 @@ export const configSchemata = { downloadsPath: z.string(), enableSpellchecker: z.boolean(), errorReporting: z.boolean(), - lastActiveTab: z.number(), + lastActiveTabId: z.string().optional(), promptDownload: z.boolean(), proxyBypass: z.string(), // eslint-disable-next-line @typescript-eslint/naming-convention diff --git a/app/common/typed-ipc.ts b/app/common/typed-ipc.ts index ca10625a7..4874c34de 100644 --- a/app/common/typed-ipc.ts +++ b/app/common/typed-ipc.ts @@ -1,5 +1,5 @@ import type {DndSettings} from "./dnd-util.ts"; -import type {MenuProperties, ServerConfig} from "./types.ts"; +import type {MenuProperties, ServerSettings} from "./types.ts"; export type MainMessage = { "clear-app-settings": () => void; @@ -13,8 +13,8 @@ export type MainMessage = { "realm-icon-changed": (serverURL: string, iconURL: string) => void; "realm-name-changed": (serverURL: string, realmName: string) => void; "reload-full-app": () => void; - "save-last-tab": (index: number) => void; - "switch-server-tab": (index: number) => void; + "save-last-tab": (tabId: string) => void; + "switch-server-tab": (serverId: string) => void; "toggle-app": () => void; "toggle-badge-option": (newValue: boolean) => void; "toggle-menubar": (showMenubar: boolean) => void; @@ -26,7 +26,7 @@ export type MainMessage = { }; export type MainCall = { - "get-server-settings": (domain: string) => ServerConfig; + "get-server-settings": (domain: string) => ServerSettings; "is-online": (url: string) => boolean; "poll-clipboard": (key: Uint8Array, sig: Uint8Array) => string | undefined; "save-server-icon": (iconURL: string) => string | null; @@ -63,7 +63,7 @@ export type RendererMessage = { "set-idle": () => void; "show-keyboard-shortcuts": () => void; "show-notification-settings": () => void; - "switch-server-tab": (index: number) => void; + "switch-server-tab": (serverId: string) => void; "tab-devtools": () => void; "toggle-autohide-menubar": ( autoHideMenubar: boolean, diff --git a/app/common/types.ts b/app/common/types.ts index def623e14..fa5295419 100644 --- a/app/common/types.ts +++ b/app/common/types.ts @@ -1,6 +1,6 @@ export type MenuProperties = { tabs: TabData[]; - activeTabIndex?: number; + activeTabId?: string; enableMenu?: boolean; }; @@ -12,6 +12,7 @@ export type NavigationItem = | "Shortcuts"; export type ServerConfig = { + id: string; url: string; alias: string; icon: string; @@ -19,6 +20,8 @@ export type ServerConfig = { zulipFeatureLevel: number; }; +export type ServerSettings = Omit; + export type TabRole = "server" | "function"; export type TabPage = "Settings" | "About"; @@ -26,5 +29,7 @@ export type TabData = { role: TabRole; page?: TabPage; label: string; - index: number; + order: number; + id: string; + serverId?: string; }; diff --git a/app/main/index.ts b/app/main/index.ts index d15de96cb..43643b532 100644 --- a/app/main/index.ts +++ b/app/main/index.ts @@ -428,9 +428,11 @@ function createMainWindow(): BrowserWindow { ipcMain.on("update-menu", (_event, properties: MenuProperties) => { AppMenu.setMenu(properties); - if (properties.activeTabIndex !== undefined) { - const activeTab = properties.tabs[properties.activeTabIndex]; - mainWindow.setTitle(`Zulip - ${activeTab.label}`); + if (properties.activeTabId !== undefined) { + const activeTab = properties.tabs.find( + (tab) => tab.id === properties.activeTabId, + ); + mainWindow.setTitle(`Zulip - ${activeTab!.label}`); } }); @@ -452,8 +454,8 @@ function createMainWindow(): BrowserWindow { }, ); - ipcMain.on("save-last-tab", (_event, index: number) => { - ConfigUtil.setConfigItem("lastActiveTab", index); + ipcMain.on("save-last-tab", (_event, tabId: string) => { + ConfigUtil.setConfigItem("lastActiveTabId", tabId); }); ipcMain.on("focus-this-webview", (event) => { diff --git a/app/main/menu.ts b/app/main/menu.ts index ae1873895..718d036b4 100644 --- a/app/main/menu.ts +++ b/app/main/menu.ts @@ -294,7 +294,7 @@ function getHelpSubmenu(): MenuItemConstructorOptions[] { function getWindowSubmenu( tabs: TabData[], - activeTabIndex?: number, + activeTabId?: string, ): MenuItemConstructorOptions[] { const initialSubmenu: MenuItemConstructorOptions[] = [ { @@ -313,8 +313,6 @@ function getWindowSubmenu( type: "separator", }); for (const tab of tabs) { - // Skip missing elements left by `delete this.tabs[index]` in - // ServerManagerView. if (tab === undefined) continue; // Do not add functional tab settings to list of windows in menu bar @@ -325,11 +323,11 @@ function getWindowSubmenu( initialSubmenu.push({ label: tab.label, accelerator: - tab.role === "function" ? "" : `${shortcutKey} + ${tab.index + 1}`, - checked: tab.index === activeTabIndex, + tab.role === "function" ? "" : `${shortcutKey} + ${tab.order + 1}`, + checked: tab.id === activeTabId, click(_item, focusedWindow) { if (focusedWindow) { - sendAction("switch-server-tab", tab.index); + sendAction("switch-server-tab", tab.serverId!); } }, type: "checkbox", @@ -346,10 +344,7 @@ function getWindowSubmenu( enabled: tabs.length > 1, click(_item, focusedWindow) { if (focusedWindow) { - sendAction( - "switch-server-tab", - getNextServer(tabs, activeTabIndex!), - ); + sendAction("switch-server-tab", getNextServer(tabs, activeTabId!)); } }, }, @@ -361,7 +356,7 @@ function getWindowSubmenu( if (focusedWindow) { sendAction( "switch-server-tab", - getPreviousServer(tabs, activeTabIndex!), + getPreviousServer(tabs, activeTabId!), ); } }, @@ -375,7 +370,7 @@ function getWindowSubmenu( function getDarwinTpl( properties: MenuProperties, ): MenuItemConstructorOptions[] { - const {tabs, activeTabIndex, enableMenu = false} = properties; + const {tabs, activeTabId, enableMenu = false} = properties; return [ { @@ -525,7 +520,7 @@ function getDarwinTpl( }, { label: t.__("Window"), - submenu: getWindowSubmenu(tabs, activeTabIndex), + submenu: getWindowSubmenu(tabs, activeTabId), }, { label: t.__("Tools"), @@ -540,7 +535,7 @@ function getDarwinTpl( } function getOtherTpl(properties: MenuProperties): MenuItemConstructorOptions[] { - const {tabs, activeTabIndex, enableMenu = false} = properties; + const {tabs, activeTabId, enableMenu = false} = properties; return [ { label: t.__("File"), @@ -673,7 +668,7 @@ function getOtherTpl(properties: MenuProperties): MenuItemConstructorOptions[] { }, { label: t.__("Window"), - submenu: getWindowSubmenu(tabs, activeTabIndex), + submenu: getWindowSubmenu(tabs, activeTabId), }, { label: t.__("Tools"), @@ -704,20 +699,28 @@ async function checkForUpdate(): Promise { await appUpdater(true); } -function getNextServer(tabs: TabData[], activeTabIndex: number): number { +function getTabByOrder(tabs: TabData[], order: number): TabData | undefined { + return tabs.find((tab) => tab.order === order); +} + +function getNextServer(tabs: TabData[], activeTabId: string): string { + const activeTab = tabs.find((tab) => tab.id === activeTabId)!; + let {order} = activeTab; do { - activeTabIndex = (activeTabIndex + 1) % tabs.length; - } while (tabs[activeTabIndex]?.role !== "server"); + order = (order + 1) % tabs.length; + } while (getTabByOrder(tabs, order)?.role !== "server"); - return activeTabIndex; + return getTabByOrder(tabs, order)!.serverId!; } -function getPreviousServer(tabs: TabData[], activeTabIndex: number): number { +function getPreviousServer(tabs: TabData[], activeTabId: string): string { + const activeTab = tabs.find((tab) => tab.id === activeTabId)!; + let {order} = activeTab; do { - activeTabIndex = (activeTabIndex - 1 + tabs.length) % tabs.length; - } while (tabs[activeTabIndex]?.role !== "server"); + order = (order - 1) % tabs.length; + } while (getTabByOrder(tabs, order)?.role !== "server"); - return activeTabIndex; + return getTabByOrder(tabs, order)!.serverId!; } export function setMenu(properties: MenuProperties): void { diff --git a/app/main/request.ts b/app/main/request.ts index 4ffbc3dcd..aeb24f41e 100644 --- a/app/main/request.ts +++ b/app/main/request.ts @@ -10,7 +10,7 @@ import {z} from "zod"; import Logger from "../common/logger-util.ts"; import * as Messages from "../common/messages.ts"; -import type {ServerConfig} from "../common/types.ts"; +import type {ServerSettings} from "../common/types.ts"; /* Request: domain-util */ @@ -42,7 +42,7 @@ const generateFilePath = (url: string): string => { export const _getServerSettings = async ( domain: string, session: Session, -): Promise => { +): Promise => { const response = await session.fetch(domain + "/api/v1/server_settings"); if (!response.ok) { throw new Error(Messages.invalidZulipServerError(domain)); diff --git a/app/renderer/js/components/functional-tab.ts b/app/renderer/js/components/functional-tab.ts index 72614deb2..7c1f5755e 100644 --- a/app/renderer/js/components/functional-tab.ts +++ b/app/renderer/js/components/functional-tab.ts @@ -43,7 +43,7 @@ export default class FunctionalTab extends Tab { templateHtml(): Html { return html` -
+
close
diff --git a/app/renderer/js/components/server-tab.ts b/app/renderer/js/components/server-tab.ts index b03c02e64..f4cb67c3e 100644 --- a/app/renderer/js/components/server-tab.ts +++ b/app/renderer/js/components/server-tab.ts @@ -9,19 +9,22 @@ import type WebView from "./webview.ts"; export type ServerTabProperties = { webview: Promise; + serverId: string; } & TabProperties; export default class ServerTab extends Tab { webview: Promise; + serverId: string; $el: Element; $name: Element; $icon: HTMLImageElement; $badge: Element; - constructor({webview, ...properties}: ServerTabProperties) { + constructor({webview, serverId, ...properties}: ServerTabProperties) { super(properties); this.webview = webview; + this.serverId = serverId; this.$el = generateNodeFromHtml(this.templateHtml()); this.properties.$root.append(this.$el); this.registerListeners(); @@ -47,7 +50,7 @@ export default class ServerTab extends Tab { templateHtml(): Html { return html` -
+
@@ -77,14 +80,14 @@ export default class ServerTab extends Tab { generateShortcutText(): string { // Only provide shortcuts for server [0..9] - if (this.properties.index >= 9) { + if (this.properties.order >= 9) { return ""; } - const shownIndex = this.properties.index + 1; + const shownIndex = this.properties.order + 1; - // Array index == Shown index - 1 - ipcRenderer.send("switch-server-tab", shownIndex - 1); + // Array index == Shown order - 1 + ipcRenderer.send("switch-server-tab", this.serverId); return process.platform === "darwin" ? `⌘${shownIndex}` diff --git a/app/renderer/js/components/tab.ts b/app/renderer/js/components/tab.ts index c4c949bc0..f18825f9a 100644 --- a/app/renderer/js/components/tab.ts +++ b/app/renderer/js/components/tab.ts @@ -7,8 +7,8 @@ export type TabProperties = { label: string; $root: Element; onClick: () => void; - index: number; - tabIndex: number; + order: number; + tabId: string; onHover?: () => void; onHoverOut?: () => void; materialIcon?: string; diff --git a/app/renderer/js/components/webview.ts b/app/renderer/js/components/webview.ts index 2e0bcf171..f203da42b 100644 --- a/app/renderer/js/components/webview.ts +++ b/app/renderer/js/components/webview.ts @@ -21,13 +21,12 @@ const shouldSilentWebview = ConfigUtil.getConfigItem("silent", false); type WebViewProperties = { $root: Element; rootWebContents: WebContents; - index: number; - tabIndex: number; + tabId: string; url: string; role: TabRole; isActive: () => boolean; switchLoading: (loading: boolean, url: string) => void; - onNetworkError: (index: number) => void; + onNetworkError: (id: string) => void; preload?: string; onTitleChange: () => void; hasPermission?: (origin: string, permission: string) => boolean; @@ -48,7 +47,7 @@ export default class WebView { ×
; $backTooltip: HTMLElement; $dndTooltip: HTMLElement; $sidebar: Element; $fullscreenPopup: Element; loading: Set; - activeTabIndex: number; + activeTabId?: string; tabs: ServerOrFunctionalTab[]; - functionalTabs: Map; - tabIndex: number; + functionalTabs: Map; presetOrgs: string[]; preferenceView?: PreferenceView; constructor() { @@ -107,15 +105,6 @@ export class ServerManagerView { this.$loadingTooltip = $actionsContainer.querySelector("#loading-tooltip")!; this.$settingsTooltip = $actionsContainer.querySelector("#setting-tooltip")!; - - // TODO: This should have been querySelector but the problem is that - // querySelector doesn't return elements not present in dom whereas somehow - // getElementsByClassName does. To fix this we need to call this after this.initTabs - // is called in this.init. - // eslint-disable-next-line unicorn/prefer-query-selector - this.$serverIconTooltip = document.getElementsByClassName( - "server-tooltip", - ) as HTMLCollectionOf; this.$backTooltip = $actionsContainer.querySelector("#back-tooltip")!; this.$dndTooltip = $actionsContainer.querySelector("#dnd-tooltip")!; @@ -128,11 +117,10 @@ export class ServerManagerView { ); this.loading = new Set(); - this.activeTabIndex = -1; + this.activeTabId = undefined; this.tabs = []; this.presetOrgs = []; this.functionalTabs = new Map(); - this.tabIndex = 0; } async init(): Promise { @@ -197,7 +185,7 @@ export class ServerManagerView { // eslint-disable-next-line @typescript-eslint/naming-convention customCSS: false, silent: false, - lastActiveTab: 0, + lastActiveTabId: undefined, dnd: false, dndPreviousSettings: { showNotification: true, @@ -334,7 +322,7 @@ export class ServerManagerView { (async () => { const serverConfig = await DomainUtil.updateSavedServer( server.url, - i, + server.id, ); tab.setLabel(serverConfig.alias); tab.setIcon(DomainUtil.iconAsUrl(serverConfig.icon)); @@ -345,22 +333,32 @@ export class ServerManagerView { } // Open last active tab - let lastActiveTab = ConfigUtil.getConfigItem("lastActiveTab", 0); - if (lastActiveTab >= servers.length) { - lastActiveTab = 0; + const firstTab = this.getTabByOrder(this.tabs, 0)!; + let lastActiveTabId = ConfigUtil.getConfigItem( + "lastActiveTabId", + firstTab.properties.tabId, + )!; + const lastActiveTab = this.getTabById(lastActiveTabId); + // Set lastActiveTab to firstTab if lastActiveTab is undefined. + // It will be undefined if user disconnected the server for lastActiveTab. + if ( + lastActiveTab === undefined || + lastActiveTab.properties.order >= servers.length + ) { + lastActiveTabId = firstTab.properties.tabId; } - // `webview.load()` for lastActiveTab before the others - await this.activateTab(lastActiveTab); + // `webview.load()` for lastActiveTabId before the others + await this.activateTab(lastActiveTabId); await Promise.all( - servers.map(async (server, i) => { - // After the lastActiveTab is activated, we load the others in the background + servers.map(async (server) => { + // After the lastActiveTabId is activated, we load the others in the background // without activating them, to prevent flashing of server icons - if (i === lastActiveTab) { + if (server.id === lastActiveTabId) { return; } - const tab = this.tabs[i]; + const tab = this.getTabByServerId(server.id); if (tab instanceof ServerTab) (await tab.webview).load(); }), ); @@ -374,30 +372,30 @@ export class ServerManagerView { } } - initServer(server: ServerConfig, index: number): ServerTab { - const tabIndex = this.getTabIndex(); - const tab = new ServerTab({ + initServer(server: ServerConfig, order: number): ServerTab { + const tabId = this.generateTabId(); + const tab: ServerTab = new ServerTab({ role: "server", icon: DomainUtil.iconAsUrl(server.icon), label: server.alias, $root: this.$tabsContainer, - onClick: this.activateLastTab.bind(this, index), - index, - tabIndex, - onHover: this.onHover.bind(this, index), - onHoverOut: this.onHoverOut.bind(this, index), + onClick: this.activateLastTab.bind(this, tabId), + order, + tabId, + serverId: server.id, + onHover: this.onHover.bind(this, tabId), + onHoverOut: this.onHoverOut.bind(this, tabId), webview: WebView.create({ $root: this.$webviewsContainer, rootWebContents, - index, - tabIndex, + tabId, url: server.url, role: "server", hasPermission: (origin: string, permission: string) => origin === server.url && permission === "notifications" && ConfigUtil.getConfigItem("showNotification", true), - isActive: () => index === this.activeTabIndex, + isActive: (): boolean => tabId === this.activeTabId, switchLoading: async (loading: boolean, url: string) => { if (loading) { this.loading.add(url); @@ -405,14 +403,14 @@ export class ServerManagerView { this.loading.delete(url); } - const tab = this.tabs[this.activeTabIndex]; + const tab = this.getTabById(this.activeTabId!); this.showLoading( tab instanceof ServerTab && this.loading.has((await tab.webview).properties.url), ); }, - onNetworkError: async (index: number) => { - await this.openNetworkTroubleshooting(index); + onNetworkError: async (tabId: string) => { + await this.openNetworkTroubleshooting(tabId); }, onTitleChange: this.updateBadge.bind(this), preload: url.pathToFileURL(path.join(bundlePath, "preload.cjs")).href, @@ -433,14 +431,14 @@ export class ServerManagerView { initServerActions(): void { const $serverImgs: NodeListOf = document.querySelectorAll(".server-icons"); - for (const [index, $serverImg] of $serverImgs.entries()) { - this.addContextMenu($serverImg, index); + for (const $serverImg of $serverImgs) { + this.addContextMenu($serverImg); if ($serverImg.src === defaultIcon) { - this.displayInitialCharLogo($serverImg, index); + this.displayInitialCharLogo($serverImg); } $serverImg.addEventListener("error", () => { - this.displayInitialCharLogo($serverImg, index); + this.displayInitialCharLogo($serverImg); }); } } @@ -456,7 +454,7 @@ export class ServerManagerView { ); }); this.$reloadButton.addEventListener("click", async () => { - const tab = this.tabs[this.activeTabIndex]; + const tab = this.getTabById(this.activeTabId!); if (tab instanceof ServerTab) (await tab.webview).reload(); }); this.$addServerButton.addEventListener("click", async () => { @@ -466,7 +464,7 @@ export class ServerManagerView { await this.openSettings("General"); }); this.$backButton.addEventListener("click", async () => { - const tab = this.tabs[this.activeTabIndex]; + const tab = this.getTabById(this.activeTabId!); if (tab instanceof ServerTab) (await tab.webview).back(); }); @@ -483,22 +481,16 @@ export class ServerManagerView { this.toggleDndButton(dnd); } - getTabIndex(): number { - const currentIndex = this.tabIndex; - this.tabIndex++; - return currentIndex; + generateTabId(): string { + return DomainUtil.generateDomainId(); } async getCurrentActiveServer(): Promise { - const tab = this.tabs[this.activeTabIndex]; + const tab = this.getTabById(this.activeTabId!); return tab instanceof ServerTab ? (await tab.webview).properties.url : ""; } - displayInitialCharLogo($img: HTMLImageElement, index: number): void { - // The index parameter is needed because webview[data-tab-id] can - // increment beyond the size of the sidebar org array and throw an - // error - + displayInitialCharLogo($img: HTMLImageElement): void { const $altIcon = document.createElement("div"); const $parent = $img.parentElement!; const $container = $parent.parentElement!; @@ -519,7 +511,7 @@ export class ServerManagerView { $img.remove(); $parent.append($altIcon); - this.addContextMenu($altIcon, index); + this.addContextMenu($altIcon); } sidebarHoverEvent( @@ -543,20 +535,18 @@ export class ServerManagerView { }); } - onHover(index: number): void { - // `this.$serverIconTooltip[index].textContent` already has realm name, so we are just - // removing the style. - this.$serverIconTooltip[index].removeAttribute("style"); + onHover(tabId: string): void { + const tooltip = this.getServerTooltip(tabId)!; + tooltip.removeAttribute("style"); // To handle position of servers' tooltip due to scrolling of list of organizations // This could not be handled using CSS, hence the top of the tooltip is made same // as that of its parent element. - const {top} = - this.$serverIconTooltip[index].parentElement!.getBoundingClientRect(); - this.$serverIconTooltip[index].style.top = `${top}px`; + const {top} = tooltip.parentElement!.getBoundingClientRect(); + tooltip.style.top = `${top}px`; } - onHoverOut(index: number): void { - this.$serverIconTooltip[index].style.display = "none"; + onHoverOut(tabId: string): void { + this.getServerTooltip(tabId)!.style.display = "none"; } async openFunctionalTab(tabProperties: { @@ -571,10 +561,9 @@ export class ServerManagerView { return; } - const index = this.tabs.length; - this.functionalTabs.set(tabProperties.page, index); - - const tabIndex = this.getTabIndex(); + const order = this.tabs.length; + const tabId = this.generateTabId(); + this.functionalTabs.set(tabProperties.page, tabId); const $view = await tabProperties.makeView(); this.$webviewsContainer.append($view); @@ -585,11 +574,11 @@ export class ServerManagerView { label: tabProperties.label, page: tabProperties.page, $root: this.$tabsContainer, - index, - tabIndex, - onClick: this.activateTab.bind(this, index), + order, + tabId, + onClick: this.activateTab.bind(this, tabId), onDestroy: async () => { - await this.destroyFunctionalTab(tabProperties.page, index); + await this.destroyFunctionalTab(tabProperties.page, tabId); tabProperties.destroyView(); }, $view, @@ -600,7 +589,7 @@ export class ServerManagerView { // closed when the functional tab DOM is ready, handled in webview.js this.$webviewsContainer.classList.remove("loaded"); - await this.activateTab(this.functionalTabs.get(tabProperties.page)!); + await this.activateTab(tabId); } async openSettings( @@ -641,8 +630,8 @@ export class ServerManagerView { }); } - async openNetworkTroubleshooting(index: number): Promise { - const tab = this.tabs[index]; + async openNetworkTroubleshooting(id: string): Promise { + const tab = this.getTabById(id); if (!(tab instanceof ServerTab)) return; const webview = await tab.webview; const reconnectUtil = new ReconnectUtil(webview); @@ -652,11 +641,11 @@ export class ServerManagerView { .loadURL(new URL("app/renderer/network.html", bundleUrl).href); } - async activateLastTab(index: number): Promise { - // Open all the tabs in background, also activate the tab based on the index - await this.activateTab(index); + async activateLastTab(tabId: string): Promise { + // Open all the tabs in background, also activate the tab based on id. + await this.activateTab(tabId); // Save last active tab via main process to avoid JSON DB errors - ipcRenderer.send("save-last-tab", index); + ipcRenderer.send("save-last-tab", tabId); } // Returns this.tabs in an way that does @@ -668,31 +657,42 @@ export class ServerManagerView { role: tab.properties.role, page: tab.properties.page, label: tab.properties.label, - index: tab.properties.index, + order: tab.properties.order, + id: tab.properties.tabId, + serverId: tab instanceof ServerTab ? tab.serverId : undefined, })); } - async activateTab(index: number, hideOldTab = true): Promise { - const tab = this.tabs[index]; + getTabByOrder( + tabs: ServerOrFunctionalTab[], + order: number, + ): ServerOrFunctionalTab | undefined { + return tabs.find((tab) => tab.properties.order === order); + } + + async activateTab(id: string, hideOldTab = true): Promise { + const tab = this.getTabById(id); if (!tab) { return; } - if (this.activeTabIndex !== -1) { - if (this.activeTabIndex === index) { + if (this.activeTabId) { + if (this.activeTabId === tab.properties.tabId) { return; } + const previousTab = this.getTabById(this.activeTabId)!; + if (hideOldTab) { // If old tab is functional tab Settings, remove focus from the settings icon at sidebar bottom if ( - this.tabs[this.activeTabIndex].properties.role === "function" && - this.tabs[this.activeTabIndex].properties.page === "Settings" + previousTab.properties.role === "function" && + previousTab.properties.page === "Settings" ) { this.$settingsButton.classList.remove("active"); } - await this.tabs[this.activeTabIndex].deactivate(); + await previousTab.deactivate(); } } @@ -706,7 +706,7 @@ export class ServerManagerView { .classList.add("disable"); } - this.activeTabIndex = index; + this.activeTabId = tab.properties.tabId; await tab.activate(); this.showLoading( @@ -718,7 +718,7 @@ export class ServerManagerView { // JSON stringify this.tabs to avoid a crash // util.inspect is being used to handle circular references tabs: this.tabsForIpc, - activeTabIndex: this.activeTabIndex, + activeTabId: this.activeTabId, // Following flag controls whether a menu item should be enabled or not enableMenu: tab.properties.role === "server", }); @@ -729,20 +729,25 @@ export class ServerManagerView { this.$loadingIndicator.classList.toggle("hidden", !loading); } - async destroyFunctionalTab(page: TabPage, index: number): Promise { - const tab = this.tabs[index]; + async destroyFunctionalTab(page: TabPage, tabId: string): Promise { + const tab = this.getTabById(tabId)!; + if (tab instanceof ServerTab && (await tab.webview).loading) { return; } + this.tabs = this.tabs.filter( + (tabObject) => tabObject.properties.tabId !== tabId, + ); await tab.destroy(); - - delete this.tabs[index]; // eslint-disable-line @typescript-eslint/no-array-delete this.functionalTabs.delete(page); // Issue #188: If the functional tab was not focused, do not activate another tab. - if (this.activeTabIndex === index) { - await this.activateTab(0, false); + if (this.activeTabId === tabId) { + await this.activateTab( + this.getTabByOrder(this.tabs, 0)!.properties.tabId, + false, + ); } } @@ -751,7 +756,7 @@ export class ServerManagerView { this.$webviewsContainer.classList.remove("loaded"); // Clear global variables - this.activeTabIndex = -1; + this.activeTabId = undefined; this.tabs = []; this.functionalTabs.clear(); @@ -761,9 +766,8 @@ export class ServerManagerView { } async reloadView(): Promise { - // Save and remember the index of last active tab so that we can use it later - const lastActiveTab = this.tabs[this.activeTabIndex].properties.index; - ConfigUtil.setConfigItem("lastActiveTab", lastActiveTab); + // Save and remember the id of last active tab so that we can use it later + ConfigUtil.setConfigItem("lastActiveTabId", this.activeTabId); // Destroy the current view and re-initiate it this.destroyView(); @@ -811,17 +815,42 @@ export class ServerManagerView { } } - async isLoggedIn(tabIndex: number): Promise { - const tab = this.tabs[tabIndex]; + async isLoggedIn(tabId: string): Promise { + const tab = this.getTabById(tabId); if (!(tab instanceof ServerTab)) return false; const webview = await tab.webview; const url = webview.getWebContents().getURL(); return !(url.endsWith("/login/") || webview.loading); } - addContextMenu($serverImg: HTMLElement, index: number): void { + getTabByServerId(serverId: string): ServerTab | undefined { + return this.tabs.find( + (tab): tab is ServerTab => + tab instanceof ServerTab && tab.serverId === serverId, + ); + } + + getTabById(tabId: string): ServerOrFunctionalTab | undefined { + return this.tabs.find((tab) => tab.properties.tabId === tabId); + } + + getServerTooltip(tabId: string): HTMLElement | undefined { + const tooltipElement = this.$tabsContainer.querySelector( + `.tab[data-tab-id="${CSS.escape(tabId)}"] .server-tooltip`, + ); + return tooltipElement instanceof HTMLElement ? tooltipElement : undefined; + } + + addContextMenu($serverImg: HTMLElement): void { $serverImg.addEventListener("contextmenu", async (event) => { event.preventDefault(); + const tabElement = $serverImg.closest(".tab"); + const tabId = + tabElement instanceof HTMLElement + ? tabElement.dataset.tabId + : undefined; + if (tabId === undefined) return; + const template = [ { label: t.__("Disconnect organization"), @@ -835,11 +864,11 @@ export class ServerManagerView { ), }); if (response === 0) { - if (DomainUtil.removeDomain(index)) { + if (DomainUtil.removeDomainById(tabId)) { ipcRenderer.send("reload-full-app"); } else { const {title, content} = Messages.orgRemovalError( - DomainUtil.getDomain(index).url, + DomainUtil.getDomainById(tabId)!.url, ); dialog.showErrorBox(title, content); } @@ -848,11 +877,11 @@ export class ServerManagerView { }, { label: t.__("Notification settings"), - enabled: await this.isLoggedIn(index), + enabled: await this.isLoggedIn(tabId), click: async () => { // Switch to tab whose icon was right-clicked - await this.activateTab(index); - const tab = this.tabs[index]; + const tab = this.getTabById(tabId); + await this.activateTab(tabId); if (tab instanceof ServerTab) (await tab.webview).showNotificationSettings(); }, @@ -860,7 +889,7 @@ export class ServerManagerView { { label: t.__("Copy Zulip URL"), click() { - clipboard.writeText(DomainUtil.getDomain(index).url); + clipboard.writeText(DomainUtil.getDomainById(tabId)!.url); }, }, ]; @@ -937,7 +966,7 @@ export class ServerManagerView { for (const [channel, listener] of webviewListeners) { ipcRenderer.on(channel, async () => { - const tab = this.tabs[this.activeTabIndex]; + const tab = this.getTabById(this.activeTabId!); if (tab instanceof ServerTab) { const activeWebview = await tab.webview; if (activeWebview) listener(activeWebview); @@ -1005,8 +1034,9 @@ export class ServerManagerView { ipcRenderer.send("reload-full-app"); }); - ipcRenderer.on("switch-server-tab", async (event, index: number) => { - await this.activateLastTab(index); + ipcRenderer.on("switch-server-tab", async (event, serverId: string) => { + const tab = this.getTabByServerId(serverId)!; + await this.activateLastTab(tab.properties.tabId); }); ipcRenderer.on("open-org-tab", async () => { @@ -1044,7 +1074,7 @@ export class ServerManagerView { if (updateMenu) { ipcRenderer.send("update-menu", { tabs: this.tabsForIpc, - activeTabIndex: this.activeTabIndex, + activeTabId: this.activeTabId, }); } }, @@ -1065,16 +1095,16 @@ export class ServerManagerView { ipcRenderer.on( "update-realm-name", (event, serverURL: string, realmName: string) => { - for (const [index, domain] of DomainUtil.getDomains().entries()) { + for (const domain of DomainUtil.getDomains()) { if (domain.url === serverURL) { - const tab = this.tabs[index]; + const tab = this.getTabById(domain.id); if (tab instanceof ServerTab) tab.setLabel(realmName); domain.alias = realmName; - DomainUtil.updateDomain(index, domain); + DomainUtil.updateDomainById(domain.id, domain); // Update the realm name also on the Window menu ipcRenderer.send("update-menu", { tabs: this.tabsForIpc, - activeTabIndex: this.activeTabIndex, + activeTabId: this.activeTabId, }); } } @@ -1085,14 +1115,14 @@ export class ServerManagerView { "update-realm-icon", async (event, serverURL: string, iconURL: string) => { await Promise.all( - DomainUtil.getDomains().map(async (domain, index) => { + DomainUtil.getDomains().map(async (domain) => { if (domain.url === serverURL) { const localIconPath = await DomainUtil.saveServerIcon(iconURL); - const tab = this.tabs[index]; + const tab = this.getTabById(domain.id); if (tab instanceof ServerTab) tab.setIcon(DomainUtil.iconAsUrl(localIconPath)); domain.icon = localIconPath; - DomainUtil.updateDomain(index, domain); + DomainUtil.updateDomainById(domain.id, domain); } }), ); @@ -1116,7 +1146,7 @@ export class ServerManagerView { (await tab.webview).webContentsId === webviewId ) { const concurrentTab: HTMLButtonElement = document.querySelector( - `div[data-tab-id="${CSS.escape(`${tab.properties.tabIndex}`)}"]`, + `div[data-tab-id="${CSS.escape(`${tab.properties.tabId}`)}"]`, )!; concurrentTab.click(); } diff --git a/app/renderer/js/pages/preference/connected-org-section.ts b/app/renderer/js/pages/preference/connected-org-section.ts index a6dd78038..6c586b4a9 100644 --- a/app/renderer/js/pages/preference/connected-org-section.ts +++ b/app/renderer/js/pages/preference/connected-org-section.ts @@ -48,11 +48,11 @@ export function initConnectedOrgSection({ // Show noServerText if no servers are there otherwise hide it $existingServers.textContent = servers.length === 0 ? noServerText : ""; - for (const [i, server] of servers.entries()) { + for (const server of servers) { initServerInfoForm({ $root: $serverInfoContainer, server, - index: i, + serverId: server.id, onChange: reloadApp, }); } diff --git a/app/renderer/js/pages/preference/server-info-form.ts b/app/renderer/js/pages/preference/server-info-form.ts index 8e440e18c..950d7d1e4 100644 --- a/app/renderer/js/pages/preference/server-info-form.ts +++ b/app/renderer/js/pages/preference/server-info-form.ts @@ -11,7 +11,7 @@ import * as DomainUtil from "../../utils/domain-util.ts"; type ServerInfoFormProperties = { $root: Element; server: ServerConfig; - index: number; + serverId: string; onChange: () => void; }; @@ -58,26 +58,39 @@ export function initServerInfoForm(properties: ServerInfoFormProperties): void { message: t.__("Are you sure you want to disconnect this organization?"), }); if (response === 0) { - if (DomainUtil.removeDomain(properties.index)) { + if (DomainUtil.removeDomainById(properties.serverId)) { ipcRenderer.send("reload-full-app"); } else { - const {title, content} = Messages.orgRemovalError( - DomainUtil.getDomain(properties.index).url, - ); - dialog.showErrorBox(title, content); + const domain = DomainUtil.getDomainById(properties.serverId); + if (domain) { + const {title, content} = Messages.orgRemovalError(domain.url); + dialog.showErrorBox(title, content); + } } } }); $openServerButton.addEventListener("click", () => { - ipcRenderer.send("forward-message", "switch-server-tab", properties.index); + ipcRenderer.send( + "forward-message", + "switch-server-tab", + properties.serverId, + ); }); $serverInfoAlias.addEventListener("click", () => { - ipcRenderer.send("forward-message", "switch-server-tab", properties.index); + ipcRenderer.send( + "forward-message", + "switch-server-tab", + properties.serverId, + ); }); $serverIcon.addEventListener("click", () => { - ipcRenderer.send("forward-message", "switch-server-tab", properties.index); + ipcRenderer.send( + "forward-message", + "switch-server-tab", + properties.serverId, + ); }); } diff --git a/app/renderer/js/utils/domain-util.ts b/app/renderer/js/utils/domain-util.ts index 0aed18954..eb8831d97 100644 --- a/app/renderer/js/utils/domain-util.ts +++ b/app/renderer/js/utils/domain-util.ts @@ -1,3 +1,5 @@ +import assert from "node:assert"; +import {randomBytes} from "node:crypto"; import fs from "node:fs"; import path from "node:path"; @@ -11,7 +13,7 @@ import * as EnterpriseUtil from "../../../common/enterprise-util.ts"; import Logger from "../../../common/logger-util.ts"; import * as Messages from "../../../common/messages.ts"; import * as t from "../../../common/translation-util.ts"; -import type {ServerConfig} from "../../../common/types.ts"; +import type {ServerConfig, ServerSettings} from "../../../common/types.ts"; import defaultIcon from "../../img/icon.png"; import {ipcRenderer} from "../typed-ipc-renderer.ts"; @@ -19,11 +21,16 @@ const logger = new Logger({ file: "domain-util.log", }); +export function generateDomainId(): string { + return randomBytes(5).toString("hex"); +} + // For historical reasons, we store this string in domain.json to denote a // missing icon; it does not change with the actual icon location. export const defaultIconSentinel = "../renderer/img/icon.png"; -const serverConfigSchema = z.object({ +const storedServerSchema = z.object({ + id: z.string().optional(), url: z.url(), alias: z.string(), icon: z.string(), @@ -31,6 +38,18 @@ const serverConfigSchema = z.object({ zulipFeatureLevel: z.number().default(0), }); +const serverConfigSchema = storedServerSchema.extend({ + id: z.string(), +}); + +function addServerId(server: z.infer): ServerConfig { + assert.ok(server.id === undefined); + return serverConfigSchema.parse({ + ...server, + id: generateDomainId(), + }); +} + let database!: JsonDB; reloadDatabase(); @@ -63,16 +82,13 @@ export function getDomains(): ServerConfig[] { } } -export function getDomain(index: number): ServerConfig { - reloadDatabase(); - return serverConfigSchema.parse( - database.getObject(`/domains[${index}]`), - ); +export function getDomainById(id: string): ServerConfig | undefined { + return getDomains().find((server) => server.id === id); } -export function updateDomain(index: number, server: ServerConfig): void { - reloadDatabase(); - serverConfigSchema.parse(server); +export function updateDomainById(id: string, server: ServerConfig): void { + const index = getDomains().findIndex((domain) => domain.id === id); + assert.ok(index !== -1, `Domain with id ${id} not found`); database.push(`/domains[${index}]`, server, true); } @@ -84,15 +100,13 @@ export async function addDomain(server: { if (server.icon) { const localIconUrl = await saveServerIcon(server.icon); server.icon = localIconUrl; - serverConfigSchema.parse(server); - database.push("/domains[]", server, true); - reloadDatabase(); } else { server.icon = defaultIconSentinel; - serverConfigSchema.parse(server); - database.push("/domains[]", server, true); - reloadDatabase(); } + + const serverWithId = addServerId(storedServerSchema.parse(server)); + database.push("/domains[]", serverWithId, true); + reloadDatabase(); } export function removeDomains(): void { @@ -100,8 +114,13 @@ export function removeDomains(): void { reloadDatabase(); } -export function removeDomain(index: number): boolean { - if (EnterpriseUtil.isPresetOrg(getDomain(index).url)) { +export function removeDomainById(id: string): boolean { + const index = getDomains().findIndex((domain) => domain.id === id); + if (index === -1) { + return false; + } + + if (EnterpriseUtil.isPresetOrg(getDomainById(id)!.url)) { return false; } @@ -119,7 +138,7 @@ export function duplicateDomain(domain: string): boolean { export async function checkDomain( domain: string, silent = false, -): Promise { +): Promise { if (!silent && duplicateDomain(domain)) { // Do not check duplicate in silent mode throw new Error("This server has been added."); @@ -128,13 +147,13 @@ export async function checkDomain( domain = formatUrl(domain); try { - return await getServerSettings(domain); + return storedServerSchema.parse(await getServerSettings(domain)); } catch { throw new Error(Messages.invalidZulipServerError(domain)); } } -async function getServerSettings(domain: string): Promise { +async function getServerSettings(domain: string): Promise { return ipcRenderer.invoke("get-server-settings", domain); } @@ -147,17 +166,21 @@ export async function saveServerIcon(iconURL: string): Promise { export async function updateSavedServer( url: string, - index: number, + id: string, ): Promise { // Does not promise successful update - const serverConfig = getDomain(index); + const serverConfig = getDomainById(id)!; const oldIcon = serverConfig.icon; try { - const newServerConfig = await checkDomain(url, true); + const newServerSetting = await checkDomain(url, true); + const newServerConfig: ServerConfig = { + ...newServerSetting, + id: serverConfig.id, + }; const localIconUrl = await saveServerIcon(newServerConfig.icon); if (!oldIcon || localIconUrl !== defaultIconSentinel) { newServerConfig.icon = localIconUrl; - updateDomain(index, newServerConfig); + updateDomainById(id, newServerConfig); reloadDatabase(); } @@ -170,6 +193,31 @@ export async function updateSavedServer( } } +function ensureDomainIds(): void { + try { + const domains = storedServerSchema + .array() + .parse(database.getObject("/domains")); + + let changed = false; + + const updatedDomains = domains.map((server) => { + if (server.id === undefined) { + changed = true; + server = addServerId(server); + } + + return server; + }); + + if (changed) { + database.push("/domains", updatedDomains, true); + } + } catch (error: unknown) { + if (!(error instanceof DataError)) throw error; + } +} + function reloadDatabase(): void { const domainJsonPath = path.join( app.getPath("userData"), @@ -194,6 +242,7 @@ function reloadDatabase(): void { } database = new JsonDB(domainJsonPath, true, true); + ensureDomainIds(); } export function formatUrl(domain: string): string {