Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: inconsistent MathML rendering & erroneous <br />s being added #5483

Merged

Conversation

NicolasNewman
Copy link
Contributor

📑 Summary

  1. Resolves rendering differences caused by different default OS fonts & MathML implementations within browsers (See Math is occasionally rendered incorrectly (e.g., fraction inside sqrt) #5435)
  2. Fixed some equations being cut-off in flowcharts

Resolves #5435

📏 Design Decisions

1)

As mentioned in the PR, differences between the default fonts for each OS was not taken into account when I implemented the useLegacyMathML config option. As a result, there is currently no way to force Mermaid to fallback to using KaTeX's stylesheets if they are provided. To allow this to be done, I added an additional config option forceLegacyMathML to explicitly tell Mermaid to default to CSS-based rendering.

Ideally this should be changed to a single config option that takes one of 3 values but I don't know what the protocol for deprecating config options are.

2)

While fixing the above I noticed the spacing for some equations in flowchart vertices were incorrect. This was due to an erroneous <br /> tags being added by the regex replacement vertexText.replace(/\\n|\n/g, '<br />') being triggered on unintended line breaks in the template string in renderKatex().

Before:
image
image

After:
image
image

📋 Tasks

Make sure you

Copy link

netlify bot commented Apr 22, 2024

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit 22bd262
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/6638eedcdfb9d20008a8e64c
😎 Deploy Preview https://deploy-preview-5483--mermaid-js.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@NicolasNewman NicolasNewman changed the title Bug Fix: fixed inconsistent MathML rendering & erroneous <br />s being added fix: fixed inconsistent MathML rendering & erroneous <br />s being added Apr 22, 2024
@NicolasNewman NicolasNewman changed the title fix: fixed inconsistent MathML rendering & erroneous <br />s being added fix: inconsistent MathML rendering & erroneous <br />s being added Apr 22, 2024
Copy link

codecov bot commented Apr 22, 2024

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 5.72%. Comparing base (e68125e) to head (22bd262).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           develop   #5483      +/-   ##
==========================================
- Coverage     5.73%   5.72%   -0.01%     
==========================================
  Files          277     278       +1     
  Lines        41999   42012      +13     
  Branches       515     490      -25     
==========================================
  Hits          2407    2407              
- Misses       39592   39605      +13     
Flag Coverage Δ
unit 5.72% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
packages/mermaid/src/diagrams/common/common.ts 53.01% <0.00%> (-0.28%) ⬇️

... and 1 file with indirect coverage changes

@NicolasNewman NicolasNewman marked this pull request as ready for review May 6, 2024 14:52
@sidharthv96 sidharthv96 merged commit f5e1df0 into mermaid-js:develop May 8, 2024
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Math is occasionally rendered incorrectly (e.g., fraction inside sqrt)
2 participants