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

[Security Solution] Secondary text fails AAA colour contrast #121758

Closed
gavinwye opened this issue Dec 21, 2021 · 15 comments
Closed

[Security Solution] Secondary text fails AAA colour contrast #121758

gavinwye opened this issue Dec 21, 2021 · 15 comments
Assignees
Labels
accessibility best practice accessibility: design system Accessibility issues that can be updated with EUI bug Fixes for quality problems that affect the customer experience fixed Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. UX: UI/UX Consultation Requires UX lead input/consult before development and UX lead approval on PR before merge. v8.0.0

Comments

@gavinwye
Copy link

Describe the bug:
Small secondary text fails AAA colour contrast

Kibana/Elasticsearch Stack version:
v 7.16.1

Original install method (e.g. download page, yum, from source, etc.):
Eden

Functional Area (e.g. Endpoint management, timelines, resolver, etc.):
Detect > Exceptions (probably all Kibana

Current behavior:
Text colour is set to #69717d on a background of #fefeff which fails AAA

See: https://colourcontrast.cc/fefeff/69717d

Expected behavior:

Changing the text colour to #000000 visually makes the design clearer and also passes AAA increasing our accessibility score overall. See before and after screenshots below.

If we're going to put contention on the page we should make sure everyone can read it.

Screenshots (if relevant):

Before
image

After
image

@gavinwye gavinwye added bug Fixes for quality problems that affect the customer experience triage_needed accessibility best practice Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. UX: UI/UX Consultation Requires UX lead input/consult before development and UX lead approval on PR before merge. accessibility: design system Accessibility issues that can be updated with EUI labels Dec 21, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@snide
Copy link
Contributor

snide commented Jan 5, 2022

@gavinwye . I think this one could be fixed by using proper EUI components. Using the proper EuiPage components (like EuiPageHeader) would standardize this with the proper spacing / coloring. Right now the header for this page looks like it has a lot of overwrites. Just bringing that up because the solution here shouldn't be to use an overwrite and set it to #000. Also remember that we have a dark mode, so that color wouldn't work for dark mode, so normally want to suggest using the components themselves.

Can see this in practice here. Small reminder that there is both a high-order component, and smaller ones that could be used to solve this problem.

https://elastic.github.io/eui/#/layout/page#restricting-page-width

@snide
Copy link
Contributor

snide commented Jan 5, 2022

While we're looking at this section, likely this weird tooltip needs some thought as well. Seems unnecessary?

image

@yctercero
Copy link
Contributor

@gavinwye @snide how do the updates here look? #122870

@gavinwye
Copy link
Author

Updates on #122870 LGTM :)

I agree with @snide that we could fix that tooltip. I'm inclined to suggest removing it seeing as it's duplicating content that's on the page. Is there anything in the code that would imply that it would ever contain anything other than the same content?

yctercero added a commit that referenced this issue Jan 17, 2022
Addresses #121758 and #121759.

Updates exceptions table export icon to be "download" icon and updates exceptions table header to use native EUI page headers.
@yctercero
Copy link
Contributor

This PR has been merged.

@MadameSheema ready for QA check!

@MadameSheema
Copy link
Member

Thanks @yctercero!! is there any reason why this fix is targeting 8.1 and not 8.0? Thanks!

@yctercero
Copy link
Contributor

@MadameSheema I wasn't sure if I could with the RC2 cut off. I can backport it.

@MadameSheema
Copy link
Member

@oatkiller can you please confirm this so in case we can backport to include the fix on RC2 bc2? Thanks!

@oatkiller
Copy link
Contributor

@MadameSheema @yctercero Please feel free to backport to 8.0.

yctercero added a commit to yctercero/kibana that referenced this issue Jan 26, 2022
Addresses elastic#121758 and elastic#121759.

Updates exceptions table export icon to be "download" icon and updates exceptions table header to use native EUI page headers.

(cherry picked from commit 8c0fbdf)
@yctercero
Copy link
Contributor

Backport to 8.0 started - #123787

@MadameSheema @oatkiller

yctercero added a commit that referenced this issue Jan 26, 2022
Addresses #121758 and #121759.

Updates exceptions table export icon to be "download" icon and updates exceptions table header to use native EUI page headers.

(cherry picked from commit 8c0fbdf)

Co-authored-by: Kibana Machine <[email protected]>
@MadameSheema
Copy link
Member

@deepikakeshav-qasource please validate the following fix on latest 8.0-rc2 BC.

@ghost
Copy link

ghost commented Feb 1, 2022

Hi @MadameSheema ,

We have validated this issue on 8.0.0 RC2 BC4 and observed that Text color has been changed to #343741 . please let us know if it is expected

Please find below testing details:

Build Details:

Version: 8.0.0 RC2 BC4
Build: 49192
Commit: 57ca5e139a33dd2eed927ce98d8231a1f217cd15

Screenshot:
image

image

Thanks!!!

@MadameSheema
Copy link
Member

@yctercero can you please check Deepika's comment? Thanks

@yctercero
Copy link
Contributor

Hey @deepikakeshav-qasource ! That is expected, it's now using the EUI component as opposed to the custom header component we were using before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility best practice accessibility: design system Accessibility issues that can be updated with EUI bug Fixes for quality problems that affect the customer experience fixed Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. UX: UI/UX Consultation Requires UX lead input/consult before development and UX lead approval on PR before merge. v8.0.0
Projects
None yet
Development

No branches or pull requests

6 participants