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]: label-has-associated-control: ignore undetermined label text #1004

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

BillyLevin
Copy link
Contributor

@BillyLevin BillyLevin commented Aug 22, 2024

Attempt to fix #966

The rule no longer errors if the existence of label text cannot be determined. The criteria for not being able to determine whether the text exists are, for each child of the label:

  1. It is a React component
  2. It has no children
  3. It is not registered as a control component. Unfortunately this means it would still error if the control component itself displays text, but I didn't see a way around that

I don't think this accounts for every case, but hopefully a good starting point

I followed the convention already used by making the new parameters for mayHaveAccessibleLabel optional, even though they're not. I assume that was to make it easier to write the unit tests. Happy to change that though if you'd like

Fixes jsx-eslint#966

The rule no longer errors if the existence of label text cannot be determined
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good! I'll confirm the tests fail without the fix, and then merge.

Copy link

codecov bot commented Aug 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.03%. Comparing base (a08fbcc) to head (a08fbcc).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1004   +/-   ##
=======================================
  Coverage   99.03%   99.03%           
=======================================
  Files         107      107           
  Lines        1660     1660           
  Branches      588      588           
=======================================
  Hits         1644     1644           
  Misses         16       16           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ljharb ljharb merged commit a08fbcc into jsx-eslint:main Aug 22, 2024
101 checks passed
@BillyLevin BillyLevin deleted the issue966 branch August 23, 2024 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants