Skip to content

Commit 544a8e1

Browse files
authored
Merge pull request #3444 from superdoc-dev/luccas/sd-3237-bug-sdt-hover-and-click-to-place-cursor-interactions-are
fix(sdt): correct cursor placement and label interactions for structured content (SD-3237)
2 parents 7bb8063 + 1d1a9c3 commit 544a8e1

125 files changed

Lines changed: 10509 additions & 631 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,10 @@ export type FlowRunLink = {
299299
history?: boolean;
300300
};
301301

302+
export const EMPTY_SDT_PLACEHOLDER_TEXT = 'Click or tap here to enter text';
303+
304+
export type SdtVisualPlaceholder = 'emptyInlineSdt' | 'emptyBlockSdt';
305+
302306
/**
303307
* Common formatting marks that can be applied to any run type.
304308
* Used by TextRun, TabRun, and other run types that support inline formatting.
@@ -351,6 +355,8 @@ export type TextRun = RunMarks & {
351355
*/
352356
dataAttrs?: Record<string, string>;
353357
sdt?: SdtMetadata;
358+
/** Layout-only placeholder for visual affordances that do not represent document text. */
359+
visualPlaceholder?: SdtVisualPlaceholder;
354360
link?: FlowRunLink;
355361
/** Token annotations for dynamic content (page numbers, etc.). */
356362
token?: 'pageNumber' | 'totalPageCount' | 'pageReference';
@@ -467,10 +473,10 @@ export type ImageRun = {
467473

468474
/**
469475
* Vertical alignment of image relative to text baseline.
470-
* Currently only 'bottom' is supported (image sits on baseline).
471-
* Future: 'top', 'middle', 'baseline', 'text-top', 'text-bottom'.
476+
* 'top' keeps the image box inside the measured line height; 'bottom'
477+
* preserves legacy baseline alignment for existing callers.
472478
*/
473-
verticalAlign?: 'bottom';
479+
verticalAlign?: 'top' | 'bottom';
474480

475481
/** Absolute ProseMirror position (inclusive) of this image run. */
476482
pmStart?: number;
@@ -2224,6 +2230,11 @@ export { isResolvedTableItem, isResolvedImageItem, isResolvedDrawingItem } from
22242230

22252231
// Pure transformations on inline-run shapes (used by pm-adapter, layout-bridge,
22262232
// and painter-dom). Located in contracts to avoid reverse stage dependencies.
2227-
export { expandRunsForInlineNewlines, sliceRunsForLine } from './run-helpers.js';
2233+
export {
2234+
expandRunsForInlineNewlines,
2235+
isEmptyInlineSdtPlaceholderRun,
2236+
isEmptySdtPlaceholderRun,
2237+
sliceRunsForLine,
2238+
} from './run-helpers.js';
22282239

22292240
export * as Engines from './engines/index.js';

packages/layout-engine/contracts/src/pm-range.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { FlowBlock, Line, ParagraphBlock, ParagraphMeasure } from './index.js';
2+
import { isEmptySdtPlaceholderRun } from './run-helpers.js';
23

34
/**
45
* Represents a ProseMirror position range for a line or fragment.
@@ -93,6 +94,15 @@ export function computeLinePmRange(block: FlowBlock, line: Line): LinePmRange {
9394
const runPmStart = coercePmStart(run);
9495
if (runPmStart == null) continue;
9596

97+
if (isEmptySdtPlaceholderRun(run)) {
98+
const runPmEnd = coercePmEnd(run) ?? runPmStart;
99+
if (pmStart == null) {
100+
pmStart = runPmStart;
101+
}
102+
pmEnd = runPmEnd;
103+
continue;
104+
}
105+
96106
if (isAtomicRunKind((run as { kind?: unknown }).kind) || isImageLikeRun(run)) {
97107
const runPmEnd = coercePmEnd(run) ?? runPmStart + 1;
98108
if (pmStart == null) {

packages/layout-engine/contracts/src/run-helpers.test.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, it, expect } from 'vitest';
22
import type { FlowBlock, Line, ParagraphBlock, Run, TabRun, TextRun, TrackedChangeMeta } from './index.js';
3-
import { expandRunsForInlineNewlines, sliceRunsForLine } from './run-helpers.js';
3+
import { expandRunsForInlineNewlines, isEmptySdtPlaceholderRun, sliceRunsForLine } from './run-helpers.js';
44

55
describe('expandRunsForInlineNewlines', () => {
66
const makeRun = (text: string, pmStart = 0): TextRun => ({
@@ -136,4 +136,34 @@ describe('sliceRunsForLine', () => {
136136
const line = makeLine({ fromRun: 0, fromChar: 2, toRun: 0, toChar: 2 });
137137
expect(sliceRunsForLine(block, line)).toEqual([]);
138138
});
139+
140+
it('preserves empty inline SDT visual placeholders', () => {
141+
const run: TextRun = {
142+
kind: 'text',
143+
text: '',
144+
fontFamily: 'Arial',
145+
fontSize: 12,
146+
pmStart: 10,
147+
pmEnd: 10,
148+
visualPlaceholder: 'emptyInlineSdt',
149+
sdt: { type: 'structuredContent', scope: 'inline', id: 'sdt-1' },
150+
};
151+
const block = makeParagraph([run]);
152+
const line = makeLine({ fromRun: 0, fromChar: 0, toRun: 0, toChar: 0 });
153+
154+
expect(sliceRunsForLine(block, line)).toEqual([run]);
155+
});
156+
157+
it('recognizes block SDT visual placeholders', () => {
158+
const run: TextRun = {
159+
kind: 'text',
160+
text: '',
161+
fontFamily: 'Arial',
162+
fontSize: 12,
163+
visualPlaceholder: 'emptyBlockSdt',
164+
sdt: { type: 'structuredContent', scope: 'block', id: 'sdt-block-1' },
165+
};
166+
167+
expect(isEmptySdtPlaceholderRun(run)).toBe(true);
168+
});
139169
});

packages/layout-engine/contracts/src/run-helpers.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,20 @@
99

1010
import type { FlowBlock, Line, Run, TextRun } from './index.js';
1111

12+
export function isEmptySdtPlaceholderRun(
13+
run: Run,
14+
): run is TextRun & { visualPlaceholder: 'emptyInlineSdt' | 'emptyBlockSdt' } {
15+
return (
16+
(run.kind === 'text' || run.kind === undefined) &&
17+
'text' in run &&
18+
((run as TextRun).visualPlaceholder === 'emptyInlineSdt' || (run as TextRun).visualPlaceholder === 'emptyBlockSdt')
19+
);
20+
}
21+
22+
export function isEmptyInlineSdtPlaceholderRun(run: Run): run is TextRun & { visualPlaceholder: 'emptyInlineSdt' } {
23+
return isEmptySdtPlaceholderRun(run) && run.visualPlaceholder === 'emptyInlineSdt';
24+
}
25+
1226
/**
1327
* Expands text runs that contain inline newlines into multiple runs.
1428
*
@@ -82,6 +96,11 @@ export function sliceRunsForLine(block: FlowBlock, line: Line): Run[] {
8296
}
8397

8498
const text = run.text ?? '';
99+
if (isEmptySdtPlaceholderRun(run)) {
100+
result.push(run);
101+
continue;
102+
}
103+
85104
const isFirstRun = runIndex === line.fromRun;
86105
const isLastRun = runIndex === line.toRun;
87106

packages/layout-engine/dom-contract/src/class-names.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,15 @@ export const DOM_CLASS_NAMES = {
2727
*/
2828
INLINE_SDT_WRAPPER: 'superdoc-structured-content-inline',
2929

30+
/** Inline structured-content label chrome. */
31+
INLINE_SDT_LABEL: 'superdoc-structured-content-inline__label',
32+
3033
/** Block-level structured-content container. */
3134
BLOCK_SDT: 'superdoc-structured-content-block',
3235

36+
/** Block-level structured-content label chrome. */
37+
BLOCK_SDT_LABEL: 'superdoc-structured-content__label',
38+
3339
/** Table fragment container (resize overlay and click-mapping target). */
3440
TABLE_FRAGMENT: 'superdoc-table-fragment',
3541

@@ -66,3 +72,9 @@ export const DOM_CLASS_NAMES = {
6672

6773
/** Union of all DOM contract class name values. */
6874
export type DomClassName = (typeof DOM_CLASS_NAMES)[keyof typeof DOM_CLASS_NAMES];
75+
76+
/** Structured-content chrome labels that should not drive text-position mapping. */
77+
export const STRUCTURED_CONTENT_CHROME_LABEL_CLASS_NAMES = [
78+
DOM_CLASS_NAMES.INLINE_SDT_LABEL,
79+
DOM_CLASS_NAMES.BLOCK_SDT_LABEL,
80+
] as const;

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest';
22

33
import {
44
DOM_CLASS_NAMES,
5+
STRUCTURED_CONTENT_CHROME_LABEL_CLASS_NAMES,
56
DATA_ATTRS,
67
DATASET_KEYS,
78
buildImagePmSelector,
@@ -24,7 +25,9 @@ describe('@superdoc/dom-contract', () => {
2425
FRAGMENT: 'superdoc-fragment',
2526
LINE: 'superdoc-line',
2627
INLINE_SDT_WRAPPER: 'superdoc-structured-content-inline',
28+
INLINE_SDT_LABEL: 'superdoc-structured-content-inline__label',
2729
BLOCK_SDT: 'superdoc-structured-content-block',
30+
BLOCK_SDT_LABEL: 'superdoc-structured-content__label',
2831
TABLE_FRAGMENT: 'superdoc-table-fragment',
2932
DOCUMENT_SECTION: 'superdoc-document-section',
3033
SDT_GROUP_HOVER: 'sdt-group-hover',
@@ -38,6 +41,13 @@ describe('@superdoc/dom-contract', () => {
3841
});
3942
});
4043

44+
it('exports the structured content chrome label class set', () => {
45+
expect(STRUCTURED_CONTENT_CHROME_LABEL_CLASS_NAMES).toEqual([
46+
DOM_CLASS_NAMES.INLINE_SDT_LABEL,
47+
DOM_CLASS_NAMES.BLOCK_SDT_LABEL,
48+
]);
49+
});
50+
4151
it('exports the stable data attribute names and dataset keys', () => {
4252
expect(DATA_ATTRS).toEqual({
4353
PM_START: 'data-pm-start',

packages/layout-engine/dom-contract/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* implementation details.
1414
*/
1515

16-
export { DOM_CLASS_NAMES } from './class-names.js';
16+
export { DOM_CLASS_NAMES, STRUCTURED_CONTENT_CHROME_LABEL_CLASS_NAMES } from './class-names.js';
1717
export type { DomClassName } from './class-names.js';
1818

1919
export { DATA_ATTRS, DATASET_KEYS, encodeLayoutStoryDataset, decodeLayoutStoryDataset } from './data-attrs.js';

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type {
33
ImageBlock,
44
DrawingBlock,
55
ImageDrawing,
6+
ImageRun,
67
BoxSpacing,
78
ImageAnchor,
89
ImageWrap,
@@ -419,6 +420,12 @@ const paragraphBlocksEqual = (a: FlowBlock & { kind: 'paragraph' }, b: FlowBlock
419420
for (let i = 0; i < a.runs.length; i += 1) {
420421
const runA = a.runs[i];
421422
const runB = b.runs[i];
423+
if (runA.kind === 'image' || runB.kind === 'image') {
424+
if (runA.kind !== 'image' || runB.kind !== 'image') return false;
425+
if (!imageRunsEqual(runA, runB)) return false;
426+
continue;
427+
}
428+
422429
// MathRun: compare textContent (derived from OMML) to detect equation changes
423430
if (runA.kind === 'math' || runB.kind === 'math') {
424431
if (runA.kind !== runB.kind) return false;
@@ -449,6 +456,32 @@ const paragraphBlocksEqual = (a: FlowBlock & { kind: 'paragraph' }, b: FlowBlock
449456
return true;
450457
};
451458

459+
const imageRunsEqual = (a: ImageRun, b: ImageRun): boolean => {
460+
return (
461+
a.src === b.src &&
462+
a.width === b.width &&
463+
a.height === b.height &&
464+
a.alt === b.alt &&
465+
a.title === b.title &&
466+
a.clipPath === b.clipPath &&
467+
a.distTop === b.distTop &&
468+
a.distBottom === b.distBottom &&
469+
a.distLeft === b.distLeft &&
470+
a.distRight === b.distRight &&
471+
a.verticalAlign === b.verticalAlign &&
472+
a.rotation === b.rotation &&
473+
a.flipH === b.flipH &&
474+
a.flipV === b.flipV &&
475+
a.gain === b.gain &&
476+
a.blacklevel === b.blacklevel &&
477+
a.grayscale === b.grayscale &&
478+
jsonEqual(a.lum, b.lum) &&
479+
jsonEqual(a.hyperlink, b.hyperlink) &&
480+
jsonEqual(a.sdt, b.sdt) &&
481+
shallowRecordEqual(a.dataAttrs, b.dataAttrs)
482+
);
483+
};
484+
452485
const imageBlocksEqual = (a: ImageBlock | ImageDrawing, b: ImageBlock | ImageDrawing): boolean => {
453486
return (
454487
a.src === b.src &&

packages/layout-engine/layout-bridge/src/dom-mapping.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
* @deprecated Use DomPointerMapping from super-editor/dom-observer instead.
2525
*/
2626

27-
import { DOM_CLASS_NAMES } from '@superdoc/dom-contract';
27+
import { DOM_CLASS_NAMES, STRUCTURED_CONTENT_CHROME_LABEL_CLASS_NAMES } from '@superdoc/dom-contract';
2828

2929
// Debug logging for click-to-position pipeline (disabled - enable for debugging)
3030
const DEBUG_CLICK_MAPPING = false;
@@ -90,6 +90,10 @@ function isVisibleRect(rect: DOMRect): boolean {
9090
return rect.width > 0 && rect.height > 0;
9191
}
9292

93+
function isStructuredContentChromeLabel(el: HTMLElement): boolean {
94+
return STRUCTURED_CONTENT_CHROME_LABEL_CLASS_NAMES.some((className) => el.classList.contains(className));
95+
}
96+
9397
/**
9498
* Maps a click coordinate to a ProseMirror document position using DOM data attributes.
9599
*
@@ -384,7 +388,8 @@ function processFragment(fragmentEl: HTMLElement, viewX: number, viewY: number):
384388
(el) =>
385389
el.dataset.pmStart !== undefined &&
386390
el.dataset.pmEnd !== undefined &&
387-
!el.classList.contains(DOM_CLASS_NAMES.INLINE_SDT_WRAPPER),
391+
!el.classList.contains(DOM_CLASS_NAMES.INLINE_SDT_WRAPPER) &&
392+
!isStructuredContentChromeLabel(el),
388393
);
389394

390395
log(
@@ -462,7 +467,8 @@ function processLineElement(lineEl: HTMLElement, viewX: number): number | null {
462467
(el) =>
463468
el.dataset.pmStart !== undefined &&
464469
el.dataset.pmEnd !== undefined &&
465-
!el.classList.contains(DOM_CLASS_NAMES.INLINE_SDT_WRAPPER),
470+
!el.classList.contains(DOM_CLASS_NAMES.INLINE_SDT_WRAPPER) &&
471+
!isStructuredContentChromeLabel(el),
466472
);
467473

468474
log(

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import type {
1010
ParagraphIndent,
1111
LeaderDecoration,
1212
} from '@superdoc/contracts';
13-
import { Engines } from '@superdoc/contracts';
13+
import { EMPTY_SDT_PLACEHOLDER_TEXT, Engines, isEmptySdtPlaceholderRun } from '@superdoc/contracts';
1414
import type { WordParagraphLayoutOutput } from '@superdoc/word-layout';
1515
import {
1616
LIST_MARKER_GAP as _LIST_MARKER_GAP,
@@ -126,6 +126,10 @@ function fontString(run: Run): string {
126126
* @returns Text content of the run, or empty string for non-text runs
127127
*/
128128
function runText(run: Run): string {
129+
if (isEmptySdtPlaceholderRun(run)) {
130+
return run.sdt?.type === 'structuredContent' && run.sdt.appearance === 'hidden' ? '' : EMPTY_SDT_PLACEHOLDER_TEXT;
131+
}
132+
129133
return 'src' in run ||
130134
run.kind === 'lineBreak' ||
131135
run.kind === 'break' ||
@@ -1380,6 +1384,17 @@ export function remeasureParagraph(
13801384
if (text.length > 0 && isTextRun(run)) {
13811385
lineMaxTextFontSize = Math.max(lineMaxTextFontSize, run.fontSize ?? 16);
13821386
}
1387+
if (isEmptySdtPlaceholderRun(run)) {
1388+
const placeholderWidth = text.length > 0 ? measureRunSliceWidth(run, 0, text.length) : 0;
1389+
if (width > 0 && width + placeholderWidth > effectiveMaxWidth - WIDTH_FUDGE_PX) {
1390+
didBreakInThisLine = true;
1391+
break;
1392+
}
1393+
width += placeholderWidth;
1394+
endRun = r;
1395+
endChar = text.length > 0 ? text.length : start + 1;
1396+
continue;
1397+
}
13831398
for (let c = start; c < text.length; c += 1) {
13841399
const ch = text[c];
13851400
if (ch === '\t') {

0 commit comments

Comments
 (0)