-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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 icon shape color for colored icons. #5937
Conversation
🦋 Changeset detectedLatest commit: 17b7831 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
commit: |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5937 +/- ##
=======================================
Coverage 4.67% 4.67%
=======================================
Files 373 372 -1
Lines 51680 51669 -11
Branches 586 611 +25
=======================================
Hits 2414 2414
+ Misses 49266 49255 -11
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
All your changes in the Mermaid code look good!
My recommended changes are:
- Can we change the test icon to something that's not a logo?
- Can you add a changeset using
pnpm changeset
? Maybe you can even make two, since you're fixing two different bugs in this PR! Please also update the PR title as well (even if it's just a general "Icon shapes improvements" since this PR handles a lot of stuff). - The added documentation in
packages/mermaid/src/docs/icon-shape.md
needs a bit of work to look good on the https://mermaid.js.org/ website
@aloisklink I have updated the icon used in visual test. Also removed Doc from this PR. Will create separate PR for doc changes. |
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.
Moving the docs into a separate PR sounds good to me!
Two more changes required!
- Please move the license for
tropical-fish
to the correct file (e.g. where the SVG is defined)! - And please use
pnpm changeset
to create a changeset for each of the bugs you've fixed!
📑 Summary
Resolves #
📏 Design Decisions
colour
to override it. All icons with fill as custom colour hexcode will not get overridden.📋 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:
.