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

The hover state of pills is low contrast and difficult to read #22134

Closed
anoadragon453 opened this issue May 10, 2022 · 10 comments
Closed

The hover state of pills is low contrast and difficult to read #22134

anoadragon453 opened this issue May 10, 2022 · 10 comments
Labels
A11y A-Appearance A-Pills O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect

Comments

@anoadragon453
Copy link
Member

Steps to reproduce

  1. Hover over a pill
  2. Notice that the background becomes light, which makes the white text difficult to read

Outcome

What did you expect?

To not have to 👀

What happened instead?

image

It looks like the related change is matrix-org/matrix-react-sdk#6398.

To be clear, I really like the interactivity that hovering over a pill now has! But I think the chosen highlight color should be tweaked.

Operating system

Arch Linux

Application version

Element Nightly version: 2022051001, Olm version: 3.2.8

How did you install the app?

The AUR

Homeserver

No response

Will you send logs?

No

@SimonBrandner
Copy link
Contributor

@gaelledel, do you think a different colour would be more suitable?

@SimonBrandner SimonBrandner added X-Needs-Design A-Appearance A-Pills A11y O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist labels May 10, 2022
@daniellekirkwood
Copy link
Contributor

@janogarcia & @germain-gg even with the new colours this is still the case. Is the contrast not being high enough something we missed in the colour pass or something we missed in the design stages?

@germain-gg
Copy link
Contributor

The new colours are fine. In this scenario the pills rely on opacity to convey the hover state.
Can adjust to something we think would look better

@janogarcia
Copy link

@daniellekirkwood @germain-gg I reported it on Figma. Unsure if it didn't get implemented due to introducing breaking changes to existing themes, though.

https://www.figma.com/file/GmiEFTUaqAulfYzou1Rwrz/Review%3A-Germain's-Typography-and-Color-pass?type=design&node-id=96-79&mode=design&t=XODj8JCEXiKJJSbl-0

Review- Germain's Typography and Color pass – Figma

@janogarcia
Copy link

Forgot to add that, on hover, we could just turn the text to primary color, instead of lowering the contrast for the background.

@daniellekirkwood
Copy link
Contributor

oh awesome! :D Thanks both

@germain-gg
Copy link
Contributor

@janogarcia i'm happy to implement the change above, but I believe we're still missing the color mapping for the hover state.
Or should we keep the opacity trick that we're using, but that now passes the color contrast check?

@germain-gg germain-gg self-assigned this Jul 20, 2023
@janogarcia
Copy link

janogarcia commented Jul 20, 2023

@germain-gg Would it be possible to just do what I described above?

That is, no change in background color at all, just slightly the text color (from color.text.secondary to color.text.primary), if possible with a subtle transition (200-300ms).

Accessibility wise there's no requirement for how much colors should change in contrast between rest and hover state as long as you're using a pointer which shape changes on hovering the element.

@germain-gg germain-gg removed their assignment Oct 12, 2023
@daniellekirkwood
Copy link
Contributor

I think this has been fixed with the most recent changes to the colour of Pills.

Hey, @robintown can you confirm? I think we can close this out when the newest changes release :)

@robintown
Copy link
Member

Yes, this is fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A11y A-Appearance A-Pills O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect
Projects
None yet
Development

No branches or pull requests

6 participants