-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
Update class diagram to v3 using new renderer #5880
base: develop
Are you sure you want to change the base?
Conversation
|
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
commit: |
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small comments from initial scan. Have not run through code in vscode for more feedback yet :)
@@ -91,6 +99,16 @@ export const addClass = function (_id: string) { | |||
classCounter++; | |||
}; | |||
|
|||
const addInterface = function (label: string, classId: string) { | |||
const _interface: Interface = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just personal preference/nit, but Im not a fan of prefacing variables names with _
]; | ||
|
||
if ( | ||
relation.relation.type1 === relationType.LOLLIPOP && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe change relation variable to classRelation. Only because it looks odd to say relation.relation.
styleClasses.set(id, styleClass); | ||
} | ||
|
||
if (style !== undefined && style !== null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: you should be able to just say if (style) {..}
}); | ||
} | ||
|
||
for (const [, value] of classes) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that the syntax is so that you can basically just "ignore" the key section of the KVP, but it always looks so weird to me
} | ||
|
||
for (const [, value] of classes) { | ||
if (value.cssClasses.includes(id)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try and minimize nested loops where possible.
if (value.cssClasses.includes(id)) {
value.styles.push(...style.flatMap(s => s.split(',')));
}
📑 Summary
Updates the class diagram to the new unified way of rendering. Includes a new "classBox" shape to be used in diagrams as well as some other updates such as: the option to hide the empty members box in class diagrams, support for handDrawn look, the introduction of the classDef statement into class diagrams, support for styling the default class, and lollipop interfaces.
Includes fixes / additions for #5562 #3139 and #4037
📏 Design Decisions
📋 Tasks
Make sure you
MERMAID_RELEASE_VERSION
is used for all new features.pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.