Skip to content

Commit 7c9774c

Browse files
chittolinagchittolinacaio-pizzol
authored
SD-2570 - fix: document API tracked changes not fully rendered on document (#2825)
* fix: document API tracked changes not fully rendered on document * refactor: unified functions/reuse existing functions * chore: small code tweaks * chore: small test tweaks * fix: caret position * fix: pm positions * test: added tests to ensure pm positions are correct * fix: expand lines once per paragraph * fix: docs * refactor: small code tweaks * feat: add trackedChange property to BreakRun * fix: skip empty segments * fix: don't expose benchmarks/remove dead code * fix(pm-adapter): preserve break for empty segments in expandRunsForInlineNewlines The previous guard early-returned the whole iteration when a segment was empty, which also skipped the break emit and cursor advance. Leading \n, trailing \n, and consecutive \n\n inputs would drop breaks and shift pm positions downstream. Guard only the empty-text push so the break logic still runs for every \n in the source text. --------- Co-authored-by: Gabriel Chittolina <gabrielchittolina1@gmail.com> Co-authored-by: Caio Pizzol <caio@superdoc.dev>
1 parent 4372d0e commit 7c9774c

15 files changed

Lines changed: 283 additions & 204 deletions

File tree

packages/layout-engine/contracts/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,7 @@ export type BreakRun = {
386386
pmStart?: number;
387387
pmEnd?: number;
388388
sdt?: SdtMetadata;
389+
trackedChange?: TrackedChangeMeta;
389390
};
390391

391392
/**

packages/layout-engine/layout-bridge/src/index.ts

Lines changed: 1 addition & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export type { HeaderFooterLayoutResult, IncrementalLayoutResult } from './increm
7474
export { computeDisplayPageNumber } from '@superdoc/layout-engine';
7575
export type { DisplayPageInfo, HeaderFooterConstraints } from '@superdoc/layout-engine';
7676
export { remeasureParagraph } from './remeasure';
77-
export { measureCharacterX } from './text-measurement';
77+
export { measureCharacterX, sliceRunsForLine } from './text-measurement';
7878
export { clickToPositionDom, findPageElement } from './dom-mapping';
7979
export { isListItem, getWordLayoutConfig, calculateTextStartIndent, extractParagraphIndent } from './list-indent-utils';
8080
export type { TextIndentCalculationParams } from './list-indent-utils';
@@ -144,10 +144,6 @@ export type { FallbackReason, SafetyConfig } from './safety-net';
144144
export { FocusWatchdog } from './focus-watchdog';
145145
export type { FocusWatchdogConfig } from './focus-watchdog';
146146

147-
// Benchmarks
148-
export { TypingPerfBenchmark } from './benchmarks';
149-
export type { BenchmarkResult, BenchmarkScenario } from './benchmarks';
150-
151147
// Paragraph Hash Utilities
152148
export {
153149
hashParagraphBorder,
@@ -1539,74 +1535,4 @@ const mapPmToX = (
15391535
return measureCharacterX(block, line, offset, availableWidth, alignmentOverride);
15401536
};
15411537

1542-
const _sliceRunsForLine = (block: FlowBlock, line: Line): Run[] => {
1543-
const result: Run[] = [];
1544-
1545-
if (block.kind !== 'paragraph') return result;
1546-
1547-
for (let runIndex = line.fromRun; runIndex <= line.toRun; runIndex += 1) {
1548-
const run = block.runs[runIndex];
1549-
if (!run) continue;
1550-
1551-
if (run.kind === 'tab') {
1552-
result.push(run);
1553-
continue;
1554-
}
1555-
1556-
// FIXED: ImageRun handling - images are atomic units, no slicing needed
1557-
if ('src' in run) {
1558-
result.push(run);
1559-
continue;
1560-
}
1561-
1562-
// LineBreakRun handling - line breaks are atomic units, no slicing needed
1563-
if (run.kind === 'lineBreak') {
1564-
result.push(run);
1565-
continue;
1566-
}
1567-
1568-
// BreakRun handling - breaks are atomic units, no slicing needed
1569-
if (run.kind === 'break') {
1570-
result.push(run);
1571-
continue;
1572-
}
1573-
1574-
// FieldAnnotationRun handling - field annotations are atomic units, no slicing needed
1575-
if (run.kind === 'fieldAnnotation') {
1576-
result.push(run);
1577-
continue;
1578-
}
1579-
1580-
// MathRun handling - math runs are atomic units, no slicing needed
1581-
if (run.kind === 'math') {
1582-
result.push(run);
1583-
continue;
1584-
}
1585-
1586-
const text = run.text ?? '';
1587-
const isFirstRun = runIndex === line.fromRun;
1588-
const isLastRun = runIndex === line.toRun;
1589-
1590-
if (isFirstRun || isLastRun) {
1591-
const start = isFirstRun ? line.fromChar : 0;
1592-
const end = isLastRun ? line.toChar : text.length;
1593-
const slice = text.slice(start, end);
1594-
const pmStart =
1595-
run.pmStart != null ? run.pmStart + start : run.pmEnd != null ? run.pmEnd - (text.length - start) : undefined;
1596-
const pmEnd =
1597-
run.pmStart != null ? run.pmStart + end : run.pmEnd != null ? run.pmEnd - (text.length - end) : undefined;
1598-
result.push({
1599-
...run,
1600-
text: slice,
1601-
pmStart,
1602-
pmEnd,
1603-
});
1604-
} else {
1605-
result.push(run);
1606-
}
1607-
}
1608-
1609-
return result;
1610-
};
1611-
16121538
// isRtlBlock is now in position-hit.ts and re-exported above.

packages/layout-engine/layout-bridge/src/text-measurement.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ export function sliceRunsForLine(block: FlowBlock, line: Line): Run[] {
379379
const start = isFirstRun ? line.fromChar : 0;
380380
const end = isLastRun ? line.toChar : text.length;
381381
const slice = text.slice(start, end);
382+
if (!slice) continue;
382383
const pmStart =
383384
run.pmStart != null ? run.pmStart + start : run.pmEnd != null ? run.pmEnd - (text.length - start) : undefined;
384385
const pmEnd =

packages/layout-engine/layout-bridge/test/benchmarks.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44

55
import { describe, it, expect, beforeEach } from 'vitest';
6-
import { TypingPerfBenchmark } from '../src/benchmarks';
6+
import { TypingPerfBenchmark } from './benchmarks';
77

88
describe('TypingPerfBenchmark', () => {
99
let benchmark: TypingPerfBenchmark;

packages/layout-engine/layout-bridge/src/benchmarks.ts renamed to packages/layout-engine/layout-bridge/test/benchmarks.ts

File renamed without changes.

packages/layout-engine/layout-bridge/src/benchmarks/index.ts renamed to packages/layout-engine/layout-bridge/test/benchmarks/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { LayoutOptions } from '@superdoc/layout-engine';
44
import { measureBlock } from '@superdoc/measuring-dom';
55
import { createDomPainter } from '@superdoc/painter-dom';
66
import { layoutDocument } from '@superdoc/layout-engine';
7-
import { incrementalLayout, measureCache, resolveMeasurementConstraints } from '../incrementalLayout';
7+
import { incrementalLayout, measureCache, resolveMeasurementConstraints } from '../../src/incrementalLayout';
88

99
const LETTER_LAYOUT: LayoutOptions = {
1010
pageSize: { w: 612, h: 792 },

packages/layout-engine/layout-bridge/test/performance.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import { describe, it, expect, beforeAll } from 'vitest';
33
import { resolveCanvas } from '../../measuring/dom/src/canvas-resolver.js';
44
import { installNodeCanvasPolyfill } from '../../measuring/dom/src/setup.ts';
5-
import { runBenchmarkSuite } from '../src/benchmarks/index';
5+
import { runBenchmarkSuite } from './benchmarks/index';
66

77
const { Canvas, usingStub } = resolveCanvas();
88

packages/layout-engine/painters/dom/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
"@superdoc/dom-contract": "workspace:*",
2323
"@superdoc/font-utils": "workspace:*",
2424
"@superdoc/layout-resolved": "workspace:*",
25+
"@superdoc/layout-bridge": "workspace:*",
26+
"@superdoc/pm-adapter": "workspace:*",
2527
"@superdoc/preset-geometry": "workspace:*",
2628
"@superdoc/url-validation": "workspace:*"
2729
},

packages/layout-engine/painters/dom/src/index.test.ts

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11357,6 +11357,95 @@ describe('applyRunDataAttributes', () => {
1135711357
}).not.toThrow();
1135811358
});
1135911359

11360+
it('renders all lines when measure indices come from inline-newline expansion', () => {
11361+
const inlineNewlineBlock: FlowBlock = {
11362+
kind: 'paragraph',
11363+
id: 'inline-newline-slice',
11364+
runs: [{ text: 'first\nsecond\nthird', fontFamily: 'Arial', fontSize: 16, pmStart: 0, pmEnd: 18 }],
11365+
};
11366+
11367+
// Measurer expands inline '\n' into: text, break, text, break, text.
11368+
const inlineNewlineMeasure: ParagraphMeasure = {
11369+
kind: 'paragraph',
11370+
lines: [
11371+
{
11372+
fromRun: 0,
11373+
fromChar: 0,
11374+
toRun: 0,
11375+
toChar: 5,
11376+
width: 40,
11377+
ascent: 12,
11378+
descent: 4,
11379+
lineHeight: 20,
11380+
},
11381+
{
11382+
fromRun: 2,
11383+
fromChar: 0,
11384+
toRun: 2,
11385+
toChar: 6,
11386+
width: 50,
11387+
ascent: 12,
11388+
descent: 4,
11389+
lineHeight: 20,
11390+
},
11391+
{
11392+
fromRun: 4,
11393+
fromChar: 0,
11394+
toRun: 4,
11395+
toChar: 5,
11396+
width: 40,
11397+
ascent: 12,
11398+
descent: 4,
11399+
lineHeight: 20,
11400+
},
11401+
],
11402+
totalHeight: 60,
11403+
};
11404+
11405+
const inlineNewlineLayout: Layout = {
11406+
pageSize: { w: 400, h: 500 },
11407+
pages: [
11408+
{
11409+
number: 1,
11410+
fragments: [
11411+
{
11412+
kind: 'para',
11413+
blockId: 'inline-newline-slice',
11414+
fromLine: 0,
11415+
toLine: 3,
11416+
x: 20,
11417+
y: 20,
11418+
width: 300,
11419+
},
11420+
],
11421+
},
11422+
],
11423+
};
11424+
11425+
const painter = createTestPainter({
11426+
blocks: [inlineNewlineBlock],
11427+
measures: [inlineNewlineMeasure],
11428+
});
11429+
11430+
expect(() => {
11431+
painter.paint(inlineNewlineLayout, mount);
11432+
}).not.toThrow();
11433+
11434+
const fragment = mount.querySelector<HTMLElement>('.superdoc-fragment');
11435+
expect(fragment).not.toBeNull();
11436+
expect(fragment?.textContent).toContain('first');
11437+
expect(fragment?.textContent).toContain('second');
11438+
expect(fragment?.textContent).toContain('third');
11439+
const lines = fragment?.querySelectorAll<HTMLElement>('.superdoc-line');
11440+
expect(lines?.length).toBe(3);
11441+
expect(lines?.[0].dataset.pmStart).toEqual('0');
11442+
expect(lines?.[0].dataset.pmEnd).toEqual('5');
11443+
expect(lines?.[1].dataset.pmStart).toEqual('6');
11444+
expect(lines?.[1].dataset.pmEnd).toEqual('12');
11445+
expect(lines?.[2].dataset.pmStart).toEqual('13');
11446+
expect(lines?.[2].dataset.pmEnd).toEqual('18');
11447+
});
11448+
1136011449
it('preserves PM positions for lineBreak runs', () => {
1136111450
const lineBreakBlock: FlowBlock = {
1136211451
kind: 'paragraph',

0 commit comments

Comments
 (0)