Skip to content

Commit

Permalink
Merge pull request #90 from Harsh-Modi278/user/harshmodi/SpinnerLintRule
Browse files Browse the repository at this point in the history
feat: Add lint rule for Spinner component
  • Loading branch information
aubreyquinn authored Sep 17, 2024
2 parents 5728487 + c3a18ba commit ca0e89d
Show file tree
Hide file tree
Showing 7 changed files with 218 additions and 5 deletions.
2 changes: 1 addition & 1 deletion COVERAGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ We currently cover the following components:
- [N/A] Skeleton
- [N/A] SkeletonItem
- [x] SpinButton
- [] Spinner
- [x] Spinner
- [] SwatchPicker
- [x] Switch
- [] Table
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ Any use of third-party trademarks or logos are subject to those third-party's po
| [radiogroup-missing-label](docs/rules/radiogroup-missing-label.md) | Accessibility: RadioGroup without label must have an accessible and visual label: aria-labelledby | ✅ | | |
| [spin-button-needs-labelling](docs/rules/spin-button-needs-labelling.md) | Accessibility: SpinButtons must have an accessible label | ✅ | | |
| [spin-button-unrecommended-labelling](docs/rules/spin-button-unrecommended-labelling.md) | Accessibility: Unrecommended accessibility labelling - SpinButton | ✅ | | |
| [spinner-needs-labelling](docs/rules/spinner-needs-labelling.md) | Accessibility: Spinner must have either aria-label or label, aria-live and aria-busy attributes | ✅ | | |
| [switch-needs-labelling](docs/rules/switch-needs-labelling.md) | Accessibility: Switch must have an accessible label | ✅ | | |
| [text-area-missing-label](docs/rules/text-area-missing-label.md) | Accessibility: Textarea must have an accessible name | ✅ | | |
| [toolbar-missing-aria](docs/rules/toolbar-missing-aria.md) | Accessibility: Toolbars need accessible labelling: aria-label or aria-labelledby | ✅ | | |
Expand Down
89 changes: 89 additions & 0 deletions docs/rules/spinner-needs-labelling.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# Accessibility: Spinner must have either aria-label or label, aria-live and aria-busy attributes (`@microsoft/fluentui-jsx-a11y/spinner-needs-labelling`)

💼 This rule is enabled in the ✅ `recommended` config.

<!-- end auto-generated rule header -->

Spinner must have either aria-label or label, aria-live and aria-busy attributes.

<https://www.w3.org/TR/html-aria/>

<https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-live>

<https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-label>

## Ways to fix

- Make sure that Spinner component has following attributes:
- aria-live
- aria-busy
- either label or aria-label

## Rule Details

This rule aims to make Spinners accessible.

Examples of **incorrect** code for this rule:

```jsx
<Spinner {...props} />
```

```jsx
<Spinner
{...props}
aria-label="some text"
/>
```

```jsx
<Spinner
{...props}
aria-live="polite"
/>
```

```jsx
<Spinner
size="large"
label="Large Spinner"
aria-live="polite"
/>
```

```jsx
<Spinner
size="large"
label="Large Spinner"
aria-busy="true"
/>
```

Examples of **correct** code for this rule:

```jsx
<Spinner
{...props}
aria-label="my screen reader text"
aria-live="polite"
aria-busy="false"
/>
```

```jsx
<Spinner
{...props}
aria-label="my screen reader text"
aria-live="polite"
aria-busy="true"
/>
```

```jsx
<Spinner
{...props}
label="my text"
aria-live="polite"
aria-busy="true"
/>
```
8 changes: 5 additions & 3 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ module.exports = {
"radiogroup-missing-label": require("./rules/radiogroup-missing-label"),
"prefer-aria-over-title-attribute": require("./rules/prefer-aria-over-title-attribute"),
"dialogbody-needs-title-content-and-actions": require("./rules/dialogbody-needs-title-content-and-actions"),
"dialogsurface-needs-aria": require("./rules/dialogsurface-needs-aria")
"dialogsurface-needs-aria": require("./rules/dialogsurface-needs-aria"),
"spinner-needs-labelling": require("./rules/spinner-needs-labelling")
},
configs: {
recommended: {
Expand Down Expand Up @@ -67,8 +68,8 @@ module.exports = {
"@microsoft/fluentui-jsx-a11y/radiogroup-missing-label": "error",
"@microsoft/fluentui-jsx-a11y/prefer-aria-over-title-attribute": "warn",
"@microsoft/fluentui-jsx-a11y/dialogbody-needs-title-content-and-actions": "error",
"@microsoft/fluentui-jsx-a11y/dialogsurface-needs-aria": "error"

"@microsoft/fluentui-jsx-a11y/dialogsurface-needs-aria": "error",
"@microsoft/fluentui-jsx-a11y/spinner-needs-labelling": "error"
}
}
}
Expand All @@ -78,3 +79,4 @@ module.exports = {
module.exports.processors = {
// add your processors here
};

55 changes: 55 additions & 0 deletions lib/rules/spinner-needs-labelling.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

"use strict";

const { hasNonEmptyProp } = require("../util/hasNonEmptyProp");
const elementType = require("jsx-ast-utils").elementType;

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = {
meta: {
// possible error messages for the rule
messages: {
noUnlabelledSpinner: "Accessibility: Spinner must have either aria-label or label, aria-live and aria-busy attributes"
},
// "problem" means the rule is identifying code that either will cause an error or may cause a confusing behavior: https://eslint.org/docs/latest/developer-guide/working-with-rules
type: "problem",
// docs for the rule
docs: {
description: "Accessibility: Spinner must have either aria-label or label, aria-live and aria-busy attributes",
recommended: true,
url: "https://www.w3.org/TR/html-aria/" // URL to the documentation page for this rule
},
schema: []
},
// create (function) returns an object with methods that ESLint calls to “visit” nodes while traversing the abstract syntax tree
create(context) {
return {
// visitor functions for different types of nodes
JSXOpeningElement(node) {
// if it is not a Spinner, return
if (elementType(node) !== "Spinner") {
return;
}

if (hasNonEmptyProp(node.attributes, "aria-busy")
&& hasNonEmptyProp(node.attributes, "aria-live")
&& (hasNonEmptyProp(node.attributes, "label") || hasNonEmptyProp(node.attributes, "aria-label"))
) {
return;
}

// if it has no visual labelling, report error
context.report({
node,
messageId: `noUnlabelledSpinner`
});
}
};
}
};

16 changes: 15 additions & 1 deletion lib/util/hasNonEmptyProp.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,21 @@ var getProp = require("jsx-ast-utils").getProp;
* @returns boolean
*/
function hasNonEmptyProp(attributes, name) {
return hasProp(attributes, name) && getPropValue(getProp(attributes, name)).trim().length > 0;
if (!hasProp(attributes, name)) {
return false;
}

const propValue = getPropValue(getProp(attributes, name));

/**
* getPropValue internally normalizes "true", "false" to boolean values.
* So it is sufficent to check if the prop exists and return.
*/
if (typeof propValue === "boolean") {
return true;
}

return propValue.trim().length > 0;
}

module.exports.hasNonEmptyProp = hasNonEmptyProp;
52 changes: 52 additions & 0 deletions tests/lib/rules/spinner-needs-labelling.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const rule = require("../../../lib/rules/spinner-needs-labelling"),
RuleTester = require("eslint").RuleTester;

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const ruleTester = new RuleTester();
ruleTester.run("spinner-needs-labelling", rule, {
valid: [
`<Spinner aria-label="my screen reader text" aria-live="polite" aria-busy="false" />`,
`<Spinner appearance="primary" label="Primary Spinner" aria-label="his screen reader text" aria-live="polite" aria-busy="true" />`,
`<Spinner appearance="primary" label="Secondary Spinner" aria-live="polite" aria-busy="false" />`,
`<Spinner appearance="primary" aria-label="your screen reader text" aria-live="polite" aria-busy="true" />`,
],
invalid: [
{
code: `<Spinner />`,
errors: [{ messageId: "noUnlabelledSpinner" }]
},
{
code: `<Spinner appearance="primary" label="Primary Spinner" />`,
errors: [{ messageId: "noUnlabelledSpinner" }]
},
{
code: `<Spinner size="large" aria-label="some text" />`,
errors: [{ messageId: "noUnlabelledSpinner" }]
},
{
code: `<Spinner size="large" label="Large Spinner" aria-live="polite" />`,
errors: [{ messageId: "noUnlabelledSpinner" }]
},
{
code: `<Spinner size="large" label="Large Spinner" aria-busy="true" />`,
errors: [{ messageId: "noUnlabelledSpinner" }]
},
{
code : `<Spinner aria-label="my screen reader text" aria-live="polite" />`,
errors: [{ messageId: "noUnlabelledSpinner" }]
}
]
});

0 comments on commit ca0e89d

Please sign in to comment.