From 8d536a6040f33b418fcc8237ad61e1a52051bcf0 Mon Sep 17 00:00:00 2001 From: Elian Doran Date: Sat, 13 Dec 2025 19:14:23 +0200 Subject: [PATCH] fix(formatting_toolbar): view mode check not working in multi-split --- apps/client/src/components/note_context.ts | 28 ++-- apps/client/src/test/easy-froca.ts | 13 +- .../widgets/ribbon/FormattingToolbar.spec.ts | 151 ++++++++++++++++++ .../src/widgets/ribbon/FormattingToolbar.tsx | 78 +++++++-- 4 files changed, 241 insertions(+), 29 deletions(-) create mode 100644 apps/client/src/widgets/ribbon/FormattingToolbar.spec.ts diff --git a/apps/client/src/components/note_context.ts b/apps/client/src/components/note_context.ts index b360c6fce..caf9bdb13 100644 --- a/apps/client/src/components/note_context.ts +++ b/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); diff --git a/apps/client/src/test/easy-froca.ts b/apps/client/src/test/easy-froca.ts index bcfa7eb0a..59d4c2e4a 100644 --- a/apps/client/src/test/easy-froca.ts +++ b/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: "" diff --git a/apps/client/src/widgets/ribbon/FormattingToolbar.spec.ts b/apps/client/src/widgets/ribbon/FormattingToolbar.spec.ts new file mode 100644 index 000000000..f35a97609 --- /dev/null +++ b/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"); + }); + }); + +}); diff --git a/apps/client/src/widgets/ribbon/FormattingToolbar.tsx b/apps/client/src/widgets/ribbon/FormattingToolbar.tsx index 8c4ba105a..a68ade290 100644 --- a/apps/client/src/widgets/ribbon/FormattingToolbar.tsx +++ b/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(); export function FixedFormattingToolbar() { const containerRef = useRef(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(); // Populate the cache with the toolbar of every note context. @@ -94,9 +89,66 @@ export function FixedFormattingToolbar() {
); } + +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"; +}