Math: implement m:rad radical/sqrt converter#2730
Math: implement m:rad radical/sqrt converter#2730caio-pizzol merged 8 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: d59d1d4272
ℹ️ 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".
caio-pizzol
left a comment
There was a problem hiding this comment.
@baristaGeek nice work, clean implementation that follows the existing converter patterns. one thing to fix: when m:deg is missing entirely, this should render as a square root. also a stale comment in the behavior tests to update now that radical is implemented. left a few inline comments.
one more thing: tests/behavior/tests/importing/math-equations.spec.ts line 114 still lists radical as "unimplemented" — should be updated now.
|
btw — for future OMML work, we have an ECMA-376 spec MCP server you can use from Claude Code to look up sections directly: |
…rs/radical.ts Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com>
…rs/radical.ts Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com>
Thanks @caio-pizzol your inline comments address all these issues. I removed the radicals from the list of examples in line 114 |
caio-pizzol
left a comment
There was a problem hiding this comment.
@baristaGeek all feedback addressed — the !deg fix, spec section number, both new tests, and the behavior test comment update. lgtm, approving.
Adds a Playwright test asserting that <msqrt> elements render in the DOM when loading a document with radical equations (degHide=true).
|
@baristaGeek two things we're adding:
|
|
🎉 This PR is included in vscode-ext v1.1.0-next.73 |
|
🎉 This PR is included in @superdoc-dev/react v1.0.0-next.27 The release is available on GitHub release |
|
🎉 This PR is included in esign v2.2.0-next.31 The release is available on GitHub release |
|
🎉 This PR is included in template-builder v1.3.0-next.33 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.24.0-next.70 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.5.0-next.71 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.3.0-next.72 |
|
🎉 This PR is included in superdoc-sdk v1.4.0 |
|
🎉 This PR is included in superdoc v1.25.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.6.0 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.3.0-next.1 |
|
🎉 This PR is included in template-builder v1.5.0-next.1 The release is available on GitHub release |
|
🎉 This PR is included in esign v2.3.0-next.1 The release is available on GitHub release |
Implemented this change using existing coding patterns and tested with the doc provided in the issue. Updated tests, added new ones for this converter, committed with the linter's guidelines and made sure the 857 tests passed.