From 47bf76d157e59779d8dff4e5c6b1d3c5678dce5a Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Fri, 10 Jan 2025 10:27:53 +0800 Subject: [PATCH] [useButton] Fix `focusableWhenDisabled` components (#1313) --- .../reference/generated/menu-radio-group.json | 5 + .../src/accordion/root/useAccordionRoot.ts | 4 +- .../trigger/useCollapsibleTrigger.ts | 24 ++- .../checkbox-item/MenuCheckboxItem.test.tsx | 49 ++++++ .../react/src/menu/item/MenuItem.test.tsx | 46 +++++- .../src/menu/radio-group/MenuRadioGroup.tsx | 35 ++-- .../menu/radio-group/MenuRadioGroupContext.ts | 1 + .../menu/radio-item/MenuRadioItem.test.tsx | 151 ++++++++++++++++++ .../src/menu/radio-item/MenuRadioItem.tsx | 18 ++- .../react/src/menu/trigger/useMenuTrigger.ts | 8 +- .../react/src/select/item/SelectItem.test.tsx | 25 ++- .../react/src/select/item/useSelectItem.ts | 7 +- .../react/src/select/root/useSelectRoot.ts | 21 ++- .../src/select/trigger/SelectTrigger.test.tsx | 58 ++++++- .../src/select/trigger/SelectTrigger.tsx | 3 +- .../src/select/trigger/useSelectTrigger.ts | 23 ++- .../react/src/use-button/useButton.test.tsx | 60 ++++++- packages/react/src/use-button/useButton.ts | 18 ++- 18 files changed, 492 insertions(+), 64 deletions(-) diff --git a/docs/reference/generated/menu-radio-group.json b/docs/reference/generated/menu-radio-group.json index fc514a3b64..8bc7d0db90 100644 --- a/docs/reference/generated/menu-radio-group.json +++ b/docs/reference/generated/menu-radio-group.json @@ -15,6 +15,11 @@ "default": "() => {}", "description": "Function called when the selected value changes." }, + "disabled": { + "type": "boolean", + "default": "false", + "description": "Whether the component should ignore user interaction." + }, "children": { "type": "React.ReactNode", "description": "The content of the component." diff --git a/packages/react/src/accordion/root/useAccordionRoot.ts b/packages/react/src/accordion/root/useAccordionRoot.ts index 31e74a6826..a1f180793c 100644 --- a/packages/react/src/accordion/root/useAccordionRoot.ts +++ b/packages/react/src/accordion/root/useAccordionRoot.ts @@ -28,9 +28,7 @@ function getActiveTriggers(accordionItemRefs: { function isDisabled(element: HTMLElement | null) { return ( - element === null || - element.hasAttribute('disabled') || - element.getAttribute('data-disabled') === 'true' + element === null || element.hasAttribute('disabled') || element.hasAttribute('data-disabled') ); } diff --git a/packages/react/src/collapsible/trigger/useCollapsibleTrigger.ts b/packages/react/src/collapsible/trigger/useCollapsibleTrigger.ts index c5a248df48..d140e49c62 100644 --- a/packages/react/src/collapsible/trigger/useCollapsibleTrigger.ts +++ b/packages/react/src/collapsible/trigger/useCollapsibleTrigger.ts @@ -20,21 +20,17 @@ export function useCollapsibleTrigger( const getRootProps: useCollapsibleTrigger.ReturnValue['getRootProps'] = React.useCallback( (externalProps: GenericHTMLProps = {}) => - mergeReactProps( - externalProps, - mergeReactProps( - { - type: 'button', - 'aria-controls': panelId, - 'aria-expanded': open, - disabled, - onClick() { - setOpen(!open); - }, - ref: handleRef, + getButtonProps( + mergeReactProps(externalProps, { + type: 'button', + 'aria-controls': panelId, + 'aria-expanded': open, + disabled, + onClick() { + setOpen(!open); }, - getButtonProps(), - ), + ref: handleRef, + }), ), [panelId, disabled, getButtonProps, handleRef, open, setOpen], ); diff --git a/packages/react/src/menu/checkbox-item/MenuCheckboxItem.test.tsx b/packages/react/src/menu/checkbox-item/MenuCheckboxItem.test.tsx index eb8e4ea270..4c9811122a 100644 --- a/packages/react/src/menu/checkbox-item/MenuCheckboxItem.test.tsx +++ b/packages/react/src/menu/checkbox-item/MenuCheckboxItem.test.tsx @@ -331,4 +331,53 @@ describe('', () => { expect(queryByRole('menu')).not.to.equal(null); }); }); + + describe('focusableWhenDisabled', () => { + it('can be focused but not interacted with when disabled', async () => { + const handleCheckedChange = spy(); + const handleClick = spy(); + const handleKeyDown = spy(); + const handleKeyUp = spy(); + + const { getByRole } = await render( + + + + + + Item + + + + + , + ); + + const item = getByRole('menuitemcheckbox'); + await act(() => item.focus()); + expect(item).toHaveFocus(); + + fireEvent.keyDown(item, { key: 'Enter' }); + expect(handleKeyDown.callCount).to.equal(1); + expect(handleClick.callCount).to.equal(0); + expect(handleCheckedChange.callCount).to.equal(0); + + fireEvent.keyUp(item, { key: 'Space' }); + expect(handleKeyUp.callCount).to.equal(1); + expect(handleClick.callCount).to.equal(0); + expect(handleCheckedChange.callCount).to.equal(0); + + fireEvent.click(item); + expect(handleKeyDown.callCount).to.equal(1); + expect(handleKeyUp.callCount).to.equal(1); + expect(handleClick.callCount).to.equal(0); + expect(handleCheckedChange.callCount).to.equal(0); + }); + }); }); diff --git a/packages/react/src/menu/item/MenuItem.test.tsx b/packages/react/src/menu/item/MenuItem.test.tsx index f38eb8408d..75b8f0ffa2 100644 --- a/packages/react/src/menu/item/MenuItem.test.tsx +++ b/packages/react/src/menu/item/MenuItem.test.tsx @@ -2,7 +2,7 @@ import * as React from 'react'; import { expect } from 'chai'; import { spy } from 'sinon'; import { MemoryRouter, Route, Routes, Link, useLocation } from 'react-router-dom'; -import { act, screen, waitFor } from '@mui/internal-test-utils'; +import { act, fireEvent, screen, waitFor } from '@mui/internal-test-utils'; import { Menu } from '@base-ui-components/react/menu'; import { describeConformance, createRenderer, isJSDOM } from '#test-utils'; @@ -237,4 +237,48 @@ describe('', () => { expect(locationDisplay).to.have.text('/'); }); }); + + describe('disabled state', () => { + it('can be focused but not interacted with when disabled', async () => { + const handleClick = spy(); + const handleKeyDown = spy(); + const handleKeyUp = spy(); + + const { getByRole } = await render( + + + + + + Item + + + + + , + ); + + const item = getByRole('menuitem'); + await act(() => item.focus()); + expect(item).toHaveFocus(); + + fireEvent.keyDown(item, { key: 'Enter' }); + expect(handleKeyDown.callCount).to.equal(1); + expect(handleClick.callCount).to.equal(0); + + fireEvent.keyUp(item, { key: 'Space' }); + expect(handleKeyUp.callCount).to.equal(1); + expect(handleClick.callCount).to.equal(0); + + fireEvent.click(item); + expect(handleKeyDown.callCount).to.equal(1); + expect(handleKeyUp.callCount).to.equal(1); + expect(handleClick.callCount).to.equal(0); + }); + }); }); diff --git a/packages/react/src/menu/radio-group/MenuRadioGroup.tsx b/packages/react/src/menu/radio-group/MenuRadioGroup.tsx index 9afd4bda16..b8e21dd685 100644 --- a/packages/react/src/menu/radio-group/MenuRadioGroup.tsx +++ b/packages/react/src/menu/radio-group/MenuRadioGroup.tsx @@ -1,14 +1,12 @@ import * as React from 'react'; import PropTypes from 'prop-types'; import { MenuRadioGroupContext } from './MenuRadioGroupContext'; +import { NOOP } from '../../utils/noop'; import { BaseUIComponentProps } from '../../utils/types'; import { useComponentRenderer } from '../../utils/useComponentRenderer'; import { useControlled } from '../../utils/useControlled'; import { useEventCallback } from '../../utils/useEventCallback'; -const EMPTY_OBJECT = {}; -const NOOP = () => {}; - /** * Groups related radio items. * Renders a `
` element. @@ -22,7 +20,8 @@ const MenuRadioGroup = React.forwardRef(function MenuRadioGroup( className, value: valueProp, defaultValue, - onValueChange: onValueChangeProp = NOOP, + onValueChange: onValueChangeProp, + disabled = false, ...other } = props; @@ -32,7 +31,7 @@ const MenuRadioGroup = React.forwardRef(function MenuRadioGroup( name: 'MenuRadioGroup', }); - const onValueChange = useEventCallback(onValueChangeProp); + const onValueChange = useEventCallback(onValueChangeProp ?? NOOP); const setValue = React.useCallback( (newValue: any, event: Event) => { @@ -42,23 +41,27 @@ const MenuRadioGroup = React.forwardRef(function MenuRadioGroup( [onValueChange, setValueUnwrapped], ); + const state = React.useMemo(() => ({ disabled }), [disabled]); + const { renderElement } = useComponentRenderer({ render: render || 'div', className, - state: EMPTY_OBJECT, + state, extraProps: { role: 'group', + 'aria-disabled': disabled || undefined, ...other, }, ref: forwardedRef, }); - const context = React.useMemo( + const context: MenuRadioGroupContext = React.useMemo( () => ({ value, setValue, + disabled, }), - [value, setValue], + [value, setValue, disabled], ); return ( @@ -92,9 +95,17 @@ namespace MenuRadioGroup { * @default () => {} */ onValueChange?: (value: any, event: Event) => void; + /** + * Whether the component should ignore user interaction. + * + * @default false + */ + disabled?: boolean; } - export type State = {}; + export type State = { + disabled: boolean; + }; } MenuRadioGroup.propTypes /* remove-proptypes */ = { @@ -117,6 +128,12 @@ MenuRadioGroup.propTypes /* remove-proptypes */ = { * To render a controlled radio group, use the `value` prop instead. */ defaultValue: PropTypes.any, + /** + * Whether the component should ignore user interaction. + * + * @default false + */ + disabled: PropTypes.bool, /** * Function called when the selected value changes. * diff --git a/packages/react/src/menu/radio-group/MenuRadioGroupContext.ts b/packages/react/src/menu/radio-group/MenuRadioGroupContext.ts index 1b8332f8b7..27b98b4ace 100644 --- a/packages/react/src/menu/radio-group/MenuRadioGroupContext.ts +++ b/packages/react/src/menu/radio-group/MenuRadioGroupContext.ts @@ -3,6 +3,7 @@ import * as React from 'react'; export interface MenuRadioGroupContext { value: any; setValue: (newValue: any, event: Event) => void; + disabled: boolean; } export const MenuRadioGroupContext = React.createContext( diff --git a/packages/react/src/menu/radio-item/MenuRadioItem.test.tsx b/packages/react/src/menu/radio-item/MenuRadioItem.test.tsx index 0a49072b98..179ea23ae1 100644 --- a/packages/react/src/menu/radio-item/MenuRadioItem.test.tsx +++ b/packages/react/src/menu/radio-item/MenuRadioItem.test.tsx @@ -9,6 +9,7 @@ import { MenuRadioGroupContext } from '../radio-group/MenuRadioGroupContext'; const testRadioGroupContext = { value: '0', setValue: () => {}, + disabled: false, }; describe('', () => { @@ -299,4 +300,154 @@ describe('', () => { expect(queryByRole('menu')).not.to.equal(null); }); }); + + describe('focusableWhenDisabled', () => { + it('can be focused but not interacted with when a radio group is disabled', async () => { + const handleClick = spy(); + const handleKeyDown = spy(); + const handleKeyUp = spy(); + const handleValueChange = spy(); + + const { getAllByRole } = await render( + + + + + + + one + + + two + + + + + + , + ); + + const [item1, item2] = getAllByRole('menuitemradio'); + + expect(item1).to.have.attribute('data-disabled'); + expect(item2).to.have.attribute('data-disabled'); + + await act(() => item1.focus()); + expect(item1).toHaveFocus(); + + fireEvent.keyDown(item1, { key: 'Enter' }); + expect(handleKeyDown.callCount).to.equal(1); + expect(handleClick.callCount).to.equal(0); + expect(handleValueChange.callCount).to.equal(0); + + fireEvent.keyUp(item1, { key: 'Space' }); + expect(handleKeyDown.callCount).to.equal(1); + expect(handleClick.callCount).to.equal(0); + expect(handleValueChange.callCount).to.equal(0); + + fireEvent.click(item1); + expect(handleClick.callCount).to.equal(0); + expect(handleValueChange.callCount).to.equal(0); + + fireEvent.keyDown(item1, { key: 'ArrowDown' }); + expect(handleKeyDown.callCount).to.equal(2); + expect(item2).toHaveFocus(); + + fireEvent.keyDown(item2, { key: 'Enter' }); + expect(handleKeyDown.callCount).to.equal(3); + expect(handleClick.callCount).to.equal(0); + expect(handleValueChange.callCount).to.equal(0); + + fireEvent.keyUp(item2, { key: 'Space' }); + expect(handleKeyDown.callCount).to.equal(3); + expect(handleClick.callCount).to.equal(0); + expect(handleValueChange.callCount).to.equal(0); + + fireEvent.click(item2); + expect(handleClick.callCount).to.equal(0); + expect(handleValueChange.callCount).to.equal(0); + }); + }); + + it('can be focused but not interacted with when individual items are disabled', async () => { + const handleClick = spy(); + const handleKeyDown = spy(); + const handleKeyUp = spy(); + const handleValueChange = spy(); + + const { getAllByRole } = await render( + + + + + + + one + + + two + + + + + + , + ); + + const [item1, item2] = getAllByRole('menuitemradio'); + + expect(item1).to.have.attribute('data-disabled'); + expect(item2).to.not.have.attribute('data-disabled'); + + await act(() => item1.focus()); + expect(item1).toHaveFocus(); + + fireEvent.keyDown(item1, { key: 'Enter' }); + expect(handleKeyDown.callCount).to.equal(1); + expect(handleClick.callCount).to.equal(0); + expect(handleValueChange.callCount).to.equal(0); + + fireEvent.keyUp(item1, { key: 'Space' }); + expect(handleKeyDown.callCount).to.equal(1); + expect(handleClick.callCount).to.equal(0); + expect(handleValueChange.callCount).to.equal(0); + + fireEvent.click(item1); + expect(handleClick.callCount).to.equal(0); + expect(handleValueChange.callCount).to.equal(0); + + fireEvent.keyDown(item1, { key: 'ArrowDown' }); + expect(handleKeyDown.callCount).to.equal(2); + expect(item2).toHaveFocus(); + + fireEvent.keyDown(item2, { key: 'Enter' }); + expect(handleKeyDown.callCount).to.equal(3); + expect(handleClick.callCount).to.equal(1); + expect(handleValueChange.callCount).to.equal(1); + expect(handleValueChange.args[0][0]).to.equal('two'); + + fireEvent.keyDown(item2, { key: 'ArrowDown' }); + expect(item1).toHaveFocus(); + }); }); diff --git a/packages/react/src/menu/radio-item/MenuRadioItem.tsx b/packages/react/src/menu/radio-item/MenuRadioItem.tsx index 26a0dc98b6..fbdfe80221 100644 --- a/packages/react/src/menu/radio-item/MenuRadioItem.tsx +++ b/packages/react/src/menu/radio-item/MenuRadioItem.tsx @@ -159,7 +159,14 @@ const MenuRadioItem = React.forwardRef(function MenuRadioItem( props: MenuRadioItem.Props, forwardedRef: React.ForwardedRef, ) { - const { id: idProp, value, label, disabled = false, closeOnClick = false, ...other } = props; + const { + id: idProp, + value, + label, + disabled: disabledProp = false, + closeOnClick = false, + ...other + } = props; const itemRef = React.useRef(null); const listItem = useCompositeListItem({ label }); @@ -171,7 +178,13 @@ const MenuRadioItem = React.forwardRef(function MenuRadioItem( const highlighted = listItem.index === activeIndex; const { events: menuEvents } = useFloatingTree()!; - const { value: selectedValue, setValue: setSelectedValue } = useMenuRadioGroupContext(); + const { + value: selectedValue, + setValue: setSelectedValue, + disabled: groupDisabled, + } = useMenuRadioGroupContext(); + + const disabled = groupDisabled || disabledProp; // This wrapper component is used as a performance optimization. // MenuRadioItem reads the context and re-renders the actual MenuRadioItem @@ -197,6 +210,7 @@ const MenuRadioItem = React.forwardRef(function MenuRadioItem( {...other} id={id} ref={mergedRef} + disabled={disabled} highlighted={highlighted} menuEvents={menuEvents} propGetter={getItemProps} diff --git a/packages/react/src/menu/trigger/useMenuTrigger.ts b/packages/react/src/menu/trigger/useMenuTrigger.ts index 132dbfd76c..b9576884ba 100644 --- a/packages/react/src/menu/trigger/useMenuTrigger.ts +++ b/packages/react/src/menu/trigger/useMenuTrigger.ts @@ -40,9 +40,8 @@ export function useMenuTrigger(parameters: useMenuTrigger.Parameters): useMenuTr const getTriggerProps = React.useCallback( (externalProps?: GenericHTMLProps): GenericHTMLProps => { - return mergeReactProps( - externalProps, - { + return getButtonProps( + mergeReactProps(externalProps, { 'aria-haspopup': 'menu' as const, tabIndex: 0, // this is needed to make the button focused after click in Safari ref: handleRef, @@ -95,8 +94,7 @@ export function useMenuTrigger(parameters: useMenuTrigger.Parameters): useMenuTr doc.addEventListener('mouseup', handleMouseUp, { once: true }); }, - }, - getButtonProps(), + }), ); }, [getButtonProps, handleRef, open, setOpen, positionerRef, allowMouseUpTriggerRef], diff --git a/packages/react/src/select/item/SelectItem.test.tsx b/packages/react/src/select/item/SelectItem.test.tsx index 1d2b1c64a4..63492a04f3 100644 --- a/packages/react/src/select/item/SelectItem.test.tsx +++ b/packages/react/src/select/item/SelectItem.test.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import { Select } from '@base-ui-components/react/select'; -import { fireEvent, flushMicrotasks, screen, waitFor } from '@mui/internal-test-utils'; +import { act, fireEvent, flushMicrotasks, screen, waitFor } from '@mui/internal-test-utils'; import { createRenderer, describeConformance, isJSDOM } from '#test-utils'; import { expect } from 'chai'; @@ -117,6 +117,29 @@ describe('', () => { }); }); + it('should focus disabled items', async () => { + await render( + + + + + + + + + two + + + + + , + ); + + const item = screen.getByText('two'); + await act(() => item.focus()); + expect(item).toHaveFocus(); + }); + it('should not select disabled item', async () => { await render( diff --git a/packages/react/src/select/item/useSelectItem.ts b/packages/react/src/select/item/useSelectItem.ts index 3499ed8509..d9298f77ee 100644 --- a/packages/react/src/select/item/useSelectItem.ts +++ b/packages/react/src/select/item/useSelectItem.ts @@ -50,9 +50,6 @@ export function useSelectItem(params: useSelectItem.Parameters): useSelectItem.R mergeReactProps<'div'>(externalProps, { 'aria-disabled': disabled || undefined, tabIndex: highlighted ? 0 : -1, - style: { - pointerEvents: disabled ? 'none' : undefined, - }, onFocus() { if (allowFocusSyncRef.current) { setActiveIndex(indexRef.current); @@ -112,6 +109,7 @@ export function useSelectItem(params: useSelectItem.Parameters): useSelectItem.R }, onClick(event) { if ( + disabled || (lastKeyRef.current === ' ' && typingRef.current) || (pointerTypeRef.current !== 'touch' && !highlighted) ) { @@ -130,6 +128,9 @@ export function useSelectItem(params: useSelectItem.Parameters): useSelectItem.R pointerTypeRef.current = event.pointerType; }, onMouseUp(event) { + if (disabled) { + return; + } const disallowSelectedMouseUp = !selectionRef.current.allowSelectedMouseUp && selected; const disallowUnselectedMouseUp = !selectionRef.current.allowUnselectedMouseUp && !selected; diff --git a/packages/react/src/select/root/useSelectRoot.ts b/packages/react/src/select/root/useSelectRoot.ts index c1c381eaa8..cdada0d008 100644 --- a/packages/react/src/select/root/useSelectRoot.ts +++ b/packages/react/src/select/root/useSelectRoot.ts @@ -20,6 +20,14 @@ import type { SelectRootContext } from './SelectRootContext'; import type { SelectIndexContext } from './SelectIndexContext'; import { useAfterExitAnimation } from '../../utils/useAfterExitAnimation'; +const EMPTY_ARRAY: never[] = []; + +function isDisabled(element: HTMLElement | null) { + return ( + element == null || element.hasAttribute('disabled') || element.hasAttribute('data-disabled') + ); +} + export function useSelectRoot(params: useSelectRoot.Parameters): useSelectRoot.ReturnValue { const { id: idProp, @@ -175,8 +183,10 @@ export function useSelectRoot(params: useSelectRoot.Parameters): useSelect }, }); + const triggerDisabled = isDisabled(triggerElement); + const click = useClick(floatingRootContext, { - enabled: !readOnly, + enabled: !readOnly && !disabled && !triggerDisabled, event: 'mousedown', }); @@ -190,10 +200,11 @@ export function useSelectRoot(params: useSelectRoot.Parameters): useSelect }); const listNavigation = useListNavigation(floatingRootContext, { - enabled: !readOnly, + enabled: !readOnly && !disabled, listRef, activeIndex, selectedIndex, + disabledIndices: EMPTY_ARRAY, onNavigate(nextActiveIndex) { // Retain the highlight while transitioning out. if (nextActiveIndex === null && !open) { @@ -207,8 +218,8 @@ export function useSelectRoot(params: useSelectRoot.Parameters): useSelect focusItemOnHover: false, }); - const typehaead = useTypeahead(floatingRootContext, { - enabled: !readOnly, + const typeahead = useTypeahead(floatingRootContext, { + enabled: !readOnly && !disabled, listRef: labelsRef, activeIndex, selectedIndex, @@ -230,7 +241,7 @@ export function useSelectRoot(params: useSelectRoot.Parameters): useSelect getReferenceProps: getRootTriggerProps, getFloatingProps: getRootPositionerProps, getItemProps, - } = useInteractions([click, dismiss, role, listNavigation, typehaead]); + } = useInteractions([click, dismiss, role, listNavigation, typeahead]); const rootContext = React.useMemo( () => ({ diff --git a/packages/react/src/select/trigger/SelectTrigger.test.tsx b/packages/react/src/select/trigger/SelectTrigger.test.tsx index 8a8edac5e2..5e1bbc70ab 100644 --- a/packages/react/src/select/trigger/SelectTrigger.test.tsx +++ b/packages/react/src/select/trigger/SelectTrigger.test.tsx @@ -2,7 +2,8 @@ import * as React from 'react'; import { Select } from '@base-ui-components/react/select'; import { createRenderer, describeConformance } from '#test-utils'; import { expect } from 'chai'; -import { act, screen } from '@mui/internal-test-utils'; +import { spy } from 'sinon'; +import { act, fireEvent, screen, waitFor } from '@mui/internal-test-utils'; describe('', () => { const { render } = createRenderer(); @@ -14,6 +15,61 @@ describe('', () => { }, })); + describe('disabled state', () => { + it('cannot be focused when disabled', async () => { + const { user } = await render( + + + + + + + + a + b + + + + , + ); + + const trigger = screen.getByTestId('trigger'); + expect(trigger).to.have.attribute('data-disabled'); + + await user.keyboard('[Tab]'); + + expect(expect(document.activeElement)).to.not.equal(trigger); + }); + + it('does not toggle the popup when disabled', async () => { + const handleOpenChange = spy(); + await render( + + + + + + + + a + b + + + + , + ); + + const trigger = screen.getByTestId('trigger'); + + fireEvent.click(trigger); + + await waitFor(() => { + expect(screen.queryByRole('listbox')).to.equal(null); + }); + expect(handleOpenChange.callCount).to.equal(0); + }); + }); + describe('style hooks', () => { it('should have the data-popup-open and data-pressed attributes when open', async () => { await render( diff --git a/packages/react/src/select/trigger/SelectTrigger.tsx b/packages/react/src/select/trigger/SelectTrigger.tsx index c9ff0b7f93..dd9b580777 100644 --- a/packages/react/src/select/trigger/SelectTrigger.tsx +++ b/packages/react/src/select/trigger/SelectTrigger.tsx @@ -35,8 +35,9 @@ const SelectTrigger = React.forwardRef(function SelectTrigger( () => ({ ...fieldState, open, + disabled, }), - [fieldState, open], + [fieldState, open, disabled], ); const { renderElement } = useComponentRenderer({ diff --git a/packages/react/src/select/trigger/useSelectTrigger.ts b/packages/react/src/select/trigger/useSelectTrigger.ts index a41b50017b..bd0b6a5f88 100644 --- a/packages/react/src/select/trigger/useSelectTrigger.ts +++ b/packages/react/src/select/trigger/useSelectTrigger.ts @@ -73,12 +73,11 @@ export function useSelectTrigger( const getTriggerProps = React.useCallback( (externalProps?: GenericHTMLProps): GenericHTMLProps => { - return mergeReactProps<'button'>( - fieldControlValidation.getValidationProps(externalProps), - { + return getButtonProps( + mergeReactProps<'button'>(fieldControlValidation.getValidationProps(externalProps), { 'aria-labelledby': labelId, 'aria-readonly': readOnly || undefined, - tabIndex: 0, // this is needed to make the button focused after click in Safari + tabIndex: disabled ? -1 : 0, // this is needed to make the button focused after click in Safari ref: handleRef, onFocus() { // The popup element shouldn't obscure the focused trigger. @@ -138,23 +137,23 @@ export function useSelectTrigger( doc.addEventListener('mouseup', handleMouseUp, { once: true }); }); }, - }, - getButtonProps(), + }), ); }, [ + alignItemToTrigger, + disabled, fieldControlValidation, - labelId, - readOnly, - handleRef, getButtonProps, + handleRef, + labelId, open, - alignItemToTrigger, + positionerElement, + readOnly, setOpen, setTouched, - value, setTouchModality, - positionerElement, + value, ], ); diff --git a/packages/react/src/use-button/useButton.test.tsx b/packages/react/src/use-button/useButton.test.tsx index 946f67ed82..aede0952bc 100644 --- a/packages/react/src/use-button/useButton.test.tsx +++ b/packages/react/src/use-button/useButton.test.tsx @@ -3,12 +3,66 @@ import { expect } from 'chai'; import { spy } from 'sinon'; import { act, fireEvent } from '@mui/internal-test-utils'; import { createRenderer, isJSDOM } from '#test-utils'; -import { useButton } from '.'; +import { useButton } from './useButton'; describe('useButton', () => { const { render, renderToString } = createRenderer(); - describe('tabIndex', () => { + describe('param: focusableWhenDisabled', () => { + it('allows disabled buttons to be focused', async () => { + function TestButton(props: React.ButtonHTMLAttributes) { + const { disabled, ...otherProps } = props; + const { getButtonProps } = useButton({ disabled, focusableWhenDisabled: true }); + + return