Merge branch 'main' into tadeu/sd-3347-feature-render-word-style-underlines

Author: harbournick

.github/workflows/superdoc-oss-sync-to-labs.yml

@@ -3,7 +3,7 @@ name: Notify Labs of OSS changes
 on:
   push:
     branches:
-      - nick/sync
+      - main
 
 permissions:
   contents: read

demos/editor/custom-ui/src/components/ActivitySidebar.tsx

@@ -70,9 +70,32 @@ export function ActivitySidebar({ composeOpen, onCloseComposer, decided }: Props
   // (separate ticket), we'll be able to interleave by document
   // position; until then this stable two-bucket ordering matches what
   // the controller used to do internally.
+  //
+  // SuperDoc models tracked changes as comment-linked entities, so
+  // `ui.comments.items` mirrors each tracked change as a synthetic
+  // comment whose id is the tracked-change id. Without de-duping, every
+  // suggestion shows twice (one comment card + one change card).
+  //
+  // We dedupe by id, NOT by the `trackedChange` flag: a real comment
+  // thread also gets `trackedChange: true` when its anchor overlaps a
+  // suggestion (`comments.list()` links it via `assignTrackedChangeLink`),
+  // so a flag filter would wrongly hide those discussions. Only the
+  // synthetic rows reuse a tracked-change id; real comments have their
+  // own. Note: this demo runs `replacements: 'independent'`, so both
+  // sides of a replacement surface as separate change rows and both
+  // synthetic comment ids land in the dedupe set. Under the default
+  // `'paired'` mode the collapsed row drops the delete-side id, which
+  // `trackChanges.list()` does not currently expose — the delete-side
+  // synthetic comment would leak as a duplicate. Engine follow-up needed
+  // before this pattern is safe in paired mode.
   const feed = useMemo<ActivityItem[]>(() => {
+    const changeIds = new Set<string>();
+    for (const tc of trackChanges.items) changeIds.add(tc.id);
     const items: ActivityItem[] = [];
-    for (const c of comments.items) items.push({ kind: 'comment', id: c.id, comment: c });
+    for (const c of comments.items) {
+      if (changeIds.has(c.id)) continue;
+      items.push({ kind: 'comment', id: c.id, comment: c });
+    }
     for (const tc of trackChanges.items) items.push({ kind: 'change', id: tc.id, change: tc.change });
     return items;
   }, [comments.items, trackChanges.items]);

packages/super-editor/src/editors/v1/core/layout-adapter/attributes/paragraph.ts

@@ -31,12 +31,12 @@ import {
   resolveParagraphProperties,
   resolveRunProperties,
   resolveDocxFontFamily,
-  getNumberingProperties,
   type ParagraphFrameProperties,
   type ParagraphProperties,
   type RunProperties,
 } from '@superdoc/style-engine/ooxml';
 import { resolveSectionDirection, resolveParagraphDirection } from '../direction/index.js';
+import { numberingDefinesMarkerFontFamily } from '../numbering-marker-font.js';
 
 const DEFAULT_DECIMAL_SEPARATOR = '.';
 const DEFAULT_TAB_INTERVAL_TWIPS = 720; // 0.5 inch
@@ -421,20 +421,14 @@ export const computeParagraphAttrs = (
     // fully defines marker font.
     let markerFontFallback: Partial<ParagraphFont> | undefined;
     if (!hasExplicitParagraphRunProperties(paragraphProperties) && previousParagraphFont) {
-      // Detect whether numbering explicitly overrides the marker font family
-      // (e.g. Symbol/Wingdings). If it does, we must NOT overwrite it.
-      const numProps = paragraphProperties.numberingProperties;
-      const numId = numProps?.numId;
-      const ilvl = numProps?.ilvl ?? 0;
-      const numberingRunProps =
-        numId != null && numId !== 0
-          ? getNumberingProperties<RunProperties>('runProperties', converterContext!, ilvl, numId)
-          : ({} as RunProperties);
-      const numberingDefinesMarkerFontFamily = numberingRunProps.fontFamily != null;
+      const pinsMarkerFontFamily = numberingDefinesMarkerFontFamily(
+        paragraphProperties.numberingProperties,
+        converterContext,
+      );
 
       markerFontFallback = {
         // When numbering explicitly sets a marker font (Symbol/Wingdings), keep it.
-        fontFamily: numberingDefinesMarkerFontFamily ? undefined : previousParagraphFont.fontFamily,
+        fontFamily: pinsMarkerFontFamily ? undefined : previousParagraphFont.fontFamily,
         // Preserve existing behavior: if the paragraph has no explicit run props,
         // marker font size inherits from the previous paragraph.
         fontSize: previousParagraphFont.fontSize,

packages/super-editor/src/editors/v1/core/layout-adapter/converters/paragraph.test.ts

@@ -352,6 +352,44 @@ describe('getLastParagraphFont', () => {
     const result = getLastParagraphFont(blocks);
     expect(result).toBeUndefined();
   });
+
+  it('skips empty first runs even when they carry stale font values', () => {
+    const blocks: FlowBlock[] = [
+      {
+        kind: 'paragraph',
+        id: '0-paragraph',
+        runs: [
+          {
+            kind: 'text',
+            text: 'Valid',
+            fontFamily: 'ValidFont',
+            fontSize: 11,
+            pmStart: 0,
+            pmEnd: 5,
+          },
+        ],
+        attrs: {},
+      },
+      {
+        kind: 'paragraph',
+        id: '1-paragraph',
+        runs: [
+          {
+            kind: 'text',
+            text: '',
+            fontFamily: 'StaleFont',
+            fontSize: 42,
+            pmStart: 5,
+            pmEnd: 5,
+          },
+        ],
+        attrs: {},
+      },
+    ];
+
+    const result = getLastParagraphFont(blocks);
+    expect(result).toEqual({ fontFamily: 'ValidFont', fontSize: 11 });
+  });
 });
 
 describe('paragraph converters', () => {

packages/super-editor/src/editors/v1/core/layout-adapter/converters/paragraph.ts

@@ -37,6 +37,7 @@ import { applyTrackedChangesModeToRuns } from '../tracked-changes.js';
 import { textNodeToRun } from './inline-converters/text-run.js';
 import { DEFAULT_HYPERLINK_CONFIG, TOKEN_INLINE_TYPES } from '../constants.js';
 import { computeRunAttrs, hasExplicitParagraphRunProperties } from '../attributes/paragraph.js';
+import { syncListMarkerFontFromParagraphRuns } from '../list-marker-font.js';
 import { resolveRunProperties } from '@superdoc/style-engine/ooxml';
 import { footnoteReferenceToBlock } from './inline-converters/footnote-reference.js';
 import { endnoteReferenceToBlock } from './inline-converters/endnote-reference.js';
@@ -604,6 +605,19 @@ export function paragraphToFlowBlocks({
   const defaultSize =
     usePreviousFont && previousParagraphFont.fontSize ? previousParagraphFont.fontSize : extracted.defaultSize;
 
+  const finalizeParagraphBlocks = (outputBlocks: FlowBlock[]): FlowBlock[] => {
+    outputBlocks.forEach((block) => {
+      if (block.kind === 'paragraph') {
+        syncListMarkerFontFromParagraphRuns({
+          block,
+          converterContext,
+          para,
+        });
+      }
+    });
+    return outputBlocks;
+  };
+
   if (paragraphAttrs.pageBreakBefore) {
     blocks.push({
       kind: 'pageBreak',
@@ -615,7 +629,7 @@ export function paragraphToFlowBlocks({
 
   if (!para.content || para.content.length === 0) {
     if (paragraphProps.runProperties?.vanish) {
-      return blocks;
+      return finalizeParagraphBlocks(blocks);
     }
     const paragraphMarkTrackedChange = getParagraphMarkTrackedChange(paragraphProps, storyKey);
     // Get the PM position of the empty paragraph for caret rendering
@@ -650,12 +664,12 @@ export function paragraphToFlowBlocks({
       sourceAnchor,
     });
     if (!trackedChangesConfig) {
-      return blocks;
+      return finalizeParagraphBlocks(blocks);
     }
 
     const paragraphBlock = blocks[blocks.length - 1];
     if (paragraphBlock?.kind !== 'paragraph') {
-      return blocks;
+      return finalizeParagraphBlocks(blocks);
     }
 
     const filteredRuns = applyTrackedChangesModeToRuns(
@@ -682,7 +696,7 @@ export function paragraphToFlowBlocks({
 
     if (trackedChangesConfig.enabled && (filteredRuns.length === 0 || isGhostTrackedListArtifact)) {
       blocks.pop();
-      return blocks;
+      return finalizeParagraphBlocks(blocks);
     }
 
     paragraphBlock.runs = filteredRuns;
@@ -691,7 +705,7 @@ export function paragraphToFlowBlocks({
       trackedChangesMode: trackedChangesConfig.mode,
       trackedChangesEnabled: trackedChangesConfig.enabled,
     };
-    return blocks;
+    return finalizeParagraphBlocks(blocks);
   }
 
   let currentRuns: Run[] = [];
@@ -923,7 +937,7 @@ export function paragraphToFlowBlocks({
   });
 
   if (!trackedChangesConfig) {
-    return blocks;
+    return finalizeParagraphBlocks(blocks);
   }
 
   const processedBlocks: FlowBlock[] = [];
@@ -953,7 +967,7 @@ export function paragraphToFlowBlocks({
     processedBlocks.push(block);
   });
 
-  return processedBlocks;
+  return finalizeParagraphBlocks(processedBlocks);
 }
 
 type InlineConverterSpec = {
@@ -1072,7 +1086,10 @@ export function getLastParagraphFont(blocks: FlowBlock[]): ParagraphFont | undef
       const para = block as ParagraphBlock;
       const firstRun = para.runs?.[0];
       if (!firstRun) continue;
-      const run = firstRun as { fontFamily?: string; fontSize?: number };
+      const run = firstRun as { text?: string; fontFamily?: string; fontSize?: number };
+      if (typeof run.text === 'string' && run.text.length === 0) {
+        continue;
+      }
       const fontFamily = typeof run.fontFamily === 'string' ? run.fontFamily.trim() : '';
       const fontSize = typeof run.fontSize === 'number' && Number.isFinite(run.fontSize) ? run.fontSize : NaN;
       if (fontFamily.length > 0 && fontSize > 0) {
@@ -1142,15 +1159,26 @@ export function handleParagraphNode(node: PMNode, context: NodeHandlerContext):
     // get() returns both the entry (if hit) and pre-computed nodeJson to avoid double serialization
     const { entry: cached, nodeJson, nodeRev } = flowBlockCache.get(prefixedStableId, node);
     if (cached) {
-      // Cache hit: reuse blocks with position adjustment
-      // Cache hit reuses previously-converted blocks as-is. That means we don't
-      // recompute previousParagraphFont (used for empty list items without
-      // explicit run properties). If the user changes the font on the prior
-      // paragraph (e.g. paragraph A), an empty list item (paragraph B) can keep
-      // the old font until the cache entry is invalidated. Narrow case, but
-      // avoids confusing incremental-edit behavior.
+      // Cache hit: reuse blocks with position adjustment, then re-sync marker font
+      // from live PM state. Empty list items have no textStyle marks, so pass
+      // previousParagraphFont instead of falling back to stale cached runs.
       const delta = pmStart - cached.pmStart;
       const reusedBlocks = shiftCachedBlocks(cached.blocks, delta);
+      const paragraphProps = node.attrs?.paragraphProperties as ParagraphProperties | undefined;
+      const previousParagraphFont = !hasExplicitParagraphRunProperties(paragraphProps)
+        ? getLastParagraphFont(blocks)
+        : undefined;
+      reusedBlocks.forEach((block) => {
+        if (block.kind === 'paragraph') {
+          syncListMarkerFontFromParagraphRuns({
+            block,
+            converterContext,
+            para: node,
+            contentFontSource: 'paragraph',
+            previousParagraphFont,
+          });
+        }
+      });
       applyTrackedGhostListAdjustments(node, reusedBlocks, context);
 
       reusedBlocks.forEach((block) => {