From 793804cd0da6a5782d203d683158282cc8d29b5b Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 23 Mar 2026 20:21:43 -0400 Subject: [PATCH] feat: Strip comments from presentation mode (#11860) * feat: Strip comment marks from documents in presentation mode Move removeMarks to shared ProsemirrorHelper and use it to strip comment marks before rendering slides. Make server ProsemirrorHelper extend the shared class to eliminate duplication and remove SharedProsemirrorHelper imports from server code. Co-Authored-By: Claude Opus 4.6 * fix: Use Set for mark lookup and cloneDeep for browser compat Use a Set for O(1) mark lookups in removeMarks traversal. Replace structuredClone with lodash/cloneDeep to support older browsers that lack the native API. Co-Authored-By: Claude Opus 4.6 * test: Add tests for ProsemirrorHelper.removeMarks Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- .../Document/components/PresentationMode.tsx | 14 +- server/commands/documentCreator.ts | 3 +- .../models/helpers/ProseMirrorHelper.test.ts | 3 +- server/models/helpers/ProsemirrorHelper.tsx | 34 +---- server/models/helpers/TextHelper.test.ts | 3 +- server/queues/tasks/APIImportTask.ts | 7 +- server/utils/DocumentConverter.ts | 3 +- shared/utils/ProsemirrorHelper.test.ts | 144 ++++++++++++++++++ shared/utils/ProsemirrorHelper.ts | 30 +++- 9 files changed, 196 insertions(+), 45 deletions(-) diff --git a/app/scenes/Document/components/PresentationMode.tsx b/app/scenes/Document/components/PresentationMode.tsx index 71692daff2..345a329406 100644 --- a/app/scenes/Document/components/PresentationMode.tsx +++ b/app/scenes/Document/components/PresentationMode.tsx @@ -7,7 +7,9 @@ import Icon from "@shared/components/Icon"; import { richExtensions } from "@shared/editor/nodes"; import { canUseElementFullscreen } from "@shared/utils/browser"; import { s, depths, hover } from "@shared/styles"; +import cloneDeep from "lodash/cloneDeep"; import type { ProsemirrorData } from "@shared/types"; +import { ProsemirrorHelper } from "@shared/utils/ProsemirrorHelper"; import { colorPalette } from "@shared/utils/collections"; import Editor from "~/components/Editor"; import NudeButton from "~/components/NudeButton"; @@ -130,8 +132,16 @@ function PresentationMode({ title, icon, iconColor, data, onClose }: Props) { const supportsFullscreen = React.useMemo(() => canUseElementFullscreen(), []); const isIdle = useIdle(3000, idleEvents); + const strippedData = React.useMemo( + () => + ProsemirrorHelper.removeMarks(cloneDeep(data), [ + "comment", + ]) as ProsemirrorData, + [data] + ); + const slides = React.useMemo(() => { - const result = splitIntoSlides(data, title, icon, iconColor); + const result = splitIntoSlides(strippedData, title, icon, iconColor); const contentSlides = result.filter((s) => s.type === "content"); const hasContent = contentSlides.length > 0 && @@ -144,7 +154,7 @@ function PresentationMode({ title, icon, iconColor, data, onClose }: Props) { } return result; - }, [data, title, icon, iconColor]); + }, [strippedData, title, icon, iconColor]); const totalSlides = slides.length; diff --git a/server/commands/documentCreator.ts b/server/commands/documentCreator.ts index 2bddde1640..b2937a62f9 100644 --- a/server/commands/documentCreator.ts +++ b/server/commands/documentCreator.ts @@ -1,5 +1,4 @@ import type { Optional } from "utility-types"; -import { ProsemirrorHelper as SharedProsemirrorHelper } from "@shared/utils/ProsemirrorHelper"; import { TextHelper } from "@shared/utils/TextHelper"; import { Document, type Template } from "@server/models"; import { DocumentHelper } from "@server/models/helpers/DocumentHelper"; @@ -94,7 +93,7 @@ export default async function documentCreator( : text ? ProsemirrorHelper.toProsemirror(text).toJSON() : template - ? SharedProsemirrorHelper.replaceTemplateVariables( + ? ProsemirrorHelper.replaceTemplateVariables( await DocumentHelper.toJSON(template), user ) diff --git a/server/models/helpers/ProseMirrorHelper.test.ts b/server/models/helpers/ProseMirrorHelper.test.ts index f3668b665e..0dfb697dc8 100644 --- a/server/models/helpers/ProseMirrorHelper.test.ts +++ b/server/models/helpers/ProseMirrorHelper.test.ts @@ -2,7 +2,6 @@ import { faker } from "@faker-js/faker"; import type { DeepPartial } from "utility-types"; import type { ProsemirrorData } from "@shared/types"; import { MentionType } from "@shared/types"; -import { ProsemirrorHelper as SharedProsemirrorHelper } from "@shared/utils/ProsemirrorHelper"; import { createContext } from "@server/context"; import { buildProseMirrorDoc, buildUser } from "@server/test/factories"; import type { MentionAttrs } from "./ProsemirrorHelper"; @@ -973,7 +972,7 @@ describe("ProsemirrorHelper", () => { }, ]); - const images = SharedProsemirrorHelper.getImages(doc); + const images = ProsemirrorHelper.getImages(doc); expect(images.length).toBe(1); expect(images[0].attrs.src).toBe("https://example.com/image.png"); expect(images[0].attrs.alt).toBe("Test image"); diff --git a/server/models/helpers/ProsemirrorHelper.tsx b/server/models/helpers/ProsemirrorHelper.tsx index 238cf55513..f067dc7c5f 100644 --- a/server/models/helpers/ProsemirrorHelper.tsx +++ b/server/models/helpers/ProsemirrorHelper.tsx @@ -21,6 +21,7 @@ import { attachmentRedirectRegex, ProsemirrorHelper as SharedProsemirrorHelper, } from "@shared/utils/ProsemirrorHelper"; + import parseDocumentSlug from "@shared/utils/parseDocumentSlug"; import { isRTL } from "@shared/utils/rtl"; import { isInternalUrl } from "@shared/utils/urls"; @@ -62,7 +63,7 @@ export type MentionAttrs = { }; @trace() -export class ProsemirrorHelper { +export class ProsemirrorHelper extends SharedProsemirrorHelper { /** * Returns the input text as a Y.Doc. * @@ -255,33 +256,6 @@ export class ProsemirrorHelper { return blockNode ? doc.copy(Fragment.fromArray([blockNode])) : undefined; } - /** - * Removes all marks from the node that match the given types. - * - * @param data The ProsemirrorData object to remove marks from - * @param marks The mark types to remove - * @returns The content with marks removed - */ - static removeMarks(doc: Node | ProsemirrorData, marks: string[]) { - const json = "toJSON" in doc ? (doc.toJSON() as ProsemirrorData) : doc; - - function removeMarksInner(node: ProsemirrorData) { - if (node.marks) { - node.marks = node.marks.filter((mark) => !marks.includes(mark.type)); - } - if (node.attrs?.marks) { - node.attrs.marks = (node.attrs.marks as { type: string }[])?.filter( - (mark) => !marks.includes(mark.type) - ); - } - if (node.content) { - node.content.forEach(removeMarksInner); - } - return node; - } - return removeMarksInner(json); - } - static async replaceInternalUrls( doc: Node | ProsemirrorData, basePath: string @@ -875,8 +849,8 @@ export class ProsemirrorHelper { doc: Node, user: User ): Promise { - const images = SharedProsemirrorHelper.getImages(doc); - const videos = SharedProsemirrorHelper.getVideos(doc); + const images = ProsemirrorHelper.getImages(doc); + const videos = ProsemirrorHelper.getVideos(doc); const nodes = [...images, ...videos]; if (!nodes.length) { diff --git a/server/models/helpers/TextHelper.test.ts b/server/models/helpers/TextHelper.test.ts index 1105392292..4be7e6f509 100644 --- a/server/models/helpers/TextHelper.test.ts +++ b/server/models/helpers/TextHelper.test.ts @@ -1,4 +1,3 @@ -import { ProsemirrorHelper as SharedProsemirrorHelper } from "@shared/utils/ProsemirrorHelper"; import { createContext } from "@server/context"; import { buildProseMirrorDoc, buildUser } from "@server/test/factories"; import { ProsemirrorHelper } from "./ProsemirrorHelper"; @@ -43,7 +42,7 @@ describe("ProsemirrorHelper", () => { }, ]); - const images = SharedProsemirrorHelper.getImages(doc); + const images = ProsemirrorHelper.getImages(doc); expect(images.length).toBe(1); expect(images[0].attrs.src).toBe("https://example.com/image.png"); expect(images[0].attrs.alt).toBe("Test image"); diff --git a/server/queues/tasks/APIImportTask.ts b/server/queues/tasks/APIImportTask.ts index 7d1b0ab9c9..334347199e 100644 --- a/server/queues/tasks/APIImportTask.ts +++ b/server/queues/tasks/APIImportTask.ts @@ -13,7 +13,6 @@ import type { ProsemirrorDoc, } from "@shared/types"; import { AttachmentPreset, ImportState, ImportTaskState } from "@shared/types"; -import { ProsemirrorHelper as SharedProseMirrorHelper } from "@shared/utils/ProsemirrorHelper"; import { createContext } from "@server/context"; import { schema } from "@server/editor"; import Logger from "@server/logging/Logger"; @@ -262,9 +261,9 @@ export default abstract class APIImportTask< }): Promise { const docNode = ProsemirrorHelper.toProsemirror(doc); const nodes = [ - ...SharedProseMirrorHelper.getImages(docNode), - ...SharedProseMirrorHelper.getVideos(docNode), - ...SharedProseMirrorHelper.getAttachments(docNode), + ...ProsemirrorHelper.getImages(docNode), + ...ProsemirrorHelper.getVideos(docNode), + ...ProsemirrorHelper.getAttachments(docNode), ]; if (!nodes.length) { diff --git a/server/utils/DocumentConverter.ts b/server/utils/DocumentConverter.ts index 1e9ab3662f..91ac6cf9fc 100644 --- a/server/utils/DocumentConverter.ts +++ b/server/utils/DocumentConverter.ts @@ -6,7 +6,6 @@ import mammoth from "mammoth"; import type { Node } from "prosemirror-model"; import { DOMParser as ProsemirrorDOMParser } from "prosemirror-model"; import yaml from "js-yaml"; -import { ProsemirrorHelper as SharedProsemirrorHelper } from "@shared/utils/ProsemirrorHelper"; import { schema, serializer } from "@server/editor"; import { FileImportError } from "@server/errors"; import { trace, traceFunction } from "@server/logging/tracing"; @@ -55,7 +54,7 @@ export class DocumentConverter { // Extract title from first H1 heading let title = ""; - const headings = SharedProsemirrorHelper.getHeadings(doc); + const headings = ProsemirrorHelper.getHeadings(doc); if (headings.length > 0 && headings[0].level === 1) { title = headings[0].title; doc = ProsemirrorHelper.removeFirstHeading(doc); diff --git a/shared/utils/ProsemirrorHelper.test.ts b/shared/utils/ProsemirrorHelper.test.ts index 33c295ec8a..e9f0759353 100644 --- a/shared/utils/ProsemirrorHelper.test.ts +++ b/shared/utils/ProsemirrorHelper.test.ts @@ -234,4 +234,148 @@ describe("ProsemirrorHelper", () => { ]); }); }); + + describe("removeMarks", () => { + it("should remove specified mark types from text nodes", () => { + const doc: ProsemirrorData = { + type: "doc", + content: [ + { + type: "paragraph", + content: [ + { + type: "text", + text: "hello", + marks: [ + { type: "comment", attrs: { id: "c1" } }, + { type: "bold" }, + ], + }, + ], + }, + ], + }; + + const result = ProsemirrorHelper.removeMarks(doc, ["comment"]); + expect(result.content![0].content![0].marks).toEqual([{ type: "bold" }]); + }); + + it("should remove marks from nested content", () => { + const doc: ProsemirrorData = { + type: "doc", + content: [ + { + type: "blockquote", + content: [ + { + type: "paragraph", + content: [ + { + type: "text", + text: "nested", + marks: [{ type: "comment", attrs: { id: "c1" } }], + }, + ], + }, + ], + }, + ], + }; + + const result = ProsemirrorHelper.removeMarks(doc, ["comment"]); + expect(result.content![0].content![0].content![0].marks).toEqual([]); + }); + + it("should remove marks from node attrs.marks", () => { + const doc: ProsemirrorData = { + type: "doc", + content: [ + { + type: "image", + attrs: { + src: "test.png", + marks: [ + { type: "comment", attrs: { id: "c1" } }, + { type: "link", attrs: { href: "url" } }, + ], + }, + }, + ], + }; + + const result = ProsemirrorHelper.removeMarks(doc, ["comment"]); + expect(result.content![0].attrs!.marks).toEqual([ + { type: "link", attrs: { href: "url" } }, + ]); + }); + + it("should remove multiple mark types at once", () => { + const doc: ProsemirrorData = { + type: "doc", + content: [ + { + type: "paragraph", + content: [ + { + type: "text", + text: "hello", + marks: [ + { type: "comment", attrs: { id: "c1" } }, + { type: "bold" }, + { type: "highlight" }, + ], + }, + ], + }, + ], + }; + + const result = ProsemirrorHelper.removeMarks(doc, [ + "comment", + "highlight", + ]); + expect(result.content![0].content![0].marks).toEqual([{ type: "bold" }]); + }); + + it("should leave nodes unchanged when no marks match", () => { + const doc: ProsemirrorData = { + type: "doc", + content: [ + { + type: "paragraph", + content: [ + { + type: "text", + text: "hello", + marks: [{ type: "bold" }], + }, + ], + }, + ], + }; + + const result = ProsemirrorHelper.removeMarks(doc, ["comment"]); + expect(result.content![0].content![0].marks).toEqual([{ type: "bold" }]); + }); + + it("should handle nodes with no marks", () => { + const doc: ProsemirrorData = { + type: "doc", + content: [ + { + type: "paragraph", + content: [ + { + type: "text", + text: "plain", + }, + ], + }, + ], + }; + + const result = ProsemirrorHelper.removeMarks(doc, ["comment"]); + expect(result.content![0].content![0].marks).toBeUndefined(); + }); + }); }); diff --git a/shared/utils/ProsemirrorHelper.ts b/shared/utils/ProsemirrorHelper.ts index d452fcfa9b..bb502c5d42 100644 --- a/shared/utils/ProsemirrorHelper.ts +++ b/shared/utils/ProsemirrorHelper.ts @@ -48,10 +48,38 @@ export const attachmentPublicRegex = /public\/([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\/(?[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})/gi; export class ProsemirrorHelper { + /** + * Remove specific mark types from all nodes in the document. + * + * @param doc the prosemirror document or JSON data. + * @param marks the mark type names to remove. + * @returns the document data with specified marks removed. + */ + static removeMarks(doc: Node | ProsemirrorData, marks: string[]) { + const json = "toJSON" in doc ? (doc.toJSON() as ProsemirrorData) : doc; + const markSet = new Set(marks); + + function removeMarksInner(node: ProsemirrorData) { + if (node.marks) { + node.marks = node.marks.filter((mark) => !markSet.has(mark.type)); + } + if (node.attrs?.marks) { + node.attrs.marks = (node.attrs.marks as { type: string }[])?.filter( + (mark) => !markSet.has(mark.type) + ); + } + if (node.content) { + node.content.forEach(removeMarksInner); + } + return node; + } + return removeMarksInner(json); + } + /** * Get a new empty document. * - * @returns A new empty document as JSON. + * @returns a new empty document as JSON. */ static getEmptyDocument(): ProsemirrorData { return {