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

a11y-label-has-associated-control audit produces false positives #9456

Closed
1 task
at-the-vr opened this issue Dec 7, 2023 · 4 comments · Fixed by #9483
Closed
1 task

a11y-label-has-associated-control audit produces false positives #9456

at-the-vr opened this issue Dec 7, 2023 · 4 comments · Fixed by #9483
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: toolbar Related to dev toolbar (scope)

Comments

@at-the-vr
Copy link
Member

at-the-vr commented Dec 7, 2023

What version of starlight are you using?

0.15.0

What version of astro are you using?

4.0.1

What package manager are you using?

pnpm

What operating system are you using?

Windows

What browser are you using?

Chrome

Describe the Bug

According to Dev Toolbar Audit the labels used for ThemeSelect& LanguageSelect are missing its controlled text
Complete Error according to Dev Toolbar ⬇

"label" tag should have an associated control and a text content.`
The label tag must be associated with a control using either for or having a nested input. Additionally, the label tag must have text content.

Link to component found from Audit: Select Component

Screenshots ⬇️

image
image

Note: The Stackblitz instance for Starlight does not highlights this issue but any local starlight repo will show this issue

Link to Minimal Reproducible Example

Not applicable

Participation

  • I am willing to submit a pull request for this issue.
@at-the-vr
Copy link
Member Author

I think it looks weird how the Audit message is being chopped up under normal viewport and looks fine by reducing the tab width via Chrome DevTools, I do wanna know how to type this Issue out to withastro/astro without sounding ambiguous

@HiDeoo
Copy link
Member

HiDeoo commented Dec 7, 2023

Thanks for the report.

For more information, the rule details are located here.

{
  code: 'a11y-label-has-associated-control',
  title: '`label` tag should have an associated control and a text content.',
  message:
    'The `label` tag must be associated with a control using either `for` or having a nested input. Additionally, the `label` tag must have text content.',
  selector: 'label:not([for])',
  match(element) {
    const inputChild = element.querySelector('input');
    if (!inputChild?.textContent) return true;
  },
},

It looks like that after matching the label:not([for]) CSS selector, it's looking inside for only input elements, but I think it should also look for meter, output, progress, select, and textarea elements as well.

This may need a second pair of eyes but my initial thinking is that this is a false positive that should be fixed upstream (and maybe a message tweak to not only mention input?)

I think it looks weird how the Audit message is being chopped up under normal viewport and looks fine by reducing the tab width via Chrome DevTools, I do wanna know how to type this Issue out to withastro/astro without sounding ambiguous

I think this one should be covered by this issue #9346

@delucis
Copy link
Member

delucis commented Dec 17, 2023

I agree with @HiDeoo that this is a false positive and should be fixed in the Astro repo. Just as extra confirmation, neither Firefox’s built-in a11y tests nor axe Dev Tools report this issue, and looking at the accessibility tree for the language switcher (for example) shows labels working as expected:

Screenshot of the accessibility tree inspector showing a name of “Select language” and value of “English”

I will transfer this over to the astro repo and add some extra context.

@delucis delucis transferred this issue from withastro/starlight Dec 17, 2023
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Dec 17, 2023
@delucis delucis changed the title Accessibilty Issue over Theme and Language Dropdowns a11y-label-has-associated-control audit produces false positives Dec 17, 2023
@delucis
Copy link
Member

delucis commented Dec 17, 2023

Quick summary of why this was moved from the Starlight repo:

The a11y-label-has-associated-control audit produces warnings on Starlight’s <select> components, which are wrapped in a <label> because it requires labels to always include an <input> element, even though other element types are supported inside <label>.

See @HiDeoo’s comment for links to a similar ESLint rule, which accounts for this.

@ematipico ematipico added feat: toolbar Related to dev toolbar (scope) - P3: minor bug An edge case that only affects very specific usage (priority) and removed needs triage Issue needs to be triaged labels Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: toolbar Related to dev toolbar (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants