fix(make-pdf): hide empty TOC page numbers and dot leaders in print - #2467
Open
dylansacks62 wants to merge 1 commit into
Open
fix(make-pdf): hide empty TOC page numbers and dot leaders in print#2467dylansacks62 wants to merge 1 commit into
dylansacks62 wants to merge 1 commit into
Conversation
Paged.js is not vendored in gstack, so the `target-counter` that would fill `.toc-page` never resolves (browse/src/meta-commands.ts:545 documents this: "make-pdf v1 ships without Paged.js; TOC renders without page numbers"). Chrome has no native target-counter, so the spans stay empty and the dot leaders run from each title into blank space, which reads as a broken document. screenCss() already hides both for HTML output. Do the same for print until page numbers actually exist, and leave a comment to restore the two rules if Paged.js ever lands. Also give H3 its own `level-3` class. buildTocBlock mapped every heading >= H2 to `level-2`, so sub-sections sat at the same indent as numbered sections and read as top-level peers. 189/189 make-pdf tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
make-pdf generate --tocproduces a table of contents where dot leaders run from each entry title into blank space. It reads as a broken document.Cause:
.toc-pageis filled by a CSStarget-counter, which only resolves under Paged.js. Paged.js is not vendored in gstack, and Chrome has no nativetarget-counter, so the spans stay permanently empty while the leader dots still render.browse/src/meta-commands.ts:545already documents the constraint: "make-pdf v1 ships without Paged.js; TOC renders without page numbers."This is not fixable in CSS alone as long as the counter has nothing to resolve against.
Fix
Hide
.toc-dotsand.toc-pagein print until page numbers actually exist.screenCss()already does exactly this for HTML output, so this brings print in line with the path that was already correct. A comment marks the two rules to restore the day Paged.js lands.Also:
buildTocBlockmapped every heading at H2 or deeper tolevel-2, so H3 sub-sections sat at the same indent as numbered sections and read as top-level peers. H3 now gets its ownlevel-3class.Result
Clean, clickable, correctly nested TOC. No trailing dots. Cover page, running headers,
N of Mfooters unaffected.Testing
bun testinmake-pdf/: 189 pass, 0 fail, 393expect()calls across 11 files.Note
If Paged.js is ever vendored, revert the single
display: nonerule and the original two declarations are restored verbatim from the comment. Thelevel-3change is independent and should stay either way.🤖 Generated with Claude Code