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.

Sign up for GitHub

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

Jump to bottom

fix: update styles on color page #4157

Merged
merged 5 commits into from
Jul 31, 2024
Merged

fix: update styles on color page #4157

merged 5 commits into from
Jul 31, 2024

Conversation

nicolethoen
Copy link
Collaborator

Closes #4067
Closes #4073

This was linked to issues Jul 25, 2024
Review ColorFamily.css and ColorFamily.js styles for replacing custom styles with existing styling options or tokenization #4067
Closed
Review commented out inline style in ColorSwatch.js #4073
Closed
@patternfly-build
Copy link
Contributor

patternfly-build commented Jul 25, 2024 •
edited
Loading

Preview: https://patternfly-org-pr-4157-site.surge.sh

@edonehoo
Copy link
Collaborator

A few things I noticed:

  1. In dark mode, the text on the color families doesn't update and can't be read in some cases:
image
  1. Also in the families, there are sometimes no tokens listed (which maybe there are just no tokens related to the color? but it looks like there should be some listed):
image

  1. Related to ^ Should we change the label from "CSS variables" to "Tokens" in the expanded family sections?

  2. This is small, but the primary background color swatch has a background, even before you hover:

image

edonehoo
edonehoo reviewed Jul 29, 2024
View reviewed changes
packages/documentation-site/patternfly-docs/content/design-guidelines/styles/colors/colors.md Outdated
To check the contrast between background and text colors, use a <a href="https://color.a11y.com/?wc3" target="_blank" className="pf-m-link">WCAG AA-compliance tool.</a>

<Divider/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Divider/>

@nicolethoen
Copy link
Collaborator Author

4. This is small, but the primary background color swatch has a background, even before you hover:

That's because without a background color, the color swatch is invisible! 😆 I'm open to other color suggestions!

Also in the families, there are sometimes no tokens listed (which maybe there are just no tokens related to the color? but it looks like there should be some listed):

I can make a 'no semantic tokens message if there are none?

@edonehoo
Copy link
Collaborator

edonehoo commented Jul 30, 2024 •
edited
Loading

That's because without a background color, the color swatch is invisible!

Oh, duh! 😆 I can't think of a different approach that's ~the golden ticket, so I think we can just stick with this one, just to get the info updated? And then maybe revisit with a designer to see if we want to change it

edonehoo
edonehoo approved these changes Jul 30, 2024
View reviewed changes
Copy link
Collaborator

@edonehoo edonehoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it 🗣️

@nicolethoen @edonehoo
Update packages/documentation-site/patternfly-docs/content/design-gui…
14616f1 
…delines/styles/colors/colors.md

Co-authored-by: Erin Donehoo <[email protected]>
@nicolethoen nicolethoen merged commit 4df0b25 into patternfly:v6 Jul 31, 2024
4 checks passed
This was referenced Jul 31, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edonehoo edonehoo edonehoo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@nicolethoen @patternfly-build @edonehoo