Skip to content

bug: list items are cut off on left edge inside table (SD-1836) #1996

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
luccas-harbour wants to merge 2 commits into
base: main
Choose a base branch
from
+137 −90
Open

bug: list items are cut off on left edge inside table (SD-1836) #1996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
91938a5
refactor: extract helper for computing marker tab width
Feb 11, 2026
8b0e219
fix: correctly compute list markers inside tables
Feb 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 1 addition & 60 deletions packages/layout-engine/painters/dom/src/renderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ import {
type SdtBoundaryOptions,
} from './utils/sdt-helpers.js';
import { SdtGroupedHover } from './utils/sdt-hover.js';
import { computeTabWidth } from './utils/marker-helpers.js';
import { generateRulerDefinitionFromPx, createRulerElement, ensureRulerStyles } from './ruler/index.js';
import { toCssFontFamily } from '@superdoc/font-utils';
import {
Expand Down Expand Up @@ -368,12 +369,6 @@ export type FragmentRenderContext = {
};

const LIST_MARKER_GAP = 8;
/**
* Default tab interval in pixels (0.5 inch at 96 DPI).
* Used when calculating tab stops for list markers that extend past the implicit tab stop.
* This matches Microsoft Word's default tab interval behavior.
*/
const DEFAULT_TAB_INTERVAL_PX = 48;
/**
* Default page height in pixels (11 inches at 96 DPI).
* Used as a fallback when page size information is not available for ruler rendering.
Expand Down Expand Up @@ -6331,57 +6326,3 @@ const resolveRunText = (run: Run, context: FragmentRenderContext): string => {
}
return run.text ?? '';
};

const computeTabWidth = (
currentPos: number,
justification: string,
tabs: number[] | undefined,
hangingIndent: number | undefined,
firstLineIndent: number | undefined,
leftIndent: number,
): number => {
const nextDefaultTabStop = currentPos + DEFAULT_TAB_INTERVAL_PX - (currentPos % DEFAULT_TAB_INTERVAL_PX);
let tabWidth: number;
if ((justification ?? 'left') === 'left') {
// Check for explicit tab stops past current position
const explicitTabs = [...(tabs ?? [])];
if (hangingIndent && hangingIndent > 0) {
// Account for hanging indent by adding an implicit tab stop at (left + hanging)
const implicitTabPos = leftIndent; // paraIndentLeft already accounts for hanging
explicitTabs.push(implicitTabPos);
// Sort tab stops to maintain order
explicitTabs.sort((a, b) => {
if (typeof a === 'number' && typeof b === 'number') {
return a - b;
}
return 0;
});
}
let targetTabStop: number | undefined;

if (Array.isArray(explicitTabs) && explicitTabs.length > 0) {
// Find the first tab stop that's past the current position
for (const tab of explicitTabs) {
if (typeof tab === 'number' && tab > currentPos) {
targetTabStop = tab;
break;
}
}
}

if (targetTabStop === undefined) {
// advance to next default 48px tab interval, matching Word behavior.
targetTabStop = nextDefaultTabStop;
}
tabWidth = targetTabStop - currentPos;
} else if (justification === 'right') {
if (firstLineIndent != null && firstLineIndent > 0) {
tabWidth = nextDefaultTabStop - currentPos;
} else {
tabWidth = hangingIndent ?? 0;
}
} else {
tabWidth = nextDefaultTabStop - currentPos;
}
return tabWidth;
};
106 changes: 76 additions & 30 deletions packages/layout-engine/painters/dom/src/table/renderTableCell.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
type SdtBoundaryOptions,
} from '../utils/sdt-helpers.js';
import { normalizeZIndex } from '@superdoc/pm-adapter/utilities.js';
import { computeTabWidth } from '../utils/marker-helpers.js';

/**
* Default gap between list marker and text content in pixels.
Expand Down Expand Up @@ -68,6 +69,7 @@ type WordLayoutMarker = {
/** Letter spacing in pixels */
letterSpacing?: number;
};
suffix?: 'tab' | 'space' | 'nothing';
};

/**
Expand Down Expand Up @@ -100,6 +102,12 @@ type MarkerRenderParams = {
markerMeasure: ParagraphMeasure['marker'];
/** Left indent in pixels */
indentLeftPx: number;
/** Hanging indent in pixels */
hangingIndentPx: number;
/** First line indent in pixels */
firstLineIndentPx: number;
/** Array of explicit tab stop positions in pixels. */
tabsPx?: number[];
};

/**
Expand Down Expand Up @@ -145,30 +153,39 @@ type TableCellIndentParams = {
* @returns Container element with marker and line as children
*/
function renderListMarker(params: MarkerRenderParams): HTMLElement {
const { doc, lineEl, markerLayout, markerMeasure, indentLeftPx } = params;
const { doc, lineEl, markerLayout, markerMeasure, indentLeftPx, hangingIndentPx, firstLineIndentPx, tabsPx } = params;

const markerJustification = markerLayout?.justification ?? 'left';

// Extract marker box width with fallback chain: layout -> measure -> 0
const markerBoxWidth =
(typeof markerLayout?.markerBoxWidthPx === 'number' ? markerLayout.markerBoxWidthPx : undefined) ??
markerMeasure?.markerWidth ??
0;
const anchorPoint = indentLeftPx - hangingIndentPx + firstLineIndentPx;

// Extract gutter width with fallback chain: layout -> measure -> default gap
const gutter =
(typeof markerLayout?.gutterWidthPx === 'number' ? markerLayout.gutterWidthPx : undefined) ??
markerMeasure?.gutterWidth ??
LIST_MARKER_GAP;

// Calculate marker start position based on justification
const markerStartPos =
markerJustification === 'left'
? indentLeftPx
: ((typeof markerLayout?.markerX === 'number' ? markerLayout.markerX : undefined) ?? indentLeftPx);
const markerJustification = markerLayout?.justification ?? 'left';
const markerTextWidth = markerMeasure?.markerTextWidth ?? 0;

let markerStartPos: number, currentPos: number;
if (markerJustification === 'left') {
markerStartPos = anchorPoint;
currentPos = markerStartPos + markerTextWidth;
} else if (markerJustification === 'right') {
markerStartPos = anchorPoint - markerTextWidth;
currentPos = anchorPoint;
} else {
markerStartPos = anchorPoint - markerTextWidth / 2;
Copy link
Contributor

@caio-pizzol caio-pizzol Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

center justification computes left = anchorPoint - markerTextWidth (half-width subtracted twice), identical to right-justified positioning. renderer.ts line 2305 has the same math -- intentional, or pre-existing bug?

currentPos = markerStartPos + markerTextWidth;
}

// Marker left position is marker start minus the width of the marker box
const markerLeftPos = markerStartPos - markerBoxWidth;
const suffix = markerLayout?.suffix ?? 'tab';
let listTabWidth = 0;
if (suffix === 'tab') {
listTabWidth = computeTabWidth(
currentPos,
markerJustification,
tabsPx,
hangingIndentPx,
firstLineIndentPx,
indentLeftPx,
);
} else if (suffix === 'space') {
listTabWidth = 4;
}

// Create container to hold both marker and line
const lineContainer = doc.createElement('div');
Expand All @@ -193,17 +210,39 @@ function renderListMarker(params: MarkerRenderParams): HTMLElement {
markerEl.style.letterSpacing = `${markerLayout.run.letterSpacing}px`;
}

// Position marker absolutely within the container
markerEl.style.position = 'absolute';
markerEl.style.left = `${markerLeftPos}px`;
markerEl.style.width = `${markerBoxWidth}px`;
markerEl.style.textAlign = markerJustification;
markerEl.style.paddingRight = `${gutter}px`;

// Align text start to the marker start position (gutter spacing comes from marker padding)
lineEl.style.paddingLeft = `${markerStartPos}px`;
// Position marker within the container
if (['center', 'right'].includes(markerJustification)) {
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Apply anchor offset for left-justified table list markers

This block only applies marker/text offset logic for center and right, so the default left case never uses anchorPoint to position the first line. In table list paragraphs with non-zero list indents/hanging indents, the marker and following text are laid out from the cell edge instead of the computed list anchor, which shifts content left and can still clip near the table border.

Useful? React with 👍 / 👎.

Copy link
Contributor

@caio-pizzol caio-pizzol Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left-justified markers start at x=0 in the container -- anchorPoint offset is not applied. continuation lines get paddingLeft = indentLeftPx, so first-line text won't align with body text.

maybe lineEl.paddingLeft = anchorPoint would fix this (like in renderer.ts)

lineEl.style.paddingLeft = parseFloat(lineEl.style.paddingLeft || '0') + currentPos + listTabWidth + 'px';
Copy link
Contributor

@caio-pizzol caio-pizzol Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

paddingLeft includes listTabWidth, but the separator element (width listTabWidth) is a sibling outside lineEl -- tab width counted twice. text shifts one tab-width too far right.

should we drop listTabWidth from the padding or move the separator inside lineEl?

if (markerJustification === 'right') {
markerEl.style.position = 'absolute';
markerEl.style.left = `${markerStartPos}px`;
} else {
markerEl.style.position = 'absolute';
markerEl.style.left = `${markerStartPos - markerTextWidth / 2}px`;
}
}
lineEl.style.display = 'inline-block';

// Add separator
let separatorEl;
if (suffix === 'tab') {
separatorEl = doc.createElement('span');
separatorEl.className = 'superdoc-tab';
separatorEl.innerHTML = ' ';
separatorEl.style.display = 'inline-block';
separatorEl.style.wordSpacing = '0px';
separatorEl.style.width = `${listTabWidth}px`;
} else if (suffix === 'space') {
separatorEl = doc.createElement('span');
separatorEl.className = 'superdoc-marker-suffix-space';
separatorEl.style.wordSpacing = '0px';
separatorEl.textContent = '\u00A0';
}

lineContainer.appendChild(markerEl);
if (separatorEl) {
lineContainer.appendChild(separatorEl);
}
lineContainer.appendChild(lineEl);

return lineContainer;
Expand Down Expand Up @@ -887,6 +926,10 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen
markerMeasure?.indentLeft ??
wordLayout?.indentLeftPx ??
(block.attrs?.indent && typeof block.attrs.indent.left === 'number' ? block.attrs.indent.left : 0);
const hangingIndentPx =
block.attrs?.indent && typeof block.attrs.indent.hanging === 'number' ? block.attrs.indent.hanging : 0;
const firstLineIndentPx =
block.attrs?.indent && typeof block.attrs.indent.firstLine === 'number' ? block.attrs.indent.firstLine : 0;
const suppressFirstLineIndent = block.attrs?.suppressFirstLineIndent === true;

// Calculate the global line indices for this block
Expand All @@ -910,6 +953,7 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen
// Create wrapper for this paragraph's SDT metadata
// Use absolute positioning within the content container to stack blocks vertically
const paraWrapper = doc.createElement('div');
paraWrapper.classList.add('superdoc-table-paragraph');
Copy link
Contributor

@caio-pizzol caio-pizzol Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

superdoc-table-paragraph class added but no CSS or JS references it.

paraWrapper.style.position = 'relative';
paraWrapper.style.left = '0';
paraWrapper.style.width = '100%';
Expand Down Expand Up @@ -978,6 +1022,8 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen
markerLayout,
markerMeasure,
indentLeftPx,
hangingIndentPx,
firstLineIndentPx,
});
Comment on lines 1023 to 1027
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Pass paragraph tab stops when rendering table list markers

renderListMarker now computes tab spacing from tabsPx, but this call site never forwards wordLayout?.tabsPx, so explicit tab stops from the paragraph are dropped for list items inside table cells. Documents that use custom list tab stops will therefore fall back to default/implicit tab widths and render marker-to-text spacing incorrectly.

Useful? React with 👍 / 👎.

renderedLines.push({ el: lineContainer, top: lineTop, height: line.lineHeight });
paraWrapper.appendChild(lineContainer);
Expand Down
60 changes: 60 additions & 0 deletions packages/layout-engine/painters/dom/src/utils/marker-helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/**
Copy link
Contributor

@caio-pizzol caio-pizzol Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extraction is justified (2 call sites) but the export has no JSDoc.

let's add a one-liner describing what it does.

* Default tab interval in pixels (0.5 inch at 96 DPI).
* Used when calculating tab stops for list markers that extend past the implicit tab stop.
* This matches Microsoft Word's default tab interval behavior.
*/
const DEFAULT_TAB_INTERVAL_PX = 48;

export const computeTabWidth = (
currentPos: number,
justification: string,
tabs: number[] | undefined,
hangingIndent: number | undefined,
firstLineIndent: number | undefined,
leftIndent: number,
): number => {
const nextDefaultTabStop = currentPos + DEFAULT_TAB_INTERVAL_PX - (currentPos % DEFAULT_TAB_INTERVAL_PX);
let tabWidth: number;
if ((justification ?? 'left') === 'left') {
Copy link
Contributor

@caio-pizzol caio-pizzol Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(justification ?? 'left') -- param is string, never null or undefined. redudant?

// Check for explicit tab stops past current position
const explicitTabs = [...(tabs ?? [])];
if (hangingIndent && hangingIndent > 0) {
// Account for hanging indent by adding an implicit tab stop at (left + hanging)
const implicitTabPos = leftIndent; // paraIndentLeft already accounts for hanging
explicitTabs.push(implicitTabPos);
// Sort tab stops to maintain order
explicitTabs.sort((a, b) => {
if (typeof a === 'number' && typeof b === 'number') {
Copy link
Contributor

@caio-pizzol caio-pizzol Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typeof guards in sort comparator looks like dead code -- the array is number[].

return a - b;
}
return 0;
});
}
let targetTabStop: number | undefined;

if (Array.isArray(explicitTabs) && explicitTabs.length > 0) {
// Find the first tab stop that's past the current position
for (const tab of explicitTabs) {
if (typeof tab === 'number' && tab > currentPos) {
targetTabStop = tab;
break;
}
}
}

if (targetTabStop === undefined) {
// advance to next default 48px tab interval, matching Word behavior.
targetTabStop = nextDefaultTabStop;
}
tabWidth = targetTabStop - currentPos;
} else if (justification === 'right') {
if (firstLineIndent != null && firstLineIndent > 0) {
tabWidth = nextDefaultTabStop - currentPos;
} else {
tabWidth = hangingIndent ?? 0;
}
} else {
tabWidth = nextDefaultTabStop - currentPos;
}
return tabWidth;
};
Loading