Skip to content

Commit 9e7b718

Browse files
shubham-padiashubham-padia
committed
tab: Use activeTab instead of activeTabIndex.
Tracking these indices separately than the tab object means that we will have low confidence trying to control the order by changing the indices. Using the Tab object instead as a reference is the right way to go about this.
1 parent 1f2739f commit 9e7b718

File tree

4 files changed

+68
-53
lines changed

4 files changed

+68
-53
lines changed

‎app/common/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
export type MenuProperties = {
22
tabs: TabData[];
3-
activeTabIndex?: number;
3+
activeTab?: TabData;
44
enableMenu?: boolean;
55
};
66

‎app/main/index.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -428,9 +428,8 @@ function createMainWindow(): BrowserWindow {
428428

429429
ipcMain.on("update-menu", (_event, properties: MenuProperties) => {
430430
AppMenu.setMenu(properties);
431-
if (properties.activeTabIndex !== undefined) {
432-
const activeTab = properties.tabs[properties.activeTabIndex];
433-
mainWindow.setTitle(`Zulip - ${activeTab.label}`);
431+
if (properties.activeTab !== undefined) {
432+
mainWindow.setTitle(`Zulip - ${properties.activeTab.label}`);
434433
}
435434
});
436435

‎app/main/menu.ts‎

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ function getHelpSubmenu(): MenuItemConstructorOptions[] {
294294

295295
function getWindowSubmenu(
296296
tabs: TabData[],
297-
activeTabIndex?: number,
297+
activeTab?: TabData,
298298
): MenuItemConstructorOptions[] {
299299
const initialSubmenu: MenuItemConstructorOptions[] = [
300300
{
@@ -326,7 +326,7 @@ function getWindowSubmenu(
326326
label: tab.label,
327327
accelerator:
328328
tab.role === "function" ? "" : `${shortcutKey} + ${tab.index + 1}`,
329-
checked: tab.index === activeTabIndex,
329+
checked: tab === activeTab,
330330
click(_item, focusedWindow) {
331331
if (focusedWindow) {
332332
sendAction("switch-server-tab", tab.index);
@@ -346,10 +346,7 @@ function getWindowSubmenu(
346346
enabled: tabs.length > 1,
347347
click(_item, focusedWindow) {
348348
if (focusedWindow) {
349-
sendAction(
350-
"switch-server-tab",
351-
getNextServer(tabs, activeTabIndex!),
352-
);
349+
sendAction("switch-server-tab", getNextServer(tabs, activeTab!));
353350
}
354351
},
355352
},
@@ -361,7 +358,7 @@ function getWindowSubmenu(
361358
if (focusedWindow) {
362359
sendAction(
363360
"switch-server-tab",
364-
getPreviousServer(tabs, activeTabIndex!),
361+
getPreviousServer(tabs, activeTab!),
365362
);
366363
}
367364
},
@@ -375,7 +372,7 @@ function getWindowSubmenu(
375372
function getDarwinTpl(
376373
properties: MenuProperties,
377374
): MenuItemConstructorOptions[] {
378-
const {tabs, activeTabIndex, enableMenu = false} = properties;
375+
const {tabs, activeTab, enableMenu = false} = properties;
379376

380377
return [
381378
{
@@ -525,7 +522,7 @@ function getDarwinTpl(
525522
},
526523
{
527524
label: t.__("Window"),
528-
submenu: getWindowSubmenu(tabs, activeTabIndex),
525+
submenu: getWindowSubmenu(tabs, activeTab),
529526
},
530527
{
531528
label: t.__("Tools"),
@@ -540,7 +537,7 @@ function getDarwinTpl(
540537
}
541538

542539
function getOtherTpl(properties: MenuProperties): MenuItemConstructorOptions[] {
543-
const {tabs, activeTabIndex, enableMenu = false} = properties;
540+
const {tabs, activeTab, enableMenu = false} = properties;
544541
return [
545542
{
546543
label: t.__("File"),
@@ -673,7 +670,7 @@ function getOtherTpl(properties: MenuProperties): MenuItemConstructorOptions[] {
673670
},
674671
{
675672
label: t.__("Window"),
676-
submenu: getWindowSubmenu(tabs, activeTabIndex),
673+
submenu: getWindowSubmenu(tabs, activeTab),
677674
},
678675
{
679676
label: t.__("Tools"),
@@ -704,20 +701,22 @@ async function checkForUpdate(): Promise<void> {
704701
await appUpdater(true);
705702
}
706703

707-
function getNextServer(tabs: TabData[], activeTabIndex: number): number {
704+
function getNextServer(tabs: TabData[], activeTab: TabData): number {
705+
let {index} = activeTab;
708706
do {
709-
activeTabIndex = (activeTabIndex + 1) % tabs.length;
710-
} while (tabs[activeTabIndex]?.role !== "server");
707+
index = (index + 1) % tabs.length;
708+
} while (tabs[index]?.role !== "server");
711709

712-
return activeTabIndex;
710+
return index;
713711
}
714712

715-
function getPreviousServer(tabs: TabData[], activeTabIndex: number): number {
713+
function getPreviousServer(tabs: TabData[], activeTab: TabData): number {
714+
let {index} = activeTab;
716715
do {
717-
activeTabIndex = (activeTabIndex - 1 + tabs.length) % tabs.length;
718-
} while (tabs[activeTabIndex]?.role !== "server");
716+
index = (index - 1 + tabs.length) % tabs.length;
717+
} while (tabs[index]?.role !== "server");
719718

720-
return activeTabIndex;
719+
return index;
721720
}
722721

723722
export function setMenu(properties: MenuProperties): void {

‎app/renderer/js/main.ts‎

Lines changed: 47 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ export class ServerManagerView {
8383
$sidebar: Element;
8484
$fullscreenPopup: Element;
8585
loading: Set<string>;
86-
activeTabIndex: number;
86+
activeTab?: ServerOrFunctionalTab;
8787
tabs: ServerOrFunctionalTab[];
8888
functionalTabs: Map<TabPage, number>;
8989
tabIndex: number;
@@ -128,7 +128,7 @@ export class ServerManagerView {
128128
);
129129

130130
this.loading = new Set();
131-
this.activeTabIndex = -1;
131+
this.activeTab = undefined;
132132
this.tabs = [];
133133
this.presetOrgs = [];
134134
this.functionalTabs = new Map();
@@ -376,7 +376,7 @@ export class ServerManagerView {
376376

377377
initServer(server: ServerConfig, index: number): ServerTab {
378378
const tabIndex = this.getTabIndex();
379-
const tab = new ServerTab({
379+
const tab: ServerTab = new ServerTab({
380380
role: "server",
381381
icon: DomainUtil.iconAsUrl(server.icon),
382382
label: server.alias,
@@ -397,18 +397,18 @@ export class ServerManagerView {
397397
origin === server.url &&
398398
permission === "notifications" &&
399399
ConfigUtil.getConfigItem("showNotification", true),
400-
isActive: () => index === this.activeTabIndex,
400+
isActive: (): boolean => this.tabs[index] === this.activeTab,
401401
switchLoading: async (loading: boolean, url: string) => {
402402
if (loading) {
403403
this.loading.add(url);
404404
} else {
405405
this.loading.delete(url);
406406
}
407407

408-
const tab = this.tabs[this.activeTabIndex];
408+
const {activeTab} = this;
409409
this.showLoading(
410-
tab instanceof ServerTab &&
411-
this.loading.has((await tab.webview).properties.url),
410+
activeTab instanceof ServerTab &&
411+
this.loading.has((await activeTab.webview).properties.url),
412412
);
413413
},
414414
onNetworkError: async (index: number) => {
@@ -456,7 +456,7 @@ export class ServerManagerView {
456456
);
457457
});
458458
this.$reloadButton.addEventListener("click", async () => {
459-
const tab = this.tabs[this.activeTabIndex];
459+
const tab = this.activeTab;
460460
if (tab instanceof ServerTab) (await tab.webview).reload();
461461
});
462462
this.$addServerButton.addEventListener("click", async () => {
@@ -466,7 +466,7 @@ export class ServerManagerView {
466466
await this.openSettings("General");
467467
});
468468
this.$backButton.addEventListener("click", async () => {
469-
const tab = this.tabs[this.activeTabIndex];
469+
const tab = this.activeTab;
470470
if (tab instanceof ServerTab) (await tab.webview).back();
471471
});
472472

@@ -490,7 +490,7 @@ export class ServerManagerView {
490490
}
491491

492492
async getCurrentActiveServer(): Promise<string> {
493-
const tab = this.tabs[this.activeTabIndex];
493+
const tab = this.activeTab;
494494
return tab instanceof ServerTab ? (await tab.webview).properties.url : "";
495495
}
496496

@@ -659,17 +659,20 @@ export class ServerManagerView {
659659
ipcRenderer.send("save-last-tab", index);
660660
}
661661

662-
// Returns this.tabs in an way that does
663-
// not crash app when this.tabs is passed into
664-
// ipcRenderer. Something about webview, and properties.webview
662+
// Returns tab in an way that does not crash app when it is passed
663+
// into ipcRenderer. Something about webview, and properties.webview
665664
// properties in ServerTab causes the app to crash.
666-
get tabsForIpc(): TabData[] {
667-
return this.tabs.map((tab) => ({
665+
tabForIpc(tab: ServerOrFunctionalTab): TabData {
666+
return {
668667
role: tab.properties.role,
669668
page: tab.properties.page,
670669
label: tab.properties.label,
671670
index: tab.properties.index,
672-
}));
671+
};
672+
}
673+
674+
get tabsForIpc(): TabData[] {
675+
return this.tabs.map((tab) => this.tabForIpc(tab));
673676
}
674677

675678
async activateTab(index: number, hideOldTab = true): Promise<void> {
@@ -678,21 +681,22 @@ export class ServerManagerView {
678681
return;
679682
}
680683

681-
if (this.activeTabIndex !== -1) {
682-
if (this.activeTabIndex === index) {
684+
const previousTab = this.activeTab;
685+
if (previousTab) {
686+
if (previousTab === tab) {
683687
return;
684688
}
685689

686690
if (hideOldTab) {
687691
// If old tab is functional tab Settings, remove focus from the settings icon at sidebar bottom
688692
if (
689-
this.tabs[this.activeTabIndex].properties.role === "function" &&
690-
this.tabs[this.activeTabIndex].properties.page === "Settings"
693+
previousTab.properties.role === "function" &&
694+
previousTab.properties.page === "Settings"
691695
) {
692696
this.$settingsButton.classList.remove("active");
693697
}
694698

695-
await this.tabs[this.activeTabIndex].deactivate();
699+
await previousTab.deactivate();
696700
}
697701
}
698702

@@ -706,7 +710,7 @@ export class ServerManagerView {
706710
.classList.add("disable");
707711
}
708712

709-
this.activeTabIndex = index;
713+
this.activeTab = tab;
710714
await tab.activate();
711715

712716
this.showLoading(
@@ -718,7 +722,7 @@ export class ServerManagerView {
718722
// JSON stringify this.tabs to avoid a crash
719723
// util.inspect is being used to handle circular references
720724
tabs: this.tabsForIpc,
721-
activeTabIndex: this.activeTabIndex,
725+
activeTab: this.tabForIpc(this.activeTab),
722726
// Following flag controls whether a menu item should be enabled or not
723727
enableMenu: tab.properties.role === "server",
724728
});
@@ -731,17 +735,24 @@ export class ServerManagerView {
731735

732736
async destroyFunctionalTab(page: TabPage, index: number): Promise<void> {
733737
const tab = this.tabs[index];
738+
if (!tab) {
739+
return;
740+
}
741+
734742
if (tab instanceof ServerTab && (await tab.webview).loading) {
735743
return;
736744
}
737745

746+
const wasActive = tab === this.activeTab;
747+
738748
await tab.destroy();
739749

740750
delete this.tabs[index]; // eslint-disable-line @typescript-eslint/no-array-delete
741751
this.functionalTabs.delete(page);
742752

743753
// Issue #188: If the functional tab was not focused, do not activate another tab.
744-
if (this.activeTabIndex === index) {
754+
if (wasActive) {
755+
this.activeTab = undefined;
745756
await this.activateTab(0, false);
746757
}
747758
}
@@ -751,7 +762,7 @@ export class ServerManagerView {
751762
this.$webviewsContainer.classList.remove("loaded");
752763

753764
// Clear global variables
754-
this.activeTabIndex = -1;
765+
this.activeTab = undefined;
755766
this.tabs = [];
756767
this.functionalTabs.clear();
757768

@@ -762,8 +773,10 @@ export class ServerManagerView {
762773

763774
async reloadView(): Promise<void> {
764775
// Save and remember the index of last active tab so that we can use it later
765-
const lastActiveTab = this.tabs[this.activeTabIndex].properties.index;
766-
ConfigUtil.setConfigItem("lastActiveTab", lastActiveTab);
776+
const {activeTab} = this;
777+
if (activeTab !== undefined) {
778+
ConfigUtil.setConfigItem("lastActiveTab", activeTab.properties.index);
779+
}
767780

768781
// Destroy the current view and re-initiate it
769782
this.destroyView();
@@ -937,7 +950,7 @@ export class ServerManagerView {
937950

938951
for (const [channel, listener] of webviewListeners) {
939952
ipcRenderer.on(channel, async () => {
940-
const tab = this.tabs[this.activeTabIndex];
953+
const tab = this.activeTab;
941954
if (tab instanceof ServerTab) {
942955
const activeWebview = await tab.webview;
943956
if (activeWebview) listener(activeWebview);
@@ -1044,7 +1057,9 @@ export class ServerManagerView {
10441057
if (updateMenu) {
10451058
ipcRenderer.send("update-menu", {
10461059
tabs: this.tabsForIpc,
1047-
activeTabIndex: this.activeTabIndex,
1060+
activeTab: this.activeTab
1061+
? this.tabForIpc(this.activeTab)
1062+
: undefined,
10481063
});
10491064
}
10501065
},
@@ -1074,7 +1089,9 @@ export class ServerManagerView {
10741089
// Update the realm name also on the Window menu
10751090
ipcRenderer.send("update-menu", {
10761091
tabs: this.tabsForIpc,
1077-
activeTabIndex: this.activeTabIndex,
1092+
activeTab: this.activeTab
1093+
? this.tabForIpc(this.activeTab)
1094+
: undefined,
10781095
});
10791096
}
10801097
}

0 commit comments

Comments
 (0)