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(eslint-plugin-template): accessibility-elements-content not allowing some attributes/inputs #397

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,18 @@ import {
createESLintRule,
getTemplateParserServices,
} from '../utils/create-eslint-rule';
import { isHiddenFromScreenReader } from '../utils/is-hidden-from-screen-reader';

type Options = [];
export type MessageIds = 'accessibilityElementsContent';
export const RULE_NAME = 'accessibility-elements-content';
const innerContentInputs: ReadonlySet<string> = new Set([
const safelistAttributes: ReadonlySet<string> = new Set([
'aria-label',
'innerHtml',
'innerHTML',
'innerText',
'outerHTML',
'title',
]);

export default createESLintRule<Options, MessageIds>({
Expand All @@ -19,30 +23,31 @@ export default createESLintRule<Options, MessageIds>({
type: 'suggestion',
docs: {
description:
'Ensures that the heading, anchor and button elements have content in it.',
'Ensures that the heading, anchor and button elements have content in it',
category: 'Best Practices',
recommended: false,
},
schema: [],
messages: {
accessibilityElementsContent: '<{{element}}/> should have content.',
accessibilityElementsContent: '<{{element}}> should have content',
},
},
defaultOptions: [],
create(context) {
const parserServices = getTemplateParserServices(context);

return {
'Element[name=/^(a|button|h1|h2|h3|h4|h5|h6)$/][children.length=0]'({
inputs,
name: element,
sourceSpan,
}: TmplAstElement) {
const hasInnerContent = inputs.some(({ name }) =>
innerContentInputs.has(name),
);
'Element[name=/^(a|button|h1|h2|h3|h4|h5|h6)$/][children.length=0]'(
node: TmplAstElement,
) {
if (isHiddenFromScreenReader(node)) return;

if (hasInnerContent) return;
const { attributes, inputs, name: element, sourceSpan } = node;
const hasAttributeSafelisted = [...attributes, ...inputs]
.map(({ name }) => name)
.some((inputName) => safelistAttributes.has(inputName));

if (hasAttributeSafelisted) return;

const loc = parserServices.convertNodeSourceSpanToLoc(sourceSpan);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
import type { TmplAstElement } from '@angular/compiler';
import { PROPERTY } from './constants';
import { getLiteralValue } from './get-literal-value';
import { getAttributeValue } from './get-attribute-value';
import { getLiteralValue } from './get-literal-value';

/**
* Returns boolean indicating that the aria-hidden prop
* is present or the value is true. Will also return true if
* there is an input with type='hidden'.
*
* <div aria-hidden /> is equivalent to the DOM as <div aria-hidden=true />.
* Whether an element has the `aria-hidden` property and its value is empty or
* `true`. It also returns `true` if the element is an `input` with `type="hidden"`.
*/
export function isHiddenFromScreenReader(node: any): boolean {
export function isHiddenFromScreenReader(node: TmplAstElement): boolean {
if (node.name.toUpperCase() === 'INPUT') {
const hidden = getAttributeValue(node, 'type');
if (typeof hidden === 'string' && hidden.toUpperCase() === 'HIDDEN') {
const typeAttributeValue = getAttributeValue(node, 'type');
if (
typeof typeAttributeValue === 'string' &&
typeAttributeValue.toUpperCase() === 'HIDDEN'
) {
return true;
}
}

const ariaHidden = getLiteralValue(getAttributeValue(node, 'aria-hidden'));
return ariaHidden === PROPERTY || ariaHidden === true;
return ariaHidden === '' || ariaHidden === PROPERTY || ariaHidden === true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,31 +27,39 @@ ruleTester.run(RULE_NAME, rule, {
'<a><app-content></app-content></a>',
'<a [innerHTML]="dangerouslySetHTML"></a>',
'<a [innerText]="text"></a>',
'<a [outerHTML]="text"></a>',
'<a aria-hidden></a>',
'<button [attr.aria-hidden]="true"></button>',
'<h5 [attr.aria-label]="text"></h5>',
'<h6 title="text"></h6>',
],
invalid: [
convertAnnotatedSourceToFailureCase({
messageId,
description: 'should fail with no content in heading tag',
annotatedSource: `
<h1 class="size-1"></h1>
~~~~~~~~~~~~~~~~~~~~~~~~
`,
messageId,
data: { element: 'h1' },
}),
convertAnnotatedSourceToFailureCase({
messageId,
description: 'should fail with no content in anchor tag',
annotatedSource: `
<a href="#" [routerLink]="['route1']"></a>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
`,
messageId,
data: { element: 'a' },
}),
convertAnnotatedSourceToFailureCase({
messageId,
description: 'should fail with no content in button tag',
annotatedSource: `
<button></button>
~~~~~~~~~~~~~~~~~
`,
messageId,
data: { element: 'button' },
}),
],
});
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ ruleTester.run(RULE_NAME, rule, {
{
// It should work when element has aria-hidden.
code: `
<div (click)="onClick()" aria-hidden"></div>
<div (click)="onClick()" aria-hidden="true"></div>
<div (click)="onClick()" [attr.aria-hidden]="true"></div>
<div (click)="onClick()" [attr.aria-hidden]="ariaHidden"></div>
Expand Down