-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
refactor: Removes hard coded colors #11977
refactor: Removes hard coded colors #11977
Conversation
Removes hard coded colors defined in src/components/styles.ts. The colors defined in this file were used only in the Select component so they were migrated to src/components/Select/styles.tsx and ajusted to conform to the theme colors.
welcome on board Michael! thanks for the first PR!! @michael-s-molina |
Codecov Report
@@ Coverage Diff @@
## master #11977 +/- ##
==========================================
- Coverage 66.26% 63.62% -2.64%
==========================================
Files 941 941
Lines 45715 45826 +111
Branches 4395 4398 +3
==========================================
- Hits 30293 29159 -1134
- Misses 15287 16491 +1204
- Partials 135 176 +41
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@junlincc Thanks! I'm very excited to work with you. |
Welcome onboard, @michael-s-molina ! Thanks for working on this. Could you also add a screenshot of Select component in real action just for sanity check? |
I think the scope of this PR is great, but I'm hoping we can use the theme context via the |
@ktmud Thanks! I added the screenshots as requested. @rusackas All styles are now coming from The Select component uses a combination of style sources (react-select theme, default theme and theme props). If we have the intention on unifying all this then we should do it in another PR because we may need a bunch of refactoring. |
I think current approach makes sense, given how react-select structures its styling. With #11916 in the work, I wouldn't invest too much in the refactoring. |
Tested locally and everything looks great. |
Congrats on the first merge! Welcome to the community! |
SUMMARY
Removes hard coded colors defined in src/components/styles.ts. The colors defined in this file were used only in the Select component so they were migrated to src/components/Select/styles.tsx and adjusted to conform to the theme colors.
Here is the theme color palette and the corresponding mapping:
Here are some screenshots of Select component:
TEST PLAN
You can view the select component inside Storybooks.
ADDITIONAL INFORMATION