From e30722a89d3d2e4bd0b66ce2a015cf14fc1deacb Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 09:51:31 -0600 Subject: [PATCH 01/45] add useTagGroupState --- packages/@react-aria/tag/package.json | 1 + packages/@react-spectrum/tag/package.json | 1 + packages/@react-stately/tag/README.md | 3 + packages/@react-stately/tag/index.ts | 13 +++++ packages/@react-stately/tag/package.json | 38 +++++++++++++ packages/@react-stately/tag/src/index.ts | 13 +++++ .../tag/src/useTagGroupState.ts | 56 +++++++++++++++++++ 7 files changed, 125 insertions(+) create mode 100644 packages/@react-stately/tag/README.md create mode 100644 packages/@react-stately/tag/index.ts create mode 100644 packages/@react-stately/tag/package.json create mode 100644 packages/@react-stately/tag/src/index.ts create mode 100644 packages/@react-stately/tag/src/useTagGroupState.ts diff --git a/packages/@react-aria/tag/package.json b/packages/@react-aria/tag/package.json index a5f7d8c7e37..40edf8a7d66 100644 --- a/packages/@react-aria/tag/package.json +++ b/packages/@react-aria/tag/package.json @@ -28,6 +28,7 @@ "@react-aria/interactions": "^3.13.0", "@react-aria/utils": "^3.14.1", "@react-stately/grid": "^3.4.1", + "@react-stately/tag": "3.0.0-alpha.1", "@react-types/grid": "^3.1.5", "@react-types/shared": "^3.16.0", "@react-types/tag": "3.0.0-beta.0" diff --git a/packages/@react-spectrum/tag/package.json b/packages/@react-spectrum/tag/package.json index 25c78a1963e..0153b1d827a 100644 --- a/packages/@react-spectrum/tag/package.json +++ b/packages/@react-spectrum/tag/package.json @@ -49,6 +49,7 @@ "@react-stately/collections": "^3.5.0", "@react-stately/grid": "^3.4.1", "@react-stately/list": "^3.6.0", + "@react-stately/tag": "3.0.0-alpha.1", "@react-types/shared": "^3.16.0", "@react-types/tag": "3.0.0-beta.0", "@spectrum-icons/workflow": "^4.0.4" diff --git a/packages/@react-stately/tag/README.md b/packages/@react-stately/tag/README.md new file mode 100644 index 00000000000..49c22e51ddb --- /dev/null +++ b/packages/@react-stately/tag/README.md @@ -0,0 +1,3 @@ +# @react-stately/tag + +This package is part of [react-spectrum](https://github.com/adobe/react-spectrum). See the repo for more details. diff --git a/packages/@react-stately/tag/index.ts b/packages/@react-stately/tag/index.ts new file mode 100644 index 00000000000..4e9931530d8 --- /dev/null +++ b/packages/@react-stately/tag/index.ts @@ -0,0 +1,13 @@ +/* + * Copyright 2022 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. + */ + +export * from './src'; diff --git a/packages/@react-stately/tag/package.json b/packages/@react-stately/tag/package.json new file mode 100644 index 00000000000..01c84495350 --- /dev/null +++ b/packages/@react-stately/tag/package.json @@ -0,0 +1,38 @@ +{ + "name": "@react-stately/tag", + "version": "3.0.0-alpha.1", + "description": "Spectrum UI components in React", + "license": "Apache-2.0", + "private": true, + "main": "dist/main.js", + "module": "dist/module.mjs", + "types": "dist/types.d.ts", + "exports": { + "types": "dist/types.d.ts", + "import": "dist/module.mjs", + "require": "dist/main.js" + }, + "source": "src/index.ts", + "files": [ + "dist", + "src" + ], + "sideEffects": false, + "repository": { + "type": "git", + "url": "https://github.com/adobe/react-spectrum" + }, + "dependencies": { + "@babel/runtime": "^7.6.2", + "@react-stately/grid": "^3.4.1", + "@react-stately/utils": "^3.0.0", + "@react-types/tag": "3.0.0-beta.0", + "react-stately": "^3.19.0" + }, + "peerDependencies": { + "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0" + }, + "publishConfig": { + "access": "public" + } +} diff --git a/packages/@react-stately/tag/src/index.ts b/packages/@react-stately/tag/src/index.ts new file mode 100644 index 00000000000..a01676123b2 --- /dev/null +++ b/packages/@react-stately/tag/src/index.ts @@ -0,0 +1,13 @@ +/* + * Copyright 2022 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. + */ + +export * from './useTagGroupState'; diff --git a/packages/@react-stately/tag/src/useTagGroupState.ts b/packages/@react-stately/tag/src/useTagGroupState.ts new file mode 100644 index 00000000000..03d66025164 --- /dev/null +++ b/packages/@react-stately/tag/src/useTagGroupState.ts @@ -0,0 +1,56 @@ +/* + * Copyright 2022 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 {GridCollection, GridState, useGridState} from '@react-stately/grid'; +import {TagGroupProps} from '@react-types/tag'; +import {useEffect, useMemo, useRef} from 'react'; +import {useListState} from 'react-stately'; + +export interface TagGroupState> extends GridState { +} + +/** + * Provides state management for a TagGroup component. + */ +export function useTagGroupState>(props: TagGroupProps): GridState { + let listState = useListState(props); + + let gridCollection = useMemo(() => new GridCollection({ + columnCount: 1, // unused, but required for grid collections + items: [...listState.collection].map(item => ({ + type: 'item', + childNodes: [{ + ...item, + index: 0, + type: 'cell' + }] + })) + // eslint-disable-next-line react-hooks/exhaustive-deps + }), [listState.collection, props.allowsRemoving]); + + let state = useGridState({ + ...props, + collection: gridCollection, + focusMode: 'cell' + }); + + let keyToRestoreOnDelete = useRef(null); + + useEffect(() => { + if (!listState.collection.getItem(state.selectionManager.focusedKey)) { + state.selectionManager.setFocusedKey(keyToRestoreOnDelete.current); + } + keyToRestoreOnDelete.current = listState.collection.getKeyBefore(state.selectionManager.focusedKey) || listState.collection.getKeyAfter(state.selectionManager.focusedKey); + }, [listState.collection, state.collection, state.selectionManager]); + + return state; +} From d7614e3a73bc7418efc66a601c457559a4278c6b Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 09:52:36 -0600 Subject: [PATCH 02/45] update TagGroup to use useTagGroupState --- packages/@react-spectrum/tag/src/TagGroup.tsx | 57 +++---------------- 1 file changed, 8 insertions(+), 49 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 29b6df9ee21..77ec6de1760 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -12,18 +12,14 @@ import {classNames, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {DOMRef} from '@react-types/shared'; -import {GridCollection, useGridState} from '@react-stately/grid'; import {mergeProps} from '@react-aria/utils'; -import React, {ReactElement, useMemo} from 'react'; +import React, {ReactElement} from 'react'; import {SpectrumTagGroupProps} from '@react-types/tag'; import styles from '@adobe/spectrum-css-temp/components/tags/vars.css'; import {Tag} from './Tag'; -import {TagKeyboardDelegate, useTagGroup} from '@react-aria/tag'; -import {useGrid} from '@react-aria/grid'; -import {useListState} from '@react-stately/list'; -import {useLocale} from '@react-aria/i18n'; import {useProviderProps} from '@react-spectrum/provider'; - +import {useTagGroup} from '@react-aria/tag'; +import {useTagGroupState} from '@react-stately/tag'; function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef) { props = useProviderProps(props); @@ -34,48 +30,11 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef } = props; let domRef = useDOMRef(ref); let {styleProps} = useStyleProps(otherProps); - let {direction} = useLocale(); - let listState = useListState(props); - let gridCollection = useMemo(() => new GridCollection({ - columnCount: 1, // unused, but required for grid collections - items: [...listState.collection].map(item => { - let childNodes = [{ - ...item, - index: 0, - type: 'cell' - }]; - - return { - type: 'item', - childNodes - }; - }) - // eslint-disable-next-line react-hooks/exhaustive-deps - }), [listState.collection, allowsRemoving]); - let state = useGridState({ - ...props, - collection: gridCollection, - focusMode: 'cell' - }); - let keyboardDelegate = new TagKeyboardDelegate({ - collection: state.collection, - disabledKeys: new Set(), - ref: domRef, - direction, - focusMode: 'cell' - }); - let {gridProps} = useGrid({ - ...props, - keyboardDelegate - }, state, domRef); - const {tagGroupProps} = useTagGroup(props); - - // Don't want the grid to be focusable or accessible via keyboard - // eslint-disable-next-line @typescript-eslint/no-unused-vars - let {tabIndex, role, ...otherGridProps} = gridProps; + let state = useTagGroupState(props); + let {tagGroupProps} = useTagGroup(props, state, domRef); return (
(props: SpectrumTagGroupProps, ref: DOMRef } role={state.collection.size ? 'grid' : null} ref={domRef}> - {[...gridCollection].map(item => ( + {[...state.collection].map(item => ( (props: SpectrumTagGroupProps, ref: DOMRef onRemove={onRemove}> {item.childNodes[0].rendered} - ))} + ))}
); } From 78e360edfe1559ffd8e58b0793cb9883b6098831 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 09:54:20 -0600 Subject: [PATCH 03/45] update useTagGroup --- packages/@react-aria/tag/src/useTagGroup.ts | 28 ++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index c6ca99cefe0..5053ce00cac 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -12,8 +12,13 @@ import {DOMAttributes, DOMProps} from '@react-types/shared'; import {filterDOMProps, mergeProps} from '@react-aria/utils'; -import {ReactNode, useState} from 'react'; +import {GridCollection} from '@react-types/grid'; +import {ReactNode, RefObject, useState} from 'react'; +import {TagGroupState} from '@react-stately/tag'; +import {TagKeyboardDelegate} from './TagKeyboardDelegate'; import {useFocusWithin} from '@react-aria/interactions'; +import {useGrid} from '@react-aria/grid'; +import {useLocale} from '@react-aria/i18n'; export interface AriaTagGroupProps extends DOMProps { children: ReactNode, @@ -25,14 +30,31 @@ export interface TagGroupAria { tagGroupProps: DOMAttributes } -export function useTagGroup(props: AriaTagGroupProps): TagGroupAria { +export function useTagGroup>(props: AriaTagGroupProps, state: TagGroupState, ref: RefObject): TagGroupAria { + let {direction} = useLocale(); + let keyboardDelegate = new TagKeyboardDelegate({ + collection: state.collection, + disabledKeys: new Set(), + ref, + direction, + focusMode: 'cell' + }); + let {gridProps} = useGrid({ + ...props, + keyboardDelegate + }, state, ref); + + // Don't want the grid to be focusable or accessible via keyboard + delete gridProps.role; + delete gridProps.tabIndex; + let [isFocusWithin, setFocusWithin] = useState(false); let {focusWithinProps} = useFocusWithin({ onFocusWithinChange: setFocusWithin }); let domProps = filterDOMProps(props); return { - tagGroupProps: mergeProps(domProps, { + tagGroupProps: mergeProps(gridProps, domProps, { 'aria-atomic': false, 'aria-relevant': 'additions', 'aria-live': isFocusWithin ? 'polite' : 'off', From 09f297a8d7230c632022b2416e542fc8ea03d2b9 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 09:54:27 -0600 Subject: [PATCH 04/45] cleanup useTag --- packages/@react-aria/tag/src/useTag.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/@react-aria/tag/src/useTag.ts b/packages/@react-aria/tag/src/useTag.ts index 3e431c90448..faf51f6f518 100644 --- a/packages/@react-aria/tag/src/useTag.ts +++ b/packages/@react-aria/tag/src/useTag.ts @@ -29,18 +29,18 @@ export interface TagAria { } export function useTag(props: TagProps, state: GridState): TagAria { - let {isFocused} = props; - const { + let { + isFocused, allowsRemoving, onRemove, item, tagRef, tagRowRef } = props; - const stringFormatter = useLocalizedStringFormatter(intlMessages); - const removeString = stringFormatter.format('remove'); - const labelId = useId(); - const buttonId = useId(); + let stringFormatter = useLocalizedStringFormatter(intlMessages); + let removeString = stringFormatter.format('remove'); + let labelId = useId(); + let buttonId = useId(); let {rowProps} = useGridRow({ node: item @@ -60,7 +60,7 @@ export function useTag(props: TagProps, state: GridState): TagAri e.preventDefault(); } } - const pressProps = { + let pressProps = { onPress: () => onRemove?.(item.childNodes[0].key) }; From 22f9f5fcc74584dfaeb4753ad2764f3353e45e06 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 09:54:32 -0600 Subject: [PATCH 05/45] cleanup stories --- .../tag/stories/TagGroup.stories.tsx | 54 +++++++++---------- 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx index 6974b152867..a0d0ea1adc9 100644 --- a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx +++ b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx @@ -12,24 +12,23 @@ import {action} from '@storybook/addon-actions'; import Audio from '@spectrum-icons/workflow/Audio'; -import {Icon} from '@react-spectrum/icon'; import {Item, TagGroup} from '../src'; import React, {useState} from 'react'; import {storiesOf} from '@storybook/react'; import {Text} from '@react-spectrum/text'; +let items = [{key: '1', label: 'Cool Tag 1'}, {key: '2', label: 'Cool Tag 2'}]; + storiesOf('TagGroup', module) .add( 'default', () => render({}) ) .add('icons', () => ( - + {item => ( - - + )} @@ -38,30 +37,29 @@ storiesOf('TagGroup', module) .add( 'onRemove', () => { - const [items, setItems] = useState([ - {key: 1, label: 'Cool Tag 1'}, - {key: 2, label: 'Another cool tag'}, - {key: 3, label: 'This tag'}, - {key: 4, label: 'What tag?'}, - {key: 5, label: 'This tag is cool too'}, - {key: 6, label: 'Shy tag'} + let [items, setItems] = useState([ + {id: 1, label: 'Cool Tag 1'}, + {id: 2, label: 'Another cool tag'}, + {id: 3, label: 'This tag'}, + {id: 4, label: 'What tag?'}, + {id: 5, label: 'This tag is cool too'}, + {id: 6, label: 'Shy tag'} ]); - const onRemove = (key) => { - const newItems = [...items].filter((item) => key !== item.key.toString()); - setItems(newItems); - action('onRemove')(key); + + let removeItem = (key) => { + setItems(prevItems => prevItems.filter((item) => key !== item.id)); }; - return ( onRemove(key)}> - {item => ( - {item.label} - )} - ); + return ( + + {item => {item.label}} + + ); } ) .add('wrapping', () => (
- + Cool Tag 1 Another cool tag This tag @@ -74,7 +72,7 @@ storiesOf('TagGroup', module) ) .add('label truncation', () => (
- + Cool Tag 1 with a really long label Another long cool tag label This tag @@ -83,19 +81,17 @@ storiesOf('TagGroup', module) ) ) .add( - 'using items prop', + 'dynamic items', () => ( - - {item => - {item.label} - } + + {item => {item.label}} ) ); function render(props: any = {}) { return ( - + Cool Tag 1 Cool Tag 2 Cool Tag 3 From 1bec84c1f6da9382b5437c0cab3ae8f1f3577840 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 10:02:17 -0600 Subject: [PATCH 06/45] add comment --- packages/@react-stately/tag/src/useTagGroupState.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/@react-stately/tag/src/useTagGroupState.ts b/packages/@react-stately/tag/src/useTagGroupState.ts index 03d66025164..6d77ff3f39b 100644 --- a/packages/@react-stately/tag/src/useTagGroupState.ts +++ b/packages/@react-stately/tag/src/useTagGroupState.ts @@ -43,13 +43,14 @@ export function useTagGroupState>( focusMode: 'cell' }); - let keyToRestoreOnDelete = useRef(null); + let keyToRestoreOnRemove = useRef(null); useEffect(() => { + // If the focused key is removed, restore focus to the tag before, or tag after if no tag before. if (!listState.collection.getItem(state.selectionManager.focusedKey)) { - state.selectionManager.setFocusedKey(keyToRestoreOnDelete.current); + state.selectionManager.setFocusedKey(keyToRestoreOnRemove.current); } - keyToRestoreOnDelete.current = listState.collection.getKeyBefore(state.selectionManager.focusedKey) || listState.collection.getKeyAfter(state.selectionManager.focusedKey); + keyToRestoreOnRemove.current = listState.collection.getKeyBefore(state.selectionManager.focusedKey) || listState.collection.getKeyAfter(state.selectionManager.focusedKey); }, [listState.collection, state.collection, state.selectionManager]); return state; From 5e7df52df569536d666d28d6573301f130d9ce20 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 10:20:22 -0600 Subject: [PATCH 07/45] remove private from package.json --- packages/@react-stately/tag/package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@react-stately/tag/package.json b/packages/@react-stately/tag/package.json index 01c84495350..c4f198a3ea6 100644 --- a/packages/@react-stately/tag/package.json +++ b/packages/@react-stately/tag/package.json @@ -3,7 +3,6 @@ "version": "3.0.0-alpha.1", "description": "Spectrum UI components in React", "license": "Apache-2.0", - "private": true, "main": "dist/main.js", "module": "dist/module.mjs", "types": "dist/types.d.ts", From 022828f10b885ee1146f4f98c7afe9dd41d61f5e Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 10:37:17 -0600 Subject: [PATCH 08/45] lint --- packages/@react-spectrum/tag/stories/TagGroup.stories.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx index a0d0ea1adc9..84df182a7b7 100644 --- a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx +++ b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx @@ -10,7 +10,6 @@ * governing permissions and limitations under the License. */ -import {action} from '@storybook/addon-actions'; import Audio from '@spectrum-icons/workflow/Audio'; import {Item, TagGroup} from '../src'; import React, {useState} from 'react'; From 50a723d96ac638cf8b5e2be4aa1bd4e2880bbb39 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 10:39:44 -0600 Subject: [PATCH 09/45] fix types --- packages/@react-aria/tag/src/useTag.ts | 4 ++-- packages/@react-aria/tag/src/useTagGroup.ts | 2 +- packages/@react-spectrum/tag/src/Tag.tsx | 3 ++- packages/@react-stately/tag/src/useTagGroupState.ts | 7 +++---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/@react-aria/tag/src/useTag.ts b/packages/@react-aria/tag/src/useTag.ts index faf51f6f518..61b7b788b09 100644 --- a/packages/@react-aria/tag/src/useTag.ts +++ b/packages/@react-aria/tag/src/useTag.ts @@ -13,7 +13,7 @@ import {ButtonHTMLAttributes, KeyboardEvent} from 'react'; import {DOMAttributes} from '@react-types/shared'; import {filterDOMProps, mergeProps, useId} from '@react-aria/utils'; -import {GridState} from '@react-stately/grid'; +import {GridCollection, GridState} from '@react-stately/grid'; // @ts-ignore import intlMessages from '../intl/*.json'; import {TagProps} from '@react-types/tag'; @@ -28,7 +28,7 @@ export interface TagAria { clearButtonProps: ButtonHTMLAttributes } -export function useTag(props: TagProps, state: GridState): TagAria { +export function useTag>(props: TagProps, state: GridState): TagAria { let { isFocused, allowsRemoving, diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index 5053ce00cac..6ebf1ac1751 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -30,7 +30,7 @@ export interface TagGroupAria { tagGroupProps: DOMAttributes } -export function useTagGroup>(props: AriaTagGroupProps, state: TagGroupState, ref: RefObject): TagGroupAria { +export function useTagGroup(props: AriaTagGroupProps, state: TagGroupState, ref: RefObject): TagGroupAria { let {direction} = useLocale(); let keyboardDelegate = new TagKeyboardDelegate({ collection: state.collection, diff --git a/packages/@react-spectrum/tag/src/Tag.tsx b/packages/@react-spectrum/tag/src/Tag.tsx index 9f5f880cc2b..d0e475a5bd3 100644 --- a/packages/@react-spectrum/tag/src/Tag.tsx +++ b/packages/@react-spectrum/tag/src/Tag.tsx @@ -12,6 +12,7 @@ import {classNames, SlotProvider, useSlotProps, useStyleProps} from '@react-spectrum/utils'; import {ClearButton} from '@react-spectrum/button'; +import {GridCollection} from '@react-stately/grid'; import {mergeProps} from '@react-aria/utils'; import React, {useRef} from 'react'; import {SpectrumTagProps} from '@react-types/tag'; @@ -21,7 +22,7 @@ import {useFocusRing} from '@react-aria/focus'; import {useHover} from '@react-aria/interactions'; import {useTag} from '@react-aria/tag'; -export function Tag(props: SpectrumTagProps) { +export function Tag>(props: SpectrumTagProps) { const { children, allowsRemoving, diff --git a/packages/@react-stately/tag/src/useTagGroupState.ts b/packages/@react-stately/tag/src/useTagGroupState.ts index 6d77ff3f39b..ed1a5e6ceab 100644 --- a/packages/@react-stately/tag/src/useTagGroupState.ts +++ b/packages/@react-stately/tag/src/useTagGroupState.ts @@ -15,13 +15,12 @@ import {TagGroupProps} from '@react-types/tag'; import {useEffect, useMemo, useRef} from 'react'; import {useListState} from 'react-stately'; -export interface TagGroupState> extends GridState { -} +export interface TagGroupState> extends GridState {} /** * Provides state management for a TagGroup component. */ -export function useTagGroupState>(props: TagGroupProps): GridState { +export function useTagGroupState>(props: TagGroupProps): TagGroupState { let listState = useListState(props); let gridCollection = useMemo(() => new GridCollection({ @@ -53,5 +52,5 @@ export function useTagGroupState>( keyToRestoreOnRemove.current = listState.collection.getKeyBefore(state.selectionManager.focusedKey) || listState.collection.getKeyAfter(state.selectionManager.focusedKey); }, [listState.collection, state.collection, state.selectionManager]); - return state; + return state as TagGroupState; } From 6cc253a9acfb97c8dda0a030049566de38d54d44 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 10:47:06 -0600 Subject: [PATCH 10/45] lint --- packages/@react-aria/tag/src/useTagGroup.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index 6ebf1ac1751..4e108c5c4db 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -12,7 +12,6 @@ import {DOMAttributes, DOMProps} from '@react-types/shared'; import {filterDOMProps, mergeProps} from '@react-aria/utils'; -import {GridCollection} from '@react-types/grid'; import {ReactNode, RefObject, useState} from 'react'; import {TagGroupState} from '@react-stately/tag'; import {TagKeyboardDelegate} from './TagKeyboardDelegate'; From 15a8b32168a8a496b517a593f34adc28dae27d78 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 10:55:15 -0600 Subject: [PATCH 11/45] fix types --- packages/@react-spectrum/tag/src/Tag.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tag/src/Tag.tsx b/packages/@react-spectrum/tag/src/Tag.tsx index d0e475a5bd3..90e43fabe6e 100644 --- a/packages/@react-spectrum/tag/src/Tag.tsx +++ b/packages/@react-spectrum/tag/src/Tag.tsx @@ -22,7 +22,7 @@ import {useFocusRing} from '@react-aria/focus'; import {useHover} from '@react-aria/interactions'; import {useTag} from '@react-aria/tag'; -export function Tag>(props: SpectrumTagProps) { +export function Tag>(props: SpectrumTagProps) { const { children, allowsRemoving, From 31ca965b473ff7fa22307c0a1c9dc99f958b46c8 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 11:54:20 -0600 Subject: [PATCH 12/45] update removal string --- packages/@react-aria/tag/intl/en-US.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-aria/tag/intl/en-US.json b/packages/@react-aria/tag/intl/en-US.json index 67ff0e1312a..26dd537ca48 100644 --- a/packages/@react-aria/tag/intl/en-US.json +++ b/packages/@react-aria/tag/intl/en-US.json @@ -1,3 +1,3 @@ { - "remove": "Remove" + "remove": "Press space or delete to remove tag." } From a8202bf89ef9ff19e4f919db1a75be59e8b10b50 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 12:01:33 -0600 Subject: [PATCH 13/45] switch to focus row instead of cell --- packages/@react-aria/tag/src/useTagGroup.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index 4e108c5c4db..ecc70f0c7f0 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -36,7 +36,7 @@ export function useTagGroup(props: AriaTagGroupProps, state: TagGroupState Date: Tue, 29 Nov 2022 14:09:09 -0600 Subject: [PATCH 14/45] fix types --- packages/@react-aria/tag/src/useTagGroup.ts | 2 +- packages/@react-stately/tag/src/useTagGroupState.ts | 8 ++++---- packages/@react-types/tag/package.json | 1 + packages/@react-types/tag/src/index.d.ts | 11 ++++++++--- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index ecc70f0c7f0..89a45b0ba48 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -29,7 +29,7 @@ export interface TagGroupAria { tagGroupProps: DOMAttributes } -export function useTagGroup(props: AriaTagGroupProps, state: TagGroupState, ref: RefObject): TagGroupAria { +export function useTagGroup(props: AriaTagGroupProps, state: TagGroupState, ref: RefObject): TagGroupAria { let {direction} = useLocale(); let keyboardDelegate = new TagKeyboardDelegate({ collection: state.collection, diff --git a/packages/@react-stately/tag/src/useTagGroupState.ts b/packages/@react-stately/tag/src/useTagGroupState.ts index ed1a5e6ceab..b7bfbca17f8 100644 --- a/packages/@react-stately/tag/src/useTagGroupState.ts +++ b/packages/@react-stately/tag/src/useTagGroupState.ts @@ -11,16 +11,16 @@ */ import {GridCollection, GridState, useGridState} from '@react-stately/grid'; -import {TagGroupProps} from '@react-types/tag'; +import {ITagGroupCollection, TagGroupProps} from '@react-types/tag'; import {useEffect, useMemo, useRef} from 'react'; import {useListState} from 'react-stately'; -export interface TagGroupState> extends GridState {} +export interface TagGroupState extends GridState>{} /** * Provides state management for a TagGroup component. */ -export function useTagGroupState>(props: TagGroupProps): TagGroupState { +export function useTagGroupState(props: TagGroupProps): TagGroupState { let listState = useListState(props); let gridCollection = useMemo(() => new GridCollection({ @@ -52,5 +52,5 @@ export function useTagGroupState>( keyToRestoreOnRemove.current = listState.collection.getKeyBefore(state.selectionManager.focusedKey) || listState.collection.getKeyAfter(state.selectionManager.focusedKey); }, [listState.collection, state.collection, state.selectionManager]); - return state as TagGroupState; + return state; } diff --git a/packages/@react-types/tag/package.json b/packages/@react-types/tag/package.json index dc9875f720f..2ecc990495b 100644 --- a/packages/@react-types/tag/package.json +++ b/packages/@react-types/tag/package.json @@ -10,6 +10,7 @@ }, "dependencies": { "@react-stately/grid": "^3.4.1", + "@react-types/grid": "^3.1.5", "@react-types/shared": "^3.16.0" }, "peerDependencies": { diff --git a/packages/@react-types/tag/src/index.d.ts b/packages/@react-types/tag/src/index.d.ts index 8c6aef4ac68..e908d13dd32 100644 --- a/packages/@react-types/tag/src/index.d.ts +++ b/packages/@react-types/tag/src/index.d.ts @@ -11,6 +11,7 @@ */ import {AriaLabelingProps, CollectionBase, DOMProps, ItemProps, Node, StyleProps} from '@react-types/shared'; +import {GridCollection} from '@react-types/grid'; import {GridState} from '@react-stately/grid'; import {Key, RefObject} from 'react'; @@ -21,7 +22,9 @@ export interface TagGroupProps extends Omit, 'disabledKeys' onRemove?: (key: Key) => void } -export interface SpectrumTagGroupProps extends TagGroupProps, DOMProps, StyleProps, AriaLabelingProps {} +export interface AriaTagGroupProps extends TagGroupProps, DOMProps, AriaLabelingProps {} + +export interface SpectrumTagGroupProps extends AriaTagGroupProps, StyleProps {} export interface TagProps extends ItemProps { isFocused: boolean, @@ -32,6 +35,8 @@ export interface TagProps extends ItemProps { tagRowRef: RefObject } -interface SpectrumTagProps extends TagProps { - state: GridState +interface SpectrumTagProps> extends TagProps { + state: GridState } + +export interface ITagGroupCollection extends GridCollection{} From 3aad4db4bf1ba54fa1fb0bbe40ff656556a91758 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 14:36:15 -0600 Subject: [PATCH 15/45] add test for focus after deleting --- .../@react-spectrum/tag/test/TagGroup.test.js | 81 ++++++++++--------- 1 file changed, 45 insertions(+), 36 deletions(-) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index 37335c931ca..657281381d4 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -18,6 +18,7 @@ import React from 'react'; import {TagGroup} from '../src'; import {theme} from '@react-spectrum/theme-default'; import userEvent from '@testing-library/user-event'; +import {chain} from '@react-aria/utils'; function pressKeyOnButton(key) { return (button) => { @@ -299,41 +300,49 @@ describe('TagGroup', function () { expect(onRemoveSpy).toHaveBeenCalledWith('1'); }); + it.each` + Name | props + ${'on `Delete` keypress'} | ${{keyPress: 'Delete'}} + ${'on `Backspace` keypress'} | ${{keyPress: 'Backspace'}} + ${'on `space` keypress'} | ${{keyPress: ' '}} + `('Can move focus after removing tag $Name', function ({Name, props}) { + + function TagGroupWithDelete(props) { + let [items, setItems] = React.useState([ + {id: 1, label: 'Cool Tag 1'}, + {id: 2, label: 'Another cool tag'}, + {id: 3, label: 'This tag'}, + {id: 4, label: 'What tag?'}, + {id: 5, label: 'This tag is cool too'}, + {id: 6, label: 'Shy tag'} + ]); + + let removeItem = (key) => { + setItems(prevItems => prevItems.filter((item) => key !== item.id)); + }; + + return ( + + + {item => {item.label}} + + + ); + } + + let {getAllByRole} = render( + + ); - // Commented out until spectrum can provide use case for these scenarios - // it.each` - // Name | Component | TagComponent | props - // ${'TagGroup'} | ${TagGroup} | ${Item} | ${{isReadOnly: true, isRemovable: true, onRemove: onRemoveSpy}} - // `('$Name is read only', ({Component, TagComponent, props}) => { - // let {getByText} = render( - // - // Tag 1 - // - // ); - // let tag = getByText('Tag 1'); - // fireEvent.keyDown(tag, {key: 'Delete', keyCode: 46}); - // expect(onRemoveSpy).not.toHaveBeenCalledWith('Tag 1', expect.anything()); - // }); - // - // it.each` - // Name | Component | TagComponent | props - // ${'Tag'} | ${TagGroup} | ${Item} | ${{validationState: 'invalid'}} - // `('$Name can be invalid', function ({Component, TagComponent, props}) { - // let {getByRole, debug} = render( - // - // Tag 1 - // - // ); - // - // debug(); - // - // let tag = getByRole('row'); - // expect(tag).toHaveAttribute('aria-invalid', 'true'); - // }); + let tags = getAllByRole('gridcell'); + userEvent.tab(); + expect(document.activeElement).toBe(tags[0]); + fireEvent.keyDown(document.activeElement, {key: props.keyPress}); + expect(onRemoveSpy).toHaveBeenCalledTimes(1); + expect(onRemoveSpy).toHaveBeenCalledWith(1); + tags = getAllByRole('gridcell'); + expect(document.activeElement).toBe(tags[0]); + pressArrowRight(tags[0]); + expect(document.activeElement).toBe(tags[1]); + }); }); - -// need to add test for focus after onremove From d5449f7b312e9a598f8437182bf271600abf1593 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 14:36:24 -0600 Subject: [PATCH 16/45] set focusMode: 'cell' --- packages/@react-stately/tag/src/useTagGroupState.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-stately/tag/src/useTagGroupState.ts b/packages/@react-stately/tag/src/useTagGroupState.ts index b7bfbca17f8..c3c82bcf8df 100644 --- a/packages/@react-stately/tag/src/useTagGroupState.ts +++ b/packages/@react-stately/tag/src/useTagGroupState.ts @@ -39,7 +39,7 @@ export function useTagGroupState(props: TagGroupProps): Tag let state = useGridState({ ...props, collection: gridCollection, - focusMode: 'cell' + focusMode: 'row' }); let keyToRestoreOnRemove = useRef(null); From f3f64d7b04798155d24c000f106739b32249f740 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 14:36:31 -0600 Subject: [PATCH 17/45] cleanup useTag --- packages/@react-aria/tag/src/useTag.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/@react-aria/tag/src/useTag.ts b/packages/@react-aria/tag/src/useTag.ts index 61b7b788b09..1af948a652f 100644 --- a/packages/@react-aria/tag/src/useTag.ts +++ b/packages/@react-aria/tag/src/useTag.ts @@ -45,9 +45,9 @@ export function useTag>(props: TagProps, state: G let {rowProps} = useGridRow({ node: item }, state, tagRowRef); + // Don't want the row to be focusable or accessible via keyboard - // eslint-disable-next-line @typescript-eslint/no-unused-vars - let {tabIndex, ...otherRowProps} = rowProps; + delete rowProps.tabIndex; let {gridCellProps} = useGridCell({ node: [...item.childNodes][0], @@ -75,7 +75,7 @@ export function useTag>(props: TagProps, state: G labelProps: { id: labelId }, - tagRowProps: otherRowProps, + tagRowProps: rowProps, tagProps: mergeProps(domProps, gridCellProps, { 'aria-errormessage': props['aria-errormessage'], 'aria-label': props['aria-label'], From f746d653ada763cbef1a531ca03914c28244681e Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 14:45:06 -0600 Subject: [PATCH 18/45] lint --- packages/@react-spectrum/tag/test/TagGroup.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index 657281381d4..e10d40aaae3 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -12,13 +12,13 @@ import {act, fireEvent, render} from '@react-spectrum/test-utils'; import {Button} from '@react-spectrum/button'; +import {chain} from '@react-aria/utils'; import {Item} from '@react-stately/collections'; import {Provider} from '@react-spectrum/provider'; import React from 'react'; import {TagGroup} from '../src'; import {theme} from '@react-spectrum/theme-default'; import userEvent from '@testing-library/user-event'; -import {chain} from '@react-aria/utils'; function pressKeyOnButton(key) { return (button) => { From 55d704e9798b24a19e884c703c441bead45f22e6 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 29 Nov 2022 17:40:20 -0600 Subject: [PATCH 19/45] update translation string punctuation --- packages/@react-aria/tag/intl/en-US.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-aria/tag/intl/en-US.json b/packages/@react-aria/tag/intl/en-US.json index 26dd537ca48..f7a6cb22c7c 100644 --- a/packages/@react-aria/tag/intl/en-US.json +++ b/packages/@react-aria/tag/intl/en-US.json @@ -1,3 +1,3 @@ { - "remove": "Press space or delete to remove tag." + "remove": "Press Space or Delete to remove tag." } From 72ebeef9cd97682eff8231cc3b5df43aedff8f55 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 30 Nov 2022 09:50:19 -0600 Subject: [PATCH 20/45] fix type --- packages/@react-aria/tag/src/useTag.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/@react-aria/tag/src/useTag.ts b/packages/@react-aria/tag/src/useTag.ts index 1af948a652f..b55d3d733cb 100644 --- a/packages/@react-aria/tag/src/useTag.ts +++ b/packages/@react-aria/tag/src/useTag.ts @@ -13,9 +13,10 @@ import {ButtonHTMLAttributes, KeyboardEvent} from 'react'; import {DOMAttributes} from '@react-types/shared'; import {filterDOMProps, mergeProps, useId} from '@react-aria/utils'; -import {GridCollection, GridState} from '@react-stately/grid'; +import {GridCollection} from '@react-stately/grid'; // @ts-ignore import intlMessages from '../intl/*.json'; +import {TagGroupState} from '@react-stately/tag'; import {TagProps} from '@react-types/tag'; import {useGridCell, useGridRow} from '@react-aria/grid'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; @@ -28,7 +29,7 @@ export interface TagAria { clearButtonProps: ButtonHTMLAttributes } -export function useTag>(props: TagProps, state: GridState): TagAria { +export function useTag>(props: TagProps, state: TagGroupState): TagAria { let { isFocused, allowsRemoving, From cc44d1844c0494f634e7305a7fa729b2b1109ca6 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 30 Nov 2022 12:27:23 -0600 Subject: [PATCH 21/45] refactor to use useGridList --- packages/@react-aria/tag/package.json | 2 + .../tag/src/TagKeyboardDelegate.ts | 65 +++++-------------- packages/@react-aria/tag/src/useTag.ts | 29 ++++----- packages/@react-aria/tag/src/useTagGroup.ts | 15 +---- packages/@react-spectrum/tag/src/Tag.tsx | 23 +++---- packages/@react-spectrum/tag/src/TagGroup.tsx | 4 +- packages/@react-stately/tag/package.json | 1 + .../tag/src/useTagGroupState.ts | 36 +++------- packages/@react-types/tag/src/index.d.ts | 3 - 9 files changed, 57 insertions(+), 121 deletions(-) diff --git a/packages/@react-aria/tag/package.json b/packages/@react-aria/tag/package.json index 40edf8a7d66..6bb713da8e3 100644 --- a/packages/@react-aria/tag/package.json +++ b/packages/@react-aria/tag/package.json @@ -24,8 +24,10 @@ "dependencies": { "@babel/runtime": "^7.6.2", "@react-aria/grid": "^3.5.1", + "@react-aria/gridlist": "^3.1.1", "@react-aria/i18n": "^3.6.2", "@react-aria/interactions": "^3.13.0", + "@react-aria/selection": "^3.12.0", "@react-aria/utils": "^3.14.1", "@react-stately/grid": "^3.4.1", "@react-stately/tag": "3.0.0-alpha.1", diff --git a/packages/@react-aria/tag/src/TagKeyboardDelegate.ts b/packages/@react-aria/tag/src/TagKeyboardDelegate.ts index 3a3ed95f5b8..3d974a3baf5 100644 --- a/packages/@react-aria/tag/src/TagKeyboardDelegate.ts +++ b/packages/@react-aria/tag/src/TagKeyboardDelegate.ts @@ -10,25 +10,25 @@ * governing permissions and limitations under the License. */ -import {GridCollection} from '@react-types/grid'; -import {GridKeyboardDelegate} from '@react-aria/grid'; +import {Collection, Direction, KeyboardDelegate} from '@react-types/shared'; import {Key} from 'react'; +export class TagKeyboardDelegate implements KeyboardDelegate { + private collection: Collection; + private direction: Direction; -export class TagKeyboardDelegate extends GridKeyboardDelegate> { - getFirstKey() { - let key = this.collection.getFirstKey(); - let item = this.collection.getItem(key); + constructor(collection: Collection, direction: Direction) { + this.collection = collection; + this.direction = direction; + } - return [...item.childNodes][0].key; + getFirstKey() { + return this.collection.getFirstKey(); } getLastKey() { - let key = this.collection.getLastKey(); - let item = this.collection.getItem(key); - - return [...item.childNodes][0].key; + return this.collection.getLastKey(); } - + getKeyRightOf(key: Key) { return this.direction === 'rtl' ? this.getKeyAbove(key) : this.getKeyBelow(key); } @@ -43,27 +43,12 @@ export class TagKeyboardDelegate extends GridKeyboardDelegate extends GridKeyboardDelegate>(props: TagProps, state: T allowsRemoving, onRemove, item, - tagRef, tagRowRef } = props; let stringFormatter = useLocalizedStringFormatter(intlMessages); @@ -43,29 +42,25 @@ export function useTag>(props: TagProps, state: T let labelId = useId(); let buttonId = useId(); - let {rowProps} = useGridRow({ + let {rowProps, gridCellProps} = useGridListItem({ node: item }, state, tagRowRef); - // Don't want the row to be focusable or accessible via keyboard - delete rowProps.tabIndex; + // We want TagKeyboardDelegate to handle keyboard events instead. + delete rowProps.onKeyDownCapture; - let {gridCellProps} = useGridCell({ - node: [...item.childNodes][0], - focusMode: 'cell' - }, state, tagRef); function onKeyDown(e: KeyboardEvent) { if (e.key === 'Delete' || e.key === 'Backspace' || e.key === ' ') { - onRemove(item.childNodes[0].key); + onRemove(item.key); e.preventDefault(); } } let pressProps = { - onPress: () => onRemove?.(item.childNodes[0].key) + onPress: () => onRemove?.(item.key) }; - isFocused = isFocused || state.selectionManager.focusedKey === item.childNodes[0].key; + isFocused = isFocused || state.selectionManager.focusedKey === item.key; let domProps = filterDOMProps(props); return { clearButtonProps: mergeProps(pressProps, { @@ -76,12 +71,14 @@ export function useTag>(props: TagProps, state: T labelProps: { id: labelId }, - tagRowProps: rowProps, + tagRowProps: { + ...rowProps, + tabIndex: (isFocused || state.selectionManager.focusedKey == null) ? 0 : -1, + onKeyDown: allowsRemoving ? onKeyDown : null + }, tagProps: mergeProps(domProps, gridCellProps, { 'aria-errormessage': props['aria-errormessage'], - 'aria-label': props['aria-label'], - onKeyDown: allowsRemoving ? onKeyDown : null, - tabIndex: (isFocused || state.selectionManager.focusedKey == null) ? 0 : -1 + 'aria-label': props['aria-label'] }) }; } diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index 89a45b0ba48..7cd34bec594 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -16,7 +16,7 @@ import {ReactNode, RefObject, useState} from 'react'; import {TagGroupState} from '@react-stately/tag'; import {TagKeyboardDelegate} from './TagKeyboardDelegate'; import {useFocusWithin} from '@react-aria/interactions'; -import {useGrid} from '@react-aria/grid'; +import {useGridList} from '@react-aria/gridlist'; import {useLocale} from '@react-aria/i18n'; export interface AriaTagGroupProps extends DOMProps { @@ -31,17 +31,8 @@ export interface TagGroupAria { export function useTagGroup(props: AriaTagGroupProps, state: TagGroupState, ref: RefObject): TagGroupAria { let {direction} = useLocale(); - let keyboardDelegate = new TagKeyboardDelegate({ - collection: state.collection, - disabledKeys: new Set(), - ref, - direction, - focusMode: 'row' - }); - let {gridProps} = useGrid({ - ...props, - keyboardDelegate - }, state, ref); + let keyboardDelegate = new TagKeyboardDelegate(state.collection, direction); + let {gridProps} = useGridList({...props, keyboardDelegate}, state, ref); // Don't want the grid to be focusable or accessible via keyboard delete gridProps.role; diff --git a/packages/@react-spectrum/tag/src/Tag.tsx b/packages/@react-spectrum/tag/src/Tag.tsx index 90e43fabe6e..4dbcaadd7b9 100644 --- a/packages/@react-spectrum/tag/src/Tag.tsx +++ b/packages/@react-spectrum/tag/src/Tag.tsx @@ -36,7 +36,6 @@ export function Tag>(props: SpectrumTagProps) { let {styleProps} = useStyleProps(otherProps); let {hoverProps, isHovered} = useHover({}); let {isFocused, isFocusVisible, focusProps} = useFocusRing({within: true}); - let tagRef = useRef(); let tagRowRef = useRef(); let {clearButtonProps, labelProps, tagProps, tagRowProps} = useTag({ ...props, @@ -44,27 +43,25 @@ export function Tag>(props: SpectrumTagProps) { allowsRemoving, item, onRemove, - tagRef, tagRowRef }, state); return (
-
+ ref={tagRowRef}> +
(props: SpectrumTagGroupProps, ref: DOMRef ref={domRef}> {[...state.collection].map(item => ( - {item.childNodes[0].rendered} + {item.rendered} ))}
diff --git a/packages/@react-stately/tag/package.json b/packages/@react-stately/tag/package.json index c4f198a3ea6..b93d10a3efd 100644 --- a/packages/@react-stately/tag/package.json +++ b/packages/@react-stately/tag/package.json @@ -24,6 +24,7 @@ "dependencies": { "@babel/runtime": "^7.6.2", "@react-stately/grid": "^3.4.1", + "@react-stately/list": "^3.6.0", "@react-stately/utils": "^3.0.0", "@react-types/tag": "3.0.0-beta.0", "react-stately": "^3.19.0" diff --git a/packages/@react-stately/tag/src/useTagGroupState.ts b/packages/@react-stately/tag/src/useTagGroupState.ts index c3c82bcf8df..1d1be2378a0 100644 --- a/packages/@react-stately/tag/src/useTagGroupState.ts +++ b/packages/@react-stately/tag/src/useTagGroupState.ts @@ -10,47 +10,27 @@ * governing permissions and limitations under the License. */ -import {GridCollection, GridState, useGridState} from '@react-stately/grid'; -import {ITagGroupCollection, TagGroupProps} from '@react-types/tag'; -import {useEffect, useMemo, useRef} from 'react'; -import {useListState} from 'react-stately'; +import {ListState, useListState} from '@react-stately/list'; +import {TagGroupProps} from '@react-types/tag'; +import {useEffect, useRef} from 'react'; -export interface TagGroupState extends GridState>{} +export interface TagGroupState extends ListState{} /** * Provides state management for a TagGroup component. */ export function useTagGroupState(props: TagGroupProps): TagGroupState { - let listState = useListState(props); - - let gridCollection = useMemo(() => new GridCollection({ - columnCount: 1, // unused, but required for grid collections - items: [...listState.collection].map(item => ({ - type: 'item', - childNodes: [{ - ...item, - index: 0, - type: 'cell' - }] - })) - // eslint-disable-next-line react-hooks/exhaustive-deps - }), [listState.collection, props.allowsRemoving]); - - let state = useGridState({ - ...props, - collection: gridCollection, - focusMode: 'row' - }); + let state = useListState(props); let keyToRestoreOnRemove = useRef(null); useEffect(() => { // If the focused key is removed, restore focus to the tag before, or tag after if no tag before. - if (!listState.collection.getItem(state.selectionManager.focusedKey)) { + if (!state.collection.getItem(state.selectionManager.focusedKey)) { state.selectionManager.setFocusedKey(keyToRestoreOnRemove.current); } - keyToRestoreOnRemove.current = listState.collection.getKeyBefore(state.selectionManager.focusedKey) || listState.collection.getKeyAfter(state.selectionManager.focusedKey); - }, [listState.collection, state.collection, state.selectionManager]); + keyToRestoreOnRemove.current = state.collection.getKeyAfter(state.selectionManager.focusedKey) || state.collection.getKeyBefore(state.selectionManager.focusedKey); + }, [state.collection, state.selectionManager]); return state; } diff --git a/packages/@react-types/tag/src/index.d.ts b/packages/@react-types/tag/src/index.d.ts index e908d13dd32..c3dd95e915d 100644 --- a/packages/@react-types/tag/src/index.d.ts +++ b/packages/@react-types/tag/src/index.d.ts @@ -31,12 +31,9 @@ export interface TagProps extends ItemProps { allowsRemoving?: boolean, item: Node, onRemove?: (key: Key) => void, - tagRef: RefObject, tagRowRef: RefObject } interface SpectrumTagProps> extends TagProps { state: GridState } - -export interface ITagGroupCollection extends GridCollection{} From e0d4bdb7de98d226a170190c600fd7dbc8b6b794 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 30 Nov 2022 12:35:47 -0600 Subject: [PATCH 22/45] update tests --- .../@react-spectrum/tag/test/TagGroup.test.js | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index e10d40aaae3..1ba85e07673 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -56,7 +56,7 @@ describe('TagGroup', function () { }); it('provides context for Tag component', function () { - let {container} = render( + let {getAllByRole} = render( Tag 1 Tag 2 @@ -64,7 +64,7 @@ describe('TagGroup', function () { ); - let tags = container.querySelectorAll('[role="gridcell"]'); + let tags = getAllByRole('row'); expect(tags.length).toBe(3); fireEvent.keyDown(tags[1], {key: 'Delete'}); @@ -72,17 +72,17 @@ describe('TagGroup', function () { }); it('has correct accessibility roles', () => { - let tree = render( + let {getByRole, getAllByRole} = render( Tag 1 ); - let tagGroup = tree.getByRole('grid'); + let tagGroup = getByRole('grid'); expect(tagGroup).toBeInTheDocument(); - let tags = tree.getAllByRole('row'); - let cells = tree.getAllByRole('gridcell'); + let tags = getAllByRole('row'); + let cells = getAllByRole('gridcell'); expect(tags).toHaveLength(cells.length); }); @@ -94,7 +94,7 @@ describe('TagGroup', function () { ); - let tags = getAllByRole('gridcell'); + let tags = getAllByRole('row'); expect(tags[0]).toHaveAttribute('tabIndex', '0'); }); @@ -105,7 +105,7 @@ describe('TagGroup', function () { ${'(up/down arrows, ltr + horizontal) TagGroup'} | ${{locale: 'de-DE'}} | ${[{action: () => {userEvent.tab();}, index: 0}, {action: pressArrowDown, index: 1}, {action: pressArrowUp, index: 0}, {action: pressArrowUp, index: 2}]} ${'(up/down arrows, rtl + horizontal) TagGroup'} | ${{locale: 'ar-AE'}} | ${[{action: () => {userEvent.tab();}, index: 0}, {action: pressArrowUp, index: 2}, {action: pressArrowDown, index: 0}, {action: pressArrowDown, index: 1}]} `('$Name shifts button focus in the correct direction on key press', function ({Name, props, orders}) { - let tree = render( + let {getAllByRole} = render( Tag 1 @@ -115,7 +115,7 @@ describe('TagGroup', function () { ); - let tags = tree.getAllByRole('gridcell'); + let tags = getAllByRole('row'); orders.forEach(({action, index}, i) => { action(document.activeElement); expect(document.activeElement).toBe(tags[index]); @@ -173,7 +173,7 @@ describe('TagGroup', function () { let buttonBefore = getByLabelText('ButtonBefore'); let buttonAfter = getByLabelText('ButtonAfter'); - let tags = getAllByRole('gridcell'); + let tags = getAllByRole('row'); act(() => {buttonBefore.focus();}); userEvent.tab(); @@ -203,7 +203,7 @@ describe('TagGroup', function () { let buttonBefore = getByLabelText('ButtonBefore'); let buttonAfter = getByLabelText('ButtonAfter'); - let tags = getAllByRole('gridcell'); + let tags = getAllByRole('row'); act(() => {buttonBefore.focus();}); expect(buttonBefore).toHaveFocus(); userEvent.tab(); @@ -226,7 +226,7 @@ describe('TagGroup', function () { let buttonBefore = getByLabelText('ButtonBefore'); let buttonAfter = getByLabelText('ButtonAfter'); - let tags = getAllByRole('gridcell'); + let tags = getAllByRole('row'); act(() => {buttonAfter.focus();}); userEvent.tab({shift: true}); expect(document.activeElement).toBe(tags[1]); @@ -245,13 +245,12 @@ describe('TagGroup', function () { ); let tagGroup = getByRole('grid'); - let tagRow = tagGroup.children[0]; - let tag = tagRow.children[0]; + let tag = tagGroup.children[0]; expect(tag).not.toHaveAttribute('icon'); expect(tag).not.toHaveAttribute('unsafe_classname'); expect(tag).toHaveAttribute('class', expect.stringContaining('test-class')); expect(tag).toHaveAttribute('class', expect.stringContaining('-item')); - expect(tag).toHaveAttribute('role', 'gridcell'); + expect(tag).toHaveAttribute('role', 'row'); expect(tag).toHaveAttribute('tabIndex', '0'); }); @@ -265,7 +264,7 @@ describe('TagGroup', function () { ); - let tags = getAllByRole('gridcell'); + let tags = getAllByRole('row'); expect(tags.length).toBe(2); expect(tags[0]).toHaveAttribute('tabIndex', '0'); expect(tags[1]).toHaveAttribute('tabIndex', '0'); @@ -334,13 +333,13 @@ describe('TagGroup', function () { ); - let tags = getAllByRole('gridcell'); + let tags = getAllByRole('row'); userEvent.tab(); expect(document.activeElement).toBe(tags[0]); fireEvent.keyDown(document.activeElement, {key: props.keyPress}); expect(onRemoveSpy).toHaveBeenCalledTimes(1); expect(onRemoveSpy).toHaveBeenCalledWith(1); - tags = getAllByRole('gridcell'); + tags = getAllByRole('row'); expect(document.activeElement).toBe(tags[0]); pressArrowRight(tags[0]); expect(document.activeElement).toBe(tags[1]); From 1df22b34cb6f45469717db5261e20f9e17b12f67 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 30 Nov 2022 12:47:27 -0600 Subject: [PATCH 23/45] improve types --- packages/@react-aria/tag/src/index.ts | 2 +- packages/@react-aria/tag/src/useTag.ts | 2 +- packages/@react-aria/tag/src/useTagGroup.ts | 13 ++++--------- packages/@react-spectrum/tag/src/Tag.tsx | 3 +-- packages/@react-types/tag/package.json | 3 ++- packages/@react-types/tag/src/index.d.ts | 7 +++---- 6 files changed, 12 insertions(+), 18 deletions(-) diff --git a/packages/@react-aria/tag/src/index.ts b/packages/@react-aria/tag/src/index.ts index cf3885d4e84..d7d10b431ff 100644 --- a/packages/@react-aria/tag/src/index.ts +++ b/packages/@react-aria/tag/src/index.ts @@ -15,5 +15,5 @@ export {useTag} from './useTag'; export {useTagGroup} from './useTagGroup'; export type {TagProps} from '@react-types/tag'; -export type {AriaTagGroupProps, TagGroupAria} from './useTagGroup'; +export type {TagGroupAria} from './useTagGroup'; export type {TagAria} from './useTag'; diff --git a/packages/@react-aria/tag/src/useTag.ts b/packages/@react-aria/tag/src/useTag.ts index d00869b52a4..62db7ea3826 100644 --- a/packages/@react-aria/tag/src/useTag.ts +++ b/packages/@react-aria/tag/src/useTag.ts @@ -29,7 +29,7 @@ export interface TagAria { clearButtonProps: ButtonHTMLAttributes } -export function useTag>(props: TagProps, state: TagGroupState): TagAria { +export function useTag(props: TagProps, state: TagGroupState): TagAria { let { isFocused, allowsRemoving, diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index 7cd34bec594..af9a4f05a81 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -10,26 +10,21 @@ * governing permissions and limitations under the License. */ -import {DOMAttributes, DOMProps} from '@react-types/shared'; +import {AriaTagGroupProps} from '@react-types/tag'; +import {DOMAttributes} from '@react-types/shared'; import {filterDOMProps, mergeProps} from '@react-aria/utils'; -import {ReactNode, RefObject, useState} from 'react'; +import {RefObject, useState} from 'react'; import {TagGroupState} from '@react-stately/tag'; import {TagKeyboardDelegate} from './TagKeyboardDelegate'; import {useFocusWithin} from '@react-aria/interactions'; import {useGridList} from '@react-aria/gridlist'; import {useLocale} from '@react-aria/i18n'; -export interface AriaTagGroupProps extends DOMProps { - children: ReactNode, - isReadOnly?: boolean, // removes close button - validationState?: 'valid' | 'invalid' -} - export interface TagGroupAria { tagGroupProps: DOMAttributes } -export function useTagGroup(props: AriaTagGroupProps, state: TagGroupState, ref: RefObject): TagGroupAria { +export function useTagGroup(props: AriaTagGroupProps, state: TagGroupState, ref: RefObject): TagGroupAria { let {direction} = useLocale(); let keyboardDelegate = new TagKeyboardDelegate(state.collection, direction); let {gridProps} = useGridList({...props, keyboardDelegate}, state, ref); diff --git a/packages/@react-spectrum/tag/src/Tag.tsx b/packages/@react-spectrum/tag/src/Tag.tsx index 4dbcaadd7b9..52f72d51da3 100644 --- a/packages/@react-spectrum/tag/src/Tag.tsx +++ b/packages/@react-spectrum/tag/src/Tag.tsx @@ -12,7 +12,6 @@ import {classNames, SlotProvider, useSlotProps, useStyleProps} from '@react-spectrum/utils'; import {ClearButton} from '@react-spectrum/button'; -import {GridCollection} from '@react-stately/grid'; import {mergeProps} from '@react-aria/utils'; import React, {useRef} from 'react'; import {SpectrumTagProps} from '@react-types/tag'; @@ -22,7 +21,7 @@ import {useFocusRing} from '@react-aria/focus'; import {useHover} from '@react-aria/interactions'; import {useTag} from '@react-aria/tag'; -export function Tag>(props: SpectrumTagProps) { +export function Tag(props: SpectrumTagProps) { const { children, allowsRemoving, diff --git a/packages/@react-types/tag/package.json b/packages/@react-types/tag/package.json index 2ecc990495b..c885239a024 100644 --- a/packages/@react-types/tag/package.json +++ b/packages/@react-types/tag/package.json @@ -11,7 +11,8 @@ "dependencies": { "@react-stately/grid": "^3.4.1", "@react-types/grid": "^3.1.5", - "@react-types/shared": "^3.16.0" + "@react-types/shared": "^3.16.0", + "react-stately": "^3.19.0" }, "peerDependencies": { "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0" diff --git a/packages/@react-types/tag/src/index.d.ts b/packages/@react-types/tag/src/index.d.ts index c3dd95e915d..e40cf2b2fd4 100644 --- a/packages/@react-types/tag/src/index.d.ts +++ b/packages/@react-types/tag/src/index.d.ts @@ -11,9 +11,8 @@ */ import {AriaLabelingProps, CollectionBase, DOMProps, ItemProps, Node, StyleProps} from '@react-types/shared'; -import {GridCollection} from '@react-types/grid'; -import {GridState} from '@react-stately/grid'; import {Key, RefObject} from 'react'; +import {ListState} from 'react-stately'; export interface TagGroupProps extends Omit, 'disabledKeys'> { /** Whether the TagGroup allows removal of tags. */ @@ -34,6 +33,6 @@ export interface TagProps extends ItemProps { tagRowRef: RefObject } -interface SpectrumTagProps> extends TagProps { - state: GridState +interface SpectrumTagProps extends TagProps { + state: ListState } From 3faa12f172cd95f405aef4c40d03c0b1456a75d9 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 30 Nov 2022 12:54:59 -0600 Subject: [PATCH 24/45] lint --- packages/@react-aria/tag/src/useTag.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@react-aria/tag/src/useTag.ts b/packages/@react-aria/tag/src/useTag.ts index 62db7ea3826..47155b05879 100644 --- a/packages/@react-aria/tag/src/useTag.ts +++ b/packages/@react-aria/tag/src/useTag.ts @@ -13,7 +13,6 @@ import {ButtonHTMLAttributes, KeyboardEvent} from 'react'; import {DOMAttributes} from '@react-types/shared'; import {filterDOMProps, mergeProps, useId} from '@react-aria/utils'; -import {GridCollection} from '@react-stately/grid'; // @ts-ignore import intlMessages from '../intl/*.json'; import {TagGroupState} from '@react-stately/tag'; From 822369028546737bf8f5d0158ab3d805a45f1eaf Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 1 Dec 2022 10:20:20 -0600 Subject: [PATCH 25/45] update comments --- packages/@react-aria/tag/src/useTag.ts | 8 ++++++-- packages/@react-aria/tag/src/useTagGroup.ts | 7 +++++++ packages/@react-stately/tag/src/useTagGroupState.ts | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/@react-aria/tag/src/useTag.ts b/packages/@react-aria/tag/src/useTag.ts index 47155b05879..c93f6cd2707 100644 --- a/packages/@react-aria/tag/src/useTag.ts +++ b/packages/@react-aria/tag/src/useTag.ts @@ -28,6 +28,11 @@ export interface TagAria { clearButtonProps: ButtonHTMLAttributes } +/** + * Provides the behavior and accessibility implementation for a tag component. + * @param props - Props to be applied to the tag. + * @param state - State for the tag group, as returned by `useTagGroupState`. + */ export function useTag(props: TagProps, state: TagGroupState): TagAria { let { isFocused, @@ -45,10 +50,9 @@ export function useTag(props: TagProps, state: TagGroupState): TagAria node: item }, state, tagRowRef); - // We want TagKeyboardDelegate to handle keyboard events instead. + // We want the group to handle keyboard navigation between tags. delete rowProps.onKeyDownCapture; - function onKeyDown(e: KeyboardEvent) { if (e.key === 'Delete' || e.key === 'Backspace' || e.key === ' ') { onRemove(item.key); diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index af9a4f05a81..258b48dec40 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -24,6 +24,13 @@ export interface TagGroupAria { tagGroupProps: DOMAttributes } +/** + * Provides the behavior and accessibility implementation for a tag group component. + * Tags allow users to categorize content. They can represent keywords or people, and are grouped to describe an item or a search request. + * @param props - Props to be applied to the tag group. + * @param state - State for the tag group, as returned by `useTagGroupState`. + * @param ref - A ref to a DOM element for the tag group. + */ export function useTagGroup(props: AriaTagGroupProps, state: TagGroupState, ref: RefObject): TagGroupAria { let {direction} = useLocale(); let keyboardDelegate = new TagKeyboardDelegate(state.collection, direction); diff --git a/packages/@react-stately/tag/src/useTagGroupState.ts b/packages/@react-stately/tag/src/useTagGroupState.ts index 1d1be2378a0..fb9e321deb3 100644 --- a/packages/@react-stately/tag/src/useTagGroupState.ts +++ b/packages/@react-stately/tag/src/useTagGroupState.ts @@ -25,7 +25,7 @@ export function useTagGroupState(props: TagGroupProps): Tag let keyToRestoreOnRemove = useRef(null); useEffect(() => { - // If the focused key is removed, restore focus to the tag before, or tag after if no tag before. + // If the focused key is removed, restore focus to the tag after, or tag before if no tag after. if (!state.collection.getItem(state.selectionManager.focusedKey)) { state.selectionManager.setFocusedKey(keyToRestoreOnRemove.current); } From a2dcdc5b8ea517cbd74cfe10262f802c537e6039 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 1 Dec 2022 10:26:45 -0600 Subject: [PATCH 26/45] add test for clicking remove button --- .../@react-spectrum/tag/test/TagGroup.test.js | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index 1ba85e07673..9ab2e85567a 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {act, fireEvent, render} from '@react-spectrum/test-utils'; +import {act, fireEvent, render, within} from '@react-spectrum/test-utils'; import {Button} from '@react-spectrum/button'; import {chain} from '@react-aria/utils'; import {Item} from '@react-stately/collections'; @@ -299,6 +299,26 @@ describe('TagGroup', function () { expect(onRemoveSpy).toHaveBeenCalledWith('1'); }); + it('should remove tag when remove button is clicked', function () { + let {getAllByRole} = render( + + + Tag 1 + Tag 2 + Tag 3 + + + ); + + let tags = getAllByRole('row'); + fireEvent.click(tags[0]); + expect(onRemoveSpy).not.toHaveBeenCalled(); + + let removeButton = within(tags[0]).getByRole('button'); + fireEvent.click(removeButton); + expect(onRemoveSpy).toHaveBeenCalledWith('1'); + }); + it.each` Name | props ${'on `Delete` keypress'} | ${{keyPress: 'Delete'}} From 45fd98fb8323d9787064e1c4a35da0d6dda61711 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 1 Dec 2022 10:28:52 -0600 Subject: [PATCH 27/45] update tests to check that onRemove is called once --- packages/@react-spectrum/tag/test/TagGroup.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index 9ab2e85567a..2d23aab2877 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -296,6 +296,7 @@ describe('TagGroup', function () { let tag = getByText('Tag 1'); fireEvent.keyDown(tag, {key: props.keyPress}); + expect(onRemoveSpy).toHaveBeenCalledTimes(1); expect(onRemoveSpy).toHaveBeenCalledWith('1'); }); @@ -316,6 +317,7 @@ describe('TagGroup', function () { let removeButton = within(tags[0]).getByRole('button'); fireEvent.click(removeButton); + expect(onRemoveSpy).toHaveBeenCalledTimes(1); expect(onRemoveSpy).toHaveBeenCalledWith('1'); }); From e4be115e8702c61267ea4499a976237142818faa Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 1 Dec 2022 13:14:49 -0600 Subject: [PATCH 28/45] cleanup onPress and clearButtonProps type --- packages/@react-aria/tag/package.json | 1 + packages/@react-aria/tag/src/useTag.ts | 17 ++++++++--------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/@react-aria/tag/package.json b/packages/@react-aria/tag/package.json index 6bb713da8e3..013be3a04a8 100644 --- a/packages/@react-aria/tag/package.json +++ b/packages/@react-aria/tag/package.json @@ -31,6 +31,7 @@ "@react-aria/utils": "^3.14.1", "@react-stately/grid": "^3.4.1", "@react-stately/tag": "3.0.0-alpha.1", + "@react-types/button": "^3.7.0", "@react-types/grid": "^3.1.5", "@react-types/shared": "^3.16.0", "@react-types/tag": "3.0.0-beta.0" diff --git a/packages/@react-aria/tag/src/useTag.ts b/packages/@react-aria/tag/src/useTag.ts index c93f6cd2707..99ea8491e66 100644 --- a/packages/@react-aria/tag/src/useTag.ts +++ b/packages/@react-aria/tag/src/useTag.ts @@ -10,11 +10,12 @@ * governing permissions and limitations under the License. */ -import {ButtonHTMLAttributes, KeyboardEvent} from 'react'; +import {AriaButtonProps} from '@react-types/button'; import {DOMAttributes} from '@react-types/shared'; import {filterDOMProps, mergeProps, useId} from '@react-aria/utils'; // @ts-ignore import intlMessages from '../intl/*.json'; +import {KeyboardEvent} from 'react'; import {TagGroupState} from '@react-stately/tag'; import {TagProps} from '@react-types/tag'; import {useGridListItem} from '@react-aria/gridlist'; @@ -25,7 +26,7 @@ export interface TagAria { labelProps: DOMAttributes, tagProps: DOMAttributes, tagRowProps: DOMAttributes, - clearButtonProps: ButtonHTMLAttributes + clearButtonProps: AriaButtonProps } /** @@ -53,24 +54,22 @@ export function useTag(props: TagProps, state: TagGroupState): TagAria // We want the group to handle keyboard navigation between tags. delete rowProps.onKeyDownCapture; - function onKeyDown(e: KeyboardEvent) { + let onKeyDown = (e: KeyboardEvent) => { if (e.key === 'Delete' || e.key === 'Backspace' || e.key === ' ') { onRemove(item.key); e.preventDefault(); } - } - let pressProps = { - onPress: () => onRemove?.(item.key) }; isFocused = isFocused || state.selectionManager.focusedKey === item.key; let domProps = filterDOMProps(props); return { - clearButtonProps: mergeProps(pressProps, { + clearButtonProps: { 'aria-label': removeString, 'aria-labelledby': `${buttonId} ${labelId}`, - id: buttonId - }), + id: buttonId, + onPress: () => allowsRemoving && onRemove ? onRemove(item.key) : null + }, labelProps: { id: labelId }, From 2b72dbcdb88c9280edcff54070a45da799ad3649 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 2 Dec 2022 09:53:36 -0600 Subject: [PATCH 29/45] fix text overflow and cleanup styles --- .../components/tags/index.css | 51 +++++++++++-------- packages/@react-spectrum/tag/src/Tag.tsx | 13 +++-- 2 files changed, 38 insertions(+), 26 deletions(-) diff --git a/packages/@adobe/spectrum-css-temp/components/tags/index.css b/packages/@adobe/spectrum-css-temp/components/tags/index.css index 82cffe91b12..fe0ffa5e652 100644 --- a/packages/@adobe/spectrum-css-temp/components/tags/index.css +++ b/packages/@adobe/spectrum-css-temp/components/tags/index.css @@ -13,7 +13,7 @@ governing permissions and limitations under the License. @import '../commons/index.css'; .spectrum-Tags { - display: inline-flex; + display: flex; flex-wrap: wrap; margin: 0; @@ -58,28 +58,37 @@ governing permissions and limitations under the License. height: calc(var(--spectrum-tag-height) - (2 * var(--spectrum-tag-border-size))); width: var(--spectrum-global-dimension-size-300); } -} -.spectrum-Tag-icon { - grid-area: icon; - margin-inline-end: var(--spectrum-global-dimension-size-100); -} + .spectrum-Tag-cell { + overflow: hidden; + text-overflow: ellipsis; + display: flex; + align-items: center; + } -.spectrum-Tag-content { - grid-area: content; - block-size: 100%; - line-height: calc(var(--spectrum-tag-height) - calc(var(--spectrum-tag-border-size) * 2)); - margin-inline-end: var(--spectrum-tag-padding-x); - flex: 1 1 auto; - font-size: var(--spectrum-tag-text-size); - cursor: default; - overflow: hidden; - white-space: nowrap; - text-overflow: ellipsis; - outline: none; -} + .spectrum-Tag-icon { + grid-area: icon; + margin-inline-end: var(--spectrum-global-dimension-size-100); + } + + .spectrum-Tag-content { + grid-area: content; + block-size: 100%; + line-height: calc(var(--spectrum-tag-height) - calc(var(--spectrum-tag-border-size) * 2)); + margin-inline-end: var(--spectrum-tag-padding-x); + flex: 1 1 auto; + font-size: var(--spectrum-tag-text-size); + cursor: default; + overflow: hidden; + white-space: nowrap; + text-overflow: ellipsis; + outline: none; + } -.tags-removable { - margin-inline-end: 0; + &.is-removable { + .spectrum-Tag-content { + margin-inline-end: 0; + } + } } diff --git a/packages/@react-spectrum/tag/src/Tag.tsx b/packages/@react-spectrum/tag/src/Tag.tsx index 52f72d51da3..67c07d79514 100644 --- a/packages/@react-spectrum/tag/src/Tag.tsx +++ b/packages/@react-spectrum/tag/src/Tag.tsx @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {classNames, SlotProvider, useSlotProps, useStyleProps} from '@react-spectrum/utils'; +import {classNames, ClearSlots, SlotProvider, useSlotProps, useStyleProps} from '@react-spectrum/utils'; import {ClearButton} from '@react-spectrum/button'; import {mergeProps} from '@react-aria/utils'; import React, {useRef} from 'react'; @@ -54,21 +54,24 @@ export function Tag(props: SpectrumTagProps) { { 'focus-ring': isFocusVisible, 'is-focused': isFocused, - 'is-hovered': isHovered + 'is-hovered': isHovered, + 'is-removable': allowsRemoving }, styleProps.className )} ref={tagRowRef}>
- {typeof children === 'string' ? {children} : children} - {allowsRemoving && } + + {allowsRemoving && } +
From 6463bbf05e20988a1209bab34d8d21af4d3141ad Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 5 Dec 2022 14:00:10 -0600 Subject: [PATCH 30/45] update dependencies --- packages/@react-aria/tag/package.json | 1 - packages/@react-stately/tag/package.json | 5 +---- packages/@react-types/tag/package.json | 3 +-- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/@react-aria/tag/package.json b/packages/@react-aria/tag/package.json index 484b181e657..28650f1b15e 100644 --- a/packages/@react-aria/tag/package.json +++ b/packages/@react-aria/tag/package.json @@ -26,7 +26,6 @@ "@react-aria/gridlist": "^3.1.1", "@react-aria/i18n": "^3.6.2", "@react-aria/interactions": "^3.13.0", - "@react-aria/selection": "^3.12.0", "@react-aria/utils": "^3.14.1", "@react-stately/grid": "^3.4.1", "@react-stately/tag": "3.0.0-alpha.1", diff --git a/packages/@react-stately/tag/package.json b/packages/@react-stately/tag/package.json index b93d10a3efd..91d3c6d5822 100644 --- a/packages/@react-stately/tag/package.json +++ b/packages/@react-stately/tag/package.json @@ -23,11 +23,8 @@ }, "dependencies": { "@babel/runtime": "^7.6.2", - "@react-stately/grid": "^3.4.1", "@react-stately/list": "^3.6.0", - "@react-stately/utils": "^3.0.0", - "@react-types/tag": "3.0.0-beta.0", - "react-stately": "^3.19.0" + "@react-types/tag": "3.0.0-beta.0" }, "peerDependencies": { "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0" diff --git a/packages/@react-types/tag/package.json b/packages/@react-types/tag/package.json index c885239a024..2ecc990495b 100644 --- a/packages/@react-types/tag/package.json +++ b/packages/@react-types/tag/package.json @@ -11,8 +11,7 @@ "dependencies": { "@react-stately/grid": "^3.4.1", "@react-types/grid": "^3.1.5", - "@react-types/shared": "^3.16.0", - "react-stately": "^3.19.0" + "@react-types/shared": "^3.16.0" }, "peerDependencies": { "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0" From 30a808ded2f4c79be396a90d9fefd170ef6d2aed Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 5 Dec 2022 14:01:26 -0600 Subject: [PATCH 31/45] cleanup Tag --- packages/@react-spectrum/tag/src/Tag.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/@react-spectrum/tag/src/Tag.tsx b/packages/@react-spectrum/tag/src/Tag.tsx index 67c07d79514..da7a25917b1 100644 --- a/packages/@react-spectrum/tag/src/Tag.tsx +++ b/packages/@react-spectrum/tag/src/Tag.tsx @@ -79,14 +79,11 @@ export function Tag(props: SpectrumTagProps) { } function TagRemoveButton(props) { - props = useSlotProps(props, 'tagRemoveButton'); let {styleProps} = useStyleProps(props); - let clearBtnRef = useRef(); return ( + {...styleProps}> From 4b4ec67a3a0b4dc8077538b610950e553f9e64b8 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 5 Dec 2022 14:20:04 -0600 Subject: [PATCH 32/45] update keyboard focus tests --- .../@react-spectrum/tag/test/TagGroup.test.js | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index 2d23aab2877..7196db1e207 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -260,22 +260,43 @@ describe('TagGroup', function () { Tag 1 Tag 2 + Tag 3 + Tag 4 ); let tags = getAllByRole('row'); - expect(tags.length).toBe(2); + expect(tags.length).toBe(4); expect(tags[0]).toHaveAttribute('tabIndex', '0'); expect(tags[1]).toHaveAttribute('tabIndex', '0'); + expect(tags[2]).toHaveAttribute('tabIndex', '0'); + expect(tags[3]).toHaveAttribute('tabIndex', '0'); - act(() => tags[0].focus()); + userEvent.tab(); expect(tags[0]).toHaveAttribute('tabIndex', '0'); expect(tags[1]).toHaveAttribute('tabIndex', '-1'); + expect(tags[2]).toHaveAttribute('tabIndex', '-1'); + expect(tags[3]).toHaveAttribute('tabIndex', '-1'); + expect(document.activeElement).toBe(tags[0]); - pressArrowRight(tags[0]); - expect(tags[0]).toHaveAttribute('tabIndex', '-1'); - expect(tags[1]).toHaveAttribute('tabIndex', '0'); + fireEvent.keyDown(document.activeElement, {key: 'ArrowRight'}); + expect(document.activeElement).toBe(tags[1]); + + fireEvent.keyDown(document.activeElement, {key: 'ArrowLeft'}); + expect(document.activeElement).toBe(tags[0]); + + fireEvent.keyDown(document.activeElement, {key: 'End'}); + expect(document.activeElement).toBe(tags[3]); + + fireEvent.keyDown(document.activeElement, {key: 'Home'}); + expect(document.activeElement).toBe(tags[0]); + + fireEvent.keyDown(document.activeElement, {key: 'PageDown'}); + expect(document.activeElement).toBe(tags[1]); + + fireEvent.keyDown(document.activeElement, {key: 'PageUp'}); + expect(document.activeElement).toBe(tags[0]); }); it.each` From c61e143c61e8165ca944a33d67ec8a48a2c9db48 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 5 Dec 2022 14:44:26 -0600 Subject: [PATCH 33/45] fix types and imports --- packages/@react-aria/tag/src/useTag.ts | 3 +-- packages/@react-aria/tag/src/useTagGroup.ts | 3 +-- packages/@react-spectrum/tag/src/Tag.tsx | 2 +- packages/@react-stately/tag/package.json | 3 ++- packages/@react-stately/tag/src/useTagGroupState.ts | 6 ++---- packages/@react-types/list/package.json | 3 ++- packages/@react-types/list/src/index.d.ts | 1 + packages/@react-types/tag/package.json | 3 +-- packages/@react-types/tag/src/index.d.ts | 6 ++++-- 9 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/@react-aria/tag/src/useTag.ts b/packages/@react-aria/tag/src/useTag.ts index 99ea8491e66..658a6fb2529 100644 --- a/packages/@react-aria/tag/src/useTag.ts +++ b/packages/@react-aria/tag/src/useTag.ts @@ -16,8 +16,7 @@ import {filterDOMProps, mergeProps, useId} from '@react-aria/utils'; // @ts-ignore import intlMessages from '../intl/*.json'; import {KeyboardEvent} from 'react'; -import {TagGroupState} from '@react-stately/tag'; -import {TagProps} from '@react-types/tag'; +import {TagGroupState, TagProps} from '@react-types/tag'; import {useGridListItem} from '@react-aria/gridlist'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index 258b48dec40..392e5c62672 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -10,11 +10,10 @@ * governing permissions and limitations under the License. */ -import {AriaTagGroupProps} from '@react-types/tag'; +import {AriaTagGroupProps, TagGroupState} from '@react-types/tag'; import {DOMAttributes} from '@react-types/shared'; import {filterDOMProps, mergeProps} from '@react-aria/utils'; import {RefObject, useState} from 'react'; -import {TagGroupState} from '@react-stately/tag'; import {TagKeyboardDelegate} from './TagKeyboardDelegate'; import {useFocusWithin} from '@react-aria/interactions'; import {useGridList} from '@react-aria/gridlist'; diff --git a/packages/@react-spectrum/tag/src/Tag.tsx b/packages/@react-spectrum/tag/src/Tag.tsx index da7a25917b1..85bcbafe7eb 100644 --- a/packages/@react-spectrum/tag/src/Tag.tsx +++ b/packages/@react-spectrum/tag/src/Tag.tsx @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {classNames, ClearSlots, SlotProvider, useSlotProps, useStyleProps} from '@react-spectrum/utils'; +import {classNames, ClearSlots, SlotProvider, useStyleProps} from '@react-spectrum/utils'; import {ClearButton} from '@react-spectrum/button'; import {mergeProps} from '@react-aria/utils'; import React, {useRef} from 'react'; diff --git a/packages/@react-stately/tag/package.json b/packages/@react-stately/tag/package.json index 91d3c6d5822..653612fd42c 100644 --- a/packages/@react-stately/tag/package.json +++ b/packages/@react-stately/tag/package.json @@ -24,7 +24,8 @@ "dependencies": { "@babel/runtime": "^7.6.2", "@react-stately/list": "^3.6.0", - "@react-types/tag": "3.0.0-beta.0" + "@react-types/tag": "3.0.0-beta.0", + "@swc/helpers": "^0.4.14" }, "peerDependencies": { "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0" diff --git a/packages/@react-stately/tag/src/useTagGroupState.ts b/packages/@react-stately/tag/src/useTagGroupState.ts index fb9e321deb3..8831c381158 100644 --- a/packages/@react-stately/tag/src/useTagGroupState.ts +++ b/packages/@react-stately/tag/src/useTagGroupState.ts @@ -10,11 +10,9 @@ * governing permissions and limitations under the License. */ -import {ListState, useListState} from '@react-stately/list'; -import {TagGroupProps} from '@react-types/tag'; +import {TagGroupProps, TagGroupState} from '@react-types/tag'; import {useEffect, useRef} from 'react'; - -export interface TagGroupState extends ListState{} +import {useListState} from '@react-stately/list'; /** * Provides state management for a TagGroup component. diff --git a/packages/@react-types/list/package.json b/packages/@react-types/list/package.json index b5fca845a3c..7d29e80c9e5 100644 --- a/packages/@react-types/list/package.json +++ b/packages/@react-types/list/package.json @@ -10,7 +10,8 @@ }, "dependencies": { "@react-aria/gridlist": "^3.1.1", - "@react-spectrum/list": "^3.2.0" + "@react-spectrum/list": "^3.2.0", + "@react-stately/list": "^3.6.0" }, "peerDependencies": { "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0" diff --git a/packages/@react-types/list/src/index.d.ts b/packages/@react-types/list/src/index.d.ts index 361a05152a0..3dd1681799c 100644 --- a/packages/@react-types/list/src/index.d.ts +++ b/packages/@react-types/list/src/index.d.ts @@ -11,4 +11,5 @@ */ export type {AriaGridListProps, GridListProps} from '@react-aria/gridlist'; +export type {ListState} from '@react-stately/list'; export type {SpectrumListViewProps} from '@react-spectrum/list'; diff --git a/packages/@react-types/tag/package.json b/packages/@react-types/tag/package.json index 2ecc990495b..eb78847e960 100644 --- a/packages/@react-types/tag/package.json +++ b/packages/@react-types/tag/package.json @@ -9,8 +9,7 @@ "url": "https://github.com/adobe/react-spectrum" }, "dependencies": { - "@react-stately/grid": "^3.4.1", - "@react-types/grid": "^3.1.5", + "@react-types/list": "^3.1.1", "@react-types/shared": "^3.16.0" }, "peerDependencies": { diff --git a/packages/@react-types/tag/src/index.d.ts b/packages/@react-types/tag/src/index.d.ts index e40cf2b2fd4..c1818805d1a 100644 --- a/packages/@react-types/tag/src/index.d.ts +++ b/packages/@react-types/tag/src/index.d.ts @@ -12,7 +12,7 @@ import {AriaLabelingProps, CollectionBase, DOMProps, ItemProps, Node, StyleProps} from '@react-types/shared'; import {Key, RefObject} from 'react'; -import {ListState} from 'react-stately'; +import {ListState} from '@react-types/list'; export interface TagGroupProps extends Omit, 'disabledKeys'> { /** Whether the TagGroup allows removal of tags. */ @@ -33,6 +33,8 @@ export interface TagProps extends ItemProps { tagRowRef: RefObject } +export interface TagGroupState extends ListState{} + interface SpectrumTagProps extends TagProps { - state: ListState + state: TagGroupState } From 34e00466898e8102093d79b43fdc7516f398cf77 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 6 Dec 2022 11:42:47 -0600 Subject: [PATCH 34/45] style fix --- packages/@adobe/spectrum-css-temp/components/tags/index.css | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/@adobe/spectrum-css-temp/components/tags/index.css b/packages/@adobe/spectrum-css-temp/components/tags/index.css index fe0ffa5e652..28d36d50e42 100644 --- a/packages/@adobe/spectrum-css-temp/components/tags/index.css +++ b/packages/@adobe/spectrum-css-temp/components/tags/index.css @@ -27,7 +27,7 @@ governing permissions and limitations under the License. --spectrum-focus-ring-border-size: var(--spectrum-tag-border-size); display: grid; - grid-template-columns: 1fr auto; + grid-template-columns: auto 1fr auto; grid-template-areas: "icon content action"; align-items: center; box-sizing: border-box; @@ -73,7 +73,6 @@ governing permissions and limitations under the License. .spectrum-Tag-content { grid-area: content; - block-size: 100%; line-height: calc(var(--spectrum-tag-height) - calc(var(--spectrum-tag-border-size) * 2)); margin-inline-end: var(--spectrum-tag-padding-x); flex: 1 1 auto; From db6132018f03612e76415b5d36de1754f2c16458 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 6 Dec 2022 11:43:00 -0600 Subject: [PATCH 35/45] update tests --- .../@react-spectrum/tag/test/TagGroup.test.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index 7196db1e207..d5bfdf1dcca 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {act, fireEvent, render, within} from '@react-spectrum/test-utils'; +import {act, fireEvent, render, triggerPress, within} from '@react-spectrum/test-utils'; import {Button} from '@react-spectrum/button'; import {chain} from '@react-aria/utils'; import {Item} from '@react-stately/collections'; @@ -23,6 +23,7 @@ import userEvent from '@testing-library/user-event'; function pressKeyOnButton(key) { return (button) => { fireEvent.keyDown(button, {key}); + fireEvent.keyUp(button, {key}); }; } @@ -68,6 +69,7 @@ describe('TagGroup', function () { expect(tags.length).toBe(3); fireEvent.keyDown(tags[1], {key: 'Delete'}); + fireEvent.keyUp(tags[1], {key: 'Delete'}); expect(onRemoveSpy).toHaveBeenCalledTimes(1); }); @@ -281,21 +283,27 @@ describe('TagGroup', function () { expect(document.activeElement).toBe(tags[0]); fireEvent.keyDown(document.activeElement, {key: 'ArrowRight'}); + fireEvent.keyUp(document.activeElement, {key: 'ArrowRight'}); expect(document.activeElement).toBe(tags[1]); fireEvent.keyDown(document.activeElement, {key: 'ArrowLeft'}); + fireEvent.keyUp(document.activeElement, {key: 'ArrowLeft'}); expect(document.activeElement).toBe(tags[0]); fireEvent.keyDown(document.activeElement, {key: 'End'}); + fireEvent.keyUp(document.activeElement, {key: 'End'}); expect(document.activeElement).toBe(tags[3]); fireEvent.keyDown(document.activeElement, {key: 'Home'}); + fireEvent.keyUp(document.activeElement, {key: 'Home'}); expect(document.activeElement).toBe(tags[0]); fireEvent.keyDown(document.activeElement, {key: 'PageDown'}); + fireEvent.keyUp(document.activeElement, {key: 'PageDown'}); expect(document.activeElement).toBe(tags[1]); fireEvent.keyDown(document.activeElement, {key: 'PageUp'}); + fireEvent.keyUp(document.activeElement, {key: 'PageUp'}); expect(document.activeElement).toBe(tags[0]); }); @@ -317,6 +325,7 @@ describe('TagGroup', function () { let tag = getByText('Tag 1'); fireEvent.keyDown(tag, {key: props.keyPress}); + fireEvent.keyUp(tag, {key: props.keyPress}); expect(onRemoveSpy).toHaveBeenCalledTimes(1); expect(onRemoveSpy).toHaveBeenCalledWith('1'); }); @@ -333,11 +342,11 @@ describe('TagGroup', function () { ); let tags = getAllByRole('row'); - fireEvent.click(tags[0]); + triggerPress(tags[0]); expect(onRemoveSpy).not.toHaveBeenCalled(); let removeButton = within(tags[0]).getByRole('button'); - fireEvent.click(removeButton); + triggerPress(removeButton); expect(onRemoveSpy).toHaveBeenCalledTimes(1); expect(onRemoveSpy).toHaveBeenCalledWith('1'); }); @@ -380,6 +389,7 @@ describe('TagGroup', function () { userEvent.tab(); expect(document.activeElement).toBe(tags[0]); fireEvent.keyDown(document.activeElement, {key: props.keyPress}); + fireEvent.keyUp(document.activeElement, {key: props.keyPress}); expect(onRemoveSpy).toHaveBeenCalledTimes(1); expect(onRemoveSpy).toHaveBeenCalledWith(1); tags = getAllByRole('row'); From 9c5cd8a211b385b2e74da5fc57a113406c774c71 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 6 Dec 2022 12:12:51 -0600 Subject: [PATCH 36/45] only restore focus if onRemove called --- packages/@react-aria/tag/src/useTag.ts | 5 +++-- .../tag/src/useTagGroupState.ts | 20 +++++++++---------- packages/@react-types/tag/src/index.d.ts | 4 +++- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/packages/@react-aria/tag/src/useTag.ts b/packages/@react-aria/tag/src/useTag.ts index 658a6fb2529..e149343455d 100644 --- a/packages/@react-aria/tag/src/useTag.ts +++ b/packages/@react-aria/tag/src/useTag.ts @@ -11,8 +11,8 @@ */ import {AriaButtonProps} from '@react-types/button'; +import {chain, filterDOMProps, mergeProps, useId} from '@react-aria/utils'; import {DOMAttributes} from '@react-types/shared'; -import {filterDOMProps, mergeProps, useId} from '@react-aria/utils'; // @ts-ignore import intlMessages from '../intl/*.json'; import {KeyboardEvent} from 'react'; @@ -37,7 +37,6 @@ export function useTag(props: TagProps, state: TagGroupState): TagAria let { isFocused, allowsRemoving, - onRemove, item, tagRowRef } = props; @@ -53,6 +52,8 @@ export function useTag(props: TagProps, state: TagGroupState): TagAria // We want the group to handle keyboard navigation between tags. delete rowProps.onKeyDownCapture; + let onRemove = (key) => chain(props.onRemove, state.onRemove)(key); + let onKeyDown = (e: KeyboardEvent) => { if (e.key === 'Delete' || e.key === 'Backspace' || e.key === ' ') { onRemove(item.key); diff --git a/packages/@react-stately/tag/src/useTagGroupState.ts b/packages/@react-stately/tag/src/useTagGroupState.ts index 8831c381158..1f4a1da0ef2 100644 --- a/packages/@react-stately/tag/src/useTagGroupState.ts +++ b/packages/@react-stately/tag/src/useTagGroupState.ts @@ -11,7 +11,6 @@ */ import {TagGroupProps, TagGroupState} from '@react-types/tag'; -import {useEffect, useRef} from 'react'; import {useListState} from '@react-stately/list'; /** @@ -20,15 +19,14 @@ import {useListState} from '@react-stately/list'; export function useTagGroupState(props: TagGroupProps): TagGroupState { let state = useListState(props); - let keyToRestoreOnRemove = useRef(null); + let onRemove = (key) => { + // If a tag is removed, restore focus to the tag after, or tag before if no tag after. + let restoreKey = state.collection.getKeyAfter(key) || state.collection.getKeyBefore(key); + state.selectionManager.setFocusedKey(restoreKey); + }; - useEffect(() => { - // If the focused key is removed, restore focus to the tag after, or tag before if no tag after. - if (!state.collection.getItem(state.selectionManager.focusedKey)) { - state.selectionManager.setFocusedKey(keyToRestoreOnRemove.current); - } - keyToRestoreOnRemove.current = state.collection.getKeyAfter(state.selectionManager.focusedKey) || state.collection.getKeyBefore(state.selectionManager.focusedKey); - }, [state.collection, state.selectionManager]); - - return state; + return { + onRemove, + ...state + }; } diff --git a/packages/@react-types/tag/src/index.d.ts b/packages/@react-types/tag/src/index.d.ts index c1818805d1a..7758da17c30 100644 --- a/packages/@react-types/tag/src/index.d.ts +++ b/packages/@react-types/tag/src/index.d.ts @@ -33,7 +33,9 @@ export interface TagProps extends ItemProps { tagRowRef: RefObject } -export interface TagGroupState extends ListState{} +export interface TagGroupState extends ListState{ + onRemove?: (key: Key) => void +} interface SpectrumTagProps extends TagProps { state: TagGroupState From ee5d3aa29f8a16c5e6e4c6828867e97b1efb9abd Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 7 Dec 2022 13:22:41 -0600 Subject: [PATCH 37/45] cleanup chain --- packages/@react-aria/tag/src/useTag.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-aria/tag/src/useTag.ts b/packages/@react-aria/tag/src/useTag.ts index e149343455d..aa4db730753 100644 --- a/packages/@react-aria/tag/src/useTag.ts +++ b/packages/@react-aria/tag/src/useTag.ts @@ -52,7 +52,7 @@ export function useTag(props: TagProps, state: TagGroupState): TagAria // We want the group to handle keyboard navigation between tags. delete rowProps.onKeyDownCapture; - let onRemove = (key) => chain(props.onRemove, state.onRemove)(key); + let onRemove = chain(props.onRemove, state.onRemove); let onKeyDown = (e: KeyboardEvent) => { if (e.key === 'Delete' || e.key === 'Backspace' || e.key === ' ') { From 274d027c98d98794048d509353e852525abcd891 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 16 Dec 2022 16:32:45 -0600 Subject: [PATCH 38/45] formatting --- packages/@react-types/tag/src/index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-types/tag/src/index.d.ts b/packages/@react-types/tag/src/index.d.ts index 7758da17c30..a7ab9a1fe44 100644 --- a/packages/@react-types/tag/src/index.d.ts +++ b/packages/@react-types/tag/src/index.d.ts @@ -33,7 +33,7 @@ export interface TagProps extends ItemProps { tagRowRef: RefObject } -export interface TagGroupState extends ListState{ +export interface TagGroupState extends ListState{ onRemove?: (key: Key) => void } From 7137dcaf83150323cdbee74df42da22e84cac246 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 16 Dec 2022 17:25:39 -0600 Subject: [PATCH 39/45] lint --- packages/@react-stately/tag/package.json | 7 +------ packages/@react-types/tag/src/index.d.ts | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/@react-stately/tag/package.json b/packages/@react-stately/tag/package.json index 653612fd42c..ec05e192d70 100644 --- a/packages/@react-stately/tag/package.json +++ b/packages/@react-stately/tag/package.json @@ -4,13 +4,8 @@ "description": "Spectrum UI components in React", "license": "Apache-2.0", "main": "dist/main.js", - "module": "dist/module.mjs", + "module": "dist/module.js", "types": "dist/types.d.ts", - "exports": { - "types": "dist/types.d.ts", - "import": "dist/module.mjs", - "require": "dist/main.js" - }, "source": "src/index.ts", "files": [ "dist", diff --git a/packages/@react-types/tag/src/index.d.ts b/packages/@react-types/tag/src/index.d.ts index a7ab9a1fe44..430ba72dcae 100644 --- a/packages/@react-types/tag/src/index.d.ts +++ b/packages/@react-types/tag/src/index.d.ts @@ -37,6 +37,6 @@ export interface TagGroupState extends ListState{ onRemove?: (key: Key) => void } -interface SpectrumTagProps extends TagProps { +export interface SpectrumTagProps extends TagProps { state: TagGroupState } From d9fe732628b5188b1b51494ca742f9cc76161f6a Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 16 Dec 2022 17:52:47 -0600 Subject: [PATCH 40/45] fix types --- packages/@react-aria/tag/src/useTag.ts | 3 ++- packages/@react-aria/tag/src/useTagGroup.ts | 3 ++- packages/@react-stately/tag/src/useTagGroupState.ts | 9 +++++++-- packages/@react-types/tag/package.json | 2 +- packages/@react-types/tag/src/index.d.ts | 6 +----- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/@react-aria/tag/src/useTag.ts b/packages/@react-aria/tag/src/useTag.ts index aa4db730753..a3b1610d751 100644 --- a/packages/@react-aria/tag/src/useTag.ts +++ b/packages/@react-aria/tag/src/useTag.ts @@ -16,7 +16,8 @@ import {DOMAttributes} from '@react-types/shared'; // @ts-ignore import intlMessages from '../intl/*.json'; import {KeyboardEvent} from 'react'; -import {TagGroupState, TagProps} from '@react-types/tag'; +import type {TagGroupState} from '@react-stately/tag'; +import {TagProps} from '@react-types/tag'; import {useGridListItem} from '@react-aria/gridlist'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index 392e5c62672..8c94126cc72 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -10,10 +10,11 @@ * governing permissions and limitations under the License. */ -import {AriaTagGroupProps, TagGroupState} from '@react-types/tag'; +import {AriaTagGroupProps} from '@react-types/tag'; import {DOMAttributes} from '@react-types/shared'; import {filterDOMProps, mergeProps} from '@react-aria/utils'; import {RefObject, useState} from 'react'; +import type {TagGroupState} from '@react-stately/tag'; import {TagKeyboardDelegate} from './TagKeyboardDelegate'; import {useFocusWithin} from '@react-aria/interactions'; import {useGridList} from '@react-aria/gridlist'; diff --git a/packages/@react-stately/tag/src/useTagGroupState.ts b/packages/@react-stately/tag/src/useTagGroupState.ts index 1f4a1da0ef2..4d0663dc692 100644 --- a/packages/@react-stately/tag/src/useTagGroupState.ts +++ b/packages/@react-stately/tag/src/useTagGroupState.ts @@ -10,8 +10,13 @@ * governing permissions and limitations under the License. */ -import {TagGroupProps, TagGroupState} from '@react-types/tag'; -import {useListState} from '@react-stately/list'; +import {Key} from 'react'; +import {ListState, useListState} from '@react-stately/list'; +import {TagGroupProps} from '@react-types/tag'; + +export interface TagGroupState extends ListState{ + onRemove?: (key: Key) => void +} /** * Provides state management for a TagGroup component. diff --git a/packages/@react-types/tag/package.json b/packages/@react-types/tag/package.json index 6edef1ac51b..b34a878d23c 100644 --- a/packages/@react-types/tag/package.json +++ b/packages/@react-types/tag/package.json @@ -9,8 +9,8 @@ "url": "https://github.com/adobe/react-spectrum" }, "dependencies": { - "@react-types/list": "^3.1.1", "@react-stately/grid": "^3.4.2", + "@react-types/list": "^3.1.1", "@react-types/shared": "^3.16.0" }, "peerDependencies": { diff --git a/packages/@react-types/tag/src/index.d.ts b/packages/@react-types/tag/src/index.d.ts index 430ba72dcae..c8e7e6de0c2 100644 --- a/packages/@react-types/tag/src/index.d.ts +++ b/packages/@react-types/tag/src/index.d.ts @@ -12,7 +12,7 @@ import {AriaLabelingProps, CollectionBase, DOMProps, ItemProps, Node, StyleProps} from '@react-types/shared'; import {Key, RefObject} from 'react'; -import {ListState} from '@react-types/list'; +import type {TagGroupState} from '@react-stately/tag'; export interface TagGroupProps extends Omit, 'disabledKeys'> { /** Whether the TagGroup allows removal of tags. */ @@ -33,10 +33,6 @@ export interface TagProps extends ItemProps { tagRowRef: RefObject } -export interface TagGroupState extends ListState{ - onRemove?: (key: Key) => void -} - export interface SpectrumTagProps extends TagProps { state: TagGroupState } From 507195aa803b97344758e65d2ab979166254a1a2 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 16 Dec 2022 18:01:46 -0600 Subject: [PATCH 41/45] lint --- packages/@react-aria/tag/package.json | 1 + packages/@react-types/tag/package.json | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/@react-aria/tag/package.json b/packages/@react-aria/tag/package.json index f2884b7da6a..f8a69edc127 100644 --- a/packages/@react-aria/tag/package.json +++ b/packages/@react-aria/tag/package.json @@ -21,6 +21,7 @@ "@react-aria/i18n": "^3.6.3", "@react-aria/interactions": "^3.13.1", "@react-aria/utils": "^3.14.2", + "@react-stately/tag": "3.0.0-alpha.1", "@react-types/button": "^3.7.0", "@react-types/shared": "^3.16.0", "@react-types/tag": "3.0.0-beta.1", diff --git a/packages/@react-types/tag/package.json b/packages/@react-types/tag/package.json index b34a878d23c..290ac6ee04b 100644 --- a/packages/@react-types/tag/package.json +++ b/packages/@react-types/tag/package.json @@ -10,6 +10,7 @@ }, "dependencies": { "@react-stately/grid": "^3.4.2", + "@react-stately/tag": "3.0.0-alpha.1", "@react-types/list": "^3.1.1", "@react-types/shared": "^3.16.0" }, From 7f3682927a339289be32af355c1fc98d7ba76fa0 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 19 Dec 2022 09:45:09 -0600 Subject: [PATCH 42/45] lint --- packages/@react-spectrum/tag/src/Tag.tsx | 7 ++++++- packages/@react-types/tag/package.json | 3 --- packages/@react-types/tag/src/index.d.ts | 5 ----- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/@react-spectrum/tag/src/Tag.tsx b/packages/@react-spectrum/tag/src/Tag.tsx index 85bcbafe7eb..e8c8579cec3 100644 --- a/packages/@react-spectrum/tag/src/Tag.tsx +++ b/packages/@react-spectrum/tag/src/Tag.tsx @@ -14,13 +14,18 @@ import {classNames, ClearSlots, SlotProvider, useStyleProps} from '@react-spectr import {ClearButton} from '@react-spectrum/button'; import {mergeProps} from '@react-aria/utils'; import React, {useRef} from 'react'; -import {SpectrumTagProps} from '@react-types/tag'; import styles from '@adobe/spectrum-css-temp/components/tags/vars.css'; +import type {TagGroupState} from '@react-stately/tag'; +import {TagProps} from '@react-types/tag'; import {Text} from '@react-spectrum/text'; import {useFocusRing} from '@react-aria/focus'; import {useHover} from '@react-aria/interactions'; import {useTag} from '@react-aria/tag'; +export interface SpectrumTagProps extends TagProps { + state: TagGroupState +} + export function Tag(props: SpectrumTagProps) { const { children, diff --git a/packages/@react-types/tag/package.json b/packages/@react-types/tag/package.json index 290ac6ee04b..c2ccd24b075 100644 --- a/packages/@react-types/tag/package.json +++ b/packages/@react-types/tag/package.json @@ -9,9 +9,6 @@ "url": "https://github.com/adobe/react-spectrum" }, "dependencies": { - "@react-stately/grid": "^3.4.2", - "@react-stately/tag": "3.0.0-alpha.1", - "@react-types/list": "^3.1.1", "@react-types/shared": "^3.16.0" }, "peerDependencies": { diff --git a/packages/@react-types/tag/src/index.d.ts b/packages/@react-types/tag/src/index.d.ts index c8e7e6de0c2..abf451cf028 100644 --- a/packages/@react-types/tag/src/index.d.ts +++ b/packages/@react-types/tag/src/index.d.ts @@ -12,7 +12,6 @@ import {AriaLabelingProps, CollectionBase, DOMProps, ItemProps, Node, StyleProps} from '@react-types/shared'; import {Key, RefObject} from 'react'; -import type {TagGroupState} from '@react-stately/tag'; export interface TagGroupProps extends Omit, 'disabledKeys'> { /** Whether the TagGroup allows removal of tags. */ @@ -32,7 +31,3 @@ export interface TagProps extends ItemProps { onRemove?: (key: Key) => void, tagRowRef: RefObject } - -export interface SpectrumTagProps extends TagProps { - state: TagGroupState -} From 4365154cf956775173ddb47a36792f330b98661c Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 19 Dec 2022 12:12:54 -0600 Subject: [PATCH 43/45] lint --- packages/@react-stately/tag/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-stately/tag/package.json b/packages/@react-stately/tag/package.json index ec05e192d70..ce9b085151f 100644 --- a/packages/@react-stately/tag/package.json +++ b/packages/@react-stately/tag/package.json @@ -19,7 +19,7 @@ "dependencies": { "@babel/runtime": "^7.6.2", "@react-stately/list": "^3.6.0", - "@react-types/tag": "3.0.0-beta.0", + "@react-types/tag": "3.0.0-beta.1", "@swc/helpers": "^0.4.14" }, "peerDependencies": { From d13c41c2856e7f371bdea67cd73fd145f711648e Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 20 Dec 2022 13:26:37 -0600 Subject: [PATCH 44/45] formatting --- packages/@react-aria/tag/src/TagKeyboardDelegate.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@react-aria/tag/src/TagKeyboardDelegate.ts b/packages/@react-aria/tag/src/TagKeyboardDelegate.ts index 3d974a3baf5..3f78d483f10 100644 --- a/packages/@react-aria/tag/src/TagKeyboardDelegate.ts +++ b/packages/@react-aria/tag/src/TagKeyboardDelegate.ts @@ -12,6 +12,7 @@ import {Collection, Direction, KeyboardDelegate} from '@react-types/shared'; import {Key} from 'react'; + export class TagKeyboardDelegate implements KeyboardDelegate { private collection: Collection; private direction: Direction; From aec919eeaba6720366c39549da13f1eae2b45cef Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 3 Jan 2023 13:00:56 -0600 Subject: [PATCH 45/45] remove unnecessary export --- packages/@react-types/list/src/index.d.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@react-types/list/src/index.d.ts b/packages/@react-types/list/src/index.d.ts index 3dd1681799c..361a05152a0 100644 --- a/packages/@react-types/list/src/index.d.ts +++ b/packages/@react-types/list/src/index.d.ts @@ -11,5 +11,4 @@ */ export type {AriaGridListProps, GridListProps} from '@react-aria/gridlist'; -export type {ListState} from '@react-stately/list'; export type {SpectrumListViewProps} from '@react-spectrum/list';