From 67f39e8bde78105c841f627560b1881394c6ca86 Mon Sep 17 00:00:00 2001 From: Elian Doran Date: Fri, 4 Apr 2025 17:57:54 +0300 Subject: [PATCH] fix(client): strange behaviour when navigating between tabs (closes #1615) --- e2e/layout/tab_bar.spec.ts | 17 ++++ e2e/support/app.ts | 24 +++++ src/public/app/components/app_context.ts | 23 +++-- src/public/app/widgets/basic_widget.ts | 2 +- .../containers/split_note_container.ts | 88 +++++++++---------- 5 files changed, 98 insertions(+), 56 deletions(-) diff --git a/e2e/layout/tab_bar.spec.ts b/e2e/layout/tab_bar.spec.ts index fe5a06a54..c5a3d7e5c 100644 --- a/e2e/layout/tab_bar.spec.ts +++ b/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(); +}); diff --git a/e2e/support/app.ts b/e2e/support/app.ts index 8ae430f1b..137c026e9 100644 --- a/e2e/support/app.ts +++ b/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. diff --git a/src/public/app/components/app_context.ts b/src/public/app/components/app_context.ts index 5c0295baa..daec01e91 100644 --- a/src/public/app/components/app_context.ts +++ b/src/public/app/components/app_context.ts @@ -66,6 +66,11 @@ export interface ExecuteCommandData 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: { - noteContext: NoteContext; - notePath?: string | null; - }; - noteSwitchedAndActivated: { - noteContext: NoteContext; - notePath: string; - }; + noteSwitched: NoteSwitchedContext; + noteSwitchedAndActivated: NoteSwitchedContext; 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; diff --git a/src/public/app/widgets/basic_widget.ts b/src/public/app/widgets/basic_widget.ts index 4e7c36760..be7b0bbd7 100644 --- a/src/public/app/widgets/basic_widget.ts +++ b/src/public/app/widgets/basic_widget.ts @@ -214,7 +214,7 @@ export class TypedBasicWidget> 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); } diff --git a/src/public/app/widgets/containers/split_note_container.ts b/src/public/app/widgets/containers/split_note_container.ts index 8e686476c..9bd5c04b5 100644 --- a/src/public/app/widgets/containers/split_note_container.ts +++ b/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 NoteContext from "../../components/note_context.js"; -import type { CommandMappings, EventNames, EventData } from "../../components/app_context.js"; +import appContext, { type CommandData, type CommandListenerData, type EventData, type EventNames, type NoteSwitchedContext } 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 { + private widgetFactory: WidgetFactory; - private widgets: Widgets; + private widgets: Record; constructor(widgetFactory: WidgetFactory) { super("row"); @@ -34,7 +30,7 @@ export default class SplitNoteContainer extends FlexContainer { 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 { this.child(widget); } - async openNewNoteSplitEvent({ ntxId, notePath, hoistedNoteId, viewScope }: { - ntxId: string; - notePath?: string; - hoistedNoteId?: string; - viewScope?: any; - }) { + async openNewNoteSplitEvent({ ntxId, notePath, hoistedNoteId, viewScope }: EventData<"openNewNoteSplit">) { 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 { 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 { } } - closeThisNoteSplitCommand({ ntxId }: { ntxId: string }): void { - appContext.tabManager.removeNoteContext(ntxId); + closeThisNoteSplitCommand({ ntxId }: CommandListenerData<"closeThisNoteSplit">) { + if (ntxId) { + appContext.tabManager.removeNoteContext(ntxId); + } } - async moveThisNoteSplitCommand({ ntxId, isMovingLeft }: { ntxId: string; isMovingLeft: boolean }): Promise { + async moveThisNoteSplitCommand({ ntxId, isMovingLeft }: CommandListenerData<"moveThisNoteSplit">) { if (!ntxId) { logError("empty ntxId!"); return; @@ -125,11 +121,11 @@ export default class SplitNoteContainer extends FlexContainer { 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 { 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 { - this.children = this.children.filter((c) => c.ntxId && !ntxIds.includes(c.ntxId)); + noteContextRemovedEvent({ ntxIds }: EventData<"noteContextRemoved">) { + 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 { } } - 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 { this.$widget.find(`[data-ntx-id="${ntxId}"]`).insertAfter(this.$widget.find(`[data-ntx-id="${afterNtxId}"]`)); } - async refresh(): Promise { + 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 { 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 { * 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(name: T, data: EventData): Promise | null { + async handleEventInChildren(name: T, data: EventData) { if (["noteSwitched", "noteSwitchedAndActivated"].includes(name)) { // this event is propagated only to the widgets of a particular tab - const noteContext = (data as NoteContextEvent).noteContext; - const widget = noteContext.ntxId ? this.widgets[noteContext.ntxId] : undefined; + const noteSwitchedContext = data as NoteSwitchedContext; + 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([ - widget.handleEvent("noteSwitched", { noteContext, notePath: noteContext.notePath }), - this.refreshNotShown({ noteContext }) - ]); + return [widget.handleEvent("noteSwitched", noteSwitchedContext), this.refreshNotShown(noteSwitchedContext)]; } 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 { - const promises: Promise[] = []; + refreshNotShown(data: NoteSwitchedContext | EventData<"activeContextChanged">) { + 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(eventPromise || Promise.resolve()); + promises.push(widget.handleEvent("activeContextChanged", { noteContext: subContext })); } }