feat(xychart): add per-point text labels for line charts#7550
feat(xychart): add per-point text labels for line charts#7550DominicBurkart wants to merge 4 commits intomermaid-js:developfrom
Conversation
Allow annotating individual data points on xychart line plots with custom text labels. The syntax extends the existing data array format: line [540 "PaLM", 65 "LLaMA", 34 "Llama 2", 7 "Mistral"] Each number can optionally be followed by a quoted string label. Labels are rendered above (vertical) or to the right (horizontal) of the data point, using the line's stroke color. Backward-compatible: existing syntax without labels works unchanged. Addresses mermaid-js#5326. https://claude.ai/code/session_01CgvREhkQ3tVYCagq1mHmjV
Document the new per-point text label syntax for line charts, including syntax examples and rendering behavior notes. https://claude.ai/code/session_01WErEVQAYaNmC3rpbevFwMg
…labels - Add minor changeset for the new point labels feature - Enhance xyChart.md docs with mermaid-example block for mixed labels and a note about fixed font size - Add 4 Cypress e2e snapshot tests covering: all-labeled, mixed-labels, horizontal orientation, and multi-line scenarios - Add MMLU to cspell dictionary https://claude.ai/code/session_011g3A6y2kdJnZh8YB4uJaxD
🦋 Changeset detectedLatest commit: 29255d2 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 #7550 +/- ##
==========================================
- Coverage 3.34% 3.34% -0.01%
==========================================
Files 524 525 +1
Lines 55392 55450 +58
Branches 795 795
==========================================
Hits 1853 1853
- Misses 53539 53597 +58
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 ↗︎
|
knsv
left a comment
knsv
left a comment
There was a problem hiding this comment.
[sisyphus-bot]
Thanks for this contribution, @DominicBurkart! Really nice feature — per-point labels on line charts is something that comes up a lot in real-world use cases like benchmark visualizations and trend annotations. The syntax extension feels natural and the backward compatibility is well preserved.
What's working well
🎉 [praise] Clean, intuitive syntax design. line [540 "PaLM", 65 "LLaMA-65B"] reads naturally and extends the existing data array format without ambiguity. Mixing labeled and unlabeled points is a great ergonomic choice.
🎉 [praise] Excellent test coverage — both parser unit tests (6 new test cases covering labels, mixed labels, spaces in labels, decimal values) and 4 E2E visual regression tests including horizontal orientation and multi-line scenarios. This is exactly what we look for.
🎉 [praise] Documentation uses MERMAID_RELEASE_VERSION placeholder correctly, changeset is properly configured as minor with feat: prefix, and source docs are in the right location (packages/mermaid/src/docs/).
Things to address
🟡 [important] — Point labels bypass textSanitizer()
In xychartDb.ts, all other user-provided text goes through textSanitizer() (which calls sanitizeText from diagrams/common/common.js):
setXAxisTitle→textSanitizer(title.text)(line 89)setXAxisBand→textSanitizer(c.text)for each category (line 99)setYAxisTitle→textSanitizer(title.text)(line 104)
But the new setLineData passes labels through without sanitization:
const labels = data.map((d) => d.label); // no textSanitizer() callThe rendering pipeline uses D3's .text() (safe) and DOMPurify sanitizes the final SVG, so this isn't an active XSS vector — but it breaks the defense-in-depth pattern established by the codebase. It would be great to add sanitization for consistency:
const labels = data.map((d) => (d.label ? textSanitizer(d.label) : ''));🟡 [important] — Generated docs/syntax/xyChart.md appears to be manually edited
The top-level docs/ directory is auto-generated from packages/mermaid/src/docs/. The PR modifies both:
packages/mermaid/src/docs/syntax/xyChart.md(source — correct ✓)docs/syntax/xyChart.md(generated — should not be manually edited)
The generated file has duplicated mermaid-example + mermaid block pairs and different formatting (> **Note** vs the source's ```note), confirming it's the build output. This file should be removed from the PR and regenerated via the docs build instead. Otherwise the next docs rebuild will overwrite these changes or create a confusing diff.
🟢 [nit] — Bar data labels are parsed but silently discarded
The JISON grammar change means bar [10 "A", 20 "B"] now parses successfully and produces {value, label} objects, but setBarData only extracts values — labels are silently dropped. This is fine for the scope of this PR (feature is for line charts), but worth a brief note in the docs that labels are currently only supported on line plots, so users don't wonder why bar labels have no effect.
🟢 [nit] — Hardcoded font size and label offset
In linePlot.ts:48-49:
const labelOffset = 10;
const fontSize = 12;These magic numbers work for now, but other text in xychart (axis labels, title) uses configurable sizes from XYChartConfig and theme variables. Not blocking, but worth considering making these configurable in a follow-up — especially fontSize, which won't scale well if users customize other chart text sizes.
Security
Reviewed the full data flow: JISON parser → ParsedDataPoint.label → xychartDb.setLineData → LinePlotData.pointLabels → linePlot.ts → TextElem → D3 .text() rendering → DOMPurify final sanitization.
No XSS or injection vulnerabilities identified. The rendering uses D3's safe .text() method (sets textContent, not innerHTML), and DOMPurify sanitizes the final SVG output. The missing textSanitizer() call (noted above) is a defense-in-depth concern, not an active vulnerability.
Looking forward to getting this landed — it's a well-scoped, well-tested feature. The two 🟡 items should be quick to address. Let me know if you have any questions!
3 participants
📑 Summary
Adds syntax for per-point text labels on xychart line plots:
From the deploy preview:
Resolves #7549
Note: I added stacked PRs to elaborate the label placement, namely:
📷 QA Screenshots
All test cases rendered via Playwright (Chromium) with the default theme. (full QA comment with details)
1. Basic Labeled Line —
line [10 "A", 20 "B", 30 "C"]2. Mixed Labels —
line [10 "Start", 20, 30 "End"]3. Backward Compatibility —
line [10, 20, 30](no labels)4. Horizontal Orientation — labeled line with
xychart horizontal5. Multiple Lines — one labeled, one unlabeled
6. Real-World Use Case — MMLU Model Performance
Test Summary
Automated tests: 58/58 parser unit tests pass, 4008/4008 full suite tests pass, 0 regressions.
📋 Tasks
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:.