feat(math): implement m:phant phantom converter#2749
feat(math): implement m:phant phantom converter#2749caio-pizzol merged 4 commits intosuperdoc-dev:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 566fc42ea0
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
3e7e2c0 to
c6b8cc6
Compare
caio-pizzol
left a comment
There was a problem hiding this comment.
@Abdeltoto thanks for contributing this! the phantom converter is a nice addition and the code is clean — matches Word perfectly on 7 out of 10 test cases.
one thing to fix: when <m:show> is missing, content renders invisible but should be visible. this affects the most common phantom pattern (zeroing a dimension without explicitly setting show). left an inline comment with a one-line fix, comparison screenshots, and 10 Word-native test files.
…2608) Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Per ECMA-376 §22.1.2.96, when <m:show> is absent from <m:phantPr> the base content should be shown. The previous check only handled bare and truthy-val variants, so a missing element was treated as hidden — breaking the most common phantom pattern (zeroing a dimension without hiding content). Adds unit coverage for the explicit-hide case and zeroed-dimension-only case, plus a behavior test loading a 12-case fixture that walks every m:phantPr configuration in the spec (show absent/bare/val=0/1/true/false, each zero flag alone and combined, m:transp passthrough).
c6b8cc6 to
6b0cb18
Compare
|
hey @Abdeltoto — didn't want to block this any longer so I pushed the last fix myself, hope that's cool. your two fixes from round 1 look great. the one still open was the "no while I was in there I went through every phantom setup the spec describes (12 in total) and they all render right. added a few more unit tests for the gaps, plus a browser test that loads all 12 in one doc. the doc is in our visual test set too, so we'll catch any future breakage. also rebased on main — a bunch of other math converters landed since you opened this, so there were some conflicts to untangle. thanks for picking this up, the code is clean. merging once CI's green. |
|
🎉 This PR is included in @superdoc-dev/react v1.2.0-next.8 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.3.0-next.11 |
|
🎉 This PR is included in template-builder v1.5.0-next.11 The release is available on GitHub release |
|
🎉 This PR is included in esign v2.3.0-next.11 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.26.0-next.11 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.7.0-next.12 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.6.0-next.9 |
|
🎉 This PR is included in superdoc-cli v0.7.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.27.0 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/react v1.2.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.6.0 |
Closes #2608
Summary
m:phantOMML-to-MathML converter (invisible spacing placeholder)<mphantom>; partial dimension control maps to<mpadded>with zeroed width/height/depthm:show,m:zeroWid,m:zeroAsc, andm:zeroDescproperty flagsMATH_OBJECT_REGISTRYSpec reference
ECMA-376 Section 22.1.2.81
Test plan
vitest runpasses for omml-to-mathml.test.ts