-
Notifications
You must be signed in to change notification settings - Fork 10
Add rule to prevent duplicate labels on TextInput #366
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
Merged
Merged
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'eslint-plugin-primer-react': patch | ||
--- | ||
|
||
Add `a11y-no-duplicate-form-labels` rule to prevent duplicate labels on TextInput components. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
## Rule Details | ||
|
||
This rule prevents accessibility issues by ensuring form controls have only one label. When a `FormControl` contains both a `FormControl.Label` and a `TextInput` with an `aria-label`, it creates duplicate labels which can confuse screen readers and other assistive technologies. | ||
|
||
👎 Examples of **incorrect** code for this rule: | ||
|
||
```jsx | ||
import {FormControl, TextInput} from '@primer/react' | ||
|
||
function ExampleComponent() { | ||
return ( | ||
// TextInput has aria-label when FormControl.Label is present | ||
<FormControl> | ||
<FormControl.Label>Form Input Label</FormControl.Label> | ||
<TextInput aria-label="Form Input Label" /> | ||
</FormControl> | ||
) | ||
} | ||
``` | ||
|
||
👍 Examples of **correct** code for this rule: | ||
|
||
```jsx | ||
import {FormControl, TextInput} from '@primer/react' | ||
|
||
function ExampleComponent() { | ||
return ( | ||
<> | ||
{/* TextInput without aria-label when FormControl.Label is present */} | ||
<FormControl> | ||
<FormControl.Label>Form Input Label</FormControl.Label> | ||
<TextInput /> | ||
</FormControl> | ||
|
||
{/* TextInput with aria-label when no FormControl.Label is present */} | ||
<FormControl> | ||
<TextInput aria-label="Form Input Label" /> | ||
</FormControl> | ||
|
||
{/* Using visuallyHidden FormControl.Label without aria-label */} | ||
<FormControl> | ||
<FormControl.Label visuallyHidden>Form Input Label</FormControl.Label> | ||
<TextInput /> | ||
</FormControl> | ||
</> | ||
) | ||
} | ||
``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
112 changes: 112 additions & 0 deletions
112
src/rules/__tests__/a11y-no-duplicate-form-labels.test.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
const rule = require('../a11y-no-duplicate-form-labels') | ||
const {RuleTester} = require('eslint') | ||
|
||
const ruleTester = new RuleTester({ | ||
parserOptions: { | ||
ecmaVersion: 'latest', | ||
sourceType: 'module', | ||
ecmaFeatures: { | ||
jsx: true, | ||
}, | ||
}, | ||
}) | ||
|
||
ruleTester.run('a11y-no-duplicate-form-labels', rule, { | ||
valid: [ | ||
// TextInput without aria-label is valid | ||
`import {FormControl, TextInput} from '@primer/react'; | ||
<FormControl> | ||
<FormControl.Label>Form Input Label</FormControl.Label> | ||
<TextInput /> | ||
</FormControl>`, | ||
|
||
// TextInput with aria-label but no FormControl.Label is valid | ||
`import {FormControl, TextInput} from '@primer/react'; | ||
<FormControl> | ||
<TextInput aria-label="Form Input Label" /> | ||
</FormControl>`, | ||
|
||
// TextInput with aria-label outside FormControl is valid | ||
`import {TextInput} from '@primer/react'; | ||
<TextInput aria-label="Form Input Label" />`, | ||
|
||
// TextInput with visuallyHidden FormControl.Label is valid | ||
`import {FormControl, TextInput} from '@primer/react'; | ||
<FormControl> | ||
<FormControl.Label visuallyHidden>Form Input Label</FormControl.Label> | ||
<TextInput /> | ||
</FormControl>`, | ||
|
||
// FormControl without FormControl.Label but with aria-label is valid | ||
`import {FormControl, TextInput} from '@primer/react'; | ||
<FormControl> | ||
<TextInput aria-label="Form Input Label" /> | ||
</FormControl>`, | ||
|
||
// Multiple TextInputs with different approaches | ||
`import {FormControl, TextInput} from '@primer/react'; | ||
<div> | ||
<FormControl> | ||
<FormControl.Label>Visible Label</FormControl.Label> | ||
<TextInput /> | ||
</FormControl> | ||
<FormControl> | ||
<TextInput aria-label="Standalone Input" /> | ||
</FormControl> | ||
</div>`, | ||
], | ||
invalid: [ | ||
{ | ||
code: `import {FormControl, TextInput} from '@primer/react'; | ||
<FormControl> | ||
<FormControl.Label>Form Input Label</FormControl.Label> | ||
<TextInput aria-label="Form Input Label" /> | ||
</FormControl>`, | ||
errors: [ | ||
{ | ||
messageId: 'duplicateLabel', | ||
}, | ||
], | ||
}, | ||
{ | ||
code: `import {FormControl, TextInput} from '@primer/react'; | ||
<FormControl> | ||
<FormControl.Label>Username</FormControl.Label> | ||
<TextInput aria-label="Enter your username" /> | ||
</FormControl>`, | ||
errors: [ | ||
{ | ||
messageId: 'duplicateLabel', | ||
}, | ||
], | ||
}, | ||
{ | ||
code: `import {FormControl, TextInput} from '@primer/react'; | ||
<FormControl> | ||
<FormControl.Label visuallyHidden>Password</FormControl.Label> | ||
<TextInput aria-label="Enter password" /> | ||
</FormControl>`, | ||
errors: [ | ||
{ | ||
messageId: 'duplicateLabel', | ||
}, | ||
], | ||
}, | ||
{ | ||
code: `import {FormControl, TextInput} from '@primer/react'; | ||
<div> | ||
<FormControl> | ||
<FormControl.Label>Email</FormControl.Label> | ||
<div> | ||
<TextInput aria-label="Email address" /> | ||
</div> | ||
</FormControl> | ||
</div>`, | ||
errors: [ | ||
{ | ||
messageId: 'duplicateLabel', | ||
}, | ||
], | ||
}, | ||
], | ||
}) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
const {isPrimerComponent} = require('../utils/is-primer-component') | ||
const {getJSXOpeningElementName} = require('../utils/get-jsx-opening-element-name') | ||
const {getJSXOpeningElementAttribute} = require('../utils/get-jsx-opening-element-attribute') | ||
|
||
const isFormControl = node => getJSXOpeningElementName(node) === 'FormControl' | ||
const isFormControlLabel = node => getJSXOpeningElementName(node) === 'FormControl.Label' | ||
const isTextInput = node => getJSXOpeningElementName(node) === 'TextInput' | ||
|
||
const hasAriaLabel = node => { | ||
const ariaLabel = getJSXOpeningElementAttribute(node, 'aria-label') | ||
return !!ariaLabel | ||
} | ||
|
||
const findFormControlLabel = (node, sourceCode) => { | ||
// Traverse up the parent chain to find FormControl | ||
let current = node.parent | ||
while (current) { | ||
if ( | ||
current.type === 'JSXElement' && | ||
isFormControl(current.openingElement) && | ||
isPrimerComponent(current.openingElement.name, sourceCode.getScope(current)) | ||
) { | ||
// Found FormControl, now check if it has a FormControl.Label child | ||
return current.children.some( | ||
child => | ||
child.type === 'JSXElement' && | ||
isFormControlLabel(child.openingElement) && | ||
isPrimerComponent(child.openingElement.name, sourceCode.getScope(child)), | ||
) | ||
} | ||
current = current.parent | ||
} | ||
return false | ||
} | ||
|
||
module.exports = { | ||
meta: { | ||
type: 'problem', | ||
docs: { | ||
description: | ||
'Prevent duplicate labels on form inputs by disallowing aria-label on TextInput when FormControl.Label is present.', | ||
url: require('../url')(module), | ||
}, | ||
schema: [], | ||
messages: { | ||
duplicateLabel: | ||
'TextInput should not have aria-label when FormControl.Label is present. Use FormControl.Label with visuallyHidden prop if needed.', | ||
}, | ||
}, | ||
create(context) { | ||
const sourceCode = context.sourceCode ?? context.getSourceCode() | ||
return { | ||
JSXOpeningElement(jsxNode) { | ||
if (isPrimerComponent(jsxNode.name, sourceCode.getScope(jsxNode)) && isTextInput(jsxNode)) { | ||
// Check if TextInput has aria-label | ||
if (hasAriaLabel(jsxNode)) { | ||
// Check if there's a FormControl.Label in the parent FormControl | ||
if (findFormControlLabel(jsxNode, sourceCode)) { | ||
context.report({ | ||
node: jsxNode, | ||
messageId: 'duplicateLabel', | ||
}) | ||
} | ||
} | ||
} | ||
}, | ||
} | ||
}, | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could make this
minor