fix(client): strange behaviour when navigating between tabs (closes #1615)

e2e/layout/tab_bar.spec.ts

@@ -100,3 +100,20 @@ test("Empty tabs are cleared out", async ({ page, context }) => {
     // Expect no empty tabs.
     expect(await app.tabBar.locator(".note-tab-wrapper").count()).toBe(1);
 });
+
+test("Search works when dismissing a tab", async ({ page, context }) => {
+    const app = new App(page, context);
+    await app.goto();
+
+    await app.goToNoteInNewTab("Table of contents");
+    await app.openAndClickNoteActionMenu("Search in note");
+    await expect(app.findAndReplaceWidget).toBeVisible();
+    app.findAndReplaceWidget.locator(".find-widget-close-button").click();
+
+    await app.addNewTab();
+    await app.goToNoteInNewTab("Sample mindmap");
+
+    await app.getTab(0).click();
+    await app.openAndClickNoteActionMenu("Search in note");
+    await expect(app.findAndReplaceWidget).toBeVisible();
+});

e2e/support/app.ts

@@ -106,6 +106,30 @@ export default class App {
         await this.noteTree.getByText(title).click();
     }
 
+    /**
+     * Opens the note context menu by clicking on it, looks for the item with the given text and clicks it.
+     *
+     * Assertions are put in place to make sure the menu is open and closed after the click.
+     * @param itemToFind the text of the item to find in the menu.
+     */
+    async openAndClickNoteActionMenu(itemToFind: string) {
+        const noteActionsButton = this.currentNoteSplit.locator(".note-actions");
+        await noteActionsButton.click();
+
+        const dropdownMenu = noteActionsButton.locator(".dropdown-menu");
+        await this.page.waitForTimeout(100);
+        await expect(dropdownMenu).toBeVisible();
+        dropdownMenu.getByText(itemToFind).click();
+        await expect(dropdownMenu).not.toBeVisible();
+    }
+
+    /**
+     * Obtains the locator to the find and replace widget, if it's being displayed.
+     */
+    get findAndReplaceWidget() {
+        return this.page.locator(".component.visible.find-replace-widget");
+    }
+
     /**
      * Executes any Trilium command on the client.
      * @param command the command to send.

src/public/app/components/app_context.ts

@@ -66,6 +66,11 @@ export interface ExecuteCommandData<T> extends CommandData {
     resolve: (data: T) => void;
 }
 
+export interface NoteSwitchedContext {
+    noteContext: NoteContext;
+    notePath: string | null;
+}
+
 /**
  * The keys represent the different commands that can be triggered via {@link AppContext#triggerCommand} (first argument), and the values represent the data or arguments definition of the given command. All data for commands must extend {@link CommandData}.
  */
@@ -122,7 +127,11 @@ export type CommandMappings = {
     hoistNote: CommandData & { noteId: string };
     leaveProtectedSession: CommandData;
     enterProtectedSession: CommandData;
-
+    noteContextReorder: CommandData & {
+        ntxIdsInOrder: string[];
+        oldMainNtxId?: string | null;
+        newMainNtxId?: string | null;
+    };
     openInTab: ContextMenuCommandData;
     openNoteInSplit: ContextMenuCommandData;
     toggleNoteHoisting: ContextMenuCommandData;
@@ -294,14 +303,8 @@ type EventMappings = {
     beforeNoteContextRemove: {
         ntxIds: string[];
     };
-    noteSwitched: {
+    noteSwitched: NoteSwitchedContext;
-        noteContext: NoteContext;
+    noteSwitchedAndActivated: NoteSwitchedContext;
-        notePath?: string | null;
-    };
-    noteSwitchedAndActivated: {
-        noteContext: NoteContext;
-        notePath: string;
-    };
     setNoteContext: {
         noteContext: NoteContext;
     };
@@ -326,8 +329,10 @@ type EventMappings = {
         ntxId: string | null;
     };
     contextsReopened: {
+        ntxId: string;
         mainNtxId: string | null;
         tabPosition: number;
+        afterNtxId?: string;
     };
     noteDetailRefreshed: {
         ntxId?: string | null;

src/public/app/widgets/basic_widget.ts

@@ -214,7 +214,7 @@ export class TypedBasicWidget<T extends TypedComponent<any>> extends TypedCompon
         return this.$widget.hasClass("hidden-int");
     }
 
-    toggleExt(show: boolean) {
+    toggleExt(show: boolean | null | "" | undefined) {
         this.$widget.toggleClass("hidden-ext", !show)
                     .toggleClass("visible", !!show);
     }

src/public/app/widgets/containers/split_note_container.ts

@@ -1,8 +1,7 @@
 import FlexContainer from "./flex_container.js";
-import appContext from "../../components/app_context.js";
+import appContext, { type CommandData, type CommandListenerData, type EventData, type EventNames, type NoteSwitchedContext } from "../../components/app_context.js";
-import NoteContext from "../../components/note_context.js";
-import type { CommandMappings, EventNames, EventData } from "../../components/app_context.js";
 import type BasicWidget from "../basic_widget.js";
+import type NoteContext from "../../components/note_context.js";
 
 interface NoteContextEvent {
     noteContext: NoteContext;
@@ -15,13 +14,10 @@ interface SplitNoteWidget extends BasicWidget {
 
 type WidgetFactory = () => SplitNoteWidget;
 
-interface Widgets {
-    [key: string]: SplitNoteWidget;
-}
-
 export default class SplitNoteContainer extends FlexContainer<SplitNoteWidget> {
+
     private widgetFactory: WidgetFactory;
-    private widgets: Widgets;
+    private widgets: Record<string, SplitNoteWidget>;
 
     constructor(widgetFactory: WidgetFactory) {
         super("row");
@@ -34,7 +30,7 @@ export default class SplitNoteContainer extends FlexContainer<SplitNoteWidget> {
         this.collapsible();
     }
 
-    async newNoteContextCreatedEvent({ noteContext }: NoteContextEvent) {
+    async newNoteContextCreatedEvent({ noteContext }: EventData<"newNoteContextCreated">) {
         const widget = this.widgetFactory();
 
         const $renderedWidget = widget.render();
@@ -57,16 +53,10 @@ export default class SplitNoteContainer extends FlexContainer<SplitNoteWidget> {
         this.child(widget);
     }
 
-    async openNewNoteSplitEvent({ ntxId, notePath, hoistedNoteId, viewScope }: {
+    async openNewNoteSplitEvent({ ntxId, notePath, hoistedNoteId, viewScope }: EventData<"openNewNoteSplit">) {
-        ntxId: string;
-        notePath?: string;
-        hoistedNoteId?: string;
-        viewScope?: any;
-    }) {
         const mainNtxId = appContext.tabManager.getActiveMainContext()?.ntxId;
-
         if (!mainNtxId) {
-            logError("empty mainNtxId!");
+            console.warn("Missing main note context ID");
             return;
         }
 
@@ -79,14 +69,18 @@ export default class SplitNoteContainer extends FlexContainer<SplitNoteWidget> {
         hoistedNoteId = hoistedNoteId || appContext.tabManager.getActiveContext()?.hoistedNoteId;
 
         const noteContext = await appContext.tabManager.openEmptyTab(null, hoistedNoteId, mainNtxId);
+        if (!noteContext.ntxId) {
+            logError("Failed to create new note context!");
+            return;
+        }
 
         // remove the original position of newly created note context
-        const ntxIds = appContext.tabManager.children.map((c) => c.ntxId).filter((id) => id !== noteContext.ntxId);
+        const ntxIds = appContext.tabManager.children.map((c) => c.ntxId).filter((id) => id !== noteContext.ntxId) as string[];
 
         // insert the note context after the originating note context
         ntxIds.splice(ntxIds.indexOf(ntxId) + 1, 0, noteContext.ntxId);
 
-        this.triggerCommand("noteContextReorder" as keyof CommandMappings, { ntxIdsInOrder: ntxIds });
+        this.triggerCommand("noteContextReorder", { ntxIdsInOrder: ntxIds });
 
         // move the note context rendered widget after the originating widget
         this.$widget.find(`[data-ntx-id="${noteContext.ntxId}"]`).insertAfter(this.$widget.find(`[data-ntx-id="${ntxId}"]`));
@@ -100,11 +94,13 @@ export default class SplitNoteContainer extends FlexContainer<SplitNoteWidget> {
         }
     }
 
-    closeThisNoteSplitCommand({ ntxId }: { ntxId: string }): void {
+    closeThisNoteSplitCommand({ ntxId }: CommandListenerData<"closeThisNoteSplit">) {
+        if (ntxId) {
             appContext.tabManager.removeNoteContext(ntxId);
         }
+    }
 
-    async moveThisNoteSplitCommand({ ntxId, isMovingLeft }: { ntxId: string; isMovingLeft: boolean }): Promise<void> {
+    async moveThisNoteSplitCommand({ ntxId, isMovingLeft }: CommandListenerData<"moveThisNoteSplit">) {
         if (!ntxId) {
             logError("empty ntxId!");
             return;
@@ -125,11 +121,11 @@ export default class SplitNoteContainer extends FlexContainer<SplitNoteWidget> {
             return;
         }
 
-        const ntxIds = contexts.map((c) => c.ntxId);
+        const ntxIds = contexts.map((c) => c.ntxId).filter((c) => !!c) as string[];
         const newNtxIds = [...ntxIds.slice(0, leftIndex), ntxIds[leftIndex + 1], ntxIds[leftIndex], ...ntxIds.slice(leftIndex + 2)];
         const isChangingMainContext = !contexts[leftIndex].mainNtxId;
 
-        this.triggerCommand("noteContextReorder" as keyof CommandMappings, {
+        this.triggerCommand("noteContextReorder", {
             ntxIdsInOrder: newNtxIds,
             oldMainNtxId: isChangingMainContext ? ntxIds[leftIndex] : null,
             newMainNtxId: isChangingMainContext ? ntxIds[leftIndex + 1] : null
@@ -142,16 +138,16 @@ export default class SplitNoteContainer extends FlexContainer<SplitNoteWidget> {
         await appContext.tabManager.activateNoteContext(isMovingLeft ? ntxIds[leftIndex + 1] : ntxIds[leftIndex]);
     }
 
-    activeContextChangedEvent(): void {
+    activeContextChangedEvent() {
         this.refresh();
     }
 
-    noteSwitchedAndActivatedEvent(): void {
+    noteSwitchedAndActivatedEvent() {
         this.refresh();
     }
 
-    noteContextRemovedEvent({ ntxIds }: { ntxIds: string[] }): void {
+    noteContextRemovedEvent({ ntxIds }: EventData<"noteContextRemoved">) {
-        this.children = this.children.filter((c) => c.ntxId && !ntxIds.includes(c.ntxId));
+        this.children = this.children.filter((c) => !ntxIds.includes(c.ntxId ?? ""));
 
         for (const ntxId of ntxIds) {
             this.$widget.find(`[data-ntx-id="${ntxId}"]`).remove();
@@ -160,7 +156,7 @@ export default class SplitNoteContainer extends FlexContainer<SplitNoteWidget> {
         }
     }
 
-    contextsReopenedEvent({ ntxId, afterNtxId }: { ntxId?: string; afterNtxId?: string }): void {
+    contextsReopenedEvent({ ntxId, afterNtxId }: EventData<"contextsReopened">) {
         if (ntxId === undefined || afterNtxId === undefined) {
             // no single split reopened
             return;
@@ -168,11 +164,13 @@ export default class SplitNoteContainer extends FlexContainer<SplitNoteWidget> {
         this.$widget.find(`[data-ntx-id="${ntxId}"]`).insertAfter(this.$widget.find(`[data-ntx-id="${afterNtxId}"]`));
     }
 
-    async refresh(): Promise<void> {
+    async refresh() {
         this.toggleExt(true);
     }
 
-    toggleExt(show: boolean): void {
+    toggleInt(show: boolean) {} // not needed
+
+    toggleExt(show: boolean) {
         const activeMainContext = appContext.tabManager.getActiveMainContext();
         const activeNtxId = activeMainContext ? activeMainContext.ntxId : null;
 
@@ -180,7 +178,7 @@ export default class SplitNoteContainer extends FlexContainer<SplitNoteWidget> {
             const noteContext = appContext.tabManager.getNoteContextById(ntxId);
 
             const widget = this.widgets[ntxId];
-            widget.toggleExt(show && activeNtxId !== null && [noteContext.ntxId, noteContext.mainNtxId].includes(activeNtxId));
+            widget.toggleExt(show && activeNtxId && [noteContext.ntxId, noteContext.mainNtxId].includes(activeNtxId));
         }
     }
 
@@ -189,50 +187,48 @@ export default class SplitNoteContainer extends FlexContainer<SplitNoteWidget> {
      * are not executed, we're waiting for the first tab activation, and then we update the tab. After this initial
      * activation, further note switches are always propagated to the tabs.
      */
-    handleEventInChildren<T extends EventNames>(name: T, data: EventData<T>): Promise<any> | null {
+    async handleEventInChildren<T extends EventNames>(name: T, data: EventData<T>) {
         if (["noteSwitched", "noteSwitchedAndActivated"].includes(name)) {
             // this event is propagated only to the widgets of a particular tab
-            const noteContext = (data as NoteContextEvent).noteContext;
+            const noteSwitchedContext = data as NoteSwitchedContext;
-            const widget = noteContext.ntxId ? this.widgets[noteContext.ntxId] : undefined;
+            if (!noteSwitchedContext?.noteContext.ntxId) {
+                return Promise.resolve();
+            }
+            const widget = this.widgets[noteSwitchedContext.noteContext.ntxId];
 
             if (!widget) {
                 return Promise.resolve();
             }
 
-            if (widget.hasBeenAlreadyShown || name === "noteSwitchedAndActivated" || appContext.tabManager.getActiveMainContext() === noteContext.getMainContext()) {
+            if (widget.hasBeenAlreadyShown || name === "noteSwitchedAndActivated" || appContext.tabManager.getActiveMainContext() === noteSwitchedContext.noteContext.getMainContext()) {
                 widget.hasBeenAlreadyShown = true;
 
-                return Promise.all([
+                return [widget.handleEvent("noteSwitched", noteSwitchedContext), this.refreshNotShown(noteSwitchedContext)];
-                    widget.handleEvent("noteSwitched", { noteContext, notePath: noteContext.notePath }),
-                    this.refreshNotShown({ noteContext })
-                ]);
             } else {
                 return Promise.resolve();
             }
         }
 
         if (name === "activeContextChanged") {
-            return this.refreshNotShown(data as NoteContextEvent);
+            return this.refreshNotShown(data as EventData<"activeContextChanged">);
         } else {
             return super.handleEventInChildren(name, data);
         }
     }
 
-    private refreshNotShown(data: NoteContextEvent): Promise<any> {
+    refreshNotShown(data: NoteSwitchedContext | EventData<"activeContextChanged">) {
-        const promises: Promise<any>[] = [];
+        const promises = [];
 
         for (const subContext of data.noteContext.getMainContext().getSubContexts()) {
             if (!subContext.ntxId) {
                 continue;
             }
-
             const widget = this.widgets[subContext.ntxId];
 
             if (!widget.hasBeenAlreadyShown) {
                 widget.hasBeenAlreadyShown = true;
 
-                const eventPromise = widget.handleEvent("activeContextChanged", { noteContext: subContext });
+                promises.push(widget.handleEvent("activeContextChanged", { noteContext: subContext }));
-                promises.push(eventPromise || Promise.resolve());
             }
         }