From ff40754b4308784f31acb54ec74842d97fe6add5 Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Wed, 13 Sep 2023 09:58:50 -0500 Subject: [PATCH 1/2] feat(button): allow tooltips on disabled buttons via disabledFocusable --- .../__snapshots__/PublicAPI-test.js.snap | 3 ++ .../react/src/components/Button/Button.tsx | 20 +++++++-- .../Button/__tests__/Button-test.js | 25 +++++++++++ .../__snapshots__/DataTable-test.js.snap | 13 ++++++ .../TableToolbarMenu-test.js.snap | 1 + .../scss/components/button/_button.scss | 43 ++++++++++++++++++- .../scss/components/button/_mixins.scss | 9 ++++ 7 files changed, 109 insertions(+), 5 deletions(-) diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index 19555590986d..2f278bf27e08 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -288,6 +288,9 @@ Map { "disabled": Object { "type": "bool", }, + "disabledFocusable": Object { + "type": "bool", + }, "hasIconOnly": Object { "type": "bool", }, diff --git a/packages/react/src/components/Button/Button.tsx b/packages/react/src/components/Button/Button.tsx index 260a92943ddb..b2f0faafaab3 100644 --- a/packages/react/src/components/Button/Button.tsx +++ b/packages/react/src/components/Button/Button.tsx @@ -121,6 +121,7 @@ const Button = React.forwardRef(function Button( className, dangerDescription = 'danger', disabled = false, + disabledFocusable = false, hasIconOnly = false, href, iconDescription, @@ -147,7 +148,7 @@ const Button = React.forwardRef(function Button( const handleClick = (evt: React.MouseEvent) => { // Prevent clicks on the tooltip from triggering the button click event - if (evt.target === tooltipRef.current) { + if (evt.target === tooltipRef.current || disabledFocusable) { evt.preventDefault(); return; } @@ -162,6 +163,7 @@ const Button = React.forwardRef(function Button( [`${prefix}--layout--size-${size}`]: size, [`${prefix}--btn--${kind}`]: kind, [`${prefix}--btn--disabled`]: disabled, + [`${prefix}--btn--disabled-focusable`]: disabledFocusable, [`${prefix}--btn--expressive`]: isExpressive, [`${prefix}--btn--icon-only`]: hasIconOnly, [`${prefix}--btn--selected`]: hasIconOnly && isSelected && kind === 'ghost', @@ -190,6 +192,7 @@ const Button = React.forwardRef(function Button( const { 'aria-pressed': ariaPressed } = rest; let otherProps: Partial = { disabled, + 'aria-disabled': disabledFocusable, type, 'aria-describedby': dangerButtonVariants.includes(kind) ? assistiveId @@ -216,7 +219,7 @@ const Button = React.forwardRef(function Button( ...otherProps, ...anchorProps, }; - } else if (href && !disabled) { + } else if (href && !disabled && !disabledFocusable) { component = 'a'; otherProps = anchorProps; } @@ -268,7 +271,9 @@ const Button = React.forwardRef(function Button( onMouseLeave={onMouseLeave} onFocus={onFocus} onBlur={onBlur} - onClick={composeEventHandlers([onClick, handleClick])} + onClick={ + !disabledFocusable && composeEventHandlers([onClick, handleClick]) + } {...rest} {...commonProps} {...otherProps}> @@ -310,6 +315,15 @@ Button.propTypes = { */ disabled: PropTypes.bool, + /** + * Disable the button but still allow it to be focusable. + * A Button can only be disabled OR disabledFocusable, not both. + * This is useful for when the button needs to remain in the tab + * order. This is most commonly used to allow a tooltip to be + * rendered on a disabled button to convey the reason why the button is disabled. + */ + disabledFocusable: PropTypes.bool, + /** * Specify if the button is an icon-only button */ diff --git a/packages/react/src/components/Button/__tests__/Button-test.js b/packages/react/src/components/Button/__tests__/Button-test.js index 458ef2e99c37..f256b195dfab 100644 --- a/packages/react/src/components/Button/__tests__/Button-test.js +++ b/packages/react/src/components/Button/__tests__/Button-test.js @@ -7,6 +7,7 @@ import { Search, Add } from '@carbon/icons-react'; import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import React from 'react'; import Button from '../../Button'; @@ -45,6 +46,30 @@ describe('Button', () => { expect(screen.getByRole('button')).toBeDisabled(); }); + it('should receive focus when given disabledFocusable', async () => { + const onClick = jest.fn(); + render( + + ); + // `toBeDisabled` does not take into account aria-disabled. + // this check ensures that the `disabled` attribute is not present + expect(screen.getByRole('button')).toBeEnabled(); + + expect(screen.getByRole('button')).not.toHaveFocus(); + await userEvent.tab(); + expect(screen.getByRole('button')).toHaveFocus(); + + await userEvent.click(); + + // To adhere to the spec, buttons with aria-disabled must also "implement + // the necessary scripting to functionally disable the button, rather than + // the use disabled attribute." + // https://www.w3.org/TR/html-aria/#docconformance-attr + expect(onClick).not.toHaveBeenCalled(); + }); + it('should render with a default button type of button', () => { render(); expect(screen.getByRole('button')).toHaveAttribute('type', 'button'); diff --git a/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap b/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap index 61255fbb3ebe..508eceab61d7 100644 --- a/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap +++ b/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap @@ -375,6 +375,7 @@ exports[`DataTable behaves as expected selection should render and match snapsho |