Skip to content

Commit 0a8a70e

Browse files
aubreyquinnaubreyquinn
authored
Merge pull request #157 from microsoft/users/sidhshar/feature-prefer-disabledfocusable--with-loading-state
Prefer 'disabledFocusable' over 'disabled' when component has loading state to maintain keyboard navigation accessibility
2 parents 80724a6 + bbedacb commit 0a8a70e

File tree

9 files changed

+933
-0
lines changed

9 files changed

+933
-0
lines changed

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ Any use of third-party trademarks or logos are subject to those third-party's po
134134
| [no-empty-buttons](docs/rules/no-empty-buttons.md) | Accessibility: Button, ToggleButton, SplitButton, MenuButton, CompoundButton must either text content or icon or child component | ✅ | | |
135135
| [no-empty-components](docs/rules/no-empty-components.md) | FluentUI components should not be empty | ✅ | | |
136136
| [prefer-aria-over-title-attribute](docs/rules/prefer-aria-over-title-attribute.md) | The title attribute is not consistently read by screen readers, and its behavior can vary depending on the screen reader and the user's settings. | | ✅ | 🔧 |
137+
| [prefer-disabledfocusable-over-disabled](docs/rules/prefer-disabledfocusable-over-disabled.md) | Prefer 'disabledFocusable' over 'disabled' when component has loading state to maintain keyboard navigation accessibility | | ✅ | 🔧 |
137138
| [progressbar-needs-labelling](docs/rules/progressbar-needs-labelling.md) | Accessibility: Progressbar must have aria-valuemin, aria-valuemax, aria-valuenow, aria-describedby and either aria-label or aria-labelledby attributes | ✅ | | |
138139
| [radio-button-missing-label](docs/rules/radio-button-missing-label.md) | Accessibility: Radio button without label must have an accessible and visual label: aria-labelledby | ✅ | | |
139140
| [radiogroup-missing-label](docs/rules/radiogroup-missing-label.md) | Accessibility: RadioGroup without label must have an accessible and visual label: aria-labelledby | ✅ | | |

‎docs/rules/prefer-disabledfocusable-over-disabled.md‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
# Prefer 'disabledFocusable' over 'disabled' when component has loading state to maintain keyboard navigation accessibility (`@microsoft/fluentui-jsx-a11y/prefer-disabledfocusable-over-disabled`)
2+
3+
⚠️ This rule _warns_ in the ✅ `recommended` config.
4+
5+
🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).
6+
7+
<!-- end auto-generated rule header -->
8+
9+
When components are in a loading state, prefer using `disabledFocusable` over `disabled` to maintain proper keyboard navigation flow and accessibility.
10+
11+
## Rule Details
12+
13+
This rule encourages the use of `disabledFocusable` instead of `disabled` when components have loading state indicators. This ensures:
14+
15+
1. **Keyboard Navigation**: The component remains in the keyboard tab order, allowing users to discover and navigate to it
16+
2. **Screen Reader Compatibility**: Screen reader users can still navigate to and understand the component's state
17+
3. **Loading State Awareness**: Users understand that the component is temporarily unavailable due to loading, not permanently disabled
18+
4. **Consistent UX**: Provides a more predictable and accessible user experience
19+
20+
### Accessibility Impact
21+
22+
- `disabled` removes elements completely from the tab order (tabindex="-1")
23+
- `disabledFocusable` keeps elements in the tab order while conveying disabled state via `aria-disabled="true"`
24+
- Loading states are temporary conditions where users benefit from knowing the component exists and will become available
25+
26+
### Applicable Components
27+
28+
This rule applies to FluentUI components that support both `disabled` and `disabledFocusable` props:
29+
30+
**Button Components:** `Button`, `ToggleButton`, `CompoundButton`, `MenuButton`, `SplitButton`
31+
**Form Controls:** `Checkbox`, `Radio`, `Switch`
32+
**Input Components:** `Input`, `Textarea`, `Combobox`, `Dropdown`, `SpinButton`, `Slider`, `DatePicker`, `TimePicker`
33+
**Other Interactive:** `Link`, `Tab`
34+
35+
### Loading State Indicators
36+
37+
The rule detects these loading-related props:
38+
- `loading`
39+
- `isLoading`
40+
- `pending`
41+
- `isPending`
42+
- `busy`
43+
- `isBusy`
44+
45+
## Examples
46+
47+
### ❌ Incorrect
48+
49+
```jsx
50+
<Button disabled loading>Submit</Button>
51+
<ToggleButton disabled isLoading />
52+
<Checkbox disabled pending />
53+
<Input disabled={true} busy={isBusy} />
54+
<SpinButton disabled={isDisabled} loading={isSubmitting} />
55+
<Combobox disabled pending />
56+
```
57+
58+
### ✅ Correct
59+
60+
```jsx
61+
<Button disabledFocusable loading>Submit</Button>
62+
<ToggleButton disabledFocusable isLoading />
63+
<Checkbox disabledFocusable pending />
64+
<Input disabledFocusable={true} busy={isBusy} />
65+
<SpinButton disabledFocusable={isDisabled} loading={isSubmitting} />
66+
<Combobox disabledFocusable pending />
67+
68+
<!-- These are acceptable since no loading state is present -->
69+
<Button disabled>Cancel</Button>
70+
<Checkbox disabled />
71+
<Input disabled={permanentlyDisabled} />
72+
73+
<!-- These are acceptable since component is not disabled -->
74+
<Button loading>Submit</Button>
75+
<SpinButton isLoading />
76+
<Input busy />
77+
```
78+
79+
## Edge Cases & Considerations
80+
81+
### Both Props Present
82+
If both `disabled` and `disabledFocusable` are present, this rule will not trigger as it represents a different configuration issue.
83+
84+
```jsx
85+
<!-- Rule will not trigger - different concern -->
86+
<Button disabled disabledFocusable loading>Submit</Button>
87+
```
88+
89+
### Non-Loading Disabled States
90+
The rule only applies when both disabled AND loading states are present:
91+
92+
```jsx
93+
<!-- ✅ Acceptable - no loading state -->
94+
<Button disabled>Permanently Disabled Action</Button>
95+
```
96+
97+
### Complex Expressions
98+
The rule works with boolean expressions and variables:
99+
100+
```jsx
101+
<!-- ❌ Will trigger -->
102+
<Button disabled={!isEnabled} loading={isSubmitting}>Submit</Button>
103+
104+
<!-- ✅ Correct -->
105+
<Button disabledFocusable={!isEnabled} loading={isSubmitting}>Submit</Button>
106+
```
107+
108+
## When Not To Use It
109+
110+
You may want to disable this rule if:
111+
112+
1. **Intentional UX Decision**: You specifically want loading components removed from tab order
113+
2. **Legacy Codebase**: Existing implementations rely on specific disabled behavior during loading
114+
3. **Custom Loading Patterns**: Your application uses non-standard loading state management
115+
116+
However, disabling this rule is generally **not recommended** as it reduces accessibility.
117+
118+
## Automatic Fixes
119+
120+
The rule provides automatic fixes that replace `disabled` with `disabledFocusable` while preserving the original prop value:
121+
122+
```jsx
123+
// Before fix
124+
<Button disabled={isSubmitting} loading>Submit</Button>
125+
126+
// After fix
127+
<Button disabledFocusable={isSubmitting} loading>Submit</Button>
128+
```
129+
130+
## Related Rules
131+
132+
- [`no-empty-buttons`](./no-empty-buttons.md) - Ensures buttons have content or accessible labeling
133+
- [`prefer-aria-over-title-attribute`](./prefer-aria-over-title-attribute.md) - Improves screen reader compatibility
134+
135+
## Further Reading
136+
137+
- [WAI-ARIA: Keyboard Interface](https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/)
138+
- [FluentUI Accessibility Guidelines](https://react.fluentui.dev/?path=/docs/concepts-developer-accessibility--page)
139+
- [Understanding ARIA: disabled vs aria-disabled](https://css-tricks.com/making-disabled-buttons-more-inclusive/)

‎lib/applicableComponents/disabledFocusableComponents.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
/**
5+
* FluentUI components that support both 'disabled' and 'disabledFocusable' props
6+
* These are components where the rule should apply
7+
*/
8+
const disabledFocusableComponents = [
9+
// Button components
10+
"Button",
11+
"ToggleButton",
12+
"CompoundButton",
13+
"MenuButton",
14+
"SplitButton",
15+
16+
// Form controls
17+
"Checkbox",
18+
"Radio",
19+
"Switch",
20+
21+
// Input components
22+
"Input",
23+
"Textarea",
24+
"Combobox",
25+
"Dropdown",
26+
"SpinButton",
27+
"Slider",
28+
"DatePicker",
29+
"TimePicker",
30+
31+
// Other interactive components
32+
"Link",
33+
"Tab"
34+
] as const;
35+
36+
export { disabledFocusableComponents };

‎lib/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ module.exports = {
4141
"@microsoft/fluentui-jsx-a11y/no-empty-buttons": "error",
4242
"@microsoft/fluentui-jsx-a11y/no-empty-components": "error",
4343
"@microsoft/fluentui-jsx-a11y/prefer-aria-over-title-attribute": "warn",
44+
"@microsoft/fluentui-jsx-a11y/prefer-disabledfocusable-over-disabled": "warn",
4445
"@microsoft/fluentui-jsx-a11y/progressbar-needs-labelling": "error",
4546
"@microsoft/fluentui-jsx-a11y/radio-button-missing-label": "error",
4647
"@microsoft/fluentui-jsx-a11y/radiogroup-missing-label": "error",
@@ -83,6 +84,7 @@ module.exports = {
8384
"no-empty-buttons": rules.noEmptyButtons,
8485
"no-empty-components": rules.noEmptyComponents,
8586
"prefer-aria-over-title-attribute": rules.preferAriaOverTitleAttribute,
87+
"prefer-disabledfocusable-over-disabled": rules.preferDisabledFocusableOverDisabled,
8688
"progressbar-needs-labelling": rules.progressbarNeedsLabelling,
8789
"radio-button-missing-label": rules.radioButtonMissingLabel,
8890
"radiogroup-missing-label": rules.radiogroupMissingLabel,

‎lib/rules/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,4 @@ export { default as tablistAndTabsNeedLabelling } from "./tablist-and-tabs-need-
3939
export { default as toolbarMissingAria } from "./toolbar-missing-aria";
4040
export { default as tooltipNotRecommended } from "./tooltip-not-recommended";
4141
export { default as visualLabelBetterThanAriaSuggestion } from "./visual-label-better-than-aria-suggestion";
42+
export { default as preferDisabledFocusableOverDisabled } from "./prefer-disabledfocusable-over-disabled";

‎lib/rules/prefer-disabledfocusable-over-disabled.ts‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
import { ESLintUtils, TSESTree } from "@typescript-eslint/utils";
5+
import { elementType } from "jsx-ast-utils";
6+
import { hasNonEmptyProp } from "../util/hasNonEmptyProp";
7+
import { hasLoadingState, getLoadingStateProp } from "../util/hasLoadingState";
8+
import { disabledFocusableComponents } from "../applicableComponents/disabledFocusableComponents";
9+
import { JSXOpeningElement } from "estree-jsx";
10+
11+
//------------------------------------------------------------------------------
12+
// Rule Definition
13+
//------------------------------------------------------------------------------
14+
15+
const rule = ESLintUtils.RuleCreator.withoutDocs({
16+
defaultOptions: [],
17+
meta: {
18+
messages: {
19+
preferDisabledFocusable:
20+
"Accessibility: Prefer 'disabledFocusable={{{{loadingProp}}}}}' over 'disabled={{{{loadingProp}}}}}' when component has loading state '{{loadingProp}}' to maintain keyboard navigation accessibility",
21+
preferDisabledFocusableGeneric:
22+
"Accessibility: Prefer 'disabledFocusable' over 'disabled' when component has loading state to maintain keyboard navigation accessibility"
23+
},
24+
type: "suggestion", // This is a suggestion for better accessibility
25+
docs: {
26+
description:
27+
"Prefer 'disabledFocusable' over 'disabled' when component has loading state to maintain keyboard navigation accessibility",
28+
recommended: "warn",
29+
url: "https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/"
30+
},
31+
fixable: "code", // Allow auto-fixing
32+
schema: []
33+
},
34+
35+
create(context) {
36+
return {
37+
JSXOpeningElement(node: TSESTree.JSXOpeningElement) {
38+
const componentName = elementType(node as JSXOpeningElement);
39+
40+
// Check if this is an applicable component
41+
if (!disabledFocusableComponents.includes(componentName as any)) {
42+
return;
43+
}
44+
45+
// Check if component has 'disabled' prop
46+
const hasDisabled = hasNonEmptyProp(node.attributes, "disabled");
47+
if (!hasDisabled) {
48+
return;
49+
}
50+
51+
// Check if component has loading state
52+
const hasLoading = hasLoadingState(node.attributes);
53+
if (!hasLoading) {
54+
return;
55+
}
56+
57+
// Check if component already has disabledFocusable (avoid conflicts)
58+
const hasDisabledFocusable = hasNonEmptyProp(node.attributes, "disabledFocusable");
59+
if (hasDisabledFocusable) {
60+
return; // Don't report if both are present - that's a different issue
61+
}
62+
63+
const loadingProp = getLoadingStateProp(node.attributes);
64+
65+
context.report({
66+
node,
67+
messageId: loadingProp ? "preferDisabledFocusable" : "preferDisabledFocusableGeneric",
68+
data: {
69+
loadingProp: loadingProp || "loading"
70+
},
71+
fix(fixer) {
72+
// Find the disabled attribute and replace it with disabledFocusable
73+
const disabledAttr = node.attributes.find(
74+
attr => attr.type === "JSXAttribute" && attr.name?.type === "JSXIdentifier" && attr.name.name === "disabled"
75+
);
76+
77+
if (disabledAttr && disabledAttr.type === "JSXAttribute" && disabledAttr.name) {
78+
return fixer.replaceText(disabledAttr.name, "disabledFocusable");
79+
}
80+
return null;
81+
}
82+
});
83+
}
84+
};
85+
}
86+
});
87+
88+
export default rule;

‎lib/util/hasLoadingState.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
import { TSESTree } from "@typescript-eslint/utils";
5+
import { hasNonEmptyProp } from "./hasNonEmptyProp";
6+
7+
/**
8+
* Common prop names that indicate a loading state in FluentUI components
9+
*/
10+
const LOADING_STATE_PROPS = ["loading", "isLoading", "pending", "isPending", "busy", "isBusy"] as const;
11+
12+
/**
13+
* Determines if the component has any loading state indicator prop
14+
* @param attributes - JSX attributes array
15+
* @returns boolean indicating if component has loading state
16+
*/
17+
export const hasLoadingState = (attributes: TSESTree.JSXOpeningElement["attributes"]): boolean => {
18+
return LOADING_STATE_PROPS.some(prop => hasNonEmptyProp(attributes, prop));
19+
};
20+
21+
/**
22+
* Gets the specific loading prop that is present (for better error messages)
23+
* @param attributes - JSX attributes array
24+
* @returns string name of the loading prop found, or null if none
25+
*/
26+
export const getLoadingStateProp = (attributes: TSESTree.JSXOpeningElement["attributes"]): string | null => {
27+
return LOADING_STATE_PROPS.find(prop => hasNonEmptyProp(attributes, prop)) ?? null;
28+
};

0 commit comments

Comments
 (0)