fix(plugin-formula): 修复公式绝对值分隔符长度异常#882
Conversation
🦋 Changeset detectedLatest commit: 038f733 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughThis PR fixes inconsistent LaTeX formula rendering by refactoring the formula card custom element to lazily initialize its span and switch KaTeX output to ChangesKaTeX Formula Rendering Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/plugin-formula/__tests__/register-custom-elem.test.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. packages/plugin-formula/src/register-custom-elem/index.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 038f733d06
ℹ️ 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".
| const span = document.createElement('span') | ||
|
|
||
| span.style.display = 'inline-block' | ||
| this.appendChild(span) |
There was a problem hiding this comment.
Appending the KaTeX render container into the custom element’s light DOM makes setFragmentData/getPlainText in core traverse KaTeX’s internal HTML+MathML nodes when users copy content, so copying a formula now yields duplicated/garbled plain text (hidden MathML annotation plus visual HTML text) in external paste targets. This regression appears when selecting formula nodes and using copy/cut; the previous Shadow DOM implementation avoided leaking these internal nodes into clipboard extraction.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/plugin-formula/__tests__/register-custom-elem.test.ts`:
- Around line 3-18: Add round-trip serialization tests for the plugin-formula
style change: extend the existing test for the custom element 'w-e-formula-card'
to exercise render -> toHtml -> parseHtml and setHtml(getHtml()) /
toHtml(parseHtml(html)) paths across both inline and class style modes and for
default and custom configuration values; specifically, invoke the module's
render (or equivalent render helper), call toHtml() on the editor state,
parseHtml(...) to rehydrate DOM, and assert that parsed output/rendered DOM
still contains '.katex', '.katex-html' and '.delimsizing.mult' for each
combination (inline/class × default/custom) and for both round-trip flows.
Ensure you reuse the existing element creation (elem =
document.createElement('w-e-formula-card')) and attribute handling
(setAttribute('data-value', ...)) while adding assertions after
setHtml(getHtml()) and toHtml(parseHtml(html)) to fully cover the required
matrix.
- Around line 11-17: The test currently calls elem.remove() after assertions
which will be skipped if an assertion throws; wrap the assertions (the block
that checks elem.shadowRoot and the various elem.querySelector(...)
expectations) in a try/finally and move elem.remove() into the finally so
cleanup always runs; locate the test that creates the element (variable elem) in
packages/plugin-formula/__tests__/register-custom-elem.test.ts and ensure
elem.remove() is executed in the finally block surrounding those expectations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e15596ba-8936-4a1a-a585-1530e8eea88b
📒 Files selected for processing (5)
.changeset/four-owls-type.mdpackages/plugin-formula/README-en.mdpackages/plugin-formula/README.mdpackages/plugin-formula/__tests__/register-custom-elem.test.tspackages/plugin-formula/src/register-custom-elem/index.ts
| describe('plugin-formula custom element', () => { | ||
| it('renders htmlAndMathml output and keeps stretchy absolute delimiters', () => { | ||
| const elem = document.createElement('w-e-formula-card') | ||
| const formula = String.raw`\left|-2\frac{4}{7}\right|` | ||
|
|
||
| elem.setAttribute('data-value', formula) | ||
| document.body.appendChild(elem) | ||
|
|
||
| expect(elem.shadowRoot).toBeNull() | ||
| expect(elem.querySelector('.katex')).not.toBeNull() | ||
| expect(elem.querySelector('.katex-html')).not.toBeNull() | ||
| expect(elem.querySelector('.katex-html .delimsizing.mult')).not.toBeNull() | ||
|
|
||
| elem.remove() | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Add the required round-trip serialization coverage for this style-related change.
This adds a useful DOM regression test, but it still misses the required round-trip matrix (render/toHtml/parseHtml, inline/class style modes, default/custom config, and setHtml(getHtml()) / toHtml(parseHtml(html))) for serialization/style changes.
As per coding guidelines, "**/*.test.{ts,tsx}: Test round-trip export/import for inline and class style modes, default and custom configuration values, across render/toHtml/parseHtml paths, and with setHtml(getHtml())/toHtml(parseHtml(html)) when modifying serialization or styles".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/plugin-formula/__tests__/register-custom-elem.test.ts` around lines
3 - 18, Add round-trip serialization tests for the plugin-formula style change:
extend the existing test for the custom element 'w-e-formula-card' to exercise
render -> toHtml -> parseHtml and setHtml(getHtml()) / toHtml(parseHtml(html))
paths across both inline and class style modes and for default and custom
configuration values; specifically, invoke the module's render (or equivalent
render helper), call toHtml() on the editor state, parseHtml(...) to rehydrate
DOM, and assert that parsed output/rendered DOM still contains '.katex',
'.katex-html' and '.delimsizing.mult' for each combination (inline/class ×
default/custom) and for both round-trip flows. Ensure you reuse the existing
element creation (elem = document.createElement('w-e-formula-card')) and
attribute handling (setAttribute('data-value', ...)) while adding assertions
after setHtml(getHtml()) and toHtml(parseHtml(html)) to fully cover the required
matrix.
| expect(elem.shadowRoot).toBeNull() | ||
| expect(elem.querySelector('.katex')).not.toBeNull() | ||
| expect(elem.querySelector('.katex-html')).not.toBeNull() | ||
| expect(elem.querySelector('.katex-html .delimsizing.mult')).not.toBeNull() | ||
|
|
||
| elem.remove() | ||
| }) |
There was a problem hiding this comment.
Ensure cleanup runs even when an assertion fails.
elem.remove() should be in finally to avoid leaking DOM state on failed assertions.
Suggested change
it('renders htmlAndMathml output and keeps stretchy absolute delimiters', () => {
const elem = document.createElement('w-e-formula-card')
const formula = String.raw`\left|-2\frac{4}{7}\right|`
- elem.setAttribute('data-value', formula)
- document.body.appendChild(elem)
-
- expect(elem.shadowRoot).toBeNull()
- expect(elem.querySelector('.katex')).not.toBeNull()
- expect(elem.querySelector('.katex-html')).not.toBeNull()
- expect(elem.querySelector('.katex-html .delimsizing.mult')).not.toBeNull()
-
- elem.remove()
+ try {
+ elem.setAttribute('data-value', formula)
+ document.body.appendChild(elem)
+
+ expect(elem.shadowRoot).toBeNull()
+ expect(elem.querySelector('.katex')).not.toBeNull()
+ expect(elem.querySelector('.katex-html')).not.toBeNull()
+ expect(elem.querySelector('.katex-html .delimsizing.mult')).not.toBeNull()
+ } finally {
+ elem.remove()
+ }
})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/plugin-formula/__tests__/register-custom-elem.test.ts` around lines
11 - 17, The test currently calls elem.remove() after assertions which will be
skipped if an assertion throws; wrap the assertions (the block that checks
elem.shadowRoot and the various elem.querySelector(...) expectations) in a
try/finally and move elem.remove() into the finally so cleanup always runs;
locate the test that creates the element (variable elem) in
packages/plugin-formula/__tests__/register-custom-elem.test.ts and ensure
elem.remove() is executed in the finally block surrounding those expectations.
1 participant
关联 issue
变更说明
plugin-formula自定义元素渲染从output: 'mathml'调整为output: 'htmlAndMathml'，避免绝对值分隔符在编辑态回显过短\left|...\right|场景并校验存在可拉伸分隔符节点验证
pnpm test packages/plugin-formula/__tests__/module.test.ts packages/plugin-formula/__tests__/register-custom-elem.test.tspnpm --filter @wangeditor-next/plugin-formula buildpnpm typecheck说明
apps/demo-react与pnpm-lock.yaml临时改动未包含在本 PR 内。Summary by CodeRabbit
Release Notes
Bug Fixes
Documentation
katex.min.css) before plugin registration.