-
Notifications
You must be signed in to change notification settings - Fork 15
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 icons to health status indicators #677
Conversation
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.
Two comments/questions, @rtorrero
- The warning sign will be a yellow triangle with white background and an exclamation mark inside, right?
- This applies to all views across the console, not just the dashboard, correct?
Thx!
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!
As @abravosuse said, please check if all the health icons all around the app are using this HealthIcon
.
Besides that, only a leftover import
I think
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.
LGTM, you can try writing a jest test for this too if you feel about it
a205a65
to
462d668
Compare
Yes, I checked all other views and can confirm that both the warning message works and that the change affects all views. Only the hosts lists was manually setting the health indicators without the component and that was already changed in the PR |
@dottorblaster Added the Jest test, thanks for the suggestion 👌 |
902ee8f
to
8d1f12c
Compare
This PR improves the health status indicators in multiple views by including an icon for additional feedback when either
passing
,warning
orcritical
.Example:
This is specially useful for people suffering from color blindness