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

feat(rule): no distracting elements should be used #760

Merged
merged 1 commit into from
Feb 15, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export { Rule as TemplatesAccessibilityAnchorContentRule } from './templateAcces
export { Rule as TemplateClickEventsHaveKeyEventsRule } from './templateClickEventsHaveKeyEventsRule';
export { Rule as TemplateAccessibilityAltTextRule } from './templateAccessibilityAltTextRule';
export { Rule as TemplateAccessibilityTableScopeRule } from './templateAccessibilityTableScopeRule';
export { Rule as TemplateNoDistractingElementsRule } from './templateNoDistractingElementsRule';
export { Rule as TemplatesNoNegatedAsync } from './templatesNoNegatedAsyncRule';
export { Rule as TemplateNoAutofocusRule } from './templateNoAutofocusRule';
export { Rule as TrackByFunctionRule } from './trackByFunctionRule';
Expand Down
51 changes: 51 additions & 0 deletions src/templateNoDistractingElementsRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { ElementAst } from '@angular/compiler';
import { sprintf } from 'sprintf-js';
import { IRuleMetadata, RuleFailure, Rules } from 'tslint/lib';
import { SourceFile } from 'typescript/lib/typescript';
import { NgWalker } from './angular/ngWalker';
import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor';

export class Rule extends Rules.AbstractRule {
static readonly metadata: IRuleMetadata = {
description: 'Enforces that no distracting elements are used',
options: null,
optionsDescription: 'Not configurable.',
rationale: 'Elements that can be visually distracting can cause accessibility issues with visually impaired users.',
ruleName: 'template-no-distracting-elements',
type: 'functionality',
typescriptOnly: true
};

static readonly FAILURE_STRING = 'Avoid using <%s/> elements as they create visual accessibility issues.';

apply(sourceFile: SourceFile): RuleFailure[] {
return this.applyWithWalker(
new NgWalker(sourceFile, this.getOptions(), {
templateVisitorCtrl: TemplateNoDistractingElementsVisitor
})
);
}
}

export function getFailureMessage(element: string) {
return sprintf(Rule.FAILURE_STRING, element);
}

class TemplateNoDistractingElementsVisitor extends BasicTemplateAstVisitor {
visitElement(prop: ElementAst, context: any): any {
this.validateElement(prop);
super.visitElement(prop, context);
}

private validateElement(el: ElementAst): void {
if (el.name === 'marquee' || el.name === 'blink') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

When I see this link, I'm not sure that we have to test 'blink'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think its fine, since all the accessibility checkers and linters check for it. Sometimes developers use code generated or from old references.

const {
sourceSpan: {
end: { offset: endOffset },
start: { offset: startOffset }
}
} = el;
this.addFailureFromStartToEnd(startOffset, endOffset, getFailureMessage(el.name));
}
}
}
58 changes: 58 additions & 0 deletions test/templateNoDistractingElementsRule.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { getFailureMessage, Rule } from '../src/templateNoDistractingElementsRule';
import { assertAnnotated, assertSuccess } from './testHelper';

const {
metadata: { ruleName }
} = Rule;

describe(ruleName, () => {
describe('failure', () => {
it('should fail when distracting element marquee is used', () => {
const source = `
@Component({
template: \`
<marquee></marquee>
~~~~~~~~~
\`
})
class Bar {}
`;
assertAnnotated({
message: getFailureMessage('marquee'),
ruleName,
source
});
});

it('should fail when distracting element blink is used', () => {
const source = `
@Component({
template: \`
<blink></blink>
~~~~~~~
\`
})
class Bar {}
`;
assertAnnotated({
message: getFailureMessage('blink'),
ruleName,
source
});
});
});

describe('success', () => {
it('should work when distracting element is not used', () => {
const source = `
@Component({
template: \`
<div>Valid</div>
\`
})
class Bar {}
`;
assertSuccess(ruleName, source);
});
});
});