From aad63c35a9f8a27b4ecf587509cdc4de1c149281 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 4 Apr 2023 11:41:58 -0500 Subject: [PATCH 01/18] add empty state and focus ring --- .../components/tags/index.css | 6 +++ packages/@react-aria/tag/src/useTagGroup.ts | 3 -- packages/@react-spectrum/tag/src/TagGroup.tsx | 47 +++++++++++-------- .../tag/stories/TagGroup.stories.tsx | 41 ++++++++++++++++ 4 files changed, 74 insertions(+), 23 deletions(-) diff --git a/packages/@adobe/spectrum-css-temp/components/tags/index.css b/packages/@adobe/spectrum-css-temp/components/tags/index.css index cdfad680be7..ef9f9ead036 100644 --- a/packages/@adobe/spectrum-css-temp/components/tags/index.css +++ b/packages/@adobe/spectrum-css-temp/components/tags/index.css @@ -18,6 +18,12 @@ governing permissions and limitations under the License. .spectrum-Tags { display: inline; + + &.focus-ring { + &:after { + box-shadow: 0 0 0 var(--spectrum-focus-ring-border-size) var(--spectrum-tag-border-color-key-focus); + } + } } .spectrum-Tags-container { diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index e1736f2744d..7fc4a706409 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -53,9 +53,6 @@ export function useTagGroup(props: AriaTagGroupProps, state: TagGroupState let {labelProps, fieldProps, descriptionProps, errorMessageProps} = useField(props); let {gridProps} = useGridList({...props, ...fieldProps, keyboardDelegate}, state, ref); - // Don't want the grid to be focusable or accessible via keyboard - delete gridProps.tabIndex; - let [isFocusWithin, setFocusWithin] = useState(false); let {focusWithinProps} = useFocusWithin({ onFocusWithinChange: setFocusWithin diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 16bafa257ef..c9af6e373a5 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -15,7 +15,7 @@ import {AriaTagGroupProps, TagKeyboardDelegate, useTagGroup} from '@react-aria/t import {classNames, useDOMRef} from '@react-spectrum/utils'; import {DOMRef, SpectrumHelpTextProps, SpectrumLabelableProps, StyleProps} from '@react-types/shared'; import {Field} from '@react-spectrum/label'; -import {FocusScope} from '@react-aria/focus'; +import {FocusRing, FocusScope} from '@react-aria/focus'; // @ts-ignore import intlMessages from '../intl/*.json'; import {ListCollection} from '@react-stately/list'; @@ -32,7 +32,9 @@ export interface SpectrumTagGroupProps extends AriaTagGroupProps, StylePro /** The label to display on the action button. */ actionLabel?: string, /** Handler that is called when the action button is pressed. */ - onAction?: () => void + onAction?: () => void, + /** Sets what the TagGroup should render when there are no tags to display. */ + renderEmptyState?: () => JSX.Element } function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef) { @@ -45,7 +47,8 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef children, actionLabel, onAction, - labelPosition + labelPosition, + renderEmptyState = () =>
None
} = props; let domRef = useDOMRef(ref); let containerRef = useRef(null); @@ -151,6 +154,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef }; let showActions = tagState.showCollapseButton || (actionLabel && onAction); + let isEmpty = state.collection.size === 0; return ( @@ -175,23 +179,26 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef style={maxRows != null && tagState.showCollapseButton && isCollapsed ? {maxHeight: tagState.maxHeight, overflow: 'hidden'} : undefined} ref={containerRef} className={classNames(styles, 'spectrum-Tags-container')}> -
- {visibleTags.map(item => ( - - {item.rendered} - - ))} -
- {showActions && + +
+ {visibleTags.map(item => ( + + {item.rendered} + + ))} + {isEmpty && renderEmptyState()} +
+
+ {showActions && !isEmpty &&
, + storyName: 'Empty state' +}; + +export const CustomEmptyState: TagGroupStory = { + render: (args) => , + storyName: 'Custom empty state' +}; + function OnRemoveExample(props) { let {withAvatar, ...otherProps} = props; let [items, setItems] = useState([ @@ -263,3 +274,33 @@ function OnRemoveExample(props) { ); } + +function EmptyStateExample(props) { + let {withCustomEmptyState, ...otherProps} = props; + let [items, setItems] = useState([ + {id: 1, label: 'Cool Tag 1'}, + {id: 2, label: 'Another cool tag'} + ]); + + let onRemove = (key) => { + setItems(prevItems => prevItems.filter((item) => key !== item.id)); + action('onRemove')(key); + }; + + let renderEmptyState: () => JSX.Element; + if (withCustomEmptyState) { + renderEmptyState = () => ( + No tags. Click here to add some. + ); + } + + return ( + onRemove(key)} {...otherProps}> + {(item: any) => ( + + {item.label} + + )} + + ); +} From b0a618de0bc467a2335a6a193e726cbae3d77833 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 4 Apr 2023 12:41:08 -0500 Subject: [PATCH 02/18] add tests --- .../@react-spectrum/tag/test/TagGroup.test.js | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index c1269d1223c..71b03d5826c 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -14,6 +14,7 @@ import {act, fireEvent, mockImplementation, render, triggerPress, within} from ' import {Button} from '@react-spectrum/button'; import {chain} from '@react-aria/utils'; import {Item} from '@react-stately/collections'; +import {Link} from '@react-spectrum/link'; import {Provider} from '@react-spectrum/provider'; import React from 'react'; import {TagGroup} from '../src'; @@ -642,4 +643,37 @@ describe('TagGroup', function () { computedStyles.mockReset(); }); + + + it('should render empty state', async function () { + let {getByText} = render( + + + {[]} + + + ); + await act(() => Promise.resolve()); // wait for MutationObserver in useHasTabbableChild or we get act warnings + expect(getByText('None')).toBeTruthy(); + }); + + it('should allow you to tab into TagGroup body if empty with link', async function () { + let computedStyles = jest.spyOn(window, 'getComputedStyle').mockImplementation(() => ({marginRight: '4px', marginTop: '4px', height: '24px'})); + + let renderEmptyState = () => ( + No tags. Click here to add some. + ); + let {getByRole} = render( + + + {[]} + + + ); + await act(() => Promise.resolve()); + let link = getByRole('link'); + userEvent.tab(); + expect(document.activeElement).toBe(link); + computedStyles.mockReset(); + }); }); From 3c9bbd03ee7d67bee5d4f23aeac1f5c02d8df769 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 4 Apr 2023 12:41:35 -0500 Subject: [PATCH 03/18] copy --- 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 71b03d5826c..ae755fb5a13 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -657,7 +657,7 @@ describe('TagGroup', function () { expect(getByText('None')).toBeTruthy(); }); - it('should allow you to tab into TagGroup body if empty with link', async function () { + it('should allow you to tab into TagGroup if empty with link', async function () { let computedStyles = jest.spyOn(window, 'getComputedStyle').mockImplementation(() => ({marginRight: '4px', marginTop: '4px', height: '24px'})); let renderEmptyState = () => ( From a653e037b616ac9acae34906fcc54edfd243dd8a Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 10 Apr 2023 12:33:48 -0500 Subject: [PATCH 04/18] remove container negative margin if empty --- .../@adobe/spectrum-css-temp/components/tags/index.css | 8 +++++--- packages/@react-spectrum/tag/src/TagGroup.tsx | 10 +++++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/@adobe/spectrum-css-temp/components/tags/index.css b/packages/@adobe/spectrum-css-temp/components/tags/index.css index ef9f9ead036..46e1f87275a 100644 --- a/packages/@adobe/spectrum-css-temp/components/tags/index.css +++ b/packages/@adobe/spectrum-css-temp/components/tags/index.css @@ -27,9 +27,11 @@ governing permissions and limitations under the License. } .spectrum-Tags-container { - /* Aligns tags with label. */ - margin-inline-start: calc(calc(var(--spectrum-taggroup-tag-gap-x) / 2) * -1); - margin-inline-end: calc(var(--spectrum-taggroup-tag-gap-x) / 2); + &:not(.spectrum-Tags-container--empty) { + /* Aligns tags with label. */ + margin-inline-start: calc(calc(var(--spectrum-taggroup-tag-gap-x) / 2) * -1); + margin-inline-end: calc(var(--spectrum-taggroup-tag-gap-x) / 2); + } } .spectrum-Tag { diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index c9af6e373a5..0a1e8c8af35 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -178,7 +178,15 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef
+ className={ + classNames( + styles, + 'spectrum-Tags-container', + { + 'spectrum-Tags-container--empty': isEmpty + } + ) + }>
Date: Mon, 10 Apr 2023 14:18:47 -0500 Subject: [PATCH 05/18] fix styles for focus ring --- packages/@adobe/spectrum-css-temp/components/tags/index.css | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/@adobe/spectrum-css-temp/components/tags/index.css b/packages/@adobe/spectrum-css-temp/components/tags/index.css index 46e1f87275a..356e0f90115 100644 --- a/packages/@adobe/spectrum-css-temp/components/tags/index.css +++ b/packages/@adobe/spectrum-css-temp/components/tags/index.css @@ -20,9 +20,9 @@ governing permissions and limitations under the License. display: inline; &.focus-ring { - &:after { - box-shadow: 0 0 0 var(--spectrum-focus-ring-border-size) var(--spectrum-tag-border-color-key-focus); - } + outline: none; + box-shadow: 0 0 0 2px var(--spectrum-tag-border-color-key-focus); + display: block; } } From 67c75315871f07ab2e02bacd572fc2d9bea3afbe Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 10 Apr 2023 17:25:00 -0500 Subject: [PATCH 06/18] update stories --- .../tag/stories/TagGroup.stories.tsx | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx index 6d59fae348b..63a5eaec276 100644 --- a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx +++ b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx @@ -277,15 +277,6 @@ function OnRemoveExample(props) { function EmptyStateExample(props) { let {withCustomEmptyState, ...otherProps} = props; - let [items, setItems] = useState([ - {id: 1, label: 'Cool Tag 1'}, - {id: 2, label: 'Another cool tag'} - ]); - - let onRemove = (key) => { - setItems(prevItems => prevItems.filter((item) => key !== item.id)); - action('onRemove')(key); - }; let renderEmptyState: () => JSX.Element; if (withCustomEmptyState) { @@ -295,12 +286,11 @@ function EmptyStateExample(props) { } return ( - onRemove(key)} {...otherProps}> - {(item: any) => ( - - {item.label} - - )} + + {[]} ); } From b5040bb93ba51d21e4162aa9888069da32d50039 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 10 Apr 2023 17:29:34 -0500 Subject: [PATCH 07/18] improve stories --- .../tag/stories/TagGroup.stories.tsx | 31 ++++++------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx index 63a5eaec276..6733789341a 100644 --- a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx +++ b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx @@ -238,12 +238,19 @@ export const WithLabelDescriptionContextualHelpAndAction: TagGroupStory = { }; export const EmptyState: TagGroupStory = { - render: (args) => , + render: (args) => ( + + [] + + ), storyName: 'Empty state' }; export const CustomEmptyState: TagGroupStory = { - render: (args) => , + ...EmptyState, + args: { + renderEmptyState: () => No tags. Click here to add some. + }, storyName: 'Custom empty state' }; @@ -274,23 +281,3 @@ function OnRemoveExample(props) { ); } - -function EmptyStateExample(props) { - let {withCustomEmptyState, ...otherProps} = props; - - let renderEmptyState: () => JSX.Element; - if (withCustomEmptyState) { - renderEmptyState = () => ( - No tags. Click here to add some. - ); - } - - return ( - - {[]} - - ); -} From bc0cac28531605c724a3380211f86efe8aab3f72 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 10 Apr 2023 17:31:23 -0500 Subject: [PATCH 08/18] fix story --- packages/@react-spectrum/tag/stories/TagGroup.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx index 6733789341a..d07da671740 100644 --- a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx +++ b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx @@ -240,7 +240,7 @@ export const WithLabelDescriptionContextualHelpAndAction: TagGroupStory = { export const EmptyState: TagGroupStory = { render: (args) => ( - [] + {[]} ), storyName: 'Empty state' From df7ccddb4f83c35d247da2cd01ee078f79c7911f Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 10 Apr 2023 17:31:56 -0500 Subject: [PATCH 09/18] add chromatic stories --- .../tag/chromatic/TagGroup.chromatic.tsx | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/@react-spectrum/tag/chromatic/TagGroup.chromatic.tsx b/packages/@react-spectrum/tag/chromatic/TagGroup.chromatic.tsx index c771e21a80e..83ead7762e6 100644 --- a/packages/@react-spectrum/tag/chromatic/TagGroup.chromatic.tsx +++ b/packages/@react-spectrum/tag/chromatic/TagGroup.chromatic.tsx @@ -13,6 +13,7 @@ import Audio from '@spectrum-icons/workflow/Audio'; import {ComponentMeta, ComponentStoryObj} from '@storybook/react'; import {Item, TagGroup} from '../'; +import {Link} from '@react-spectrum/link'; import React from 'react'; import {Text} from '@react-spectrum/text'; @@ -124,3 +125,19 @@ export const MaxRowsCustomAction: TagGroupStory = { ] }; +export const EmptyState: TagGroupStory = { + render: (args) => ( + + {[]} + + ), + storyName: 'Empty state' +}; + +export const CustomEmptyState: TagGroupStory = { + ...EmptyState, + args: { + renderEmptyState: () => No tags. Click here to add some. + }, + storyName: 'Custom empty state' +}; From cdde4066a28f583e569e32de5061cd6916204f87 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 10 Apr 2023 17:38:02 -0500 Subject: [PATCH 10/18] use translated string for None --- packages/@react-spectrum/tag/intl/en-US.json | 3 ++- packages/@react-spectrum/tag/src/TagGroup.tsx | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/@react-spectrum/tag/intl/en-US.json b/packages/@react-spectrum/tag/intl/en-US.json index 4c03c6aaf71..e078154e56d 100644 --- a/packages/@react-spectrum/tag/intl/en-US.json +++ b/packages/@react-spectrum/tag/intl/en-US.json @@ -1,5 +1,6 @@ { "showAllButtonLabel": "Show all ({tagCount, number})", "hideButtonLabel": "Show less", - "actions": "Actions" + "actions": "Actions", + "noTags": "None" } diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 0a1e8c8af35..d7a1d44ceb4 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -48,7 +48,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef actionLabel, onAction, labelPosition, - renderEmptyState = () =>
None
+ renderEmptyState } = props; let domRef = useDOMRef(ref); let containerRef = useRef(null); @@ -67,6 +67,9 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef delete props.onAction; let {gridProps, labelProps, descriptionProps, errorMessageProps} = useTagGroup({...props, keyboardDelegate}, state, tagsRef); let actionsId = useId(); + if (!renderEmptyState) { + renderEmptyState = () =>
{stringFormatter.format('noTags')}
; + } let updateVisibleTagCount = useCallback(() => { if (maxRows > 0) { From 2b3c0d3b27dbaf22b13aa0781e4b0c2a2ec6eb59 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 12 Apr 2023 18:00:23 -0500 Subject: [PATCH 11/18] move default to prop destructuring --- packages/@react-spectrum/tag/src/TagGroup.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 4154e697657..4c27c4a0514 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -47,7 +47,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef actionLabel, onAction, labelPosition, - renderEmptyState + renderEmptyState = () =>
{stringFormatter.format('noTags')}
} = props; let domRef = useDOMRef(ref); let containerRef = useRef(null); @@ -66,9 +66,6 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef delete props.onAction; let {gridProps, labelProps, descriptionProps, errorMessageProps} = useTagGroup({...props, keyboardDelegate}, state, tagsRef); let actionsId = useId(); - if (!renderEmptyState) { - renderEmptyState = () =>
{stringFormatter.format('noTags')}
; - } let updateVisibleTagCount = useCallback(() => { if (maxRows > 0) { From 9f1abf9a2679f924c43604da0be3ce5740cc743f Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 12 Apr 2023 18:00:47 -0500 Subject: [PATCH 12/18] fix focus-visible style and add comment --- packages/@adobe/spectrum-css-temp/components/tags/index.css | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/@adobe/spectrum-css-temp/components/tags/index.css b/packages/@adobe/spectrum-css-temp/components/tags/index.css index 356e0f90115..b4471f213c0 100644 --- a/packages/@adobe/spectrum-css-temp/components/tags/index.css +++ b/packages/@adobe/spectrum-css-temp/components/tags/index.css @@ -19,9 +19,14 @@ governing permissions and limitations under the License. .spectrum-Tags { display: inline; + &:focus-visible { + outline: none; + } + &.focus-ring { outline: none; box-shadow: 0 0 0 2px var(--spectrum-tag-border-color-key-focus); + /* Allows us to use a box-shadow for the focus ring. Should not cause layout shifts since it is within a container. */ display: block; } } From 588f4108dd6cf8a46890b055b63a098dd47c4b53 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 24 Apr 2023 17:34:05 -0500 Subject: [PATCH 13/18] add min-height to empty state --- .../@adobe/spectrum-css-temp/components/tags/index.css | 4 ++++ packages/@react-spectrum/tag/src/TagGroup.tsx | 8 ++++++-- 2 files changed, 10 insertions(+), 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 b4471f213c0..b7468f62777 100644 --- a/packages/@adobe/spectrum-css-temp/components/tags/index.css +++ b/packages/@adobe/spectrum-css-temp/components/tags/index.css @@ -139,3 +139,7 @@ governing permissions and limitations under the License. display: grid; } } + +.spectrum-Tags-empty-state { + min-height: var(--spectrum-tag-height); +} diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 4c27c4a0514..6475d4a0f7f 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -47,7 +47,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef actionLabel, onAction, labelPosition, - renderEmptyState = () =>
{stringFormatter.format('noTags')}
+ renderEmptyState = () => stringFormatter.format('noTags') } = props; let domRef = useDOMRef(ref); let containerRef = useRef(null); @@ -202,7 +202,11 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef {item.rendered} ))} - {isEmpty && renderEmptyState()} + {isEmpty && ( +
+ {renderEmptyState()} +
+ )}
{showActions && !isEmpty && From ab55328e5cf04c23fdaa63e3516485adb27ffd27 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 25 Apr 2023 10:54:27 -0500 Subject: [PATCH 14/18] handle error on removing all tags with maxRows --- packages/@react-spectrum/tag/src/TagGroup.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 6475d4a0f7f..a1abc9ab1a4 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -79,6 +79,13 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef let tags = [...currTagsRef.children]; let buttons = [...currContainerRef.parentElement.querySelectorAll('button')]; + if (tags.length === 0 || buttons.length === 0) { + return { + visibleTagCount: 0, + showCollapseButton: false, + maxHeight: undefined + }; + } let currY = -Infinity; let rowCount = 0; let index = 0; From e9e1ed1bce2c83a662eb7d48b939973ea542fd4b Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 5 May 2023 17:06:23 -0500 Subject: [PATCH 15/18] focus container after last tag removed --- packages/@react-aria/tag/src/useTag.ts | 13 ++++- .../@react-spectrum/tag/test/TagGroup.test.js | 52 +++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/packages/@react-aria/tag/src/useTag.ts b/packages/@react-aria/tag/src/useTag.ts index b7f7806147a..1b8da7b9e18 100644 --- a/packages/@react-aria/tag/src/useTag.ts +++ b/packages/@react-aria/tag/src/useTag.ts @@ -63,9 +63,20 @@ export function useTag(props: AriaTagProps, state: ListState, ref: RefO if (e.key === 'Delete' || e.key === 'Backspace') { onRemove(item.key); e.preventDefault(); + + // Focus container if last tag was removed. + if (state.collection.size === 1 && state.collection.getFirstKey() === item.key) { + let lastTag = ref; + let container = ref.current.parentElement; + setTimeout(() => { + if (!lastTag.current) { + container.focus(); + } + }, 50); + } } }; - + let modality: string = useInteractionModality(); if (modality === 'virtual' && (typeof window !== 'undefined' && 'ontouchstart' in window)) { modality = 'touch'; diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index ae755fb5a13..76455bc5c30 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -419,6 +419,58 @@ describe('TagGroup', function () { expect(document.activeElement).toBe(tags[1]); }); + it.each` + Name | props + ${'on `Delete` keypress'} | ${{keyPress: 'Delete'}} + ${'on `Backspace` keypress'} | ${{keyPress: 'Backspace'}} + `('Should focus container after last tag is removed $Name', function ({Name, props}) { + + function TagGroupWithDelete(props) { + let [items, setItems] = React.useState([ + {id: 1, label: 'Cool Tag 1'}, + {id: 2, label: 'Another cool tag'} + ]); + + let removeItem = (key) => { + setItems(prevItems => prevItems.filter((item) => key !== item.id)); + }; + + return ( + + + {item => {item.label}} + + + ); + } + + let {getAllByRole, getByRole, queryAllByRole} = render( + + ); + + let tags = getAllByRole('row'); + let container = getByRole('grid'); + 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'); + expect(document.activeElement).toBe(tags[0]); + fireEvent.keyDown(document.activeElement, {key: props.keyPress}); + fireEvent.keyUp(document.activeElement, {key: props.keyPress}); + expect(onRemoveSpy).toHaveBeenCalledTimes(2); + expect(onRemoveSpy).toHaveBeenCalledWith(2); + + act(() => jest.runAllTimers()); + + tags = queryAllByRole('row'); + expect(tags.length).toBe(0); + expect(document.activeElement).toBe(container); + }); + it('maxRows should limit the number of tags shown', function () { let offsetWidth = jest.spyOn(HTMLElement.prototype, 'getBoundingClientRect') .mockImplementationOnce(() => ({x: 200, y: 300, width: 75, height: 32, top: 300, right: 275, bottom: 335, left: 200})) From 8eb60a3d98195ba9bf413011491f60ecbfc5617e Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 5 May 2023 17:19:11 -0500 Subject: [PATCH 16/18] add ar-AE string for chromatic --- packages/@react-spectrum/tag/intl/ar-AE.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tag/intl/ar-AE.json b/packages/@react-spectrum/tag/intl/ar-AE.json index 85eff99f69b..6939cff6c6d 100644 --- a/packages/@react-spectrum/tag/intl/ar-AE.json +++ b/packages/@react-spectrum/tag/intl/ar-AE.json @@ -1,5 +1,6 @@ { "actions": "الإجراءات", "hideButtonLabel": "إظهار أقل", - "showAllButtonLabel": "عرض الكل ({tagCount, number})" + "showAllButtonLabel": "عرض الكل ({tagCount, number})", + "noTags": "None" } From 3a3eb1950f72cd0e8c618a560c149622f2912dc7 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 8 May 2023 10:39:25 -0500 Subject: [PATCH 17/18] switch to useEffect --- packages/@react-aria/tag/src/useTag.ts | 11 ----------- packages/@react-aria/tag/src/useTagGroup.ts | 11 ++++++++++- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/@react-aria/tag/src/useTag.ts b/packages/@react-aria/tag/src/useTag.ts index 1b8da7b9e18..4371bb60e4b 100644 --- a/packages/@react-aria/tag/src/useTag.ts +++ b/packages/@react-aria/tag/src/useTag.ts @@ -63,17 +63,6 @@ export function useTag(props: AriaTagProps, state: ListState, ref: RefO if (e.key === 'Delete' || e.key === 'Backspace') { onRemove(item.key); e.preventDefault(); - - // Focus container if last tag was removed. - if (state.collection.size === 1 && state.collection.getFirstKey() === item.key) { - let lastTag = ref; - let container = ref.current.parentElement; - setTimeout(() => { - if (!lastTag.current) { - container.focus(); - } - }, 50); - } } }; diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index 428284a8dcc..481a2dab495 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -13,7 +13,7 @@ import {AriaLabelingProps, DOMAttributes, DOMProps, Validation} from '@react-types/shared'; import {filterDOMProps, mergeProps} from '@react-aria/utils'; import type {ListState} from '@react-stately/list'; -import {RefObject, useState} from 'react'; +import {RefObject, useEffect, useRef, useState} from 'react'; import {TagGroupProps} from '@react-types/tag'; import {TagKeyboardDelegate} from './TagKeyboardDelegate'; import {useField} from '@react-aria/label'; @@ -58,6 +58,15 @@ export function useTagGroup(props: AriaTagGroupProps, state: ListState, onFocusWithinChange: setFocusWithin }); let domProps = filterDOMProps(props); + + // If the last tag is removed, focus the container. + let prevCount = useRef(state.collection.size); + useEffect(() => { + if (prevCount.current > 0 && state.collection.size === 0 && isFocusWithin) { + ref.current.focus(); + } + }, [state.collection.size, isFocusWithin, ref]); + return { gridProps: mergeProps(gridProps, domProps, { role: state.collection.size ? 'grid' : null, From 38cd4ef13a72761248347b136ef35362e7d6133d Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 8 May 2023 16:43:13 -0500 Subject: [PATCH 18/18] update prevCount --- packages/@react-aria/tag/src/useTagGroup.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index 481a2dab495..be565052e37 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -65,6 +65,7 @@ export function useTagGroup(props: AriaTagGroupProps, state: ListState, if (prevCount.current > 0 && state.collection.size === 0 && isFocusWithin) { ref.current.focus(); } + prevCount.current = state.collection.size; }, [state.collection.size, isFocusWithin, ref]); return {