Skip to content

Commit f276b34

Browse files
kendallertupizzcaio-pizzol
authored
feat(pm-adapter): synthesize internal link for PAGEREF with \h switch (#2899)
* feat(super-editor): render imported heading/bookmark cross-references (SD-2495) Wire the sd:crossReference v3 translator into the v2 importer entity list so REF / NOTEREF / STYLEREF fields imported from DOCX actually produce PM crossReference nodes instead of being silently dropped by the dispatch loop. Walk nested run wrappers when extracting the field's cached display text, and render the cross-reference as an internal hyperlink when the instruction carries the \\h switch. Route clicks on internal #bookmark anchors through goToAnchor so rendered cross-references navigate to their target in the document. Fixes IT-949 — Word cross-references (e.g. "Section 15") now appear in the viewer and are searchable, matching Word's output. * fix(presentation-editor): anchor nav writes scrollTop to the real scroll container #scrollPageIntoView wrote scrollTop to #visibleHost, which is typically overflow: visible and therefore not the actual scroll target. Anchor navigation (TOC clicks and SD-2495 cross-reference click-to-navigate) silently did nothing whenever the bookmark target was outside the current viewport — the PM selection moved but the viewport never scrolled. Write to #scrollContainer (the resolved scrollable ancestor) as the primary target, plus #visibleHost for backward compatibility with legacy layouts and the existing test harness that mocks scrollTop on the host element. This unblocks SD-2495's cross-reference click-to-navigate on docs where cross-references and their targets live on different pages. * feat(layout): show-bookmarks bracket indicators (SD-2454) Opt-in visual indicators for bookmark positions — mirrors Word's "Show bookmarks" (File > Options > Advanced). Off by default. - Pm-adapter bookmark-start and new bookmark-end converters emit gray `[` and `]` marker TextRuns when `layoutEngineOptions.showBookmarks` is true. Markers flow through pagination and line breaking as real characters, matching Word's own visual model. - Auto-generated bookmarks (`_Toc…`, `_Ref…`, `_GoBack`) are hidden even when the feature is on — matching Word. A `renderedBookmarkIds` set on the converter context pairs suppression so closing brackets don't orphan open ones. - PresentationEditor.setShowBookmarks toggles at runtime: clears the flow-block cache and schedules a re-render. - SuperDoc.setShowBookmarks is the public API passthrough. - Dev app gets a Show/Hide bookmarks toggle button in the header. - CSS: subtle gray, non-selectable so users don't include brackets in copied text. Bookmark name surfaces via the native title tooltip on the opening bracket. * test: close regression gaps for SD-2495 / SD-2454 / anchor-nav fix Fills the test gaps surfaced by the testing-excellence review of this PR: - crossReferenceImporter.integration.test.js (new, 4 tests): exercises the full v2 body pipeline (preprocessor -> dispatcher -> entity handler -> v3 translator). Asserts crossReferenceEntity is a member of the defaultNodeListHandler entities list, so the exact root cause that produced IT-949 ("Section 15" vanishing) fails loudly if a future refactor drops the wire-up. Unit tests of the translator alone cannot catch this — they bypass the dispatcher. - EditorInputManager.anchorClick.test.ts (new, 4 tests): pins the SD-2537 click-to-navigate routing. Clicking #bookmark hrefs routes through goToAnchor (was TOC-only before). External and empty-fragment hrefs are explicitly NOT routed. - cross-reference.test.ts: added marks-propagation test (node.marks flow into the emitted TextRun so italic/textStyle on xref text survives — SD-2537 "preserve surrounding run styling" AC). - bookmark-markers.test.ts: converted the `for` loop over auto-generated bookmark names into `it.each`. Each input now reports per-case on failure, complies with testing-excellence's "no control flow inside test bodies" guideline. - PresentationEditor.test.ts: documents why the scrollContainer-vs- visibleHost branch of the SD-2495 scrollPageIntoView fix isn't unit- testable here (happy-dom doesn't propagate inline overflow through getComputedStyle, which is what findScrollableAncestor uses). * feat(pm-adapter): synthesize internal link for PAGEREF with \h switch Mirrors the pattern added for crossReference in #2882. When a PAGEREF field instruction carries the `\h` switch, attach a FlowRunLink via `buildFlowRunLink({ anchor: bookmarkId })` so clicks on the rendered page number navigate to the referenced bookmark through the existing anchor-link routing (`EditorInputManager.#handleLinkClick` → `goToAnchor`). Previously the PAGEREF inline converter emitted the `pageReference` token run with `pageRefMetadata.bookmarkId` but no `link`, so the DOM layer never produced a clickable element for PAGEREFs. TOC entries and other hyperlinked page references imported from Word therefore failed to navigate on click, even though Word honored the `\h` switch. Depends on #2882 for `buildFlowRunLink` export and the generalized anchor-click routing. * fix(pm-adapter): match PAGEREF \h switch case-insensitively Word field switches are case-insensitive per the field-code grammar, so `\H` should produce the hyperlink the same as `\h`. Reviewer (codex-connector) flagged that the original `/\\h\b/` regex skipped the link synthesis for instructions like `PAGEREF _Toc123 \H`, leaving them non-navigable even though the author had requested hyperlink behavior. Adds the `i` flag and a regression test with an uppercase switch. * test: add coverage for standalone PAGEREF \h + REF-family preprocessors Behavior test (tests/behavior/tests/navigation/pageref-standalone-click.spec.ts): Covers the PR's load-bearing case - a PAGEREF \h field NOT wrapped in a <w:hyperlink>. The existing toc-anchor-scroll.spec.ts only exercises the wrapped-in-hyperlink shape, where the outer link mark already propagates via marksAsAttrs and the PR is a no-op. Fixtures exercise both \h and \H (case-insensitivity per ECMA-376 17.16.1). Preprocessor unit tests (ref/noteref/styleref): These three importer modules were added in #2882 without tests. Each verifies the preprocessor produces a sd:crossReference node with the right fieldType and preserves the instruction text verbatim. * test(superdoc): cover setShowBookmarks propagation and no-op guard Closes the Codecov patch-coverage gap on SuperDoc.js flagged on PR #2899. The uncovered setShowBookmarks method came in via the SD-2454 merge and had no test for its config mutation, no-op short-circuit, or Boolean() coercion. Models the existing setDisableContextMenu test pattern. * refactor(pm-adapter): drop TextRun casts + case-insensitive \h in cross-reference Addresses review feedback on #2899: - page-reference.ts: remove redundant `as TextRun` casts — textNodeToRun already returns TextRun, so the casts are noise (cross-reference.ts:34 already does this cleanly). Shorten the \h comment. - cross-reference.ts: add the `i` flag to the \h switch regex to match ECMA-376 §17.16.1, same fix as 7f19358 for page-reference. Add a regression test covering `\H`. * test: add unit coverage for scroll fan-out when ancestor != host Stubs window.getComputedStyle to mark a wrapper element as scrollable so #findScrollableAncestor returns the wrapper, then asserts both the wrapper and the visibleHost receive scrollTop. This pins the SD-2495 fix: a revert to the pre-fix one-liner (writing only to the visibleHost) now fails. --------- Co-authored-by: Tadeu Tupinamba <tadeu.tupiz@gmail.com> Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com> Co-authored-by: Caio Pizzol <caio@superdoc.dev>
1 parent da38a24 commit f276b34

32 files changed

Lines changed: 1352 additions & 32 deletions

packages/layout-engine/painters/dom/src/renderer.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5383,6 +5383,14 @@ export class DomPainter {
53835383
elem.style.zIndex = '1';
53845384
applyRunDataAttributes(elem as HTMLElement, (run as TextRun).dataAttrs);
53855385

5386+
// SD-2454: bookmark marker runs carry a data-bookmark-name attribute.
5387+
// Surface the bookmark name as a native `title` tooltip so hovering the
5388+
// opening bracket identifies which bookmark is being marked.
5389+
const bookmarkName = (run as TextRun).dataAttrs?.['data-bookmark-name'];
5390+
if (bookmarkName) {
5391+
(elem as HTMLElement).title = bookmarkName;
5392+
}
5393+
53865394
// Assert PM positions are present for cursor fallback
53875395
assertPmPositions(run, 'paragraph text run');
53885396

packages/layout-engine/painters/dom/src/styles.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,21 @@ const LINK_AND_TOC_STYLES = `
197197
}
198198
}
199199
200+
/* SD-2454: bookmark bracket indicators.
201+
* When the showBookmarks layout option is enabled, the pm-adapter emits
202+
* [ and ] marker TextRuns at bookmark start/end positions. Mirror Word's
203+
* visual treatment: subtle gray, non-selectable so users can't accidentally
204+
* include the brackets in copied text. The bookmark name is surfaced via
205+
* the native title tooltip on the opening bracket. */
206+
[data-bookmark-marker="start"],
207+
[data-bookmark-marker="end"] {
208+
color: #8b8b8b;
209+
user-select: none;
210+
cursor: default;
211+
font-weight: normal;
212+
}
213+
214+
200215
/* Reduced motion support */
201216
@media (prefers-reduced-motion: reduce) {
202217
.superdoc-link {

packages/layout-engine/pm-adapter/src/converter-context.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,23 @@ export type ConverterContext = {
5454
* Used by table creation paths to determine which style to apply to new tables.
5555
*/
5656
defaultTableStyleId?: string;
57+
/**
58+
* When true, emit visible gray `[` and `]` marker TextRuns at bookmarkStart
59+
* and bookmarkEnd positions — matching Word's "Show bookmarks" feature
60+
* (File > Options > Advanced). Off by default because bookmarks are a
61+
* structural concept, not a visual one. SD-2454.
62+
*/
63+
showBookmarks?: boolean;
64+
65+
/**
66+
* Populated by the bookmark-start inline converter during conversion: the
67+
* set of bookmark numeric ids (as strings) that actually rendered a start
68+
* marker. The bookmark-end converter reads this set to suppress emitting
69+
* an orphan `]` for a start it also suppressed (e.g. `_Toc…` / `_Ref…`
70+
* auto-generated bookmarks filtered out by the `showBookmarks` feature).
71+
* SD-2454.
72+
*/
73+
renderedBookmarkIds?: Set<string>;
5774
};
5875

5976
/**
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import type { TextRun } from '@superdoc/contracts';
2+
import type { PMNode } from '../../types.js';
3+
import { textNodeToRun } from './text-run.js';
4+
import { type InlineConverterParams } from './common.js';
5+
6+
/**
7+
* Converts a `bookmarkEnd` PM node.
8+
*
9+
* SD-2454: when `converterContext.showBookmarks` is true, emit a visible gray
10+
* `]` marker at the bookmark end. Matches Word's "Show bookmarks" rendering.
11+
* Returns void (no visual output) when the option is off, preserving today's
12+
* behavior where bookmarkEnd is an invisible structural marker.
13+
*
14+
* The PM schema does not store the bookmark name on bookmarkEnd — only the
15+
* numeric `id` that matches the corresponding bookmarkStart. We therefore
16+
* don't set a tooltip on the closing bracket (Word also omits the name on
17+
* the closing bracket's hover). Styling and identification happen on the
18+
* opening bracket.
19+
*/
20+
export function bookmarkEndNodeToRun(params: InlineConverterParams): TextRun | void {
21+
const { node, converterContext } = params;
22+
if (converterContext?.showBookmarks !== true) return;
23+
24+
const nodeAttrs =
25+
typeof node.attrs === 'object' && node.attrs !== null ? (node.attrs as Record<string, unknown>) : {};
26+
const bookmarkId = typeof nodeAttrs.id === 'string' || typeof nodeAttrs.id === 'number' ? String(nodeAttrs.id) : '';
27+
28+
// Only emit `]` if we emitted the matching `[`. Keeps brackets paired and
29+
// prevents an orphan closing bracket for a suppressed auto-generated
30+
// bookmark (`_Toc…`, `_Ref…`, `_GoBack`).
31+
const rendered = converterContext?.renderedBookmarkIds;
32+
if (rendered && bookmarkId && !rendered.has(bookmarkId)) return;
33+
34+
const run = textNodeToRun({
35+
...params,
36+
node: { type: 'text', text: ']', marks: [...(node.marks ?? [])] } as PMNode,
37+
});
38+
run.dataAttrs = {
39+
...(run.dataAttrs ?? {}),
40+
'data-bookmark-marker': 'end',
41+
...(bookmarkId ? { 'data-bookmark-id': bookmarkId } : {}),
42+
};
43+
return run;
44+
}
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import { describe, it, expect, vi } from 'vitest';
2+
import type { TextRun } from '@superdoc/contracts';
3+
import type { PMNode } from '../../types.js';
4+
import type { InlineConverterParams } from './common.js';
5+
6+
vi.mock('./text-run.js', () => ({
7+
textNodeToRun: vi.fn(
8+
(params: InlineConverterParams): TextRun => ({
9+
text: params.node.text || '',
10+
fontFamily: params.defaultFont,
11+
fontSize: params.defaultSize,
12+
}),
13+
),
14+
}));
15+
16+
import { bookmarkStartNodeToBlocks } from './bookmark-start.js';
17+
import { bookmarkEndNodeToRun } from './bookmark-end.js';
18+
19+
function makeParams(
20+
node: PMNode,
21+
opts: { showBookmarks?: boolean; bookmarks?: Map<string, number>; renderedBookmarkIds?: Set<string> } = {},
22+
): InlineConverterParams {
23+
return {
24+
node,
25+
positions: new WeakMap(),
26+
defaultFont: 'Calibri',
27+
defaultSize: 16,
28+
inheritedMarks: [],
29+
sdtMetadata: undefined,
30+
hyperlinkConfig: { enableRichHyperlinks: false },
31+
themeColors: undefined,
32+
runProperties: undefined,
33+
paragraphProperties: undefined,
34+
converterContext: {
35+
translatedNumbering: {},
36+
translatedLinkedStyles: { docDefaults: {}, latentStyles: {}, styles: {} },
37+
showBookmarks: opts.showBookmarks ?? false,
38+
renderedBookmarkIds: opts.renderedBookmarkIds,
39+
} as unknown as InlineConverterParams['converterContext'],
40+
enableComments: false,
41+
visitNode: vi.fn(),
42+
bookmarks: opts.bookmarks,
43+
tabOrdinal: 0,
44+
paragraphAttrs: {},
45+
nextBlockId: vi.fn(),
46+
} as InlineConverterParams;
47+
}
48+
49+
describe('bookmarkStartNodeToBlocks (SD-2454)', () => {
50+
it('emits no visible run when showBookmarks is off (default)', () => {
51+
const node: PMNode = { type: 'bookmarkStart', attrs: { name: 'chapter1', id: '1' } };
52+
const result = bookmarkStartNodeToBlocks(makeParams(node, { showBookmarks: false }));
53+
expect(result).toBeUndefined();
54+
});
55+
56+
it('emits a `[` TextRun with bookmark-name data attr when showBookmarks is on', () => {
57+
const node: PMNode = { type: 'bookmarkStart', attrs: { name: 'chapter1', id: '1' } };
58+
const result = bookmarkStartNodeToBlocks(makeParams(node, { showBookmarks: true }));
59+
expect(result).toBeDefined();
60+
expect(result!.text).toBe('[');
61+
expect(result!.dataAttrs).toEqual({
62+
'data-bookmark-name': 'chapter1',
63+
'data-bookmark-marker': 'start',
64+
});
65+
});
66+
67+
// Matches Word behavior: `_Toc…`, `_Ref…`, `_GoBack` etc. are hidden from
68+
// Show Bookmarks because they are internally generated for headings,
69+
// fields, or navigation — showing them would clutter the document.
70+
it.each(['_Toc1234', '_Ref506192326', '_GoBack'])('suppresses marker for auto-generated bookmark "%s"', (name) => {
71+
const node: PMNode = { type: 'bookmarkStart', attrs: { name, id: '1' } };
72+
const result = bookmarkStartNodeToBlocks(makeParams(node, { showBookmarks: true }));
73+
expect(result).toBeUndefined();
74+
});
75+
76+
it('still records bookmark position for cross-reference resolution regardless of showBookmarks', () => {
77+
const bookmarks = new Map<string, number>();
78+
const node: PMNode = { type: 'bookmarkStart', attrs: { name: 'chapter1', id: '1' } };
79+
const params = makeParams(node, { showBookmarks: false, bookmarks });
80+
// Seed the position map
81+
params.positions.set(node, { start: 42, end: 42 });
82+
bookmarkStartNodeToBlocks(params);
83+
expect(bookmarks.get('chapter1')).toBe(42);
84+
});
85+
});
86+
87+
describe('bookmarkEndNodeToRun (SD-2454)', () => {
88+
it('emits no run when showBookmarks is off (default)', () => {
89+
const node: PMNode = { type: 'bookmarkEnd', attrs: { id: '1' } };
90+
const result = bookmarkEndNodeToRun(makeParams(node, { showBookmarks: false }));
91+
expect(result).toBeUndefined();
92+
});
93+
94+
it('emits a `]` TextRun when the matching start was rendered', () => {
95+
const node: PMNode = { type: 'bookmarkEnd', attrs: { id: '1' } };
96+
const result = bookmarkEndNodeToRun(makeParams(node, { showBookmarks: true, renderedBookmarkIds: new Set(['1']) }));
97+
expect(result).toBeDefined();
98+
expect(result!.text).toBe(']');
99+
expect(result!.dataAttrs).toEqual({
100+
'data-bookmark-marker': 'end',
101+
'data-bookmark-id': '1',
102+
});
103+
});
104+
105+
it('suppresses `]` when the matching start was also suppressed (no orphan brackets)', () => {
106+
const node: PMNode = { type: 'bookmarkEnd', attrs: { id: '42' } };
107+
// Start with id 42 was suppressed — renderedBookmarkIds does not include it
108+
const result = bookmarkEndNodeToRun(
109+
makeParams(node, { showBookmarks: true, renderedBookmarkIds: new Set(['99']) }),
110+
);
111+
expect(result).toBeUndefined();
112+
});
113+
});
Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,68 @@
1-
import { type InlineConverterParams } from './common';
1+
import type { TextRun } from '@superdoc/contracts';
2+
import type { PMNode } from '../../types.js';
3+
import { textNodeToRun } from './text-run.js';
4+
import { type InlineConverterParams } from './common.js';
25

3-
export function bookmarkStartNodeToBlocks({
4-
node,
5-
positions,
6-
bookmarks,
7-
visitNode,
8-
inheritedMarks,
9-
sdtMetadata,
10-
runProperties,
11-
}: InlineConverterParams): void {
12-
// Track bookmark position for cross-reference resolution
6+
/**
7+
* Converts a `bookmarkStart` PM node.
8+
*
9+
* Primary job: record the bookmark's PM position in the `bookmarks` Map so
10+
* cross-reference navigation (goToAnchor) can resolve `#<bookmark-name>`
11+
* hrefs to a document position.
12+
*
13+
* SD-2454: when `converterContext.showBookmarks` is true, also emit a visible
14+
* gray `[` marker at the bookmark start, matching Word's opt-in "Show
15+
* bookmarks" feature. The marker is a regular TextRun so it flows through
16+
* pagination and line breaking like any other character; `dataAttrs` tag it
17+
* so DomPainter can style it gray and set a tooltip with the bookmark name.
18+
*
19+
* When `showBookmarks` is false (the default), the converter still descends
20+
* into any content inside the bookmark span but emits no visual output.
21+
*/
22+
export function bookmarkStartNodeToBlocks(params: InlineConverterParams): TextRun | void {
23+
const { node, positions, bookmarks, visitNode, inheritedMarks, sdtMetadata, runProperties, converterContext } =
24+
params;
1325
const nodeAttrs =
1426
typeof node.attrs === 'object' && node.attrs !== null ? (node.attrs as Record<string, unknown>) : {};
1527
const bookmarkName = typeof nodeAttrs.name === 'string' ? nodeAttrs.name : undefined;
28+
1629
if (bookmarkName && bookmarks) {
1730
const nodePos = positions.get(node);
1831
if (nodePos) {
1932
bookmarks.set(bookmarkName, nodePos.start);
2033
}
2134
}
22-
// Process any content inside the bookmark (usually empty)
35+
36+
// Word hides `_Toc…` / `_Ref…` / other `_`-prefixed bookmarks from its Show
37+
// Bookmarks rendering because they're autogenerated (headings, fields).
38+
// Mirror that so opt-in markers don't pollute every heading and xref target.
39+
const shouldRender =
40+
converterContext?.showBookmarks === true && typeof bookmarkName === 'string' && !bookmarkName.startsWith('_');
41+
42+
let run: TextRun | undefined;
43+
if (shouldRender) {
44+
run = textNodeToRun({
45+
...params,
46+
node: { type: 'text', text: '[', marks: [...(node.marks ?? [])] } as PMNode,
47+
});
48+
run.dataAttrs = {
49+
...(run.dataAttrs ?? {}),
50+
'data-bookmark-name': bookmarkName!,
51+
'data-bookmark-marker': 'start',
52+
};
53+
// Record the id so the matching bookmarkEnd converter knows to emit `]`.
54+
// Without this, suppressing a `_`-prefixed start leaves an orphan `]`.
55+
const bookmarkIdRaw = nodeAttrs.id;
56+
const bookmarkId =
57+
typeof bookmarkIdRaw === 'string' || typeof bookmarkIdRaw === 'number' ? String(bookmarkIdRaw) : '';
58+
if (bookmarkId && converterContext?.renderedBookmarkIds) {
59+
converterContext.renderedBookmarkIds.add(bookmarkId);
60+
}
61+
}
62+
2363
if (Array.isArray(node.content)) {
2464
node.content.forEach((child) => visitNode(child, inheritedMarks, sdtMetadata, runProperties));
2565
}
66+
67+
return run;
2668
}

0 commit comments

Comments
 (0)