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

label-has-associated-control not checking an actual valid configuration #956

Open
tomdglenn91 opened this issue Oct 5, 2023 · 2 comments · May be fixed by #1042
Open

label-has-associated-control not checking an actual valid configuration #956

tomdglenn91 opened this issue Oct 5, 2023 · 2 comments · May be fixed by #1042

Comments

@tomdglenn91
Copy link

tomdglenn91 commented Oct 5, 2023

Apologies if I've misunderstood the rule or configured this wrong.

I'm noticing in our code base that labels are often misconfigured, using name as the matcher, not id, which isn't a valid pairing.

I want to ensure that every (custom) input element has an associated label, and it's configured correctly. So the following works out the box:

Given the following configuration:

    'jsx-a11y/label-has-associated-control': [
      'warn',
      {
        labelComponents: ['FormLabel'],
        controlComponents: ['Input', 'Select']
      },
    ],

I expect this to fail, as no HtmlFor, which it fails.

<>
   <FormLabel>Foo</FormLabel>
   <Input name="foo" />
</>

I expect this to fail, as the htmlFor doesn't link to anything, but it doesn't fail. This is the scenario i see in our code base a lot that i want to protect against.

<>
   <FormLabel htmlFor="foo">Foo</FormLabel>
   <Input name="foo" />
</>

Then expect this to pass

<>
   <FormLabel htmlFor="foo">Foo</FormLabel>
   <Input id="foo" name="foo" />
</>

Given that:

This rule checks that any label tag (or an indicated custom component that will output a label tag) either (1) wraps an input element (or an indicated custom component that will output an input tag) or (2) has an htmlFor attribute and that the label tag has text content.

I feel like my expected behaviour is not the reality, but I wasn't quite sure.

Should my expectation work, or am I just misunderstanding the library/rule? Thanks.

@ljharb
Copy link
Member

ljharb commented Oct 9, 2023

I would agree that has an htmlFor attribute and that the label tag has text content. matches the reality and not your expectation.

However, I would expect that 2 should also be checking that the htmlFor value matches the label's id value.

That seems like a useful option to add.

@michaelfaith
Copy link
Contributor

I have this change ready to go, but want to land this change making the error messages more specific to the assert type first: #1041

The follow-up change adding the option proposed here builds on that change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants