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

Bug #4691 | Arrow heads in sequence diagram #4709

Conversation

mastersibin
Copy link
Contributor

@mastersibin mastersibin commented Aug 8, 2023

Resolves #4691

@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Merging #4709 (970b088) into develop (bcb0817) will increase coverage by 31.11%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           develop    #4709       +/-   ##
============================================
+ Coverage    46.03%   77.14%   +31.11%     
============================================
  Files           53      146       +93     
  Lines         6736    14677     +7941     
  Branches        18      564      +546     
============================================
+ Hits          3101    11323     +8222     
+ Misses        3635     3245      -390     
- Partials         0      109      +109     
Flag Coverage Δ
e2e 83.90% <100.00%> (?)
unit 46.03% <ø> (ø)

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

Files Changed Coverage Δ
.../mermaid/src/diagrams/sequence/sequenceRenderer.ts 88.76% <100.00%> (ø)

... and 138 files with indirect coverage changes

@sidharthv96
Copy link
Member

sidharthv96 commented Aug 8, 2023

Can you please add some screenshots of before & after and also some tests?
I also think we should be fixing the root cause, of why the arrow didn't line up in the right place? The 4px value might change in the future, or depending on diagram text, which will break this fix.

@mastersibin
Copy link
Contributor Author

Before
BeforeSequence

After
AfterSequence

@sidharthv96
Copy link
Member

image
image

The arrows don't align perfectly. We need a different approach, which solves the problem at the root.

@mastersibin
Copy link
Contributor Author

image image

The arrows don't align perfectly. We need a different approach, which solves the problem at the root.

I agree, its like putting a bandaid on. I'll try to find what the root cause is.

@sidharthv96 sidharthv96 marked this pull request as draft August 10, 2023 14:54
@sidharthv96 sidharthv96 closed this Sep 2, 2023
@sidharthv96
Copy link
Member

Superseded by #4804

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve arrow positions on sequence diagrams
2 participants