Added save to png button, improved zooming, force stand color palette#7750
Added save to png button, improved zooming, force stand color palette#7750romangrigorii wants to merge 3 commits into
Conversation
Improved zooming experience Forced correct appliaction of standard mermaid color pallete
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: a9fb703 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 |
@mermaid-js/examples
mermaid
@mermaid-js/layout-elk
@mermaid-js/layout-tidy-tree
@mermaid-js/mermaid-zenuml
@mermaid-js/parser
@mermaid-js/tiny
commit: |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #7750 +/- ##
==========================================
- Coverage 3.27% 3.26% -0.01%
==========================================
Files 600 600
Lines 60657 60724 +67
Branches 916 916
==========================================
Hits 1985 1985
- Misses 58672 58739 +67
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
ashishjain0512
left a comment
ashishjain0512
left a comment
There was a problem hiding this comment.
Thanks @romangrigorii — this is a nice quality-of-life upgrade for the local editor. The three threads of the PR (PNG export, refactored pan/zoom with proper listener cleanup, removing the forced dark-color overrides) are coherent and well-scoped to packages/mermaid-local-editor/static/. No impact on the published mermaid library, no changeset needed.
What's working well
🎉 AbortController plumbing is the right way to do this. renderer.js:14-19 aborts the previous render's listeners before attaching new ones, then passes { signal } to every addEventListener call. That replaces the previous doc.onwheel = …; doc.onmousedown = …; … style, which was leak-prone (each new render reassigned the handlers on the same iframe document, and the cleanup at line 78 of the old version was brittle and not paired with each addition). The new shape is the modern, correct pattern.
🎉 Cursor-anchored multiplicative zoom at renderer.js:120-138 is a real UX win — factor = e.deltaY < 0 ? 1.1 : 1/1.1, then panX/Y adjusted by ratio = scale/oldScale so the point under the cursor stays put. Previously the wheel was a fixed-step additive state.scale += e.deltaY * -0.0015, which "zooms toward the origin" and feels disorienting. This is the standard approach used by mapping libraries; nice to see it here.
🎉 Overlay-on-top-of-iframe with iframe.style.cssText = '…pointer-events:none…' and a separate overlay div absolutely positioned over it (renderer.js:101-110) is the cleanest way to handle pan/zoom over sandboxed content. The previous approach of attaching doc.onwheel etc. inside the iframe's document had to deal with cross-document coordinate translation; this hoists the event handling into the parent's coordinate space, which makes the zoom math cleaner.
🎉 The color-override cleanup matches the stated intent ("force standard color palette"). Removing text { fill: #e6e6e6 !important; }, the white-bold selected-text rule, and the explicit fill on hover (lines previously at renderer.js:88-119) means the user's chosen mermaid theme actually drives the visible colors. Combined with the theme: 'default' change in config.js:8, the editor now displays what the user would see when they embed their diagram elsewhere, which is much more useful than a forced dark palette.
🎉 PNG export builds a clean 2× canvas with white background fill (ui.js:96-110) — the ctx.scale(scale, scale) + fillRect(0, 0, width, height) in scaled coordinates does the right thing, and URL.revokeObjectURL(url) is called after drawImage to free the Blob URL.
A few non-blocking notes
🟢 [nit] The PR description still has template placeholders. Resolves #<your issue id here>, empty "Design Decisions" section, and the task checkboxes are unchecked. Mermaid's PR template is mostly a self-checklist for contributors, but filling in 1-2 lines about why the theme is changing default would help future-you trace this back if anyone asks "why is my editor white now?". Either fill it in or drop the placeholder lines — your call.
🟢 [nit] PNG export width fallback can underflow on percentage widths at ui.js:80. parseFloat(svg.getAttribute('width')) returns 100 for a width="100%" attribute (rather than NaN), which means the || chain skips the 800 fallback and you'd get a 100×something PNG. Mermaid's SVGs almost always set a viewBox so vb.width carries the day, but if a diagram ever omits viewBox while keeping width="100%", the export silently produces a tiny PNG. A defensive Number.isFinite(…) && !svg.getAttribute('width')?.endsWith('%') would harden it.
🟢 [nit] PNG export has no img.onerror and no URL revoke on failure at ui.js:88-110. If the SVG can't be loaded as an image for some reason (rare, but possible with diagrams containing external <image href> references the browser blocks), the download just silently never happens and the Blob URL leaks until the page reloads. A two-line img.onerror = () => URL.revokeObjectURL(url) is enough.
🟢 [nit] mouseup / mousemove on document (renderer.js:148-176) won't fire if the user releases the mouse button outside the browser window, which can leave isPanning === true and the cursor stuck as grabbing. Switching to pointerup (or adding a mouseleave on window) handles that, but this is an edge case — most users won't notice.
💡 [suggestion] The theme: 'default' change is a visible behavior shift for everyone using the local editor — anyone who liked the old dark theme will now open a white page. Worth a one-line callout in the PR body, even just "switched default theme from dark to default so user-chosen %%{init: {'theme': '…'}}%% directives are respected".
Verification I ran
- Read the full diff across all 5 files (the fetch script only tiered 2, but I pulled the others via
gh pr diffand walked through them). - Confirmed the change is scoped to
packages/mermaid-local-editor/static/— that package isn't imported by the publishedmermaidlibrary (zero grep hits inpackages/mermaid/src/) and is in the e2e-ignorable list per the recently-merged scope-detect work, so no test coverage impact on the core. - Spawned the XSS sub-agent on the PNG export path → no new sinks.
XMLSerializer → Blob → Image → drawImage → toDataURLis a same-origin canvas pipeline and the source SVG was already DOMPurify-sanitized upstream. The pre-existingADD_TAGS: ['foreignObject']is confirmed scoped to this package only (does not leak into published code), andiframe.sandbox = 'allow-same-origin'(withoutallow-scripts) plus the expliciton*-attribute stripping is intact.
Severity tally: 🔴 0 / 🟡 0 / 🟢 4 / 💡 1 / 🎉 5
Approving. Thanks for the polish work on the editor!
|
Quick follow-up to my own review — I was wrong about "no changeset needed." When @mermaid-js/mermaid-local-editor was first added in #7626, it shipped with a changeset (`.changeset/crazy-jobs-see.md` → `'@mermaid-js/mermaid-local-editor': patch`), so the package is tracked by changesets. The release workflow in `.github/workflows/release.yml` runs `changesets/action` on master and the package isn't in the `ignore` list of `.changeset/config.json`, so version bumps are expected even though `"private": true` keeps it off npm. Could you add a changeset for this one? A single `.changeset/something-something.md` like: ```md'@mermaid-js/mermaid-local-editor': patchfeat: add PNG export, cursor-anchored zoom, AbortController-based listener cleanup, and switch to default theme so user-chosen themes are respected would round it out. `pnpm changeset` will generate the filename for you. The CI doesn't enforce this at PR time (the changeset action only runs on master), so it's easy to miss — sorry for not flagging it the first time around. My approval stands. |
…vements Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
kedar49
left a comment
kedar49
left a comment
There was a problem hiding this comment.
In the PNG export flow, URL.revokeObjectURL(url) only happens insideimg.onload.Consider also handling img.onerror to avoid leaking object URLs on failed image loads.
3 participants
Improved zooming experience
Forced correct appliaction of standard mermaid color palette
📑 Summary
Brief description about the content of your PR.
Resolves #
📏 Design Decisions
Describe the way your implementation works or what design decisions you made if applicable.
📋 Tasks
Make sure you
MERMAID_RELEASE_VERSIONis used for all new features.pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.