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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: adding "Critical Region"/"Option" and "Break" blocks to sequence diagram #3063

Conversation

financelurker
Copy link

馃搼 Summary

This PR adds the capability to use a "Critical Region" with "Option" blocks and a "Break" block within sequence diagrams.

Resolves #3062

馃搹 Design Decisions

I've just extended the sequence diagram parser and renderer with aligning the implementation to the existing alt-else/opt blocks.

馃搵 Tasks

Make sure you

  • 馃摉 have read the contribution guidelines
  • 馃捇 have added unit/e2e tests (if appropriate)
  • 馃敄 targeted develop branch

@financelurker
Copy link
Author

financelurker commented May 21, 2022

Hi @knsv
The failed lint step seems like a common issue in PRs... don't know why that happens.
The failed static analysis / check tests step: It fails with Error: Resource not accessible by integration - is it possible, that this fails because I've added two new e2e tests and the previous snapshots were not found?

And, locally on my machine one e2e test fails: "should render a state with states in it" with Image was 3.1119696969696973% different from saved snapshot with 82156 different pixels. See diff for details: /mermaid/cypress/snapshots/All Integration Specs/diff_output/State-diagram-should-render-a-state-with-states-in-it.diff.png although I haven't changed anything there...

It would be awesome if this change gets into the next release 馃憣馃徎

@knsv
Copy link
Collaborator

knsv commented May 31, 2022

Agree, we are looking at the linting issues. We will also add a description for the snapshots. A good workflow when creating a PR would be to generate a local baseline before any changes are made and do a check against that baseline after the changes. Sometimes unexpected side-effects happens.

Copy link
Collaborator

@knsv knsv left a comment

Choose a reason for hiding this comment

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

This looks really good! Extra appreciation for you taking the time to add the corresponding documentation and tests! Thanks!

@knsv knsv merged commit ee61a26 into mermaid-js:develop May 31, 2022
7 of 9 checks passed
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.

"Critical Region" and "Break" blocks in sequence diagrams
3 participants