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

Low contrast ratio for certain background colors #88

Open
1 of 5 tasks
KojiAndoJC opened this issue Sep 21, 2024 · 2 comments
Open
1 of 5 tasks

Low contrast ratio for certain background colors #88

KojiAndoJC opened this issue Sep 21, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@KojiAndoJC
Copy link

What is the problem?

When setting navigationBackgroundColor to #4e9a06, the text displays with low contrast. The contrast ratio falls below the AA rating, triggering a warning in Chrome DevTools.
Screenshot 2024-09-21 164949

Reproduction Steps

Set the configuration JSON as follows:

[
  {
    "env": {
      "account": "your_account_id"
    },
    "style": {
      "navigationBackgroundColor": "#4e9a06"
    }
  }
]

Sign in Method

  • Direct login to target account via AWS SSO
  • Direct login to target account via IAM user
  • Login to a jump account via AWS SSO, then switch role to target account
  • Login to a jump account via IAM user, then switch role to target account
  • Sign in as the root user

Browser Name

Chrome

Browser Version

129.0.6668.59 (Official Build) (64-bit)

Other information

The current logic for determining foreground color is as follows:

  1. Color constants:
const AWSUI_COLOR_GRAY_300 = '#d5dbdb'
const AWSUI_COLOR_GRAY_900 = '#16191f'
  1. Luminance calculation:
const getLuminance = (r: number, g: number, b: number) =>
  (0.299 * r + 0.587 * g + 0.114 * b) / 255

const isLuminant = (color: string): boolean | undefined => {
  const rrggbb = color.match(
    /#([a-fA-F0-9]{2})([a-fA-F0-9]{2})([a-fA-F0-9]{2})$/
  )
  if (rrggbb) {
    const r = parseInt(rrggbb[1], 16)
    const g = parseInt(rrggbb[2], 16)
    const b = parseInt(rrggbb[3], 16)
    return getLuminance(r, g, b) > 0.5
  }
  // (snip)
}
  1. Foreground color selection:
const foregroundColor = isLuminant(navigationBackgroundColor)
  ? AWSUI_COLOR_GRAY_900
  : AWSUI_COLOR_GRAY_300

Causes of the low contrast issue:

  1. The lighter color (#d5dbdb) is darker than the default white (#ffffff), resulting in lower contrast.

  2. The threshold for determining darker foreground color usage is not optimized.

Contrast calculation:
The contrast ratio is calculated using the following equation (source: https://web.dev/learn/accessibility/color-contrast):

(L1 + 0.05) / (L2 + 0.05)
Where:
L1 is the relative luminance of the lighter color
L2 is the relative luminance of the darker color

To find the optimal threshold, we can calculate the middle point (L_middle) by solving:

(L1 + 0.05) / (L_middle + 0.05) = (L_middle + 0.05) / (L2 + 0.05)
L_middle = sqrt((L2 + 0.05) * (L1 + 0.05)) - 0.05

Using the current getLuminance function:

  • L1 (#d5dbdb luminance) = 0.8517882352941177
  • L2 (#16191f luminance) = 0.09720392156862745
  • L_middle = 0.31434429412266945

The isLuminant function should use this L_middle as a threshold instead of 0.5.

In this case, the luminance of the background color (#4e9a06) is 0.44864313725490196, which is ≥ L_middle. This causes isLuminant to return true, resulting in the use of the darker foreground color and, consequently, higher contrast.

For more precise contrast calculations, the luminance should be calculated using a different formula, as described in this article: https://css-tricks.com/understanding-web-accessibility-color-contrast-guidelines-and-ratios/

Please note that I'm not an expert in color theory or accessibility standards, so my understanding and calculations might be incorrect. I welcome any corrections or additional insights from those more knowledgeable in this area.

@KojiAndoJC KojiAndoJC added the bug Something isn't working label Sep 21, 2024
@xhiroga
Copy link
Owner

xhiroga commented Sep 22, 2024

Thank you for your insightful feedback on our color contrast logic, @KojiAndoJC san!

I agree that improvements are needed. I plan to align our implementation with the CSS Color Module Level 6 specification, specifically the contrast-color() function.

However, as browser support is currently limited, we'll implement a similar logic within our repository. This approach will allow us to enhance our color contrast immediately while preparing for future adoption of the official CSS specification. Your contribution is greatly appreciated!

@KojiAndoJC
Copy link
Author

Thank you for your response, @xhiroga. I agree that the contrast-color() solution is smart!

While you're considering implementing similar logic, I realized there's one more point I'd like to add. Even though the logic I suggested would result in higher contrast, some users might prefer the current foreground color. To accommodate these users, it might be helpful to implement a foregroundColor configuration option. This way, users could choose between the automatic high-contrast selection and their preferred color, offering more flexibility.

As I'm not fully familiar with the entire codebase or all use cases, I think it's best to wait for your implementation. However, I'm happy to provide any additional feedback or clarification if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants