[EuiBadge] Increase color contrast of selected text (+ bonus cleanup) #7752
Conversation
+ invert those colors for `::selection` text
+ fix low contrast selection color on disabled badges - skip inline style custom colors completely if disabled, no need to calculate or apply them. this allows us to remove `!important` CSS - skip other named colors as well, also no need to apply them. and clickable styles, that definitely makes no sense
|
@MichaelMarcialis and @1Copenut, I think I'd like your input on this before moving forward further with this PR. It certainly accomplishes the goal of increasing selection color contrast, but I don't know if it looks... good... precisely either (see screenshots in PR description) 🙈 I'm worried the change is a little too startling to ship with as a change and I'm wondering if we should consider other approaches. Maybe using CSS color functions like https://developer.mozilla.org/en-US/docs/Web/CSS/filter-function/contrast as an alternative? |
|
Preview staging links for this PR:
|
💚 Build Succeeded
|
There was a problem hiding this comment.
@cee-chen I appreciate the ping on this one. I looked at your screenshots and live preview in light and dark themes a few times. I highlighted all badges and did spot checks for contrast ratios. All were at least 5:1, many significantly higher.
That's my objective measure. My subjective measure is, I love the look of these when text is selected. It's bold, and that's a good thing. It was immediately clear to me what text was highlighted and what was not. The high-contrast aesthetic made the UI more usable, and that's my primary concern.
I'll defer to @MichaelMarcialis for the final call if this is too bold for a first release, but for my part it's 💯 with zero qualifications.
|
Haha, it's fascinating how subjective visuals are! Definitely would appreciate feedback from all corners on this one! |
|
Thanks for the ping, @cee-chen. I largely agree with @1Copenut's assessment here. I think it's more important that the text highlighting is clear/obvious than it is to be aesthetically pleasing. That said, I think the idea of inverting the text and background colors is a simple and elegant way to achieve this. CCing the rest of the @elastic/platform-design team in case they have any thoughts as well. Forgive a potentially silly follow-up question: Since color is our only means of indicating the text highlighting, does that mean that text highlighting background colors must have a contrast ratio of 3:1 with the page background (in addition to the 4.5:1 contrast between highlight text and background colors)? If so, do we need to update our general text highlighting styles as well? |
Not a silly question at all. I think there may be some requirement for this with respect to adjacent colors. I read through SC 1.4.11: Non-text Contrast (Level AA) and there's a whole section on |
|
Thanks for chiming in y'all! It sounds like for the purposes of this PR we're good merging this as-is without any visual changes - am I reading that correctly? Let's continue further selection/color contrast discussions in a separate issue or possibly an actual GH discussion! |
|
@1Copenut Could I get an approval from you on this PR if it's good to ship? |
|
Discussion link for completeness: #7766 |
`v94.5.0-backport.1` ⏩ `v94.5.1` [Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams) --- ## [`v94.5.1`](https://github.com/elastic/eui/releases/v94.5.1) **Bug fixes** - Fixed an `EuiDualRange`s with `showInput` bug, where `min`/`max` values and invalid states were not being correctly set if values were empty strings ([#7767](elastic/eui#7767)) **Accessibility** - Improved `EuiDatePicker` and `EuiSuperDatePicker`'s time selection screen reader UX ([#7726](elastic/eui#7726)) - Improved the accessibility of `EuiDatePicker` by providing full screen-reader-only week day names to the calendar header ([#7748](elastic/eui#7748)) - Improved `EuiBadge`'s ability to tell when text within the badge is selected/highlighted and selection color contrast ([#7752](elastic/eui#7752))
Summary
closes #7722
This PR changes the
::selectioncolor & background color of badges in order to make the selection more apparent to users. It also accomplishes some CSS cleanup (via utilizing CSS variables) at the same time. As always, I recommend following along by commit.QA
General checklist
::selectionon<button>elements, this is not a bug(?)- [ ] Checked for accessibility including keyboard-only and screenreader modes- [ ] Checked in mobile- [ ] Added or updated jest and cypress tests- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)