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/support fa kit custom icons #5430

Merged

Conversation

jakobskrym
Copy link
Contributor

@jakobskrym jakobskrym commented Mar 30, 2024

📑 Summary

Extends the current fontawesome support to also handle the case of custom icons being served through the fak prefix.

📏 Design Decisions

Logic already existed for the basic fontawesome prefixes so mainly extended that and added a section to the flowchart documentation about it.

📋 Tasks

Make sure you

@github-actions github-actions bot added the Type: Enhancement New feature or request label Mar 30, 2024
Copy link

netlify bot commented Mar 30, 2024

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit 38beca1
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/660cff0cda7fa100095e7163
😎 Deploy Preview https://deploy-preview-5430--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 Mar 30, 2024

Codecov Report

Attention: Patch coverage is 0% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 5.74%. Comparing base (e852596) to head (38beca1).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #5430   +/-   ##
=======================================
  Coverage     5.74%   5.74%           
=======================================
  Files          277     276    -1     
  Lines        41898   41886   -12     
  Branches       489     514   +25     
=======================================
  Hits          2407    2407           
+ Misses       39491   39479   -12     
Flag Coverage Δ
unit 5.74% <0.00%> (+<0.01%) ⬆️

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

Files Coverage Δ
packages/mermaid/src/dagre-wrapper/createLabel.js 0.00% <0.00%> (ø)
...ges/mermaid/src/diagrams/flowchart/flowRenderer.js 0.00% <0.00%> (ø)
packages/mermaid/src/rendering-util/createText.ts 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

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.

Changes looks good.

Can you also move the replace function into utils, so that we can avoid duplicating the logic 3 times?

@jakobskrym
Copy link
Contributor Author

Changes looks good.

Can you also move the replace function into utils, so that we can avoid duplicating the logic 3 times?

Will do 👍

@jakobskrym
Copy link
Contributor Author

jakobskrym commented Apr 1, 2024

Done - also added some test cases for the util function and then tried with the demo to make sure it works as intended 👍 (removed it though as it's a bit of an edge case and I didn't want it to be dependent on my fontawesome kit).

Screenshot 2024-04-01 at 19 04 06

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 @jakobskrym 🚀

packages/mermaid/src/rendering-util/createText.ts Outdated Show resolved Hide resolved
jakobskrym and others added 2 commits April 3, 2024 09:02
@sidharthv96 sidharthv96 merged commit 3ccfea8 into mermaid-js:develop Apr 3, 2024
19 checks passed
Copy link

mermaid-bot bot commented Apr 3, 2024

@jakobskrym, 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.

@jakobskrym jakobskrym deleted the feature/support-fa-kit-custom-icons branch April 4, 2024 08:14
@jakobskrym
Copy link
Contributor Author

@sidharthv96 any idea when this will make it out to production? Saw that it wasn't included in the latest release 🧐

@jakobskrym
Copy link
Contributor Author

Another ping regarding this @sidharthv96 - really want to start using it in our projects 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants