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

Fix - static class attributes are not rendered underlined #5013

Conversation

SteffenLm
Copy link
Contributor

📑 Summary

This PR fixes the incorrect rendering of static class attributes in a class diagram.

Resolves #5009

📏 Design Decisions

It just fixes the regular expression which was wrong. Now it detects the right characters ('$' and '*').

📋 Tasks

Make sure you

Please let me know if i missed something!

Copy link

netlify bot commented Nov 3, 2023

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit b5fd8fb
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/6545518ac3092900087da290
😎 Deploy Preview https://deploy-preview-5013--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.

Copy link

codecov bot commented Nov 3, 2023

Codecov Report

Merging #5013 (b5fd8fb) into develop (f5bd1e0) will increase coverage by 37.20%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           develop    #5013       +/-   ##
============================================
+ Coverage    42.94%   80.14%   +37.20%     
============================================
  Files           23      164      +141     
  Lines         4972    13820     +8848     
  Branches        21      693      +672     
============================================
+ Hits          2135    11076     +8941     
+ Misses        2836     2595      -241     
- Partials         1      149      +148     
Flag Coverage Δ
e2e 86.19% <100.00%> (?)
unit 42.94% <ø> (ø)

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

Files Coverage Δ
packages/mermaid/src/diagrams/class/classTypes.ts 93.47% <100.00%> (ø)

... and 159 files with indirect coverage changes

@SteffenLm
Copy link
Contributor Author

@jgreywolf thanks for approving the PR but who can merge it?

@jgreywolf jgreywolf added this pull request to the merge queue Nov 13, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 13, 2023
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.

Nice work with the PR, love the tests.
Would prefer to let @knsv decide regarding removing ?.

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.

Let's do it!

Copy link
Member

@aloisklink aloisklink left a comment

Choose a reason for hiding this comment

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

Looks good to me too!

I think the merge failed due to a random/flakey test, so retrying it should hopefully fix it.

Edit: Ah, it's cool to see that you work at Airbus Immenstaad, I also used to work there :)

@aloisklink aloisklink added Type: Bug / Error Something isn't working or is incorrect Graph: Class labels Nov 14, 2023
@aloisklink aloisklink added this pull request to the merge queue Nov 14, 2023
Merged via the queue into mermaid-js:develop with commit c56025e Nov 14, 2023
17 checks passed
Copy link

mermaid-bot bot commented Nov 14, 2023

@SteffenLm, Thank you for the contribution!
You are now eligible for a year of Premium account on MermaidChart.
Sign up with your GitHub account to activate.

@SteffenLm
Copy link
Contributor Author

Looks good to me too!

I think the merge failed due to a random/flakey test, so retrying it should hopefully fix it.

Edit: Ah, it's cool to see that you work at Airbus Immenstaad, I also used to work there :)

What a pitty. I like to work with colleagues outside the company ;) let's stay in contact :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Graph: Class Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Class attributes marked as static ($) don't render text-decoration: underline
5 participants