-
Notifications
You must be signed in to change notification settings - Fork 95
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
feat(icon): added critical-risk pficon #4758
Conversation
Preview: https://patternfly-pr-4758.surge.sh A11y report: https://patternfly-pr-4758-a11y.surge.sh |
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.
@mcoker other than seeing that you did add this to the files attached in this PR, is there anything else I can or should do to review this. So far, looks good as far as I can tell.
@mcarrano just verify it looks OK on the icon page - https://patternfly-pr-4758.surge.sh/icons |
@mcoker OK. Forgot about that page. Yes, I see it and it looks good. But I do have another question- In your comment here: #4747 (comment), you noted that the other three icons were already present in the icon set. If that's the case, how come they don't show up on the Icons page referenced above? |
@mcarrano the icons on that page are from the "pficon" icon font - the other icons are part of Font Awesome solid. I could probably update that page to include our bundled FAS icons, too, if you think that would be helpful. |
@mcoker that's probably not necessary since it's not a page that appears publicly on the website. I will approve. |
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.
Thanks, @mcoker !
🎉 This PR is included in version 4.185.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
@mcoker Thank you a lot! |
fixes #4747