Skip to content

Commit 5dd00c8

Browse files
authored
fix: update marker on font change (#3596)
* fix: update marker on font change * fix: preserve explicit numbering * fix: cache fix for empty runs * fix: ignore empty runs * fix: review comments
1 parent 34debe5 commit 5dd00c8

8 files changed

Lines changed: 772 additions & 44 deletions

File tree

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

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,12 @@ import {
3131
resolveParagraphProperties,
3232
resolveRunProperties,
3333
resolveDocxFontFamily,
34-
getNumberingProperties,
3534
type ParagraphFrameProperties,
3635
type ParagraphProperties,
3736
type RunProperties,
3837
} from '@superdoc/style-engine/ooxml';
3938
import { resolveSectionDirection, resolveParagraphDirection } from '../direction/index.js';
39+
import { numberingDefinesMarkerFontFamily } from '../numbering-marker-font.js';
4040

4141
const DEFAULT_DECIMAL_SEPARATOR = '.';
4242
const DEFAULT_TAB_INTERVAL_TWIPS = 720; // 0.5 inch
@@ -421,20 +421,14 @@ export const computeParagraphAttrs = (
421421
// fully defines marker font.
422422
let markerFontFallback: Partial<ParagraphFont> | undefined;
423423
if (!hasExplicitParagraphRunProperties(paragraphProperties) && previousParagraphFont) {
424-
// Detect whether numbering explicitly overrides the marker font family
425-
// (e.g. Symbol/Wingdings). If it does, we must NOT overwrite it.
426-
const numProps = paragraphProperties.numberingProperties;
427-
const numId = numProps?.numId;
428-
const ilvl = numProps?.ilvl ?? 0;
429-
const numberingRunProps =
430-
numId != null && numId !== 0
431-
? getNumberingProperties<RunProperties>('runProperties', converterContext!, ilvl, numId)
432-
: ({} as RunProperties);
433-
const numberingDefinesMarkerFontFamily = numberingRunProps.fontFamily != null;
424+
const pinsMarkerFontFamily = numberingDefinesMarkerFontFamily(
425+
paragraphProperties.numberingProperties,
426+
converterContext,
427+
);
434428

435429
markerFontFallback = {
436430
// When numbering explicitly sets a marker font (Symbol/Wingdings), keep it.
437-
fontFamily: numberingDefinesMarkerFontFamily ? undefined : previousParagraphFont.fontFamily,
431+
fontFamily: pinsMarkerFontFamily ? undefined : previousParagraphFont.fontFamily,
438432
// Preserve existing behavior: if the paragraph has no explicit run props,
439433
// marker font size inherits from the previous paragraph.
440434
fontSize: previousParagraphFont.fontSize,

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,44 @@ describe('getLastParagraphFont', () => {
352352
const result = getLastParagraphFont(blocks);
353353
expect(result).toBeUndefined();
354354
});
355+
356+
it('skips empty first runs even when they carry stale font values', () => {
357+
const blocks: FlowBlock[] = [
358+
{
359+
kind: 'paragraph',
360+
id: '0-paragraph',
361+
runs: [
362+
{
363+
kind: 'text',
364+
text: 'Valid',
365+
fontFamily: 'ValidFont',
366+
fontSize: 11,
367+
pmStart: 0,
368+
pmEnd: 5,
369+
},
370+
],
371+
attrs: {},
372+
},
373+
{
374+
kind: 'paragraph',
375+
id: '1-paragraph',
376+
runs: [
377+
{
378+
kind: 'text',
379+
text: '',
380+
fontFamily: 'StaleFont',
381+
fontSize: 42,
382+
pmStart: 5,
383+
pmEnd: 5,
384+
},
385+
],
386+
attrs: {},
387+
},
388+
];
389+
390+
const result = getLastParagraphFont(blocks);
391+
expect(result).toEqual({ fontFamily: 'ValidFont', fontSize: 11 });
392+
});
355393
});
356394

357395
describe('paragraph converters', () => {

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

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import { applyTrackedChangesModeToRuns } from '../tracked-changes.js';
3737
import { textNodeToRun } from './inline-converters/text-run.js';
3838
import { DEFAULT_HYPERLINK_CONFIG, TOKEN_INLINE_TYPES } from '../constants.js';
3939
import { computeRunAttrs, hasExplicitParagraphRunProperties } from '../attributes/paragraph.js';
40+
import { syncListMarkerFontFromParagraphRuns } from '../list-marker-font.js';
4041
import { resolveRunProperties } from '@superdoc/style-engine/ooxml';
4142
import { footnoteReferenceToBlock } from './inline-converters/footnote-reference.js';
4243
import { endnoteReferenceToBlock } from './inline-converters/endnote-reference.js';
@@ -604,6 +605,19 @@ export function paragraphToFlowBlocks({
604605
const defaultSize =
605606
usePreviousFont && previousParagraphFont.fontSize ? previousParagraphFont.fontSize : extracted.defaultSize;
606607

608+
const finalizeParagraphBlocks = (outputBlocks: FlowBlock[]): FlowBlock[] => {
609+
outputBlocks.forEach((block) => {
610+
if (block.kind === 'paragraph') {
611+
syncListMarkerFontFromParagraphRuns({
612+
block,
613+
converterContext,
614+
para,
615+
});
616+
}
617+
});
618+
return outputBlocks;
619+
};
620+
607621
if (paragraphAttrs.pageBreakBefore) {
608622
blocks.push({
609623
kind: 'pageBreak',
@@ -615,7 +629,7 @@ export function paragraphToFlowBlocks({
615629

616630
if (!para.content || para.content.length === 0) {
617631
if (paragraphProps.runProperties?.vanish) {
618-
return blocks;
632+
return finalizeParagraphBlocks(blocks);
619633
}
620634
const paragraphMarkTrackedChange = getParagraphMarkTrackedChange(paragraphProps, storyKey);
621635
// Get the PM position of the empty paragraph for caret rendering
@@ -650,12 +664,12 @@ export function paragraphToFlowBlocks({
650664
sourceAnchor,
651665
});
652666
if (!trackedChangesConfig) {
653-
return blocks;
667+
return finalizeParagraphBlocks(blocks);
654668
}
655669

656670
const paragraphBlock = blocks[blocks.length - 1];
657671
if (paragraphBlock?.kind !== 'paragraph') {
658-
return blocks;
672+
return finalizeParagraphBlocks(blocks);
659673
}
660674

661675
const filteredRuns = applyTrackedChangesModeToRuns(
@@ -682,7 +696,7 @@ export function paragraphToFlowBlocks({
682696

683697
if (trackedChangesConfig.enabled && (filteredRuns.length === 0 || isGhostTrackedListArtifact)) {
684698
blocks.pop();
685-
return blocks;
699+
return finalizeParagraphBlocks(blocks);
686700
}
687701

688702
paragraphBlock.runs = filteredRuns;
@@ -691,7 +705,7 @@ export function paragraphToFlowBlocks({
691705
trackedChangesMode: trackedChangesConfig.mode,
692706
trackedChangesEnabled: trackedChangesConfig.enabled,
693707
};
694-
return blocks;
708+
return finalizeParagraphBlocks(blocks);
695709
}
696710

697711
let currentRuns: Run[] = [];
@@ -914,7 +928,7 @@ export function paragraphToFlowBlocks({
914928
});
915929

916930
if (!trackedChangesConfig) {
917-
return blocks;
931+
return finalizeParagraphBlocks(blocks);
918932
}
919933

920934
const processedBlocks: FlowBlock[] = [];
@@ -944,7 +958,7 @@ export function paragraphToFlowBlocks({
944958
processedBlocks.push(block);
945959
});
946960

947-
return processedBlocks;
961+
return finalizeParagraphBlocks(processedBlocks);
948962
}
949963

950964
type InlineConverterSpec = {
@@ -1063,7 +1077,10 @@ export function getLastParagraphFont(blocks: FlowBlock[]): ParagraphFont | undef
10631077
const para = block as ParagraphBlock;
10641078
const firstRun = para.runs?.[0];
10651079
if (!firstRun) continue;
1066-
const run = firstRun as { fontFamily?: string; fontSize?: number };
1080+
const run = firstRun as { text?: string; fontFamily?: string; fontSize?: number };
1081+
if (typeof run.text === 'string' && run.text.length === 0) {
1082+
continue;
1083+
}
10671084
const fontFamily = typeof run.fontFamily === 'string' ? run.fontFamily.trim() : '';
10681085
const fontSize = typeof run.fontSize === 'number' && Number.isFinite(run.fontSize) ? run.fontSize : NaN;
10691086
if (fontFamily.length > 0 && fontSize > 0) {
@@ -1133,15 +1150,26 @@ export function handleParagraphNode(node: PMNode, context: NodeHandlerContext):
11331150
// get() returns both the entry (if hit) and pre-computed nodeJson to avoid double serialization
11341151
const { entry: cached, nodeJson, nodeRev } = flowBlockCache.get(prefixedStableId, node);
11351152
if (cached) {
1136-
// Cache hit: reuse blocks with position adjustment
1137-
// Cache hit reuses previously-converted blocks as-is. That means we don't
1138-
// recompute previousParagraphFont (used for empty list items without
1139-
// explicit run properties). If the user changes the font on the prior
1140-
// paragraph (e.g. paragraph A), an empty list item (paragraph B) can keep
1141-
// the old font until the cache entry is invalidated. Narrow case, but
1142-
// avoids confusing incremental-edit behavior.
1153+
// Cache hit: reuse blocks with position adjustment, then re-sync marker font
1154+
// from live PM state. Empty list items have no textStyle marks, so pass
1155+
// previousParagraphFont instead of falling back to stale cached runs.
11431156
const delta = pmStart - cached.pmStart;
11441157
const reusedBlocks = shiftCachedBlocks(cached.blocks, delta);
1158+
const paragraphProps = node.attrs?.paragraphProperties as ParagraphProperties | undefined;
1159+
const previousParagraphFont = !hasExplicitParagraphRunProperties(paragraphProps)
1160+
? getLastParagraphFont(blocks)
1161+
: undefined;
1162+
reusedBlocks.forEach((block) => {
1163+
if (block.kind === 'paragraph') {
1164+
syncListMarkerFontFromParagraphRuns({
1165+
block,
1166+
converterContext,
1167+
para: node,
1168+
contentFontSource: 'paragraph',
1169+
previousParagraphFont,
1170+
});
1171+
}
1172+
});
11451173
applyTrackedGhostListAdjustments(node, reusedBlocks, context);
11461174

11471175
reusedBlocks.forEach((block) => {

0 commit comments

Comments
 (0)