Skip to content

Fixes #3745#3746

Open
tom-material wants to merge 2 commits into
superdoc-dev:mainfrom
tom-material:tom/nested-block-sdts
Open

Fixes #3745#3746
tom-material wants to merge 2 commits into
superdoc-dev:mainfrom
tom-material:tom/nested-block-sdts

Conversation

@tom-material

Copy link
Copy Markdown

Summary

  • Preserve direct child structuredContentBlock nodes during block SDT
    layout conversion
  • Add a regression test for nested block SDT content and metadata

Test plan

  • pnpm exec vitest run --config packages/layout-engine/tests/ vitest.config.mjs packages/layout-engine/tests/src/sdt-metadata.test.ts
  • pnpm exec prettier --check packages/super-editor/src/editors/v1/core/ layout-adapter/sdt/structured-content-block.ts packages/layout-engine/tests/ src/sdt-metadata.test.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1f664e5a60

ℹ️ 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".

// paragraphs without advancing currentParagraphIndex, since
// findParagraphsWithSectPr does not recurse structuredContentBlock.
if (child.type === 'structuredContentBlock') {
handleStructuredContentBlockNode(child, context);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve outer SDT metadata when recursing

When a block SDT directly contains another structuredContentBlock (especially outer SDT -> inner SDT -> paragraph with no direct paragraph/table children on the outer), this recursive call converts the inner subtree with only the inner node's metadata, so no emitted FlowBlock carries the outer structuredContentMetadata. The painter derives block-SDT grouping/chrome from attrs.sdt, so the outer content control effectively disappears from layout/rendering for this valid nested shape; pass the inherited metadata through or record it separately instead of dropping it at the recursion boundary.

Useful? React with 👍 / 👎.

@caio-pizzol

Copy link
Copy Markdown
Contributor

Thanks for this, Tom. You found the right failing path: a block w:sdt nested directly inside another block w:sdt was skipped during layout conversion, so valid content was dropped.

We checked it against the OOXML schema and a Word-authored fixture. Nested block SDTs are valid, and the full fix needs to preserve the outer and inner controls as a hierarchy, not just recurse into the inner one. That crosses more of the rendering stack than this PR touches, so we're going to take it over rather than ask you to keep expanding it.

We're tracking the full fix in #3752 and will open the first follow-up PR linked there, then close this once that's up. We'll keep coverage for the repro shape and credit your work. Thanks again for the clear repro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants