Repository navigation
feat: Add Venn diagram - #5932
Conversation
🦋 Changeset detectedLatest commit: 37582dc 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 site 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 #5932 +/- ##
========================================
Coverage 3.58% 3.58%
========================================
Files 475 483 +8
Lines 47605 48132 +527
Branches 741 758 +17
========================================
+ Hits 1706 1726 +20
- Misses 45899 46406 +507
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 ↗︎
|
|
This draft has broken CI and conflicts, how can we move It forward? |
|
I am still working to add text nodes in each circle. |
|
+1 any timeline on this one? :) |
|
wen |
By forking @exoego 's PR and fixing the conflicts + broken CI. Hopefully adding the text in each node as he mentioned. |
|
It's a shame that this stalls, looks like a solid implementation of a very needed feature |
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
948eb90 to
91921c1
Compare
Yep I posted yesterday mentioning this lovely PR and the issue’s many upvotes. 🤞 |
knsv
left a comment
There was a problem hiding this comment.
Thank you for this contribution — and I want to start with an apology. This PR has been waiting too long for a proper review, and that's on us. I'm sorry for the delay. We're committed to seeing this through now and getting it across the finish line together with the Ishikawa PR.
The diagram looks fantastic. It's clear this is something the community has been longing for, and your implementation captures it beautifully. The syntax
is clean and the support for 2, 3, and 4 sets with intersections is really well done.
Here's my review:
What's great:
- The visual output looks wonderful — the intersections are clear and readable
- The syntax is intuitive and well-designed
- Supporting multiple set counts (2–4) with proper intersections is exactly right
Things to address:
- Theme support: I notice themes aren't supported yet. You had this working in the Ishikawa diagram — it would be great to bring the same approach here. In particular, we have support for color scales in the base theme (used in journey diagrams for sequential section colors). The fillTypes pattern in journey diagrams could work well for the set colors. I'd also suggest looking at the pie chart color logic - it handles slices and calculates visible text color against contrasting backgrounds, which seems directly relevant for the Venn set fills and labels.
- Sizing with useMaxWidth: The diagram currently renders much larger than other diagram types when useMaxWidth is enabled — it dwarfs a pie chart in comparison, for example. This can be problematic when embedding in documentation sites alongside other diagrams. The sizing should be consistent with how other diagrams handle useMaxWidth.
- Visual regression tests: Please add visual snapshot tests - see cypress/rendering/xyz.spec.js for examples. It's really easy: just add a diagram per test. The image snapshot comparison catches any future regressions automatically. This is one of our most important quality safeguards across all diagram types.
- Handdrawn/rough mode: Would be nice to support, but I'm not sure if @upsetjs/venn.js supports it. If it doesn't, that's understandable - but we should document that handdrawn look is not available for this diagram type.
- Styling approach: The syntax is great, but for someone familiar with Mermaid conventions, it might feel more natural to set color options using style statements rather than the text-based approach. Worth considering whether the styling can align with Mermaid's existing patterns.
Again - great work and sorry for the wait. Let's get both of these diagrams in.
|
@knsv Thanks for the detailed review 🙇
It's good for me to squash commits when merging. |
|
Nice progress @exoego — 4 of 5 items from my previous review are addressed, and the hand-drawn mode and style statement work came out really well. What's working well: Schema fix — I've pushed this directly:
One thing to address: Nits (non-blocking): vennDiagram.ts imports styles as flowStyles — copy-paste leftover, should be vennStyles. Approving — with the schema fix already pushed, this is good to go. Great work on the test coverage and the rough.js implementation. 🎉 Again, great work. Ping me if you want to work more with Mermaid. We have a spot open on the core team for you! |
|
I'll work on 7415 and other nits in subsequent PRs so this PR itself gets merged soon🙇 |
59bab4c
📑 Summary
Resolves #2583
📏 Design Decisions
This PR leverages upsetjs/venn.js, which is a maintained d3 plugin for Venn/Euler diagram.
My syntax proposal is:
setdefines a single set (circle)union A,Bdefines an overlap between multiple setstextdefines a text node which belongs to one ofsetorunionblock,mindmapandtreemap.Text nodes can wrap when very long, while set node can not wrap (I've tried to implement auto-wrap for set, but no luck)
📋 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:.Screenshot