feat(math): implement m:sSup superscript converter (SD-2372)#2634
Conversation
![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: 59a0b942ed
ℹ️ 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".
packages/layout-engine/painters/dom/src/features/math/converters/superscript.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
@gpardhivvarma nice work — clean implementation, spec reference is correct.
simple cases like x² work great. compound expressions like (x+1)² don't render right — the parts end up flat instead of grouped. same thing happens in the subscript PR and fraction.ts, so we'll fix them all together in a follow-up.
nested superscripts, fractions as base, and property handling all work. tests look good.
i made a .docx with 10 variations:
sd-2372-superscript-edge-cases.docx
Word:
SuperDoc:
cases 2, 3, and 7 break (the ones with multiple parts in the base or exponent). the rest render correctly.
packages/layout-engine/painters/dom/src/features/math/converters/superscript.ts
Outdated
Show resolved
Hide resolved
Add OMML m:sSup → MathML <msup> converter following the fraction.ts pattern. Also move m:f from the "not yet implemented" registry section to "implemented" where it belongs. Closes superdoc-dev#2595
Converters for msup, mfrac were appending raw DocumentFragments directly, causing multi-token expressions (e.g. x+1) to produce too many direct children. MathML script/fraction elements require exactly 2 children. Wrap each operand in <mrow> to group them, matching the pattern already used by bar.ts.
gpardhivvarma
force-pushed
the
feat/math-ssup-superscript-converter
branch
from
3faf0d9 to
387ee26
Compare
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
@gpardhivvarma mrow wrapping is in, fraction fix is a nice bonus. all feedback from last round is addressed.
tested with a 15-case edge case doc (simple, multi-run, nested, cross-converter with fractions and bars, triple nesting, ctrlPr, empty props, sibling superscripts) — all pass.
approving.
|
🎉 This PR is included in vscode-ext v1.1.0-next.54 |
|
🎉 This PR is included in template-builder v1.3.0-next.11 The release is available on GitHub release |
|
🎉 This PR is included in esign v2.2.0-next.12 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.24.0-next.51 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.5.0-next.51 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.3.0-next.52 |
|
🎉 This PR is included in @superdoc-dev/react v1.0.0-next.11 The release is available on GitHub release |
2 participants
Summary
m:sSup→ MathML<msup>converter following thefraction.tspatternm:ffrom the "not yet implemented" registry section to "implemented" where it belongsm:supgraceful degradationTest plan
pnpm --filter @superdoc/painter-dom test—omml-to-mathml.test.tspasses (17 tests)Closes #2595