Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
aad63c3
add empty state and focus ring
reidbarber Apr 4, 2023
b0a618d
add tests
reidbarber Apr 4, 2023
3c9bbd0
copy
reidbarber Apr 4, 2023
a653e03
remove container negative margin if empty
reidbarber Apr 10, 2023
29ffb19
fix styles for focus ring
reidbarber Apr 10, 2023
67c7531
update stories
reidbarber Apr 10, 2023
b5040bb
improve stories
reidbarber Apr 10, 2023
bc0cac2
fix story
reidbarber Apr 10, 2023
df7ccdd
add chromatic stories
reidbarber Apr 10, 2023
cdde406
use translated string for None
reidbarber Apr 10, 2023
342c934
Merge branch 'main' into taggroup-empty-state
reidbarber Apr 12, 2023
2b3c0d3
move default to prop destructuring
reidbarber Apr 12, 2023
9f1abf9
fix focus-visible style and add comment
reidbarber Apr 12, 2023
c4ee85d
Merge branch 'main' into taggroup-empty-state
snowystinger Apr 13, 2023
22acd60
Merge branch 'main' into taggroup-empty-state
LFDanLu Apr 14, 2023
588f410
add min-height to empty state
reidbarber Apr 24, 2023
ab55328
handle error on removing all tags with maxRows
reidbarber Apr 25, 2023
81f5a99
Merge branch 'main' into taggroup-empty-state
reidbarber May 5, 2023
e9e1ed1
focus container after last tag removed
reidbarber May 5, 2023
8eb60a3
add ar-AE string for chromatic
reidbarber May 5, 2023
3a3eb19
switch to useEffect
reidbarber May 8, 2023
2fe6eb0
Merge branch 'main' into taggroup-empty-state
reidbarber May 8, 2023
3164f7f
Merge branch 'main' into taggroup-empty-state
snowystinger May 8, 2023
e6d7ca8
Merge branch 'main' into taggroup-empty-state
LFDanLu May 8, 2023
72566a4
Merge branch 'main' into taggroup-empty-state
reidbarber May 8, 2023
0786ddb
Merge branch 'taggroup-empty-state' of https://github.com/adobe/react…
reidbarber May 8, 2023
38cd4ef
update prevCount
reidbarber May 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions packages/@adobe/spectrum-css-temp/components/tags/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,25 @@ 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;
Comment thread
reidbarber marked this conversation as resolved.
}
}

.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 {
Expand Down Expand Up @@ -126,3 +139,7 @@ governing permissions and limitations under the License.
display: grid;
}
}

.spectrum-Tags-empty-state {
min-height: var(--spectrum-tag-height);
}
2 changes: 1 addition & 1 deletion packages/@react-aria/tag/src/useTag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export function useTag<T>(props: AriaTagProps<T>, state: ListState<T>, ref: RefO
e.preventDefault();
}
};

let modality: string = useInteractionModality();
if (modality === 'virtual' && (typeof window !== 'undefined' && 'ontouchstart' in window)) {
modality = 'touch';
Expand Down
15 changes: 11 additions & 4 deletions packages/@react-aria/tag/src/useTagGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -53,14 +53,21 @@ export function useTagGroup<T>(props: AriaTagGroupProps<T>, state: ListState<T>,
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
});
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();
}
prevCount.current = state.collection.size;
}, [state.collection.size, isFocusWithin, ref]);

return {
gridProps: mergeProps(gridProps, domProps, {
role: state.collection.size ? 'grid' : null,
Expand Down
17 changes: 17 additions & 0 deletions packages/@react-spectrum/tag/chromatic/TagGroup.chromatic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -124,3 +125,19 @@ export const MaxRowsCustomAction: TagGroupStory = {
]
};

export const EmptyState: TagGroupStory = {
render: (args) => (
<TagGroup label="Tag group with empty state" {...args}>
{[]}
</TagGroup>
),
storyName: 'Empty state'
};

export const CustomEmptyState: TagGroupStory = {
...EmptyState,
args: {
renderEmptyState: () => <span>No tags. <Link><a href="//react-spectrum.com">Click here</a></Link> to add some.</span>
},
storyName: 'Custom empty state'
};
3 changes: 2 additions & 1 deletion packages/@react-spectrum/tag/intl/ar-AE.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"actions": "الإجراءات",
"hideButtonLabel": "إظهار أقل",
"showAllButtonLabel": "عرض الكل ({tagCount, number})"
"showAllButtonLabel": "عرض الكل ({tagCount, number})",
"noTags": "None"
}
3 changes: 2 additions & 1 deletion packages/@react-spectrum/tag/intl/en-US.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"showAllButtonLabel": "Show all ({tagCount, number})",
"hideButtonLabel": "Show less",
"actions": "Actions"
"actions": "Actions",
"noTags": "None"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran chromatic and it broke cuz it doesn't have ar-AE translations, wanna put a dummy string for noTags for ar-AE for now?

}
68 changes: 47 additions & 21 deletions packages/@react-spectrum/tag/src/TagGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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, useListState} from '@react-stately/list';
Expand All @@ -31,7 +31,9 @@ export interface SpectrumTagGroupProps<T> extends Omit<AriaTagGroupProps<T>, 'ke
/** 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<T extends object>(props: SpectrumTagGroupProps<T>, ref: DOMRef<HTMLDivElement>) {
Expand All @@ -44,7 +46,8 @@ function TagGroup<T extends object>(props: SpectrumTagGroupProps<T>, ref: DOMRef
children,
actionLabel,
onAction,
labelPosition
labelPosition,
renderEmptyState = () => stringFormatter.format('noTags')
} = props;
let domRef = useDOMRef(ref);
let containerRef = useRef(null);
Expand Down Expand Up @@ -76,6 +79,13 @@ function TagGroup<T extends object>(props: SpectrumTagGroupProps<T>, 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;
Expand Down Expand Up @@ -150,6 +160,7 @@ function TagGroup<T extends object>(props: SpectrumTagGroupProps<T>, ref: DOMRef
};

let showActions = tagState.showCollapseButton || (actionLabel && onAction);
let isEmpty = state.collection.size === 0;

return (
<FocusScope>
Expand All @@ -173,24 +184,39 @@ function TagGroup<T extends object>(props: SpectrumTagGroupProps<T>, ref: DOMRef
<div
style={maxRows != null && tagState.showCollapseButton && isCollapsed ? {maxHeight: tagState.maxHeight, overflow: 'hidden'} : undefined}
ref={containerRef}
className={classNames(styles, 'spectrum-Tags-container')}>
<div
ref={tagsRef}
{...gridProps}
className={classNames(styles, 'spectrum-Tags')}>
{visibleTags.map(item => (
<Tag
{...item.props}
key={item.key}
item={item}
state={state}
allowsRemoving={allowsRemoving}
onRemove={onRemove}>
{item.rendered}
</Tag>
))}
</div>
{showActions &&
className={
classNames(
styles,
'spectrum-Tags-container',
{
'spectrum-Tags-container--empty': isEmpty
}
)
}>
<FocusRing focusRingClass={classNames(styles, 'focus-ring')}>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure the FocusRing makes sense when ShowAll hides all items. The hiding all items in this story is an existing bug. Would this go away if we fix that bug and always show one tag?

<div
ref={tagsRef}
{...gridProps}
className={classNames(styles, 'spectrum-Tags')}>
{visibleTags.map(item => (
<Tag
{...item.props}
key={item.key}
item={item}
state={state}
allowsRemoving={allowsRemoving}
onRemove={onRemove}>
{item.rendered}
</Tag>
))}
{isEmpty && (
<div className={classNames(styles, 'spectrum-Tags-empty-state')}>
{renderEmptyState()}
</div>
)}
</div>
</FocusRing>
{showActions && !isEmpty &&
<Provider isDisabled={false}>
<div
role="group"
Expand Down
18 changes: 18 additions & 0 deletions packages/@react-spectrum/tag/stories/TagGroup.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {Content} from '@react-spectrum/view';
import {ContextualHelp} from '@react-spectrum/contextualhelp';
import {Heading, Text} from '@react-spectrum/text';
import {Item, SpectrumTagGroupProps, TagGroup} from '../src';
import {Link} from '@react-spectrum/link';
import React, {useState} from 'react';

let manyItems = [];
Expand Down Expand Up @@ -236,6 +237,23 @@ export const WithLabelDescriptionContextualHelpAndAction: TagGroupStory = {
name: 'with label, description, contextual help + action'
};

export const EmptyState: TagGroupStory = {
render: (args) => (
<TagGroup label="Tag group with empty state" {...args}>
{[]}
</TagGroup>
),
storyName: 'Empty state'
};

export const CustomEmptyState: TagGroupStory = {
...EmptyState,
args: {
renderEmptyState: () => <span>No tags. <Link><a href="//react-spectrum.com">Click here</a></Link> to add some.</span>
},
storyName: 'Custom empty state'
};

function OnRemoveExample(props) {
let {withAvatar, ...otherProps} = props;
let [items, setItems] = useState([
Expand Down
86 changes: 86 additions & 0 deletions packages/@react-spectrum/tag/test/TagGroup.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -418,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 (
<Provider theme={theme}>
<TagGroup items={items} aria-label="tag group" allowsRemoving onRemove={chain(removeItem, onRemoveSpy)} {...props}>
{item => <Item>{item.label}</Item>}
</TagGroup>
</Provider>
);
}

let {getAllByRole, getByRole, queryAllByRole} = render(
<TagGroupWithDelete {...props} />
);

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}))
Expand Down Expand Up @@ -642,4 +695,37 @@ describe('TagGroup', function () {

computedStyles.mockReset();
});


it('should render empty state', async function () {
let {getByText} = render(
<Provider theme={theme}>
<TagGroup aria-label="tag group">
{[]}
</TagGroup>
</Provider>
);
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 if empty with link', async function () {
let computedStyles = jest.spyOn(window, 'getComputedStyle').mockImplementation(() => ({marginRight: '4px', marginTop: '4px', height: '24px'}));

let renderEmptyState = () => (
<span>No tags. <Link><a href="//react-spectrum.com">Click here</a></Link> to add some.</span>
);
let {getByRole} = render(
<Provider theme={theme}>
<TagGroup aria-label="tag group" renderEmptyState={renderEmptyState}>
{[]}
</TagGroup>
</Provider>
);
await act(() => Promise.resolve());
let link = getByRole('link');
userEvent.tab();
expect(document.activeElement).toBe(link);
computedStyles.mockReset();
});
});