Skip to content

Commit

Permalink
[Checkbox, Menu, Switch] Audit focus states on disabled components (#734
Browse files Browse the repository at this point in the history
)

Co-authored-by: hakimuddinhh <[email protected]>
  • Loading branch information
onehanddev and hakim-toptal authored Oct 28, 2024
1 parent f23c962 commit 7f8705d
Show file tree
Hide file tree
Showing 8 changed files with 10 additions and 25 deletions.
1 change: 0 additions & 1 deletion docs/data/api/menu-trigger.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
"props": {
"className": { "type": { "name": "union", "description": "func<br>&#124;&nbsp;string" } },
"disabled": { "type": { "name": "bool" }, "default": "false" },
"focusableWhenDisabled": { "type": { "name": "bool" }, "default": "false" },
"label": { "type": { "name": "string" } },
"render": { "type": { "name": "union", "description": "element<br>&#124;&nbsp;func" } }
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@
"description": "Class names applied to the element or a function that returns them based on the component&#39;s state."
},
"disabled": { "description": "If <code>true</code>, the component is disabled." },
"focusableWhenDisabled": {
"description": "If <code>true</code>, allows a disabled button to receive focus."
},
"label": { "description": "Label of the button" },
"render": { "description": "A function to customize rendering of the component." }
},
Expand Down
8 changes: 4 additions & 4 deletions packages/mui-base/src/Checkbox/Root/CheckboxRoot.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,14 @@ describe('<Checkbox.Root />', () => {
});

describe('prop: disabled', () => {
it('should have the `aria-disabled` attribute', async () => {
it('should have the `disabled` attribute', async () => {
const { getAllByRole } = await render(<Checkbox.Root disabled />);
expect(getAllByRole('checkbox')[0]).to.have.attribute('aria-disabled', 'true');
expect(getAllByRole('checkbox')[0]).to.have.attribute('disabled');
});

it('should not have the aria attribute when `disabled` is not set', async () => {
it('should not have the `disabled` attribute when `disabled` is not set', async () => {
const { getAllByRole } = await render(<Checkbox.Root />);
expect(getAllByRole('checkbox')[0]).not.to.have.attribute('aria-disabled');
expect(getAllByRole('checkbox')[0]).not.to.have.attribute('disabled');
});

it('should not change its state when clicked', async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/Checkbox/Root/useCheckboxRoot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ export function useCheckboxRoot(params: UseCheckboxRoot.Parameters): UseCheckbox
value: 'off',
type: 'button',
role: 'checkbox',
disabled,
'aria-checked': indeterminate ? 'mixed' : checked,
'aria-disabled': disabled || undefined,
'aria-readonly': readOnly || undefined,
'aria-labelledby': labelId,
onBlur() {
Expand Down
10 changes: 0 additions & 10 deletions packages/mui-base/src/Menu/Trigger/MenuTrigger.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,6 @@ namespace MenuTrigger {
* @default false
*/
disabled?: boolean;
/**
* If `true`, allows a disabled button to receive focus.
* @default false
*/
focusableWhenDisabled?: boolean;
/**
* Label of the button
*/
Expand Down Expand Up @@ -101,11 +96,6 @@ MenuTrigger.propTypes /* remove-proptypes */ = {
* @default false
*/
disabled: PropTypes.bool,
/**
* If `true`, allows a disabled button to receive focus.
* @default false
*/
focusableWhenDisabled: PropTypes.bool,
/**
* Label of the button
*/
Expand Down
1 change: 0 additions & 1 deletion packages/mui-base/src/Menu/Trigger/useMenuTrigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ export function useMenuTrigger(parameters: useMenuTrigger.Parameters): useMenuTr

const { getButtonProps, buttonRef } = useButton({
disabled,
focusableWhenDisabled: false,
buttonRef: mergedRef,
});

Expand Down
8 changes: 4 additions & 4 deletions packages/mui-base/src/Switch/Root/SwitchRoot.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,14 @@ describe('<Switch.Root />', () => {
});

describe('prop: disabled', () => {
it('should have the `aria-disabled` attribute', async () => {
it('should have the `disabled` attribute', async () => {
const { getByRole } = await render(<Switch.Root disabled />);
expect(getByRole('switch')).to.have.attribute('aria-disabled', 'true');
expect(getByRole('switch')).to.have.attribute('disabled');
});

it('should not have the aria attribute when `disabled` is not set', async () => {
it('should not have the `disabled` attribute when `disabled` is not set', async () => {
const { getByRole } = await render(<Switch.Root />);
expect(getByRole('switch')).not.to.have.attribute('aria-disabled');
expect(getByRole('switch')).not.to.have.attribute('disabled');
});

it('should not change its state when clicked', async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/Switch/Root/useSwitchRoot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ export function useSwitchRoot(params: useSwitchRoot.Parameters): useSwitchRoot.R
ref: buttonRef,
type: 'button',
role: 'switch',
disabled,
'aria-checked': checked,
'aria-disabled': disabled || undefined,
'aria-readonly': readOnly,
'aria-labelledby': labelId,
onBlur() {
Expand Down

0 comments on commit 7f8705d

Please sign in to comment.