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

chore: checkbox light mode #7165

Merged
merged 2 commits into from May 14, 2024
Merged

chore: checkbox light mode #7165

merged 2 commits into from May 14, 2024

Conversation

deboer-tim
Copy link
Collaborator

What does this PR do?

Add entries to the color registry for checkbox, with support for light mode. Light mode colors are the same for now since I couldn't find a precedent, except for unchecked.

Took this opportunity to add focus (hover) states since they were missing and didn't match other UI elements. For unchecked the outline becomes purple, for all other states it's one step lighter for dark mode and darker for light mode.

Screenshot / video of UI

Screen.Recording.2024-05-10.at.11.44.10.AM.mov

What issues does this PR fix or reference?

Part of #4914.

How to test this PR?

View checkboxes on main pages; tests added.

  • Tests are covering the bug fix or the new feature

Add entries to the color registry for checkbox, with support for
light mode. Light mode colors are the same for now since I couldn't
find a precedent, except for unchecked.

Took this opportunity to add focus (hover) states since they were
missing and didn't match other UI elements. For unchecked the outline
becomes purple, for all other states it's one step lighter for dark
mode and darker for light mode.

Part of #4914.

Signed-off-by: Tim deBoer <[email protected]>
@deboer-tim deboer-tim requested review from benoitf and a team as code owners May 10, 2024 15:45
@deboer-tim deboer-tim requested review from cdrage, axel7083 and ekidneyrh and removed request for a team May 10, 2024 15:45
@ekidneyrh
Copy link
Contributor

Spent some time on this this morning. I was having a look at the contrast and was thinking maybe it would be a good idea to simplify the colours and have just two main colours - one for hover and one for the default. By using less colours the hover stands out some more as we can lighten it two steps without messing the contrast. Not all the colours previous could be lightened without the contrast going below an AA rating.

I also changed the disable colour to match the +/- button here: #6929 (comment)

Checkbox.pdf
Input-checkbox

WDYT?

@deboer-tim
Copy link
Collaborator Author

I really like the simplification (especially getting rid of dusty purple) and have only looked at dark mode so far, but in context I find the colors too bright and somewhat distracting. The purple are lighter than the purples we use in most places, and disabled looks too close to a stop button.

I'm not a designer, but I tried playing with these pulling colors from elsewhere, and he's what I came up with.

Original:

Screenshot 2024-05-13 at 8 21 01 AM

Proposed:

Screenshot 2024-05-13 at 8 14 26 AM

Proposed with darker colors:
purple 500/600 to match Buttons (could be one step brighter IMHO), unchecked gray-400 (same as start/stop/delete buttons), and charcoal-300 (just matching what was there before).

Screenshot 2024-05-13 at 8 28 20 AM

@ekidneyrh
Copy link
Contributor

image
Just checking the contrast, I was trying to use AAA rating colours but AA can suffice. I think purple 400 and 500 would work better as purple-600 is too low, and I would stick with the gray-700 for the disabled tho just bc the charcoal-300 is a fail.

I think the disabled checkbox in context with the other checkboxes on the page would be a signifier to the user what it is. If it's the only one on the page, the hover with the X would let them know it's not selectable. If it's an issue you're concerned with, we could change that box to a different disabled one, such as:
image

We could also just swap the unchecked focused and unfocused colours could make it less distracting.

Checkbox1

@deboer-tim
Copy link
Collaborator Author

Updated with these colors. My preference would still be a darker disabled color (I think a non-AAA would actually be a good thing here, because we don't want disabled to stand out as much as the ones you can select), but if you can approve lets merge and we can respond later if there is feedback.

@deboer-tim deboer-tim merged commit e31d31e into main May 14, 2024
8 checks passed
@deboer-tim deboer-tim deleted the input-checkbox branch May 14, 2024 18:34
@podman-desktop-bot podman-desktop-bot added this to the 1.11.0 milestone May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants