SD-2508 - fix: toc not being displayed inside w:sdt#2850
SD-2508 - fix: toc not being displayed inside w:sdt#2850chittolinag wants to merge 6 commits intomainfrom
Conversation
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad510e09aa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| child.content, | ||
| { docPartGallery: docPartGallery ?? '', docPartObjectId, tocInstruction, sdtMetadata: docPartSdtMetadata }, |
There was a problem hiding this comment.
The new non-"Table of Contents" branch calls processTocChildren with child.content instead of the tableOfContents node itself, which drops the nested node context where processTocChildren reads getNodeInstruction(child) (toc.ts). In documents where a custom gallery stores the TOC field code on the nested tableOfContents node (the case this patch targets), tocInstruction now stays undefined and paragraph TOC metadata is incomplete, so downstream TOC behavior can differ from the standard gallery path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
confirmed at runtime - child has the field codes, wrapper doesn't. covered in my review.
…SDTs Word stores TOC field codes on the child tableOfContents node, not the wrapper SDT. The new branch was passing the wrapper's tocInstruction, which is undefined for Custom TOC docs, silently dropping per-TOC options like '\o "1-3"'. Mirror processTocChildren's existing recursion (toc.ts:162-172) and prefer the child's instruction. Adds 3 unit tests for the new branch covering instruction preference, fallback to the wrapper, and the Array.isArray guard.
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
hey @chittolinag! loaded the customer's "Custom Table of Contents" doc against your branch and the TOC entries render now :)
i pushed a follow-up commit (56377a1) with the missing child-instruction handling and 3 unit tests for the new branch.
two things still open, both inline.
| blocks.push(block); | ||
| recordBlockKind?.(block.kind); | ||
| } | ||
| } else if (child.type === 'tableOfContents' && Array.isArray(child.content)) { |
There was a problem hiding this comment.
your first commit also added a child.type === 'table' branch with a test, and the "refactor" commit removed it. the linear ticket title is "drops tables" - was that handled somewhere else, or did it slip out of scope?
There was a problem hiding this comment.
@caio-pizzol I had misunderstood the ticket back then. My understanding is that the ticket description might point to the wrong direction because the issue is related to TOC, not Tables. The document linked in the ticket has a TOC that is not being rendered, not a table.
| processTocChildren( | ||
| child.content, | ||
| { | ||
| docPartGallery: docPartGallery ?? '', | ||
| docPartObjectId, | ||
| tocInstruction: childTocInstruction, | ||
| sdtMetadata: docPartSdtMetadata, | ||
| }, | ||
| { | ||
| nextBlockId, | ||
| positions, | ||
| bookmarks, | ||
| hyperlinkConfig, | ||
| enableComments, | ||
| trackedChangesConfig, | ||
| converters, | ||
| converterContext, | ||
| }, | ||
| { blocks, recordBlockKind }, | ||
| ); |
There was a problem hiding this comment.
git rebase origin/main hits a conflict here - #2872 added themeColors and sectionState to the existing branch's call. if the merge keeps both, this new branch silently drops them, so theme colors and section breaks won't fire on custom-TOC content. worth mirroring the existing branch's call shape when you resolve.
|
@artem-harbour mind doing a review on this one pls? |
|
@chittolina pls resolve merge conflicts |
6 participants
Issue
Documents with a TOC inside a
w:sdtwhosedocPartGalleryis not exactly"Table of Contents"(e.g."Custom Table of Contents") have the TOC dropped on render.The field preprocessor converts TOC field codes (
w:fldChar begin/end) into a singlesd:tableOfContentselement regardless of the gallery type. This means the PMdocumentPartObjectnode for any custom TOC gallery ends up with atableOfContentschild.handleDocumentPartObjectNodedispatches ondocPartGallery === 'Table of Contents'for theprocessTocChildrenpath. For any other gallery string it falls to an else-if loop that only handledparagraphchildren —tableOfContentschildren were silently dropped.Proposed solution
Add a
tableOfContentsbranch to the else-if loop indocument-part-object.tsthat forwards to the existingprocessTocChildrenhelper. No changes to the import handler or preprocessing pipeline.Side notes
This PR fixes the issue where the TOC is completely dropped and doesn't show up in the document. However, there are still rendering issues with the TOC. This other PR is open and fixes the rendering issue.