fix(formatting_toolbar): view mode check not working in multi-split

apps/client/src/components/note_context.ts

@@ -1,18 +1,19 @@
-import protectedSessionHolder from "../services/protected_session_holder.js";
-import server from "../services/server.js";
-import utils from "../services/utils.js";
-import appContext, { type EventData, type EventListener } from "./app_context.js";
-import treeService from "../services/tree.js";
-import Component from "./component.js";
-import froca from "../services/froca.js";
-import hoistedNoteService from "../services/hoisted_note.js";
-import options from "../services/options.js";
-import type { ViewScope } from "../services/link.js";
-import type FNote from "../entities/fnote.js";
 import type { CKTextEditor } from "@triliumnext/ckeditor5";
 import type CodeMirror from "@triliumnext/codemirror";
+
+import type FNote from "../entities/fnote.js";
 import { closeActiveDialog } from "../services/dialog.js";
+import froca from "../services/froca.js";
+import hoistedNoteService from "../services/hoisted_note.js";
+import type { ViewScope } from "../services/link.js";
+import options from "../services/options.js";
+import protectedSessionHolder from "../services/protected_session_holder.js";
+import server from "../services/server.js";
+import treeService from "../services/tree.js";
+import utils from "../services/utils.js";
 import { ReactWrappedWidget } from "../widgets/basic_widget.js";
+import appContext, { type EventData, type EventListener } from "./app_context.js";
+import Component from "./component.js";
 
 export interface SetNoteOpts {
     triggerSwitchEvent?: unknown;
@@ -64,21 +65,25 @@ class NoteContext extends Component implements EventListener<"entitiesReloaded">
     }
 
     async setNote(inputNotePath: string | undefined, opts: SetNoteOpts = {}) {
+        console.log("Set note to ", inputNotePath);
         opts.triggerSwitchEvent = opts.triggerSwitchEvent !== undefined ? opts.triggerSwitchEvent : true;
         opts.viewScope = opts.viewScope || {};
         opts.viewScope.viewMode = opts.viewScope.viewMode || "default";
 
         if (!inputNotePath) {
+            console.log("EXIT A");
             return;
         }
 
         const resolvedNotePath = await this.getResolvedNotePath(inputNotePath);
 
         if (!resolvedNotePath) {
+            console.log("EXIT B");
             return;
         }
 
         if (this.notePath === resolvedNotePath && utils.areObjectsEqual(this.viewScope, opts.viewScope)) {
+            console.log("EXIT C");
             return;
         }
 
@@ -89,6 +94,7 @@ class NoteContext extends Component implements EventListener<"entitiesReloaded">
         this.notePath = resolvedNotePath;
         this.viewScope = opts.viewScope;
         ({ noteId: this.noteId, parentNoteId: this.parentNoteId } = treeService.getNoteIdAndParentIdFromUrl(resolvedNotePath));
+        console.log("Note ID set to ", this.noteId);
 
         this.saveToRecentNotes(resolvedNotePath);
 

apps/client/src/test/easy-froca.ts

@@ -1,10 +1,12 @@
-import utils from "../services/utils.js";
+import { NoteType } from "@triliumnext/commons";
+
+import FAttribute from "../entities/fattribute.js";
+import FBlob from "../entities/fblob.js";
+import FBranch from "../entities/fbranch.js";
 import FNote from "../entities/fnote.js";
 import froca from "../services/froca.js";
-import FAttribute from "../entities/fattribute.js";
 import noteAttributeCache from "../services/note_attribute_cache.js";
-import FBranch from "../entities/fbranch.js";
-import FBlob from "../entities/fblob.js";
+import utils from "../services/utils.js";
 
 type AttributeDefinitions = { [key in `#${string}`]: string; };
 type RelationDefinitions = { [key in `~${string}`]: string; };
@@ -12,6 +14,7 @@ type RelationDefinitions = { [key in `~${string}`]: string; };
 interface NoteDefinition extends AttributeDefinitions, RelationDefinitions {
     id?: string | undefined;
     title: string;
+    type?: NoteType;
     children?: NoteDefinition[];
     content?: string;
 }
@@ -45,7 +48,7 @@ export function buildNote(noteDef: NoteDefinition) {
     const note = new FNote(froca, {
         noteId: noteDef.id ?? utils.randomString(12),
         title: noteDef.title,
-        type: "text",
+        type: noteDef.type ?? "text",
         mime: "text/html",
         isProtected: false,
         blobId: ""

apps/client/src/widgets/ribbon/FormattingToolbar.spec.ts

@@ -0,0 +1,151 @@
+import { NoteType } from "@triliumnext/commons";
+import { beforeAll, describe, expect, it, vi } from "vitest";
+
+import NoteContext from "../../components/note_context";
+import { ViewMode } from "../../services/link";
+import { randomString } from "../../services/utils";
+import { buildNote } from "../../test/easy-froca";
+import { getFormattingToolbarState } from "./FormattingToolbar";
+
+interface NoteContextInfo {
+    type: NoteType;
+    viewScope?: ViewMode;
+    isReadOnly?: boolean;
+}
+
+describe("Formatting toolbar logic", () => {
+    beforeAll(() => {
+        vi.mock("../../services/tree.ts", () => ({
+            default: {
+                getActiveContextNotePath() {
+                    return "root";
+                },
+                resolveNotePath(inputNotePath: string) {
+                    return inputNotePath;
+                },
+                getNoteIdFromUrl(url) {
+                    return url.split("/").at(-1);
+                }
+            }
+        }));
+
+        buildNote({
+            id: "root",
+            title: "Root"
+        });
+    });
+
+    async function buildConfig(noteContextInfos: NoteContextInfo[], activeIndex: number = 0) {
+        const noteContexts: NoteContext[] = [];
+        for (const noteContextData of noteContextInfos) {
+            const noteContext = new NoteContext(randomString(10));
+            const note = buildNote({
+                title: randomString(5),
+                type: noteContextData.type
+            });
+
+            noteContext.noteId = note.noteId;
+            expect(noteContext.note).toBe(note);
+            noteContext.viewScope = {
+                viewMode: noteContextData.viewScope ?? "default"
+            };
+            noteContext.isReadOnly = async () => !!noteContextData.isReadOnly;
+            noteContext.getSubContexts = () => [];
+            noteContexts.push(noteContext);
+        };
+
+        const mainNoteContext = noteContexts[0];
+        for (const noteContext of noteContexts) {
+            noteContext.getMainContext = () => mainNoteContext;
+        }
+
+        mainNoteContext.getSubContexts = () => noteContexts;
+        return noteContexts[activeIndex];
+    }
+
+    async function testSplit(noteContextInfos: NoteContextInfo[], activeIndex: number = 0, editor = "ckeditor-classic") {
+        const noteContext = await buildConfig(noteContextInfos, activeIndex);
+        return await getFormattingToolbarState(noteContext, noteContext.note, editor);
+    }
+
+    describe("Single split", () => {
+        it("should be hidden for floating toolbar", async () => {
+            expect(await testSplit([ { type: "text" } ], 0, "ckeditor-balloon")).toBe("hidden");
+        });
+
+        it("should be visible for single text note", async () => {
+            expect(await testSplit([ { type: "text" } ])).toBe("visible");
+        });
+
+        it("should be hidden for read-only text note", async () => {
+            expect(await testSplit([ { type: "text", isReadOnly: true } ])).toBe("hidden");
+        });
+
+        it("should be hidden for non-text note", async () => {
+            expect(await testSplit([ { type: "code" } ])).toBe("hidden");
+        });
+
+        it("should be hidden for wrong view mode", async () => {
+            expect(await testSplit([ { type: "text", viewScope: "attachments" } ])).toBe("hidden");
+        });
+    });
+
+    describe("Multi split", () => {
+        it("should be hidden for floating toolbar", async () => {
+            expect(await testSplit([
+                { type: "text" },
+                { type: "text" },
+            ], 0, "ckeditor-balloon")).toBe("hidden");
+        });
+
+        it("should be visible for two text notes", async () => {
+            expect(await testSplit([
+                { type: "text" },
+                { type: "text" },
+            ])).toBe("visible");
+        });
+
+        it("should be disabled if on a non-text note", async () => {
+            expect(await testSplit([
+                { type: "text" },
+                { type: "code" },
+            ], 1)).toBe("disabled");
+        });
+
+        it("should be hidden for all non-text notes", async () => {
+            expect(await testSplit([
+                { type: "code" },
+                { type: "canvas" },
+            ])).toBe("hidden");
+        });
+
+        it("should be hidden for all read-only text notes", async () => {
+            expect(await testSplit([
+                { type: "text", isReadOnly: true },
+                { type: "text", isReadOnly: true },
+            ])).toBe("hidden");
+        });
+
+        it("should be visible for mixed view mode", async () => {
+            expect(await testSplit([
+                { type: "text" },
+                { type: "text", viewScope: "attachments" }
+            ])).toBe("visible");
+        });
+
+        it("should be hidden for all wrong view mode", async () => {
+            expect(await testSplit([
+                { type: "text", viewScope: "attachments" },
+                { type: "text", viewScope: "attachments" }
+            ])).toBe("hidden");
+        });
+
+        it("should be disabled for wrong view mode", async () => {
+            expect(await testSplit([
+                { type: "text" },
+                { type: "text", viewScope: "attachments" }
+            ], 1)).toBe("disabled");
+        });
+    });
+
+});

apps/client/src/widgets/ribbon/FormattingToolbar.tsx

@@ -1,7 +1,9 @@
 import clsx from "clsx";
-import { useEffect, useRef, useState } from "preact/hooks";
+import { useCallback, useEffect, useRef, useState } from "preact/hooks";
 
-import { useActiveNoteContext, useIsNoteReadOnly, useNoteProperty, useTriliumEvent, useTriliumOption } from "../react/hooks";
+import NoteContext from "../../components/note_context";
+import FNote from "../../entities/fnote";
+import { useActiveNoteContext, useIsNoteReadOnly, useNoteProperty, useTriliumEvent, useTriliumEvents, useTriliumOption } from "../react/hooks";
 import { TabContext } from "./ribbon-interface";
 
 /**
@@ -42,16 +44,9 @@ const toolbarCache = new Map<string, HTMLElement | null | undefined>();
 
 export function FixedFormattingToolbar() {
     const containerRef = useRef<HTMLDivElement>(null);
-    const [ textNoteEditorType ] = useTriliumOption("textNoteEditorType");
-    const { note, noteContext, ntxId, viewScope } = useActiveNoteContext();
+    const { note, noteContext, ntxId } = useActiveNoteContext();
     const noteType = useNoteProperty(note, "type");
-    const { isReadOnly } = useIsNoteReadOnly(note, noteContext);
-    const shown = (
-        viewScope?.viewMode === "default" &&
-        textNoteEditorType === "ckeditor-classic" &&
-        (noteContext?.getMainContext().getSubContexts() ?? []).some(sub => sub.note?.type === "text") &&
-        !isReadOnly
-    );
+    const renderState = useRenderState(noteContext, note);
     const [ toolbarToRender, setToolbarToRender ] = useState<HTMLElement | null | undefined>();
 
     // Populate the cache with the toolbar of every note context.
@@ -94,9 +89,66 @@ export function FixedFormattingToolbar() {
         <div
             ref={containerRef}
             className={clsx("classic-toolbar-widget", {
-                "hidden-ext": !shown,
-                "disabled": noteType !== "text"
+                "hidden-ext": renderState === "hidden",
+                "disabled": renderState === "disabled"
             })}
         />
     );
 }
+
+function useRenderState(activeNoteContext: NoteContext | undefined, activeNote: FNote | null | undefined) {
+    const [ textNoteEditorType ] = useTriliumOption("textNoteEditorType");
+    const [ state, setState ] = useState("hidden");
+
+    useTriliumEvents([ "newNoteContextCreated", "noteContextRemoved" ], () => {
+        getFormattingToolbarState(activeNoteContext, activeNote, textNoteEditorType).then(setState);
+    });
+
+    useEffect(() => {
+        getFormattingToolbarState(activeNoteContext, activeNote, textNoteEditorType).then(setState);
+    }, [ activeNoteContext, activeNote, textNoteEditorType ]);
+
+    return state;
+}
+
+export async function getFormattingToolbarState(activeNoteContext: NoteContext | undefined, activeNote: FNote | null | undefined, textNoteEditorType: string) {
+    if (!activeNoteContext || textNoteEditorType !== "ckeditor-classic") {
+        return "hidden";
+    }
+
+    const subContexts = activeNoteContext?.getMainContext().getSubContexts() ?? [];
+    if (subContexts.length === 1) {
+        if (activeNote?.type !== "text" || activeNoteContext.viewScope?.viewMode !== "default") {
+            return "hidden";
+        }
+
+        const isReadOnly = await activeNoteContext.isReadOnly();
+        if (isReadOnly) {
+            return "hidden";
+        }
+
+        return "visible";
+    }
+
+    // If there are multiple note contexts (e.g. splits), the logic is slightly different.
+    const textNoteContexts = subContexts.filter(s => s.note?.type === "text" && s.viewScope?.viewMode === "default");
+    const textNoteContextsReadOnly = await Promise.all(textNoteContexts.map(sc => sc.isReadOnly()));
+
+    // If all text notes are hidden, no need to display the toolbar at all.
+    if (textNoteContextsReadOnly.indexOf(false) === -1) {
+        return "hidden";
+    }
+
+    // If the current subcontext is not a text note, but there is at least an editable text then it must be disabled.
+    if (activeNote?.type !== "text") return "disabled";
+
+    // If the current subcontext is a text note, it must not be read-only.
+    if (activeNote.type === "text") {
+        const subContextIndex = textNoteContexts.indexOf(activeNoteContext);
+        if (subContextIndex !== -1) {
+            if (textNoteContextsReadOnly[subContextIndex]) return "disabled";
+        }
+        if (activeNoteContext.viewScope?.viewMode !== "default") return "disabled";
+    }
+    return "visible";
+}