Skip to content

Commit c4d9770

Browse files
VladaHarbourVladaHarbour
committed
fix: large header is missing and cursor is buggy in doc with footnotes
1 parent 4ba8992 commit c4d9770

10 files changed

Lines changed: 259 additions & 37 deletions

File tree

‎packages/layout-engine/layout-bridge/src/incrementalLayout.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1742,6 +1742,8 @@ export async function incrementalLayout(
17421742
footnoteReservedByPageIndex,
17431743
headerContentHeights,
17441744
footerContentHeights,
1745+
headerContentHeightsByRId,
1746+
footerContentHeightsByRId,
17451747
remeasureParagraph: (block: FlowBlock, maxWidth: number, firstLineIndent?: number) =>
17461748
remeasureParagraph(block as ParagraphBlock, maxWidth, firstLineIndent),
17471749
});

‎packages/layout-engine/layout-bridge/test/footnoteRelayoutKeepsHeaderHeightsByRId.test.ts‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
import { describe, it, expect, vi } from 'vitest';
2+
import type { FlowBlock, Measure } from '@superdoc/contracts';
3+
import type { HeaderFooterConstraints } from '@superdoc/layout-engine';
4+
import * as layoutEngine from '@superdoc/layout-engine';
5+
import { incrementalLayout } from '../src/incrementalLayout';
6+
7+
const makeParagraph = (id: string, text: string, pmStart: number): FlowBlock => ({
8+
kind: 'paragraph',
9+
id,
10+
runs: [{ text, fontFamily: 'Arial', fontSize: 12, pmStart, pmEnd: pmStart + text.length }],
11+
});
12+
13+
const makeMeasure = (lineHeight: number, textLength: number): Measure => ({
14+
kind: 'paragraph',
15+
lines: [
16+
{
17+
fromRun: 0,
18+
fromChar: 0,
19+
toRun: 0,
20+
toChar: textLength,
21+
width: 200,
22+
ascent: lineHeight * 0.8,
23+
descent: lineHeight * 0.2,
24+
lineHeight,
25+
},
26+
],
27+
totalHeight: lineHeight,
28+
});
29+
30+
const makeMultiLineMeasure = (lineHeight: number, lineCount: number): Measure => {
31+
const lines = Array.from({ length: lineCount }, (_, i) => ({
32+
fromRun: 0,
33+
fromChar: i,
34+
toRun: 0,
35+
toChar: i + 1,
36+
width: 200,
37+
ascent: lineHeight * 0.8,
38+
descent: lineHeight * 0.2,
39+
lineHeight,
40+
}));
41+
return {
42+
kind: 'paragraph',
43+
lines,
44+
totalHeight: lineCount * lineHeight,
45+
};
46+
};
47+
48+
/**
49+
* Footnote reserve relayout must keep headerContentHeightsByRId / footerContentHeightsByRId.
50+
* Otherwise per-rId header height is dropped, topMargin is not inflated, and body overlaps a tall header.
51+
*/
52+
describe('Footnote relayout preserves headerContentHeightsByRId', () => {
53+
it('passes by-RId header maps on every layoutDocument call that reserves footnote space', async () => {
54+
const BODY_LINE_HEIGHT = 20;
55+
const FOOTNOTE_LINE_HEIGHT = 12;
56+
const HEADER_CONTENT_HEIGHT = 100;
57+
const LINES_ON_PAGE_1_WITHOUT_RESERVE = 12;
58+
const FOOTNOTE_LINES = 5;
59+
60+
const headerBlock = makeParagraph('hdr-rId1-line', 'Tall header line', 0);
61+
62+
let pos = 0;
63+
const bodyBlocks: FlowBlock[] = [];
64+
for (let i = 0; i < LINES_ON_PAGE_1_WITHOUT_RESERVE; i += 1) {
65+
const text = `Line ${i + 1}.`;
66+
bodyBlocks.push(makeParagraph(`body-${i}`, text, pos));
67+
pos += text.length + 1;
68+
}
69+
const refPos = pos - 2;
70+
const footnoteBlock = makeParagraph(
71+
'footnote-1-0-paragraph',
72+
'Footnote content that spans multiple lines here.',
73+
0,
74+
);
75+
76+
const measureBlock = vi.fn(async (block: FlowBlock) => {
77+
if (block.id.startsWith('hdr-')) {
78+
return makeMeasure(HEADER_CONTENT_HEIGHT, block.runs?.[0]?.text?.length ?? 1);
79+
}
80+
if (block.id.startsWith('footnote-')) {
81+
return makeMultiLineMeasure(FOOTNOTE_LINE_HEIGHT, FOOTNOTE_LINES);
82+
}
83+
const textLength = block.kind === 'paragraph' ? (block.runs?.[0]?.text?.length ?? 1) : 1;
84+
return makeMeasure(BODY_LINE_HEIGHT, textLength);
85+
});
86+
87+
const contentHeight = 240;
88+
const margins = { top: 72, right: 72, bottom: 72, left: 72, header: 72, footer: 72 };
89+
const pageHeight = contentHeight + margins.top + margins.bottom;
90+
const pageWidth = 612;
91+
const contentWidth = pageWidth - margins.left - margins.right;
92+
93+
const constraints: HeaderFooterConstraints = {
94+
width: contentWidth,
95+
height: margins.top,
96+
pageWidth,
97+
pageHeight,
98+
margins: { left: margins.left, right: margins.right, top: margins.top, bottom: margins.bottom },
99+
};
100+
101+
const layoutDocSpy = vi.spyOn(layoutEngine, 'layoutDocument');
102+
103+
const { layout } = await incrementalLayout(
104+
[],
105+
null,
106+
bodyBlocks,
107+
{
108+
pageSize: { w: pageWidth, h: pageHeight },
109+
margins,
110+
sectionMetadata: [{ sectionIndex: 0, headerRefs: { default: 'rId1' } }],
111+
footnotes: {
112+
refs: [{ id: '1', pos: refPos }],
113+
blocksById: new Map([['1', [footnoteBlock]]]),
114+
topPadding: 4,
115+
dividerHeight: 2,
116+
},
117+
},
118+
measureBlock,
119+
{
120+
headerBlocksByRId: new Map([['rId1', [headerBlock]]]),
121+
constraints,
122+
},
123+
);
124+
125+
const reserveCalls = layoutDocSpy.mock.calls.filter((call) => {
126+
const opts = call[2] as { footnoteReservedByPageIndex?: number[] };
127+
return opts?.footnoteReservedByPageIndex?.some((h) => h > 0);
128+
});
129+
layoutDocSpy.mockRestore();
130+
131+
expect(reserveCalls.length).toBeGreaterThanOrEqual(1);
132+
for (const call of reserveCalls) {
133+
const opts = call[2] as {
134+
headerContentHeightsByRId?: Map<string, number>;
135+
};
136+
expect(opts.headerContentHeightsByRId).toBeInstanceOf(Map);
137+
expect(opts.headerContentHeightsByRId?.get('rId1')).toBeGreaterThanOrEqual(HEADER_CONTENT_HEIGHT - 1);
138+
}
139+
140+
const page1 = layout.pages[0];
141+
const headerDistance = page1.margins?.header ?? margins.header;
142+
const minBodyTop = Math.max(margins.top, headerDistance + HEADER_CONTENT_HEIGHT);
143+
const firstBody = page1.fragments.find((f) => String(f.blockId).startsWith('body-') && f.kind === 'para');
144+
expect(firstBody && 'y' in firstBody && typeof firstBody.y === 'number').toBe(true);
145+
expect((firstBody as { y: number }).y).toBeGreaterThanOrEqual(minBodyTop - 1);
146+
});
147+
});

‎packages/super-editor/src/editors/v1/core/header-footer/HeaderFooterPerRidLayout.ts‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -436,29 +436,30 @@ async function layoutWithPerSectionConstraints(
436436
const rId = defaultRIdPerSection.get(section.sectionIndex);
437437
if (!rId || !blocksByRId.has(rId)) continue;
438438

439-
// Resolve the minimum width needed for tables in this section.
440-
// For pct tables, this depends on the section's content width.
441-
const contentWidth = buildSectionContentWidth(section, fallbackConstraints);
442-
const tableWidthSpec = tableWidthSpecByRId.get(rId);
443-
const tableMinWidth = resolveTableMinWidth(tableWidthSpec, contentWidth);
444-
const sectionConstraints = buildConstraintsForSection(section, fallbackConstraints, tableMinWidth || undefined);
445-
const effectiveWidth = sectionConstraints.width;
446-
// Include vertical geometry in the key so sections with different page heights,
447-
// vertical margins, or header distance get separate layouts (page-relative anchors
448-
// and header band origin resolve differently).
449-
const groupKey = `${rId}::w${effectiveWidth}::ph${sectionConstraints.pageHeight ?? ''}::mt${sectionConstraints.margins?.top ?? ''}::mb${sectionConstraints.margins?.bottom ?? ''}::mh${sectionConstraints.margins?.header ?? ''}`;
450-
451-
let group = groups.get(groupKey);
452-
if (!group) {
453-
group = {
454-
sectionConstraints,
455-
sectionIndices: [],
456-
rId,
457-
effectiveWidth,
458-
};
459-
groups.set(groupKey, group);
439+
// Resolve the minimum width needed for tables in this section.
440+
// For pct tables, this depends on the section's content width.
441+
const contentWidth = buildSectionContentWidth(section, fallbackConstraints);
442+
const tableWidthSpec = tableWidthSpecByRId.get(rId);
443+
const tableMinWidth = resolveTableMinWidth(tableWidthSpec, contentWidth);
444+
const sectionConstraints = buildConstraintsForSection(section, fallbackConstraints, tableMinWidth || undefined);
445+
const effectiveWidth = sectionConstraints.width;
446+
// Include vertical geometry in the key so sections with different page heights,
447+
// vertical margins, or header distance get separate layouts (page-relative anchors
448+
// and header band origin resolve differently).
449+
const groupKey = `${rId}::w${effectiveWidth}::ph${sectionConstraints.pageHeight ?? ''}::mt${sectionConstraints.margins?.top ?? ''}::mb${sectionConstraints.margins?.bottom ?? ''}::mh${sectionConstraints.margins?.header ?? ''}`;
450+
451+
let group = groups.get(groupKey);
452+
if (!group) {
453+
group = {
454+
sectionConstraints,
455+
sectionIndices: [],
456+
rId,
457+
effectiveWidth,
458+
};
459+
groups.set(groupKey, group);
460+
}
461+
group.sectionIndices.push(section.sectionIndex);
460462
}
461-
group.sectionIndices.push(section.sectionIndex);
462463
}
463464

464465
// Measure and layout each unique (rId, effectiveWidth) group

‎packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import type { Mapping } from 'prosemirror-transform';
2020
import { Editor } from '../Editor.js';
2121
import { EventEmitter } from '../EventEmitter.js';
2222
import { EpochPositionMapper } from './layout/EpochPositionMapper.js';
23-
import { DomPositionIndex } from '../../dom-observer/DomPositionIndex.js';
23+
import { DomPositionIndex, isFootnotePaintedBlockHost } from '../../dom-observer/DomPositionIndex.js';
2424
import { DomPositionIndexObserverManager } from '../../dom-observer/DomPositionIndexObserverManager.js';
2525
import {
2626
computeDomCaretPageLocal as computeDomCaretPageLocalFromDom,
@@ -118,6 +118,7 @@ import { isHeaderFooterPartId } from '../parts/adapters/header-footer-part-descr
118118
import type { PartChangedEvent } from '../parts/types.js';
119119
import { isInRegisteredSurface } from './utils/uiSurfaceRegistry.js';
120120
import { buildSemanticFootnoteBlocks } from './semantic-flow-footnotes.js';
121+
import { isFootnoteLayoutBlockId } from './semantic-flow-constants.js';
121122

122123
type ThreadAnchorScrollPlan = {
123124
achievedClientY: number;
@@ -2436,6 +2437,9 @@ export class PresentationEditor extends EventEmitter {
24362437
for (let idx = 0; idx < layout.pages.length; idx++) {
24372438
const page = layout.pages[idx];
24382439
for (const fragment of page.fragments) {
2440+
if (isFootnoteLayoutBlockId((fragment as { blockId?: string }).blockId)) {
2441+
continue;
2442+
}
24392443
const frag = fragment as { pmStart?: number; pmEnd?: number };
24402444
if (frag.pmStart != null && frag.pmEnd != null && clampedPos >= frag.pmStart && clampedPos <= frag.pmEnd) {
24412445
pageIndex = idx;
@@ -2578,6 +2582,7 @@ export class PresentationEditor extends EventEmitter {
25782582
// Skip header/footer fragments — their PM positions come from a separate
25792583
// document and can overlap with body positions, causing incorrect matches.
25802584
if (htmlEl.closest('.superdoc-page-header, .superdoc-page-footer')) continue;
2585+
if (isFootnotePaintedBlockHost(htmlEl)) continue;
25812586

25822587
const start = Number(htmlEl.dataset.pmStart);
25832588
const end = Number(htmlEl.dataset.pmEnd);
@@ -2634,6 +2639,9 @@ export class PresentationEditor extends EventEmitter {
26342639
for (let idx = 0; idx < layout.pages.length; idx++) {
26352640
const page = layout.pages[idx];
26362641
for (const fragment of page.fragments) {
2642+
if (isFootnoteLayoutBlockId((fragment as { blockId?: string }).blockId)) {
2643+
continue;
2644+
}
26372645
const frag = fragment as { pmStart?: number; pmEnd?: number };
26382646
if (frag.pmStart != null && frag.pmEnd != null && clampedPos >= frag.pmStart && clampedPos <= frag.pmEnd) {
26392647
pageIndex = idx;
@@ -6726,6 +6734,9 @@ export class PresentationEditor extends EventEmitter {
67266734
for (let pageIdx = 0; pageIdx < layout.pages.length; pageIdx++) {
67276735
const page = layout.pages[pageIdx];
67286736
for (const fragment of page.fragments) {
6737+
if (isFootnoteLayoutBlockId((fragment as { blockId?: string }).blockId)) {
6738+
continue;
6739+
}
67296740
const frag = fragment as { pmStart?: number; pmEnd?: number };
67306741
if (frag.pmStart != null && frag.pmEnd != null) {
67316742
if (pos >= frag.pmStart && pos <= frag.pmEnd) {

‎packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import {
3939
} from '../tables/TableSelectionUtilities.js';
4040
import { debugLog } from '../selection/SelectionDebug.js';
4141
import { DOM_CLASS_NAMES, buildAnnotationSelector, DRAGGABLE_SELECTOR } from '@superdoc/dom-contract';
42-
import { isSemanticFootnoteBlockId } from '../semantic-flow-constants.js';
42+
import { isFootnoteLayoutBlockId } from '../semantic-flow-constants.js';
4343
import { CommentsPluginKey } from '@extensions/comment/comments-plugin.js';
4444

4545
// =============================================================================
@@ -70,15 +70,6 @@ type CommentThreadHit = {
7070
threadId: string | null;
7171
};
7272

73-
/**
74-
* Block IDs for footnote content use prefix "footnote-{id}-" (see FootnotesBuilder).
75-
* Semantic footnote blocks use the {@link isSemanticFootnoteBlockId} helper from
76-
* shared constants — it matches both heading and body footnote block IDs.
77-
*/
78-
function isFootnoteBlockId(blockId: string): boolean {
79-
return typeof blockId === 'string' && (blockId.startsWith('footnote-') || isSemanticFootnoteBlockId(blockId));
80-
}
81-
8273
function getCommentHighlightThreadIds(target: EventTarget | null): string[] {
8374
if (!(target instanceof Element)) {
8475
return [];
@@ -1074,7 +1065,7 @@ export class EditorInputManager {
10741065
// Disallow cursor placement in footnote lines: keep current selection and only focus editor.
10751066
const fragmentEl = target?.closest?.('[data-block-id]') as HTMLElement | null;
10761067
const clickedBlockId = fragmentEl?.getAttribute?.('data-block-id') ?? '';
1077-
if (isFootnoteBlockId(clickedBlockId)) {
1068+
if (isFootnoteLayoutBlockId(clickedBlockId)) {
10781069
if (!isDraggableAnnotation) event.preventDefault();
10791070
this.#focusEditor();
10801071
return;
@@ -1182,7 +1173,7 @@ export class EditorInputManager {
11821173

11831174
// Disallow cursor placement in footnote lines (footnote content is read-only in the layout).
11841175
// Keep the current selection unchanged instead of moving caret to document start.
1185-
if (isFootnoteBlockId(rawHit.blockId)) {
1176+
if (isFootnoteLayoutBlockId(rawHit.blockId)) {
11861177
this.#focusEditor();
11871178
return;
11881179
}
@@ -1918,7 +1909,7 @@ export class EditorInputManager {
19181909
if (!rawHit) return;
19191910

19201911
// Don't extend selection into footnote lines
1921-
if (isFootnoteBlockId(rawHit.blockId)) return;
1912+
if (isFootnoteLayoutBlockId(rawHit.blockId)) return;
19221913

19231914
const editor = this.#deps.getEditor();
19241915
const doc = editor.state?.doc;

‎packages/super-editor/src/editors/v1/core/presentation-editor/semantic-flow-constants.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,14 @@ export const SEMANTIC_FOOTNOTE_BLOCK_ID_PREFIX = '__sd_semantic_footnote';
1212
export function isSemanticFootnoteBlockId(blockId: string): boolean {
1313
return typeof blockId === 'string' && blockId.startsWith(SEMANTIC_FOOTNOTE_BLOCK_ID_PREFIX);
1414
}
15+
16+
/**
17+
* True when a layout / painted `data-block-id` belongs to the footnote band
18+
* (DOCX `footnote-*` fragments from FootnotesBuilder, semantic-flow `__sd_semantic_footnote*`
19+
* bodies — heading included via that prefix — and separators).
20+
* Use for hit-testing, DomPositionIndex (`isFootnotePaintedBlockHost`), and layout fragment scans.
21+
*/
22+
export function isFootnoteLayoutBlockId(blockId: string | null | undefined): boolean {
23+
if (typeof blockId !== 'string' || blockId.length === 0) return false;
24+
return blockId.startsWith('footnote-') || isSemanticFootnoteBlockId(blockId);
25+
}

‎packages/super-editor/src/editors/v1/core/presentation-editor/tests/DomPositionIndex.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,41 @@ describe('DomPositionIndex', () => {
6565
expect(index.findElementAtPosition(1)?.textContent).toBe('body');
6666
});
6767

68+
it('skips footnote painted fragments when building the index', () => {
69+
const container = document.createElement('div');
70+
const page = document.createElement('div');
71+
page.className = 'superdoc-page';
72+
page.dataset.pageIndex = '0';
73+
74+
const bodyLine = document.createElement('div');
75+
bodyLine.className = 'superdoc-line';
76+
const bodySpan = document.createElement('span');
77+
bodySpan.setAttribute('data-pm-start', '1');
78+
bodySpan.setAttribute('data-pm-end', '5');
79+
bodySpan.textContent = 'body';
80+
bodyLine.appendChild(bodySpan);
81+
page.appendChild(bodyLine);
82+
83+
const fnBlock = document.createElement('div');
84+
fnBlock.className = 'superdoc-line';
85+
fnBlock.setAttribute('data-block-id', 'footnote-1-0-paragraph');
86+
const fnSpan = document.createElement('span');
87+
fnSpan.setAttribute('data-pm-start', '100');
88+
fnSpan.setAttribute('data-pm-end', '120');
89+
fnSpan.textContent = 'fn text';
90+
fnBlock.appendChild(fnSpan);
91+
page.appendChild(fnBlock);
92+
93+
container.appendChild(page);
94+
95+
const index = new DomPositionIndex();
96+
index.rebuild(container);
97+
98+
expect(index.size).toBe(1);
99+
expect(index.findElementAtPosition(3)?.textContent).toBe('body');
100+
expect(index.findEntryClosestToPosition(50)?.el.textContent).toBe('body');
101+
});
102+
68103
it('skips footer-only content when building the index', () => {
69104
const container = document.createElement('div');
70105
container.innerHTML = `

‎packages/super-editor/src/editors/v1/core/presentation-editor/tests/semantic-flow-footnotes.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { FlowBlock } from '@superdoc/contracts';
33

44
import { buildSemanticFootnoteBlocks } from '../semantic-flow-footnotes.js';
55
import {
6+
isFootnoteLayoutBlockId,
67
isSemanticFootnoteBlockId,
78
SEMANTIC_FOOTNOTES_HEADING_BLOCK_ID,
89
SEMANTIC_FOOTNOTE_BLOCK_ID_PREFIX,
@@ -64,4 +65,13 @@ describe('semantic-flow-footnotes', () => {
6465
expect(isSemanticFootnoteBlockId(`${SEMANTIC_FOOTNOTE_BLOCK_ID_PREFIX}-1-0-0-fn-1`)).toBe(true);
6566
expect(isSemanticFootnoteBlockId('footnote-1-0')).toBe(false);
6667
});
68+
69+
it('isFootnoteLayoutBlockId matches DOCX and semantic painted footnote block ids', () => {
70+
expect(isFootnoteLayoutBlockId('footnote-1-0-paragraph')).toBe(true);
71+
expect(isFootnoteLayoutBlockId(SEMANTIC_FOOTNOTES_HEADING_BLOCK_ID)).toBe(true);
72+
expect(isFootnoteLayoutBlockId(`${SEMANTIC_FOOTNOTE_BLOCK_ID_PREFIX}-1-0-0-fn-1`)).toBe(true);
73+
expect(isFootnoteLayoutBlockId('body-1')).toBe(false);
74+
expect(isFootnoteLayoutBlockId(null)).toBe(false);
75+
expect(isFootnoteLayoutBlockId('')).toBe(false);
76+
});
6777
});

0 commit comments

Comments
 (0)