From 4e97d4b3eb330cc95d76a159ea7b710b3578c599 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Fri, 21 Aug 2020 13:49:53 -0700 Subject: [PATCH 1/9] Add Spectrum CheckboxGroup component --- .../checkbox/src/useCheckboxGroup.ts | 9 +- .../checkbox/src/useCheckboxGroupItem.ts | 20 +- packages/@react-aria/checkbox/src/utils.ts | 15 + .../checkbox/test/useCheckboxGroup.test.tsx | 53 +++- .../@react-spectrum/checkbox/package.json | 2 + .../@react-spectrum/checkbox/src/Checkbox.tsx | 22 +- .../checkbox/src/CheckboxGroup.tsx | 100 +++++++ .../@react-spectrum/checkbox/src/context.ts | 16 ++ .../@react-spectrum/checkbox/src/index.ts | 1 + .../stories/CheckboxGroup.stories.tsx | 102 +++++++ .../checkbox/test/CheckboxGroup.test.js | 259 ++++++++++++++++++ ...ate.mdx => useCheckboxGroupState.mdx-temp} | 0 .../checkbox/src/useCheckboxGroupState.ts | 33 ++- .../test/useCheckboxGroupState.test.tsx | 20 +- packages/@react-types/checkbox/src/index.d.ts | 35 ++- 15 files changed, 639 insertions(+), 48 deletions(-) create mode 100644 packages/@react-aria/checkbox/src/utils.ts create mode 100644 packages/@react-spectrum/checkbox/src/CheckboxGroup.tsx create mode 100644 packages/@react-spectrum/checkbox/src/context.ts create mode 100644 packages/@react-spectrum/checkbox/stories/CheckboxGroup.stories.tsx create mode 100644 packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js rename packages/@react-stately/checkbox/docs/{useCheckboxGroupState.mdx => useCheckboxGroupState.mdx-temp} (100%) diff --git a/packages/@react-aria/checkbox/src/useCheckboxGroup.ts b/packages/@react-aria/checkbox/src/useCheckboxGroup.ts index 28be095a42e..2a78648f75c 100644 --- a/packages/@react-aria/checkbox/src/useCheckboxGroup.ts +++ b/packages/@react-aria/checkbox/src/useCheckboxGroup.ts @@ -11,6 +11,8 @@ */ import {AriaCheckboxGroupProps} from '@react-types/checkbox'; +import {checkboxGroupNames} from './utils'; +import {CheckboxGroupState} from '@react-stately/checkbox'; import {filterDOMProps, mergeProps} from '@react-aria/utils'; import {HTMLAttributes} from 'react'; import {useLabel} from '@react-aria/label'; @@ -28,8 +30,8 @@ interface CheckboxGroupAria { * @param props - Props for the checkbox group. * @param state - State for the checkbox group, as returned by `useCheckboxGroupState`. */ -export function useCheckboxGroup(props: AriaCheckboxGroupProps): CheckboxGroupAria { - let {isDisabled} = props; +export function useCheckboxGroup(props: AriaCheckboxGroupProps, state: CheckboxGroupState): CheckboxGroupAria { + let {isDisabled, name} = props; let {labelProps, fieldProps} = useLabel({ ...props, @@ -40,6 +42,9 @@ export function useCheckboxGroup(props: AriaCheckboxGroupProps): CheckboxGroupAr let domProps = filterDOMProps(props, {labelable: true}); + // Pass name prop from group to all items by attaching to the state. + checkboxGroupNames.set(state, name); + return { checkboxGroupProps: mergeProps(domProps, { role: 'group', diff --git a/packages/@react-aria/checkbox/src/useCheckboxGroupItem.ts b/packages/@react-aria/checkbox/src/useCheckboxGroupItem.ts index 5a917eca608..1bb925e88eb 100644 --- a/packages/@react-aria/checkbox/src/useCheckboxGroupItem.ts +++ b/packages/@react-aria/checkbox/src/useCheckboxGroupItem.ts @@ -12,6 +12,7 @@ import {AriaCheckboxGroupItemProps} from '@react-types/checkbox'; import {CheckboxAria, useCheckbox} from './useCheckbox'; +import {checkboxGroupNames} from './utils'; import {CheckboxGroupState} from '@react-stately/checkbox'; import {RefObject} from 'react'; import {useToggleState} from '@react-stately/toggle'; @@ -25,8 +26,8 @@ import {useToggleState} from '@react-stately/toggle'; */ export function useCheckboxGroupItem(props: AriaCheckboxGroupItemProps, state: CheckboxGroupState, inputRef: RefObject): CheckboxAria { const toggleState = useToggleState({ - isReadOnly: props.isReadOnly, - isSelected: state.value.includes(props.value), + isReadOnly: props.isReadOnly || state.isReadOnly, + isSelected: state.isSelected(props.value), onChange(isSelected) { if (isSelected) { state.addValue(props.value); @@ -39,5 +40,18 @@ export function useCheckboxGroupItem(props: AriaCheckboxGroupItemProps, state: C } } }); - return useCheckbox({...props, name: state.name}, toggleState, inputRef); + + let {inputProps} = useCheckbox({ + ...props, + isReadOnly: props.isReadOnly || state.isReadOnly, + isDisabled: props.isDisabled || state.isDisabled, + name: checkboxGroupNames.get(state) + }, toggleState, inputRef); + + // Delete required and aria-invalid from inputProps since they don't really make sense + // when used in a checkbox group. These will be communicated by the group itself. + delete inputProps.required; + delete inputProps['aria-invalid']; + + return {inputProps}; } diff --git a/packages/@react-aria/checkbox/src/utils.ts b/packages/@react-aria/checkbox/src/utils.ts new file mode 100644 index 00000000000..3161b05cd4c --- /dev/null +++ b/packages/@react-aria/checkbox/src/utils.ts @@ -0,0 +1,15 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {CheckboxGroupState} from '@react-stately/checkbox'; + +export const checkboxGroupNames = new WeakMap(); diff --git a/packages/@react-aria/checkbox/test/useCheckboxGroup.test.tsx b/packages/@react-aria/checkbox/test/useCheckboxGroup.test.tsx index c1f2e477c58..0363a38e446 100644 --- a/packages/@react-aria/checkbox/test/useCheckboxGroup.test.tsx +++ b/packages/@react-aria/checkbox/test/useCheckboxGroup.test.tsx @@ -26,7 +26,7 @@ function Checkbox({checkboxGroupState, ...props}: AriaCheckboxProps & { checkbox function CheckboxGroup({groupProps, checkboxProps}: {groupProps: AriaCheckboxGroupProps, checkboxProps: AriaCheckboxProps[]}) { const state = useCheckboxGroupState(groupProps); - const {checkboxGroupProps, labelProps} = useCheckboxGroup(groupProps); + const {checkboxGroupProps, labelProps} = useCheckboxGroup(groupProps, state); return (
{groupProps.label && {groupProps.label}} @@ -55,10 +55,9 @@ describe('useCheckboxGroup', () => { expect(checkboxGroup).toBeInTheDocument(); expect(checkboxes.length).toBe(3); - let groupName = checkboxes[0].getAttribute('name'); - expect(checkboxes[0]).toHaveAttribute('name', groupName); - expect(checkboxes[1]).toHaveAttribute('name', groupName); - expect(checkboxes[2]).toHaveAttribute('name', groupName); + expect(checkboxes[0]).not.toHaveAttribute('name'); + expect(checkboxes[1]).not.toHaveAttribute('name'); + expect(checkboxes[2]).not.toHaveAttribute('name'); expect(checkboxes[0].value).toBe('dogs'); expect(checkboxes[1].value).toBe('cats'); @@ -160,7 +159,7 @@ describe('useCheckboxGroup', () => { }); it('sets aria-disabled when isDisabled is true', () => { - let {getByRole} = render( + let {getAllByRole, getByRole} = render( { {value: 'dragons', children: 'Dragons'} ]} /> ); - let checkboxGroup = getByRole('group', {exact: true}); + let checkboxGroup = getByRole('group', {exact: true}); expect(checkboxGroup).toHaveAttribute('aria-disabled', 'true'); + + let checkboxes = getAllByRole('checkbox') as HTMLInputElement[]; + expect(checkboxes[0]).toHaveAttribute('disabled'); + expect(checkboxes[0]).toHaveAttribute('disabled'); + expect(checkboxes[0]).toHaveAttribute('disabled'); }); it('doesn\'t set aria-disabled by default', () => { - let {getByRole} = render( + let {getAllByRole, getByRole} = render( { {value: 'dragons', children: 'Dragons'} ]} /> ); - let checkboxGroup = getByRole('group', {exact: true}); + let checkboxGroup = getByRole('group', {exact: true}); expect(checkboxGroup).not.toHaveAttribute('aria-disabled'); + + let checkboxes = getAllByRole('checkbox') as HTMLInputElement[]; + expect(checkboxes[0]).not.toHaveAttribute('disabled'); + expect(checkboxes[0]).not.toHaveAttribute('disabled'); + expect(checkboxes[0]).not.toHaveAttribute('disabled'); }); it('doesn\'t set aria-disabled when isDisabled is false', () => { - let {getByRole} = render( + let {getAllByRole, getByRole} = render( { {value: 'dragons', children: 'Dragons'} ]} /> ); - let checkboxGroup = getByRole('group', {exact: true}); + let checkboxGroup = getByRole('group', {exact: true}); expect(checkboxGroup).not.toHaveAttribute('aria-disabled'); + + let checkboxes = getAllByRole('checkbox') as HTMLInputElement[]; + expect(checkboxes[0]).not.toHaveAttribute('disabled'); + expect(checkboxes[0]).not.toHaveAttribute('disabled'); + expect(checkboxes[0]).not.toHaveAttribute('disabled'); + }); + + it('sets readOnly on each checkbox', () => { + let {getAllByRole} = render( + + ); + + let checkboxes = getAllByRole('checkbox') as HTMLInputElement[]; + expect(checkboxes[0]).toHaveAttribute('readonly'); + expect(checkboxes[0]).toHaveAttribute('readonly'); + expect(checkboxes[0]).toHaveAttribute('readonly'); }); it('should not update state for readonly checkbox', () => { diff --git a/packages/@react-spectrum/checkbox/package.json b/packages/@react-spectrum/checkbox/package.json index fc186fb0bba..710f3fa16c6 100644 --- a/packages/@react-spectrum/checkbox/package.json +++ b/packages/@react-spectrum/checkbox/package.json @@ -35,6 +35,8 @@ "@react-aria/checkbox": "^3.1.0", "@react-aria/focus": "^3.1.0", "@react-aria/interactions": "^3.1.0", + "@react-spectrum/form": "3.2.0", + "@react-spectrum/label": "3.2.0", "@react-spectrum/utils": "^3.1.0", "@react-stately/checkbox": "^3.1.0", "@react-stately/toggle": "^3.1.0", diff --git a/packages/@react-spectrum/checkbox/src/Checkbox.tsx b/packages/@react-spectrum/checkbox/src/Checkbox.tsx index 717610aac00..a321b9c34c2 100644 --- a/packages/@react-spectrum/checkbox/src/Checkbox.tsx +++ b/packages/@react-spectrum/checkbox/src/Checkbox.tsx @@ -10,15 +10,16 @@ * governing permissions and limitations under the License. */ +import {CheckboxGroupContext} from './context'; import CheckmarkSmall from '@spectrum-icons/ui/CheckmarkSmall'; import {classNames, useFocusableRef, useStyleProps} from '@react-spectrum/utils'; import DashSmall from '@spectrum-icons/ui/DashSmall'; import {FocusableRef} from '@react-types/shared'; import {FocusRing} from '@react-aria/focus'; -import React, {forwardRef, useRef} from 'react'; +import React, {forwardRef, useContext, useRef} from 'react'; import {SpectrumCheckboxProps} from '@react-types/checkbox'; import styles from '@adobe/spectrum-css-temp/components/checkbox/vars.css'; -import {useCheckbox} from '@react-aria/checkbox'; +import {useCheckbox, useCheckboxGroupItem} from '@react-aria/checkbox'; import {useHover} from '@react-aria/interactions'; import {useProviderProps} from '@react-spectrum/provider'; import {useToggleState} from '@react-stately/toggle'; @@ -31,6 +32,7 @@ function Checkbox(props: SpectrumCheckboxProps, ref: FocusableRef(null); let domRef = useFocusableRef(ref, inputRef); - let state = useToggleState(props); - let {inputProps} = useCheckbox(props, state, inputRef); + + // Swap hooks depending on whether this checkbox is inside a CheckboxGroup. + // This is a bit unorthodox. Typically, hooks cannot be called in a conditional, + // but since the checkbox won't move in and out of a group, it should be safe. + let groupState = useContext(CheckboxGroupContext); + let {inputProps} = groupState + // eslint-disable-next-line react-hooks/rules-of-hooks + ? useCheckboxGroupItem(props, groupState, inputRef) + // eslint-disable-next-line react-hooks/rules-of-hooks + : useCheckbox(props, useToggleState(props), inputRef); let markIcon = isIndeterminate ? @@ -55,10 +65,10 @@ function Checkbox(props: SpectrumCheckboxProps, ref: FocusableRef) { + props = useProviderProps(props); + props = useFormProps(props); + let { + isEmphasized, + isRequired, + necessityIndicator, + label, + labelPosition = 'top' as LabelPosition, + labelAlign, + children, + orientation = 'vertical', + validationState, + ...otherProps + } = props; + let domRef = useDOMRef(ref); + let {styleProps} = useStyleProps(otherProps); + let state = useCheckboxGroupState(props); + let {labelProps, checkboxGroupProps} = useCheckboxGroup(props, state); + + return ( +
+ classNames( + labelStyles, + 'spectrum-Field' + ), + styleProps.className + ) + } + ref={domRef}> + {label && + + } +
+ + + {children} + + +
+
+ ); +} + +const _CheckboxGroup = React.forwardRef(CheckboxGroup); +export {_CheckboxGroup as CheckboxGroup}; diff --git a/packages/@react-spectrum/checkbox/src/context.ts b/packages/@react-spectrum/checkbox/src/context.ts new file mode 100644 index 00000000000..ffe139e6c57 --- /dev/null +++ b/packages/@react-spectrum/checkbox/src/context.ts @@ -0,0 +1,16 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {CheckboxGroupState} from '@react-stately/checkbox'; +import React from 'react'; + +export const CheckboxGroupContext = React.createContext(null); diff --git a/packages/@react-spectrum/checkbox/src/index.ts b/packages/@react-spectrum/checkbox/src/index.ts index 3f1ff02a498..111079ec916 100644 --- a/packages/@react-spectrum/checkbox/src/index.ts +++ b/packages/@react-spectrum/checkbox/src/index.ts @@ -13,3 +13,4 @@ /// export * from './Checkbox'; +export * from './CheckboxGroup'; diff --git a/packages/@react-spectrum/checkbox/stories/CheckboxGroup.stories.tsx b/packages/@react-spectrum/checkbox/stories/CheckboxGroup.stories.tsx new file mode 100644 index 00000000000..3e4c25fbd27 --- /dev/null +++ b/packages/@react-spectrum/checkbox/stories/CheckboxGroup.stories.tsx @@ -0,0 +1,102 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {action} from '@storybook/addon-actions'; +import {Checkbox, CheckboxGroup} from '../'; +import React from 'react'; +import {SpectrumCheckboxGroupProps} from '@react-types/checkbox'; +import {storiesOf} from '@storybook/react'; + +storiesOf('CheckboxGroup', module) + .addParameters({providerSwitcher: {status: 'positive'}}) + .add( + 'Default', + () => render() + ) + .add( + 'defaultValue: dragons', + () => render({defaultValue: ['dragons']}) + ) + .add( + 'controlled: dragons', + () => render({value: ['dragons']}) + ) + .add( + 'labelPosition: side', + () => render({labelPosition: 'side'}) + ) + .add( + 'labelAlign: end', + () => render({labelAlign: 'end'}) + ) + .add( + 'horizontal', + () => render({orientation: 'horizontal'}) + ) + .add( + 'horizontal, labelPosition: side', + () => render({orientation: 'horizontal', labelPosition: 'side'}) + ) + .add( + 'horizontal, labelAlign: end', + () => render({orientation: 'horizontal', labelAlign: 'end'}) + ) + .add( + 'isDisabled', + () => render({isDisabled: true}) + ) + .add( + 'isDisabled on one checkbox', + () => render({}, [{}, {isDisabled: true}, {}]) + ) + .add( + 'isDisabled on one checkbox horizontal', + () => render({orientation: 'horizontal'}, [{}, {isDisabled: true}, {}]) + ) + .add( + 'isRequired', + () => render({isRequired: true}) + ) + .add( + 'isReadOnly', + () => render({isReadOnly: true}) + ) + .add( + 'isEmphasized', + () => render({isEmphasized: true}) + ) + .add( + 'validationState: "invalid"', + () => render({validationState: 'invalid'}) + ) + .add( + 'no visible label', + () => render({label: null, 'aria-label': 'Pets'}) + ) + .add( + 'autoFocus on one checkbox', + () => render({}, [{}, {autoFocus: true}, {}]) + ) + .add( + 'form name', + () => render({name: 'pets'}) + ); + +function render(props: Omit = {}, checkboxProps: any[] = []) { + return ( + + Dogs + Cats + Dragons + + ); +} diff --git a/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js b/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js new file mode 100644 index 00000000000..72a91b2bb04 --- /dev/null +++ b/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js @@ -0,0 +1,259 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {act, render, within} from '@testing-library/react'; +import {Checkbox, CheckboxGroup} from '../'; +import {Provider} from '@react-spectrum/provider'; +import React from 'react'; +import {theme} from '@react-spectrum/theme-default'; +import userEvent from '@testing-library/user-event'; + +describe('CheckboxGroup', () => { + it('handles defaults', () => { + let onChangeSpy = jest.fn(); + let {getByRole, getAllByRole, getByLabelText} = render( + + + Dogs + Cats + Dragons + + + ); + + let checkboxGroup = getByRole('group', {exact: true}); + let checkboxes = getAllByRole('checkbox'); + expect(checkboxGroup).toBeInTheDocument(); + expect(checkboxes.length).toBe(3); + + expect(checkboxes[0]).not.toHaveAttribute('name'); + expect(checkboxes[1]).not.toHaveAttribute('name'); + expect(checkboxes[2]).not.toHaveAttribute('name'); + + expect(checkboxes[0].value).toBe('dogs'); + expect(checkboxes[1].value).toBe('cats'); + expect(checkboxes[2].value).toBe('dragons'); + + expect(checkboxes[0].checked).toBe(false); + expect(checkboxes[1].checked).toBe(false); + expect(checkboxes[2].checked).toBe(false); + + let dragons = getByLabelText('Dragons'); + act(() => {userEvent.click(dragons);}); + expect(onChangeSpy).toHaveBeenCalledTimes(1); + expect(onChangeSpy).toHaveBeenCalledWith(['dragons']); + + expect(checkboxes[0].checked).toBe(false); + expect(checkboxes[1].checked).toBe(false); + expect(checkboxes[2].checked).toBe(true); + }); + + it('can have a default value', () => { + let {getByLabelText} = render( + + + Dogs + Cats + Dragons + + + ); + + expect(getByLabelText('Cats').checked).toBe(true); + }); + + it('name can be controlled', () => { + let {getAllByRole} = render( + + + Dogs + Cats + Dragons + + + ); + + let checkboxes = getAllByRole('checkbox'); + + expect(checkboxes[0]).toHaveAttribute('name', 'awesome-react-aria'); + expect(checkboxes[1]).toHaveAttribute('name', 'awesome-react-aria'); + expect(checkboxes[2]).toHaveAttribute('name', 'awesome-react-aria'); + }); + + it('supports labeling', () => { + let {getByRole} = render( + + + Dogs + Cats + Dragons + + + ); + let checkboxGroup = getByRole('group', {exact: true}); + + let labelId = checkboxGroup.getAttribute('aria-labelledby'); + expect(labelId).toBeDefined(); + let label = document.getElementById(labelId); + expect(label).toHaveTextContent('Favorite Pet'); + }); + + it('supports aria-label', () => { + let {getByRole} = render( + + + Dogs + Cats + Dragons + + + ); + let checkboxGroup = getByRole('group', {exact: true}); + + expect(checkboxGroup).toHaveAttribute('aria-label', 'My Favorite Pet'); + }); + + it('supports custom props', () => { + let {getByRole} = render( + + + Dogs + Cats + Dragons + + + ); + let checkboxGroup = getByRole('group', {exact: true}); + + expect(checkboxGroup).toHaveAttribute('data-testid', 'favorite-pet'); + }); + + it('sets aria-disabled when isDisabled is true', () => { + let {getAllByRole, getByRole} = render( + + + Dogs + Cats + Dragons + + + ); + + let checkboxGroup = getByRole('group', {exact: true}); + expect(checkboxGroup).toHaveAttribute('aria-disabled', 'true'); + + let checkboxes = getAllByRole('checkbox'); + expect(checkboxes[0]).toHaveAttribute('disabled'); + expect(checkboxes[0]).toHaveAttribute('disabled'); + expect(checkboxes[0]).toHaveAttribute('disabled'); + }); + + it('doesn\'t set aria-disabled by default', () => { + let {getAllByRole, getByRole} = render( + + + Dogs + Cats + Dragons + + + ); + + let checkboxGroup = getByRole('group', {exact: true}); + expect(checkboxGroup).not.toHaveAttribute('aria-disabled'); + + let checkboxes = getAllByRole('checkbox'); + expect(checkboxes[0]).not.toHaveAttribute('disabled'); + expect(checkboxes[0]).not.toHaveAttribute('disabled'); + expect(checkboxes[0]).not.toHaveAttribute('disabled'); + }); + + it('doesn\'t set aria-disabled when isDisabled is false', () => { + let {getAllByRole, getByRole} = render( + + + Dogs + Cats + Dragons + + + ); + + let checkboxGroup = getByRole('group', {exact: true}); + expect(checkboxGroup).not.toHaveAttribute('aria-disabled'); + + let checkboxes = getAllByRole('checkbox'); + expect(checkboxes[0]).not.toHaveAttribute('disabled'); + expect(checkboxes[0]).not.toHaveAttribute('disabled'); + expect(checkboxes[0]).not.toHaveAttribute('disabled'); + }); + + it('sets readOnly on each checkbox', () => { + let {getAllByRole} = render( + + + Dogs + Cats + Dragons + + + ); + + let checkboxes = getAllByRole('checkbox'); + expect(checkboxes[0]).toHaveAttribute('readonly'); + expect(checkboxes[0]).toHaveAttribute('readonly'); + expect(checkboxes[0]).toHaveAttribute('readonly'); + }); + + it('should not update state for readonly checkbox', () => { + let groupOnChangeSpy = jest.fn(); + let checkboxOnChangeSpy = jest.fn(); + let {getAllByRole, getByLabelText} = render( + + + Dogs + Cats + Dragons + + + ); + + let checkboxes = getAllByRole('checkbox'); + let dragons = getByLabelText('Dragons'); + + act(() => {userEvent.click(dragons);}); + + expect(groupOnChangeSpy).toHaveBeenCalledTimes(0); + expect(checkboxOnChangeSpy).toHaveBeenCalledTimes(0); + expect(checkboxes[2].checked).toBe(false); + }); + + it('adds required to group label', () => { + let {getByRole} = render( + + + Dogs + Cats + Dragons + + + ); + + let checkboxGroup = getByRole('group', {exact: true}); + let labelId = checkboxGroup.getAttribute('aria-labelledby'); + let label = document.getElementById(labelId); + expect(label).toHaveTextContent('Favorite Pet'); + + let necessityIndicator = within(label).getByRole('img'); + expect(necessityIndicator).toHaveAttribute('aria-label', '(required)'); + }); +}); diff --git a/packages/@react-stately/checkbox/docs/useCheckboxGroupState.mdx b/packages/@react-stately/checkbox/docs/useCheckboxGroupState.mdx-temp similarity index 100% rename from packages/@react-stately/checkbox/docs/useCheckboxGroupState.mdx rename to packages/@react-stately/checkbox/docs/useCheckboxGroupState.mdx-temp diff --git a/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts b/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts index 918134e4801..e9865744b7a 100644 --- a/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts +++ b/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts @@ -12,15 +12,20 @@ import {CheckboxGroupProps} from '@react-types/checkbox'; import {useControlledState} from '@react-stately/utils'; -import {useMemo} from 'react'; export interface CheckboxGroupState { - /** The name for the group, used for native form submission. */ - readonly name: string, - /** Current selected values. */ readonly value: readonly string[], + /** Whether the checkbox group is disabled. */ + readonly isDisabled: boolean, + + /** Whether the checkbox group is read only. */ + readonly isReadOnly: boolean, + + /** Returns whether the given value is selected. */ + isSelected(value: string): boolean, + /** Sets the selected values. */ setValue(value: string[]): void, @@ -34,23 +39,23 @@ export interface CheckboxGroupState { toggleValue(value: string): void } -let instance = Math.round(Math.random() * 10000000000); -let i = 0; - /** * Provides state management for a checkbox group component. Provides a name for the group, * and manages selection and focus state. */ export function useCheckboxGroupState(props: CheckboxGroupProps = {}): CheckboxGroupState { - let name = useMemo(() => props.name || `checkbox-group-${instance}-${++i}`, [props.name]); - let [selectedValue, setValue] = useControlledState(props.value, props.defaultValue || [], props.onChange); + let [selectedValues, setValue] = useControlledState(props.value, props.defaultValue || [], props.onChange); const state: CheckboxGroupState = { - name, - value: selectedValue, + value: selectedValues, setValue, + isDisabled: props.isDisabled || false, + isReadOnly: props.isReadOnly || false, + isSelected(value) { + return selectedValues.includes(value); + }, addValue(value) { - if (props.isReadOnly) { + if (props.isReadOnly || props.isDisabled) { return; } setValue(values => { @@ -61,7 +66,7 @@ export function useCheckboxGroupState(props: CheckboxGroupProps = {}): CheckboxG }); }, removeValue(value) { - if (props.isReadOnly) { + if (props.isReadOnly || props.isDisabled) { return; } setValue(values => { @@ -72,7 +77,7 @@ export function useCheckboxGroupState(props: CheckboxGroupProps = {}): CheckboxG }); }, toggleValue(value) { - if (props.isReadOnly) { + if (props.isReadOnly || props.isDisabled) { return; } setValue(values => { diff --git a/packages/@react-stately/checkbox/test/useCheckboxGroupState.test.tsx b/packages/@react-stately/checkbox/test/useCheckboxGroupState.test.tsx index 912ee5302d2..82885b8e258 100644 --- a/packages/@react-stately/checkbox/test/useCheckboxGroupState.test.tsx +++ b/packages/@react-stately/checkbox/test/useCheckboxGroupState.test.tsx @@ -19,8 +19,9 @@ describe('useCheckboxGroupState', () => { function Test() { const state = useCheckboxGroupState(); - expect(typeof state.name).toBe('string'); expect(state.value).toEqual([]); + expect(state.isDisabled).toBe(false); + expect(state.isReadOnly).toBe(false); expect(typeof state.setValue).toBe('function'); expect(typeof state.addValue).toBe('function'); expect(typeof state.removeValue).toBe('function'); @@ -31,11 +32,22 @@ describe('useCheckboxGroupState', () => { render(); }); - it('should return the same `name` that has been provided', () => { + it('should return the same `isDisabled` that has been provided', () => { function Test() { - const state = useCheckboxGroupState({name: 'foo'}); + const state = useCheckboxGroupState({isDisabled: true}); - expect(state.name).toBe('foo'); + expect(state.isDisabled).toBe(true); + + return null; + } + render(); + }); + + it('should return the same `isReadOnly` that has been provided', () => { + function Test() { + const state = useCheckboxGroupState({isReadOnly: true}); + + expect(state.isReadOnly).toBe(true); return null; } diff --git a/packages/@react-types/checkbox/src/index.d.ts b/packages/@react-types/checkbox/src/index.d.ts index 92253e59063..ac79c4d352e 100644 --- a/packages/@react-types/checkbox/src/index.d.ts +++ b/packages/@react-types/checkbox/src/index.d.ts @@ -18,11 +18,13 @@ import { FocusableProps, InputBase, LabelableProps, + Orientation, + SpectrumLabelableProps, StyleProps, Validation, ValueBase } from '@react-types/shared'; -import {ReactNode} from 'react'; +import {ReactElement, ReactNode} from 'react'; export interface ToggleProps extends InputBase, Validation, FocusableProps { /** @@ -58,12 +60,7 @@ export interface AriaToggleProps extends ToggleProps, FocusableDOMProps, AriaLab 'aria-controls'?: string } -export interface CheckboxGroupProps extends ValueBase, InputBase, LabelableProps { - /** - * The name of the CheckboxGroup, used when submitting an HTML form. - */ - name?: string -} +export interface CheckboxGroupProps extends ValueBase, InputBase, LabelableProps {} export interface CheckboxProps extends ToggleProps { /** @@ -75,7 +72,12 @@ export interface CheckboxProps extends ToggleProps { export interface AriaCheckboxProps extends CheckboxProps, AriaToggleProps {} -export interface AriaCheckboxGroupProps extends CheckboxGroupProps, DOMProps, AriaLabelingProps, AriaValidationProps {} +export interface AriaCheckboxGroupProps extends CheckboxGroupProps, DOMProps, AriaLabelingProps, AriaValidationProps { + /** + * The name of the CheckboxGroup, used when submitting an HTML form. + */ + name?: string +} export interface AriaCheckboxGroupItemProps extends Omit {} @@ -85,3 +87,20 @@ export interface SpectrumCheckboxProps extends AriaCheckboxProps, StyleProps { */ isEmphasized?: boolean } + +export interface SpectrumCheckboxGroupProps extends AriaCheckboxGroupProps, SpectrumLabelableProps, Validation, StyleProps { + /** + * The Checkboxes contained within the CheckboxGroup. + */ + children: ReactElement | ReactElement[], + /** + * The axis the checkboxes should align with. + * @default 'vertical' + */ + orientation?: Orientation, + /** + * By default, checkboxes are not emphasized (gray). + * The emphasized (blue) version provides visual prominence. + */ + isEmphasized?: boolean +} From 4d9157ff69ecb03dab7277d1b637ebb2529b754d Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Fri, 21 Aug 2020 14:03:58 -0700 Subject: [PATCH 2/9] Rename checkboxGroupProps to groupProps to match ARIA role --- packages/@react-aria/checkbox/src/useCheckboxGroup.ts | 4 ++-- packages/@react-aria/checkbox/test/useCheckboxGroup.test.tsx | 2 +- packages/@react-spectrum/checkbox/src/CheckboxGroup.tsx | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/@react-aria/checkbox/src/useCheckboxGroup.ts b/packages/@react-aria/checkbox/src/useCheckboxGroup.ts index 2a78648f75c..ecf2e2f20aa 100644 --- a/packages/@react-aria/checkbox/src/useCheckboxGroup.ts +++ b/packages/@react-aria/checkbox/src/useCheckboxGroup.ts @@ -19,7 +19,7 @@ import {useLabel} from '@react-aria/label'; interface CheckboxGroupAria { /** Props for the checkbox group wrapper element. */ - checkboxGroupProps: HTMLAttributes, + groupProps: HTMLAttributes, /** Props for the checkbox group's visible label (if any). */ labelProps: HTMLAttributes } @@ -46,7 +46,7 @@ export function useCheckboxGroup(props: AriaCheckboxGroupProps, state: CheckboxG checkboxGroupNames.set(state, name); return { - checkboxGroupProps: mergeProps(domProps, { + groupProps: mergeProps(domProps, { role: 'group', 'aria-disabled': isDisabled || undefined, ...fieldProps diff --git a/packages/@react-aria/checkbox/test/useCheckboxGroup.test.tsx b/packages/@react-aria/checkbox/test/useCheckboxGroup.test.tsx index 0363a38e446..33b4ada87db 100644 --- a/packages/@react-aria/checkbox/test/useCheckboxGroup.test.tsx +++ b/packages/@react-aria/checkbox/test/useCheckboxGroup.test.tsx @@ -26,7 +26,7 @@ function Checkbox({checkboxGroupState, ...props}: AriaCheckboxProps & { checkbox function CheckboxGroup({groupProps, checkboxProps}: {groupProps: AriaCheckboxGroupProps, checkboxProps: AriaCheckboxProps[]}) { const state = useCheckboxGroupState(groupProps); - const {checkboxGroupProps, labelProps} = useCheckboxGroup(groupProps, state); + const {groupProps: checkboxGroupProps, labelProps} = useCheckboxGroup(groupProps, state); return (
{groupProps.label && {groupProps.label}} diff --git a/packages/@react-spectrum/checkbox/src/CheckboxGroup.tsx b/packages/@react-spectrum/checkbox/src/CheckboxGroup.tsx index 26f83209c06..b35091bf1cf 100644 --- a/packages/@react-spectrum/checkbox/src/CheckboxGroup.tsx +++ b/packages/@react-spectrum/checkbox/src/CheckboxGroup.tsx @@ -41,12 +41,12 @@ function CheckboxGroup(props: SpectrumCheckboxGroupProps, ref: DOMRef Date: Fri, 21 Aug 2020 17:14:36 -0700 Subject: [PATCH 3/9] Improve handling of individual checkbox props within a group --- .../checkbox/src/useCheckboxGroupItem.ts | 7 +- .../@react-spectrum/checkbox/src/Checkbox.tsx | 17 +++- .../stories/CheckboxGroup.stories.tsx | 4 + .../checkbox/test/CheckboxGroup.test.js | 98 +++++++++++++++++-- 4 files changed, 110 insertions(+), 16 deletions(-) diff --git a/packages/@react-aria/checkbox/src/useCheckboxGroupItem.ts b/packages/@react-aria/checkbox/src/useCheckboxGroupItem.ts index 1bb925e88eb..de7c42f9082 100644 --- a/packages/@react-aria/checkbox/src/useCheckboxGroupItem.ts +++ b/packages/@react-aria/checkbox/src/useCheckboxGroupItem.ts @@ -45,13 +45,8 @@ export function useCheckboxGroupItem(props: AriaCheckboxGroupItemProps, state: C ...props, isReadOnly: props.isReadOnly || state.isReadOnly, isDisabled: props.isDisabled || state.isDisabled, - name: checkboxGroupNames.get(state) + name: props.name || checkboxGroupNames.get(state) }, toggleState, inputRef); - // Delete required and aria-invalid from inputProps since they don't really make sense - // when used in a checkbox group. These will be communicated by the group itself. - delete inputProps.required; - delete inputProps['aria-invalid']; - return {inputProps}; } diff --git a/packages/@react-spectrum/checkbox/src/Checkbox.tsx b/packages/@react-spectrum/checkbox/src/Checkbox.tsx index a321b9c34c2..e92b2cf3d98 100644 --- a/packages/@react-spectrum/checkbox/src/Checkbox.tsx +++ b/packages/@react-spectrum/checkbox/src/Checkbox.tsx @@ -25,6 +25,7 @@ import {useProviderProps} from '@react-spectrum/provider'; import {useToggleState} from '@react-stately/toggle'; function Checkbox(props: SpectrumCheckboxProps, ref: FocusableRef) { + let originalProps = props; props = useProviderProps(props); let { isIndeterminate = false, @@ -47,7 +48,13 @@ function Checkbox(props: SpectrumCheckboxProps, ref: FocusableRef : ; + if (groupState) { + for (let key of ['isSelected', 'defaultSelected', 'isEmphasized']) { + if (originalProps[key] != null) { + console.warn(`${key} is unsupported on individual elements within a . Please apply these props to the group instead.`); + } + } + } + return (