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

Fix low colour contrast and incorrect label markup #889

Merged
merged 2 commits into from
Sep 26, 2024

Conversation

MatMoore
Copy link
Contributor

@MatMoore MatMoore commented Sep 26, 2024

The markup is wrong because <button>s should not have a separate <label> element - a button's label is its text content.

This was originally flagged as a low contrast issue by WAVE, but when I corrected the markup it went away.

Although the existing colour meets the requirement for non-text elements at a ratio of 3.91:1, I thought I may as well switch it to the darker blue to make it clearer (9.37:1).

Before:
Screenshot 2024-09-26 at 15 46 45

After:
Screenshot 2024-09-26 at 15 46 28

This label was not associated with anything.

If it's just regular text inside a `<button>` then a label is not
required.

> An <input> element with a type="button" declaration and a valid value attribute does not need a label associated with it. Doing so may actually interfere with how assistive technology parses the button input. The same applies for the <button> element.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/label
This is no longer being flagged by Wave, but there was still
a low contrast between the search icon and its background.

Using dark-blue gives us a 9.37:1 contrast ratio which is easier
to perceive.
@MatMoore MatMoore requested a review from a team as a code owner September 26, 2024 14:46
@MatMoore MatMoore merged commit 7333579 into main Sep 26, 2024
23 checks passed
@MatMoore MatMoore deleted the fix-low-colour-contrast branch September 26, 2024 15:08
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.

2 participants