Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Mouse events have key events rule #849

Merged
Merged
Show file tree
Hide file tree
Changes from 8 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
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,15 @@ We recommend you specify exact versions of lint libraries, including `tslint-mic
</td>
<td>2.0.11</td>
</tr>
<tr>
<td>
<code>react-a11y-mouse-event-has-key-event</code>
</td>
<td>
For accessibility of your website, elements with mouseOver/mouseOut should be accompanied by onFocus/onBlur keyboard events.
</td>
<td>@next</td>
</tr>
<tr>
<td>
<code>react-a11y-no-onchange</code>
Expand Down
1 change: 1 addition & 0 deletions configs/latest.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"rulesDirectory": ["../"],
"rules": {
"react-a11y-iframes": true,
"react-a11y-mouse-event-has-key-event": true,
"void-zero": true
}
}
111 changes: 111 additions & 0 deletions src/reactA11yMouseEventHasKeyEventRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import * as ts from 'typescript';
import * as Lint from 'tslint';
import * as tsutils from 'tsutils';

import { ExtendedMetadata } from './utils/ExtendedMetadata';
import { getJsxAttributesFromJsxElement } from './utils/JsxAttribute';

const MOUSE_EVENTS: {
onMouseOver: {
value: 'onmouseover';
jsxValue: 'onMouseOver';
};
onMouseOut: {
value: 'onmouseout';
jsxValue: 'onMouseOut';
};
} = {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
onMouseOver: {
value: 'onmouseover',
jsxValue: 'onMouseOver'
},
onMouseOut: {
value: 'onmouseout',
jsxValue: 'onMouseOut'
}
};

const FOCUS_EVENTS: {
onFocus: {
value: 'onfocus';
jsxValue: 'onFocus';
};
onBlur: {
value: 'onblur';
jsxValue: 'onBlur';
};
} = {
onFocus: {
value: 'onfocus',
jsxValue: 'onFocus'
},
onBlur: {
value: 'onblur',
jsxValue: 'onBlur'
}
};

type MouseEvents = keyof typeof MOUSE_EVENTS;
type FocusEvents = keyof typeof FOCUS_EVENTS;
type AttributeType = { [propName: string]: ts.JsxAttribute };
interface CheckMouseEventArgs {
mouseEvent: typeof MOUSE_EVENTS.onMouseOver | typeof MOUSE_EVENTS.onMouseOut;
focusEvent: typeof FOCUS_EVENTS.onBlur | typeof FOCUS_EVENTS.onFocus;
node: ts.Node;
ctx: Lint.WalkContext<void>;
}

function getFailureString(mouseEvent: MouseEvents, focusEvent: FocusEvents) {
return `${mouseEvent} must be accompanied by ${focusEvent}.`;
}

export class Rule extends Lint.Rules.AbstractRule {
public static metadata: ExtendedMetadata = {
ruleName: 'react-a11y-mouse-event-has-key-event',
type: 'maintainability',
description:
'For accessibility of your website, elements with mouseOver/mouseOut should be accompanied by onFocus/onBlur keyboard events.',
rationale: `References:
<ul>
<li><a href="http://oaa-accessibility.org/wcag20/rule/59/">Focusable elements with mouseOver should also have onFocus event handlers.</a></li>
<li><a href="http://oaa-accessibility.org/wcag20/rule/60/">Focusable elements with onMouseOut should also have onBlur event handlers.</a></li>
</ul>`,
options: null, // tslint:disable-line:no-null-keyword
optionsDescription: '',
typescriptOnly: true,
issueClass: 'Non-SDL',
issueType: 'Error',
severity: 'Important',
level: 'Opportunity for Excellence',
group: 'Accessibility'
};

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithFunction(sourceFile, walk);
}
}
function checkMouseEventForFocus({ mouseEvent, focusEvent, node, ctx }: CheckMouseEventArgs): void {
const attributes: AttributeType = getJsxAttributesFromJsxElement(node);

if (attributes === undefined) {
return;
}

const attributeKeys = new Set(Object.keys(attributes));
if (attributeKeys.has(mouseEvent.value) && !attributeKeys.has(focusEvent.value)) {
const errorMessage = getFailureString(mouseEvent.jsxValue, focusEvent.jsxValue);
ctx.addFailureAt(node.getStart(), node.getWidth(), errorMessage);
}
}

function walk(ctx: Lint.WalkContext<void>) {
function cb(node: ts.Node): void {
if (tsutils.isJsxSelfClosingElement(node) || tsutils.isJsxOpeningElement(node)) {
checkMouseEventForFocus({ mouseEvent: MOUSE_EVENTS.onMouseOver, focusEvent: FOCUS_EVENTS.onFocus, node, ctx });
checkMouseEventForFocus({ mouseEvent: MOUSE_EVENTS.onMouseOut, focusEvent: FOCUS_EVENTS.onBlur, node, ctx });
}
return ts.forEachChild(node, cb);
}

return ts.forEachChild(ctx.sourceFile, cb);
}
17 changes: 17 additions & 0 deletions tests/react-a11y-mouse-event-has-key-event/test.tsx.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import * as React from 'react'
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved

const element = (<div onMouseOver={() => {}}>click</div>)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [onMouseOver must be accompanied by onFocus.]
const elementTwo = (<div onMouseOut={() => {}}>click</div>)
~~~~~~~~~~~~~~~~~~~~~~~~~~~ [onMouseOut must be accompanied by onBlur.]
const elementSelfClosing = (<div onMouseOut={() => {}}/>)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [onMouseOut must be accompanied by onBlur.]

const Foo = (<div onMouseOver={() => {}} {...props} />)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [onMouseOver must be accompanied by onFocus.]
const Bar = (<div onMouseOut={() => {}} {...props} />)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [onMouseOut must be accompanied by onBlur.]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Complains for now.

Choose a reason for hiding this comment

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

@lizzzp1 could you change it to not complain if there's a ..., actually? It would be annoying to have a ...props that includes the correct corresponding onBlue/onFocus and still have this complain.

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 thought that required the type checker that was a follow up? I'm really confused now.

Choose a reason for hiding this comment

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

The type checker is just required to know whether the ...props contains the corresponding onBlur/onFocus. Without the type checker, we have know way of knowing whether a JSX element that contains ...props violates the rule or not. Until the type checker logic is added, that leaves us with two options:

  • Ignore ...props _(pretend they aren't therE): the rule would give complaints in some places where it shouldn't
  • Assume ...props contains the right attributes: the rule wouldn't give complaints in some places where it should

In order to not irritate folks with false complaints, IMO we should err on the side of caution with the first option.

Does that seem reasonable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, if props are ignored then it would complain like the above wouldn't it? Wouldn't the 2nd option be better so it doesn't complain if it encounters spread props?

Choose a reason for hiding this comment

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

Oh agh sorry I meant to say the second option! You're absolutely correct 😅

const Baz = <div />
const Foobar = <div {...props} />
const elementWithFocus = <div onMouseOut={() => {}} onBlur={() => {}}>click</div>
const elementWithFocusTwo = <div onMouseOver={() => {}} onFocus={() => {}}>click</div>
5 changes: 5 additions & 0 deletions tests/react-a11y-mouse-event-has-key-event/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"react-a11y-mouse-event-has-key-event": true
}
}
1 change: 1 addition & 0 deletions tslint-warnings.csv
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ react-a11y-image-button-has-alt,Enforce that inputs element with type="image" mu
react-a11y-img-has-alt,"Enforce that an img element contains the non-empty alt attribute. For decorative images, using empty alt attribute and role="presentation".",TSLINT1OM69KS,tslint,Non-SDL,Warning,Important,Opportunity for Excellence,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,,
react-a11y-input-elements,"For accessibility of your website, HTML input boxes and text areas must include default, place-holding characters.",TSLINTT7DC6U,tslint,Non-SDL,Warning,Moderate,Opportunity for Excellence,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,,
react-a11y-lang,"For accessibility of your website, html elements must have a valid lang attribute.",TSLINTQ046RM,tslint,Non-SDL,Warning,Low,Opportunity for Excellence,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,,
react-a11y-mouse-event-has-key-event,"For accessibility of your website, elements with mouseOver/mouseOut should be accompanied by onFocus/onBlur keyboard events.",TSLINT2DDJKM,tslint,Non-SDL,Error,Important,Opportunity for Excellence,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,,
react-a11y-no-onchange,"For accessibility of your website, enforce usage of onBlur over onChange on select menus.",TSLINTNO0TDD,tslint,Non-SDL,Warning,Important,Opportunity for Excellence,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,,
react-a11y-props,Enforce all `aria-*` attributes are valid. Elements cannot use an invalid `aria-*` attribute.,TSLINT1682S78,tslint,Non-SDL,Warning,Important,Opportunity for Excellence,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,,
react-a11y-proptypes,Enforce ARIA state and property values are valid.,TSLINT1DLB1JE,tslint,Non-SDL,Warning,Important,Opportunity for Excellence,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,,
Expand Down
1 change: 1 addition & 0 deletions tslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@
"react-a11y-input-elements": true,
"react-a11y-lang": true,
"react-a11y-meta": true,
"react-a11y-mouse-event-has-key-event": true,
"react-a11y-no-onchange": true,
"react-a11y-props": true,
"react-a11y-proptypes": true,
Expand Down