fix: correct tab leader from/to bounds when effectiveIndent is applied by roncallyt · Pull Request #2141 · superdoc-dev/superdoc

roncallyt · 2026-02-22T20:17:37Z

Summary

Fixes tab leader rendering in Word-generated Tables of Contents. Leaders were overlapping with section titles on the left and page numbers on the right due to effectiveIndent not being considered when calculating leader boundaries.

Closes #2075

Changes

layout-engine/measuring: Added effectiveIndent to from calculation so leaders start after indented text
layout-engine/measuring: Updated to after look-ahead to account for effectiveIndent, ensuring leaders end where right-aligned content begins

Showcase

ToC with lined tabs (3).docx

Before

After

ToC with dotted tabs (1).docx

Before

After

Observations

When I ran "pnpm run lint", got a lot of warnings that were unrelated to my changes, as it's unrelated I decided to not fix for now to keep this PR focused, but if needed I can fix them.
The same occurred to "pnpm run format:check", one warning shows up from another package unrelated to my changes, so I kept the same decision.

- Add effectiveIndent to leader `from` calculation so leaders start after indented text - Update leader `to` after look-ahead to account for effectiveIndent, ensuring leaders end where right-aligned content begins - Fix applies to `end`, `center`, and `decimal` tab stop alignments - Add unit tests for leader positioning with and without paragraph indent Closes superdoc-dev#2075

chatgpt-codex-connector · 2026-02-22T20:17:42Z

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

caio-pizzol

Hey @roncallyt, nice work!

The root cause is correct - leader bounds weren't accounting for effectiveIndent, so they overlapped with text and page numbers. The fix is clean and the before/after screenshots look great.

Couple of things to address before merging (details in inline comments)

Also, it would be great to add a visual regression test in tests/visual/ using one of the attached TOC documents to prevent future regressions on this.

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

Leader `from` and `to` were mixing relative and absolute coordinate spaces, causing start-aligned tabs with leaders to be off by `effectiveIndent` px. Both values now use absolute coordinates consistently. For end/center/decimal alignments, `to` is still overwritten by the look-ahead correction.

`leader.to` passed even without the absolute-coordinate fix. I measured the "42" text width independently so the check actually covers the `to` correction. Collapse the "without indent" and "with indent" variants into a single `it.each`.

roncallyt · 2026-02-23T21:41:14Z

@caio-pizzol I tried creating visual regression tests, but seems like I don't have permissions to do so. Perharps it's something to do with my Cloudflare account.

Anyway, is there another way to create them?

caio-pizzol · 2026-02-23T21:53:27Z

@caio-pizzol I tried creating visual regression tests, but seems like I don't have permissions to do so. Perharps it's something to do with my Cloudflare account.

Anyway, is there another way to create them?

Right! Let us add that on your side for now

caio-pizzol

all four items initial are addressed -- absolute coordinates, test precision, deduplication. looks good. left two non-blocking inline comments.

we'll add the visual regression test on our side.

caio-pizzol · 2026-02-23T21:55:50Z

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

@@ -1383,6 +1382,13 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P
              groupStartX = Math.max(0, relativeTarget - beforeDecimal);
            }

+            // Update leader "to" ensuring leaders end where right-aligned content begins
+            if (currentLine.leaders && currentLine.leaders.length > 0) {
+              const lastLeader = currentLine.leaders.at(-1);
+
+              if (lastLeader) lastLeader.to = groupStartX + effectiveIndent;
+            }
+
            // Set up active tab group for subsequent run processing
            activeTabGroup = {
              measure: groupMeasure,
@@ -2307,9 +2313,8 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P
        // Emit leader decoration if requested
        if (stop && stop.leader && stop.leader !== 'none' && stop.leader !== 'middleDot') {
          const leaderStyle: 'heavy' | 'dot' | 'hyphen' | 'underscore' = stop.leader;
-          const relativeTarget = clampedTarget - effectiveIndent;
-          const from = Math.min(originX, relativeTarget);
-          const to = Math.max(originX, relativeTarget);
+          const from = Math.min(originX + effectiveIndent, clampedTarget);
+          const to = Math.max(originX + effectiveIndent, clampedTarget);
          if (!currentLine.leaders) currentLine.leaders = [];
          currentLine.leaders.push({ from, to, style: leaderStyle });


the inline-tab path here doesn't correct to after content is positioned (the explicit tab-run path does at L1389).

means inline \t + right-aligned tab + leader could still overlap.

not urgent since TOCs use explicit tab runs -- just something to know about.

caio-pizzol · 2026-02-23T21:56:25Z

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


+            // Update leader "to" ensuring leaders end where right-aligned content begins
+            if (currentLine.leaders && currentLine.leaders.length > 0) {
+              const lastLeader = currentLine.leaders.at(-1);


.at(-1) grabs the last leader on the line, which might not be the one this tab just pushed. if a previous tab has a leader but this tab doesn't, this overwrites the wrong one.

easy fix: guard this block with stop.leader too, or save a reference when you push instead of fishing it back with .at(-1).

Fixed, I preferred to save a reference when pushing to the leaders array, it worked properly.

caio-pizzol · 2026-02-23T21:57:32Z

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


+            // Update leader "to" ensuring leaders end where right-aligned content begins
+            if (currentLine.leaders && currentLine.leaders.length > 0) {
+              const lastLeader = currentLine.leaders.at(-1);


nit: .length > 0 is redundant here -- .at(-1) returns undefined on empty arrays and the if (lastLeader) below handles it. can simplify to

Suggested change

const lastLeader = currentLine.leaders.at(-1);

const lastLeader = currentLine.leaders?.at(-1);

As I decided to save a reference to the current leader, this adjustment became unnecessary.

caio-pizzol · 2026-02-23T21:58:08Z

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

+      const measure = expectParagraphMeasure(await measureBlock(block, 1000));
+      expect(measure.lines).toHaveLength(1);
+
+      const leaders = measure.lines[0].leaders;


nit: expect(leaders).toBeDefined() + expect(leaders!.length).toBe(1) can be just expect(leaders).toHaveLength(1) -- covers both checks and avoids the ! assertion.

- Replace `toBeDefined()` and `toBe(1)` for `toHaveLength(1)`

Now saves a direct reference when pushing to the leaders array and uses it in the look-ahead correction.

caio-pizzol

@roncallyt thanks again for the fixes

left an inline comment + one note below. nothing blocking.

packages/layout-engine/layout-bridge/src/remeasure.ts:804 (not in diff, noting here)

applyTabLayoutToLines still computes relativeTarget = clampedTarget - effectiveIndent for leader from/to, so remeasured lines get relative coordinates while initial measurement now produces absolute ones. the renderer uses ld.from directly as CSS left without paddingLeft, so remeasured leaders land effectiveIndent pixels too far left. this was pre-existing, but the PR widens the gap between the two paths — worth fixing together or tracking in a follow-up issue?

caio-pizzol · 2026-02-27T15:10:40Z

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

+          const from = Math.min(originX + effectiveIndent, clampedTarget);
+          const to = Math.max(originX + effectiveIndent, clampedTarget);
          if (!currentLine.leaders) currentLine.leaders = [];
          currentLine.leaders.push({ from, to, style: leaderStyle });


the inline tab path sets pendingTabAlignment but never updates the leader's to after aligning end/center/decimal stops — the same correction that TabRun applies at line ~1385. for a typical TOC this won't show up (they use TabRun objects), but if someone hits an end-aligned stop with a leader via an inline \t, the dots would stretch past where the text starts. worth a follow-up?

Yeah, that makes sense, I'll look into it

roncallyt · 2026-02-27T17:34:10Z

Good catch! I'd prefer to fix this in the same PR to keep both paths consistent. I'll let you know as soon as I'm finished.

caio-pizzol reviewed Feb 23, 2026

View reviewed changes

caio-pizzol assigned roncallyt Feb 23, 2026

roncallyt added 2 commits February 23, 2026 14:25

caio-pizzol self-requested a review February 23, 2026 21:53

caio-pizzol approved these changes Feb 23, 2026

View reviewed changes

caio-pizzol requested a review from harbournick February 23, 2026 21:58

roncallyt added 2 commits February 23, 2026 18:28

test: fix unnecessary check and avoid ! assertion

ed692ed

- Replace `toBeDefined()` and `toBe(1)` for `toHaveLength(1)`

fix: save leader reference on push to avoid wrong leader mutation

3ff8cc6

Now saves a direct reference when pushing to the leaders array and uses it in the look-ahead correction.

caio-pizzol added the community label Feb 27, 2026

caio-pizzol reviewed Feb 27, 2026

View reviewed changes

	const lastLeader = currentLine.leaders.at(-1);
	const lastLeader = currentLine.leaders?.at(-1);

Conversation

roncallyt commented Feb 22, 2026

Summary

Changes

Showcase

Before

After

Before

After

Observations

Uh oh!

chatgpt-codex-connector bot commented Feb 22, 2026

Uh oh!

caio-pizzol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

roncallyt commented Feb 23, 2026

Uh oh!

caio-pizzol commented Feb 23, 2026

Uh oh!

caio-pizzol left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roncallyt Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caio-pizzol left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roncallyt commented Feb 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

roncallyt Feb 23, 2026 •

edited

Loading

caio-pizzol left a comment •

edited

Loading