Skip to content

Commit

Permalink
Button: Stabilize __experimentalIsFocusable prop
Browse files Browse the repository at this point in the history
  • Loading branch information
mirka committed Jun 4, 2024
1 parent 34fa834 commit 00d5e97
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 16 deletions.
13 changes: 13 additions & 0 deletions packages/components/src/button/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,17 @@ The presence of a `href` prop determines whether an `anchor` element is rendered

Props not included in this set will be applied to the `a` or `button` element.

#### `accessibleWhenDisabled`: `boolean`

Whether to keep the button focusable when disabled.

In most cases, it is recommended to set this to `true`. Disabling a control without maintaining focusability can cause accessibility issues, by hiding their presence from screen reader users, or by preventing focus from returning to a trigger element.

Learn more about the [focusability of disabled controls](https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/#focusabilityofdisabledcontrols) in the WAI-ARIA Authoring Practices Guide.

- Required: No
- Default: `false`

#### `children`: `ReactNode`

The button's children.
Expand All @@ -137,6 +148,8 @@ An accessible description for the button.

Whether the button is disabled. If `true`, this will force a `button` element to be rendered, even when an `href` is given.

In most cases, it is recommended to also set the `accessibleWhenDisabled` prop to `true`.

- Required: No

#### `href`: `string`
Expand Down
10 changes: 6 additions & 4 deletions packages/components/src/button/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { positionToPlacement } from '../popover/utils';
const disabledEventsOnDisabledButton = [ 'onMouseDown', 'onClick' ] as const;

function useDeprecatedProps( {
__experimentalIsFocusable,
isDefault,
isPrimary,
isSecondary,
Expand All @@ -43,7 +44,8 @@ function useDeprecatedProps( {
let computedSize = size;
let computedVariant = variant;

const newProps: { 'aria-pressed'?: boolean } = {
const newProps = {
accessibleWhenDisabled: __experimentalIsFocusable,
// @todo Mark `isPressed` as deprecated
'aria-pressed': isPressed,
};
Expand Down Expand Up @@ -91,6 +93,7 @@ export function UnforwardedButton(
) {
const {
__next40pxDefaultSize,
accessibleWhenDisabled,
isBusy,
isDestructive,
className,
Expand All @@ -106,7 +109,6 @@ export function UnforwardedButton(
size = 'default',
text,
variant,
__experimentalIsFocusable: isFocusable,
describedBy,
...buttonOrAnchorProps
} = useDeprecatedProps( props );
Expand Down Expand Up @@ -159,7 +161,7 @@ export function UnforwardedButton(
'has-icon': !! icon,
} );

const trulyDisabled = disabled && ! isFocusable;
const trulyDisabled = disabled && ! accessibleWhenDisabled;
const Tag = href !== undefined && ! trulyDisabled ? 'a' : 'button';
const buttonProps: ComponentPropsWithoutRef< 'button' > =
Tag === 'button'
Expand All @@ -177,7 +179,7 @@ export function UnforwardedButton(
const disableEventProps: {
[ key: string ]: ( event: MouseEvent ) => void;
} = {};
if ( disabled && isFocusable ) {
if ( disabled && accessibleWhenDisabled ) {
// In this case, the button will be disabled, but still focusable and
// perceivable by screen reader users.
buttonProps[ 'aria-disabled' ] = true;
Expand Down
12 changes: 10 additions & 2 deletions packages/components/src/button/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ describe( 'Button', () => {
} );

it( 'should add only aria-disabled attribute when disabled and isFocusable are true', () => {
render( <Button disabled __experimentalIsFocusable /> );
render( <Button disabled accessibleWhenDisabled /> );
const button = screen.getByRole( 'button' );

expect( button ).toBeEnabled();
Expand Down Expand Up @@ -617,6 +617,14 @@ describe( 'Button', () => {
'mixed'
);
} );

it( 'should not break when the legacy __experimentalIsFocusable prop is passed', () => {
render( <Button disabled __experimentalIsFocusable /> );
const button = screen.getByRole( 'button' );

expect( button ).toBeEnabled();
expect( button ).toHaveAttribute( 'aria-disabled' );
} );
} );

describe( 'static typing', () => {
Expand All @@ -632,7 +640,7 @@ describe( 'Button', () => {
{ /* @ts-expect-error */ }
<Button type="invalidtype" />
{ /* @ts-expect-error - although the runtime behavior will allow this to be an anchor, this is probably a mistake. */ }
<Button disabled __experimentalIsFocusable href="foo" />
<Button disabled accessibleWhenDisabled href="foo" />
</>;
} );
} );
37 changes: 27 additions & 10 deletions packages/components/src/button/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@ type BaseButtonProps = {
* @default false
*/
__next40pxDefaultSize?: boolean;
/**
* Whether to keep the button focusable when disabled.
*
* In most cases, it is recommended to set this to `true`. Disabling a control without maintaining focusability
* can cause accessibility issues, by hiding their presence from screen reader users,
* or by preventing focus from returning to a trigger element.
*
* Learn more about the [focusability of disabled controls](https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/#focusabilityofdisabledcontrols)
* in the WAI-ARIA Authoring Practices Guide.
*
* @default false
*/
accessibleWhenDisabled?: boolean;
/**
* The button's children.
*/
Expand Down Expand Up @@ -105,28 +118,24 @@ type BaseButtonProps = {
* 'link' (the link button styles)
*/
variant?: 'primary' | 'secondary' | 'tertiary' | 'link';
/**
* Whether to keep the button focusable when disabled.
*
* @default false
*/
__experimentalIsFocusable?: boolean;
};

type _ButtonProps = {
/**
* Whether the button is disabled.
* Whether the button is disabled. If `true`, this will force a `button` element
* to be rendered, even when an `href` is given.
*
* If `true`, this will force a `button` element to be rendered, even when an `href` is given.
* In most cases, it is recommended to also set the `accessibleWhenDisabled` prop to `true`.
*/
disabled?: boolean;
};

type AnchorProps = {
/**
* Whether the button is disabled.
* Whether the button is disabled. If `true`, this will force a `button` element
* to be rendered, even when an `href` is given.
*
* If `true`, this will force a `button` element to be rendered, even when an `href` is given.
* In most cases, it is recommended to also set the `accessibleWhenDisabled` prop to `true`.
*/
disabled?: false;
/**
Expand All @@ -140,6 +149,14 @@ type AnchorProps = {
};

export type DeprecatedButtonProps = {
/**
* Whether to keep the button focusable when disabled.
*
* @default false
* @deprecated Use the `accessibleWhenDisabled` prop instead.
* @ignore
*/
__experimentalIsFocusable?: boolean;
/**
* Gives the button a default style.
*
Expand Down

0 comments on commit 00d5e97

Please sign in to comment.