-
Notifications
You must be signed in to change notification settings - Fork 11
Update a11y-link-in-text-block rule to include HTML anchor elements #345
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: cb31a75 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Co-authored-by: TylerJDev <[email protected]>
a11y-link-in-text-block
rule to include HTML anchor elements
@copilot, can we ignore HTML anchor elements that have |
I've updated the rule to ignore HTML anchor elements with
This follows the same pattern as the existing skip for anchors with Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
873d8c1
to
a4ad769
Compare
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.
Pull Request Overview
This PR extends the existing a11y-link-in-text-block
ESLint rule to also detect plain HTML <a>
elements in text blocks, auto-fix them to use Primer’s Link
component, and document/tests these changes.
- Added
isHTMLElement
check, detection logic, and autofix for HTML anchors in text blocks - Expanded tests to cover valid/invalid HTML anchor scenarios
- Updated rule documentation to describe new anchor conversion behavior
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
src/rules/a11y-link-in-text-block.js | Added HTML anchor detection in text blocks, autofix to Link |
src/rules/tests/a11y-link-in-text-block.test.js | Added valid/invalid cases for HTML <a> conversion |
docs/rules/a11y-link-in-text-block.md | Updated documentation and examples for HTML anchor conversion |
.changeset/odd-pumas-care.md | Bumped version and noted HTML anchor detection feature |
}) | ||
|
||
const index = siblings.findIndex(childNode => { | ||
return childNode.range === node.range |
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.
Comparing arrays with ===
always returns false; use object identity (e.g., childNode === node
) or compare numeric range values instead to correctly find the node’s index.
return childNode.range === node.range | |
return ( | |
childNode.range[0] === node.range[0] && | |
childNode.range[1] === node.range[1] | |
) |
Copilot uses AI. Check for mistakes.
const attributes = node.openingElement.attributes.map(attr => sourceCode.getText(attr)).join(' ') | ||
|
||
// Create the Link component opening and closing tags | ||
const openingTag = `<Link ${attributes}>` |
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.
When converting HTML anchors to Link
, the rule should add the inline
prop for consistency with the linkInTextBlock
rule; include inline
in the opening tag (e.g., <Link inline ${attributes}>
).
const openingTag = `<Link ${attributes}>` | |
const openingTag = `<Link inline ${attributes}>` |
Copilot uses AI. Check for mistakes.
|
||
There are certain edge cases that the linter skips to avoid false positives including: | ||
|
||
- `<Link className="...">` because there may be distinguishing styles applied. | ||
- `<Link className="...">` or `<a className="...">` because there may be distinguishing styles applied. |
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.
Documentation should also mention that <a>
elements with an id
attribute are skipped by the rule, since the code filters on id
similarly to className
.
- `<Link className="...">` or `<a className="...">` because there may be distinguishing styles applied. | |
- `<Link className="...">`, `<a className="...">`, or `<a id="...">` because there may be distinguishing styles or behaviors applied. |
Copilot uses AI. Check for mistakes.
This PR adds support for detecting HTML anchor elements (
<a>
) in text blocks to the existinga11y-link-in-text-block
rule. The rule now recommends converting HTML anchors toLink
components from@primer/react
for consistent styling and accessibility.Changes
Link
componentsExamples
Before (Flagged by this rule):
After autofix:
Similar to the existing check for
Link
components, the rule skips certain cases to avoid false positives:className
attributeFixes #344.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.