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

Add Iconify support #827

Merged
merged 8 commits into from
Jan 20, 2025

Conversation

ynaka81
Copy link
Contributor

@ynaka81 ynaka81 commented Jan 11, 2025

📑 Summary

Support Iconify icon packs that are used in architecture-beta.

Resolves #764, #805

📏 Design Decisions

I have the same issue described in the #764 and #805. I think it makes sense to add the way similar as the comment #764 (comment) to the upstream because it doesn't have any side effect if it's lazy load.
Although it might be reasonable to support any icon packs with a command line option, it would be good to support commonly used logos and mdi first as Markdown Preview Mermaid Support supports them.

📋 Tasks

Make sure you

  • 📖 have read the contribution guidelines
  • 💻 have added unit/e2e tests (if appropriate)
  • 🔖 targeted master branch

src/index.js Outdated Show resolved Hide resolved
aloisklink

This comment was marked as resolved.

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 perfect to me! Thanks for making all of those changes 😄

Since we copy mermaid-js/mermaid release cycle (except for bugs/security issues), this will only be released the next time Mermaid has a release.

So I'll leave this PR open for a bit in case @MindaugasLaganeckas wants to review the Docker test changes.

@sumitj18s
Copy link

Thanks @ynaka81 for the effort. Could you please merge this PR to main branch. We are looking forward for this PR to fix the icons issue. I am getting ? as an icon when running mermaid-cli when accessing the aws icons, e.g., aws-s3.

@aloisklink aloisklink merged commit 8a45bb3 into mermaid-js:master Jan 20, 2025
4 of 5 checks passed
@aloisklink
Copy link
Member

@sumitj18s, I've merged this PR to master!

However, it will only be released officially the next time mermaid-js/mermaid has a release (and there's a slight but unlikely chance that we might change the API/CLI flags before the release).

@ynaka81
Copy link
Contributor Author

ynaka81 commented Jan 20, 2025

Thanks for merging this PR! I'm happy to contribute to this repository. It's totally fine to change the API/CLI flags in a way that the community thinks is easier to use and I'm looking forward to the release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to add additional icon packs for architecture diagrams
3 participants