fix: render state diagram click tooltips#7751
Conversation
🦋 Changeset detectedLatest commit: 0ea9ba6 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 |
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@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 #7751 +/- ##
==========================================
- Coverage 3.27% 3.27% -0.01%
==========================================
Files 600 600
Lines 60657 60695 +38
Branches 916 916
==========================================
Hits 1985 1985
- Misses 58672 58710 +38
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 ↗︎
|
There was a problem hiding this comment.
Pull request overview
This PR adds state diagram click tooltip support by attaching Mermaid’s shared tooltip behavior to rendered state nodes and setting tooltip titles on clickable wrapped nodes.
Changes:
- Adds tooltip binding support to
StateDB. - Sets click directive tooltip titles on wrapped state nodes in the v3 unified renderer.
- Adds a regression test and patch changeset.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
packages/mermaid/src/diagrams/state/stateRenderer-v3-unified.ts |
Narrows click target selection and adds tooltip titles to wrapped nodes. |
packages/mermaid/src/diagrams/state/stateDb.ts |
Adds tooltip binding via bindFunctions. |
packages/mermaid/src/diagrams/state/stateRenderer-v3-unified.spec.js |
Adds regression coverage for clickable tooltip rendering. |
.changeset/state-tooltip-links.md |
Adds a patch release note. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Hi @puneetdixit200. Thanks for publishing this PR. Some feedback: [sisyphos-bot] What's working well 🎉 Idiomatic pattern. The funs/bindFunctions/setupToolTips triad is exactly how flowDb.ts and classDb.ts implement this. New contributors often miss this pattern; following 🎉 Binding in constructor. this.bindFunctions = this.bindFunctions.bind(this) is needed because mermaidAPI.ts:477 extracts the method as a property (bindFunctions: 🎉 clear() reinitializes funs. Each render gets a fresh tooltip setup. Missing this is a common bug with the funs pattern; the PR gets it right. 🎉 DOMPurify usage is correct. User-controlled tooltip text flows from stateId→linkInfo.tooltip→tooltipElem.html(DOMPurify.sanitize(title)). The sanitization happens at the 🎉 Hand-drawn mode coverage. The rough-node selector and it.each parametrized tests are a thoughtful addition in the follow-up commit. 🎉 Tests verified. Both parametrized tests pass against the PR's code (stateRenderer-v3-unified.spec.js, 2/2 ✓). Things to address 🟡 [important] — No Cypress integration test for the click directive behavior The change adds matchedElem.setAttribute('title', tooltip) to the renderer and wires up the full bindFunctions chain. The unit tests cover the tooltip lifecycle well, but cypress/integration/rendering/stateDiagram-v2.spec.js has no test for click behavior. A test like: it('renders clickable state nodes with title attribute', () => { would give regression coverage for the wrapping + title attribution. 🟢 [nit] — Redundant .text(title) before .html(...) in setupToolTips (stateDb.ts:663) tooltipElem The .style() positioning doesn't depend on .text() being called first — it can be chained directly on tooltipElem. The .text() call is a no-op (.html() overwrites it on the 💡 [suggestion] — setupToolTips is now copy-pasted across three DB classes stateDb.ts, flowDb.ts, and classDb.ts now each have nearly identical 30-line setupToolTips implementations. The only difference is the selector string ('g.node' vs 'g.node, Security No XSS concerns. The tooltip text is user-controlled content that flows through DOMPurify.sanitize() before being set as innerHTML. The title attribute on SVG elements does Summary: The fix is correctly implemented, follows established patterns, and the tests pass. The bindFunctions chain integrates cleanly with mermaidAPI.ts. A Cypress test |
Summary
mermaidTooltipbehavior.A: Google.Tests
vitest run packages/mermaid/src/diagrams/state/stateDiagram.spec.js packages/mermaid/src/diagrams/state/stateDiagram-v2.spec.js packages/mermaid/src/diagrams/state/stateDb.spec.js packages/mermaid/src/diagrams/state/stateRenderer-v3-unified.spec.js packages/mermaid/src/diagrams/state/parser/state-parser.spec.js packages/mermaid/src/diagrams/state/parser/state-style.spec.jscypress run --spec cypress/integration/rendering/state/stateDiagram-v2.spec.js --config video=falseprettier --checkon touched fileseslint --quieton touched filesCloses #6701