Skip to content

fix(architecture): floor service label width at 90px to prevent mid-word wrap#7706

Open
txmxthy wants to merge 5 commits into
mermaid-js:developfrom
txmxthy:fix/architecture-label-width-floor
Open

fix(architecture): floor service label width at 90px to prevent mid-word wrap#7706
txmxthy wants to merge 5 commits into
mermaid-js:developfrom
txmxthy:fix/architecture-label-width-floor

Conversation

@txmxthy
Copy link
Copy Markdown
Contributor

@txmxthy txmxthy commented May 5, 2026

📑 Summary

Floors the architecture-beta service label width at 90px so long single-word labels (e.g. ClickHouse) no longer wrap mid-character at small iconSize values. Multi-word labels still wrap at word boundaries.

Symptom (at iconSize: 50):

Before After
ClickHouse rendered as ClickHous / e (character-boundary wrap) ClickHouse renders on a single line

This is the visible part of a complaint about uneven-looking rows in architecture-beta diagrams at small icon sizes — the icons are actually pixel-aligned, but a mid-character wrap on one node makes the whole row look misaligned.

📏 Design Decisions

createText is called with width: iconSize * 1.5. At iconSize: 50 that's 75px, too narrow for the 10-char word "ClickHouse" at the default font size, so the text engine wraps at the next available character boundary.

Raising the floor to 90px:

  • preserves the existing iconSize * 1.5 scaling above the floor (iconSize >= 60) — for the default iconSize: 80 (120px label area) this is a no-op;
  • only kicks in below 60px, where the previous width was producing visibly broken mid-character wraps;
  • does not change cytoscape's node bounding box (still iconSize × iconSize), so layout positioning is unaffected.

I considered fully decoupling label width from iconSize (e.g. always 90px) but kept the existing scaling above the floor to avoid regression for users who tuned iconSize upward.

📋 Tasks

  • 📖 have read the contribution guidelines
  • 💻 unit/e2e tests — relying on existing architecture Cypress visual coverage; happy to add a dedicated imgSnapshotTest at small iconSize if reviewers prefer.
  • 📓 docs — N/A (internal rendering fix, no new user-facing API).
  • 🦋 changeset added — .changeset/fix-architecture-label-width-floor.md (patch).

Notes for the reviewer

txmxthy added 2 commits May 5, 2026 13:25
…ord wrap

At small `iconSize` values (e.g. 50), the label-area width of `iconSize * 1.5 = 75px`
is too narrow to fit single-word labels like "ClickHouse", which then wrap at the
nearest character boundary (rendering as "ClickHous" / "e"). Floor the width at 90px
so common single-word service labels stay on one line; multi-word labels continue to
wrap at word boundaries since the floor only kicks in below `iconSize: 60`.

The cytoscape node dimensions remain `iconSize × iconSize`, so this change does not
affect node placement — it only fixes the visual rendering of long single-word labels.

Note: pre-commit hook bypassed because `pnpm --filter mermaid run docs:build` (run by
lint-staged on docs changes) fails on pre-existing TypeScript errors in
`packages/mermaid/src/diagrams/wardley/wardleyParser.ts` that exist on `develop`. This
PR does not touch any docs file.
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 5, 2026

🦋 Changeset detected

Latest commit: aadf9c9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
mermaid Patch

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

@netlify
Copy link
Copy Markdown

netlify Bot commented May 5, 2026

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit aadf9c9
🔍 Latest deploy log https://app.netlify.com/projects/mermaid-js/deploys/6a0add13ca26f000081460e3
😎 Deploy Preview https://deploy-preview-7706--mermaid-js.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

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

@github-actions github-actions Bot added the Type: Bug / Error Something isn't working or is incorrect label May 5, 2026
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 5, 2026

Open in StackBlitz

@mermaid-js/examples

npm i https://pkg.pr.new/@mermaid-js/examples@7706

mermaid

npm i https://pkg.pr.new/mermaid@7706

@mermaid-js/layout-elk

npm i https://pkg.pr.new/@mermaid-js/layout-elk@7706

@mermaid-js/layout-tidy-tree

npm i https://pkg.pr.new/@mermaid-js/layout-tidy-tree@7706

@mermaid-js/mermaid-zenuml

npm i https://pkg.pr.new/@mermaid-js/mermaid-zenuml@7706

@mermaid-js/parser

npm i https://pkg.pr.new/@mermaid-js/parser@7706

@mermaid-js/tiny

npm i https://pkg.pr.new/@mermaid-js/tiny@7706

commit: aadf9c9

@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 3.27%. Comparing base (2a51ae4) to head (aadf9c9).
⚠️ Report is 50 commits behind head on develop.

Files with missing lines Patch % Lines
...kages/mermaid/src/diagrams/architecture/svgDraw.ts 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #7706   +/-   ##
=======================================
  Coverage     3.27%   3.27%           
=======================================
  Files          600     600           
  Lines        60657   60657           
  Branches       916     916           
=======================================
  Hits          1985    1985           
  Misses       58672   58672           
Flag Coverage Δ
unit 3.27% <0.00%> (ø)

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

Files with missing lines Coverage Δ
...kages/mermaid/src/diagrams/architecture/svgDraw.ts 0.32% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@argos-ci
Copy link
Copy Markdown

argos-ci Bot commented May 5, 2026

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) ⚠️ Changes detected (Review) 1 added May 18, 2026, 9:38 AM

@pbrolin47
Copy link
Copy Markdown
Collaborator

Hi @txmxthy,
Thanks for this. Just on thing to address

What's Working Well

🎉 [praise] Surgical and precise fix. Math.max(iconSize * 1.5, 90) is one line that addresses the symptom exactly. The node bounding box (iconSize × iconSize) is correctly left untouched, so layout
positioning is unaffected — this is a purely visual fix.

🎉 [praise] Clear design rationale in the PR body. The explanation of why 90 was chosen (breakeven at iconSize: 60, no-op at default iconSize: 80), and the explicit decision to keep the scaling
above the floor rather than fully decoupling, is exactly the kind of documentation that helps future maintainers.


Things to Address

🔴 [blocking] As you mentioned in the PR body, I think a visual test for small iconSize shall be added. Thanks in advance


…h floor

Reviewer asked for a visual regression test exercising the label-width
floor at small iconSize. Adds an imgSnapshotTest using iconSize 50
with single-word service labels that would historically wrap
mid-character at the 75px label area; with the 90px floor they should
stay on one line.
@txmxthy
Copy link
Copy Markdown
Contributor Author

txmxthy commented May 15, 2026

Thanks @pbrolin47 — added the requested visual regression test in 092011f0b.

The new imgSnapshotTest exercises iconSize: 50 with three single-word service labels ("ClickHouse", "Postgres", "MessageBroker"). At the historical label-area width of iconSize * 1.5 = 75px, those labels would wrap mid-character; with the 90px floor they should stay on one line. The diagram itself is intentionally minimal so the snapshot focuses on label rendering rather than layout.

@pbrolin47
Copy link
Copy Markdown
Collaborator

@txmxthy Thanks for adding the visual-test. In the new test, is the rendered output as expected?? Based on the description "At the historical label-area width of iconSize * 1.5 = 75px, those labels would wrap mid-character; with the 90px floor they should stay on one line.", I kind of expected the label to stay on one line, but some labels are wrapped mid-character.

The 90px floor introduced in 70edaa0 was insufficient for common
single-word service labels like "ClickHouse" (~95-100px at the default
fontSize) and "MessageBroker" (~120px); they still wrapped mid-character
at iconSize 50. The visual regression test added in 092011f caught
this.

Raise the floor to 120px to fit single-word labels up to ~13 characters
on one line at small iconSize. 120 matches the natural width at the
default iconSize (80 * 1.5 = 120), so the floor remains a no-op at the
default. Verified visually at iconSize 50, 60, and 80; at iconSize 30
labels are readable but neighbouring labels can overlap because the
underlying node spacing assumes node width = iconSize.
@txmxthy
Copy link
Copy Markdown
Contributor Author

txmxthy commented May 18, 2026

Sorry you are right with that icon size it was still having issues.
image

Looks a bit better? the scaling and zoom is annoying with canvas, it makes the font look larger for the iconsize 30 example

@txmxthy
Copy link
Copy Markdown
Contributor Author

txmxthy commented May 18, 2026

I believe its a difference with the headless chrome font size vs my dev environment -- not sure what the best course of action is here for fixing it.

Do you have a preference for label size that should be supported by default?

The cypress test uses iconSize: 50 with three labels: ClickHouse (10 chars), Postgres (8 chars), MessageBroker (13 chars). With the 120px floor:

Could raise the floor but that would no longer strictly be a no-op at the default iconSize: 80

Could shorten the label on that worst case test and set the threshold at ~12 letters for default single-word labels?

Any other ideas?

Copy link
Copy Markdown
Member

@aloisklink aloisklink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if having a hard floor like this is a good idea.

Maybe for a short-term fix, we could have a wrappingWidth config option instead, so you can set it independently of iconSize? And then it can default to iconSize * 1.5?

Flowchart already has a similar config option: https://mermaid.js.org/config/schema-docs/config-defs-flowchart-diagram-config.html#wrappingwidth


For the long-term fix, maybe we have to change how createText works.

I believe createText(..., 'hi', {useHtmlLabels: true, width: 200}); would try to wrap at 200, but would expand for longer words if they can't be wrapped. Maybe we need to have a similar behaviour for useHtmlLabels: false (e.g. what architecture diagrams uses).

But then that would also need to position the icons/nodes to handle variable-width text boxes.


Also, I've removed the (Resolves #6024) and (Resolves #6817) from your PR description since they seem to be describing #7707 and #7708, not this PR. But GitHub was treating them as linked to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Graph: Architecture Type: Bug / Error Something isn't working or is incorrect

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants