fix: warn when implicit label has no visible text#1864
fix: warn when implicit label has no visible text#1864ShivaniNR wants to merge 3 commits intohtmlhint:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the input-requires-label rule to ensure that labels with nested inputs contain descriptive, non-whitespace text. While the changes introduce a text listener and updated tests, a logic bug was identified where the use of a single boolean flag incorrectly tracks text presence across multiple or nested labels. The reviewer provides actionable feedback and code suggestions to replace this logic with a state stack, ensuring correct validation for each label's scope.
There was a problem hiding this comment.
Pull request overview
Updates the input-requires-label rule to warn when an implicit <label> (wrapping an <input>) contains no non-whitespace text, closing an accessibility gap introduced when implicit-label support was added in #1852.
Changes:
- Track per-implicit-label state (text presence + input range) and mark wrapped inputs as unlabeled when the label has no text.
- Listen to parser
textevents to detect non-whitespace label content (including within child elements). - Expand test coverage for empty/whitespace-only implicit labels and multi-label/nested-label regressions.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/rules/input-requires-label.spec.js | Updates/extends specs to cover implicit labels with/without visible text. |
| src/core/rules/input-requires-label.ts | Adds label state stack + text listener to detect empty implicit labels and trigger existing warnings. |
| dist/core/rules/input-requires-label.js | Compiled output reflecting the TypeScript rule changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
PR looking really good. thanks for your work on this! Just one comment left from Copilot to review I think. |
Thanks! Moved the two tests to the Error cases block as Copilot suggested. |
Short description of what this resolves
Closes #1860
An implicit label wrapping an input (
<label><input /></label>) without any visible text is inaccessible; screen readers find the input but have no name to announce. After #1852 added implicit label support, this case was silently accepted.Proposed changes
labelHasTextflag that resets when an implicit<label>openstextevent listener that sets the flag when non-whitespace text is found inside the label</label>close, if no text was found, nested inputs are marked as unlabeled, so the existing warning fires