Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Enhancement : Badge Component #66555
Enhancement : Badge Component #66555
Changes from all commits
09b3cfe
5877120
409496c
35f553e
ddcb92a
78a6f9e
60d19e2
2c6cb4f
27cf958
6e5da3a
e16faaa
e75a6d8
b928d63
bc9563b
8a22bfd
2133d46
a5e9a5c
dd84e8b
76de6a3
9146bed
c7d0a71
e1c568c
5ca58da
af9da7d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
@WordPress/gutenberg-design I just want to make sure we measure twice and cut once here because publishing a new icon is kind of irreversible 😄
Would it be worth renaming these so it follows the convention of
foo
+fooFilled
like we do for the other pairs like this? For example, change the canonical name ofwarning
tocautionFilled
(maintaining back compat with an aliased export of course).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.
Oh, yes, good suggestion. Just to clarify; that means any existing instances of
warning
would map tocautionFilled
?We should update the
cautionFilled
icon so that the details match the newcaution
icon too. Do you think it makes sense to do that in this PR, or separately?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.
Correct 👍
Yes, let me pull the icon changes out of this PR so we can address it separately with proper care. I'll prepare a PR so you can push up changes to the SVG.
@Vrishabhsk Keeping this PR on hold for just a bit more. Thank you for your patience!
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.
@jameskoster Updates to the SVG can be pushed here: #67895