From a5af47266e5b96c02fe45037bfea4881b0a7e684 Mon Sep 17 00:00:00 2001 From: Luccas Correa Date: Thu, 22 Jan 2026 18:48:26 -0300 Subject: [PATCH 1/5] refactor: simplify paragraph and table converters in pm-adapter (SD-1587) (#1806) * fix: remove dead code * refactor: table and paragraph converters to avoid redirection * test: adjust existing tests * fix: remove unused imports and dead code * fix: pass theme colors to node handlers * refactor: simplify type definitions for converters --------- Co-authored-by: Luccas Correa --- packages/layout-engine/contracts/src/index.ts | 2 +- .../src/attributes/paragraph-styles.ts | 670 ------------------ .../src/converters/paragraph.test.ts | 96 ++- .../pm-adapter/src/converters/paragraph.ts | 139 ++-- .../pm-adapter/src/converters/table.test.ts | 176 +++-- .../pm-adapter/src/converters/table.ts | 217 +++--- .../layout-engine/pm-adapter/src/index.d.ts | 2 +- .../pm-adapter/src/index.test.ts | 43 +- .../layout-engine/pm-adapter/src/index.ts | 4 +- .../pm-adapter/src/internal.test.ts | 226 +----- .../layout-engine/pm-adapter/src/internal.ts | 297 +------- .../pm-adapter/src/sdt/document-index.test.ts | 15 +- .../pm-adapter/src/sdt/document-index.ts | 19 +- .../src/sdt/document-part-object.test.ts | 35 +- .../src/sdt/document-part-object.ts | 22 +- .../src/sdt/document-section.test.ts | 64 +- .../pm-adapter/src/sdt/document-section.ts | 194 +++-- .../src/sdt/structured-content-block.test.ts | 79 ++- .../src/sdt/structured-content-block.ts | 29 +- .../pm-adapter/src/sdt/toc.test.ts | 81 ++- .../layout-engine/pm-adapter/src/sdt/toc.ts | 65 +- .../layout-engine/pm-adapter/src/types.ts | 118 ++- 22 files changed, 781 insertions(+), 1812 deletions(-) delete mode 100644 packages/layout-engine/pm-adapter/src/attributes/paragraph-styles.ts diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index ac89f3f3e4..fc0e485a6d 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -162,7 +162,7 @@ export type RunMarks = { style?: 'single' | 'double' | 'dotted' | 'dashed' | 'wavy'; /** Underline color as hex string (defaults to text color). */ color?: string; - }; + } | null; /** Strikethrough text decoration. */ strike?: boolean; /** Highlight (background) color as hex string. */ diff --git a/packages/layout-engine/pm-adapter/src/attributes/paragraph-styles.ts b/packages/layout-engine/pm-adapter/src/attributes/paragraph-styles.ts deleted file mode 100644 index 8126e28eda..0000000000 --- a/packages/layout-engine/pm-adapter/src/attributes/paragraph-styles.ts +++ /dev/null @@ -1,670 +0,0 @@ -import type { ParagraphAttrs, ParagraphIndent, ParagraphSpacing } from '@superdoc/contracts'; -import { createOoxmlResolver, resolveDocxFontFamily, type OoxmlTranslator } from '@superdoc/style-engine/ooxml'; -import { SuperConverter } from '@superdoc/super-editor/converter/internal/SuperConverter.js'; -import { translator as w_pPrTranslator } from '@superdoc/super-editor/converter/internal/v3/handlers/w/pPr/index.js'; -import { translator as w_rPrTranslator } from '@superdoc/super-editor/converter/internal/v3/handlers/w/rpr/index.js'; -import type { PMNode } from '../types.js'; -import type { ConverterContext, ConverterNumberingContext } from '../converter-context.js'; -import { hasParagraphStyleContext } from '../converter-context.js'; -import type { ResolvedParagraphProperties } from '@superdoc/word-layout'; -import { normalizeAlignment } from './spacing-indent.js'; - -/** - * Empty numbering context used as a fallback when documents don't have lists. - * This allows paragraph style resolution to proceed even without numbering data. - */ -const EMPTY_NUMBERING_CONTEXT: ConverterNumberingContext = { - definitions: {}, - abstracts: {}, -}; - -const toOoxmlTranslator = (translator: { xmlName: string; encode: (params: any) => unknown }): OoxmlTranslator => ({ - xmlName: translator.xmlName, - encode: (params) => translator.encode(params) as Record | null | undefined, -}); - -const ooxmlResolver = createOoxmlResolver({ - pPr: toOoxmlTranslator(w_pPrTranslator), - rPr: toOoxmlTranslator(w_rPrTranslator), -}); - -/** - * Result of hydrating paragraph attributes from style resolution. - * - * Contains paragraph-level formatting properties resolved from the style cascade, - * including document defaults and paragraph style definitions. - * - * @property resolved - Complete resolved paragraph properties from style engine - * @property spacing - Paragraph spacing (before, after, line) in OOXML units - * @property indent - Paragraph indentation (left, right, firstLine, hanging) in OOXML units - * @property borders - Paragraph border definitions (top, right, bottom, left) - * @property shading - Paragraph background shading and fill color - * @property alignment - Paragraph text alignment (left, right, center, justify) - * @property tabStops - Custom tab stop definitions - * @property keepLines - Keep all lines of paragraph together (prevent pagination splits) - * @property keepNext - Keep paragraph with next paragraph (prevent page break between) - * @property numberingProperties - Numbering/list properties (numId, ilvl, etc.) - * @property contextualSpacing - Contextual spacing flag from OOXML w:contextualSpacing. - * - * ## contextualSpacing Property - * - * Implements MS Word's "Don't add space between paragraphs of the same style" setting - * (OOXML w:contextualSpacing element). When true, spacing before/after is suppressed - * between consecutive paragraphs that share the same paragraph style. - * - * **Common Usage:** - * - ListBullet and ListNumber styles typically define contextualSpacing=true - * - Prevents excessive spacing between consecutive list items - * - Maintains spacing between list items and non-list paragraphs - * - * **OOXML Structure:** - * In OOXML, w:contextualSpacing is a sibling to w:spacing, not nested within it: - * ```xml - * - * - * - * - * ``` - * - * **Fallback Priority in computeParagraphAttrs:** - * 1. normalizedSpacing.contextualSpacing - From spacing XML element - * 2. paragraphProps.contextualSpacing - Direct pPr property - * 3. attrs.contextualSpacing - ProseMirror node attributes - * 4. hydrated.contextualSpacing - From style resolution (this property) - * - * @example - * ```typescript - * // Style resolution for ListBullet with contextualSpacing - * const hydrated: ParagraphStyleHydration = { - * spacing: { before: 0, after: 0 }, - * indent: { left: 720, hanging: 360 }, - * contextualSpacing: true, // Suppress spacing between same-style paragraphs - * }; - * ``` - */ -export type ParagraphStyleHydration = { - resolved?: ResolvedParagraphProperties; - spacing?: ParagraphSpacing; - indent?: ParagraphIndent; - borders?: ParagraphAttrs['borders']; - shading?: ParagraphAttrs['shading']; - alignment?: ParagraphAttrs['alignment']; - tabStops?: unknown; - keepLines?: boolean; - keepNext?: boolean; - numberingProperties?: Record; - contextualSpacing?: boolean; -}; - -/** - * Hydrates paragraph-level attributes from a linked style when converter context is available. - * - * This function works even when styleId is null or undefined, as it will apply docDefaults - * from the document's styles.xml through the resolveParagraphProperties function. This ensures - * that all paragraphs receive at minimum the document's default spacing and formatting. - * - * The helper never mutates the ProseMirror node; callers should merge the returned - * attributes with existing attrs, preserving explicit overrides on the node. - * - * Normal style semantics (doc defaults, w:default flags) are delegated to - * resolveParagraphProperties which already mirrors Word's cascade rules. - * - * @param para - The ProseMirror paragraph node to hydrate - * @param context - The converter context containing DOCX and optional numbering data - * @param preResolved - Optional pre-resolved paragraph properties to use instead of resolving - * @returns Hydrated paragraph attributes or null if context is missing or resolution fails. - * Returns null when: - * - context is undefined or missing docx data (checked by hasParagraphStyleContext) - * - resolveParagraphProperties returns null or undefined - * - * @remarks - * - Provides an empty numbering fallback (EMPTY_NUMBERING_CONTEXT) for documents without lists, - * ensuring paragraph style resolution can proceed even when context.numbering is undefined. - * - Uses null-safe checks (!= null) for numberingProperties, indent, and spacing to handle - * both null and undefined consistently. - */ -export const hydrateParagraphStyleAttrs = ( - para: PMNode, - context?: ConverterContext, - preResolved?: ResolvedParagraphProperties, -): ParagraphStyleHydration | null => { - if (!hasParagraphStyleContext(context)) { - return null; - } - const attrs = para.attrs ?? {}; - const paragraphProps = - typeof attrs.paragraphProperties === 'object' && attrs.paragraphProperties !== null - ? (attrs.paragraphProperties as Record) - : {}; - const styleIdSource = attrs.styleId ?? paragraphProps.styleId; - const styleId = typeof styleIdSource === 'string' && styleIdSource.trim() ? styleIdSource : null; - - const inlineProps: Record = { styleId }; - - const numberingProperties = cloneIfObject(attrs.numberingProperties ?? paragraphProps.numberingProperties); - if (numberingProperties != null) { - inlineProps.numberingProperties = numberingProperties; - } - - const indent = cloneIfObject(attrs.indent ?? paragraphProps.indent); - if (indent != null) { - inlineProps.indent = indent; - } - - const spacing = cloneIfObject(attrs.spacing ?? paragraphProps.spacing); - if (spacing != null) { - inlineProps.spacing = spacing; - } - - const resolverParams = { - docx: context.docx, - // Provide empty numbering context if not present - documents without lists - // should still get docDefaults spacing from style resolution - numbering: context.numbering ?? EMPTY_NUMBERING_CONTEXT, - }; - - // Cast to bypass JSDoc type mismatch - the JS function actually accepts { docx, numbering } - const resolved = preResolved ?? ooxmlResolver.resolveParagraphProperties(resolverParams as never, inlineProps); - if (!resolved) { - return null; - } - - // TypeScript: resolved could be ResolvedParagraphProperties (from preResolved) - // or the extended type from resolveParagraphProperties. - // We safely access properties using optional chaining and type assertions. - type ExtendedResolvedProps = ResolvedParagraphProperties & { - borders?: unknown; - shading?: unknown; - justification?: unknown; - tabStops?: unknown; - keepLines?: boolean; - keepNext?: boolean; - outlineLvl?: number; - /** - * Contextual spacing from style resolution. - * In OOXML, w:contextualSpacing is a sibling to w:spacing, not nested within it. - * When true, spacing is suppressed between paragraphs of the same style. - */ - contextualSpacing?: boolean; - }; - const resolvedExtended = resolved as ExtendedResolvedProps; - const resolvedAsRecord = resolved as Record; - let resolvedIndent = cloneIfObject(resolvedAsRecord.indent) as ParagraphIndent | undefined; - - // Word built-in heading styles do NOT inherit Normal's first-line indent. - // If the resolved paragraph is a heading (outline level present or styleId starts with headingX) - // and no explicit indent was defined on the style/para, normalize indent to zero. - const styleIdLower = typeof styleId === 'string' ? styleId.toLowerCase() : ''; - const isHeadingStyle = - typeof resolvedExtended.outlineLvl === 'number' || - styleIdLower.startsWith('heading ') || - styleIdLower.startsWith('heading'); - const onlyFirstLineIndent = - resolvedIndent && - resolvedIndent.firstLine != null && - resolvedIndent.hanging == null && - resolvedIndent.left == null && - resolvedIndent.right == null; - if (isHeadingStyle && (!resolvedIndent || Object.keys(resolvedIndent).length === 0 || onlyFirstLineIndent)) { - // Clear inherited firstLine/hanging from Normal - resolvedIndent = { firstLine: 0, hanging: 0, left: resolvedIndent?.left, right: resolvedIndent?.right }; - } - - // Get resolved spacing from style cascade (docDefaults -> paragraph style) - let resolvedSpacing = cloneIfObject(resolvedAsRecord.spacing) as ParagraphSpacing | undefined; - - // Apply table style paragraph properties if present - // Per OOXML spec, table style pPr applies between docDefaults and paragraph style - // But since we can't easily inject into the style resolver, we apply table style - // spacing as a base that can be overridden by explicit paragraph properties - const tableStyleParagraphProps = context.tableStyleParagraphProps; - if (tableStyleParagraphProps?.spacing) { - const tableSpacing = tableStyleParagraphProps.spacing; - - // Only apply table style spacing for properties NOT explicitly set on the paragraph - // This maintains the cascade: table style wins over docDefaults, but paragraph wins over table style - const paragraphHasExplicitSpacing = Boolean(spacing); - - if (!paragraphHasExplicitSpacing) { - // No explicit paragraph spacing - use table style spacing as base, merged with resolved - resolvedSpacing = { - ...resolvedSpacing, - ...tableSpacing, - }; - } else { - // Paragraph has explicit spacing - it should win, but fill in missing values from table style - // This ensures partial paragraph spacing (e.g., only 'line') still gets 'before'/'after' from table style - resolvedSpacing = { - ...tableSpacing, - ...resolvedSpacing, - }; - } - } - - const normalizedAlign = normalizeAlignment(resolvedExtended.justification); - - const hydrated: ParagraphStyleHydration = { - resolved, - spacing: resolvedSpacing, - indent: resolvedIndent, - borders: cloneIfObject(resolvedExtended.borders) as ParagraphAttrs['borders'], - shading: cloneIfObject(resolvedExtended.shading) as ParagraphAttrs['shading'], - alignment: normalizedAlign, - tabStops: cloneIfObject(resolvedExtended.tabStops), - keepLines: resolvedExtended.keepLines, - keepNext: resolvedExtended.keepNext, - numberingProperties: cloneIfObject(resolvedAsRecord.numberingProperties) as Record | undefined, - // Extract contextualSpacing from style resolution - this is a sibling to spacing in OOXML, - // not nested within it. When true, suppresses spacing between paragraphs of the same style. - contextualSpacing: resolvedExtended.contextualSpacing, - }; - return hydrated; -}; - -const cloneIfObject = (value: T): T | undefined => { - if (!value || typeof value !== 'object') return value as T | undefined; - if (Array.isArray(value)) { - return value.map((entry) => (typeof entry === 'object' ? { ...entry } : entry)) as unknown as T; - } - return { ...(value as Record) } as T; -}; - -/** - * Result of hydrating character/run attributes from style resolution. - * - * Contains run-level formatting properties resolved from the OOXML cascade: - * docDefaults (w:rPrDefault) -> Normal style -> paragraph style rPr -> character style -> inline rPr - * - * All font sizes are in OOXML half-points (1pt = 2 half-points). - * Font family is the resolved CSS font-family string. - */ -export type CharacterStyleHydration = { - /** - * Resolved CSS font-family string (e.g., "Calibri, sans-serif"). - * Comes from w:rFonts with theme resolution applied. - */ - fontFamily?: string; - /** - * Font size in OOXML half-points (1pt = 2 half-points). - * Always valid positive number due to fallback cascade in resolveRunProperties. - */ - fontSize: number; - /** - * Text color as hex string (e.g., "FF0000"). - * Extracted from w:color/@w:val (auto values are ignored). - */ - color?: string; - /** Bold formatting. True if w:b is present and not explicitly off. */ - bold?: boolean; - /** Italic formatting. True if w:i is present and not explicitly off. */ - italic?: boolean; - /** Strikethrough formatting. True if w:strike is present and not explicitly off. */ - strike?: boolean; - /** - * Underline formatting with type and optional color. - * Extracted from w:u element. - */ - underline?: { - type?: string; - color?: string; - }; - /** Letter spacing in OOXML twips. Extracted from w:spacing/@w:val. */ - letterSpacing?: number; -}; - -/** - * Builds a CharacterStyleHydration object from resolved run properties. - * - * This function extracts and normalizes character formatting properties from the - * OOXML style cascade into a consistent format for rendering. It handles font - * families with theme resolution, font sizes in half-points, colors, boolean - * formatting flags, underlines, and letter spacing. - * - * @param resolved - The resolved run properties from the OOXML cascade (docDefaults -> styles -> inline) - * @param docx - Optional DOCX context for font family resolution and theme processing - * @returns CharacterStyleHydration object with normalized character formatting properties - * - * @example - * ```typescript - * const resolved = { - * fontFamily: { ascii: 'Calibri', hAnsi: 'Calibri' }, - * fontSize: 22, // 11pt in half-points - * bold: true, - * color: { val: 'FF0000' }, - * }; - * const hydration = buildCharacterStyleHydration(resolved, docx); - * // Returns: { - * // fontFamily: 'Calibri', - * // fontSize: 22, - * // bold: true, - * // color: 'FF0000', - * // } - * ``` - */ -const buildCharacterStyleHydration = ( - resolved: Record, - docx?: Record, -): CharacterStyleHydration => { - const fontFamily = extractFontFamily(resolved.fontFamily, docx); - const fontSize = typeof resolved.fontSize === 'number' ? resolved.fontSize : 20; // Default 10pt - const color = extractColorValue(resolved.color); - const bold = normalizeBooleanProp(resolved.bold); - const italic = normalizeBooleanProp(resolved.italic); - const strike = normalizeBooleanProp(resolved.strike); - const underline = extractUnderline(resolved.underline); - const letterSpacing = typeof resolved.letterSpacing === 'number' ? resolved.letterSpacing : undefined; - - return { - fontFamily, - fontSize, - color, - bold, - italic, - strike, - underline, - letterSpacing, - }; -}; - -/** - * Hydrates character/run-level attributes from the OOXML style cascade. - * - * This function resolves character formatting by calling `resolveRunProperties` from the shared resolver, - * which applies the correct OOXML cascade order: - * 1. Document defaults (w:rPrDefault in w:docDefaults) - * 2. Normal style run properties - * 3. Paragraph style run properties (w:rPr inside paragraph style) - * 4. Numbering level run properties (if applicable) - * - * IMPORTANT: This function does NOT include w:pPr/w:rPr (paragraph-level run properties) in the cascade. - * In OOXML, w:pPr/w:rPr is specifically for: - * - The paragraph mark glyph - * - New text typed at the end of the paragraph by the user - * It is NOT meant to be inherited by existing runs without explicit formatting. - * - * @param para - The ProseMirror paragraph node to hydrate - * @param context - The converter context containing DOCX and optional numbering data - * @param resolvedPpr - Optional pre-resolved paragraph properties (for style chain) - * @returns Hydrated character attributes or null if context is missing or resolution fails - * - * @example - * ```typescript - * const charHydration = hydrateCharacterStyleAttrs(para, converterContext); - * if (charHydration) { - * const fontSizePx = charHydration.fontSize / 2 * (96 / 72); // half-points to px - * const fontFamily = charHydration.fontFamily ?? 'Arial'; - * } - * ``` - */ -export const hydrateCharacterStyleAttrs = ( - para: PMNode, - context?: ConverterContext, - resolvedPpr?: Record, -): CharacterStyleHydration | null => { - if (!hasParagraphStyleContext(context)) { - return null; - } - - const attrs = para.attrs ?? {}; - const paragraphProps = - typeof attrs.paragraphProperties === 'object' && attrs.paragraphProperties !== null - ? (attrs.paragraphProperties as Record) - : {}; - - // Get styleId for paragraph style chain - const styleIdSource = attrs.styleId ?? paragraphProps.styleId; - const styleId = typeof styleIdSource === 'string' && styleIdSource.trim() ? styleIdSource : null; - - // For paragraph-level character defaults, we do NOT use w:pPr/w:rPr as inline properties. - // In OOXML, w:pPr/w:rPr is only for NEW text typed at the paragraph end, not for existing runs. - // Runs without explicit w:rPr should inherit from: docDefaults → Normal → paragraph style rPr. - const inlineRpr: Record = {}; - - // Build resolved paragraph properties for the style chain - // This includes styleId and numberingProperties which affect run property resolution - const pprForChain: Record = resolvedPpr ?? { styleId }; - const numberingProps = attrs.numberingProperties ?? paragraphProps.numberingProperties; - if (numberingProps != null) { - pprForChain.numberingProperties = numberingProps; - } - - const resolverParams = { - docx: context.docx, - numbering: context.numbering ?? EMPTY_NUMBERING_CONTEXT, - }; - - // Call resolveRunProperties to get correctly cascaded character properties - // Cast to bypass JSDoc type mismatch - the JS function actually accepts { docx, numbering } - let resolved: Record | null = null; - try { - resolved = ooxmlResolver.resolveRunProperties( - resolverParams as never, - inlineRpr, - pprForChain, - false, // not list number marker - false, // not numberingDefinedInline - ) as Record; - - // Validate that resolved is a non-null object - if (!resolved || typeof resolved !== 'object') { - return null; - } - } catch { - return null; - } - - return buildCharacterStyleHydration(resolved, context.docx); -}; - -/** - * Hydrates list marker run properties using the OOXML cascade. - * - * This mirrors Word's behavior for numbering markers by resolving: - * docDefaults -> Normal style -> paragraph style rPr -> character style -> inline rPr -> numbering rPr - * - * @param para - The ProseMirror paragraph node to hydrate - * @param context - The converter context containing DOCX and optional numbering data - * @param resolvedPpr - Optional pre-resolved paragraph properties (for style chain) - * @returns Hydrated marker character attributes or null if context is missing or resolution fails - */ -export const hydrateMarkerStyleAttrs = ( - para: PMNode, - context?: ConverterContext, - resolvedPpr?: Record, -): CharacterStyleHydration | null => { - if (!hasParagraphStyleContext(context)) { - return null; - } - - const attrs = para.attrs ?? {}; - const paragraphProps = - typeof attrs.paragraphProperties === 'object' && attrs.paragraphProperties !== null - ? (attrs.paragraphProperties as Record) - : {}; - - const styleIdSource = attrs.styleId ?? paragraphProps.styleId; - const styleId = typeof styleIdSource === 'string' && styleIdSource.trim() ? styleIdSource : null; - - // For list markers, we do NOT use w:pPr/w:rPr as inline properties. - // Marker styling comes from numbering definition rPr, not paragraph's default run properties. - const inlineRpr: Record = {}; - - const numberingProps = attrs.numberingProperties ?? paragraphProps.numberingProperties; - const numberingDefinedInline = (numberingProps as Record | undefined)?.numId != null; - - const pprForChain: Record = resolvedPpr ? { ...resolvedPpr } : { styleId }; - if (styleId && !pprForChain.styleId) { - pprForChain.styleId = styleId; - } - if (numberingProps != null) { - pprForChain.numberingProperties = numberingProps; - } - - const resolverParams = { - docx: context.docx, - numbering: context.numbering ?? EMPTY_NUMBERING_CONTEXT, - }; - - let resolved: Record | null = null; - try { - resolved = ooxmlResolver.resolveRunProperties( - resolverParams as never, - inlineRpr, - pprForChain, - true, - numberingDefinedInline, - ) as Record; - - if (!resolved || typeof resolved !== 'object') { - return null; - } - } catch { - return null; - } - - return buildCharacterStyleHydration(resolved, context.docx); -}; - -/** - * Extracts CSS font-family string from resolved OOXML fontFamily object. - * - * OOXML stores fonts as a structured object with multiple font slots. - * This helper resolves the ascii font (or asciiTheme) and converts it to CSS. - * Non-ascii slots (hAnsi/eastAsia/cs) are not used here. - * - * @param fontFamily - OOXML font family object or undefined - * @returns CSS font-family string (e.g., "Calibri"), or undefined if no font found - * - * @example - * ```typescript - * // Standard OOXML font object - * extractFontFamily({ ascii: 'Calibri', hAnsi: 'Calibri', eastAsia: 'MS Mincho' }) - * // Returns: 'Calibri' - * - * // Invalid input - * extractFontFamily(null) - * // Returns: undefined - * ``` - */ -function extractFontFamily(fontFamily: unknown, docx?: Record): string | undefined { - if (!fontFamily || typeof fontFamily !== 'object') return undefined; - // Cast SuperConverter to access toCssFontFamily (JS static method not typed) - const toCssFontFamily = ( - SuperConverter as { toCssFontFamily?: (fontName: string, docx?: Record) => string } - ).toCssFontFamily; - const resolved = resolveDocxFontFamily(fontFamily as Record, docx ?? null, toCssFontFamily); - return resolved ?? undefined; -} - -/** - * Extracts hex color value from resolved OOXML color object. - * - * OOXML colors are stored as objects with a `val` property containing the hex value: - * `{ val: 'FF0000' }` for red, `{ val: 'auto' }` for automatic color. - * - * This function extracts the color hex string without the `#` prefix, matching OOXML format. - * - * @param color - OOXML color object or undefined - * @returns Hex color string (e.g., "FF0000"), or undefined if invalid/auto - * - * @example - * ```typescript - * // Standard OOXML color - * extractColorValue({ val: 'FF0000' }) - * // Returns: 'FF0000' - * - * // Invalid input - * extractColorValue(null) - * // Returns: undefined - * ``` - */ -function extractColorValue(color: unknown): string | undefined { - if (!color || typeof color !== 'object') return undefined; - const c = color as Record; - const val = c.val; - if (typeof val !== 'string') return undefined; - if (!val || val.toLowerCase() === 'auto') return undefined; - return val; -} - -/** - * Normalizes OOXML boolean toggle properties to JavaScript boolean values. - * - * OOXML boolean properties (w:b, w:i, etc.) can be represented in multiple formats: - * - Boolean: `true` or `false` - * - Number: `1` (true) or `0` (false) - * - String: `'1'`, `'true'`, `'on'` (true) or `'0'`, `'false'`, `'off'` (false) - * - Empty string: `''` (true - OOXML treats absence of value as true for toggle properties) - * - * This function normalizes all valid OOXML boolean representations to JavaScript booleans. - * - * @param value - OOXML boolean value in any valid format - * @returns JavaScript boolean (true/false) or undefined if value is null/undefined - * - * @example - * ```typescript - * normalizeBooleanProp(true) // Returns: true - * normalizeBooleanProp(1) // Returns: true - * normalizeBooleanProp('1') // Returns: true - * normalizeBooleanProp('on') // Returns: true - * normalizeBooleanProp('') // Returns: true (OOXML convention) - * normalizeBooleanProp(false) // Returns: false - * normalizeBooleanProp('0') // Returns: false - * normalizeBooleanProp(null) // Returns: undefined - * ``` - */ -function normalizeBooleanProp(value: unknown): boolean | undefined { - if (value == null) return undefined; - if (typeof value === 'boolean') return value; - if (typeof value === 'number') return value !== 0; - if (typeof value === 'string') { - const lower = value.toLowerCase(); - if (lower === '0' || lower === 'false' || lower === 'off') return false; - if (lower === '1' || lower === 'true' || lower === 'on' || lower === '') return true; - } - return Boolean(value); -} - -/** - * Extracts underline properties from resolved OOXML underline object. - * - * OOXML underlines are stored as objects with type and optional color: - * - `w:val` or `type`: Underline style (single, double, thick, dotted, etc.) - * - `w:color` or `color`: Hex color string (optional) - * - * Valid underline types include: single, double, thick, dotted, dash, dotDash, dotDotDash, wave, etc. - * The special value "none" is treated as no underline (returns undefined). - * - * @param underline - OOXML underline object or undefined - * @returns Underline object with type and optional color, or undefined if no underline - * - * @example - * ```typescript - * // Standard single underline - * extractUnderline({ 'w:val': 'single' }) - * // Returns: { type: 'single', color: undefined } - * - * // Double underline with color - * extractUnderline({ type: 'double', color: 'FF0000' }) - * // Returns: { type: 'double', color: 'FF0000' } - * - * // No underline - * extractUnderline({ 'w:val': 'none' }) - * // Returns: undefined - * - * // Invalid input - * extractUnderline(null) - * // Returns: undefined - * ``` - */ -function extractUnderline(underline: unknown): CharacterStyleHydration['underline'] | undefined { - if (!underline || typeof underline !== 'object') return undefined; - const u = underline as Record; - const type = u['w:val'] ?? u.type ?? u.val; - if (typeof type !== 'string' || type === 'none') return undefined; - const color = u['w:color'] ?? u.color; - return { - type, - color: typeof color === 'string' ? color : undefined, - }; -} diff --git a/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts b/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts index 06690f04cc..01dfcd79ae 100644 --- a/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts +++ b/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts @@ -8,7 +8,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { - paragraphToFlowBlocks, + paragraphToFlowBlocks as baseParagraphToFlowBlocks, mergeAdjacentRuns, dataAttrsCompatible, commentsCompatible, @@ -22,6 +22,8 @@ import type { TrackedChangesConfig, HyperlinkConfig, StyleContext, + ThemeColorPalette, + NestedConverters, } from '../types.js'; import type { ConverterContext } from '../converter-context.js'; import type { Run, TextRun, FlowBlock, ParagraphBlock, TrackedChangeMeta, ImageRun } from '@superdoc/contracts'; @@ -79,6 +81,76 @@ import { applyTrackedChangesModeToRuns, } from '../tracked-changes.js'; +const DEFAULT_HYPERLINK_CONFIG: HyperlinkConfig = { enableRichHyperlinks: false }; +let defaultConverterContext: ConverterContext = { + translatedNumbering: {}, + translatedLinkedStyles: { + docDefaults: { + runProperties: {}, + paragraphProperties: {}, + }, + styles: {}, + }, +}; + +const isConverters = (value: unknown): value is NestedConverters => { + if (!value || typeof value !== 'object') return false; + return ( + 'paragraphToFlowBlocks' in value || + 'tableNodeToBlock' in value || + 'imageNodeToBlock' in value || + 'contentBlockNodeToDrawingBlock' in value || + 'vectorShapeNodeToDrawingBlock' in value || + 'shapeGroupNodeToDrawingBlock' in value || + 'shapeContainerNodeToDrawingBlock' in value || + 'shapeTextboxNodeToDrawingBlock' in value + ); +}; + +const paragraphToFlowBlocks = ( + para: PMNode, + nextBlockId: BlockIdGenerator, + positions: PositionMap, + defaultFont: string, + defaultSize: number, + styleContext: StyleContext, + trackedChangesConfig?: TrackedChangesConfig, + bookmarks?: Map, + hyperlinkConfig?: HyperlinkConfig, + themeColors?: ThemeColorPalette, + converterContextOrConverters?: ConverterContext | NestedConverters, + maybeConverters?: NestedConverters, +) => { + let converterContext: ConverterContext | undefined; + let converters: NestedConverters | undefined; + + if (isConverters(maybeConverters)) { + converters = maybeConverters; + } + + if (isConverters(converterContextOrConverters)) { + converters = converterContextOrConverters; + } else if (converterContextOrConverters) { + converterContext = converterContextOrConverters as ConverterContext; + } + + return baseParagraphToFlowBlocks({ + para, + nextBlockId, + positions, + defaultFont, + defaultSize, + styleContext, + trackedChangesConfig, + bookmarks, + hyperlinkConfig: hyperlinkConfig ?? DEFAULT_HYPERLINK_CONFIG, + themeColors, + converters: converters as NestedConverters, + converterContext: converterContext ?? defaultConverterContext, + enableComments: true, + }); +}; + describe('paragraph converters', () => { describe('mergeAdjacentRuns', () => { it('should return empty array unchanged', () => { @@ -626,6 +698,7 @@ describe('paragraph converters', () => { styles: {}, }, }; + defaultConverterContext = converterContext; // Setup default mock returns vi.mocked(computeParagraphAttrs).mockReturnValue({ paragraphAttrs: {}, resolvedParagraphProperties: {} }); @@ -1899,16 +1972,17 @@ describe('paragraph converters', () => { ); expect(converters.tableNodeToBlock).toHaveBeenCalledWith( - tableNode, - nextBlockId, - positions, - 'Arial', - 16, - styleContext, - trackedChanges, - bookmarks, - hyperlinkConfig, - undefined, + expect.objectContaining({ + node: tableNode, + nextBlockId, + positions, + defaultFont: 'Arial', + defaultSize: 16, + styleContext, + trackedChangesConfig: trackedChanges, + bookmarks, + hyperlinkConfig, + }), ); expect(blocks.some((b) => b.kind === 'table')).toBe(true); }); diff --git a/packages/layout-engine/pm-adapter/src/converters/paragraph.ts b/packages/layout-engine/pm-adapter/src/converters/paragraph.ts index d12913a3f2..111be4564d 100644 --- a/packages/layout-engine/pm-adapter/src/converters/paragraph.ts +++ b/packages/layout-engine/pm-adapter/src/converters/paragraph.ts @@ -13,23 +13,11 @@ import type { Run, TextRun, ImageRun, - ImageBlock, - TrackedChangeMeta, SdtMetadata, FieldAnnotationRun, FieldAnnotationMetadata, } from '@superdoc/contracts'; -import type { - PMNode, - PMMark, - BlockIdGenerator, - PositionMap, - StyleContext, - TrackedChangesConfig, - HyperlinkConfig, - NodeHandlerContext, - ThemeColorPalette, -} from '../types.js'; +import type { PMNode, PMMark, PositionMap, NodeHandlerContext, ParagraphToFlowBlocksParams } from '../types.js'; import type { ConverterContext } from '../converter-context.js'; import { computeParagraphAttrs, deepClone } from '../attributes/index.js'; import { resolveNodeSdtMetadata, getNodeInstruction } from '../sdt/index.js'; @@ -43,7 +31,7 @@ import { import { textNodeToRun, tabNodeToRun, tokenNodeToRun } from './text-run.js'; import { contentBlockNodeToDrawingBlock } from './content-block.js'; import { DEFAULT_HYPERLINK_CONFIG, TOKEN_INLINE_TYPES } from '../constants.js'; -import { ptToPx, pickNumber, isPlainObject, twipsToPx } from '../utilities.js'; +import { pickNumber, isPlainObject } from '../utilities.js'; import { computeRunAttrs } from '../attributes/paragraph.js'; import { resolveRunProperties } from '@superdoc/style-engine/ooxml'; @@ -533,68 +521,21 @@ const applyInlineRunProperties = ( * @param enableComments - Whether to include comment marks in the output (defaults to true). Set to false for viewing modes where comments should be hidden. * @returns Array of FlowBlocks (paragraphs, images, drawings, page breaks, etc.) */ -export function paragraphToFlowBlocks( - para: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - styleContext: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig: HyperlinkConfig = DEFAULT_HYPERLINK_CONFIG, - themeColors?: ThemeColorPalette, - // Converter dependencies injected to avoid circular imports - converters?: { - contentBlockNodeToDrawingBlock?: ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - ) => FlowBlock | null; - imageNodeToBlock: ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - trackedMeta?: TrackedChangeMeta, - trackedChanges?: TrackedChangesConfig, - ) => ImageBlock | null; - vectorShapeNodeToDrawingBlock: ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - ) => FlowBlock | null; - shapeGroupNodeToDrawingBlock: ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - ) => FlowBlock | null; - shapeContainerNodeToDrawingBlock: ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - ) => FlowBlock | null; - shapeTextboxNodeToDrawingBlock: ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - ) => FlowBlock | null; - tableNodeToBlock: ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - styleContext: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig?: HyperlinkConfig, - themeColors?: ThemeColorPalette, - converterContext?: ConverterContext, - ) => FlowBlock | null; - }, - converterContext?: ConverterContext, +export function paragraphToFlowBlocks({ + para, + nextBlockId, + positions, + defaultFont, + defaultSize, + styleContext, + trackedChangesConfig, + bookmarks, + hyperlinkConfig = DEFAULT_HYPERLINK_CONFIG, + themeColors, + converters, + converterContext, enableComments = true, -): FlowBlock[] { +}: ParagraphToFlowBlocksParams): FlowBlock[] { const paragraphProps = typeof para.attrs?.paragraphProperties === 'object' && para.attrs.paragraphProperties !== null ? (para.attrs.paragraphProperties as ParagraphProperties) @@ -1033,14 +974,14 @@ export function paragraphToFlowBlocks( const anchorParagraphId = nextId(); flushParagraph(); const mergedMarks = [...(node.marks ?? []), ...(inheritedMarks ?? [])]; - const trackedMeta = trackedChanges?.enabled ? collectTrackedChangeFromMarks(mergedMarks) : undefined; - if (shouldHideTrackedNode(trackedMeta, trackedChanges)) { + const trackedMeta = trackedChangesConfig?.enabled ? collectTrackedChangeFromMarks(mergedMarks) : undefined; + if (shouldHideTrackedNode(trackedMeta, trackedChangesConfig)) { return; } if (converters?.imageNodeToBlock) { - const imageBlock = converters.imageNodeToBlock(node, nextBlockId, positions, trackedMeta, trackedChanges); + const imageBlock = converters.imageNodeToBlock(node, nextBlockId, positions, trackedMeta, trackedChangesConfig); if (imageBlock && imageBlock.kind === 'image') { - annotateBlockWithTrackedChange(imageBlock, trackedMeta, trackedChanges); + annotateBlockWithTrackedChange(imageBlock, trackedMeta, trackedChangesConfig); blocks.push(attachAnchorParagraphId(imageBlock, anchorParagraphId)); } } @@ -1157,19 +1098,21 @@ export function paragraphToFlowBlocks( const anchorParagraphId = nextId(); flushParagraph(); if (converters?.tableNodeToBlock) { - const tableBlock = converters.tableNodeToBlock( + const tableBlock = converters.tableNodeToBlock({ node, nextBlockId, positions, defaultFont, defaultSize, styleContext, - trackedChanges, + trackedChangesConfig, bookmarks, hyperlinkConfig, themeColors, - ...(converterContext !== undefined ? [converterContext] : []), - ); + converterContext, + converters, + enableComments, + }); if (tableBlock) { blocks.push(attachAnchorParagraphId(tableBlock, anchorParagraphId)); } @@ -1258,7 +1201,7 @@ export function paragraphToFlowBlocks( } }); - if (!trackedChanges) { + if (!trackedChangesConfig) { return blocks; } @@ -1270,20 +1213,20 @@ export function paragraphToFlowBlocks( } const filteredRuns = applyTrackedChangesModeToRuns( block.runs, - trackedChanges, + trackedChangesConfig, hyperlinkConfig, applyMarksToRun, themeColors, enableComments, ); - if (trackedChanges.enabled && filteredRuns.length === 0) { + if (trackedChangesConfig.enabled && filteredRuns.length === 0) { return; } block.runs = filteredRuns; block.attrs = { ...(block.attrs ?? {}), - trackedChangesMode: trackedChanges.mode, - trackedChangesEnabled: trackedChanges.enabled, + trackedChangesMode: trackedChangesConfig.mode, + trackedChangesEnabled: trackedChangesConfig.enabled, }; processedBlocks.push(block); }); @@ -1313,6 +1256,9 @@ export function handleParagraphNode(node: PMNode, context: NodeHandlerContext): hyperlinkConfig, sectionState, converters, + converterContext, + themeColors, + enableComments, } = context; const { ranges: sectionRanges, currentSectionIndex, currentParagraphIndex } = sectionState; @@ -1331,13 +1277,10 @@ export function handleParagraphNode(node: PMNode, context: NodeHandlerContext): } } - const paragraphToFlowBlocks = converters?.paragraphToFlowBlocks; - if (!paragraphToFlowBlocks) { - return; - } + const paragraphToFlowBlocks = converters.paragraphToFlowBlocks; - const paragraphBlocks = paragraphToFlowBlocks( - node, + const paragraphBlocks = paragraphToFlowBlocks({ + para: node, nextBlockId, positions, defaultFont, @@ -1346,9 +1289,11 @@ export function handleParagraphNode(node: PMNode, context: NodeHandlerContext): trackedChangesConfig, bookmarks, hyperlinkConfig, - undefined, // themeColors - not available in NodeHandlerContext - context.converterContext, - ); + themeColors, + converterContext, + converters, + enableComments, + }); paragraphBlocks.forEach((block) => { blocks.push(block); recordBlockKind(block.kind); diff --git a/packages/layout-engine/pm-adapter/src/converters/table.test.ts b/packages/layout-engine/pm-adapter/src/converters/table.test.ts index 19c7504115..7380838283 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.test.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.test.ts @@ -3,11 +3,64 @@ */ import { describe, it, expect, vi } from 'vitest'; -import { tableNodeToBlock, handleTableNode } from './table.js'; -import type { PMNode, BlockIdGenerator, PositionMap, StyleContext } from '../types.js'; +import { tableNodeToBlock as baseTableNodeToBlock, handleTableNode } from './table.js'; +import type { + PMNode, + BlockIdGenerator, + PositionMap, + StyleContext, + TrackedChangesConfig, + HyperlinkConfig, + ThemeColorPalette, + NestedConverters, +} from '../types.js'; +import type { ConverterContext } from '../converter-context.js'; import type { FlowBlock, ParagraphBlock, TableBlock, ImageBlock } from '@superdoc/contracts'; import { twipsToPx } from '../utilities.js'; +const DEFAULT_HYPERLINK_CONFIG: HyperlinkConfig = { enableRichHyperlinks: false }; +const DEFAULT_CONVERTER_CONTEXT: ConverterContext = { + translatedNumbering: {}, + translatedLinkedStyles: { + docDefaults: {}, + latentStyles: {}, + styles: {}, + }, +}; + +const tableNodeToBlock = ( + node: PMNode, + nextBlockId: BlockIdGenerator, + positions: PositionMap, + defaultFont: string, + defaultSize: number, + styleContext: StyleContext, + trackedChangesConfig?: TrackedChangesConfig, + bookmarks?: Map, + hyperlinkConfig?: HyperlinkConfig, + themeColors?: ThemeColorPalette, + paragraphToFlowBlocks?: NestedConverters['paragraphToFlowBlocks'], + converterContext?: ConverterContext, +) => { + const converters = paragraphToFlowBlocks ? ({ paragraphToFlowBlocks } as NestedConverters) : ({} as NestedConverters); + + return baseTableNodeToBlock({ + node, + nextBlockId, + positions, + defaultFont, + defaultSize, + styleContext, + trackedChangesConfig, + bookmarks, + hyperlinkConfig: hyperlinkConfig ?? DEFAULT_HYPERLINK_CONFIG, + themeColors, + converterContext: converterContext ?? DEFAULT_CONVERTER_CONTEXT, + converters, + enableComments: true, + }); +}; + describe('table converter', () => { const mockStyleContext: StyleContext = { defaults: { @@ -21,12 +74,12 @@ describe('table converter', () => { const mockBlockIdGenerator: BlockIdGenerator = vi.fn((kind) => `test-${kind}`); const mockPositionMap: PositionMap = new Map(); - const mockParagraphConverter = vi.fn((node) => { + const mockParagraphConverter = vi.fn((params) => { return [ { kind: 'paragraph', id: 'p1', - runs: [{ text: node.content?.[0]?.text || 'text', fontFamily: 'Arial', fontSize: 12 }], + runs: [{ text: params.para.content?.[0]?.text || 'text', fontFamily: 'Arial', fontSize: 12 }], } as ParagraphBlock, ]; }); @@ -228,10 +281,9 @@ describe('table converter', () => { const converterContext = { docx: { foo: 'bar' } } as never; - const paragraphSpy = vi.fn((para, ...args) => { - const [, , , , , , , , , passedConverterContext] = args; - expect(passedConverterContext).toBe(converterContext); - return mockParagraphConverter(para); + const paragraphSpy = vi.fn((params) => { + expect(params.converterContext).toBe(converterContext); + return mockParagraphConverter(params); }); const result = tableNodeToBlock( @@ -272,26 +324,24 @@ describe('table converter', () => { const imageBlock: ImageBlock = { kind: 'image', id: 'image-1', src: 'image.png' }; const imageConverter = vi.fn().mockReturnValue(imageBlock); - const result = tableNodeToBlock( + const result = baseTableNodeToBlock({ node, - mockBlockIdGenerator, - mockPositionMap, - 'Arial', - 16, - mockStyleContext, - undefined, - undefined, - undefined, - undefined, - mockParagraphConverter, - undefined, - { - converters: { - imageNodeToBlock: imageConverter, - paragraphToFlowBlocks: mockParagraphConverter, - }, - }, - ) as TableBlock; + nextBlockId: mockBlockIdGenerator, + positions: mockPositionMap, + defaultFont: 'Arial', + defaultSize: 16, + styleContext: mockStyleContext, + trackedChangesConfig: undefined, + bookmarks: undefined, + hyperlinkConfig: DEFAULT_HYPERLINK_CONFIG, + themeColors: undefined, + converterContext: DEFAULT_CONVERTER_CONTEXT, + converters: { + paragraphToFlowBlocks: mockParagraphConverter, + imageNodeToBlock: imageConverter, + } as NestedConverters, + enableComments: true, + }) as TableBlock; expect(imageConverter).toHaveBeenCalled(); expect(result.rows[0].cells[0].blocks?.[0]).toBe(imageBlock); @@ -327,25 +377,23 @@ describe('table converter', () => { } as ParagraphBlock, ]); - const result = tableNodeToBlock( + const result = baseTableNodeToBlock({ node, - mockBlockIdGenerator, - mockPositionMap, - 'Arial', - 16, - mockStyleContext, - undefined, - undefined, - undefined, - undefined, - paragraphConverter, - undefined, - { - converters: { - paragraphToFlowBlocks: paragraphConverter, - }, - }, - ) as TableBlock; + nextBlockId: mockBlockIdGenerator, + positions: mockPositionMap, + defaultFont: 'Arial', + defaultSize: 16, + styleContext: mockStyleContext, + trackedChangesConfig: undefined, + bookmarks: undefined, + hyperlinkConfig: DEFAULT_HYPERLINK_CONFIG, + themeColors: undefined, + converterContext: DEFAULT_CONVERTER_CONTEXT, + converters: { + paragraphToFlowBlocks: paragraphConverter, + } as NestedConverters, + enableComments: true, + }) as TableBlock; const cellBlocks = result.rows[0].cells[0].blocks ?? []; expect(cellBlocks[0]?.kind).toBe('paragraph'); @@ -402,26 +450,24 @@ describe('table converter', () => { const tableConverter = vi.fn().mockReturnValue(nestedTableBlock); - const result = tableNodeToBlock( + const result = baseTableNodeToBlock({ node, - mockBlockIdGenerator, - mockPositionMap, - 'Arial', - 16, - mockStyleContext, - undefined, - undefined, - undefined, - undefined, - mockParagraphConverter, - undefined, - { - converters: { - paragraphToFlowBlocks: mockParagraphConverter, - tableNodeToBlock: tableConverter, - }, - }, - ) as TableBlock; + nextBlockId: mockBlockIdGenerator, + positions: mockPositionMap, + defaultFont: 'Arial', + defaultSize: 16, + styleContext: mockStyleContext, + trackedChangesConfig: undefined, + bookmarks: undefined, + hyperlinkConfig: DEFAULT_HYPERLINK_CONFIG, + themeColors: undefined, + converterContext: DEFAULT_CONVERTER_CONTEXT, + converters: { + paragraphToFlowBlocks: mockParagraphConverter, + tableNodeToBlock: tableConverter, + } as NestedConverters, + enableComments: true, + }) as TableBlock; const cellBlocks = result.rows[0].cells[0].blocks ?? []; const nestedTable = cellBlocks.find((block) => block.kind === 'table') as TableBlock | undefined; @@ -1138,7 +1184,7 @@ describe('table converter', () => { expect(mockConverter).toHaveBeenCalled(); // Verify tracked changes config was passed const callArgs = mockConverter.mock.calls[0]; - expect(callArgs[6]).toEqual(trackedChangesConfig); + expect(callArgs[0].trackedChangesConfig).toEqual(trackedChangesConfig); }); it('returns null when all rows have no cells', () => { diff --git a/packages/layout-engine/pm-adapter/src/converters/table.ts b/packages/layout-engine/pm-adapter/src/converters/table.ts index 1a96de9992..20bf7603d5 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.ts @@ -29,8 +29,8 @@ import type { HyperlinkConfig, ThemeColorPalette, ConverterContext, - TableNodeToBlockOptions, NestedConverters, + TableNodeToBlockParams, } from '../types.js'; import { extractTableBorders, extractCellBorders, extractCellPadding } from '../attributes/index.js'; import { pickNumber, twipsToPx } from '../utilities.js'; @@ -44,33 +44,19 @@ import { } from '../sdt/index.js'; import { TableProperties } from '@superdoc/style-engine/ooxml'; -type ParagraphConverter = ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - styleContext: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig?: HyperlinkConfig, - themeColors?: ThemeColorPalette, - converterContext?: ConverterContext, -) => FlowBlock[]; - type TableParserDependencies = { nextBlockId: BlockIdGenerator; positions: PositionMap; defaultFont: string; defaultSize: number; styleContext: StyleContext; - trackedChanges?: TrackedChangesConfig; + trackedChangesConfig?: TrackedChangesConfig; bookmarks?: Map; - hyperlinkConfig?: HyperlinkConfig; + hyperlinkConfig: HyperlinkConfig; themeColors?: ThemeColorPalette; - paragraphToFlowBlocks?: ParagraphConverter; - converterContext?: ConverterContext; - converters?: NestedConverters; + converterContext: ConverterContext; + converters: NestedConverters; + enableComments: boolean; }; type ParseTableCellArgs = { @@ -220,7 +206,7 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { // Create enhanced converter context with table style paragraph props for the style cascade // This allows paragraphs inside table cells to inherit table style's pPr // Also includes backgroundColor for auto text color resolution - const cellConverterContext: ConverterContext | undefined = + const cellConverterContext: ConverterContext = tableProperties || cellBackgroundColor ? ({ ...context.converterContext, @@ -229,7 +215,7 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { } as ConverterContext) : context.converterContext; - const paragraphToFlowBlocks = context.converters?.paragraphToFlowBlocks ?? context.paragraphToFlowBlocks; + const paragraphToFlowBlocks = context.converters.paragraphToFlowBlocks; const tableNodeToBlock = context.converters?.tableNodeToBlock; /** @@ -261,19 +247,21 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { for (const childNode of cellNode.content) { if (childNode.type === 'paragraph') { if (!paragraphToFlowBlocks) continue; - const paragraphBlocks = paragraphToFlowBlocks( - childNode, - context.nextBlockId, - context.positions, - context.defaultFont, - context.defaultSize, - context.styleContext, - context.trackedChanges, - context.bookmarks, - context.hyperlinkConfig, - context.themeColors, - cellConverterContext, - ); + const paragraphBlocks = paragraphToFlowBlocks({ + para: childNode, + nextBlockId: context.nextBlockId, + positions: context.positions, + defaultFont: context.defaultFont, + defaultSize: context.defaultSize, + styleContext: context.styleContext, + trackedChangesConfig: context.trackedChangesConfig, + bookmarks: context.bookmarks, + hyperlinkConfig: context.hyperlinkConfig, + themeColors: context.themeColors, + converterContext: cellConverterContext, + converters: context.converters, + enableComments: context.enableComments, + }); appendParagraphBlocks(paragraphBlocks); continue; } @@ -283,38 +271,40 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { for (const nestedNode of childNode.content) { if (nestedNode.type === 'paragraph') { if (!paragraphToFlowBlocks) continue; - const paragraphBlocks = paragraphToFlowBlocks( - nestedNode, - context.nextBlockId, - context.positions, - context.defaultFont, - context.defaultSize, - context.styleContext, - context.trackedChanges, - context.bookmarks, - context.hyperlinkConfig, - context.themeColors, - cellConverterContext, - ); + const paragraphBlocks = paragraphToFlowBlocks({ + para: nestedNode, + nextBlockId: context.nextBlockId, + positions: context.positions, + defaultFont: context.defaultFont, + defaultSize: context.defaultSize, + styleContext: context.styleContext, + trackedChangesConfig: context.trackedChangesConfig, + bookmarks: context.bookmarks, + hyperlinkConfig: context.hyperlinkConfig, + themeColors: context.themeColors, + converterContext: cellConverterContext, + converters: context.converters, + enableComments: context.enableComments, + }); appendParagraphBlocks(paragraphBlocks, structuredContentMetadata); continue; } if (nestedNode.type === 'table' && tableNodeToBlock) { - const tableBlock = tableNodeToBlock( - nestedNode, - context.nextBlockId, - context.positions, - context.defaultFont, - context.defaultSize, - context.styleContext, - context.trackedChanges, - context.bookmarks, - context.hyperlinkConfig, - context.themeColors, - paragraphToFlowBlocks, - context.converterContext, - { converters: context.converters }, - ); + const tableBlock = tableNodeToBlock({ + node: nestedNode, + nextBlockId: context.nextBlockId, + positions: context.positions, + defaultFont: context.defaultFont, + defaultSize: context.defaultSize, + styleContext: context.styleContext, + trackedChangesConfig: context.trackedChangesConfig, + bookmarks: context.bookmarks, + hyperlinkConfig: context.hyperlinkConfig, + themeColors: context.themeColors, + converterContext: context.converterContext, + converters: context.converters, + enableComments: context.enableComments, + }); if (tableBlock && tableBlock.kind === 'table') { applySdtMetadataToTableBlock(tableBlock, structuredContentMetadata); blocks.push(tableBlock); @@ -326,21 +316,21 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { } if (childNode.type === 'table' && tableNodeToBlock) { - const tableBlock = tableNodeToBlock( - childNode, - context.nextBlockId, - context.positions, - context.defaultFont, - context.defaultSize, - context.styleContext, - context.trackedChanges, - context.bookmarks, - context.hyperlinkConfig, - context.themeColors, - paragraphToFlowBlocks, - context.converterContext, - { converters: context.converters }, - ); + const tableBlock = tableNodeToBlock({ + node: childNode, + nextBlockId: context.nextBlockId, + positions: context.positions, + defaultFont: context.defaultFont, + defaultSize: context.defaultSize, + styleContext: context.styleContext, + trackedChangesConfig: context.trackedChangesConfig, + bookmarks: context.bookmarks, + hyperlinkConfig: context.hyperlinkConfig, + themeColors: context.themeColors, + converterContext: context.converterContext, + converters: context.converters, + enableComments: context.enableComments, + }); if (tableBlock && tableBlock.kind === 'table') { blocks.push(tableBlock); } @@ -349,8 +339,8 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { if (childNode.type === 'image' && context.converters?.imageNodeToBlock) { const mergedMarks = [...(childNode.marks ?? [])]; - const trackedMeta = context.trackedChanges ? collectTrackedChangeFromMarks(mergedMarks) : undefined; - if (shouldHideTrackedNode(trackedMeta, context.trackedChanges)) { + const trackedMeta = context.trackedChangesConfig ? collectTrackedChangeFromMarks(mergedMarks) : undefined; + if (shouldHideTrackedNode(trackedMeta, context.trackedChangesConfig)) { continue; } const imageBlock = context.converters.imageNodeToBlock( @@ -358,10 +348,10 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { context.nextBlockId, context.positions, trackedMeta, - context.trackedChanges, + context.trackedChangesConfig, ); if (imageBlock && imageBlock.kind === 'image') { - annotateBlockWithTrackedChange(imageBlock, trackedMeta, context.trackedChanges); + annotateBlockWithTrackedChange(imageBlock, trackedMeta, context.trackedChangesConfig); blocks.push(imageBlock); } continue; @@ -680,35 +670,23 @@ function extractFloatingTableAnchorWrap(node: PMNode): { anchor?: TableAnchor; w * @param paragraphToFlowBlocks - Paragraph converter function (injected to avoid circular deps) * @returns TableBlock or null if conversion fails */ -export function tableNodeToBlock( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - _styleContext: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig?: HyperlinkConfig, - themeColors?: ThemeColorPalette, - paragraphToFlowBlocks?: ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - styleContext: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig?: HyperlinkConfig, - themeColors?: ThemeColorPalette, - converterContext?: ConverterContext, - ) => FlowBlock[], - converterContext?: ConverterContext, - options?: TableNodeToBlockOptions, -): FlowBlock | null { +export function tableNodeToBlock({ + node, + nextBlockId, + positions, + defaultFont, + defaultSize, + styleContext, + trackedChangesConfig, + bookmarks, + hyperlinkConfig, + themeColors, + converterContext, + converters, + enableComments, +}: TableNodeToBlockParams): FlowBlock | null { if (!Array.isArray(node.content) || node.content.length === 0) return null; - const paragraphConverter = paragraphToFlowBlocks ?? options?.converters?.paragraphToFlowBlocks; + const paragraphConverter = converters.paragraphToFlowBlocks; if (!paragraphConverter) return null; const parserDeps: TableParserDependencies = { @@ -716,14 +694,14 @@ export function tableNodeToBlock( positions, defaultFont, defaultSize, - styleContext: _styleContext, - trackedChanges, + styleContext, + trackedChangesConfig, bookmarks, hyperlinkConfig, themeColors, - paragraphToFlowBlocks: paragraphConverter, converterContext, - converters: options?.converters, + converters, + enableComments, }; const hydratedTableStyle = hydrateTableStyleAttrs(node, converterContext); @@ -908,9 +886,10 @@ export function handleTableNode(node: PMNode, context: NodeHandlerContext): void hyperlinkConfig, converters, converterContext, + enableComments, } = context; - const tableBlock = tableNodeToBlock( + const tableBlock = tableNodeToBlock({ node, nextBlockId, positions, @@ -920,11 +899,11 @@ export function handleTableNode(node: PMNode, context: NodeHandlerContext): void trackedChangesConfig, bookmarks, hyperlinkConfig, - undefined, // themeColors - converters?.paragraphToFlowBlocks, + themeColors: undefined, converterContext, - { converters }, - ); + converters, + enableComments, + }); if (tableBlock) { blocks.push(tableBlock); recordBlockKind(tableBlock.kind); diff --git a/packages/layout-engine/pm-adapter/src/index.d.ts b/packages/layout-engine/pm-adapter/src/index.d.ts index 1d90c62aa8..7203f384a8 100644 --- a/packages/layout-engine/pm-adapter/src/index.d.ts +++ b/packages/layout-engine/pm-adapter/src/index.d.ts @@ -31,4 +31,4 @@ export type { FlowBlocksResult, } from './types.js'; export { SectionType } from './types.js'; -export { toFlowBlocks, toFlowBlocksMap } from './internal.js'; +export { toFlowBlocks } from './internal.js'; diff --git a/packages/layout-engine/pm-adapter/src/index.test.ts b/packages/layout-engine/pm-adapter/src/index.test.ts index a84adce63f..1b3b6ccf1d 100644 --- a/packages/layout-engine/pm-adapter/src/index.test.ts +++ b/packages/layout-engine/pm-adapter/src/index.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest'; -import { toFlowBlocks as baseToFlowBlocks, toFlowBlocksMap as baseToFlowBlocksMap } from './index.js'; -import type { PMNode, PMMark, AdapterOptions, BatchAdapterOptions, PMDocumentMap } from './index.js'; +import { toFlowBlocks as baseToFlowBlocks } from './index.js'; +import type { PMNode, PMMark, AdapterOptions } from './index.js'; import type { FlowBlock, ImageBlock, TableBlock } from '@superdoc/contracts'; import basicParagraphFixture from './fixtures/basic-paragraph.json'; import edgeCasesFixture from './fixtures/edge-cases.json'; @@ -25,9 +25,6 @@ const DEFAULT_CONVERTER_CONTEXT = { const toFlowBlocks = (pmDoc: PMNode | object, options: AdapterOptions = {}) => baseToFlowBlocks(pmDoc, { converterContext: DEFAULT_CONVERTER_CONTEXT, ...options }); -const toFlowBlocksMap = (docs: PMDocumentMap, options: BatchAdapterOptions = {}) => - baseToFlowBlocksMap(docs, { converterContext: DEFAULT_CONVERTER_CONTEXT, ...options }); - const createTestBodySectPr = () => ({ type: 'element', name: 'w:sectPr', @@ -1208,42 +1205,6 @@ describe('toFlowBlocks', () => { }); }); - describe('batch conversion', () => { - it('converts a map of documents with unique prefixes', () => { - const docs = { - default: { - type: 'doc', - content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Default' }] }], - }, - first: { - type: 'doc', - content: [{ type: 'paragraph', content: [{ type: 'text', text: 'First' }] }], - }, - empty: null, - }; - - const result = toFlowBlocksMap(docs, { - blockIdPrefixFactory: (key) => `header-${key}-`, - }); - - expect(Object.keys(result)).toEqual(['default', 'first']); - expect(result.default[0].id.startsWith('header-default-')).toBe(true); - expect(result.first[0].id.startsWith('header-first-')).toBe(true); - }); - - it('falls back to key-based prefixes when factory is absent', () => { - const docs = { - default: { - type: 'doc', - content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Default' }] }], - }, - }; - - const result = toFlowBlocksMap(docs); - expect(result.default[0].id.startsWith('default-')).toBe(true); - }); - }); - it('populates pm ranges on runs', () => { const pmDoc = { type: 'doc', diff --git a/packages/layout-engine/pm-adapter/src/index.ts b/packages/layout-engine/pm-adapter/src/index.ts index 2d14f7c312..8294f80af7 100644 --- a/packages/layout-engine/pm-adapter/src/index.ts +++ b/packages/layout-engine/pm-adapter/src/index.ts @@ -31,11 +31,11 @@ export type { PMDocumentMap, BatchAdapterOptions, FlowBlocksResult, - ConverterContext + ConverterContext, } from './types.js'; // Re-export enum as value export { SectionType } from './types.js'; // Re-export public API functions from internal implementation -export { toFlowBlocks, toFlowBlocksMap } from './internal.js'; +export { toFlowBlocks } from './internal.js'; diff --git a/packages/layout-engine/pm-adapter/src/internal.test.ts b/packages/layout-engine/pm-adapter/src/internal.test.ts index 093882604f..144c14c437 100644 --- a/packages/layout-engine/pm-adapter/src/internal.test.ts +++ b/packages/layout-engine/pm-adapter/src/internal.test.ts @@ -4,12 +4,11 @@ * Tests cover: * - nodeHandlers dispatch map * - toFlowBlocks() main conversion function - * - toFlowBlocksMap() batch converter * - paragraphToFlowBlocks() wrapper function */ import { describe, it, expect, beforeEach, vi } from 'vitest'; -import { toFlowBlocks, toFlowBlocksMap, nodeHandlers } from './internal.js'; +import { toFlowBlocks, nodeHandlers } from './internal.js'; import type { PMNode, AdapterOptions, BatchAdapterOptions, PMDocumentMap } from './types.js'; // Mock all external dependencies @@ -812,191 +811,6 @@ describe('internal', () => { }); }); - describe('toFlowBlocksMap', () => { - it('should return empty object for empty documents map', () => { - const result = toFlowBlocksMap({}); - expect(result).toEqual({}); - }); - - it('should return empty object for null documents', () => { - const result = toFlowBlocksMap(null as never); - expect(result).toEqual({}); - }); - - it('should return empty object for undefined documents', () => { - const result = toFlowBlocksMap(undefined as never); - expect(result).toEqual({}); - }); - - it('should convert single document', () => { - const documents: PMDocumentMap = { - doc1: { - type: 'doc', - content: [{ type: 'paragraph', content: [] }], - }, - }; - - vi.mocked(handleParagraphNode).mockImplementationOnce((node, context) => { - context.blocks.push({ kind: 'paragraph', id: '0-paragraph' } as never); - }); - - const result = toFlowBlocksMap(documents); - - expect(result).toHaveProperty('doc1'); - expect(Array.isArray(result.doc1)).toBe(true); - expect(result.doc1!.length).toBeGreaterThan(0); - }); - - it('should convert multiple documents', () => { - const documents: PMDocumentMap = { - header: { type: 'doc', content: [{ type: 'paragraph', content: [] }] }, - body: { type: 'doc', content: [{ type: 'paragraph', content: [] }] }, - footer: { type: 'doc', content: [{ type: 'paragraph', content: [] }] }, - }; - - vi.mocked(handleParagraphNode).mockImplementation((node, context) => { - context.blocks.push({ kind: 'paragraph', id: '0-paragraph' } as never); - }); - - const result = toFlowBlocksMap(documents); - - expect(Object.keys(result)).toEqual(['header', 'body', 'footer']); - expect(result.header).toBeDefined(); - expect(result.body).toBeDefined(); - expect(result.footer).toBeDefined(); - }); - - it('should use document key as default prefix', () => { - const documents: PMDocumentMap = { - 'test-doc': { type: 'doc', content: [{ type: 'paragraph', content: [] }] }, - }; - - toFlowBlocksMap(documents); - - // The prefix should be 'test-doc-' by default - expect(handleParagraphNode).toHaveBeenCalled(); - }); - - it('should use blockIdPrefixFactory when provided', () => { - const documents: PMDocumentMap = { - header: { type: 'doc', content: [{ type: 'paragraph', content: [] }] }, - footer: { type: 'doc', content: [{ type: 'paragraph', content: [] }] }, - }; - - const prefixFactory = vi.fn((key: string) => `custom-${key}-`); - const options: BatchAdapterOptions = { blockIdPrefixFactory: prefixFactory }; - - toFlowBlocksMap(documents, options); - - expect(prefixFactory).toHaveBeenCalledWith('header'); - expect(prefixFactory).toHaveBeenCalledWith('footer'); - expect(prefixFactory).toHaveBeenCalledTimes(2); - }); - - it('should use blockIdPrefix as fallback when no factory', () => { - const documents: PMDocumentMap = { - doc1: { type: 'doc', content: [{ type: 'paragraph', content: [] }] }, - }; - - toFlowBlocksMap(documents, { blockIdPrefix: 'fallback-' }); - - // Should use 'fallback-' instead of 'doc1-' - expect(handleParagraphNode).toHaveBeenCalled(); - }); - - it('should skip null documents', () => { - const documents: PMDocumentMap = { - valid: { type: 'doc', content: [{ type: 'paragraph', content: [] }] }, - invalid: null, - }; - - vi.mocked(handleParagraphNode).mockImplementation((node, context) => { - context.blocks.push({ kind: 'paragraph', id: '0-paragraph' } as never); - }); - - const result = toFlowBlocksMap(documents); - - expect(result).toHaveProperty('valid'); - expect(result).not.toHaveProperty('invalid'); - }); - - it('should skip undefined documents', () => { - const documents: PMDocumentMap = { - valid: { type: 'doc', content: [{ type: 'paragraph', content: [] }] }, - invalid: undefined, - }; - - vi.mocked(handleParagraphNode).mockImplementation((node, context) => { - context.blocks.push({ kind: 'paragraph', id: '0-paragraph' } as never); - }); - - const result = toFlowBlocksMap(documents); - - expect(result).toHaveProperty('valid'); - expect(result).not.toHaveProperty('invalid'); - }); - - it('should pass adapter options to each conversion', () => { - const documents: PMDocumentMap = { - doc1: { type: 'doc', content: [{ type: 'paragraph', content: [] }] }, - }; - - const options: BatchAdapterOptions = { - defaultFont: 'Courier', - defaultSize: 10, - trackedChangesMode: 'original', - }; - - toFlowBlocksMap(documents, options); - - expect(handleParagraphNode).toHaveBeenCalledWith( - expect.any(Object), - expect.objectContaining({ - defaultFont: 'Courier', - defaultSize: 10, - trackedChangesConfig: expect.objectContaining({ - mode: 'original', - }), - }), - ); - }); - - it('should return result keyed by document keys', () => { - const documents: PMDocumentMap = { - alpha: { type: 'doc', content: [] }, - beta: { type: 'doc', content: [] }, - gamma: { type: 'doc', content: [] }, - }; - - const result = toFlowBlocksMap(documents); - - expect(Object.keys(result).sort()).toEqual(['alpha', 'beta', 'gamma']); - }); - - it('should extract blocks correctly from each document', () => { - const documents: PMDocumentMap = { - doc1: { type: 'doc', content: [{ type: 'paragraph', content: [] }] }, - doc2: { type: 'doc', content: [{ type: 'paragraph', content: [] }] }, - }; - - let callCount = 0; - vi.mocked(handleParagraphNode).mockImplementation((node, context) => { - callCount++; - context.blocks.push({ - kind: 'paragraph', - id: `${callCount}-paragraph`, - } as never); - }); - - const result = toFlowBlocksMap(documents); - - expect(result.doc1).toHaveLength(1); - expect(result.doc2).toHaveLength(1); - expect(result.doc1![0]!.id).toBe('1-paragraph'); - expect(result.doc2![0]!.id).toBe('2-paragraph'); - }); - }); - describe('paragraphToFlowBlocks wrapper', () => { // Note: paragraphToFlowBlocks is not exported, so we test it indirectly // through the converters that are passed to handlers @@ -1082,29 +896,33 @@ describe('internal', () => { const converterCtx = { docx: { foo: 'bar' } } as never; const themeColors = { primary: '#123456' } as never; - paragraphConverter( - paraNode, - context.nextBlockId, - context.positions, - context.defaultFont, - context.defaultSize, - context.styleContext, - context.trackedChangesConfig, - context.bookmarks, - context.hyperlinkConfig, + paragraphConverter({ + para: paraNode, + nextBlockId: context.nextBlockId, + positions: context.positions, + defaultFont: context.defaultFont, + defaultSize: context.defaultSize, + styleContext: context.styleContext, + trackedChangesConfig: context.trackedChangesConfig, + bookmarks: context.bookmarks, + hyperlinkConfig: context.hyperlinkConfig, themeColors, - converterCtx, - ); + converters: context.converters, + enableComments: context.enableComments, + converterContext: converterCtx, + }); const lastCall = vi.mocked(paragraphToFlowBlocks).mock.calls.at(-1); - expect(lastCall?.[9]).toBe(themeColors); - expect(lastCall?.[10]).toEqual( + expect(lastCall?.[0]).toEqual( expect.objectContaining({ - imageNodeToBlock: expect.any(Function), - tableNodeToBlock: expect.any(Function), + themeColors, + converterContext: converterCtx, + converters: expect.objectContaining({ + imageNodeToBlock: expect.any(Function), + tableNodeToBlock: expect.any(Function), + }), }), ); - expect(lastCall?.[11]).toBe(converterCtx); }); }); }); diff --git a/packages/layout-engine/pm-adapter/src/internal.ts b/packages/layout-engine/pm-adapter/src/internal.ts index 5cfe21b934..eeb62a6568 100644 --- a/packages/layout-engine/pm-adapter/src/internal.ts +++ b/packages/layout-engine/pm-adapter/src/internal.ts @@ -24,7 +24,7 @@ import { createBlockIdGenerator, } from './utilities.js'; import { - paragraphToFlowBlocks as paragraphToFlowBlocksImpl, + paragraphToFlowBlocks, contentBlockNodeToDrawingBlock, imageNodeToBlock, handleImageNode, @@ -36,7 +36,7 @@ import { handleShapeGroupNode, handleShapeContainerNode, handleShapeTextboxNode, - tableNodeToBlock as tableNodeToBlockImpl, + tableNodeToBlock, handleTableNode, hydrateImageBlocks, handleParagraphNode, @@ -54,20 +54,12 @@ import type { HyperlinkConfig, FlowBlocksResult, AdapterOptions, - BlockIdGenerator, - PositionMap, NodeHandlerContext, NodeHandler, - PMDocumentMap, - BatchAdapterOptions, - ThemeColorPalette, + NestedConverters, ConverterContext, - TableNodeToBlockOptions, - ParagraphToFlowBlocksConverter, - TableNodeToBlockConverter, } from './types.js'; import { defaultDecimalSeparatorFor } from '@superdoc/locale-utils'; -import { DEFAULT_HYPERLINK_CONFIG } from './constants'; const DEFAULT_FONT = 'Arial'; const DEFAULT_SIZE = 16; @@ -93,6 +85,17 @@ export const nodeHandlers: Record = { shapeTextbox: handleShapeTextboxNode, }; +export const converters: NestedConverters = { + contentBlockNodeToDrawingBlock, + imageNodeToBlock, + vectorShapeNodeToDrawingBlock, + shapeGroupNodeToDrawingBlock, + shapeContainerNodeToDrawingBlock, + shapeTextboxNodeToDrawingBlock, + tableNodeToBlock, + paragraphToFlowBlocks, +}; + /** * Convert a ProseMirror document to FlowBlock array with bookmark tracking. * @@ -159,8 +162,21 @@ export function toFlowBlocks(pmDoc: PMNode | object, options?: AdapterOptions): enableRichHyperlinks: options?.enableRichHyperlinks ?? false, }; const enableComments = options?.enableComments ?? true; - const themeColors = options?.themeColors; - const converterContext = options?.converterContext; + const converterContext: ConverterContext = options?.converterContext ?? { + translatedNumbering: {}, + translatedLinkedStyles: { + docDefaults: { + runProperties: { + fontFamily: { + ascii: defaultFont, + }, + fontSize: pxToPt(defaultSize) ?? 12, + }, + }, + latentStyles: {}, + styles: {}, + }, + }; if (!doc.content) { return { blocks: [], bookmarks: new Map() }; @@ -178,28 +194,6 @@ export function toFlowBlocks(pmDoc: PMNode | object, options?: AdapterOptions): blockCounts[kind] = (blockCounts[kind] ?? 0) + 1; }; - // Track B: List counter tracker for sequential numbering - // Maps "numId:ilvl" -> current counter value for that list/level - const listCounters = new Map(); - - const getListCounter = (numId: number, ilvl: number): number => { - const key = `${numId}:${ilvl}`; - return listCounters.get(key) ?? 0; - }; - - const incrementListCounter = (numId: number, ilvl: number): number => { - const key = `${numId}:${ilvl}`; - const current = listCounters.get(key) ?? 0; - const next = current + 1; - listCounters.set(key, next); - return next; - }; - - const resetListCounter = (numId: number, ilvl: number): void => { - const key = `${numId}:${ilvl}`; - listCounters.set(key, 0); - }; - // Range-aware section analysis (matches toFlowBlocks semantics) const bodySectionProps = doc.attrs?.bodySectPr ?? doc.attrs?.sectPr; const sectionRanges = options?.emitSectionBreaks ? analyzeSectionRanges(doc, bodySectionProps) : []; @@ -215,72 +209,6 @@ export function toFlowBlocks(pmDoc: PMNode | object, options?: AdapterOptions): recordBlockKind(sectionBreak.kind); } - const paragraphConverter = ( - para: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - context: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig?: HyperlinkConfig, - themeColorsParam?: ThemeColorPalette, - converterCtx?: ConverterContext, - ): FlowBlock[] => - paragraphToFlowBlocks( - para, - nextBlockId, - positions, - defaultFont, - defaultSize, - context, - trackedChanges, - bookmarks, - hyperlinkConfig, - themeColorsParam ?? themeColors, - converterCtx ?? converterContext, - enableComments, - ); - - const tableConverter = ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - context: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig?: HyperlinkConfig, - themeColorsParam?: ThemeColorPalette, - converterCtx?: ConverterContext, - ): FlowBlock | null => - tableNodeToBlock( - node, - nextBlockId, - positions, - defaultFont, - defaultSize, - context, - trackedChanges, - bookmarks, - hyperlinkConfig, - themeColorsParam ?? themeColors, - paragraphConverter, - converterCtx ?? converterContext, - { - converters: { - paragraphToFlowBlocks: paragraphConverter, - imageNodeToBlock, - vectorShapeNodeToDrawingBlock, - shapeGroupNodeToDrawingBlock, - shapeContainerNodeToDrawingBlock, - shapeTextboxNodeToDrawingBlock, - }, - }, - ); - // Build handler context for node processing const handlerContext: NodeHandlerContext = { blocks, @@ -300,16 +228,8 @@ export function toFlowBlocks(pmDoc: PMNode | object, options?: AdapterOptions): currentSectionIndex: 0, currentParagraphIndex: 0, }, - converters: { - // Type assertion needed due to signature mismatch between actual function and type definition - paragraphToFlowBlocks: paragraphConverter as unknown as ParagraphToFlowBlocksConverter, - tableNodeToBlock: tableConverter as unknown as TableNodeToBlockConverter, - imageNodeToBlock, - vectorShapeNodeToDrawingBlock, - shapeGroupNodeToDrawingBlock, - shapeContainerNodeToDrawingBlock, - shapeTextboxNodeToDrawingBlock, - }, + converters, + themeColors: options?.themeColors, }; // Process nodes using handler dispatch pattern @@ -343,27 +263,6 @@ export function toFlowBlocks(pmDoc: PMNode | object, options?: AdapterOptions): return { blocks: mergedBlocks, bookmarks }; } -export function toFlowBlocksMap(documents: PMDocumentMap, options?: BatchAdapterOptions): Record { - const { blockIdPrefixFactory, ...adapterOptions } = options ?? {}; - const result: Record = {}; - if (!documents) { - return result; - } - - Object.entries(documents).forEach(([key, doc]) => { - if (!doc) return; - const prefix = blockIdPrefixFactory?.(key) ?? adapterOptions.blockIdPrefix ?? `${key}-`; - const perDocOptions: AdapterOptions = { - ...adapterOptions, - blockIdPrefix: prefix, - }; - const { blocks } = toFlowBlocks(doc, perDocOptions); - result[key] = blocks; - }); - - return result; -} - /** * Merge drop-cap paragraphs with their following text paragraphs. * @@ -424,137 +323,3 @@ function mergeDropCapParagraphs(blocks: FlowBlock[]): FlowBlock[] { return result; } - -/** - * Wrapper for paragraphToFlowBlocks that injects block node converters. - * - * Paragraphs can contain inline images, shapes, and tables. This wrapper - * injects those converters so the paragraph implementation can handle them. - * - * @see converters/paragraph.ts for the actual implementation - */ -function paragraphToFlowBlocks( - para: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - styleContext: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig: HyperlinkConfig = DEFAULT_HYPERLINK_CONFIG, - themeColors?: ThemeColorPalette, - converterContext?: ConverterContext, - enableComments = true, -): FlowBlock[] { - return paragraphToFlowBlocksImpl( - para, - nextBlockId, - positions, - defaultFont, - defaultSize, - styleContext, - trackedChanges, - bookmarks, - hyperlinkConfig, - themeColors, - { - contentBlockNodeToDrawingBlock, - imageNodeToBlock, - vectorShapeNodeToDrawingBlock, - shapeGroupNodeToDrawingBlock, - shapeContainerNodeToDrawingBlock, - shapeTextboxNodeToDrawingBlock, - tableNodeToBlock: ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - styleContext: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig?: HyperlinkConfig, - themeColors?: ThemeColorPalette, - converterCtx?: ConverterContext, - ) => - tableNodeToBlockImpl( - node, - nextBlockId, - positions, - defaultFont, - defaultSize, - styleContext, - trackedChanges, - bookmarks, - hyperlinkConfig, - themeColors, - paragraphToFlowBlocks, - converterCtx ?? converterContext, - { - converters: { - // Type assertion needed due to signature mismatch between actual function and type definition - paragraphToFlowBlocks: paragraphToFlowBlocksImpl as unknown as ParagraphToFlowBlocksConverter, - imageNodeToBlock, - vectorShapeNodeToDrawingBlock, - shapeGroupNodeToDrawingBlock, - shapeContainerNodeToDrawingBlock, - shapeTextboxNodeToDrawingBlock, - }, - }, - ), - }, - converterContext, - enableComments, - ); -} - -/** - * Wrapper for tableNodeToBlock that injects the paragraph converter. - * - * Tables contain paragraphs in their cells. This wrapper injects the - * paragraph converter so table cells can be properly converted. - * - * @see converters/table.ts for the actual implementation - */ -function tableNodeToBlock( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - styleContext: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig?: HyperlinkConfig, - themeColors?: ThemeColorPalette, - _paragraphToFlowBlocksParam?: unknown, - converterContext?: ConverterContext, - options?: TableNodeToBlockOptions, -): FlowBlock | null { - return tableNodeToBlockImpl( - node, - nextBlockId, - positions, - defaultFont, - defaultSize, - styleContext, - trackedChanges, - bookmarks, - hyperlinkConfig, - themeColors, - paragraphToFlowBlocks, - converterContext, - options ?? { - converters: { - // Type assertion needed due to signature mismatch between actual function and type definition - paragraphToFlowBlocks: paragraphToFlowBlocksImpl as unknown as ParagraphToFlowBlocksConverter, - imageNodeToBlock, - vectorShapeNodeToDrawingBlock, - shapeGroupNodeToDrawingBlock, - shapeContainerNodeToDrawingBlock, - shapeTextboxNodeToDrawingBlock, - }, - }, - ); -} diff --git a/packages/layout-engine/pm-adapter/src/sdt/document-index.test.ts b/packages/layout-engine/pm-adapter/src/sdt/document-index.test.ts index 63be3e95cf..7334fbb5fd 100644 --- a/packages/layout-engine/pm-adapter/src/sdt/document-index.test.ts +++ b/packages/layout-engine/pm-adapter/src/sdt/document-index.test.ts @@ -13,10 +13,10 @@ describe('document-index', () => { let mockParagraphToFlowBlocks: ReturnType; beforeEach(() => { - mockParagraphToFlowBlocks = vi.fn().mockImplementation((para) => [ + mockParagraphToFlowBlocks = vi.fn().mockImplementation((params) => [ { kind: 'paragraph', - id: `block-${para.attrs?.id || 'unknown'}`, + id: `block-${params.para.attrs?.id || 'unknown'}`, runs: [{ text: 'test', fontFamily: 'Arial', fontSize: 12 }], }, ]); @@ -35,8 +35,8 @@ describe('document-index', () => { resetListCounter: vi.fn(), }, trackedChangesConfig: {}, - bookmarks: {}, - hyperlinkConfig: {}, + bookmarks: new Map(), + hyperlinkConfig: { enableRichHyperlinks: false }, sectionState: { ranges: [], currentSectionIndex: 0, @@ -46,6 +46,7 @@ describe('document-index', () => { paragraphToFlowBlocks: mockParagraphToFlowBlocks, }, converterContext: {}, + enableComments: true, }; }); @@ -103,7 +104,7 @@ describe('document-index', () => { expect(mockContext.blocks).toHaveLength(0); }); - it('returns early if paragraphToFlowBlocks converter is not available', () => { + it('throws if paragraphToFlowBlocks converter is not available', () => { const indexNode: PMNode = { type: 'index', content: [{ type: 'paragraph', attrs: { id: 'p1' } }], @@ -111,9 +112,7 @@ describe('document-index', () => { mockContext.converters = {}; - handleIndexNode(indexNode, mockContext); - - expect(mockContext.blocks).toHaveLength(0); + expect(() => handleIndexNode(indexNode, mockContext)).toThrow(); }); it('increments currentParagraphIndex for each paragraph', () => { diff --git a/packages/layout-engine/pm-adapter/src/sdt/document-index.ts b/packages/layout-engine/pm-adapter/src/sdt/document-index.ts index 41dfcebbc3..71da588d79 100644 --- a/packages/layout-engine/pm-adapter/src/sdt/document-index.ts +++ b/packages/layout-engine/pm-adapter/src/sdt/document-index.ts @@ -53,12 +53,11 @@ export function handleIndexNode(node: PMNode, context: NodeHandlerContext): void hyperlinkConfig, sectionState, converters, + themeColors, + enableComments, } = context; - const paragraphToFlowBlocks = converters?.paragraphToFlowBlocks; - if (!paragraphToFlowBlocks) { - return; - } + const paragraphToFlowBlocks = converters.paragraphToFlowBlocks; children.forEach((child) => { if (child.type !== 'paragraph') { @@ -79,8 +78,8 @@ export function handleIndexNode(node: PMNode, context: NodeHandlerContext): void } } - const paragraphBlocks = paragraphToFlowBlocks( - child, + const paragraphBlocks = paragraphToFlowBlocks({ + para: child, nextBlockId, positions, defaultFont, @@ -89,9 +88,11 @@ export function handleIndexNode(node: PMNode, context: NodeHandlerContext): void trackedChangesConfig, bookmarks, hyperlinkConfig, - undefined, // themeColors - not available in NodeHandlerContext - context.converterContext, - ); + themeColors, + converterContext: context.converterContext, + enableComments: enableComments, + converters, + }); paragraphBlocks.forEach((block) => { blocks.push(block); diff --git a/packages/layout-engine/pm-adapter/src/sdt/document-part-object.test.ts b/packages/layout-engine/pm-adapter/src/sdt/document-part-object.test.ts index 8a924f70e7..cc01400575 100644 --- a/packages/layout-engine/pm-adapter/src/sdt/document-part-object.test.ts +++ b/packages/layout-engine/pm-adapter/src/sdt/document-part-object.test.ts @@ -37,6 +37,8 @@ describe('document-part-object', () => { const mockHyperlinkConfig = { enableRichHyperlinks: false, }; + const mockConverterContext = { docx: {} } as never; + const mockEnableComments = true; const mockParagraphConverter = vi.fn((_node: PMNode) => [ { @@ -61,6 +63,8 @@ describe('document-part-object', () => { styleContext: mockStyleContext, bookmarks: new Map(), hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, tableNodeToBlock: vi.fn(), @@ -120,9 +124,12 @@ describe('document-part-object', () => { docPartObjectId: 'toc-1', tocInstruction: 'TOC \\o "1-3"', }), + expect.objectContaining({ + converters: mockContext.converters, + converterContext: mockConverterContext, + enableComments: mockEnableComments, + }), expect.any(Object), - expect.any(Object), - mockParagraphConverter, ); }); }); @@ -174,9 +181,12 @@ describe('document-part-object', () => { docPartObjectId: 'toc-123', tocInstruction: 'TOC \\o "1-3"', }), + expect.objectContaining({ + converters: mockContext.converters, + converterContext: mockConverterContext, + enableComments: mockEnableComments, + }), expect.any(Object), - expect.any(Object), - mockParagraphConverter, ); }); @@ -274,7 +284,7 @@ describe('document-part-object', () => { // ==================== Missing Dependencies Tests ==================== describe('Missing dependencies', () => { - it('should not process when paragraphToFlowBlocks converter is missing', () => { + it('should still call processTocChildren even if paragraphToFlowBlocks is missing', () => { const node: PMNode = { type: 'documentPartObject', content: [ @@ -302,10 +312,10 @@ describe('document-part-object', () => { handleDocumentPartObjectNode(node, contextWithoutConverter); - expect(tocModule.processTocChildren).not.toHaveBeenCalled(); + expect(tocModule.processTocChildren).toHaveBeenCalled(); }); - it('should not process when converters is missing entirely', () => { + it('should throw when converters is missing entirely', () => { const node: PMNode = { type: 'documentPartObject', content: [ @@ -323,9 +333,7 @@ describe('document-part-object', () => { converters: undefined as never, }; - handleDocumentPartObjectNode(node, contextWithoutConverters); - - expect(tocModule.processTocChildren).not.toHaveBeenCalled(); + expect(() => handleDocumentPartObjectNode(node, contextWithoutConverters)).toThrow(); }); }); @@ -354,6 +362,9 @@ describe('document-part-object', () => { styleContext: mockStyleContext, bookmarks: mockContext.bookmarks, hyperlinkConfig: mockHyperlinkConfig, + converters: mockContext.converters, + converterContext: mockConverterContext, + enableComments: mockEnableComments, }), ); }); @@ -378,7 +389,7 @@ describe('document-part-object', () => { }); }); - it('should pass paragraphToFlowBlocks converter as fifth argument', () => { + it('should pass converters in context', () => { const node: PMNode = { type: 'documentPartObject', content: [{ type: 'paragraph' }], @@ -392,7 +403,7 @@ describe('document-part-object', () => { handleDocumentPartObjectNode(node, mockContext); const callArgs = vi.mocked(tocModule.processTocChildren).mock.calls[0]; - expect(callArgs[4]).toBe(mockParagraphConverter); + expect(callArgs[2].converters).toBe(mockContext.converters); }); }); diff --git a/packages/layout-engine/pm-adapter/src/sdt/document-part-object.ts b/packages/layout-engine/pm-adapter/src/sdt/document-part-object.ts index c37400408c..c3ea6583e4 100644 --- a/packages/layout-engine/pm-adapter/src/sdt/document-part-object.ts +++ b/packages/layout-engine/pm-adapter/src/sdt/document-part-object.ts @@ -31,15 +31,18 @@ export function handleDocumentPartObjectNode(node: PMNode, context: NodeHandlerC bookmarks, hyperlinkConfig, converters, + converterContext, + enableComments, trackedChangesConfig, + themeColors, } = context; const docPartGallery = getDocPartGallery(node); const docPartObjectId = getDocPartObjectId(node); const tocInstruction = getNodeInstruction(node); const docPartSdtMetadata = resolveNodeSdtMetadata(node, 'docPartObject'); - const paragraphToFlowBlocks = converters?.paragraphToFlowBlocks; + const paragraphToFlowBlocks = converters.paragraphToFlowBlocks; - if (docPartGallery === 'Table of Contents' && paragraphToFlowBlocks) { + if (docPartGallery === 'Table of Contents') { processTocChildren( Array.from(node.content), { docPartGallery, docPartObjectId, tocInstruction, sdtMetadata: docPartSdtMetadata }, @@ -51,16 +54,19 @@ export function handleDocumentPartObjectNode(node: PMNode, context: NodeHandlerC styleContext, bookmarks, hyperlinkConfig, + enableComments, + trackedChangesConfig, + converters, + converterContext, }, { blocks, recordBlockKind }, - paragraphToFlowBlocks, ); } else if (paragraphToFlowBlocks) { // For non-ToC gallery types (page numbers, etc.), process child paragraphs normally for (const child of node.content) { if (child.type === 'paragraph') { - const childBlocks = paragraphToFlowBlocks( - child, + const childBlocks = paragraphToFlowBlocks({ + para: child, nextBlockId, positions, defaultFont, @@ -69,7 +75,11 @@ export function handleDocumentPartObjectNode(node: PMNode, context: NodeHandlerC trackedChangesConfig, bookmarks, hyperlinkConfig, - ); + converters, + themeColors, + enableComments, + converterContext, + }); for (const block of childBlocks) { blocks.push(block); recordBlockKind(block.kind); diff --git a/packages/layout-engine/pm-adapter/src/sdt/document-section.test.ts b/packages/layout-engine/pm-adapter/src/sdt/document-section.test.ts index 082017c7be..bed28bda83 100644 --- a/packages/layout-engine/pm-adapter/src/sdt/document-section.test.ts +++ b/packages/layout-engine/pm-adapter/src/sdt/document-section.test.ts @@ -137,11 +137,11 @@ describe('document-section', () => { const blocks: FlowBlock[] = []; const recordBlockKind = vi.fn(); - const mockParagraphConverter = vi.fn((para) => [ + const mockParagraphConverter = vi.fn((params) => [ { kind: 'paragraph', - id: `p-${para.content[0].text}`, - runs: [{ text: para.content[0].text, fontFamily: 'Arial', fontSize: 12 }], + id: `p-${params.para.content[0].text}`, + runs: [{ text: params.para.content[0].text, fontFamily: 'Arial', fontSize: 12 }], } as ParagraphBlock, ]); @@ -270,15 +270,15 @@ describe('document-section', () => { ); expect(mockParagraphConverter).toHaveBeenCalledWith( - children[0], - mockBlockIdGenerator, - mockPositionMap, - 'Arial', - 12, - mockStyleContext, - undefined, - undefined, - mockHyperlinkConfig, + expect.objectContaining({ + para: children[0], + nextBlockId: mockBlockIdGenerator, + positions: mockPositionMap, + defaultFont: 'Arial', + defaultSize: 12, + styleContext: mockStyleContext, + hyperlinkConfig: mockHyperlinkConfig, + }), ); }); @@ -465,15 +465,15 @@ describe('document-section', () => { ); expect(mockTableConverter).toHaveBeenCalledWith( - children[0], - mockBlockIdGenerator, - mockPositionMap, - 'Arial', - 12, - mockStyleContext, - undefined, - undefined, - mockHyperlinkConfig, + expect.objectContaining({ + node: children[0], + nextBlockId: mockBlockIdGenerator, + positions: mockPositionMap, + defaultFont: 'Arial', + defaultSize: 12, + styleContext: mockStyleContext, + hyperlinkConfig: mockHyperlinkConfig, + }), ); expect(metadataModule.applySdtMetadataToTableBlock).toHaveBeenCalledWith(mockTableBlock, sectionMetadata); expect(blocks).toHaveLength(1); @@ -1244,9 +1244,9 @@ describe('document-section', () => { positions: mockPositionMap, defaultFont: 'Arial', defaultSize: 12, + converters: expect.any(Object), }), { blocks, recordBlockKind }, - mockParagraphConverter, ); }); @@ -1435,7 +1435,6 @@ describe('document-section', () => { }), expect.anything(), expect.anything(), - expect.anything(), ); }); }); @@ -1472,15 +1471,16 @@ describe('document-section', () => { ); expect(mockParagraphConverter).toHaveBeenCalledWith( - children[0], - mockBlockIdGenerator, - mockPositionMap, - 'Arial', - 12, - mockStyleContext, - undefined, - mockBookmarks, - mockHyperlinkConfig, + expect.objectContaining({ + para: children[0], + nextBlockId: mockBlockIdGenerator, + positions: mockPositionMap, + defaultFont: 'Arial', + defaultSize: 12, + styleContext: mockStyleContext, + bookmarks: mockBookmarks, + hyperlinkConfig: mockHyperlinkConfig, + }), ); }); diff --git a/packages/layout-engine/pm-adapter/src/sdt/document-section.ts b/packages/layout-engine/pm-adapter/src/sdt/document-section.ts index 544291e540..a1ee3960cc 100644 --- a/packages/layout-engine/pm-adapter/src/sdt/document-section.ts +++ b/packages/layout-engine/pm-adapter/src/sdt/document-section.ts @@ -14,6 +14,9 @@ import type { HyperlinkConfig, NodeHandlerContext, TrackedChangesConfig, + NestedConverters, + ConverterContext, + ThemeColorPalette, } from '../types.js'; import { applySdtMetadataToParagraphBlocks, @@ -25,48 +28,6 @@ import { } from './metadata.js'; import { processTocChildren } from './toc.js'; -/** - * Type for paragraph converter function. - * This is injected to avoid circular dependencies. - */ -type ParagraphConverter = ( - para: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - styleContext: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig?: HyperlinkConfig, -) => FlowBlock[]; - -/** - * Type for table converter function. - */ -type TableConverter = ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - styleContext: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig?: HyperlinkConfig, -) => FlowBlock | null; - -/** - * Type for image converter function. - */ -type ImageConverter = ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - trackedMeta?: TrackedChangeMeta, - trackedChanges?: TrackedChangesConfig, -) => FlowBlock | null; - /** * Context object containing processing dependencies and configuration. */ @@ -76,8 +37,12 @@ interface ProcessingContext { defaultFont: string; defaultSize: number; styleContext: StyleContext; + trackedChangesConfig?: TrackedChangesConfig; bookmarks?: Map; hyperlinkConfig: HyperlinkConfig; + enableComments: boolean; + converterContext: ConverterContext; + themeColors?: ThemeColorPalette; } /** @@ -88,15 +53,6 @@ interface ProcessingOutput { recordBlockKind: (kind: FlowBlock['kind']) => void; } -/** - * Collection of converter functions for different node types. - */ -interface NodeConverters { - paragraphToFlowBlocks: ParagraphConverter; - tableNodeToBlock: TableConverter; - imageNodeToBlock: ImageConverter; -} - /** * Processes a paragraph child node by converting it to flow blocks, * applying section metadata, and adding to output. @@ -112,19 +68,22 @@ function processParagraphChild( sectionMetadata: SdtMetadata | undefined, context: ProcessingContext, output: ProcessingOutput, - converters: NodeConverters, + converters: NestedConverters, ): void { - const paragraphBlocks = converters.paragraphToFlowBlocks( - child, - context.nextBlockId, - context.positions, - context.defaultFont, - context.defaultSize, - context.styleContext, - undefined, // trackedChanges - context.bookmarks, - context.hyperlinkConfig, - ); + const paragraphBlocks = converters!.paragraphToFlowBlocks!({ + para: child, + nextBlockId: context.nextBlockId, + positions: context.positions, + defaultFont: context.defaultFont, + defaultSize: context.defaultSize, + styleContext: context.styleContext, + trackedChangesConfig: undefined, // trackedChanges + bookmarks: context.bookmarks, + hyperlinkConfig: context.hyperlinkConfig, + converters, + enableComments: context.enableComments, + converterContext: context.converterContext, + }); applySdtMetadataToParagraphBlocks( paragraphBlocks.filter((b) => b.kind === 'paragraph') as ParagraphBlock[], sectionMetadata, @@ -150,19 +109,22 @@ function processTableChild( sectionMetadata: SdtMetadata | undefined, context: ProcessingContext, output: ProcessingOutput, - converters: NodeConverters, + converters: NestedConverters, ): void { - const tableBlock = converters.tableNodeToBlock( - child, - context.nextBlockId, - context.positions, - context.defaultFont, - context.defaultSize, - context.styleContext, - undefined, - undefined, - context.hyperlinkConfig, - ); + const tableBlock = converters.tableNodeToBlock({ + node: child, + nextBlockId: context.nextBlockId, + positions: context.positions, + defaultFont: context.defaultFont, + defaultSize: context.defaultSize, + styleContext: context.styleContext, + trackedChangesConfig: context.trackedChangesConfig, + bookmarks: context.bookmarks, + hyperlinkConfig: context.hyperlinkConfig, + enableComments: context.enableComments, + converters, + converterContext: context.converterContext, + }); if (tableBlock) { applySdtMetadataToTableBlock(tableBlock, sectionMetadata); output.blocks.push(tableBlock); @@ -185,7 +147,7 @@ function processImageChild( sectionMetadata: SdtMetadata | undefined, context: ProcessingContext, output: ProcessingOutput, - converters: NodeConverters, + converters: NestedConverters, ): void { const imageBlock = converters.imageNodeToBlock(child, context.nextBlockId, context.positions); if (imageBlock && imageBlock.kind === 'image') { @@ -215,23 +177,27 @@ function processNestedStructuredContent( sectionMetadata: SdtMetadata | undefined, context: ProcessingContext, output: ProcessingOutput, - converters: NodeConverters, + converters: NestedConverters, ): void { // Nested structured content block inside section - unwrap and chain metadata const nestedMetadata = resolveNodeSdtMetadata(child, 'structuredContentBlock'); child.content?.forEach((grandchild) => { if (grandchild.type === 'paragraph') { - const paragraphBlocks = converters.paragraphToFlowBlocks( - grandchild, - context.nextBlockId, - context.positions, - context.defaultFont, - context.defaultSize, - context.styleContext, - undefined, // trackedChanges - context.bookmarks, - context.hyperlinkConfig, - ); + const paragraphBlocks = converters.paragraphToFlowBlocks({ + para: grandchild, + nextBlockId: context.nextBlockId, + positions: context.positions, + defaultFont: context.defaultFont, + defaultSize: context.defaultSize, + styleContext: context.styleContext, + trackedChangesConfig: context.trackedChangesConfig, + bookmarks: context.bookmarks, + hyperlinkConfig: context.hyperlinkConfig, + converters, + enableComments: context.enableComments, + converterContext: context.converterContext, + themeColors: context.themeColors, + }); // Apply nested structured content metadata first, then section metadata const paraOnly = paragraphBlocks.filter((b) => b.kind === 'paragraph') as ParagraphBlock[]; applySdtMetadataToParagraphBlocks(paraOnly, nestedMetadata); @@ -241,17 +207,21 @@ function processNestedStructuredContent( output.recordBlockKind(block.kind); }); } else if (grandchild.type === 'table') { - const tableBlock = converters.tableNodeToBlock( - grandchild, - context.nextBlockId, - context.positions, - context.defaultFont, - context.defaultSize, - context.styleContext, - undefined, - undefined, - context.hyperlinkConfig, - ); + const tableBlock = converters.tableNodeToBlock({ + node: grandchild, + nextBlockId: context.nextBlockId, + positions: context.positions, + defaultFont: context.defaultFont, + defaultSize: context.defaultSize, + styleContext: context.styleContext, + trackedChangesConfig: context.trackedChangesConfig, + bookmarks: context.bookmarks, + hyperlinkConfig: context.hyperlinkConfig, + enableComments: context.enableComments, + themeColors: context.themeColors, + converters, + converterContext: context.converterContext, + }); if (tableBlock) { if (nestedMetadata) applySdtMetadataToTableBlock(tableBlock, nestedMetadata); applySdtMetadataToTableBlock(tableBlock, sectionMetadata); @@ -278,7 +248,7 @@ function processDocumentPartObject( sectionMetadata: SdtMetadata | undefined, context: ProcessingContext, output: ProcessingOutput, - converters: NodeConverters, + converters: NestedConverters, ): void { // Nested doc part (e.g., TOC) inside section const docPartGallery = getDocPartGallery(child); @@ -301,9 +271,12 @@ function processDocumentPartObject( styleContext: context.styleContext, bookmarks: context.bookmarks, hyperlinkConfig: context.hyperlinkConfig, + enableComments: context.enableComments, + themeColors: context.themeColors, + converters, + converterContext: context.converterContext, }, { blocks: output.blocks, recordBlockKind: output.recordBlockKind }, - converters.paragraphToFlowBlocks, ); // Apply section metadata to TOC paragraphs while preserving docPart metadata @@ -347,7 +320,7 @@ export function processDocumentSectionChildren( sectionMetadata: SdtMetadata | undefined, context: ProcessingContext, output: ProcessingOutput, - converters: NodeConverters, + converters: NestedConverters, ): void { children.forEach((child) => { if (child.type === 'paragraph') { @@ -386,16 +359,13 @@ export function handleDocumentSectionNode(node: PMNode, context: NodeHandlerCont bookmarks, hyperlinkConfig, converters, + enableComments, + converterContext, + trackedChangesConfig, + themeColors, } = context; const sectionMetadata = resolveNodeSdtMetadata(node, 'documentSection'); - // Get converters from context - const convertersToUse: NodeConverters = { - paragraphToFlowBlocks: converters?.paragraphToFlowBlocks || ((): FlowBlock[] => []), - tableNodeToBlock: converters?.tableNodeToBlock || ((): FlowBlock | null => null), - imageNodeToBlock: converters?.imageNodeToBlock || ((): FlowBlock | null => null), - }; - processDocumentSectionChildren( node.content, sectionMetadata, @@ -406,9 +376,13 @@ export function handleDocumentSectionNode(node: PMNode, context: NodeHandlerCont defaultSize, styleContext, bookmarks, + trackedChangesConfig, hyperlinkConfig, + themeColors, + enableComments, + converterContext, }, { blocks, recordBlockKind }, - convertersToUse, + converters, ); } diff --git a/packages/layout-engine/pm-adapter/src/sdt/structured-content-block.test.ts b/packages/layout-engine/pm-adapter/src/sdt/structured-content-block.test.ts index 7680d914d1..4b70c4331e 100644 --- a/packages/layout-engine/pm-adapter/src/sdt/structured-content-block.test.ts +++ b/packages/layout-engine/pm-adapter/src/sdt/structured-content-block.test.ts @@ -31,6 +31,8 @@ describe('structured-content-block', () => { }; const mockTrackedChangesConfig = undefined; const mockBookmarks = new Map(); + const mockEnableComments = true; + const mockConverterContext = { docx: {} } as never; const scbMetadata: SdtMetadata = { type: 'structuredContentBlock', @@ -64,6 +66,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: vi.fn(), }, @@ -75,7 +79,7 @@ describe('structured-content-block', () => { expect(recordBlockKind).not.toHaveBeenCalled(); }); - it('should return early if paragraphToFlowBlocks is not provided', () => { + it('should throw if paragraphToFlowBlocks is not provided', () => { const node: PMNode = { type: 'structuredContentBlock', attrs: { id: 'scb-1' }, @@ -96,13 +100,12 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: undefined, }; - handleStructuredContentBlockNode(node, context); - - expect(blocks).toHaveLength(0); - expect(recordBlockKind).not.toHaveBeenCalled(); + expect(() => handleStructuredContentBlockNode(node, context)).toThrow(); }); it('should handle empty children array', () => { @@ -128,6 +131,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: vi.fn(), }, @@ -170,6 +175,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -199,11 +206,11 @@ describe('structured-content-block', () => { vi.mocked(metadataModule.resolveNodeSdtMetadata).mockReturnValue(scbMetadata); - const mockParagraphConverter = vi.fn((para) => [ + const mockParagraphConverter = vi.fn((params) => [ { kind: 'paragraph', - id: `p-${para.content[0].text}`, - runs: [{ text: para.content[0].text, fontFamily: 'Arial', fontSize: 12 }], + id: `p-${params.para.content[0].text}`, + runs: [{ text: params.para.content[0].text, fontFamily: 'Arial', fontSize: 12 }], } as ParagraphBlock, ]); @@ -218,6 +225,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -255,6 +264,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -293,6 +304,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -301,15 +314,19 @@ describe('structured-content-block', () => { handleStructuredContentBlockNode(node, context); expect(mockParagraphConverter).toHaveBeenCalledWith( - node.content[0], - mockBlockIdGenerator, - mockPositionMap, - 'Arial', - 12, - mockStyleContext, - mockTrackedChangesConfig, - mockBookmarks, - mockHyperlinkConfig, + expect.objectContaining({ + para: node.content[0], + nextBlockId: mockBlockIdGenerator, + positions: mockPositionMap, + defaultFont: 'Arial', + defaultSize: 12, + styleContext: mockStyleContext, + trackedChangesConfig: mockTrackedChangesConfig, + bookmarks: mockBookmarks, + hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, + }), ); }); @@ -342,6 +359,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -381,6 +400,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -420,6 +441,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -465,6 +488,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -500,6 +525,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -543,6 +570,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -578,6 +607,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: vi.fn(), }, @@ -622,6 +653,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -665,6 +698,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -705,6 +740,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -741,6 +778,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: vi.fn(), }, @@ -776,6 +815,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -819,6 +860,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, @@ -862,6 +905,8 @@ describe('structured-content-block', () => { trackedChangesConfig: mockTrackedChangesConfig, bookmarks: mockBookmarks, hyperlinkConfig: mockHyperlinkConfig, + enableComments: mockEnableComments, + converterContext: mockConverterContext, converters: { paragraphToFlowBlocks: mockParagraphConverter, }, diff --git a/packages/layout-engine/pm-adapter/src/sdt/structured-content-block.ts b/packages/layout-engine/pm-adapter/src/sdt/structured-content-block.ts index 8cf65df41f..9c1f4b952b 100644 --- a/packages/layout-engine/pm-adapter/src/sdt/structured-content-block.ts +++ b/packages/layout-engine/pm-adapter/src/sdt/structured-content-block.ts @@ -31,18 +31,17 @@ export function handleStructuredContentBlockNode(node: PMNode, context: NodeHand bookmarks, hyperlinkConfig, converters, + converterContext, + enableComments, + themeColors, } = context; const structuredContentMetadata = resolveNodeSdtMetadata(node, 'structuredContentBlock'); - const paragraphToFlowBlocks = converters?.paragraphToFlowBlocks; - - if (!paragraphToFlowBlocks) { - return; - } + const paragraphToFlowBlocks = converters.paragraphToFlowBlocks; node.content.forEach((child) => { if (child.type === 'paragraph') { - const paragraphBlocks = paragraphToFlowBlocks( - child, + const paragraphBlocks = paragraphToFlowBlocks({ + para: child, nextBlockId, positions, defaultFont, @@ -51,7 +50,11 @@ export function handleStructuredContentBlockNode(node: PMNode, context: NodeHand trackedChangesConfig, bookmarks, hyperlinkConfig, - ); + themeColors, + enableComments, + converters, + converterContext, + }); applySdtMetadataToParagraphBlocks( paragraphBlocks.filter((b) => b.kind === 'paragraph') as ParagraphBlock[], structuredContentMetadata, @@ -63,8 +66,8 @@ export function handleStructuredContentBlockNode(node: PMNode, context: NodeHand } else if (child.type === 'table') { const tableNodeToBlock = converters?.tableNodeToBlock; if (tableNodeToBlock) { - const tableBlock = tableNodeToBlock( - child, + const tableBlock = tableNodeToBlock({ + node: child, nextBlockId, positions, defaultFont, @@ -73,7 +76,11 @@ export function handleStructuredContentBlockNode(node: PMNode, context: NodeHand trackedChangesConfig, bookmarks, hyperlinkConfig, - ); + themeColors, + enableComments, + converters, + converterContext, + }); if (tableBlock) { applySdtMetadataToTableBlock(tableBlock as TableBlock, structuredContentMetadata); blocks.push(tableBlock); diff --git a/packages/layout-engine/pm-adapter/src/sdt/toc.test.ts b/packages/layout-engine/pm-adapter/src/sdt/toc.test.ts index 52d6b95298..9214a45ba8 100644 --- a/packages/layout-engine/pm-adapter/src/sdt/toc.test.ts +++ b/packages/layout-engine/pm-adapter/src/sdt/toc.test.ts @@ -152,6 +152,7 @@ describe('toc', () => { const mockHyperlinkConfig = { mode: 'preserve' as const, }; + const mockConverterContext = { docx: {} } as never; it('processes direct paragraph children', () => { const children: PMNode[] = [ @@ -168,12 +169,12 @@ describe('toc', () => { const blocks: FlowBlock[] = []; const recordBlockKind = vi.fn(); - const mockParagraphConverter = vi.fn((para) => { + const mockParagraphConverter = vi.fn((params) => { return [ { kind: 'paragraph', - id: `p-${para.content[0].text}`, - runs: [{ text: para.content[0].text, fontFamily: 'Arial', fontSize: 12 }], + id: `p-${params.para.content[0].text}`, + runs: [{ text: params.para.content[0].text, fontFamily: 'Arial', fontSize: 12 }], }, ]; }); @@ -193,9 +194,11 @@ describe('toc', () => { defaultSize: 12, styleContext: mockStyleContext, hyperlinkConfig: mockHyperlinkConfig, + enableComments: true, + converters: { paragraphToFlowBlocks: mockParagraphConverter } as never, + converterContext: mockConverterContext, }, { blocks, recordBlockKind }, - mockParagraphConverter as never, ); expect(blocks).toHaveLength(2); @@ -223,12 +226,12 @@ describe('toc', () => { const blocks: FlowBlock[] = []; const recordBlockKind = vi.fn(); - const mockParagraphConverter = vi.fn((para) => { + const mockParagraphConverter = vi.fn((params) => { return [ { kind: 'paragraph', - id: `p-${para.content[0].text}`, - runs: [{ text: para.content[0].text, fontFamily: 'Arial', fontSize: 12 }], + id: `p-${params.para.content[0].text}`, + runs: [{ text: params.para.content[0].text, fontFamily: 'Arial', fontSize: 12 }], }, ]; }); @@ -248,9 +251,11 @@ describe('toc', () => { defaultSize: 12, styleContext: mockStyleContext, hyperlinkConfig: mockHyperlinkConfig, + enableComments: true, + converters: { paragraphToFlowBlocks: mockParagraphConverter } as never, + converterContext: mockConverterContext, }, { blocks, recordBlockKind }, - mockParagraphConverter as never, ); expect(blocks).toHaveLength(1); @@ -276,12 +281,12 @@ describe('toc', () => { const blocks: FlowBlock[] = []; const recordBlockKind = vi.fn(); - const mockParagraphConverter = vi.fn((para) => { + const mockParagraphConverter = vi.fn((params) => { return [ { kind: 'paragraph', - id: `p-${para.content[0].text}`, - runs: [{ text: para.content[0].text, fontFamily: 'Arial', fontSize: 12 }], + id: `p-${params.para.content[0].text}`, + runs: [{ text: params.para.content[0].text, fontFamily: 'Arial', fontSize: 12 }], }, ]; }); @@ -301,9 +306,11 @@ describe('toc', () => { defaultSize: 12, styleContext: mockStyleContext, hyperlinkConfig: mockHyperlinkConfig, + enableComments: true, + converters: { paragraphToFlowBlocks: mockParagraphConverter } as never, + converterContext: mockConverterContext, }, { blocks, recordBlockKind }, - mockParagraphConverter as never, ); expect(blocks).toHaveLength(1); @@ -328,12 +335,12 @@ describe('toc', () => { uniqueId: 'toc-123', }; - const mockParagraphConverter = vi.fn((para) => { + const mockParagraphConverter = vi.fn((params) => { return [ { kind: 'paragraph', - id: `p-${para.content[0].text}`, - runs: [{ text: para.content[0].text, fontFamily: 'Arial', fontSize: 12 }], + id: `p-${params.para.content[0].text}`, + runs: [{ text: params.para.content[0].text, fontFamily: 'Arial', fontSize: 12 }], }, ]; }); @@ -352,9 +359,11 @@ describe('toc', () => { defaultSize: 12, styleContext: mockStyleContext, hyperlinkConfig: mockHyperlinkConfig, + enableComments: true, + converters: { paragraphToFlowBlocks: mockParagraphConverter } as never, + converterContext: mockConverterContext, }, { blocks, recordBlockKind }, - mockParagraphConverter as never, ); expect(blocks).toHaveLength(1); @@ -385,12 +394,12 @@ describe('toc', () => { const blocks: FlowBlock[] = []; const recordBlockKind = vi.fn(); - const mockParagraphConverter = vi.fn((para) => { + const mockParagraphConverter = vi.fn((params) => { return [ { kind: 'paragraph', - id: `p-${para.content[0].text}`, - runs: [{ text: para.content[0].text, fontFamily: 'Arial', fontSize: 12 }], + id: `p-${params.para.content[0].text}`, + runs: [{ text: params.para.content[0].text, fontFamily: 'Arial', fontSize: 12 }], }, ]; }); @@ -408,9 +417,11 @@ describe('toc', () => { defaultSize: 12, styleContext: mockStyleContext, hyperlinkConfig: mockHyperlinkConfig, + enableComments: true, + converters: { paragraphToFlowBlocks: mockParagraphConverter } as never, + converterContext: mockConverterContext, }, { blocks, recordBlockKind }, - mockParagraphConverter as never, ); expect(blocks).toHaveLength(3); @@ -432,7 +443,7 @@ describe('toc', () => { const mockBookmarks = new Map([['bookmark1', 42]]); const mockTrackedChanges = { enabled: true }; - const mockParagraphConverter = vi.fn((_para) => { + const mockParagraphConverter = vi.fn((_params) => { return [ { kind: 'paragraph', @@ -455,23 +466,29 @@ describe('toc', () => { defaultSize: 14, styleContext: mockStyleContext, bookmarks: mockBookmarks, - trackedChanges: mockTrackedChanges, + trackedChangesConfig: mockTrackedChanges, hyperlinkConfig: mockHyperlinkConfig, + enableComments: false, + converters: { paragraphToFlowBlocks: mockParagraphConverter } as never, + converterContext: mockConverterContext, }, { blocks, recordBlockKind }, - mockParagraphConverter as never, ); expect(mockParagraphConverter).toHaveBeenCalledWith( - children[0], - mockBlockIdGenerator, - mockPositionMap, - 'Calibri', - 14, - mockStyleContext, - mockTrackedChanges, - mockBookmarks, - mockHyperlinkConfig, + expect.objectContaining({ + para: children[0], + nextBlockId: mockBlockIdGenerator, + positions: mockPositionMap, + defaultFont: 'Calibri', + defaultSize: 14, + styleContext: mockStyleContext, + trackedChangesConfig: mockTrackedChanges, + bookmarks: mockBookmarks, + hyperlinkConfig: mockHyperlinkConfig, + enableComments: false, + converterContext: mockConverterContext, + }), ); }); }); diff --git a/packages/layout-engine/pm-adapter/src/sdt/toc.ts b/packages/layout-engine/pm-adapter/src/sdt/toc.ts index 4c36d86ad4..ecff0c6fbd 100644 --- a/packages/layout-engine/pm-adapter/src/sdt/toc.ts +++ b/packages/layout-engine/pm-adapter/src/sdt/toc.ts @@ -14,6 +14,9 @@ import type { HyperlinkConfig, TrackedChangesConfig, NodeHandlerContext, + NestedConverters, + ConverterContext, + ThemeColorPalette, } from '../types.js'; import { applySdtMetadataToParagraphBlocks, getNodeInstruction } from './metadata.js'; @@ -96,42 +99,39 @@ export function processTocChildren( defaultSize: number; styleContext: StyleContext; bookmarks?: Map; - trackedChanges?: TrackedChangesConfig; + trackedChangesConfig?: TrackedChangesConfig; hyperlinkConfig: HyperlinkConfig; + enableComments: boolean; + converters: NestedConverters; + converterContext: ConverterContext; + themeColors?: ThemeColorPalette; }, outputArrays: { blocks: FlowBlock[]; recordBlockKind: (kind: FlowBlock['kind']) => void; }, - paragraphConverter: ( - para: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - styleContext: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig?: HyperlinkConfig, - ) => FlowBlock[], ): void { + const paragraphConverter = context.converters.paragraphToFlowBlocks; const { docPartGallery, docPartObjectId, tocInstruction } = metadata; const { blocks, recordBlockKind } = outputArrays; children.forEach((child) => { if (child.type === 'paragraph') { // Direct paragraph child - convert and tag - const paragraphBlocks = paragraphConverter( - child, - context.nextBlockId, - context.positions, - context.defaultFont, - context.defaultSize, - context.styleContext, - context.trackedChanges, - context.bookmarks, - context.hyperlinkConfig, - ); + const paragraphBlocks = paragraphConverter({ + para: child, + nextBlockId: context.nextBlockId, + positions: context.positions, + defaultFont: context.defaultFont, + defaultSize: context.defaultSize, + styleContext: context.styleContext, + trackedChangesConfig: context.trackedChangesConfig, + bookmarks: context.bookmarks, + hyperlinkConfig: context.hyperlinkConfig, + converters: context.converters, + enableComments: context.enableComments, + converterContext: context.converterContext, + }); applyTocMetadata(paragraphBlocks, { gallery: docPartGallery, @@ -157,7 +157,6 @@ export function processTocChildren( { docPartGallery, docPartObjectId, tocInstruction: finalInstruction, sdtMetadata: metadata.sdtMetadata }, context, outputArrays, - paragraphConverter, ); } }); @@ -185,17 +184,17 @@ export function handleTableOfContentsNode(node: PMNode, context: NodeHandlerCont bookmarks, hyperlinkConfig, converters, + converterContext, + themeColors, + enableComments, } = context; const tocInstruction = getNodeInstruction(node); - const paragraphToFlowBlocks = converters?.paragraphToFlowBlocks; - if (!paragraphToFlowBlocks) { - return; - } + const paragraphToFlowBlocks = converters.paragraphToFlowBlocks; node.content.forEach((child) => { if (child.type === 'paragraph') { - const paragraphBlocks = paragraphToFlowBlocks( - child, + const paragraphBlocks = paragraphToFlowBlocks({ + para: child, nextBlockId, positions, defaultFont, @@ -203,8 +202,12 @@ export function handleTableOfContentsNode(node: PMNode, context: NodeHandlerCont styleContext, trackedChangesConfig, bookmarks, + themeColors, hyperlinkConfig, - ); + converters, + enableComments, + converterContext, + }); paragraphBlocks.forEach((block) => { if (block.kind === 'paragraph') { if (!block.attrs) block.attrs = {}; diff --git a/packages/layout-engine/pm-adapter/src/types.ts b/packages/layout-engine/pm-adapter/src/types.ts index 5444c6c70d..2f3cb3d094 100644 --- a/packages/layout-engine/pm-adapter/src/types.ts +++ b/packages/layout-engine/pm-adapter/src/types.ts @@ -2,10 +2,20 @@ * Type definitions for ProseMirror to FlowBlock adapter */ -import type { TrackedChangesMode, SectionMetadata, FlowBlock, TrackedChangeMeta, Engines } from '@superdoc/contracts'; +import type { TrackedChangesMode, SectionMetadata, FlowBlock } from '@superdoc/contracts'; import type { StyleContext as StyleEngineContext, ComputedParagraphStyle } from '@superdoc/style-engine'; import type { SectionRange } from './sections/index.js'; import type { ConverterContext } from './converter-context.js'; +import type { paragraphToFlowBlocks } from './converters/paragraph.js'; +import type { tableNodeToBlock } from './converters/table.js'; +import type { contentBlockNodeToDrawingBlock } from './converters/content-block.js'; +import type { imageNodeToBlock } from './converters/image.js'; +import type { + shapeContainerNodeToDrawingBlock, + shapeGroupNodeToDrawingBlock, + shapeTextboxNodeToDrawingBlock, + vectorShapeNodeToDrawingBlock, +} from './converters/shapes.js'; export type { ConverterContext } from './converter-context.js'; export type StyleContext = StyleEngineContext; @@ -273,7 +283,7 @@ export interface NodeHandlerContext { defaultFont: string; defaultSize: number; styleContext: StyleContext; - converterContext?: ConverterContext; + converterContext: ConverterContext; // Tracked changes & hyperlinks trackedChangesConfig: TrackedChangesConfig; @@ -293,7 +303,8 @@ export interface NodeHandlerContext { }; // Converters for nested content - converters?: NestedConverters; + converters: NestedConverters; + themeColors?: ThemeColorPalette; } /** @@ -302,67 +313,47 @@ export interface NodeHandlerContext { */ export type NodeHandler = (node: PMNode, context: NodeHandlerContext) => void; -/** - * List counter context for numbering - */ - -export type ParagraphToFlowBlocksConverter = ( - para: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - styleContext: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig?: HyperlinkConfig, - themeColors?: ThemeColorPalette, - converterContext?: ConverterContext, -) => FlowBlock[]; - -export type ImageNodeToBlockConverter = ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - trackedMeta?: TrackedChangeMeta, - trackedChanges?: TrackedChangesConfig, -) => FlowBlock | null; - -export type DrawingNodeToBlockConverter = ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, -) => FlowBlock | null; - -export type TableNodeToBlockOptions = { - converters?: NestedConverters; +export type ParagraphToFlowBlocksParams = { + para: PMNode; + nextBlockId: BlockIdGenerator; + positions: PositionMap; + defaultFont: string; + defaultSize: number; + styleContext: StyleContext; + trackedChangesConfig?: TrackedChangesConfig; + hyperlinkConfig: HyperlinkConfig; + themeColors?: ThemeColorPalette; + bookmarks?: Map; + converters: NestedConverters; + enableComments: boolean; + converterContext: ConverterContext; }; -export type TableNodeToBlockConverter = ( - node: PMNode, - nextBlockId: BlockIdGenerator, - positions: PositionMap, - defaultFont: string, - defaultSize: number, - styleContext: StyleContext, - trackedChanges?: TrackedChangesConfig, - bookmarks?: Map, - hyperlinkConfig?: HyperlinkConfig, - themeColors?: ThemeColorPalette, - paragraphToFlowBlocks?: ParagraphToFlowBlocksConverter, - converterContext?: ConverterContext, - options?: TableNodeToBlockOptions, -) => FlowBlock | null; +export type TableNodeToBlockParams = { + node: PMNode; + nextBlockId: BlockIdGenerator; + positions: PositionMap; + defaultFont: string; + defaultSize: number; + styleContext: StyleContext; + trackedChangesConfig?: TrackedChangesConfig; + bookmarks?: Map; + hyperlinkConfig: HyperlinkConfig; + themeColors?: ThemeColorPalette; + converterContext: ConverterContext; + converters: NestedConverters; + enableComments: boolean; +}; export type NestedConverters = { - paragraphToFlowBlocks?: ParagraphToFlowBlocksConverter; - tableNodeToBlock?: TableNodeToBlockConverter; - contentBlockNodeToDrawingBlock?: DrawingNodeToBlockConverter; - imageNodeToBlock?: ImageNodeToBlockConverter; - vectorShapeNodeToDrawingBlock?: DrawingNodeToBlockConverter; - shapeGroupNodeToDrawingBlock?: DrawingNodeToBlockConverter; - shapeContainerNodeToDrawingBlock?: DrawingNodeToBlockConverter; - shapeTextboxNodeToDrawingBlock?: DrawingNodeToBlockConverter; + paragraphToFlowBlocks: typeof paragraphToFlowBlocks; + tableNodeToBlock: typeof tableNodeToBlock; + contentBlockNodeToDrawingBlock: typeof contentBlockNodeToDrawingBlock; + imageNodeToBlock: typeof imageNodeToBlock; + vectorShapeNodeToDrawingBlock: typeof vectorShapeNodeToDrawingBlock; + shapeGroupNodeToDrawingBlock: typeof shapeGroupNodeToDrawingBlock; + shapeContainerNodeToDrawingBlock: typeof shapeContainerNodeToDrawingBlock; + shapeTextboxNodeToDrawingBlock: typeof shapeTextboxNodeToDrawingBlock; }; /** @@ -399,10 +390,3 @@ export interface OoxmlBorder { * Underline style type derived from TextRun contract */ export type UnderlineStyle = NonNullable['style']; - -/** - * Engine type aliases - */ -export type NumberingLevelEngine = Engines.NumberingLevel; -export type EngineParagraphSpacing = Engines.ParagraphSpacing; -export type EngineParagraphIndent = Engines.ParagraphIndent; From e26e967b3b964fe3456decf38c2c12e1ddd40f56 Mon Sep 17 00:00:00 2001 From: Matthew Connelly Date: Tue, 27 Jan 2026 21:54:52 -0500 Subject: [PATCH 2/5] fix: sync presentationEditor with hidden host --- .../presentation-editor/PresentationEditor.ts | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts index 98e7bdfcc4..a1927334ca 100644 --- a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts @@ -1,5 +1,6 @@ import { NodeSelection, Selection, TextSelection } from 'prosemirror-state'; import { CellSelection } from 'prosemirror-tables'; +import { DecorationSet } from 'prosemirror-view'; import type { EditorState, Transaction } from 'prosemirror-state'; import type { Node as ProseMirrorNode, Mark } from 'prosemirror-model'; import type { Mapping } from 'prosemirror-transform'; @@ -273,6 +274,10 @@ export class PresentationEditor extends EventEmitter { #htmlAnnotationMeasureAttempts = 0; #domPositionIndex = new DomPositionIndex(); #domIndexObserverManager: DomPositionIndexObserverManager | null = null; + /** Cached list of plugins that provide decorations (plugins don't change after init) */ + #decorationPlugins: Array<{ + props: { decorations: (state: EditorState) => DecorationSet | null | undefined }; + }> | null = null; #rafHandle: number | null = null; #editorListeners: Array<{ event: string; handler: (...args: unknown[]) => void }> = []; #sectionMetadata: SectionMetadata[] = []; @@ -384,6 +389,7 @@ export class PresentationEditor extends EventEmitter { getPainterHost: () => this.#painterHost, onRebuild: () => { this.#rebuildDomPositionIndex(); + this.#syncDecorationAttributes(); this.#selectionSync.requestRender({ immediate: true }); }, }); @@ -2185,6 +2191,56 @@ export class PresentationEditor extends EventEmitter { } } + /** + * Syncs decoration classes/attributes from PM plugins to painted elements. + * Skips internal plugins (like track-changes) whose decorations the painter handles. + */ + #syncDecorationAttributes(): void { + const view = this.#editor?.view; + if (!view) return; + + const { state } = view; + + // Cache plugins with decorations, excluding painter-handled ones (plugins don't change after init) + if (this.#decorationPlugins === null) { + this.#decorationPlugins = state.plugins.filter( + (p) => p.props.decorations && p.spec.key !== TrackChangesBasePluginKey, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ) as any; + } + if (this.#decorationPlugins.length === 0) return; + + try { + for (const plugin of this.#decorationPlugins) { + const decorationSet = plugin.props.decorations.call(plugin, state); + if (!(decorationSet instanceof DecorationSet)) continue; + + for (const decoration of decorationSet.find(0, state.doc.content.size)) { + // @ts-expect-error - inline property exists but not in types + if (!decoration.inline) continue; + + const { from, to } = decoration; + // @ts-expect-error - type.attrs holds DOM attributes for inline decorations + const decorationAttrs = decoration.type?.attrs || {}; + + const classes = (decorationAttrs.class?.split(/\s+/) || []).filter((c: string) => c); + const attrs = Object.entries(decorationAttrs) + .filter(([k, v]) => k !== 'class' && (k.startsWith('data-') || k === 'style') && typeof v === 'string') + .map(([name, value]) => ({ name, value: value as string })); + + if (classes.length === 0 && attrs.length === 0) continue; + + for (const entry of this.#domPositionIndex.findEntriesInRange(from, to)) { + classes.forEach((cls: string) => entry.el.classList.add(cls)); + attrs.forEach((attr) => entry.el.setAttribute(attr.name, attr.value)); + } + } + } + } catch (error) { + debugLog('warn', 'Decoration sync failed', { error: String(error) }); + } + } + #setupEditorListeners() { const handleUpdate = ({ transaction }: { transaction?: Transaction }) => { const trackedChangesChanged = this.#syncTrackedChangesPreferences(); @@ -2992,6 +3048,7 @@ export class PresentationEditor extends EventEmitter { painter.paint(layout, this.#painterHost, mapping ?? undefined); this.#applyVertAlignToLayout(); this.#rebuildDomPositionIndex(); + this.#syncDecorationAttributes(); this.#domIndexObserverManager?.resume(); this.#layoutEpoch = layoutEpoch; if (this.#updateHtmlAnnotationMeasurements(layoutEpoch)) { From fc67a3a52facc2fc82797e37d0369d0e19419bd7 Mon Sep 17 00:00:00 2001 From: Matthew Connelly Date: Tue, 17 Feb 2026 18:45:22 -0500 Subject: [PATCH 3/5] fix: add tests for presentation editor sync --- .../presentation-editor/PresentationEditor.ts | 2 +- .../PresentationEditor.decorationSync.test.ts | 491 ++++++++++++++++++ 2 files changed, 492 insertions(+), 1 deletion(-) create mode 100644 packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.decorationSync.test.ts diff --git a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts index a1927334ca..5b42b44307 100644 --- a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts @@ -2197,7 +2197,7 @@ export class PresentationEditor extends EventEmitter { */ #syncDecorationAttributes(): void { const view = this.#editor?.view; - if (!view) return; + if (!view?.state) return; const { state } = view; diff --git a/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.decorationSync.test.ts b/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.decorationSync.test.ts new file mode 100644 index 0000000000..d293d4e7de --- /dev/null +++ b/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.decorationSync.test.ts @@ -0,0 +1,491 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { DecorationSet } from 'prosemirror-view'; +import { PluginKey } from 'prosemirror-state'; + +import { PresentationEditor } from '../PresentationEditor.js'; + +// Create a plugin key for our test highlight plugin +const testHighlightPluginKey = new PluginKey('testHighlight'); + +/** + * Creates a mock decoration object that mimics ProseMirror's Decoration.inline structure. + * The sync method accesses decoration.inline, decoration.from, decoration.to, and decoration.type.attrs + */ +const createMockDecoration = (from: number, to: number, attrs: Record) => ({ + inline: true, + from, + to, + type: { attrs }, +}); + +/** + * Creates a mock DecorationSet that the sync method can iterate over. + * The sync method calls decorationSet.find(0, docSize) and checks instanceof DecorationSet. + */ +const createMockDecorationSet = ( + decorations: Array<{ from: number; to: number; class?: string; attrs?: Record }>, +) => { + const mockDecorations = decorations.map(({ from, to, class: className, attrs }) => + createMockDecoration(from, to, { + ...(className ? { class: className } : {}), + ...attrs, + }), + ); + + // Create an object that passes instanceof DecorationSet check by using the actual prototype + const mockSet = Object.create(DecorationSet.prototype); + mockSet.find = () => mockDecorations; + return mockSet; +}; + +/** + * Creates a mock highlight plugin similar to customer implementations. + * Only mocks spec.key and props.decorations() - the two properties #syncDecorationAttributes + * reads. Real plugins have state.init/apply logic, but the sync method just reads the current + * DecorationSet, so we return a static snapshot. + */ +const createMockHighlightPlugin = ( + decorations: Array<{ from: number; to: number; class?: string; attrs?: Record }>, +) => { + return { + spec: { + key: testHighlightPluginKey, + }, + props: { + decorations: () => createMockDecorationSet(decorations), + }, + }; +}; + +/** + * PresentationEditor requires extensive mocking due to its many dependencies. + * This follows the established testing pattern used across other PresentationEditor + * test files (e.g., getElementAtPos, zoom, collaboration tests). + */ +const { + createDefaultConverter, + mockClickToPosition, + mockIncrementalLayout, + mockToFlowBlocks, + mockSelectionToRects, + mockCreateDomPainter, + mockEditorConverterStore, + mockEditorOverlayManager, + mockPlugins, +} = vi.hoisted(() => { + const createDefaultConverter = () => ({ + headers: {}, + footers: {}, + headerIds: { + default: null, + first: null, + even: null, + odd: null, + ids: [], + }, + footerIds: { + default: null, + first: null, + even: null, + odd: null, + ids: [], + }, + }); + + const converterStore = { + current: createDefaultConverter() as ReturnType & Record, + mediaFiles: {} as Record, + }; + + // Plugins array that tests can modify + const plugins: Array = []; + + return { + createDefaultConverter, + mockClickToPosition: vi.fn(() => null), + mockIncrementalLayout: vi.fn(async () => ({ layout: { pages: [] }, measures: [] })), + mockToFlowBlocks: vi.fn(() => ({ blocks: [], bookmarks: new Map() })), + mockSelectionToRects: vi.fn(() => []), + mockCreateDomPainter: vi.fn(() => ({ + paint: vi.fn(), + destroy: vi.fn(), + setZoom: vi.fn(), + setLayoutMode: vi.fn(), + setProviders: vi.fn(), + setData: vi.fn(), + })), + mockEditorConverterStore: converterStore, + mockEditorOverlayManager: vi.fn().mockImplementation(() => ({ + showEditingOverlay: vi.fn(() => ({ + success: true, + editorHost: document.createElement('div'), + reason: null, + })), + hideEditingOverlay: vi.fn(), + showSelectionOverlay: vi.fn(), + hideSelectionOverlay: vi.fn(), + setOnDimmingClick: vi.fn(), + getActiveEditorHost: vi.fn(() => null), + destroy: vi.fn(), + })), + mockPlugins: plugins, + }; +}); + +vi.mock('../../Editor.js', () => { + return { + Editor: vi.fn().mockImplementation(() => { + const domElement = document.createElement('div'); + + const mockState = { + selection: { from: 0, to: 0 }, + plugins: mockPlugins, + doc: { + nodeSize: 1000, + content: { + size: 998, + }, + descendants: vi.fn(), + nodesBetween: vi.fn(), + resolve: vi.fn((pos: number) => ({ + pos, + depth: 0, + parent: { inlineContent: true }, + })), + }, + tr: { + setSelection: vi.fn().mockReturnThis(), + }, + }; + + return { + setDocumentMode: vi.fn(), + setOptions: vi.fn(), + on: vi.fn(), + off: vi.fn(), + destroy: vi.fn(), + getJSON: vi.fn(() => ({ type: 'doc', content: [] })), + isEditable: true, + state: mockState, + view: { + dom: domElement, + focus: vi.fn(), + dispatch: vi.fn(), + state: mockState, // Also expose state on view for #syncDecorationAttributes + }, + options: { + documentId: 'test-doc', + element: document.createElement('div'), + }, + converter: mockEditorConverterStore.current, + storage: { + image: { + media: mockEditorConverterStore.mediaFiles, + }, + }, + }; + }), + }; +}); + +vi.mock('@superdoc/pm-adapter', () => ({ + toFlowBlocks: mockToFlowBlocks, +})); + +vi.mock('@superdoc/layout-bridge', () => ({ + incrementalLayout: mockIncrementalLayout, + selectionToRects: mockSelectionToRects, + clickToPosition: mockClickToPosition, + createDragHandler: vi.fn(() => () => {}), + getFragmentAtPosition: vi.fn(() => null), + computeLinePmRange: vi.fn(() => ({ from: 0, to: 0 })), + measureCharacterX: vi.fn(() => 0), + extractIdentifierFromConverter: vi.fn(() => ({ + extractHeaderId: vi.fn(() => null), + extractFooterId: vi.fn(() => null), + })), + buildMultiSectionIdentifier: vi.fn(() => ({ sections: [] })), + getHeaderFooterType: vi.fn(() => null), + getHeaderFooterTypeForSection: vi.fn(() => null), + getBucketForPageNumber: vi.fn(() => 0), + getBucketRepresentative: vi.fn(() => 0), + layoutHeaderFooterWithCache: vi.fn(async () => ({})), + computeDisplayPageNumber: vi.fn((pages: Array<{ number?: number }>) => + pages.map((p) => ({ displayText: String(p.number ?? 1) })), + ), + PageGeometryHelper: vi.fn().mockImplementation(() => ({ + updateLayout: vi.fn(), + getPageIndexAtY: vi.fn(() => 0), + getNearestPageIndex: vi.fn(() => 0), + getPageTop: vi.fn(() => 0), + getPageGap: vi.fn(() => 0), + getLayout: vi.fn(() => ({ pages: [] })), + })), +})); + +vi.mock('@superdoc/painter-dom', () => ({ + createDomPainter: mockCreateDomPainter, + DOM_CLASS_NAMES: { + PAGE: 'superdoc-page', + FRAGMENT: 'superdoc-fragment', + LINE: 'superdoc-line', + INLINE_SDT_WRAPPER: 'superdoc-structured-content-inline', + BLOCK_SDT: 'superdoc-structured-content-block', + DOCUMENT_SECTION: 'superdoc-document-section', + }, +})); + +vi.mock('../../header-footer/EditorOverlayManager.js', () => ({ + EditorOverlayManager: mockEditorOverlayManager, +})); + +/** + * Tests for #syncDecorationAttributes() - syncs ProseMirror plugin decorations to painted DOM. + * + * Coverage: + * - Class syncing: single class, multiple classes, multiple elements, range boundaries + * - Attribute syncing: data-* attributes, style attribute, combined class + attrs + * - Multiple decorations: non-overlapping ranges, overlapping ranges + * - Edge cases: empty sets, plugins without decorations, empty decorations, attribute filtering + */ +describe('PresentationEditor.decorationSync', () => { + let container: HTMLElement; + let editor: PresentationEditor; + let painterHost: HTMLElement; + + /** + * Waits for the DomPositionIndexObserverManager to process DOM mutations. + * + * When we append elements to painterHost, the MutationObserver triggers + * scheduleRebuild(), which queues onRebuild() via requestAnimationFrame. + * The observer has built-in debounce protection, so multiple mutations + * only trigger one rebuild. + * + * We use a double RAF here as a waiting mechanism (not a trigger): + * - Frame 1: The observer's scheduled RAF callback runs + * - Frame 2: We know Frame 1 completed, safe to assert + * + * This doesn't cause duplicate onRebuild calls - it's just ensuring + * the prior frame's work finished before we check results. + */ + const waitForSync = () => + new Promise((resolve) => { + requestAnimationFrame(() => { + requestAnimationFrame(() => { + resolve(); + }); + }); + }); + + /** + * Sets up the editor with the given decorations and returns the painterHost. + * Handles plugin creation, registration, and editor instantiation. + */ + const setupWithDecorations = ( + decorations: Array<{ from: number; to: number; class?: string; attrs?: Record }>, + ) => { + const plugin = createMockHighlightPlugin(decorations); + mockPlugins.push(plugin); + + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + painterHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + }; + + /** + * Creates a painted span element with PM position attributes and appends it to painterHost. + * Returns the created element for assertions. + */ + const addSpan = (start: number, end: number, text = 'text'): HTMLSpanElement => { + const span = document.createElement('span'); + span.dataset.pmStart = String(start); + span.dataset.pmEnd = String(end); + span.textContent = text; + painterHost.appendChild(span); + return span; + }; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + + vi.clearAllMocks(); + mockEditorConverterStore.current = createDefaultConverter(); + mockEditorConverterStore.mediaFiles = {}; + mockPlugins.length = 0; + }); + + afterEach(() => { + editor?.destroy(); + container?.remove(); + }); + + describe('class decoration syncing', () => { + it('applies decoration classes to painted elements within the decoration range', async () => { + setupWithDecorations([{ from: 5, to: 15, class: 'highlight-selection' }]); + const span = addSpan(5, 15); + + await waitForSync(); + + expect(span.classList.contains('highlight-selection')).toBe(true); + }); + + it('applies decoration classes to multiple elements spanning the range', async () => { + setupWithDecorations([{ from: 5, to: 25, class: 'highlight-selection' }]); + const span1 = addSpan(5, 12); + const span2 = addSpan(12, 20); + const span3 = addSpan(20, 25); + + await waitForSync(); + + expect(span1.classList.contains('highlight-selection')).toBe(true); + expect(span2.classList.contains('highlight-selection')).toBe(true); + expect(span3.classList.contains('highlight-selection')).toBe(true); + }); + + it('applies multiple CSS classes from a single decoration', async () => { + setupWithDecorations([{ from: 5, to: 15, class: 'highlight-selection focus-active custom-style' }]); + const span = addSpan(5, 15); + + await waitForSync(); + + expect(span.classList.contains('highlight-selection')).toBe(true); + expect(span.classList.contains('focus-active')).toBe(true); + expect(span.classList.contains('custom-style')).toBe(true); + }); + + it('does not apply classes to elements outside the decoration range', async () => { + setupWithDecorations([{ from: 10, to: 20, class: 'highlight-selection' }]); + const spanBefore = addSpan(1, 9, 'before'); + const spanWithin = addSpan(10, 20, 'within'); + const spanAfter = addSpan(21, 30, 'after'); + + await waitForSync(); + + expect(spanBefore.classList.contains('highlight-selection')).toBe(false); + expect(spanWithin.classList.contains('highlight-selection')).toBe(true); + expect(spanAfter.classList.contains('highlight-selection')).toBe(false); + }); + }); + + describe('data attribute syncing', () => { + it('applies data-* attributes from decorations to painted elements', async () => { + setupWithDecorations([ + { from: 5, to: 15, attrs: { 'data-highlight-id': 'highlight-123', 'data-clause-type': 'important' } }, + ]); + const span = addSpan(5, 15); + + await waitForSync(); + + expect(span.getAttribute('data-highlight-id')).toBe('highlight-123'); + expect(span.getAttribute('data-clause-type')).toBe('important'); + }); + + it('applies style attribute from decorations', async () => { + setupWithDecorations([ + { from: 5, to: 15, attrs: { style: 'background-color: yellow; border: 1px solid orange;' } }, + ]); + const span = addSpan(5, 15); + + await waitForSync(); + + expect(span.getAttribute('style')).toBe('background-color: yellow; border: 1px solid orange;'); + }); + + it('applies both classes and data attributes together', async () => { + setupWithDecorations([ + { from: 5, to: 15, class: 'highlight-selection', attrs: { 'data-highlight-id': 'test-456' } }, + ]); + const span = addSpan(5, 15); + + await waitForSync(); + + expect(span.classList.contains('highlight-selection')).toBe(true); + expect(span.getAttribute('data-highlight-id')).toBe('test-456'); + }); + }); + + describe('multiple decorations', () => { + it('handles multiple non-overlapping decorations', async () => { + setupWithDecorations([ + { from: 5, to: 10, class: 'highlight-a' }, + { from: 15, to: 20, class: 'highlight-b' }, + ]); + const span1 = addSpan(5, 10); + const span2 = addSpan(15, 20); + + await waitForSync(); + + expect(span1.classList.contains('highlight-a')).toBe(true); + expect(span1.classList.contains('highlight-b')).toBe(false); + expect(span2.classList.contains('highlight-a')).toBe(false); + expect(span2.classList.contains('highlight-b')).toBe(true); + }); + + it('handles overlapping decorations by applying all classes', async () => { + setupWithDecorations([ + { from: 5, to: 20, class: 'highlight-outer' }, + { from: 10, to: 15, class: 'highlight-inner' }, + ]); + const spanOverlap = addSpan(10, 15, 'overlap'); + const spanOuter = addSpan(5, 10, 'outer only'); + + await waitForSync(); + + expect(spanOverlap.classList.contains('highlight-outer')).toBe(true); + expect(spanOverlap.classList.contains('highlight-inner')).toBe(true); + expect(spanOuter.classList.contains('highlight-outer')).toBe(true); + expect(spanOuter.classList.contains('highlight-inner')).toBe(false); + }); + }); + + describe('edge cases', () => { + it('handles empty decoration set gracefully', async () => { + setupWithDecorations([]); + const span = addSpan(5, 15); + + await waitForSync(); + + expect(span.classList.length).toBe(0); + }); + + it('handles plugins without decorations prop', async () => { + // Manually push a plugin without decorations prop (can't use setupWithDecorations) + mockPlugins.push({ spec: { key: new PluginKey('noDecorations') }, props: {} }); + + editor = new PresentationEditor({ element: container, documentId: 'test-doc' }); + painterHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + + const span = addSpan(5, 15); + + await waitForSync(); + + expect(span.classList.length).toBe(0); + }); + + it('handles decorations with no class or attributes gracefully', async () => { + setupWithDecorations([{ from: 5, to: 15 }]); + const span = addSpan(5, 15); + + await waitForSync(); + + expect(span.classList.length).toBe(0); + }); + + it('ignores non-data and non-style attributes', async () => { + setupWithDecorations([ + { from: 5, to: 15, attrs: { 'data-valid': 'yes', id: 'should-be-ignored', onclick: 'alert("xss")' } }, + ]); + const span = addSpan(5, 15); + + await waitForSync(); + + expect(span.getAttribute('data-valid')).toBe('yes'); + expect(span.getAttribute('id')).toBeNull(); + expect(span.getAttribute('onclick')).toBeNull(); + }); + }); +}); From e052a60a3d82b4eed37041c707f5d5d9535e33d1 Mon Sep 17 00:00:00 2001 From: Matthew Connelly Date: Tue, 17 Feb 2026 19:16:16 -0500 Subject: [PATCH 4/5] fix: preserve pm-adapter exports in decorationSync test mock --- .../tests/PresentationEditor.decorationSync.test.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.decorationSync.test.ts b/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.decorationSync.test.ts index d293d4e7de..31b530d031 100644 --- a/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.decorationSync.test.ts +++ b/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.decorationSync.test.ts @@ -188,9 +188,13 @@ vi.mock('../../Editor.js', () => { }; }); -vi.mock('@superdoc/pm-adapter', () => ({ - toFlowBlocks: mockToFlowBlocks, -})); +vi.mock('@superdoc/pm-adapter', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + toFlowBlocks: mockToFlowBlocks, + }; +}); vi.mock('@superdoc/layout-bridge', () => ({ incrementalLayout: mockIncrementalLayout, From 3e6a3c489a720f87ad246c800b8ef5acf8235f0b Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Wed, 18 Feb 2026 11:19:47 -0800 Subject: [PATCH 5/5] fix(super-editor): sync external ProseMirror decorations to presentation DOM --- .../presentation-editor/PresentationEditor.ts | 119 +-- .../dom/DecorationBridge.test.ts | 748 ++++++++++++++++++ .../dom/DecorationBridge.ts | 554 +++++++++++++ .../PresentationEditor.decorationSync.test.ts | 104 ++- 4 files changed, 1470 insertions(+), 55 deletions(-) create mode 100644 packages/super-editor/src/core/presentation-editor/dom/DecorationBridge.test.ts create mode 100644 packages/super-editor/src/core/presentation-editor/dom/DecorationBridge.ts diff --git a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts index a44ce3ff41..b2693d7ec4 100644 --- a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts @@ -1,7 +1,7 @@ import { NodeSelection, Selection, TextSelection } from 'prosemirror-state'; import { SlashMenuPluginKey } from '@extensions/slash-menu/slash-menu.js'; import { CellSelection } from 'prosemirror-tables'; -import { DecorationSet } from 'prosemirror-view'; +import { DecorationBridge } from './dom/DecorationBridge.js'; import type { EditorState, Transaction } from 'prosemirror-state'; import type { Node as ProseMirrorNode, Mark } from 'prosemirror-model'; import type { Mapping } from 'prosemirror-transform'; @@ -102,6 +102,7 @@ import type { Fragment, } from '@superdoc/contracts'; import { extractHeaderFooterSpace as _extractHeaderFooterSpace } from '@superdoc/contracts'; +// TrackChangesBasePluginKey is used by #syncTrackedChangesPreferences and getTrackChangesPluginState. import { TrackChangesBasePluginKey } from '@extensions/track-changes/plugins/index.js'; // Collaboration cursor imports @@ -291,10 +292,10 @@ export class PresentationEditor extends EventEmitter { #htmlAnnotationMeasureAttempts = 0; #domPositionIndex = new DomPositionIndex(); #domIndexObserverManager: DomPositionIndexObserverManager | null = null; - /** Cached list of plugins that provide decorations (plugins don't change after init) */ - #decorationPlugins: Array<{ - props: { decorations: (state: EditorState) => DecorationSet | null | undefined }; - }> | null = null; + /** Bridges external PM plugin decorations onto painted DOM elements. */ + #decorationBridge = new DecorationBridge(); + /** RAF handle for coalesced decoration sync scheduling. */ + #decorationSyncRafHandle: number | null = null; #rafHandle: number | null = null; #editorListeners: Array<{ event: string; handler: (...args: unknown[]) => void }> = []; #scrollHandler: (() => void) | null = null; @@ -427,7 +428,7 @@ export class PresentationEditor extends EventEmitter { getPainterHost: () => this.#painterHost, onRebuild: () => { this.#rebuildDomPositionIndex(); - this.#syncDecorationAttributes(); + this.#syncDecorations(); this.#selectionSync.requestRender({ immediate: true }); }, }); @@ -2178,6 +2179,16 @@ export class PresentationEditor extends EventEmitter { }, 'Layout RAF'); } + // Cancel pending decoration sync RAF + if (this.#decorationSyncRafHandle != null) { + safeCleanup(() => { + const win = this.#visibleHost?.ownerDocument?.defaultView ?? window; + win.cancelAnimationFrame(this.#decorationSyncRafHandle!); + this.#decorationSyncRafHandle = null; + }, 'Decoration sync RAF'); + } + this.#decorationBridge.destroy(); + // Cancel pending cursor awareness update if (this.#cursorUpdateTimer !== null) { clearTimeout(this.#cursorUpdateTimer); @@ -2271,53 +2282,51 @@ export class PresentationEditor extends EventEmitter { } /** - * Syncs decoration classes/attributes from PM plugins to painted elements. - * Skips internal plugins (like track-changes) whose decorations the painter handles. + * Runs a full decoration bridge sync: reads external plugin decorations and + * reconciles them onto painted DOM elements (add/update/remove). + * + * Called synchronously from post-paint and observer-rebuild paths where the + * DOM index is guaranteed to be fresh. */ - #syncDecorationAttributes(): void { - const view = this.#editor?.view; - if (!view?.state) return; - - const { state } = view; - - // Cache plugins with decorations, excluding painter-handled ones (plugins don't change after init) - if (this.#decorationPlugins === null) { - this.#decorationPlugins = state.plugins.filter( - (p) => p.props.decorations && p.spec.key !== TrackChangesBasePluginKey, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ) as any; - } - if (this.#decorationPlugins.length === 0) return; + #syncDecorations(): void { + const state = this.#editor?.view?.state; + if (!state) return; try { - for (const plugin of this.#decorationPlugins) { - const decorationSet = plugin.props.decorations.call(plugin, state); - if (!(decorationSet instanceof DecorationSet)) continue; - - for (const decoration of decorationSet.find(0, state.doc.content.size)) { - // @ts-expect-error - inline property exists but not in types - if (!decoration.inline) continue; + this.#decorationBridge.sync(state, this.#domPositionIndex); + } catch (error) { + debugLog('warn', 'Decoration bridge sync failed', { error: String(error) }); + } + } - const { from, to } = decoration; - // @ts-expect-error - type.attrs holds DOM attributes for inline decorations - const decorationAttrs = decoration.type?.attrs || {}; + /** + * Schedules a decoration sync on the next animation frame, coalesced so + * rapid transactions (cursor movement, selection changes) don't cause + * redundant work. + * + * Skips scheduling when: + * - A rerender is already pending (post-paint will sync). + * - No DecorationSet references have actually changed (identity check). + */ + #scheduleDecorationSync(): void { + // If a full rerender is pending, the post-paint path will sync. Skip. + if (this.#renderScheduled || this.#isRerendering) return; - const classes = (decorationAttrs.class?.split(/\s+/) || []).filter((c: string) => c); - const attrs = Object.entries(decorationAttrs) - .filter(([k, v]) => k !== 'class' && (k.startsWith('data-') || k === 'style') && typeof v === 'string') - .map(([name, value]) => ({ name, value: value as string })); + // Cheap identity check: bail if no DecorationSet references changed. + const state = this.#editor?.view?.state; + if (!state || !this.#decorationBridge.hasChanges(state)) return; - if (classes.length === 0 && attrs.length === 0) continue; + // Already scheduled — RAF will handle it. + if (this.#decorationSyncRafHandle != null) return; - for (const entry of this.#domPositionIndex.findEntriesInRange(from, to)) { - classes.forEach((cls: string) => entry.el.classList.add(cls)); - attrs.forEach((attr) => entry.el.setAttribute(attr.name, attr.value)); - } - } - } - } catch (error) { - debugLog('warn', 'Decoration sync failed', { error: String(error) }); - } + const win = this.#visibleHost?.ownerDocument?.defaultView ?? window; + this.#decorationSyncRafHandle = win.requestAnimationFrame(() => { + this.#decorationSyncRafHandle = null; + // Re-check: a rerender may have been scheduled between when we queued + // this RAF and when it fires. The post-paint path will sync instead. + if (this.#renderScheduled || this.#isRerendering) return; + this.#syncDecorations(); + }); } #setupEditorListeners() { @@ -2368,10 +2377,26 @@ export class PresentationEditor extends EventEmitter { this.#updateLocalAwarenessCursor(); this.#scheduleA11ySelectionAnnouncement(); }; + + // The 'transaction' event fires for ALL transactions (doc changes, + // selection changes, meta-only). The 'update' event only fires for + // docChanged transactions, and 'selectionUpdate' only for selection + // changes. A meta-only transaction (e.g., a custom command that sets + // plugin state without editing text) fires neither. + // + // We listen on 'transaction' so the decoration bridge picks up changes + // from any transaction type. The bridge's own identity check + RAF + // coalescing prevent unnecessary work. + const handleTransaction = () => { + this.#scheduleDecorationSync(); + }; + this.#editor.on('update', handleUpdate); this.#editor.on('selectionUpdate', handleSelection); + this.#editor.on('transaction', handleTransaction); this.#editorListeners.push({ event: 'update', handler: handleUpdate as (...args: unknown[]) => void }); this.#editorListeners.push({ event: 'selectionUpdate', handler: handleSelection as (...args: unknown[]) => void }); + this.#editorListeners.push({ event: 'transaction', handler: handleTransaction as (...args: unknown[]) => void }); // Listen for page style changes (e.g., margin adjustments via ruler). // These changes don't modify document content (docChanged === false), @@ -3219,7 +3244,7 @@ export class PresentationEditor extends EventEmitter { const painterPostStart = perfNow(); this.#applyVertAlignToLayout(); this.#rebuildDomPositionIndex(); - this.#syncDecorationAttributes(); + this.#syncDecorations(); this.#domIndexObserverManager?.resume(); const painterPostEnd = perfNow(); perfLog(`[Perf] painter.postPaint: ${(painterPostEnd - painterPostStart).toFixed(2)}ms`); diff --git a/packages/super-editor/src/core/presentation-editor/dom/DecorationBridge.test.ts b/packages/super-editor/src/core/presentation-editor/dom/DecorationBridge.test.ts new file mode 100644 index 0000000000..8aeaa291bf --- /dev/null +++ b/packages/super-editor/src/core/presentation-editor/dom/DecorationBridge.test.ts @@ -0,0 +1,748 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { DecorationSet } from 'prosemirror-view'; +import { PluginKey } from 'prosemirror-state'; +import type { EditorState, Plugin } from 'prosemirror-state'; + +import { DecorationBridge } from './DecorationBridge.js'; +import { DomPositionIndex } from './DomPositionIndex.js'; + +// --------------------------------------------------------------------------- +// Test helpers +// --------------------------------------------------------------------------- + +/** Minimal decoration attrs for concise test authoring. */ +type DecoAttrs = { from: number; to: number; class?: string; attrs?: Record }; + +/** Creates a mock inline ProseMirror decoration. */ +const mockDecoration = (from: number, to: number, attrs: Record) => ({ + inline: true, + from, + to, + type: { attrs }, +}); + +/** Creates a DecorationSet-compatible object from a list of decoration descriptors. */ +const mockDecorationSet = (items: DecoAttrs[]): DecorationSet => { + const decorations = items.map(({ from, to, class: cls, attrs }) => + mockDecoration(from, to, { ...(cls ? { class: cls } : {}), ...attrs }), + ); + const set = Object.create(DecorationSet.prototype); + set.find = () => decorations; + return set; +}; + +/** Creates a mock external plugin with a decoration set. */ +const externalPlugin = (keyName: string, items: DecoAttrs[]): Plugin => { + const set = mockDecorationSet(items); + return { + key: `${keyName}$1`, + spec: { key: new PluginKey(keyName) }, + props: { decorations: () => set }, + } as unknown as Plugin; +}; + +/** Creates a mock external plugin whose decoration set can be swapped. */ +const mutableExternalPlugin = (keyName: string) => { + let currentSet: DecorationSet = DecorationSet.empty; + const plugin = { + key: `${keyName}$1`, + spec: { key: new PluginKey(keyName) }, + props: { decorations: () => currentSet }, + } as unknown as Plugin; + const setDecorations = (items: DecoAttrs[]) => { + currentSet = items.length > 0 ? mockDecorationSet(items) : DecorationSet.empty; + }; + return { plugin, setDecorations }; +}; + +/** + * Builds a minimal mock EditorState from a plugin list. + * Only the fields used by DecorationBridge are populated. + */ +const mockState = (plugins: Plugin[]): EditorState => + ({ + plugins, + doc: { content: { size: 1000 } }, + }) as unknown as EditorState; + +/** + * Creates a real DomPositionIndex backed by a container element. + * Elements appended to the container with `data-pm-start`/`data-pm-end` + * become queryable after calling `rebuild()`. + */ +const createIndex = () => { + const container = document.createElement('div'); + const index = new DomPositionIndex(); + + const addSpan = (start: number, end: number, text = 'x'): HTMLSpanElement => { + const span = document.createElement('span'); + span.dataset.pmStart = String(start); + span.dataset.pmEnd = String(end); + span.textContent = text; + container.appendChild(span); + return span; + }; + + const rebuild = () => index.rebuild(container); + + return { container, index, addSpan, rebuild }; +}; + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('DecorationBridge', () => { + let bridge: DecorationBridge; + + beforeEach(() => { + bridge = new DecorationBridge(); + }); + + afterEach(() => { + bridge.destroy(); + }); + + // ----------------------------------------------------------------------- + // Apply — fresh elements + // ----------------------------------------------------------------------- + + describe('applying decorations to fresh elements', () => { + it('applies a single class to an element within the decoration range', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const plugin = externalPlugin('highlight', [{ from: 5, to: 15, class: 'hl' }]); + bridge.sync(mockState([plugin]), index); + + expect(span.classList.contains('hl')).toBe(true); + }); + + it('applies multiple classes from a space-separated class string', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const plugin = externalPlugin('highlight', [{ from: 5, to: 15, class: 'hl-a hl-b hl-c' }]); + bridge.sync(mockState([plugin]), index); + + expect(span.classList.contains('hl-a')).toBe(true); + expect(span.classList.contains('hl-b')).toBe(true); + expect(span.classList.contains('hl-c')).toBe(true); + }); + + it('applies data-* attributes from decorations', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const plugin = externalPlugin('highlight', [ + { from: 5, to: 15, attrs: { 'data-id': '42', 'data-type': 'clause' } }, + ]); + bridge.sync(mockState([plugin]), index); + + expect(span.getAttribute('data-id')).toBe('42'); + expect(span.getAttribute('data-type')).toBe('clause'); + }); + + it('applies both classes and data attributes together', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const plugin = externalPlugin('highlight', [{ from: 5, to: 15, class: 'hl', attrs: { 'data-id': '1' } }]); + bridge.sync(mockState([plugin]), index); + + expect(span.classList.contains('hl')).toBe(true); + expect(span.getAttribute('data-id')).toBe('1'); + }); + + it('applies decorations across multiple elements spanning the range', () => { + const { index, addSpan, rebuild } = createIndex(); + const span1 = addSpan(5, 12); + const span2 = addSpan(12, 20); + const span3 = addSpan(20, 25); + rebuild(); + + const plugin = externalPlugin('highlight', [{ from: 5, to: 25, class: 'hl' }]); + bridge.sync(mockState([plugin]), index); + + expect(span1.classList.contains('hl')).toBe(true); + expect(span2.classList.contains('hl')).toBe(true); + expect(span3.classList.contains('hl')).toBe(true); + }); + + it('does not apply decorations to elements outside the range', () => { + const { index, addSpan, rebuild } = createIndex(); + const before = addSpan(1, 4); + const within = addSpan(5, 15); + const after = addSpan(16, 25); + rebuild(); + + const plugin = externalPlugin('highlight', [{ from: 5, to: 15, class: 'hl' }]); + bridge.sync(mockState([plugin]), index); + + expect(before.classList.contains('hl')).toBe(false); + expect(within.classList.contains('hl')).toBe(true); + expect(after.classList.contains('hl')).toBe(false); + }); + }); + + // ----------------------------------------------------------------------- + // Removal — stale decorations cleaned up + // ----------------------------------------------------------------------- + + describe('removing stale decorations', () => { + it('removes classes when decorations are cleared', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, class: 'hl' }]); + bridge.sync(mockState([plugin]), index); + expect(span.classList.contains('hl')).toBe(true); + + // Clear decorations and re-sync. + setDecorations([]); + bridge.sync(mockState([plugin]), index); + expect(span.classList.contains('hl')).toBe(false); + }); + + it('removes data attributes when decorations are cleared', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, attrs: { 'data-id': '42' } }]); + bridge.sync(mockState([plugin]), index); + expect(span.getAttribute('data-id')).toBe('42'); + + setDecorations([]); + bridge.sync(mockState([plugin]), index); + expect(span.getAttribute('data-id')).toBeNull(); + }); + + it('removes classes from elements that leave a shrinking range', () => { + const { index, addSpan, rebuild } = createIndex(); + const span1 = addSpan(5, 12); + const span2 = addSpan(12, 20); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + // Initially covers both spans. + setDecorations([{ from: 5, to: 20, class: 'hl' }]); + bridge.sync(mockState([plugin]), index); + expect(span1.classList.contains('hl')).toBe(true); + expect(span2.classList.contains('hl')).toBe(true); + + // Shrink range to only cover span1. + setDecorations([{ from: 5, to: 12, class: 'hl' }]); + bridge.sync(mockState([plugin]), index); + expect(span1.classList.contains('hl')).toBe(true); + expect(span2.classList.contains('hl')).toBe(false); + }); + }); + + // ----------------------------------------------------------------------- + // Update — range/attr changes + // ----------------------------------------------------------------------- + + describe('updating decorations on change', () => { + it('updates classes when decoration class changes', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, class: 'old-class' }]); + bridge.sync(mockState([plugin]), index); + expect(span.classList.contains('old-class')).toBe(true); + + setDecorations([{ from: 5, to: 15, class: 'new-class' }]); + bridge.sync(mockState([plugin]), index); + expect(span.classList.contains('old-class')).toBe(false); + expect(span.classList.contains('new-class')).toBe(true); + }); + + it('updates data attribute values', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, attrs: { 'data-id': 'v1' } }]); + bridge.sync(mockState([plugin]), index); + expect(span.getAttribute('data-id')).toBe('v1'); + + setDecorations([{ from: 5, to: 15, attrs: { 'data-id': 'v2' } }]); + bridge.sync(mockState([plugin]), index); + expect(span.getAttribute('data-id')).toBe('v2'); + }); + + it('removes stale data attributes when key is no longer present', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, attrs: { 'data-a': '1', 'data-b': '2' } }]); + bridge.sync(mockState([plugin]), index); + expect(span.getAttribute('data-a')).toBe('1'); + expect(span.getAttribute('data-b')).toBe('2'); + + // Only data-a remains. + setDecorations([{ from: 5, to: 15, attrs: { 'data-a': '1' } }]); + bridge.sync(mockState([plugin]), index); + expect(span.getAttribute('data-a')).toBe('1'); + expect(span.getAttribute('data-b')).toBeNull(); + }); + }); + + // ----------------------------------------------------------------------- + // Merge semantics + // ----------------------------------------------------------------------- + + describe('merge semantics for overlapping decorations', () => { + it('unions classes from overlapping decorations', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const plugin = externalPlugin('highlight', [ + { from: 5, to: 20, class: 'outer' }, + { from: 3, to: 15, class: 'inner' }, + ]); + bridge.sync(mockState([plugin]), index); + + expect(span.classList.contains('outer')).toBe(true); + expect(span.classList.contains('inner')).toBe(true); + }); + + it('last plugin wins for conflicting data-* keys', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const pluginA = externalPlugin('first', [{ from: 5, to: 15, attrs: { 'data-owner': 'plugin-a' } }]); + const pluginB = externalPlugin('second', [{ from: 5, to: 15, attrs: { 'data-owner': 'plugin-b' } }]); + // pluginB is later in the array, so it wins. + bridge.sync(mockState([pluginA, pluginB]), index); + + expect(span.getAttribute('data-owner')).toBe('plugin-b'); + }); + }); + + // ----------------------------------------------------------------------- + // Style property handling + // ----------------------------------------------------------------------- + + describe('style property handling', () => { + it('applies individual style properties from decoration style attribute', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const plugin = externalPlugin('highlight', [{ from: 5, to: 15, attrs: { style: 'background-color: yellow;' } }]); + bridge.sync(mockState([plugin]), index); + + expect(span.style.getPropertyValue('background-color')).toBe('yellow'); + }); + + it('applies multiple style properties from a single decoration', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const plugin = externalPlugin('highlight', [ + { + from: 5, + to: 15, + attrs: { style: 'background-color: rgba(0, 178, 169, 0.3); border: 1.5px solid rgb(229, 57, 53);' }, + }, + ]); + bridge.sync(mockState([plugin]), index); + + expect(span.style.getPropertyValue('background-color')).toBe('rgba(0, 178, 169, 0.3)'); + }); + + it('applies style properties alongside class and data-* from the same decoration', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const plugin = externalPlugin('highlight', [ + { from: 5, to: 15, class: 'hl', attrs: { style: 'color: red;', 'data-id': '1' } }, + ]); + bridge.sync(mockState([plugin]), index); + + expect(span.classList.contains('hl')).toBe(true); + expect(span.getAttribute('data-id')).toBe('1'); + expect(span.style.getPropertyValue('color')).toBe('red'); + }); + + it('removes style properties when decorations are cleared', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, attrs: { style: 'background-color: yellow;' } }]); + bridge.sync(mockState([plugin]), index); + expect(span.style.getPropertyValue('background-color')).toBe('yellow'); + + setDecorations([]); + bridge.sync(mockState([plugin]), index); + expect(span.style.getPropertyValue('background-color')).toBe(''); + }); + + it('updates style properties when decoration style changes', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, attrs: { style: 'color: red;' } }]); + bridge.sync(mockState([plugin]), index); + expect(span.style.getPropertyValue('color')).toBe('red'); + + setDecorations([{ from: 5, to: 15, attrs: { style: 'color: blue;' } }]); + bridge.sync(mockState([plugin]), index); + expect(span.style.getPropertyValue('color')).toBe('blue'); + }); + + it('removes stale style properties when they disappear from decorations', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, attrs: { style: 'color: red; font-weight: bold;' } }]); + bridge.sync(mockState([plugin]), index); + expect(span.style.getPropertyValue('color')).toBe('red'); + expect(span.style.getPropertyValue('font-weight')).toBe('bold'); + + // Only color remains. + setDecorations([{ from: 5, to: 15, attrs: { style: 'color: red;' } }]); + bridge.sync(mockState([plugin]), index); + expect(span.style.getPropertyValue('color')).toBe('red'); + expect(span.style.getPropertyValue('font-weight')).toBe(''); + }); + + it('does not clobber painter-owned style properties', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + span.style.setProperty('font-size', '14px'); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, attrs: { style: 'background-color: yellow;' } }]); + bridge.sync(mockState([plugin]), index); + + // Bridge-owned property is applied. + expect(span.style.getPropertyValue('background-color')).toBe('yellow'); + // Painter-owned property is untouched. + expect(span.style.getPropertyValue('font-size')).toBe('14px'); + + // Clear bridge decorations — painter-owned property must survive. + setDecorations([]); + bridge.sync(mockState([plugin]), index); + expect(span.style.getPropertyValue('background-color')).toBe(''); + expect(span.style.getPropertyValue('font-size')).toBe('14px'); + }); + + it('ignores empty style strings', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const plugin = externalPlugin('highlight', [{ from: 5, to: 15, attrs: { style: ' ' } }]); + bridge.sync(mockState([plugin]), index); + + expect(span.getAttribute('style')).toBeNull(); + }); + }); + + // ----------------------------------------------------------------------- + // Security filtering + // ----------------------------------------------------------------------- + + describe('security filtering', () => { + it('ignores non-data-* attributes like id, onclick, href', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const plugin = externalPlugin('highlight', [ + { from: 5, to: 15, attrs: { id: 'bad', onclick: 'alert(1)', href: 'http://evil.com', 'data-ok': 'yes' } }, + ]); + bridge.sync(mockState([plugin]), index); + + expect(span.getAttribute('id')).toBeNull(); + expect(span.getAttribute('onclick')).toBeNull(); + expect(span.getAttribute('href')).toBeNull(); + expect(span.getAttribute('data-ok')).toBe('yes'); + }); + }); + + // ----------------------------------------------------------------------- + // Repaint safety — fresh elements + // ----------------------------------------------------------------------- + + describe('repaint safety', () => { + it('applies decorations to fresh elements that replace old ones', () => { + const { container, index, addSpan, rebuild } = createIndex(); + const oldSpan = addSpan(5, 15); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, class: 'hl' }]); + bridge.sync(mockState([plugin]), index); + expect(oldSpan.classList.contains('hl')).toBe(true); + + // Simulate DomPainter repaint: remove old element, add new one. + container.removeChild(oldSpan); + const newSpan = addSpan(5, 15); + rebuild(); + + bridge.sync(mockState([plugin]), index); + expect(newSpan.classList.contains('hl')).toBe(true); + }); + }); + + // ----------------------------------------------------------------------- + // Internal plugin exclusion + // ----------------------------------------------------------------------- + + describe('internal plugin exclusion', () => { + it('excludes plugins with known internal key prefixes', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + // Simulate an internal plugin with an unexported key prefix. + const internalPlugin = { + key: 'placeholder$1', + spec: { key: new PluginKey('placeholder') }, + props: { decorations: () => mockDecorationSet([{ from: 5, to: 15, class: 'internal' }]) }, + } as unknown as Plugin; + + bridge.sync(mockState([internalPlugin]), index); + + expect(span.classList.contains('internal')).toBe(false); + }); + + it('excludes the built-in search plugin (unexported key)', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const searchPlugin = { + key: 'search$1', + spec: { key: new PluginKey('search') }, + props: { decorations: () => mockDecorationSet([{ from: 5, to: 15, class: 'search-match' }]) }, + } as unknown as Plugin; + + bridge.sync(mockState([searchPlugin]), index); + + expect(span.classList.contains('search-match')).toBe(false); + }); + }); + + // ----------------------------------------------------------------------- + // hasChanges — identity check + // ----------------------------------------------------------------------- + + describe('hasChanges identity check', () => { + it('returns true when DecorationSet reference changes', () => { + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, class: 'hl' }]); + + const { index, addSpan, rebuild } = createIndex(); + addSpan(5, 15); + rebuild(); + + const state = mockState([plugin]); + // First sync captures the initial set. + bridge.sync(state, index); + + // Same reference → no changes. + expect(bridge.hasChanges(state)).toBe(false); + + // New reference → has changes. + setDecorations([{ from: 5, to: 15, class: 'hl-new' }]); + expect(bridge.hasChanges(state)).toBe(true); + }); + + it('returns false when no eligible plugins exist and none were synced before', () => { + const state = mockState([]); + expect(bridge.hasChanges(state)).toBe(false); + }); + + it('returns true when eligible plugins drop to zero but state was previously synced', () => { + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, class: 'hl' }]); + + const { index, addSpan, rebuild } = createIndex(); + addSpan(5, 15); + rebuild(); + + // Sync once to populate prevDecorationSets. + bridge.sync(mockState([plugin]), index); + expect(bridge.hasChanges(mockState([plugin]))).toBe(false); + + // All plugins removed — bridge should detect stale state needs cleanup. + expect(bridge.hasChanges(mockState([]))).toBe(true); + }); + }); + + // ----------------------------------------------------------------------- + // Edge cases + // ----------------------------------------------------------------------- + + describe('edge cases', () => { + it('handles empty decoration set without errors', () => { + const { index, addSpan, rebuild } = createIndex(); + addSpan(5, 15); + rebuild(); + + const plugin = externalPlugin('highlight', []); + bridge.sync(mockState([plugin]), index); + // No errors thrown, no classes applied. + }); + + it('handles decorations with no class or attributes', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const plugin = externalPlugin('highlight', [{ from: 5, to: 15 }]); + bridge.sync(mockState([plugin]), index); + + expect(span.classList.length).toBe(0); + }); + + it('does not touch painter-owned classes during removal', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + span.classList.add('painter-owned'); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, class: 'bridge-class' }]); + bridge.sync(mockState([plugin]), index); + expect(span.classList.contains('painter-owned')).toBe(true); + expect(span.classList.contains('bridge-class')).toBe(true); + + // Clear bridge decorations — painter-owned class must survive. + setDecorations([]); + bridge.sync(mockState([plugin]), index); + expect(span.classList.contains('painter-owned')).toBe(true); + expect(span.classList.contains('bridge-class')).toBe(false); + }); + + it('restores painter-owned class when decoration uses the same class name', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + span.classList.add('shared-class'); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, class: 'shared-class' }]); + bridge.sync(mockState([plugin]), index); + expect(span.classList.contains('shared-class')).toBe(true); + + // Clear — painter-owned class must still be present. + setDecorations([]); + bridge.sync(mockState([plugin]), index); + expect(span.classList.contains('shared-class')).toBe(true); + }); + + it('does not touch painter-owned data attributes during removal', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + span.setAttribute('data-painter', 'yes'); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, attrs: { 'data-bridge': 'yes' } }]); + bridge.sync(mockState([plugin]), index); + + setDecorations([]); + bridge.sync(mockState([plugin]), index); + expect(span.getAttribute('data-painter')).toBe('yes'); + expect(span.getAttribute('data-bridge')).toBeNull(); + }); + + it('restores painter-owned data-attr value when decoration overwrites it', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + span.setAttribute('data-id', 'painter-value'); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, attrs: { 'data-id': 'bridge-value' } }]); + bridge.sync(mockState([plugin]), index); + expect(span.getAttribute('data-id')).toBe('bridge-value'); + + // Clear — original painter value must be restored. + setDecorations([]); + bridge.sync(mockState([plugin]), index); + expect(span.getAttribute('data-id')).toBe('painter-value'); + }); + + it('restores painter-owned style property when decoration overwrites it', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + span.style.setProperty('background-color', 'white'); + rebuild(); + + const { plugin, setDecorations } = mutableExternalPlugin('highlight'); + setDecorations([{ from: 5, to: 15, attrs: { style: 'background-color: yellow;' } }]); + bridge.sync(mockState([plugin]), index); + expect(span.style.getPropertyValue('background-color')).toBe('yellow'); + + // Clear — original painter value must be restored. + setDecorations([]); + bridge.sync(mockState([plugin]), index); + expect(span.style.getPropertyValue('background-color')).toBe('white'); + }); + }); + + // ----------------------------------------------------------------------- + // Plugin cache invalidation + // ----------------------------------------------------------------------- + + describe('plugin cache invalidation', () => { + it('picks up newly registered plugins', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + // Start with no plugins. + bridge.sync(mockState([]), index); + expect(span.classList.contains('hl')).toBe(false); + + // Register a new plugin (simulates Editor.registerPlugin). + const plugin = externalPlugin('highlight', [{ from: 5, to: 15, class: 'hl' }]); + bridge.sync(mockState([plugin]), index); + expect(span.classList.contains('hl')).toBe(true); + }); + + it('stops syncing decorations from unregistered plugins', () => { + const { index, addSpan, rebuild } = createIndex(); + const span = addSpan(5, 15); + rebuild(); + + const plugin = externalPlugin('highlight', [{ from: 5, to: 15, class: 'hl' }]); + bridge.sync(mockState([plugin]), index); + expect(span.classList.contains('hl')).toBe(true); + + // Unregister (new plugins array without the plugin). + bridge.sync(mockState([]), index); + expect(span.classList.contains('hl')).toBe(false); + }); + }); +}); diff --git a/packages/super-editor/src/core/presentation-editor/dom/DecorationBridge.ts b/packages/super-editor/src/core/presentation-editor/dom/DecorationBridge.ts new file mode 100644 index 0000000000..6fdad41d76 --- /dev/null +++ b/packages/super-editor/src/core/presentation-editor/dom/DecorationBridge.ts @@ -0,0 +1,554 @@ +import { DecorationSet } from 'prosemirror-view'; +import type { EditorState, Plugin, PluginKey } from 'prosemirror-state'; + +import { TrackChangesBasePluginKey } from '@extensions/track-changes/plugins/index.js'; +import { CommentsPluginKey } from '@extensions/comment/comments-plugin.js'; +import { customSearchHighlightsKey } from '@extensions/search/search.js'; +import { AiPluginKey } from '@extensions/ai/ai-plugin.js'; +import { CustomSelectionPluginKey } from '@extensions/custom-selection/custom-selection.js'; +import { LinkedStylesPluginKey } from '@extensions/linked-styles/plugin.js'; +import { NodeResizerKey } from '@extensions/noderesizer/noderesizer.js'; + +import type { DomPositionIndex, DomPositionIndexEntry } from './DomPositionIndex.js'; + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +/** + * Tracks what the bridge has applied to a single DOM element. + * Used for diffing on the next sync pass so stale state is removed cleanly. + * + * Prior-value maps record what existed on the element BEFORE the bridge touched + * it. On removal, the bridge restores these values instead of blindly deleting, + * so painter-owned properties are never clobbered. + */ +interface AppliedState { + classes: Set; + dataAttrs: Map; + /** Individual CSS properties applied by the bridge (property name → value). */ + styleProps: Map; + + /** Classes that existed on the element before the bridge added them. */ + priorClasses: Set; + /** Data-attr values that existed before the bridge set them (null = attr did not exist). */ + priorDataAttrs: Map; + /** Style property values before the bridge set them (empty string = prop did not exist). */ + priorStyleProps: Map; +} + +/** + * Desired decoration payload for a single DOM element, accumulated across all + * eligible plugins before being committed to the DOM. + */ +interface DesiredState { + classes: Set; + dataAttrs: Map; + /** Individual CSS properties desired by decorations (property name → value). */ + styleProps: Map; +} + +// --------------------------------------------------------------------------- +// Internal plugin exclusion +// --------------------------------------------------------------------------- + +/** + * Exported plugin keys whose decorations are rendered by the painter or other + * internal systems. Matched by reference identity (`plugin.spec.key === ref`). + */ +const EXCLUDED_PLUGIN_KEY_REF_LIST: PluginKey[] = [ + TrackChangesBasePluginKey, + CommentsPluginKey, + customSearchHighlightsKey, + AiPluginKey, + CustomSelectionPluginKey, + LinkedStylesPluginKey, + NodeResizerKey, +]; + +const EXCLUDED_PLUGIN_KEY_REFS: ReadonlySet = new Set([...EXCLUDED_PLUGIN_KEY_REF_LIST]); + +/** + * String prefixes for internal plugins whose keys are NOT exported. + * ProseMirror sets `plugin.key` to `'$'`, so we match the + * prefix before the `$` separator. + * + * | Prefix | Source file | Why excluded | + * |-------------------|------------------------------------------------|-------------------------------| + * | placeholder | extensions/placeholder/placeholder.js | Editor chrome (empty-state) | + * | tabPlugin | extensions/tab/tab.js | Layout-level tab sizing | + * | dropcapPlugin | extensions/paragraph/dropcapPlugin.js | Layout-level margin adjust | + * | ImagePosition | extensions/image/imageHelpers/imagePositionPlugin.js | Layout-level image positioning | + * | ImageRegistration | extensions/image/imageHelpers/imageRegistrationPlugin.js | Upload placeholder chrome | + * | search | extensions/search/prosemirror-search-patched.js | Painter handles search highlights | + * | yjs-cursor | y-prosemirror collaboration cursor plugin | Remote cursor UI layer | + */ +const EXCLUDED_PLUGIN_KEY_PREFIXES: readonly string[] = [ + 'placeholder', + 'tabPlugin', + 'dropcapPlugin', + 'ImagePosition', + 'ImageRegistration', + 'search', + 'yjs-cursor', +]; + +// --------------------------------------------------------------------------- +// DecorationBridge +// --------------------------------------------------------------------------- + +/** + * Bridges ProseMirror plugin decorations onto DomPainter-rendered elements. + * + * The layout engine renders into its own DOM tree, so PM decorations (which + * target the hidden contenteditable) are invisible to the user. This bridge + * reads inline decoration `class` and `data-*` attributes from eligible + * external plugins and mirrors them onto the painted elements, with a full + * add/update/remove reconciliation lifecycle. + * + * ## Ownership boundary + * The bridge tracks exactly which classes and data-attributes it has applied + * via a WeakMap keyed by DOM element. It never touches classes or attributes + * owned by the painter or other systems. + * + * ## Merge semantics + * - **Classes**: union of all classes from all overlapping decorations. + * - **`data-*` attributes**: later plugin in `state.plugins` order wins for + * the same key on the same element. + * - **`style`**: parsed into individual CSS properties and applied via + * `el.style.setProperty()` so painter-owned properties are never clobbered. + * Later plugin wins per CSS property name. + */ +export class DecorationBridge { + /** Tracks bridge-owned state per painted DOM element. */ + #applied = new WeakMap(); + + /** Cached list of plugins eligible for bridging. */ + #eligiblePlugins: Plugin[] = []; + + /** Identity snapshot of `state.plugins` when `#eligiblePlugins` was last built. */ + #pluginListSnapshot: readonly Plugin[] = []; + + /** Last-seen DecorationSet per plugin, for cheap identity-based skip. */ + #prevDecorationSets = new Map(); + + /** True if the last sync had at least one eligible plugin. Used to detect the → 0 transition. */ + #hadEligiblePlugins = false; + + // ------------------------------------------------------------------------- + // Public API + // ------------------------------------------------------------------------- + + /** + * Runs a full reconciliation pass: reads decoration state from eligible PM + * plugins, maps them to painted DOM via the position index, and diffs + * against previously applied state. + * + * @returns `true` if any DOM mutations were made, `false` if skipped. + */ + sync(state: EditorState, domIndex: DomPositionIndex): boolean { + this.#refreshEligiblePlugins(state); + + const docSize = state.doc.content.size; + const desired = + this.#eligiblePlugins.length > 0 + ? this.#collectDesiredState(state, domIndex, docSize) + : new Map(); + + this.#hadEligiblePlugins = this.#eligiblePlugins.length > 0; + return this.#reconcile(desired, domIndex, docSize); + } + + /** + * Checks whether any eligible plugin's DecorationSet has changed since the + * last sync. Use this as a cheap gate before calling `sync()`. + * + * @returns `true` if at least one DecorationSet reference changed. + */ + hasChanges(state: EditorState): boolean { + this.#refreshEligiblePlugins(state); + + // Transition from some plugins → zero: stale bridge state needs cleanup. + if (this.#eligiblePlugins.length === 0) { + return this.#hadEligiblePlugins; + } + + for (const plugin of this.#eligiblePlugins) { + const currentSet = this.#getDecorationSet(plugin, state); + if (currentSet !== this.#prevDecorationSets.get(plugin)) return true; + } + return false; + } + + /** + * Removes all bridge-owned classes and data-attributes from the DOM. + * Called during teardown. + */ + destroy(): void { + this.#eligiblePlugins = []; + this.#pluginListSnapshot = []; + this.#prevDecorationSets.clear(); + this.#hadEligiblePlugins = false; + // WeakMap entries are garbage collected with their elements. + } + + // ------------------------------------------------------------------------- + // Plugin filtering + // ------------------------------------------------------------------------- + + /** + * Rebuilds the eligible plugin list when the plugin array has changed. + * Uses a two-tier strategy: + * 1. Exclude by exported PluginKey reference (7 known internal keys). + * 2. Exclude by plugin.key string prefix (5 unexported internal keys). + */ + #refreshEligiblePlugins(state: EditorState): void { + if (state.plugins === this.#pluginListSnapshot) return; + + this.#pluginListSnapshot = state.plugins; + this.#eligiblePlugins = state.plugins.filter((plugin) => { + if (!plugin.props.decorations) return false; + if (this.#isExcludedByKeyRef(plugin)) return false; + if (this.#isExcludedByKeyPrefix(plugin)) return false; + return true; + }); + + // Prune stale entries from the identity map. + const eligibleSet = new Set(this.#eligiblePlugins); + for (const key of this.#prevDecorationSets.keys()) { + if (!eligibleSet.has(key)) this.#prevDecorationSets.delete(key); + } + } + + /** Checks if a plugin's key matches one of the exported internal PluginKey references. */ + #isExcludedByKeyRef(plugin: Plugin): boolean { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const specKey = (plugin as any).spec?.key; + return specKey != null && EXCLUDED_PLUGIN_KEY_REFS.has(specKey); + } + + /** Checks if a plugin's key string starts with a known internal prefix. */ + #isExcludedByKeyPrefix(plugin: Plugin): boolean { + // ProseMirror formats plugin.key as '$'. + const keyString: string = (plugin as unknown as Record).key ?? ''; + return EXCLUDED_PLUGIN_KEY_PREFIXES.some((prefix) => keyString === prefix || keyString.startsWith(`${prefix}$`)); + } + + // ------------------------------------------------------------------------- + // Decoration collection + // ------------------------------------------------------------------------- + + /** + * Reads inline decorations from all eligible plugins and accumulates + * desired class/data-attr state per painted DOM element. + * + * Returns a Map of DOM element → desired state. Elements that are in the + * position index but have no decorations are NOT included (they'll be + * handled as removals in reconcile). + */ + #collectDesiredState( + state: EditorState, + domIndex: DomPositionIndex, + docSize: number, + ): Map { + const desired = new Map(); + + for (const plugin of this.#eligiblePlugins) { + const decorationSet = this.#getDecorationSet(plugin, state); + this.#prevDecorationSets.set(plugin, decorationSet); + if (decorationSet === DecorationSet.empty) continue; + + const decorations = decorationSet.find(0, docSize); + for (const decoration of decorations) { + if (!this.#isInlineDecoration(decoration)) continue; + + const attrs = this.#extractSafeAttrs(decoration); + if (attrs.classes.length === 0 && attrs.dataEntries.length === 0 && attrs.styleEntries.length === 0) continue; + + const entries = domIndex.findEntriesInRange(decoration.from, decoration.to); + for (const entry of entries) { + const state = this.#getOrCreateDesired(desired, entry.el); + for (const cls of attrs.classes) state.classes.add(cls); + for (const [key, value] of attrs.dataEntries) state.dataAttrs.set(key, value); + for (const [prop, value] of attrs.styleEntries) state.styleProps.set(prop, value); + } + } + } + + return desired; + } + + /** Safely retrieves the DecorationSet from a plugin, returning empty on failure. */ + #getDecorationSet(plugin: Plugin, state: EditorState): DecorationSet { + try { + const result = plugin.props.decorations?.call(plugin, state); + return result instanceof DecorationSet ? result : DecorationSet.empty; + } catch { + return DecorationSet.empty; + } + } + + /** Checks if a decoration is an inline decoration (not widget or node). */ + #isInlineDecoration(decoration: { from: number; to: number }): boolean { + // @ts-expect-error - ProseMirror's internal `inline` flag is not typed. + return decoration.inline === true; + } + + /** + * Extracts bridge-safe attributes from a decoration: + * - `class` is split into individual class names. + * - `data-*` attributes are preserved. + * - `style` is parsed into individual CSS properties (property-level, not raw string). + * - All other attributes (id, onclick, href, etc.) are ignored for security. + */ + #extractSafeAttrs(decoration: { from: number; to: number }): { + classes: string[]; + dataEntries: [string, string][]; + styleEntries: [string, string][]; + } { + // @ts-expect-error - ProseMirror's `type.attrs` is not in the public types. + const raw: Record = decoration.type?.attrs ?? {}; + + const classes = typeof raw.class === 'string' ? raw.class.split(/\s+/).filter((c: string) => c.length > 0) : []; + + const dataEntries: [string, string][] = []; + for (const [key, value] of Object.entries(raw)) { + if (key === 'class' || key === 'style') continue; + if (!key.startsWith('data-')) continue; + if (typeof value !== 'string') continue; + dataEntries.push([key, value]); + } + + const styleEntries: [string, string][] = + typeof raw.style === 'string' ? DecorationBridge.#parseStyleString(raw.style) : []; + + return { classes, dataEntries, styleEntries }; + } + + /** + * Parses a CSS style string into individual [property, value] pairs. + * Uses a temporary element so the browser handles shorthand expansion, + * vendor prefixes, and validation. + */ + static #parseStyleString(cssText: string): [string, string][] { + if (!cssText.trim()) return []; + + const temp = document.createElement('span'); + temp.style.cssText = cssText; + + const entries: [string, string][] = []; + for (let i = 0; i < temp.style.length; i++) { + const prop = temp.style.item(i); + const value = temp.style.getPropertyValue(prop); + if (prop && value) entries.push([prop, value]); + } + return entries; + } + + /** Gets or creates the desired state for an element. */ + #getOrCreateDesired(map: Map, el: HTMLElement): DesiredState { + let state = map.get(el); + if (!state) { + state = { classes: new Set(), dataAttrs: new Map(), styleProps: new Map() }; + map.set(el, state); + } + return state; + } + + // ------------------------------------------------------------------------- + // Reconciliation + // ------------------------------------------------------------------------- + + /** + * Diffs desired state against previously applied state and updates the DOM. + * + * Three cases per element: + * 1. **New element** (in desired, not in applied): apply all desired state. + * 2. **Updated element** (in both): add new, remove stale. + * 3. **Removed element** (in applied, not in desired): remove all bridge state. + * + * Case 3 is handled by scanning the position index for elements that have + * applied state but no desired state. + */ + #reconcile(desired: Map, domIndex: DomPositionIndex, docSize: number): boolean { + let mutated = false; + + // Apply or update: iterate elements that should have decorations. + for (const [el, desiredState] of desired) { + const applied = this.#applied.get(el); + + if (!applied) { + // Case 1: fresh element, no prior state. + this.#applyFresh(el, desiredState); + mutated = true; + } else { + // Case 2: element has prior state — diff and update. + if (this.#applyDiff(el, applied, desiredState)) mutated = true; + } + } + + // Case 3: remove stale state from elements no longer covered. + // We scan all indexed elements and check for orphaned applied state. + const allEntries = docSize > 0 ? domIndex.findEntriesInRange(0, docSize) : []; + for (const entry of allEntries) { + if (desired.has(entry.el)) continue; + + const applied = this.#applied.get(entry.el); + if (!applied) continue; + + this.#removeAll(entry.el, applied); + mutated = true; + } + + return mutated; + } + + /** + * Applies decoration state to a fresh element (no prior bridge state). + */ + #applyFresh(el: HTMLElement, desired: DesiredState): void { + const tracked: AppliedState = { + classes: new Set(), + dataAttrs: new Map(), + styleProps: new Map(), + priorClasses: new Set(), + priorDataAttrs: new Map(), + priorStyleProps: new Map(), + }; + + for (const cls of desired.classes) { + if (el.classList.contains(cls)) tracked.priorClasses.add(cls); + el.classList.add(cls); + tracked.classes.add(cls); + } + for (const [key, value] of desired.dataAttrs) { + const prior = el.getAttribute(key); + if (prior !== null) tracked.priorDataAttrs.set(key, prior); + el.setAttribute(key, value); + tracked.dataAttrs.set(key, value); + } + for (const [prop, value] of desired.styleProps) { + const prior = el.style.getPropertyValue(prop); + if (prior) tracked.priorStyleProps.set(prop, prior); + el.style.setProperty(prop, value); + tracked.styleProps.set(prop, value); + } + + this.#applied.set(el, tracked); + } + + /** + * Diffs desired vs applied state and makes minimal DOM updates. + * @returns `true` if any DOM mutations were made. + */ + #applyDiff(el: HTMLElement, applied: AppliedState, desired: DesiredState): boolean { + let mutated = false; + + // Classes: add new, remove stale (restoring painter-owned on removal). + for (const cls of desired.classes) { + if (!applied.classes.has(cls)) { + if (el.classList.contains(cls)) applied.priorClasses.add(cls); + el.classList.add(cls); + applied.classes.add(cls); + mutated = true; + } + } + for (const cls of applied.classes) { + if (!desired.classes.has(cls)) { + if (!applied.priorClasses.has(cls)) { + el.classList.remove(cls); + } + applied.priorClasses.delete(cls); + applied.classes.delete(cls); + mutated = true; + } + } + + // Data attributes: add/update new, remove stale (restoring prior values). + for (const [key, value] of desired.dataAttrs) { + if (applied.dataAttrs.get(key) !== value) { + if (!applied.dataAttrs.has(key)) { + const prior = el.getAttribute(key); + if (prior !== null) applied.priorDataAttrs.set(key, prior); + } + el.setAttribute(key, value); + applied.dataAttrs.set(key, value); + mutated = true; + } + } + for (const key of applied.dataAttrs.keys()) { + if (!desired.dataAttrs.has(key)) { + const prior = applied.priorDataAttrs.get(key); + if (prior != null) { + el.setAttribute(key, prior); + } else { + el.removeAttribute(key); + } + applied.priorDataAttrs.delete(key); + applied.dataAttrs.delete(key); + mutated = true; + } + } + + // Style properties: add/update new, remove stale (restoring prior values). + for (const [prop, value] of desired.styleProps) { + if (applied.styleProps.get(prop) !== value) { + if (!applied.styleProps.has(prop)) { + const prior = el.style.getPropertyValue(prop); + if (prior) applied.priorStyleProps.set(prop, prior); + } + el.style.setProperty(prop, value); + applied.styleProps.set(prop, value); + mutated = true; + } + } + for (const prop of applied.styleProps.keys()) { + if (!desired.styleProps.has(prop)) { + const prior = applied.priorStyleProps.get(prop); + if (prior) { + el.style.setProperty(prop, prior); + } else { + el.style.removeProperty(prop); + } + applied.priorStyleProps.delete(prop); + applied.styleProps.delete(prop); + mutated = true; + } + } + + // If all bridge state was removed, clean up the WeakMap entry. + if (applied.classes.size === 0 && applied.dataAttrs.size === 0 && applied.styleProps.size === 0) { + this.#applied.delete(el); + } + + return mutated; + } + + /** + * Removes all bridge-owned state from an element. + */ + #removeAll(el: HTMLElement, applied: AppliedState): void { + for (const cls of applied.classes) { + if (!applied.priorClasses.has(cls)) { + el.classList.remove(cls); + } + } + for (const key of applied.dataAttrs.keys()) { + const prior = applied.priorDataAttrs.get(key); + if (prior != null) { + el.setAttribute(key, prior); + } else { + el.removeAttribute(key); + } + } + for (const prop of applied.styleProps.keys()) { + const prior = applied.priorStyleProps.get(prop); + if (prior) { + el.style.setProperty(prop, prior); + } else { + el.style.removeProperty(prop); + } + } + this.#applied.delete(el); + } +} diff --git a/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.decorationSync.test.ts b/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.decorationSync.test.ts index 31b530d031..208bde15c1 100644 --- a/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.decorationSync.test.ts +++ b/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.decorationSync.test.ts @@ -57,6 +57,25 @@ const createMockHighlightPlugin = ( }; }; +/** + * Creates a mutable mock plugin whose decoration set can be swapped at runtime. + * Simulates the real customer flow: a command dispatches a setMeta transaction, + * the plugin's apply() returns a new DecorationSet, and the bridge picks it up. + */ +const createMutableMockPlugin = () => { + let currentSet: ReturnType = createMockDecorationSet([]); + const plugin = { + spec: { key: testHighlightPluginKey }, + props: { decorations: () => currentSet }, + }; + const setDecorations = ( + items: Array<{ from: number; to: number; class?: string; attrs?: Record }>, + ) => { + currentSet = items.length > 0 ? createMockDecorationSet(items) : DecorationSet.empty; + }; + return { plugin, setDecorations }; +}; + /** * PresentationEditor requires extensive mocking due to its many dependencies. * This follows the established testing pattern used across other PresentationEditor @@ -72,6 +91,7 @@ const { mockEditorConverterStore, mockEditorOverlayManager, mockPlugins, + mockEditorOn, } = vi.hoisted(() => { const createDefaultConverter = () => ({ headers: {}, @@ -100,6 +120,9 @@ const { // Plugins array that tests can modify const plugins: Array = []; + // Shared mock for editor.on() — lets tests extract registered event handlers. + const editorOn = vi.fn(); + return { createDefaultConverter, mockClickToPosition: vi.fn(() => null), @@ -129,6 +152,7 @@ const { destroy: vi.fn(), })), mockPlugins: plugins, + mockEditorOn: editorOn, }; }); @@ -161,7 +185,7 @@ vi.mock('../../Editor.js', () => { return { setDocumentMode: vi.fn(), setOptions: vi.fn(), - on: vi.fn(), + on: mockEditorOn, off: vi.fn(), destroy: vi.fn(), getJSON: vi.fn(() => ({ type: 'doc', content: [] })), @@ -244,13 +268,18 @@ vi.mock('../../header-footer/EditorOverlayManager.js', () => ({ })); /** - * Tests for #syncDecorationAttributes() - syncs ProseMirror plugin decorations to painted DOM. + * Integration tests for decoration bridge sync via PresentationEditor. + * + * These tests verify that DecorationBridge is wired correctly into PresentationEditor's + * lifecycle (observer-triggered rebuild → sync). For unit-level tests of bridge reconciliation + * logic, see DecorationBridge.test.ts. * * Coverage: * - Class syncing: single class, multiple classes, multiple elements, range boundaries - * - Attribute syncing: data-* attributes, style attribute, combined class + attrs + * - Attribute syncing: data-* attributes, combined class + attrs * - Multiple decorations: non-overlapping ranges, overlapping ranges * - Edge cases: empty sets, plugins without decorations, empty decorations, attribute filtering + * - Style properties applied at the property level (setProperty/removeProperty) */ describe('PresentationEditor.decorationSync', () => { let container: HTMLElement; @@ -388,15 +417,13 @@ describe('PresentationEditor.decorationSync', () => { expect(span.getAttribute('data-clause-type')).toBe('important'); }); - it('applies style attribute from decorations', async () => { - setupWithDecorations([ - { from: 5, to: 15, attrs: { style: 'background-color: yellow; border: 1px solid orange;' } }, - ]); + it('applies style properties from decoration style attribute', async () => { + setupWithDecorations([{ from: 5, to: 15, attrs: { style: 'background-color: yellow;' } }]); const span = addSpan(5, 15); await waitForSync(); - expect(span.getAttribute('style')).toBe('background-color: yellow; border: 1px solid orange;'); + expect(span.style.getPropertyValue('background-color')).toBe('yellow'); }); it('applies both classes and data attributes together', async () => { @@ -492,4 +519,65 @@ describe('PresentationEditor.decorationSync', () => { expect(span.getAttribute('onclick')).toBeNull(); }); }); + + // ----------------------------------------------------------------------- + // Transaction-driven sync (the real customer flow) + // ----------------------------------------------------------------------- + + describe('transaction-driven decoration sync', () => { + /** + * Extracts the 'transaction' event handler that PresentationEditor registered + * on the mock editor. This simulates the real customer flow: a command dispatches + * a setMeta transaction → Editor fires 'transaction' → bridge syncs. + */ + const getTransactionHandler = (): (() => void) => { + const onCalls: Array<[string, () => void]> = mockEditorOn.mock.calls; + const match = onCalls.find(([event]) => event === 'transaction'); + if (!match) throw new Error('No transaction handler registered on mock editor'); + return match[1]; + }; + + it('syncs decorations when a transaction fires (setMeta customer flow)', async () => { + const { plugin, setDecorations } = createMutableMockPlugin(); + mockPlugins.push(plugin); + + editor = new PresentationEditor({ element: container, documentId: 'test-doc' }); + painterHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + const span = addSpan(5, 15); + + // Wait for initial MutationObserver sync (no decorations yet). + await waitForSync(); + expect(span.classList.contains('highlight-selection')).toBe(false); + + // Simulate the customer command: plugin state updates, then transaction fires. + setDecorations([{ from: 5, to: 15, class: 'highlight-selection' }]); + const fireTransaction = getTransactionHandler(); + fireTransaction(); + + await waitForSync(); + expect(span.classList.contains('highlight-selection')).toBe(true); + }); + + it('clears decorations when plugin state is emptied via transaction', async () => { + const { plugin, setDecorations } = createMutableMockPlugin(); + setDecorations([{ from: 5, to: 15, class: 'highlight-selection' }]); + mockPlugins.push(plugin); + + editor = new PresentationEditor({ element: container, documentId: 'test-doc' }); + painterHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + const span = addSpan(5, 15); + + // Wait for initial sync — highlight should be applied. + await waitForSync(); + expect(span.classList.contains('highlight-selection')).toBe(true); + + // Clear decorations and fire transaction (simulates clearHighlight command). + setDecorations([]); + const fireTransaction = getTransactionHandler(); + fireTransaction(); + + await waitForSync(); + expect(span.classList.contains('highlight-selection')).toBe(false); + }); + }); });