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.

Sign up for GitHub

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

Jump to bottom

Improve arrow positions on sequence diagrams #4691

Closed
simonborje opened this issue Aug 1, 2023 · 6 comments · Fixed by #4804
Closed

Improve arrow positions on sequence diagrams #4691

simonborje opened this issue Aug 1, 2023 · 6 comments · Fixed by #4804
Labels
Area: Development Graph: Sequence Internals: Renderer Status: Approved Is ready to be worked on Status: Awaiting PR Type: Bug / Error Something isn't working or is incorrect

Comments

@simonborje
Copy link

Description

Compared to most other sequence diagrams I've seen generated by other software, arrow heads in mermaid sequence diagrams are positioned differently. I would not necessarily say that the mermaid way is "wrong", but at least to my eyes it does look a bit "off" compared to what I'm used to seeing.

The largest difference is that arrow heads extend into activation boxes rather than end at the border of the box.
Also, at larger scales, the tip of the arrow heads seem to slightly shoot past the line that they point to when no activation box exist.
I have added two screenshots to illustrate how it looks now and how it might look if it was drawn like most examples I could find.

Steps to reproduce

Link to the example used for the screenshots: mermaid.live

Screenshots

Current look:
image

Mock-up:
image

Code Sample

No response

Setup

  • Mermaid version: 10.3.0
  • Browser and Version: Firefox 115.0.3

Suggested Solutions

No response

Additional Context

No response
@simonborje simonborje added Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect labels Aug 1, 2023
@sidharthv96
Copy link
Member

sidharthv96 commented Aug 1, 2023 •
edited
Loading

This is not "just" a sequence diagram issue. Similar issue exists with flowcharts.
If someone who is familiar with SVGs could pitch in, that would be wonderful!

image image image

@sidharthv96
Copy link
Member

Related

@mastersibin
Copy link
Contributor

I solved the bug for sequence diagrams

@sidharthv96
Copy link
Member

@mastersibin, Wonderful! Can you raise a PR? Thank you :)

@mastersibin mastersibin mentioned this issue Aug 8, 2023
Closed
@mastersibin
Copy link
Contributor

@sidharthv96 done.

@simonborje
Copy link
Author

Thank you for looking into the issue!
I realize that my original description was pretty vague, so for clarification I see two issues:

  1. The arrow heads go too far into the lines/boxes
  2. The right arrows point at the line rather than the start of the box, e.g.:
    image
    vs
    image
    I'm not sure if the PR addresses one or both of these issues.

sidharthv96 added a commit that referenced this issue Sep 2, 2023
@sidharthv96
fix: #4691 Align arrowheads properly in sequenceDiagram
498f75e
@sidharthv96 sidharthv96 mentioned this issue Sep 2, 2023
Merged
4 tasks
@knsv knsv closed this as completed in #4804 Sep 5, 2023
sidharthv96 added a commit that referenced this issue Sep 6, 2023
@sidharthv96
Merge branch 'develop' into UpdateClasstoMatchUmlSpecs
f1a6ef1 
* develop: (56 commits)
  chore: Fix unit tests
  chore(deps): update all patch dependencies
  chore: Update docs
  Update docs
  New Mermaid Live Editor for Confluence Cloud (#4814)
  Update link to Discourse theme component (#4811)
  Update flowchart.md (#4810)
  chore: remove unneeded `CommomDB`
  chore: Update docs
  "CSS" instead of "css" in flowchart.md (#4797)
  Update CONTRIBUTING.md
  Update CONTRIBUTING.md
  fix: typos (#4801)
  chore: Align with convention
  chore: Add JSDoc to apply in sequenceDB
  refactor: Tidy up direction handling
  chore: Fix flowchart arrow
  chore: Add test to verify activate
  chore: Update tests snapshot
  fix: #4691 Align arrowheads properly in sequenceDiagram
  ...
sidharthv96 added a commit to sidharthv96/mermaid that referenced this issue Sep 6, 2023
@sidharthv96
Merge branch 'develop' into sidv/removeDirectiveGrammar
32729f8 
* develop: (26 commits)
  chore: Fix unit tests
  chore(deps): update all patch dependencies
  Update docs
  chore: remove unneeded `CommomDB`
  fix: Add support for `~test Array~string~`
  chore: Add JSDoc to apply in sequenceDB
  refactor: Tidy up direction handling
  chore: Fix flowchart arrow
  chore: Add test to verify activate
  chore: Update tests snapshot
  fix: mermaid-js#4691 Align arrowheads properly in sequenceDiagram
  chore: move `commonDb` into `diagrams/common/commonDb`
  Update docs
  run prettier fix
  Apply suggestions from code review
  chore: Add comments in edge handling
  chore: Make aggregation arrow transparent
  chore: Remove structuredClone
  chore: Make extension arrow transparent
  chore: Align edge markers properly in class
  ...
@sidharthv96 sidharthv96 mentioned this issue Mar 26, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area: Development Graph: Sequence Internals: Renderer Status: Approved Is ready to be worked on Status: Awaiting PR Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Milestone
No milestone
3 participants
@simonborje @sidharthv96 @mastersibin