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

Feature/1768 sequencediagram custom grouping #4675

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

hersberger
Copy link

@hersberger hersberger commented Jul 27, 2023

📑 Summary

Adds method for creating a grouping of actions (similar to loop, alt, break), but with a custom label.

image

Resolves #1768

📏 Design Decisions

Syntax uses "group" as a keyword, followed by a colon, followed by the custom label. White space around the colon is optional.

Example:

group:label [description]
... statements ...
end

📋 Tasks

Make sure you

@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (91907fe) 43.04% compared to head (dbb797a) 79.34%.
Report is 3 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           develop    #4675       +/-   ##
============================================
+ Coverage    43.04%   79.34%   +36.29%     
============================================
  Files           23      167      +144     
  Lines         5018    13916     +8898     
  Branches        21      710      +689     
============================================
+ Hits          2160    11041     +8881     
+ Misses        2857     2720      -137     
- Partials         1      155      +154     
Flag Coverage Δ
e2e 85.10% <0.00%> (?)
unit 43.21% <ø> (+0.16%) ⬆️

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

Files Coverage Δ
...ckages/mermaid/src/diagrams/sequence/sequenceDb.js 82.64% <0.00%> (ø)
.../mermaid/src/diagrams/sequence/sequenceRenderer.ts 87.20% <0.00%> (ø)

... and 161 files with indirect coverage changes

@hersberger hersberger marked this pull request as draft July 27, 2023 14:36
@hersberger hersberger marked this pull request as ready for review July 27, 2023 14:57
@hersberger hersberger marked this pull request as draft July 27, 2023 15:52
@hersberger hersberger marked this pull request as ready for review July 27, 2023 17:18
@@ -369,6 +369,30 @@ sequenceDiagram
API-->BillingService: Start billing process
```

## Custom Grouping
Copy link
Member

Choose a reason for hiding this comment

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

Make sure MERMAID_RELEASE_VERSION is used for all new features.

Copy link
Author

Choose a reason for hiding this comment

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

Added

Copy link
Member

@sidharthv96 sidharthv96 left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, I'm not sure why the coverage tests didn't run properly, but you can ignore those warnings for now.

Can you please add some screenshots of the diagram in the description?

@sidharthv96
Copy link
Member

The group label and description should support "quoting", so we can have spaces and unicode symbols in it.

Separating the label and description with just spaces doesn't seem like the most ideal way, any suggestions there?


Can you also add some e2e tests with nested grouping?

image

* develop: (788 commits)
  Changes to gantt.html 1. Added a Gantt diagram that demonstrates to users that hashtages and semicolons can be added to titles, sections, and task data. Changes to gantt.spec.js 1. Added unit tests to ensure that semicolons and hashtags didn't break the functionality of the gantt diagram when used in titles, sections or task data. Changes to /parser/gantt.spec.js 1. Added rendering tests to ensure that semicolons and hashtags in titles, sections, and task data didn't break the rendering of Gantt diagrams.
  Lint
  Remove echo
  RefTest
  Echo event
  Update cypress
  Fix applitools
  docs: fix lint
  docs: move community to Discord
  Swap condition blocks to avoid using negation
  Reposition const declaration to ideal place
  Change repetitive values into consts
  docs: fix swimm link
  add sequenceDiagram link e2e test
  Fix Update Browserslist
  Use pnpm/action-setup@v2
  Fix lint
  Cleanup e2e.yml
  Ignore push events on merge queue
  Remove ::
  ...
Copy link

netlify bot commented Jan 27, 2024

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit dbb797a
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/65b52459229ee100082cc5a4
😎 Deploy Preview https://deploy-preview-4675--mermaid-js.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@github-actions github-actions bot added the Type: Enhancement New feature or request label Jan 27, 2024
sequenceDiagram
Alice-->Bob: I have a secret
Bob-->Alice: What is it?
group:Whisper So nobody hears
Copy link
Member

@sidharthv96 sidharthv96 Jan 27, 2024

Choose a reason for hiding this comment

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

Suggested change
group:Whisper So nobody hears
group:Whisper Slowly [So nobody hears]

Let's wrap the description in [], so that we can have labels with spaces.
group:"Whisper" ["So nobody hears"] Quoting support can also be added to support unicode.

Copy link
Member

Choose a reason for hiding this comment

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

@nirname I'm not really sure if this is the optimal approach to support group.
What do you think?

The following syntaxes need to be supported, without breaking existing diagrams having actors named "group".

group: Name [Desc]
group : Name With Space [Desc with space]
group: "Quoted name" ["Quoted desc"]

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll revive this PR but have to thoroughly check this

@nirname nirname self-assigned this Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Type: Enhancement New feature or request
Projects
Status: Abandoned
Development

Successfully merging this pull request may close these issues.

Sequence diagram request: Grouping interactions in own defined group
3 participants