-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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: bringing metric type icon styles into SelectControl #14531
Conversation
Codecov Report
@@ Coverage Diff @@
## master #14531 +/- ##
==========================================
+ Coverage 77.09% 77.33% +0.23%
==========================================
Files 959 959
Lines 48309 48310 +1
Branches 5661 5661
==========================================
+ Hits 37243 37359 +116
+ Misses 10866 10751 -115
Partials 200 200
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
0346d7b
to
f7abce4
Compare
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!
i agree that we need to standardize our components once and for all, to prevent future regressions. and I think now is probably a good time to finish the project. though we don't have much left, it's necessary to come up with a good testing plan for high-risk PRs. |
SUMMARY
PR #14485 cleaned up a bunch of LESS styles and moved them into Emotion. One style was also needed by the
SelectControl
component, and there was fallout from moving it away. This PR fixes it forward, putting the Emotion based styles where they're needed.I believe this is further indication that we need to standardize our Select components (there are several), to minimize these surprise regressions.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
After:
TEST PLAN
Pick any viz with a
group by
control - e.g. the Table viz. Check that the options in the menu don't look broken.Test all over the place for related fallout.
ADDITIONAL INFORMATION