Skip to content
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

PF cleanup: remove hardcoded CSS values #368

Open
nicolethoen opened this issue Nov 21, 2024 · 1 comment
Open

PF cleanup: remove hardcoded CSS values #368

nicolethoen opened this issue Nov 21, 2024 · 1 comment
Labels
good first issue Good for newcomers tech debt UX Related to the user experience
Milestone

Comments

@nicolethoen
Copy link

nicolethoen commented Nov 21, 2024

In custom CSS files, colors should never be hardcoded (some examples of non-recommended practices are here and here.

Alternatively, it'd be better to use PF semantic tokens (this will keep the styles dark theme compatible).
I am absolutely happy to help you find the correct semantic tokens. They all live in this css file. Any by default, a semantic token is one with NO number at the end of the token name.

i.e. --pf-t--global--color--brand--100 is NOT a semantic token and should not be used.
Instead --pf-t--global--color--brand--default is a semantic token and would be better to use when trying to use the brand color.

Some semantic tokens I think you'll use often based on looking at your code:

  • instead of white for a background color use var(--pf-t--global--background--color--primary--default)
  • instead of white for an inverse text color, use var(--pf-t--global--text--color--inverse)
  • instead of black for an inverse background color, use var(--pf-t--global--background--color--inverse--default)
  • instead of red for a text color, use var(--pf-t--global--text--color--required)

It's also easy to identify code that is overriding the PF API's colors incorrectly is to add class="pf-v6-theme-dark" to the html or root element. Anything that does not flip to dark mode correctly is something to investigate.

@vishnoianil vishnoianil added good first issue Good for newcomers UX Related to the user experience tech debt labels Nov 21, 2024
@vishnoianil vishnoianil added this to UI Nov 21, 2024
@vishnoianil vishnoianil moved this to Ready in UI Nov 21, 2024
@vishnoianil vishnoianil added this to the release-1.1 milestone Nov 21, 2024
@aalemayhu
Copy link

Is this still relevant? I would love to take a look at this to get familiar with the code base. I see there are several pull requests open and not sure if anyone else has started on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers tech debt UX Related to the user experience
Projects
Status: Ready
Development

No branches or pull requests

3 participants