-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
chore: Migrates ControlHeader icons #15265
chore: Migrates ControlHeader icons #15265
Conversation
Codecov Report
@@ Coverage Diff @@
## master #15265 +/- ##
==========================================
- Coverage 77.26% 77.03% -0.24%
==========================================
Files 971 972 +1
Lines 50308 50316 +8
Branches 6140 6140
==========================================
- Hits 38869 38759 -110
- Misses 11235 11353 +118
Partials 204 204
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
/testenv up |
@pkdotson Ephemeral environment spinning up at http://54.191.171.237:8080. Credentials are |
@michael-s-molina did the new icons get design signoff? I notice they are a little different but that's probably because the new icons aren't exactly the same. |
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!
@pkdotson Exactly. The new warning and error icons are different. |
One small detail to consider... I noticed that the icons are a little high up, vertically. It seems the While this sounds like a sensible global change, I'm not sure what fallout/misalignments it might cause. We can pus this PR through as is, or stop and poke around at this. Your call. If you want to merge as is, maybe we can make a project board of "Things to try when we have visual regression testing" |
Someone probably fixed this while changing another requirement. This is the current state with latest |
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
Migrates
ControlHeader
icons.@pkdotson
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
1 - Go to explore
2 - Change the controls to show the icons
ADDITIONAL INFORMATION