From 1fe70019c606d65949e557633bfa68fb781c7997 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 19 Apr 2021 15:33:30 +0200 Subject: [PATCH] add `disabled` prop to RadioGroup and RadioGroup Option Also did some general cleanup which in turn fixed an issue where the RadioGroup is unreachable when a value is used that doesn't exist in the list of options. Fixes: #378 --- .../radio-group/radio-group.test.tsx | 134 ++++++++++++++- .../components/radio-group/radio-group.tsx | 148 ++++++++++------ .../radio-group/radio-group.test.ts | 162 +++++++++++++++++- .../src/components/radio-group/radio-group.ts | 76 +++++--- .../src/test-utils/vue-testing-library.ts | 1 + 5 files changed, 439 insertions(+), 82 deletions(-) diff --git a/packages/@headlessui-react/src/components/radio-group/radio-group.test.tsx b/packages/@headlessui-react/src/components/radio-group/radio-group.test.tsx index 5f2030bebb..0509a88174 100644 --- a/packages/@headlessui-react/src/components/radio-group/radio-group.test.tsx +++ b/packages/@headlessui-react/src/components/radio-group/radio-group.test.tsx @@ -55,7 +55,7 @@ describe('Safe guards', () => { }) describe('Rendering', () => { - it('should be possible to render a RadioGroup, where the first element is tabbable', async () => { + it('should be possible to render a RadioGroup, where the first element is tabbable (value is undefined)', async () => { render( Pizza Delivery @@ -72,6 +72,23 @@ describe('Rendering', () => { assertNotFocusable(getByText('Dine in')) }) + it('should be possible to render a RadioGroup, where the first element is tabbable (value is null)', async () => { + render( + + Pizza Delivery + Pickup + Home delivery + Dine in + + ) + + expect(getRadioGroupOptions()).toHaveLength(3) + + assertFocusable(getByText('Pickup')) + assertNotFocusable(getByText('Home delivery')) + assertNotFocusable(getByText('Dine in')) + }) + it('should be possible to render a RadioGroup with an active value', async () => { render( @@ -120,6 +137,121 @@ describe('Rendering', () => { await press(Keys.ArrowUp) // Up again assertActiveElement(getByText('Home delivery')) }) + + it('should be possible to disable a RadioGroup', async () => { + let changeFn = jest.fn() + + function Example() { + let [disabled, setDisabled] = useState(true) + return ( + <> + + + Pizza Delivery + Pickup + Home delivery + Dine in + + {JSON.stringify} + + + + ) + } + + render() + + // Try to click one a few options + await click(getByText('Pickup')) + await click(getByText('Dine in')) + + // Verify that the RadioGroup.Option gets the disabled state + expect(document.querySelector('[data-value="render-prop"]')).toHaveTextContent( + JSON.stringify({ + checked: false, + disabled: true, + active: false, + }) + ) + + // Make sure that the onChange handler never got called + expect(changeFn).toHaveBeenCalledTimes(0) + + // Toggle the disabled state + await click(getByText('Toggle')) + + // Verify that the RadioGroup.Option gets the disabled state + expect(document.querySelector('[data-value="render-prop"]')).toHaveTextContent( + JSON.stringify({ + checked: false, + disabled: false, + active: false, + }) + ) + + // Try to click one a few options + await click(getByText('Pickup')) + + // Make sure that the onChange handler got called + expect(changeFn).toHaveBeenCalledTimes(1) + }) + + it('should be possible to disable a RadioGroup.Option', async () => { + let changeFn = jest.fn() + + function Example() { + let [disabled, setDisabled] = useState(true) + return ( + <> + + + Pizza Delivery + Pickup + Home delivery + Dine in + + {JSON.stringify} + + + + ) + } + + render() + + // Try to click the disabled option + await click(document.querySelector('[data-value="render-prop"]')) + + // Verify that the RadioGroup.Option gets the disabled state + expect(document.querySelector('[data-value="render-prop"]')).toHaveTextContent( + JSON.stringify({ + checked: false, + disabled: true, + active: false, + }) + ) + + // Make sure that the onChange handler never got called + expect(changeFn).toHaveBeenCalledTimes(0) + + // Toggle the disabled state + await click(getByText('Toggle')) + + // Verify that the RadioGroup.Option gets the disabled state + expect(document.querySelector('[data-value="render-prop"]')).toHaveTextContent( + JSON.stringify({ + checked: false, + disabled: false, + active: false, + }) + ) + + // Try to click one a few options + await click(document.querySelector('[data-value="render-prop"]')) + + // Make sure that the onChange handler got called + expect(changeFn).toHaveBeenCalledTimes(1) + }) }) describe('Keyboard interactions', () => { diff --git a/packages/@headlessui-react/src/components/radio-group/radio-group.tsx b/packages/@headlessui-react/src/components/radio-group/radio-group.tsx index d4934b6c43..0716c95809 100644 --- a/packages/@headlessui-react/src/components/radio-group/radio-group.tsx +++ b/packages/@headlessui-react/src/components/radio-group/radio-group.tsx @@ -7,10 +7,10 @@ import React, { useRef, // Types - Dispatch, ElementType, MutableRefObject, KeyboardEvent as ReactKeyboardEvent, + ContextType, } from 'react' import { Props, Expand } from '../../types' @@ -28,11 +28,10 @@ import { useTreeWalker } from '../../hooks/use-tree-walker' interface Option { id: string element: MutableRefObject - propsRef: MutableRefObject<{ value: unknown }> + propsRef: MutableRefObject<{ value: unknown; disabled: boolean }> } interface StateDefinition { - propsRef: MutableRefObject<{ value: unknown; onChange(value: unknown): void }> options: Option[] } @@ -69,7 +68,14 @@ let reducers: { }, } -let RadioGroupContext = createContext<[StateDefinition, Dispatch] | null>(null) +let RadioGroupContext = createContext<{ + registerOption(option: Option): () => void + change(value: unknown): boolean + value: unknown + firstOption?: Option + containsCheckedOption: boolean + disabled: boolean +} | null>(null) RadioGroupContext.displayName = 'RadioGroupContext' function useRadioGroupContext(component: string) { @@ -106,30 +112,40 @@ export function RadioGroup< disabled?: boolean } ) { - let { value, onChange, ...passThroughProps } = props - let reducerBag = useReducer(stateReducer, { - propsRef: { current: { value, onChange } }, + let { value, onChange, disabled = false, ...passThroughProps } = props + let [{ options }, dispatch] = useReducer(stateReducer, { options: [], } as StateDefinition) - let [{ propsRef, options }] = reducerBag let [labelledby, LabelProvider] = useLabels() let [describedby, DescriptionProvider] = useDescriptions() let id = `headlessui-radiogroup-${useId()}` let radioGroupRef = useRef(null) - useIsoMorphicEffect(() => { - propsRef.current.value = value - }, [value, propsRef]) - useIsoMorphicEffect(() => { - propsRef.current.onChange = onChange - }, [onChange, propsRef]) + let firstOption = useMemo( + () => + options.find(option => { + if (option.propsRef.current.disabled) return false + return true + }), + [options] + ) + let containsCheckedOption = useMemo( + () => options.some(option => option.propsRef.current.value === value), + [options, value] + ) let triggerChange = useCallback( nextValue => { - if (nextValue === value) return - return onChange(nextValue) + if (disabled) return false + if (nextValue === value) return false + let nextOption = options.find(option => option.propsRef.current.value === nextValue)?.propsRef + .current + if (nextOption?.disabled) return false + + onChange(nextValue) + return true }, - [onChange, value] + [onChange, value, disabled, options] ) useTreeWalker({ @@ -149,6 +165,10 @@ export function RadioGroup< let container = radioGroupRef.current if (!container) return + let all = options + .filter(option => option.propsRef.current.disabled === false) + .map(radio => radio.element.current) as HTMLElement[] + switch (event.key) { case Keys.ArrowLeft: case Keys.ArrowUp: @@ -156,10 +176,7 @@ export function RadioGroup< event.preventDefault() event.stopPropagation() - let result = focusIn( - options.map(radio => radio.element.current) as HTMLElement[], - Focus.Previous | Focus.WrapAround - ) + let result = focusIn(all, Focus.Previous | Focus.WrapAround) if (result === FocusResult.Success) { let activeOption = options.find( @@ -176,10 +193,7 @@ export function RadioGroup< event.preventDefault() event.stopPropagation() - let result = focusIn( - options.map(option => option.element.current) as HTMLElement[], - Focus.Next | Focus.WrapAround - ) + let result = focusIn(all, Focus.Next | Focus.WrapAround) if (result === FocusResult.Success) { let activeOption = options.find( @@ -206,6 +220,26 @@ export function RadioGroup< [radioGroupRef, options, triggerChange] ) + let registerOption = useCallback( + (option: Option) => { + dispatch({ type: ActionTypes.RegisterOption, ...option }) + return () => dispatch({ type: ActionTypes.UnregisterOption, id: option.id }) + }, + [dispatch] + ) + + let api = useMemo>( + () => ({ + registerOption, + firstOption, + containsCheckedOption, + change: triggerChange, + disabled, + value, + }), + [triggerChange, firstOption, containsCheckedOption, disabled, value] + ) + let propsWeControl = { ref: radioGroupRef, id, @@ -218,7 +252,7 @@ export function RadioGroup< return ( - + {render({ props: { ...passThroughProps, ...propsWeControl }, defaultTag: DEFAULT_RADIO_GROUP_TAG, @@ -241,6 +275,7 @@ let DEFAULT_OPTION_TAG = 'div' as const interface OptionRenderPropArg { checked: boolean active: boolean + disabled: boolean } type RadioPropsWeControl = | 'aria-checked' @@ -258,8 +293,9 @@ function Option< // But today is not that day.. TType = Parameters[0]['value'] >( - props: Props & { + props: Props & { value: TType + disabled?: boolean } ) { let optionRef = useRef(null) @@ -269,35 +305,46 @@ function Option< let [describedby, DescriptionProvider] = useDescriptions() let { addFlag, removeFlag, hasFlag } = useFlags(OptionState.Empty) - let { value, ...passThroughProps } = props - let propsRef = useRef({ value }) + let { value, disabled = false, ...passThroughProps } = props + let propsRef = useRef({ value, disabled }) useIsoMorphicEffect(() => { propsRef.current.value = value }, [value, propsRef]) - - let [{ propsRef: radioGroupPropsRef, options }, dispatch] = useRadioGroupContext( - [RadioGroup.name, Option.name].join('.') - ) - useIsoMorphicEffect(() => { - dispatch({ type: ActionTypes.RegisterOption, id, element: optionRef, propsRef }) - return () => dispatch({ type: ActionTypes.UnregisterOption, id }) - }, [id, dispatch, optionRef, props]) + propsRef.current.disabled = disabled + }, [disabled, propsRef]) + + let { + registerOption, + disabled: radioGroupDisabled, + change, + firstOption, + containsCheckedOption, + value: radioGroupValue, + } = useRadioGroupContext([RadioGroup.name, Option.name].join('.')) + + useIsoMorphicEffect(() => registerOption({ id, element: optionRef, propsRef }), [ + id, + registerOption, + optionRef, + props, + ]) let handleClick = useCallback(() => { - if (radioGroupPropsRef.current.value === value) return + if (!change(value)) return addFlag(OptionState.Active) - radioGroupPropsRef.current.onChange(value) optionRef.current?.focus() - }, [addFlag, radioGroupPropsRef, value]) + }, [addFlag, change, value]) let handleFocus = useCallback(() => addFlag(OptionState.Active), [addFlag]) let handleBlur = useCallback(() => removeFlag(OptionState.Active), [removeFlag]) - let firstRadio = options?.[0]?.id === id - let checked = radioGroupPropsRef.current.value === value + let isFirstOption = firstOption?.id === id + let isDisabled = radioGroupDisabled || disabled + + let checked = radioGroupValue === value let propsWeControl = { ref: optionRef, id, @@ -305,14 +352,19 @@ function Option< 'aria-checked': checked ? 'true' : 'false', 'aria-labelledby': labelledby, 'aria-describedby': describedby, - tabIndex: checked ? 0 : radioGroupPropsRef.current.value === undefined && firstRadio ? 0 : -1, - onClick: handleClick, - onFocus: handleFocus, - onBlur: handleBlur, + tabIndex: (() => { + if (isDisabled) return -1 + if (checked) return 0 + if (!containsCheckedOption && isFirstOption) return 0 + return -1 + })(), + onClick: isDisabled ? undefined : handleClick, + onFocus: isDisabled ? undefined : handleFocus, + onBlur: isDisabled ? undefined : handleBlur, } let slot = useMemo( - () => ({ checked, active: hasFlag(OptionState.Active) }), - [checked, hasFlag] + () => ({ checked, disabled: isDisabled, active: hasFlag(OptionState.Active) }), + [checked, hasFlag, radioGroupDisabled, isDisabled] ) return ( diff --git a/packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts b/packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts index 29a6a304ca..caec60dcd1 100644 --- a/packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts +++ b/packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts @@ -1,5 +1,6 @@ import { defineComponent, nextTick, ref, watch } from 'vue' import { render } from '../../test-utils/vue-testing-library' +import { screen } from '@testing-library/dom' import { RadioGroup, RadioGroupOption, RadioGroupLabel, RadioGroupDescription } from './radio-group' @@ -88,7 +89,7 @@ describe('Safe guards', () => { }) describe('Rendering', () => { - it('should be possible to render a RadioGroup, where the first element is tabbable', async () => { + it('should be possible to render a RadioGroup, where the first element is tabbable (value is undefined)', async () => { renderTemplate({ template: html` @@ -113,6 +114,31 @@ describe('Rendering', () => { assertNotFocusable(getByText('Dine in')) }) + it('should be possible to render a RadioGroup, where the first element is tabbable (value is null)', async () => { + renderTemplate({ + template: html` + + Pizza Delivery + Pickup + Home delivery + Dine in + + `, + setup() { + let deliveryMethod = ref(null) + return { deliveryMethod } + }, + }) + + await new Promise(nextTick) + + expect(getRadioGroupOptions()).toHaveLength(3) + + assertFocusable(getByText('Pickup')) + assertNotFocusable(getByText('Home delivery')) + assertNotFocusable(getByText('Dine in')) + }) + it('should be possible to render a RadioGroup with an active value', async () => { renderTemplate({ template: html` @@ -189,7 +215,7 @@ describe('Rendering', () => { await new Promise(nextTick) expect(document.querySelector('[id^="headlessui-radiogroup-option-"]')).toHaveTextContent( - `Pickup - ${JSON.stringify({ checked: false, active: false })}` + `Pickup - ${JSON.stringify({ checked: false, disabled: false, active: false })}` ) }) @@ -220,13 +246,13 @@ describe('Rendering', () => { document.querySelectorAll('[id^="headlessui-radiogroup-option-"]') ) expect(pickup).toHaveTextContent( - `Pickup - ${JSON.stringify({ checked: false, active: false })}` + `Pickup - ${JSON.stringify({ checked: false, disabled: false, active: false })}` ) expect(homeDelivery).toHaveTextContent( - `Home delivery - ${JSON.stringify({ checked: false, active: false })}` + `Home delivery - ${JSON.stringify({ checked: false, disabled: false, active: false })}` ) expect(dineIn).toHaveTextContent( - `Dine in - ${JSON.stringify({ checked: false, active: false })}` + `Dine in - ${JSON.stringify({ checked: false, disabled: false, active: false })}` ) await click(homeDelivery) @@ -235,13 +261,13 @@ describe('Rendering', () => { ) expect(pickup).toHaveTextContent( - `Pickup - ${JSON.stringify({ checked: false, active: false })}` + `Pickup - ${JSON.stringify({ checked: false, disabled: false, active: false })}` ) expect(homeDelivery).toHaveTextContent( - `Home delivery - ${JSON.stringify({ checked: true, active: true })}` + `Home delivery - ${JSON.stringify({ checked: true, disabled: false, active: true })}` ) expect(dineIn).toHaveTextContent( - `Dine in - ${JSON.stringify({ checked: false, active: false })}` + `Dine in - ${JSON.stringify({ checked: false, disabled: false, active: false })}` ) }) @@ -293,6 +319,126 @@ describe('Rendering', () => { expect(getByText('Pickup')).toHaveClass('abc') }) + + it('should be possible to disable a RadioGroup', async () => { + let changeFn = jest.fn() + renderTemplate({ + template: html` + + + Pizza Delivery + Pickup + Home delivery + Dine in + + {{JSON.stringify(data)}} + + + `, + setup() { + let deliveryMethod = ref(undefined) + let disabled = ref(true) + watch([deliveryMethod], () => changeFn(deliveryMethod.value)) + return { deliveryMethod, disabled } + }, + }) + + // Try to click one a few options + await click(getByText('Pickup')) + await click(getByText('Dine in')) + + // Verify that the RadioGroup.Option gets the disabled state + expect(document.querySelector('[data-value="render-prop"]')).toHaveTextContent( + JSON.stringify({ + checked: false, + disabled: true, + active: false, + }) + ) + + // Make sure that the onChange handler never got called + expect(changeFn).toHaveBeenCalledTimes(0) + + // Toggle the disabled state + await click(getByText('Toggle')) + + // Verify that the RadioGroup.Option gets the disabled state + expect(document.querySelector('[data-value="render-prop"]')).toHaveTextContent( + JSON.stringify({ + checked: false, + disabled: false, + active: false, + }) + ) + + // Try to click one a few options + await click(getByText('Pickup')) + + // Make sure that the onChange handler got called + expect(changeFn).toHaveBeenCalledTimes(1) + }) + + it('should be possible to disable a RadioGroup.Option', async () => { + let changeFn = jest.fn() + renderTemplate({ + template: html` + + + Pizza Delivery + Pickup + Home delivery + Dine in + + {{JSON.stringify(data)}} + + + `, + setup() { + let deliveryMethod = ref(undefined) + let disabled = ref(true) + watch([deliveryMethod], () => changeFn(deliveryMethod.value)) + return { deliveryMethod, disabled } + }, + }) + + // Try to click the disabled option + await click(document.querySelector('[data-value="render-prop"]')) + + // Verify that the RadioGroup.Option gets the disabled state + expect(document.querySelector('[data-value="render-prop"]')).toHaveTextContent( + JSON.stringify({ + checked: false, + disabled: true, + active: false, + }) + ) + + // Make sure that the onChange handler never got called + expect(changeFn).toHaveBeenCalledTimes(0) + + // Toggle the disabled state + await click(getByText('Toggle')) + + // Verify that the RadioGroup.Option gets the disabled state + expect(document.querySelector('[data-value="render-prop"]')).toHaveTextContent( + JSON.stringify({ + checked: false, + disabled: false, + active: false, + }) + ) + + // Try to click one a few options + await click(document.querySelector('[data-value="render-prop"]')) + + // Make sure that the onChange handler got called + expect(changeFn).toHaveBeenCalledTimes(1) + }) }) describe('Keyboard interactions', () => { diff --git a/packages/@headlessui-vue/src/components/radio-group/radio-group.ts b/packages/@headlessui-vue/src/components/radio-group/radio-group.ts index 0286705b41..a18278e8ec 100644 --- a/packages/@headlessui-vue/src/components/radio-group/radio-group.ts +++ b/packages/@headlessui-vue/src/components/radio-group/radio-group.ts @@ -26,16 +26,19 @@ import { useTreeWalker } from '../../hooks/use-tree-walker' interface Option { id: string element: Ref - propsRef: Ref<{ value: unknown }> + propsRef: Ref<{ value: unknown; disabled: boolean }> } interface StateDefinition { // State options: Ref value: Ref + disabled: Ref + firstOption: Ref