-
Notifications
You must be signed in to change notification settings - Fork 197
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
Organise and fix bugs in color definitions #4969
Conversation
Full-stack documentation: https://docs.openverse.org/_preview/4969 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
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.
I like the cleanup but I'm a little wary of removing the list of all values from tailwind.config
because we lose the ability to use search during development. Maybe we could move the generation to a separate script, and generate the tailwind file using it?
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.
I like this approach; LGTM.
To @obulat's point I think if I was searching for a CSS var, like "--color-info-1", for example, the result returned from frontend/src/styles/tailwind.css would be much more useful than the now-unavaliable tailwind.config.ts value, anyway.
Adding onto Zack's point, searching in colors: {
yellow: {
7: 'var(--color-yellow-7)',
},
} |
Description
This PR organises colors into groups in the
tailwind.css
andtailwind.config.ts
files for clarity.Color changes
This PR removes redundant and/or unused colors and also adds missing colors.
tailwind.css
tailwind.config.ts
color-gray
colors.gray.DEFAULT
color-gray-13-0
colors.gray-opacity.13.0
color-white-0
colors.white.0
--color-bg-zero
backgroundColor.zero
colors.gray.light-gray
colors.gray.dark-gray
--color-bg-blur
Functions in Tailwind config
This PR also uses functions to generate color shade and opacity scales in
tailwind.config.ts
. Typing out 13 shades for each color is understandably more readable but also error-prone. There is an existing case of such error where a value was missed.openverse/frontend/tailwind.config.ts
Lines 171 to 172 in 0ad26ae
Still, to preserve most of the past readability, programmatic definition is kept to a minimum and is confined to two of the most repetitive usages.
Testing Instructions
This PR should result in no change to the user-interface.
Checklist
Update index.md
).main
) or a parent feature branch.ov just catalog/generate-docs
for catalogPRs) or the media properties generator (
ov just catalog/generate-docs media-props
for the catalog or
ov just api/generate-docs
for the API) where applicable.Developer Certificate of Origin
Developer Certificate of Origin