Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ellipse shapes (using A(-description-) are broken in flowcharts #5976

Open
aloisklink opened this issue Oct 18, 2024 · 0 comments
Open

Ellipse shapes (using A(-description-) are broken in flowcharts #5976

aloisklink opened this issue Oct 18, 2024 · 0 comments
Labels
Graph: Flow Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect

Comments

@aloisklink
Copy link
Member

Description

Mermaid has syntax for ellipse shapes in flowcharts, e.g. by doing:

```mermaid
graph TD
  A(-My ellipse shape-)
```

However, it looks like the new default flowchart renderer in Mermaid v9.2.0 broke this.

Steps to reproduce

Try copying the above diagram into the mermaid live editor, or any other mermaid environment.

Screenshots

For example, the following diagram renders like the following using Mermaid 9.1.7, but fails in Mermaid 9.2.0 (unless we set flowchart: { defaultRenderer: 'dagre-d3' } in the config:

            graph TD 
              good(-My ellipse shape-)
              pill([My pill shape])

image

Code Sample

No response

Setup

  • Mermaid version: Since v9.2.0
  • Browser and Version: [Chrome, Edge, Firefox]

Suggested Solutions

The easiest way to fix this would probably be to add the ellipse shape back into: https://github.com/mermaid-js/mermaid/blob/9afb181d064c45abd870e43a6765029e16bd6e39/packages/mermaid/src/rendering-util/rendering-elements/shapes.ts

I believe previously, the ellipse was drawn using dagre-d3's ellipse shape: https://github.com/dagrejs/dagre-d3/blob/6cccb322003a43c30b29019b9ec43885e99aa4a6/lib/shapes.js#L31-L45

Although, we could also remove support for ellipse shapes, considering nobody seems to have noticed that it broke, and that documentation for the ellipse shape was never added (see #274 (comment)).

Additional Context

No response

@aloisklink aloisklink added Type: Bug / Error Something isn't working or is incorrect Graph: Flow labels Oct 18, 2024
@github-actions github-actions bot added the Status: Triage Needs to be verified, categorized, etc label Oct 18, 2024
aloisklink added a commit to aloisklink/mermaid that referenced this issue Oct 18, 2024
Currently, invalid shapes cause an error when rendering, but not when
parsing. This confuses the Mermaid Live Editor, e.g. by not showing the
error message.

Instead, I've added an `isValidShape()` that validates if the shape is
valid at parse time. This only checks shapes using the new extended
shapes syntax. Currenlty, using `A(-this is an ellipse node-)` is broken
(see mermaid-js#5976) and also causes an invalid shape error at render time, but
I've ignored it in this PR, so it will continue pass at parse-time
(we have unit tests checking ellipse parsing).

See: mermaid-js#5976
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Graph: Flow Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Development

No branches or pull requests

1 participant