From 1a39b79dfa2308beb480f6c6ef757cc3581768b3 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 16 Nov 2022 14:07:37 -0600 Subject: [PATCH 01/42] add defaultVisibleRows and initial functionality --- packages/@react-spectrum/tag/src/TagGroup.tsx | 53 +++++++++++++++++-- .../tag/stories/TagGroup.stories.tsx | 16 +++++- packages/@react-types/tag/src/index.d.ts | 4 +- 3 files changed, 65 insertions(+), 8 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 29b6df9ee21..3d4f5934bf4 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -10,11 +10,12 @@ * governing permissions and limitations under the License. */ +import {Button} from '@react-spectrum/button'; 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 {mergeProps, useResizeObserver} from '@react-aria/utils'; +import React, {ReactElement, useCallback, useEffect, useMemo, useState} from 'react'; import {SpectrumTagGroupProps} from '@react-types/tag'; import styles from '@adobe/spectrum-css-temp/components/tags/vars.css'; import {Tag} from './Tag'; @@ -24,12 +25,12 @@ import {useListState} from '@react-stately/list'; import {useLocale} from '@react-aria/i18n'; import {useProviderProps} from '@react-spectrum/provider'; - function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef) { props = useProviderProps(props); let { allowsRemoving, onRemove, + defaultVisibleRows, ...otherProps } = props; let domRef = useDOMRef(ref); @@ -70,6 +71,43 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef }, state, domRef); const {tagGroupProps} = useTagGroup(props); + let [visibleTagCount, setVisibleTagCount] = useState(gridCollection.size); + let [isCollapsed, setIsCollapsed] = useState(defaultVisibleRows != null); + + let checkVisibleTagCount = useCallback(() => { + if (defaultVisibleRows != null && isCollapsed) { + let currY = -Infinity; + let rowCount = 0; + let index = 0; + let items = [...domRef.current.children]; + for (let item of items) { + let {y} = item.getBoundingClientRect(); + + if (y !== currY) { + currY = y; + rowCount++; + } + + if (rowCount > defaultVisibleRows) { + break; + } + index++; + } + setVisibleTagCount(index); + } + }, [defaultVisibleRows, domRef, isCollapsed]); + + useEffect(() => { + checkVisibleTagCount(); + }, [props.children, checkVisibleTagCount]); + + useResizeObserver({ref: domRef, onResize: checkVisibleTagCount}); + + let visibleTags = [...gridCollection]; + if (defaultVisibleRows != null && isCollapsed) { + visibleTags = visibleTags.slice(0, visibleTagCount); + } + // 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; @@ -85,7 +123,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef } role={state.collection.size ? 'grid' : null} ref={domRef}> - {[...gridCollection].map(item => ( + {visibleTags.map(item => ( (props: SpectrumTagGroupProps, ref: DOMRef onRemove={onRemove}> {item.childNodes[0].rendered} - ))} + ))} + {defaultVisibleRows != null && visibleTagCount < gridCollection.size && + + } ); } diff --git a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx index 6974b152867..205159d4bac 100644 --- a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx +++ b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx @@ -60,7 +60,7 @@ storiesOf('TagGroup', module) } ) .add('wrapping', () => ( -
+
Cool Tag 1 Another cool tag @@ -91,7 +91,19 @@ storiesOf('TagGroup', module) } ) - ); + ) + .add('defaultVisibleRows: 2', () => ( +
+ + Cool Tag 1 + Another cool tag + This tag + What tag? + This tag is cool too + Shy tag + +
+ )); function render(props: any = {}) { return ( diff --git a/packages/@react-types/tag/src/index.d.ts b/packages/@react-types/tag/src/index.d.ts index 8c6aef4ac68..4c99e1ea998 100644 --- a/packages/@react-types/tag/src/index.d.ts +++ b/packages/@react-types/tag/src/index.d.ts @@ -18,7 +18,9 @@ export interface TagGroupProps extends Omit, 'disabledKeys' /** Whether the TagGroup allows removal of tags. */ allowsRemoving?: boolean, /** Called when the user removes a tag. */ - onRemove?: (key: Key) => void + onRemove?: (key: Key) => void, + /** Limit the number of rows initially shown. This will render a button that allows the user to expand to show all tags. */ + defaultVisibleRows?: number } export interface SpectrumTagGroupProps extends TagGroupProps, DOMProps, StyleProps, AriaLabelingProps {} From aba67e9bf6554d52c00a411baef42bde87c88ee3 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 16 Nov 2022 14:07:47 -0600 Subject: [PATCH 02/42] cleanup --- 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 31eeeb1796519099714d35bdf3cd07503475dd63 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 16 Nov 2022 15:05:02 -0600 Subject: [PATCH 03/42] rename to maxRows --- packages/@react-spectrum/tag/src/TagGroup.tsx | 14 +++++++------- .../tag/stories/TagGroup.stories.tsx | 4 ++-- packages/@react-types/tag/src/index.d.ts | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 3d4f5934bf4..7da6ed1d3b4 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -30,7 +30,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef let { allowsRemoving, onRemove, - defaultVisibleRows, + maxRows, ...otherProps } = props; let domRef = useDOMRef(ref); @@ -72,10 +72,10 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef const {tagGroupProps} = useTagGroup(props); let [visibleTagCount, setVisibleTagCount] = useState(gridCollection.size); - let [isCollapsed, setIsCollapsed] = useState(defaultVisibleRows != null); + let [isCollapsed, setIsCollapsed] = useState(maxRows != null); let checkVisibleTagCount = useCallback(() => { - if (defaultVisibleRows != null && isCollapsed) { + if (maxRows != null && isCollapsed) { let currY = -Infinity; let rowCount = 0; let index = 0; @@ -88,14 +88,14 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef rowCount++; } - if (rowCount > defaultVisibleRows) { + if (rowCount > maxRows) { break; } index++; } setVisibleTagCount(index); } - }, [defaultVisibleRows, domRef, isCollapsed]); + }, [maxRows, domRef, isCollapsed]); useEffect(() => { checkVisibleTagCount(); @@ -104,7 +104,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef useResizeObserver({ref: domRef, onResize: checkVisibleTagCount}); let visibleTags = [...gridCollection]; - if (defaultVisibleRows != null && isCollapsed) { + if (maxRows != null && isCollapsed) { visibleTags = visibleTags.slice(0, visibleTagCount); } @@ -134,7 +134,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef {item.childNodes[0].rendered} ))} - {defaultVisibleRows != null && visibleTagCount < gridCollection.size && + {maxRows != null && visibleTagCount < gridCollection.size && diff --git a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx index 205159d4bac..ca46d6b928f 100644 --- a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx +++ b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx @@ -92,9 +92,9 @@ storiesOf('TagGroup', module) ) ) - .add('defaultVisibleRows: 2', () => ( + .add('maxRows: 2', () => (
- + Cool Tag 1 Another cool tag This tag diff --git a/packages/@react-types/tag/src/index.d.ts b/packages/@react-types/tag/src/index.d.ts index 4c99e1ea998..02755c30e88 100644 --- a/packages/@react-types/tag/src/index.d.ts +++ b/packages/@react-types/tag/src/index.d.ts @@ -20,7 +20,7 @@ export interface TagGroupProps extends Omit, 'disabledKeys' /** Called when the user removes a tag. */ onRemove?: (key: Key) => void, /** Limit the number of rows initially shown. This will render a button that allows the user to expand to show all tags. */ - defaultVisibleRows?: number + maxRows?: number } export interface SpectrumTagGroupProps extends TagGroupProps, DOMProps, StyleProps, AriaLabelingProps {} From 2937e02f8127cf5b73e86d6f1a42b963a5b7898e Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 16 Nov 2022 17:30:09 -0600 Subject: [PATCH 04/42] switch to use quiet ActionButton --- packages/@react-spectrum/tag/src/TagGroup.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 7da6ed1d3b4..de146f06520 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {Button} from '@react-spectrum/button'; +import {ActionButton} from '@react-spectrum/button'; import {classNames, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {DOMRef} from '@react-types/shared'; import {GridCollection, useGridState} from '@react-stately/grid'; @@ -135,9 +135,9 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef ))} {maxRows != null && visibleTagCount < gridCollection.size && - + }
); From c93a1456de7b2e6bce71a9d74a554bc5f5650636 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 17 Nov 2022 10:45:50 -0600 Subject: [PATCH 05/42] include collapse button on last row --- packages/@react-spectrum/tag/src/TagGroup.tsx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index de146f06520..631244018c3 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -93,9 +93,21 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef } index++; } + + // Hide tags on last row until there is room for the collapse button. + let end = direction === 'ltr' ? 'right' : 'left'; + let containerEnd = domRef.current.getBoundingClientRect()[end]; + let collapseButtonWidth = items[items.length - 1].getBoundingClientRect().width; + let lastTagEnd = items[index - 1]?.getBoundingClientRect()[end]; + + while (containerEnd - lastTagEnd < collapseButtonWidth) { + lastTagEnd = items[index - 1]?.getBoundingClientRect()[end]; + index--; + } + setVisibleTagCount(index); } - }, [maxRows, domRef, isCollapsed]); + }, [maxRows, isCollapsed, domRef, direction]); useEffect(() => { checkVisibleTagCount(); From 5b1d0739b9b322eb4d95b8aebe1fce422ac310dc Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 1 Dec 2022 12:10:05 -0600 Subject: [PATCH 06/42] revert cleanup --- 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 faf51f6f518..3e431c90448 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, + let {isFocused} = props; + const { allowsRemoving, onRemove, item, tagRef, tagRowRef } = props; - let stringFormatter = useLocalizedStringFormatter(intlMessages); - let removeString = stringFormatter.format('remove'); - let labelId = useId(); - let buttonId = useId(); + const stringFormatter = useLocalizedStringFormatter(intlMessages); + const removeString = stringFormatter.format('remove'); + const labelId = useId(); + const buttonId = useId(); let {rowProps} = useGridRow({ node: item @@ -60,7 +60,7 @@ export function useTag(props: TagProps, state: GridState): TagAri e.preventDefault(); } } - let pressProps = { + const pressProps = { onPress: () => onRemove?.(item.childNodes[0].key) }; From 888562d1526e1987556db868238418b7e92ccaae Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 2 Dec 2022 14:15:55 -0600 Subject: [PATCH 07/42] update logic --- packages/@react-spectrum/tag/src/TagGroup.tsx | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 631244018c3..a6a36bde24d 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -14,8 +14,8 @@ import {ActionButton} from '@react-spectrum/button'; import {classNames, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {DOMRef} from '@react-types/shared'; import {GridCollection, useGridState} from '@react-stately/grid'; -import {mergeProps, useResizeObserver} from '@react-aria/utils'; -import React, {ReactElement, useCallback, useEffect, useMemo, useState} from 'react'; +import {mergeProps, useLayoutEffect, useResizeObserver} from '@react-aria/utils'; +import React, {ReactElement, useCallback, useMemo, useState} from 'react'; import {SpectrumTagGroupProps} from '@react-types/tag'; import styles from '@adobe/spectrum-css-temp/components/tags/vars.css'; import {Tag} from './Tag'; @@ -74,14 +74,15 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef let [visibleTagCount, setVisibleTagCount] = useState(gridCollection.size); let [isCollapsed, setIsCollapsed] = useState(maxRows != null); + // If maxRows is provided, update the number of tags to show based on the height. let checkVisibleTagCount = useCallback(() => { if (maxRows != null && isCollapsed) { let currY = -Infinity; let rowCount = 0; let index = 0; - let items = [...domRef.current.children]; - for (let item of items) { - let {y} = item.getBoundingClientRect(); + let currentVisibleTags = [...domRef.current.children].filter(el => el.tagName !== 'BUTTON'); + for (let tag of currentVisibleTags) { + let {y} = tag.getBoundingClientRect(); if (y !== currY) { currY = y; @@ -97,19 +98,19 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef // Hide tags on last row until there is room for the collapse button. let end = direction === 'ltr' ? 'right' : 'left'; let containerEnd = domRef.current.getBoundingClientRect()[end]; - let collapseButtonWidth = items[items.length - 1].getBoundingClientRect().width; - let lastTagEnd = items[index - 1]?.getBoundingClientRect()[end]; + let collapseButtonWidth = domRef.current.querySelector('button')?.getBoundingClientRect().width; + let lastTagEnd = currentVisibleTags[index - 1]?.getBoundingClientRect()[end]; while (containerEnd - lastTagEnd < collapseButtonWidth) { - lastTagEnd = items[index - 1]?.getBoundingClientRect()[end]; index--; + lastTagEnd = currentVisibleTags[index - 1]?.getBoundingClientRect()[end]; } setVisibleTagCount(index); } }, [maxRows, isCollapsed, domRef, direction]); - useEffect(() => { + useLayoutEffect(() => { checkVisibleTagCount(); }, [props.children, checkVisibleTagCount]); From 3a9785746037f21c2a53fd1657bfdc178197e51b Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 5 Dec 2022 12:17:49 -0600 Subject: [PATCH 08/42] update to use generator --- packages/@react-spectrum/tag/src/TagGroup.tsx | 74 +++++++++++++------ 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index a6a36bde24d..0ac92f193e9 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -14,7 +14,7 @@ import {ActionButton} from '@react-spectrum/button'; import {classNames, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {DOMRef} from '@react-types/shared'; import {GridCollection, useGridState} from '@react-stately/grid'; -import {mergeProps, useLayoutEffect, useResizeObserver} from '@react-aria/utils'; +import {mergeProps, useLayoutEffect, useResizeObserver, useValueEffect} from '@react-aria/utils'; import React, {ReactElement, useCallback, useMemo, useState} from 'react'; import {SpectrumTagGroupProps} from '@react-types/tag'; import styles from '@adobe/spectrum-css-temp/components/tags/vars.css'; @@ -31,6 +31,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef allowsRemoving, onRemove, maxRows, + children, ...otherProps } = props; let domRef = useDOMRef(ref); @@ -71,18 +72,32 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef }, state, domRef); const {tagGroupProps} = useTagGroup(props); - let [visibleTagCount, setVisibleTagCount] = useState(gridCollection.size); let [isCollapsed, setIsCollapsed] = useState(maxRows != null); - // If maxRows is provided, update the number of tags to show based on the height. - let checkVisibleTagCount = useCallback(() => { - if (maxRows != null && isCollapsed) { + // Not using React.Children.toArray because it mutates the key prop. + let childArray: ReactElement[] = []; + React.Children.forEach(children, child => { + if (React.isValidElement(child)) { + childArray.push(child); + } + }); + let [visibleTagCount, setVisibleTagCount] = useValueEffect(childArray.length); + + let updateVisibleTagCount = useCallback(() => { + let computeVisibleTagCount = () => { + // Refs can be null at runtime. + let currDomRef: HTMLDivElement | null = domRef.current; + if (!currDomRef) { + return; + } + + let items = [...currDomRef.children].filter(el => el.tagName !== 'BUTTON') as HTMLDivElement[]; let currY = -Infinity; let rowCount = 0; let index = 0; - let currentVisibleTags = [...domRef.current.children].filter(el => el.tagName !== 'BUTTON'); - for (let tag of currentVisibleTags) { - let {y} = tag.getBoundingClientRect(); + // Count rows and show tags until we hit the maxRows. + for (let item of items) { + let {y} = item.getBoundingClientRect(); if (y !== currY) { currY = y; @@ -96,25 +111,36 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef } // Hide tags on last row until there is room for the collapse button. - let end = direction === 'ltr' ? 'right' : 'left'; - let containerEnd = domRef.current.getBoundingClientRect()[end]; - let collapseButtonWidth = domRef.current.querySelector('button')?.getBoundingClientRect().width; - let lastTagEnd = currentVisibleTags[index - 1]?.getBoundingClientRect()[end]; - - while (containerEnd - lastTagEnd < collapseButtonWidth) { - index--; - lastTagEnd = currentVisibleTags[index - 1]?.getBoundingClientRect()[end]; - } + // let end = direction === 'ltr' ? 'right' : 'left'; + // let containerEnd = currDomRef.getBoundingClientRect()[end]; + // let collapseButtonWidth = [...currDomRef.children].find(el => el.tagName !== 'BUTTON').getBoundingClientRect().width; + // let lastTagEnd = items[index - 1]?.getBoundingClientRect()[end]; + // let lastTagY = items[index - 1]?.getBoundingClientRect().y; + // let lastRowY = lastTagY; - setVisibleTagCount(index); - } - }, [maxRows, isCollapsed, domRef, direction]); + // while (containerEnd - lastTagEnd < collapseButtonWidth && lastTagY === lastRowY) { + // index--; + // lastTagEnd = items[index - 1]?.getBoundingClientRect()[end]; + // lastTagY = items[index - 1]?.getBoundingClientRect().y; + // } + + return index; + }; + + setVisibleTagCount(function *() { + // Update to show all items. + yield childArray.length; - useLayoutEffect(() => { - checkVisibleTagCount(); - }, [props.children, checkVisibleTagCount]); + // Measure, and update to show the items until maxRows is reached. + let newVisibleTagCount = computeVisibleTagCount(); + yield newVisibleTagCount; + }); + }, [childArray.length, domRef, maxRows, setVisibleTagCount]); - useResizeObserver({ref: domRef, onResize: checkVisibleTagCount}); + useResizeObserver({ref: domRef, onResize: updateVisibleTagCount}); + + // eslint-disable-next-line react-hooks/exhaustive-deps + useLayoutEffect(updateVisibleTagCount, [children]); let visibleTags = [...gridCollection]; if (maxRows != null && isCollapsed) { From 4b2ecbe135f40798a456392b8428151c7e8ee8b1 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 6 Dec 2022 10:10:56 -0600 Subject: [PATCH 09/42] include show all button on last row --- packages/@react-spectrum/tag/src/TagGroup.tsx | 43 +++++++++++-------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 0ac92f193e9..3caa2fd8f82 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -91,13 +91,13 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef return; } - let items = [...currDomRef.children].filter(el => el.tagName !== 'BUTTON') as HTMLDivElement[]; + let tags = [...currDomRef.children].filter(el => el.tagName !== 'BUTTON') as HTMLDivElement[]; let currY = -Infinity; let rowCount = 0; let index = 0; // Count rows and show tags until we hit the maxRows. - for (let item of items) { - let {y} = item.getBoundingClientRect(); + for (let tag of tags) { + let {y} = tag.getBoundingClientRect(); if (y !== currY) { currY = y; @@ -109,21 +109,6 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef } index++; } - - // Hide tags on last row until there is room for the collapse button. - // let end = direction === 'ltr' ? 'right' : 'left'; - // let containerEnd = currDomRef.getBoundingClientRect()[end]; - // let collapseButtonWidth = [...currDomRef.children].find(el => el.tagName !== 'BUTTON').getBoundingClientRect().width; - // let lastTagEnd = items[index - 1]?.getBoundingClientRect()[end]; - // let lastTagY = items[index - 1]?.getBoundingClientRect().y; - // let lastRowY = lastTagY; - - // while (containerEnd - lastTagEnd < collapseButtonWidth && lastTagY === lastRowY) { - // index--; - // lastTagEnd = items[index - 1]?.getBoundingClientRect()[end]; - // lastTagY = items[index - 1]?.getBoundingClientRect().y; - // } - return index; }; @@ -134,8 +119,28 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef // Measure, and update to show the items until maxRows is reached. let newVisibleTagCount = computeVisibleTagCount(); yield newVisibleTagCount; + + // Remove tags until there is space for the collapse button on the last row. + let tags = [...domRef.current.children].filter(el => el.tagName !== 'BUTTON'); + let collapseButton = [...domRef.current.children].find(el => el.tagName === 'BUTTON'); + let lastTagY = tags[newVisibleTagCount - 1].getBoundingClientRect().y; + if (collapseButton?.getBoundingClientRect().y > lastTagY) { + let end = direction === 'ltr' ? 'right' : 'left'; + let containerEnd = domRef.current.getBoundingClientRect()[end]; + let lastTagEnd = tags[newVisibleTagCount - 1].getBoundingClientRect()[end]; + let collapseButtonWidth = collapseButton.getBoundingClientRect().width; + let firstTagY = tags[0].getBoundingClientRect().y; + let lastRowY = lastTagY; + + while (containerEnd - lastTagEnd < collapseButtonWidth && lastTagY === lastRowY && firstTagY !== lastRowY) { + newVisibleTagCount--; + yield newVisibleTagCount; + lastTagEnd = tags[newVisibleTagCount - 1].getBoundingClientRect()[end]; + lastTagY = tags[newVisibleTagCount - 1].getBoundingClientRect().y; + } + } }); - }, [childArray.length, domRef, maxRows, setVisibleTagCount]); + }, [childArray.length, direction, domRef, maxRows, setVisibleTagCount]); useResizeObserver({ref: domRef, onResize: updateVisibleTagCount}); From 49b75a34ca9dd93fbdb188e8c98533d5909460b6 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 6 Dec 2022 10:11:28 -0600 Subject: [PATCH 10/42] update story to fill width for resizing --- 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 ca46d6b928f..cb8a83ca9d3 100644 --- a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx +++ b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx @@ -94,7 +94,7 @@ storiesOf('TagGroup', module) ) .add('maxRows: 2', () => (
- + Cool Tag 1 Another cool tag This tag From bba7bacf7510a4db4046d13bb7aae5d6e1c778e2 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 6 Dec 2022 10:22:30 -0600 Subject: [PATCH 11/42] add test --- .../@react-spectrum/tag/test/TagGroup.test.js | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index 37335c931ca..8f74b96c246 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -299,6 +299,85 @@ describe('TagGroup', function () { expect(onRemoveSpy).toHaveBeenCalledWith('1'); }); + it('maxRows should limit the number of tags shown', function () { + let offsetWidth = jest.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementationOnce(function () { + return { + left: 0, + top: 0, + x: 0, + y: 0, + width: 100, + height: 50 + }; + }).mockImplementationOnce(function () { + return { + left: 0, + top: 0, + x: 0, + y: 0, + width: 100, + height: 50 + }; + }).mockImplementationOnce(function () { + return { + left: 0, + top: 0, + x: 0, + y: 100, + width: 100, + height: 50 + }; + }).mockImplementationOnce(function () { + return { + left: 0, + top: 0, + x: 0, + y: 100, + width: 100, + height: 50 + }; + }).mockImplementation(function () { + return { + left: 0, + top: 0, + x: 0, + y: 200, + width: 100, + height: 50 + }; + }); + let {getAllByRole, getByRole} = render( + + + Tag 1 + Tag 2 + Tag 3 + Tag 4 + Tag 5 + Tag 6 + Tag 7 + + + ); + + let tags = getAllByRole('gridcell'); + expect(tags.length).toBe(4); + + let button = getByRole('button'); + expect(button).toHaveTextContent('Show all (7)'); + + userEvent.click(button); + tags = getAllByRole('gridcell'); + expect(tags.length).toBe(7); + expect(button).toHaveTextContent('Show less'); + + userEvent.click(button); + tags = getAllByRole('gridcell'); + expect(tags.length).toBe(4); + expect(button).toHaveTextContent('Show all (7)'); + + offsetWidth.mockReset(); + }); // Commented out until spectrum can provide use case for these scenarios // it.each` From 4639f4d444375c35bb7cfd590e37141b122fa51b Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 6 Dec 2022 11:12:52 -0600 Subject: [PATCH 12/42] cleanup --- packages/@react-spectrum/tag/src/TagGroup.tsx | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 3caa2fd8f82..2dd9696a117 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -73,15 +73,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef const {tagGroupProps} = useTagGroup(props); let [isCollapsed, setIsCollapsed] = useState(maxRows != null); - - // Not using React.Children.toArray because it mutates the key prop. - let childArray: ReactElement[] = []; - React.Children.forEach(children, child => { - if (React.isValidElement(child)) { - childArray.push(child); - } - }); - let [visibleTagCount, setVisibleTagCount] = useValueEffect(childArray.length); + let [visibleTagCount, setVisibleTagCount] = useValueEffect(gridCollection.size); let updateVisibleTagCount = useCallback(() => { let computeVisibleTagCount = () => { @@ -114,7 +106,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef setVisibleTagCount(function *() { // Update to show all items. - yield childArray.length; + yield gridCollection.size; // Measure, and update to show the items until maxRows is reached. let newVisibleTagCount = computeVisibleTagCount(); @@ -140,7 +132,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef } } }); - }, [childArray.length, direction, domRef, maxRows, setVisibleTagCount]); + }, [direction, domRef, gridCollection.size, maxRows, setVisibleTagCount]); useResizeObserver({ref: domRef, onResize: updateVisibleTagCount}); From af7d23ebcd14478418a2b9a552f7509084f745d9 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 6 Dec 2022 17:37:33 -0600 Subject: [PATCH 13/42] add FocusScope --- packages/@react-spectrum/tag/src/TagGroup.tsx | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 2dd9696a117..aaf5c18eef4 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -13,6 +13,7 @@ import {ActionButton} from '@react-spectrum/button'; import {classNames, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {DOMRef} from '@react-types/shared'; +import {FocusScope} from '@react-aria/focus'; import {GridCollection, useGridState} from '@react-stately/grid'; import {mergeProps, useLayoutEffect, useResizeObserver, useValueEffect} from '@react-aria/utils'; import React, {ReactElement, useCallback, useMemo, useState} from 'react'; @@ -148,34 +149,36 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef // eslint-disable-next-line @typescript-eslint/no-unused-vars let {tabIndex, role, ...otherGridProps} = gridProps; return ( -
+
- {visibleTags.map(item => ( - - {item.childNodes[0].rendered} - + role={state.collection.size ? 'grid' : null} + ref={domRef}> + {visibleTags.map(item => ( + + {item.childNodes[0].rendered} + ))} - {maxRows != null && visibleTagCount < gridCollection.size && + {maxRows != null && visibleTagCount < gridCollection.size && setIsCollapsed(!isCollapsed)}> {isCollapsed ? `Show all (${gridCollection.size})` : 'Show less '} } -
+
+ ); } From 9e7bd1c63bd5158f6ddba2a4ffe99bafdc337eba Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 7 Dec 2022 13:00:08 -0600 Subject: [PATCH 14/42] improve performance --- packages/@react-spectrum/tag/src/TagGroup.tsx | 107 +++++++++--------- 1 file changed, 56 insertions(+), 51 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index aaf5c18eef4..5d9b7f4dce3 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -77,62 +77,67 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef let [visibleTagCount, setVisibleTagCount] = useValueEffect(gridCollection.size); let updateVisibleTagCount = useCallback(() => { - let computeVisibleTagCount = () => { - // Refs can be null at runtime. - let currDomRef: HTMLDivElement | null = domRef.current; - if (!currDomRef) { - return; - } - - let tags = [...currDomRef.children].filter(el => el.tagName !== 'BUTTON') as HTMLDivElement[]; - let currY = -Infinity; - let rowCount = 0; - let index = 0; - // Count rows and show tags until we hit the maxRows. - for (let tag of tags) { - let {y} = tag.getBoundingClientRect(); - - if (y !== currY) { - currY = y; - rowCount++; + if (maxRows !== null) { + let computeVisibleTagCount = (collapseButtonWidth?: number) => { + // Refs can be null at runtime. + let currDomRef: HTMLDivElement | null = domRef.current; + if (!currDomRef) { + return; } - if (rowCount > maxRows) { - break; + let tags = [...currDomRef.children].filter(el => el.tagName !== 'BUTTON'); + let currY = -Infinity; + let rowCount = 0; + let index = 0; + let tagWidths = []; + // Count rows and show tags until we hit the maxRows. + for (let tag of tags) { + let {y} = tag.getBoundingClientRect(); + + if (y !== currY) { + currY = y; + rowCount++; + } + + if (rowCount > maxRows) { + break; + } + tagWidths.push(tag.getBoundingClientRect().width); + index++; } - index++; - } - return index; - }; - - setVisibleTagCount(function *() { - // Update to show all items. - yield gridCollection.size; - // Measure, and update to show the items until maxRows is reached. - let newVisibleTagCount = computeVisibleTagCount(); - yield newVisibleTagCount; - - // Remove tags until there is space for the collapse button on the last row. - let tags = [...domRef.current.children].filter(el => el.tagName !== 'BUTTON'); - let collapseButton = [...domRef.current.children].find(el => el.tagName === 'BUTTON'); - let lastTagY = tags[newVisibleTagCount - 1].getBoundingClientRect().y; - if (collapseButton?.getBoundingClientRect().y > lastTagY) { - let end = direction === 'ltr' ? 'right' : 'left'; - let containerEnd = domRef.current.getBoundingClientRect()[end]; - let lastTagEnd = tags[newVisibleTagCount - 1].getBoundingClientRect()[end]; - let collapseButtonWidth = collapseButton.getBoundingClientRect().width; - let firstTagY = tags[0].getBoundingClientRect().y; - let lastRowY = lastTagY; - - while (containerEnd - lastTagEnd < collapseButtonWidth && lastTagY === lastRowY && firstTagY !== lastRowY) { - newVisibleTagCount--; - yield newVisibleTagCount; - lastTagEnd = tags[newVisibleTagCount - 1].getBoundingClientRect()[end]; - lastTagY = tags[newVisibleTagCount - 1].getBoundingClientRect().y; + // Remove tags until there is space for the collapse button on the last row. + if (collapseButtonWidth) { + let end = direction === 'ltr' ? 'right' : 'left'; + let containerEnd = currDomRef.getBoundingClientRect()[end]; + let lastTagEnd = tags[index - 1]?.getBoundingClientRect()[end]; + let availableWidth = containerEnd - lastTagEnd; + for (let tagWidth of tagWidths.reverse()) { + if (availableWidth > collapseButtonWidth || index <= 1) { + break; + } + availableWidth += tagWidth; + index--; + } } - } - }); + + return index; + }; + + setVisibleTagCount(function *() { + // Update to show all items. + yield gridCollection.size; + + // Measure, and update to show the items until maxRows is reached. + let newVisibleTagCount = computeVisibleTagCount(); + yield newVisibleTagCount; + + if (newVisibleTagCount < gridCollection.size) { + let collapseButtonWidth = [...domRef.current.children].find(el => el.tagName === 'BUTTON')?.getBoundingClientRect().width; + yield computeVisibleTagCount(collapseButtonWidth); + } + }); + } }, [direction, domRef, gridCollection.size, maxRows, setVisibleTagCount]); useResizeObserver({ref: domRef, onResize: updateVisibleTagCount}); From 58ffb28405c39b17619f7cc5ded04193d4b633bb Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 7 Dec 2022 13:16:10 -0600 Subject: [PATCH 15/42] add function for making room for collapse button --- packages/@react-spectrum/tag/src/TagGroup.tsx | 43 +++++++++++-------- .../@react-spectrum/tag/test/TagGroup.test.js | 4 +- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 5d9b7f4dce3..5c789adb7a7 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -78,7 +78,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef let updateVisibleTagCount = useCallback(() => { if (maxRows !== null) { - let computeVisibleTagCount = (collapseButtonWidth?: number) => { + let computeVisibleTagCount = () => { // Refs can be null at runtime. let currDomRef: HTMLDivElement | null = domRef.current; if (!currDomRef) { @@ -105,22 +105,28 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef tagWidths.push(tag.getBoundingClientRect().width); index++; } + + return {index, tagWidths}; + }; - // Remove tags until there is space for the collapse button on the last row. - if (collapseButtonWidth) { - let end = direction === 'ltr' ? 'right' : 'left'; - let containerEnd = currDomRef.getBoundingClientRect()[end]; - let lastTagEnd = tags[index - 1]?.getBoundingClientRect()[end]; - let availableWidth = containerEnd - lastTagEnd; - for (let tagWidth of tagWidths.reverse()) { - if (availableWidth > collapseButtonWidth || index <= 1) { - break; - } - availableWidth += tagWidth; - index--; + let clearSpaceForCollapseButton = ({index, tagWidths, collapseButtonWidth}) => { + // Refs can be null at runtime. + let currDomRef: HTMLDivElement | null = domRef.current; + if (!currDomRef) { + return; + } + let tags = [...currDomRef.children].filter(el => el.tagName !== 'BUTTON'); + let end = direction === 'ltr' ? 'right' : 'left'; + let containerEnd = currDomRef.getBoundingClientRect()[end]; + let lastTagEnd = tags[index - 1]?.getBoundingClientRect()[end]; + let availableWidth = containerEnd - lastTagEnd; + for (let tagWidth of tagWidths.reverse()) { + if (availableWidth > collapseButtonWidth || index <= 1) { + break; } + availableWidth += tagWidth; + index--; } - return index; }; @@ -129,12 +135,13 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef yield gridCollection.size; // Measure, and update to show the items until maxRows is reached. - let newVisibleTagCount = computeVisibleTagCount(); - yield newVisibleTagCount; + let {index, tagWidths} = computeVisibleTagCount(); + yield index; - if (newVisibleTagCount < gridCollection.size) { + // Remove tags until there is space for the collapse button on the last row. + if (index < gridCollection.size) { let collapseButtonWidth = [...domRef.current.children].find(el => el.tagName === 'BUTTON')?.getBoundingClientRect().width; - yield computeVisibleTagCount(collapseButtonWidth); + yield clearSpaceForCollapseButton({index, tagWidths, collapseButtonWidth}); } }); } diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index 8f74b96c246..7e84cc1bc4d 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -361,7 +361,7 @@ describe('TagGroup', function () { ); let tags = getAllByRole('gridcell'); - expect(tags.length).toBe(4); + expect(tags.length).toBe(1); let button = getByRole('button'); expect(button).toHaveTextContent('Show all (7)'); @@ -373,7 +373,7 @@ describe('TagGroup', function () { userEvent.click(button); tags = getAllByRole('gridcell'); - expect(tags.length).toBe(4); + expect(tags.length).toBe(1); expect(button).toHaveTextContent('Show all (7)'); offsetWidth.mockReset(); From dfc95aac9c7960bdebca3853ce9846e9bf2c9f40 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 7 Dec 2022 16:13:38 -0600 Subject: [PATCH 16/42] fix indention --- packages/@react-spectrum/tag/src/TagGroup.tsx | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 5c789adb7a7..e42dfd6bf72 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -165,12 +165,12 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef
{visibleTags.map(item => ( @@ -183,12 +183,12 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef onRemove={onRemove}> {item.childNodes[0].rendered} - ))} + ))} {maxRows != null && visibleTagCount < gridCollection.size && - setIsCollapsed(!isCollapsed)}> - {isCollapsed ? `Show all (${gridCollection.size})` : 'Show less '} - - } + setIsCollapsed(!isCollapsed)}> + {isCollapsed ? `Show all (${gridCollection.size})` : 'Show less '} + + }
); From ca308a3da45df5ef54ff2aec3405d9e230be31b6 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 7 Dec 2022 17:14:58 -0600 Subject: [PATCH 17/42] move into one function --- packages/@react-spectrum/tag/src/TagGroup.tsx | 56 ++++++++----------- 1 file changed, 22 insertions(+), 34 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index e42dfd6bf72..27be23dc2b9 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -74,7 +74,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef const {tagGroupProps} = useTagGroup(props); let [isCollapsed, setIsCollapsed] = useState(maxRows != null); - let [visibleTagCount, setVisibleTagCount] = useValueEffect(gridCollection.size); + let [tagState, setTagState] = useValueEffect({visibleTagCount: gridCollection.size, showCollapseButton: false}); let updateVisibleTagCount = useCallback(() => { if (maxRows !== null) { @@ -105,47 +105,35 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef tagWidths.push(tag.getBoundingClientRect().width); index++; } - - return {index, tagWidths}; - }; - let clearSpaceForCollapseButton = ({index, tagWidths, collapseButtonWidth}) => { - // Refs can be null at runtime. - let currDomRef: HTMLDivElement | null = domRef.current; - if (!currDomRef) { - return; - } - let tags = [...currDomRef.children].filter(el => el.tagName !== 'BUTTON'); - let end = direction === 'ltr' ? 'right' : 'left'; - let containerEnd = currDomRef.getBoundingClientRect()[end]; - let lastTagEnd = tags[index - 1]?.getBoundingClientRect()[end]; - let availableWidth = containerEnd - lastTagEnd; - for (let tagWidth of tagWidths.reverse()) { - if (availableWidth > collapseButtonWidth || index <= 1) { - break; + // Remove tags until there is space for the collapse button on the last row. + if (tagState.showCollapseButton) { + let collapseButtonWidth = [...domRef.current.children].find(el => el.tagName === 'BUTTON')?.getBoundingClientRect().width; + let end = direction === 'ltr' ? 'right' : 'left'; + let containerEnd = currDomRef.getBoundingClientRect()[end]; + let lastTagEnd = tags[index - 1]?.getBoundingClientRect()[end]; + let availableWidth = containerEnd - lastTagEnd; + for (let tagWidth of tagWidths.reverse()) { + if (availableWidth > collapseButtonWidth || index <= 1) { + break; + } + availableWidth += tagWidth; + index--; } - availableWidth += tagWidth; - index--; } - return index; + + return {visibleTagCount: index, showCollapseButton: index < gridCollection.size}; }; - setVisibleTagCount(function *() { + setTagState(function *() { // Update to show all items. - yield gridCollection.size; + yield {visibleTagCount: gridCollection.size, showCollapseButton: true}; // Measure, and update to show the items until maxRows is reached. - let {index, tagWidths} = computeVisibleTagCount(); - yield index; - - // Remove tags until there is space for the collapse button on the last row. - if (index < gridCollection.size) { - let collapseButtonWidth = [...domRef.current.children].find(el => el.tagName === 'BUTTON')?.getBoundingClientRect().width; - yield clearSpaceForCollapseButton({index, tagWidths, collapseButtonWidth}); - } + yield computeVisibleTagCount(); }); } - }, [direction, domRef, gridCollection.size, maxRows, setVisibleTagCount]); + }, [direction, domRef, gridCollection.size, maxRows, setTagState, tagState.showCollapseButton]); useResizeObserver({ref: domRef, onResize: updateVisibleTagCount}); @@ -154,7 +142,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef let visibleTags = [...gridCollection]; if (maxRows != null && isCollapsed) { - visibleTags = visibleTags.slice(0, visibleTagCount); + visibleTags = visibleTags.slice(0, tagState.visibleTagCount); } // Don't want the grid to be focusable or accessible via keyboard @@ -184,7 +172,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef {item.childNodes[0].rendered} ))} - {maxRows != null && visibleTagCount < gridCollection.size && + {tagState.showCollapseButton && setIsCollapsed(!isCollapsed)}> {isCollapsed ? `Show all (${gridCollection.size})` : 'Show less '} From 62392e71c2bd8cdb42fe3ff65989a518a463b6c9 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 7 Dec 2022 17:16:23 -0600 Subject: [PATCH 18/42] update test --- packages/@react-spectrum/tag/test/TagGroup.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index 7e84cc1bc4d..8664fcee28a 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -361,7 +361,7 @@ describe('TagGroup', function () { ); let tags = getAllByRole('gridcell'); - expect(tags.length).toBe(1); + expect(tags.length).toBe(2); let button = getByRole('button'); expect(button).toHaveTextContent('Show all (7)'); @@ -373,7 +373,7 @@ describe('TagGroup', function () { userEvent.click(button); tags = getAllByRole('gridcell'); - expect(tags.length).toBe(1); + expect(tags.length).toBe(2); expect(button).toHaveTextContent('Show all (7)'); offsetWidth.mockReset(); From 5505e2f6fbcf4dbdc075b6e86daea49570a1cbba Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 7 Dec 2022 17:41:00 -0600 Subject: [PATCH 19/42] address review comments --- packages/@react-spectrum/tag/src/TagGroup.tsx | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 27be23dc2b9..f3e7dd96c88 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -85,7 +85,14 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef return; } - let tags = [...currDomRef.children].filter(el => el.tagName !== 'BUTTON'); + let [tags, button] = [...currDomRef.children].reduce((acc, el) => { + if (el.tagName !== 'BUTTON') { + acc[0].push(el); + } else { + acc[1] = el; + } + return acc; + }, [[], null]); let currY = -Infinity; let rowCount = 0; let index = 0; @@ -107,21 +114,18 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef } // Remove tags until there is space for the collapse button on the last row. - if (tagState.showCollapseButton) { - let collapseButtonWidth = [...domRef.current.children].find(el => el.tagName === 'BUTTON')?.getBoundingClientRect().width; - let end = direction === 'ltr' ? 'right' : 'left'; - let containerEnd = currDomRef.getBoundingClientRect()[end]; - let lastTagEnd = tags[index - 1]?.getBoundingClientRect()[end]; - let availableWidth = containerEnd - lastTagEnd; - for (let tagWidth of tagWidths.reverse()) { - if (availableWidth > collapseButtonWidth || index <= 1) { - break; - } - availableWidth += tagWidth; - index--; + let buttonWidth = button.getBoundingClientRect().width; + let end = direction === 'ltr' ? 'right' : 'left'; + let containerEnd = currDomRef.getBoundingClientRect()[end]; + let lastTagEnd = tags[index - 1]?.getBoundingClientRect()[end]; + let availableWidth = containerEnd - lastTagEnd; + for (let tagWidth of tagWidths.reverse()) { + if (availableWidth > buttonWidth || index <= 1) { + break; } + availableWidth += tagWidth; + index--; } - return {visibleTagCount: index, showCollapseButton: index < gridCollection.size}; }; @@ -133,7 +137,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef yield computeVisibleTagCount(); }); } - }, [direction, domRef, gridCollection.size, maxRows, setTagState, tagState.showCollapseButton]); + }, [direction, domRef, gridCollection.size, maxRows, setTagState]); useResizeObserver({ref: domRef, onResize: updateVisibleTagCount}); From 4d71351343482bc66ea6d84c5f5b661b244105ea Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 8 Dec 2022 09:49:08 -0600 Subject: [PATCH 20/42] fix logic for showing button --- packages/@react-spectrum/tag/src/TagGroup.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index f3e7dd96c88..eae4520cfbd 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -77,7 +77,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef let [tagState, setTagState] = useValueEffect({visibleTagCount: gridCollection.size, showCollapseButton: false}); let updateVisibleTagCount = useCallback(() => { - if (maxRows !== null) { + if (maxRows > 0) { let computeVisibleTagCount = () => { // Refs can be null at runtime. let currDomRef: HTMLDivElement | null = domRef.current; From 692654fc22fe518b55ffba714161358a10b31051 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 8 Dec 2022 09:49:43 -0600 Subject: [PATCH 21/42] fix test --- packages/@react-spectrum/tag/test/TagGroup.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index 8664fcee28a..7e84cc1bc4d 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -361,7 +361,7 @@ describe('TagGroup', function () { ); let tags = getAllByRole('gridcell'); - expect(tags.length).toBe(2); + expect(tags.length).toBe(1); let button = getByRole('button'); expect(button).toHaveTextContent('Show all (7)'); @@ -373,7 +373,7 @@ describe('TagGroup', function () { userEvent.click(button); tags = getAllByRole('gridcell'); - expect(tags.length).toBe(2); + expect(tags.length).toBe(1); expect(button).toHaveTextContent('Show all (7)'); offsetWidth.mockReset(); From 7941553c5282b80dde6aee8580031cdf59aa7610 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 9 Dec 2022 11:37:22 -0600 Subject: [PATCH 22/42] condense getBoundingClientRect calls --- packages/@react-spectrum/tag/src/TagGroup.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index eae4520cfbd..0a10eec3453 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -99,7 +99,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef let tagWidths = []; // Count rows and show tags until we hit the maxRows. for (let tag of tags) { - let {y} = tag.getBoundingClientRect(); + let {width, y} = tag.getBoundingClientRect(); if (y !== currY) { currY = y; @@ -109,7 +109,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef if (rowCount > maxRows) { break; } - tagWidths.push(tag.getBoundingClientRect().width); + tagWidths.push(width); index++; } From fdbfc14a5ee0ea7df1d2464ac3ca956758764200 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 9 Dec 2022 11:37:33 -0600 Subject: [PATCH 23/42] update test --- .../@react-spectrum/tag/test/TagGroup.test.js | 59 ++++--------------- 1 file changed, 11 insertions(+), 48 deletions(-) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index 7e84cc1bc4d..95edb2d55dc 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -300,52 +300,15 @@ describe('TagGroup', function () { }); it('maxRows should limit the number of tags shown', function () { - let offsetWidth = jest.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementationOnce(function () { - return { - left: 0, - top: 0, - x: 0, - y: 0, - width: 100, - height: 50 - }; - }).mockImplementationOnce(function () { - return { - left: 0, - top: 0, - x: 0, - y: 0, - width: 100, - height: 50 - }; - }).mockImplementationOnce(function () { - return { - left: 0, - top: 0, - x: 0, - y: 100, - width: 100, - height: 50 - }; - }).mockImplementationOnce(function () { - return { - left: 0, - top: 0, - x: 0, - y: 100, - width: 100, - height: 50 - }; - }).mockImplementation(function () { - return { - left: 0, - top: 0, - x: 0, - y: 200, - width: 100, - height: 50 - }; - }); + let offsetWidth = jest.spyOn(HTMLElement.prototype, 'getBoundingClientRect') + .mockImplementationOnce(() => ({x: 199, y: 305, width: 77.234375, height: 32, top: 305, right: 276.234375, bottom: 337, left: 199})) + .mockImplementationOnce(() => ({x: 276.234375, y: 305, width: 109.75, height: 32, top: 305, right: 385.984375, bottom: 337, left: 276.234375})) + .mockImplementationOnce(() => ({x: 199, y: 337, width: 66.5546875, height: 32, top: 337, right: 265.5546875, bottom: 369, left: 199})) + .mockImplementationOnce(() => ({x: 265.5546875, y: 337, width: 77.3984375, height: 32, top: 337, right: 342.953125, bottom: 369, left: 265.5546875})) + .mockImplementationOnce(() => ({x: 199, y: 369, width: 119.171875, height: 32, top: 369, right: 318.171875, bottom: 401, left: 199})) + .mockImplementationOnce(() => ({x: 199, y: 401, width: 94.1953125, height: 32, top: 401, right: 293.1953125, bottom: 433, left: 199})) + .mockImplementationOnce(() => ({x: 199, y: 305, width: 200, height: 128, top: 305, right: 399, bottom: 433, left: 199})) + .mockImplementationOnce(() => ({x: 265.5546875, y: 337, width: 77.3984375, height: 32, top: 337, right: 342.953125, bottom: 369, left: 265.5546875})); let {getAllByRole, getByRole} = render( @@ -361,7 +324,7 @@ describe('TagGroup', function () { ); let tags = getAllByRole('gridcell'); - expect(tags.length).toBe(1); + expect(tags.length).toBe(3); let button = getByRole('button'); expect(button).toHaveTextContent('Show all (7)'); @@ -373,7 +336,7 @@ describe('TagGroup', function () { userEvent.click(button); tags = getAllByRole('gridcell'); - expect(tags.length).toBe(1); + expect(tags.length).toBe(3); expect(button).toHaveTextContent('Show all (7)'); offsetWidth.mockReset(); From f61d815f8a2280ce2cacd69d8b7904b41759ed98 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 9 Dec 2022 12:34:43 -0600 Subject: [PATCH 24/42] don't hide tags if all tags shown --- packages/@react-spectrum/tag/src/TagGroup.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 0a10eec3453..f82bfacfc22 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -120,7 +120,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef let lastTagEnd = tags[index - 1]?.getBoundingClientRect()[end]; let availableWidth = containerEnd - lastTagEnd; for (let tagWidth of tagWidths.reverse()) { - if (availableWidth > buttonWidth || index <= 1) { + if (availableWidth > buttonWidth || index <= 1 || index >= gridCollection.size) { break; } availableWidth += tagWidth; From a24043168415471fafcc38d1ef7590d90af8f811 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Mon, 12 Dec 2022 09:34:17 -0600 Subject: [PATCH 25/42] update mock values --- .../@react-spectrum/tag/test/TagGroup.test.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index 95edb2d55dc..8ce40da1b86 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -301,14 +301,14 @@ describe('TagGroup', function () { it('maxRows should limit the number of tags shown', function () { let offsetWidth = jest.spyOn(HTMLElement.prototype, 'getBoundingClientRect') - .mockImplementationOnce(() => ({x: 199, y: 305, width: 77.234375, height: 32, top: 305, right: 276.234375, bottom: 337, left: 199})) - .mockImplementationOnce(() => ({x: 276.234375, y: 305, width: 109.75, height: 32, top: 305, right: 385.984375, bottom: 337, left: 276.234375})) - .mockImplementationOnce(() => ({x: 199, y: 337, width: 66.5546875, height: 32, top: 337, right: 265.5546875, bottom: 369, left: 199})) - .mockImplementationOnce(() => ({x: 265.5546875, y: 337, width: 77.3984375, height: 32, top: 337, right: 342.953125, bottom: 369, left: 265.5546875})) - .mockImplementationOnce(() => ({x: 199, y: 369, width: 119.171875, height: 32, top: 369, right: 318.171875, bottom: 401, left: 199})) - .mockImplementationOnce(() => ({x: 199, y: 401, width: 94.1953125, height: 32, top: 401, right: 293.1953125, bottom: 433, left: 199})) - .mockImplementationOnce(() => ({x: 199, y: 305, width: 200, height: 128, top: 305, right: 399, bottom: 433, left: 199})) - .mockImplementationOnce(() => ({x: 265.5546875, y: 337, width: 77.3984375, height: 32, top: 337, right: 342.953125, bottom: 369, left: 265.5546875})); + .mockImplementationOnce(() => ({x: 200, y: 300, width: 75, height: 32, top: 300, right: 275, bottom: 335, left: 200})) + .mockImplementationOnce(() => ({x: 275, y: 300, width: 110, height: 32, top: 300, right: 385, bottom: 335, left: 275})) + .mockImplementationOnce(() => ({x: 200, y: 335, width: 65, height: 32, top: 335, right: 265, bottom: 370, left: 200})) + .mockImplementationOnce(() => ({x: 265, y: 335, width: 75, height: 32, top: 335, right: 345, bottom: 370, left: 265})) + .mockImplementationOnce(() => ({x: 200, y: 370, width: 120, height: 32, top: 370, right: 320, bottom: 400, left: 200})) + .mockImplementationOnce(() => ({x: 200, y: 400, width: 95, height: 32, top: 400, right: 290, bottom: 435, left: 200})) + .mockImplementationOnce(() => ({x: 200, y: 300, width: 200, height: 128, top: 300, right: 400, bottom: 435, left: 200})) + .mockImplementationOnce(() => ({x: 265, y: 335, width: 75, height: 32, top: 335, right: 345, bottom: 370, left: 265})); let {getAllByRole, getByRole} = render( From 6a1f7465961d2258d48575cd163860c3b11653cf Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 20 Dec 2022 12:14:31 -0600 Subject: [PATCH 26/42] update to CSF3 --- .../tag/stories/TagGroup.stories.tsx | 164 +++++++++++------- 1 file changed, 103 insertions(+), 61 deletions(-) diff --git a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx index cb8a83ca9d3..cbc6ace616b 100644 --- a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx +++ b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx @@ -12,56 +12,73 @@ import {action} from '@storybook/addon-actions'; import Audio from '@spectrum-icons/workflow/Audio'; -import {Icon} from '@react-spectrum/icon'; +import {ComponentMeta, ComponentStoryObj} from '@storybook/react'; import {Item, TagGroup} from '../src'; import React, {useState} from 'react'; -import {storiesOf} from '@storybook/react'; +import {TagGroupProps} from '@react-types/tag'; import {Text} from '@react-spectrum/text'; -storiesOf('TagGroup', module) - .add( - 'default', - () => render({}) - ) - .add('icons', () => ( - - {item => ( +let manyItems = []; +for (let i = 0; i < 50; i++) { + let item = {key: i}; + manyItems.push(item); +} + +export default { + title: 'TagGroup', + component: TagGroup, + args: { + onRemove: action('onRemove') + }, + argTypes: { + onRemove: { + table: { + disable: true + } + }, + maxRows: {type: 'number'} + } +} as ComponentMeta; + +export type TagGroupStory = ComponentStoryObj; + +export const Default: TagGroupStory = { + render: (args) => render(args) +}; + +function render(props: TagGroupProps) { + return ( + + Cool Tag 1 + Cool Tag 2 + Cool Tag 3 + + ); +} + +export const WithIcons: TagGroupStory = { + args: {items: [{key: '1', label: 'Cool Tag 1'}, {key: '2', label: 'Cool Tag 2'}]}, + render: (args) => ( + + {(item: any) => ( - - + )} - )) - .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'} - ]); - const onRemove = (key) => { - const newItems = [...items].filter((item) => key !== item.key.toString()); - setItems(newItems); - action('onRemove')(key); - }; - - return ( onRemove(key)}> - {item => ( - {item.label} - )} - ); - } ) - .add('wrapping', () => ( +}; + +export const OnRemove: TagGroupStory = { + render: (args) => , + storyName: 'onRemove' +}; + +export const Wrapping: TagGroupStory = { + render: (args) => (
- + Cool Tag 1 Another cool tag This tag @@ -71,30 +88,25 @@ storiesOf('TagGroup', module)
) - ) - .add('label truncation', () => ( +}; + +export const LabelTruncation: TagGroupStory = { + render: (args) => (
- + Cool Tag 1 with a really long label Another long cool tag label This tag
) - ) - .add( - 'using items prop', - () => ( - - {item => - {item.label} - } - - ) - ) - .add('maxRows: 2', () => ( +}; + +export const MaxRows: TagGroupStory = { + args: {maxRows: 2}, + render: (args) => (
- + Cool Tag 1 Another cool tag This tag @@ -103,14 +115,44 @@ storiesOf('TagGroup', module) Shy tag
- )); + ), + storyName: 'maxRows' +}; + +export const MaxRowsManyTags: TagGroupStory = { + args: {maxRows: 2}, + render: (args) => ( +
+ + {(item: any) => ( + {`Tag ${item.key}`} + )} + +
+ ), + storyName: 'maxRows with many tags' +}; + +function OnRemoveExample() { + let [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 onRemove = (key) => { + const newItems = [...items].filter((item) => key !== item.key.toString()); + setItems(newItems); + action('onRemove')(key); + }; -function render(props: any = {}) { return ( - - Cool Tag 1 - Cool Tag 2 - Cool Tag 3 + onRemove(key)}> + {item => ( + {item.label} + )} ); } From d527fd1fa377f8a002b0dc59518c8c791c285477 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 20 Dec 2022 12:24:40 -0600 Subject: [PATCH 27/42] add test for not showing button --- .../@react-spectrum/tag/test/TagGroup.test.js | 60 ++++++++----------- 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index 8ce40da1b86..43bf2c71f28 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -342,40 +342,28 @@ describe('TagGroup', function () { offsetWidth.mockReset(); }); - // 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'); - // }); -}); + it('maxRows should not show button if there is enough room to show all tags', 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})) + .mockImplementationOnce(() => ({x: 275, y: 300, width: 110, height: 32, top: 300, right: 385, bottom: 335, left: 275})) + .mockImplementationOnce(() => ({x: 200, y: 335, width: 65, height: 32, top: 335, right: 265, bottom: 370, left: 200})) + .mockImplementationOnce(() => ({x: 265, y: 335, width: 75, height: 32, top: 335, right: 345, bottom: 370, left: 265})) + .mockImplementationOnce(() => ({x: 200, y: 370, width: 120, height: 32, top: 370, right: 320, bottom: 400, left: 200})); + let {getAllByRole, queryAllByRole} = render( + + + Tag 1 + Tag 2 + + + ); + + let tags = getAllByRole('gridcell'); + expect(tags.length).toBe(2); -// need to add test for focus after onremove + let buttons = queryAllByRole('button'); + expect(buttons.length).toBe(0); + + offsetWidth.mockReset(); + }); +}); From 32479a12a982097d5c393f58909cf67e2f0a6c5e Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 3 Jan 2023 15:24:26 -0600 Subject: [PATCH 28/42] handle case of container width smaller than tag --- packages/@react-spectrum/tag/src/TagGroup.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index f82bfacfc22..8b3c4ea7e89 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -120,7 +120,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef let lastTagEnd = tags[index - 1]?.getBoundingClientRect()[end]; let availableWidth = containerEnd - lastTagEnd; for (let tagWidth of tagWidths.reverse()) { - if (availableWidth > buttonWidth || index <= 1 || index >= gridCollection.size) { + if (availableWidth >= buttonWidth || index <= 1 || index >= gridCollection.size) { break; } availableWidth += tagWidth; From bf793f5bdfa2cf68037a4290f5170e56da8f42af Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 4 Jan 2023 11:38:39 -0600 Subject: [PATCH 29/42] update deps --- packages/@react-spectrum/tag/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@react-spectrum/tag/package.json b/packages/@react-spectrum/tag/package.json index 03a1ff08d9a..f378545a60d 100644 --- a/packages/@react-spectrum/tag/package.json +++ b/packages/@react-spectrum/tag/package.json @@ -32,6 +32,7 @@ }, "dependencies": { "@react-aria/focus": "^3.10.1", + "@react-aria/i18n": "^3.6.3", "@react-aria/interactions": "^3.13.1", "@react-aria/tag": "3.0.0-beta.1", "@react-aria/utils": "^3.14.2", From 4e7ca75e17956c8689abff352b1169e7f4ae6233 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 5 Jan 2023 10:59:39 -0600 Subject: [PATCH 30/42] use custom keyboardDelegate when collapsed --- packages/@react-aria/tag/package.json | 1 + .../@react-aria/tag/src/TagKeyboardDelegate.ts | 2 ++ packages/@react-aria/tag/src/useTagGroup.ts | 2 +- packages/@react-spectrum/tag/package.json | 1 + packages/@react-spectrum/tag/src/TagGroup.tsx | 16 ++++++++++------ packages/@react-types/tag/package.json | 1 + packages/@react-types/tag/src/index.d.ts | 9 ++++++++- 7 files changed, 24 insertions(+), 8 deletions(-) diff --git a/packages/@react-aria/tag/package.json b/packages/@react-aria/tag/package.json index f8a69edc127..945934d639b 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/list": "^3.6.1", "@react-stately/tag": "3.0.0-alpha.1", "@react-types/button": "^3.7.0", "@react-types/shared": "^3.16.0", diff --git a/packages/@react-aria/tag/src/TagKeyboardDelegate.ts b/packages/@react-aria/tag/src/TagKeyboardDelegate.ts index 3f78d483f10..b3428022079 100644 --- a/packages/@react-aria/tag/src/TagKeyboardDelegate.ts +++ b/packages/@react-aria/tag/src/TagKeyboardDelegate.ts @@ -45,7 +45,9 @@ export class TagKeyboardDelegate implements KeyboardDelegate { } // Find the next item + console.log(this.collection); key = this.collection.getKeyAfter(key); + if (key != null) { return key; } else { diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index 8c94126cc72..217bbf862e1 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -33,7 +33,7 @@ export interface TagGroupAria { */ export function useTagGroup(props: AriaTagGroupProps, state: TagGroupState, ref: RefObject): TagGroupAria { let {direction} = useLocale(); - let keyboardDelegate = new TagKeyboardDelegate(state.collection, direction); + let keyboardDelegate = props.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 diff --git a/packages/@react-spectrum/tag/package.json b/packages/@react-spectrum/tag/package.json index f378545a60d..c153392cc1e 100644 --- a/packages/@react-spectrum/tag/package.json +++ b/packages/@react-spectrum/tag/package.json @@ -40,6 +40,7 @@ "@react-spectrum/text": "^3.3.4", "@react-spectrum/utils": "^3.8.1", "@react-stately/collections": "^3.5.1", + "@react-stately/list": "^3.6.1", "@react-stately/tag": "3.0.0-alpha.1", "@react-types/shared": "^3.16.0", "@react-types/tag": "3.0.0-beta.1", diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 8a4f14df837..9d2e980d643 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -14,14 +14,15 @@ import {ActionButton} from '@react-spectrum/button'; import {classNames, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {DOMRef} from '@react-types/shared'; import {FocusScope} from '@react-aria/focus'; +import {ListCollection} from '@react-stately/list'; import {mergeProps, useLayoutEffect, useResizeObserver, useValueEffect} from '@react-aria/utils'; -import React, {ReactElement, useCallback, useState} from 'react'; +import React, {ReactElement, useCallback, useMemo, useState} 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 {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) { @@ -36,12 +37,15 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef let domRef = useDOMRef(ref); let {styleProps} = useStyleProps(otherProps); let {direction} = useLocale(); - - let state = useTagGroupState(props); - let {tagGroupProps} = useTagGroup(props, state, domRef); - let [isCollapsed, setIsCollapsed] = useState(maxRows != null); + let state = useTagGroupState(props); let [tagState, setTagState] = useValueEffect({visibleTagCount: state.collection.size, showCollapseButton: false}); + let keyboardDelegate = useMemo(() => ( + isCollapsed + ? new TagKeyboardDelegate(new ListCollection([...state.collection].slice(0, tagState.visibleTagCount)), direction) + : new TagKeyboardDelegate(new ListCollection([...state.collection]), direction) + ), [direction, isCollapsed, state.collection, tagState.visibleTagCount]) as TagKeyboardDelegate; + let {tagGroupProps} = useTagGroup({...props, keyboardDelegate}, state, domRef); let updateVisibleTagCount = useCallback(() => { if (maxRows > 0) { diff --git a/packages/@react-types/tag/package.json b/packages/@react-types/tag/package.json index c2ccd24b075..daa53ea54e7 100644 --- a/packages/@react-types/tag/package.json +++ b/packages/@react-types/tag/package.json @@ -9,6 +9,7 @@ "url": "https://github.com/adobe/react-spectrum" }, "dependencies": { + "@react-aria/tag": "3.0.0-beta.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 1a48d8a4448..37b7946f694 100644 --- a/packages/@react-types/tag/src/index.d.ts +++ b/packages/@react-types/tag/src/index.d.ts @@ -12,6 +12,7 @@ import {AriaLabelingProps, CollectionBase, DOMProps, ItemProps, Node, StyleProps} from '@react-types/shared'; import {Key, RefObject} from 'react'; +import type {TagKeyboardDelegate} from '@react-aria/tag'; export interface TagGroupProps extends Omit, 'disabledKeys'> { /** Whether the TagGroup allows removal of tags. */ @@ -22,7 +23,13 @@ export interface TagGroupProps extends Omit, 'disabledKeys' maxRows?: number } -export interface AriaTagGroupProps extends TagGroupProps, DOMProps, AriaLabelingProps {} +export interface AriaTagGroupProps extends TagGroupProps, DOMProps, AriaLabelingProps { + /** + * An optional keyboard delegate to handle arrow key navigation, + * to override the default. + */ + keyboardDelegate?: TagKeyboardDelegate +} export interface SpectrumTagGroupProps extends AriaTagGroupProps, StyleProps {} From fcafa649a4bbcd30cb6e04397ef7745c870f16be Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 5 Jan 2023 11:07:21 -0600 Subject: [PATCH 31/42] lint --- packages/@react-aria/tag/src/TagKeyboardDelegate.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@react-aria/tag/src/TagKeyboardDelegate.ts b/packages/@react-aria/tag/src/TagKeyboardDelegate.ts index b3428022079..8e46e4238bd 100644 --- a/packages/@react-aria/tag/src/TagKeyboardDelegate.ts +++ b/packages/@react-aria/tag/src/TagKeyboardDelegate.ts @@ -45,7 +45,6 @@ export class TagKeyboardDelegate implements KeyboardDelegate { } // Find the next item - console.log(this.collection); key = this.collection.getKeyAfter(key); if (key != null) { From df2409f116eb232653590939206f660c4c562b93 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 5 Jan 2023 17:30:41 -0600 Subject: [PATCH 32/42] allow tabbing to collapse button --- packages/@react-aria/gridlist/src/useGridList.ts | 12 +++++++++--- packages/@react-spectrum/tag/src/TagGroup.tsx | 2 +- packages/@react-types/tag/src/index.d.ts | 6 +++++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/@react-aria/gridlist/src/useGridList.ts b/packages/@react-aria/gridlist/src/useGridList.ts index c35350a480a..a65d0822746 100644 --- a/packages/@react-aria/gridlist/src/useGridList.ts +++ b/packages/@react-aria/gridlist/src/useGridList.ts @@ -45,7 +45,11 @@ export interface AriaGridListOptions extends Omit, 'chil * An optional keyboard delegate implementation for type to select, * to override the default. */ - keyboardDelegate?: KeyboardDelegate + keyboardDelegate?: KeyboardDelegate, + /** + * Whether navigation through tab key is enabled. + */ + allowsTabNavigation?: boolean } export interface GridListAria { @@ -64,7 +68,8 @@ export function useGridList(props: AriaGridListOptions, state: ListState(props: AriaGridListOptions, state: ListState(props: SpectrumTagGroupProps, ref: DOMRef ? new TagKeyboardDelegate(new ListCollection([...state.collection].slice(0, tagState.visibleTagCount)), direction) : new TagKeyboardDelegate(new ListCollection([...state.collection]), direction) ), [direction, isCollapsed, state.collection, tagState.visibleTagCount]) as TagKeyboardDelegate; - let {tagGroupProps} = useTagGroup({...props, keyboardDelegate}, state, domRef); + let {tagGroupProps} = useTagGroup({...props, keyboardDelegate, allowsTabNavigation: tagState.showCollapseButton}, state, domRef); let updateVisibleTagCount = useCallback(() => { if (maxRows > 0) { diff --git a/packages/@react-types/tag/src/index.d.ts b/packages/@react-types/tag/src/index.d.ts index 37b7946f694..6da39192302 100644 --- a/packages/@react-types/tag/src/index.d.ts +++ b/packages/@react-types/tag/src/index.d.ts @@ -28,7 +28,11 @@ export interface AriaTagGroupProps extends TagGroupProps, DOMProps, AriaLa * An optional keyboard delegate to handle arrow key navigation, * to override the default. */ - keyboardDelegate?: TagKeyboardDelegate + keyboardDelegate?: TagKeyboardDelegate, + /** + * Whether navigation through tab key is enabled. + */ + allowsTabNavigation?: boolean } export interface SpectrumTagGroupProps extends AriaTagGroupProps, StyleProps {} From 81f1877ed4f8905cff5c104dd7f4b0c9c75668ec Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 5 Jan 2023 18:26:52 -0600 Subject: [PATCH 33/42] lint --- packages/@react-aria/tag/src/index.ts | 2 +- packages/@react-aria/tag/src/useTagGroup.ts | 18 +++++++++++++++--- packages/@react-spectrum/tag/src/TagGroup.tsx | 7 ++++--- packages/@react-spectrum/tag/src/index.ts | 2 +- .../tag/stories/TagGroup.stories.tsx | 5 ++--- packages/@react-types/tag/package.json | 1 - packages/@react-types/tag/src/index.d.ts | 17 +---------------- 7 files changed, 24 insertions(+), 28 deletions(-) diff --git a/packages/@react-aria/tag/src/index.ts b/packages/@react-aria/tag/src/index.ts index d7d10b431ff..d723ad97dab 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 {TagGroupAria} from './useTagGroup'; +export type {TagGroupAria, AriaTagGroupProps} from './useTagGroup'; export type {TagAria} from './useTag'; diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index 217bbf862e1..f18219db184 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -10,10 +10,10 @@ * governing permissions and limitations under the License. */ -import {AriaTagGroupProps} from '@react-types/tag'; -import {DOMAttributes} from '@react-types/shared'; +import {AriaLabelingProps, DOMAttributes, DOMProps} from '@react-types/shared'; import {filterDOMProps, mergeProps} from '@react-aria/utils'; import {RefObject, useState} from 'react'; +import {TagGroupProps} from '@react-types/tag'; import type {TagGroupState} from '@react-stately/tag'; import {TagKeyboardDelegate} from './TagKeyboardDelegate'; import {useFocusWithin} from '@react-aria/interactions'; @@ -24,6 +24,18 @@ export interface TagGroupAria { tagGroupProps: DOMAttributes } +export interface AriaTagGroupProps extends TagGroupProps, DOMProps, AriaLabelingProps { + /** + * An optional keyboard delegate to handle arrow key navigation, + * to override the default. + */ + keyboardDelegate?: TagKeyboardDelegate, + /** + * Whether navigation through tab key is enabled. + */ + allowsTabNavigation?: boolean +} + /** * 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. @@ -51,6 +63,6 @@ export function useTagGroup(props: AriaTagGroupProps, state: TagGroupState 'aria-relevant': 'additions', 'aria-live': isFocusWithin ? 'polite' : 'off', ...focusWithinProps - } as DOMAttributes) + }) }; } diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index cd251e190a6..5824d81c110 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -11,20 +11,21 @@ */ import {ActionButton} from '@react-spectrum/button'; +import {AriaTagGroupProps, TagKeyboardDelegate, useTagGroup} from '@react-aria/tag'; import {classNames, useDOMRef, useStyleProps} from '@react-spectrum/utils'; -import {DOMRef} from '@react-types/shared'; +import {DOMRef, StyleProps} from '@react-types/shared'; import {FocusScope} from '@react-aria/focus'; import {ListCollection} from '@react-stately/list'; import {mergeProps, useLayoutEffect, useResizeObserver, useValueEffect} from '@react-aria/utils'; import React, {ReactElement, useCallback, useMemo, useState} 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 {useLocale} from '@react-aria/i18n'; import {useProviderProps} from '@react-spectrum/provider'; import {useTagGroupState} from '@react-stately/tag'; +export interface SpectrumTagGroupProps extends AriaTagGroupProps, StyleProps {} + function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef) { props = useProviderProps(props); let { diff --git a/packages/@react-spectrum/tag/src/index.ts b/packages/@react-spectrum/tag/src/index.ts index fa14cf4eccb..59f733c31e1 100644 --- a/packages/@react-spectrum/tag/src/index.ts +++ b/packages/@react-spectrum/tag/src/index.ts @@ -12,4 +12,4 @@ export {TagGroup} from './TagGroup'; export {Item} from '@react-stately/collections'; -export type {SpectrumTagGroupProps} from '@react-types/tag'; +export type {SpectrumTagGroupProps} from './TagGroup'; diff --git a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx index 62be5ed00f5..1224d2cc290 100644 --- a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx +++ b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx @@ -13,9 +13,8 @@ import {action} from '@storybook/addon-actions'; import Audio from '@spectrum-icons/workflow/Audio'; import {ComponentMeta, ComponentStoryObj} from '@storybook/react'; -import {Item, TagGroup} from '../src'; +import {Item, SpectrumTagGroupProps, TagGroup} from '../src'; import React, {useState} from 'react'; -import {TagGroupProps} from '@react-types/tag'; import {Text} from '@react-spectrum/text'; let manyItems = []; @@ -46,7 +45,7 @@ export const Default: TagGroupStory = { render: (args) => render(args) }; -function render(props: TagGroupProps) { +function render(props: SpectrumTagGroupProps) { return ( Cool Tag 1 diff --git a/packages/@react-types/tag/package.json b/packages/@react-types/tag/package.json index daa53ea54e7..c2ccd24b075 100644 --- a/packages/@react-types/tag/package.json +++ b/packages/@react-types/tag/package.json @@ -9,7 +9,6 @@ "url": "https://github.com/adobe/react-spectrum" }, "dependencies": { - "@react-aria/tag": "3.0.0-beta.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 6da39192302..3632f49c130 100644 --- a/packages/@react-types/tag/src/index.d.ts +++ b/packages/@react-types/tag/src/index.d.ts @@ -10,9 +10,8 @@ * governing permissions and limitations under the License. */ -import {AriaLabelingProps, CollectionBase, DOMProps, ItemProps, Node, StyleProps} from '@react-types/shared'; +import {CollectionBase, ItemProps, Node} from '@react-types/shared'; import {Key, RefObject} from 'react'; -import type {TagKeyboardDelegate} from '@react-aria/tag'; export interface TagGroupProps extends Omit, 'disabledKeys'> { /** Whether the TagGroup allows removal of tags. */ @@ -23,20 +22,6 @@ export interface TagGroupProps extends Omit, 'disabledKeys' maxRows?: number } -export interface AriaTagGroupProps extends TagGroupProps, DOMProps, AriaLabelingProps { - /** - * An optional keyboard delegate to handle arrow key navigation, - * to override the default. - */ - keyboardDelegate?: TagKeyboardDelegate, - /** - * Whether navigation through tab key is enabled. - */ - allowsTabNavigation?: boolean -} - -export interface SpectrumTagGroupProps extends AriaTagGroupProps, StyleProps {} - export interface TagProps extends ItemProps { isFocused: boolean, allowsRemoving?: boolean, From a1c5d7a528722dbc102ef00ee93eb845e9e3be3b Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 6 Jan 2023 11:38:10 -0600 Subject: [PATCH 34/42] fix button losing focus if focusedKey collapsed --- packages/@react-spectrum/tag/src/TagGroup.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 5824d81c110..dd335e6d3bf 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -121,6 +121,12 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef visibleTags = visibleTags.slice(0, tagState.visibleTagCount); } + let handlePressCollapse = () => { + // Prevents button from losing focus if focusedKey got collapsed. + state.selectionManager.setFocusedKey(null); + setIsCollapsed(prevCollapsed => !prevCollapsed); + }; + return (
(props: SpectrumTagGroupProps, ref: DOMRef ))} {tagState.showCollapseButton && - setIsCollapsed(!isCollapsed)}> + {isCollapsed ? `Show all (${state.collection.size})` : 'Show less '} } From aa360a67f99409bc5ed32ef9ec9cacad2a4424a6 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 6 Jan 2023 13:44:45 -0600 Subject: [PATCH 35/42] add story for onRemove + maxRows --- .../tag/stories/TagGroup.stories.tsx | 102 +++++++++--------- 1 file changed, 50 insertions(+), 52 deletions(-) diff --git a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx index 1224d2cc290..78035577c9b 100644 --- a/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx +++ b/packages/@react-spectrum/tag/stories/TagGroup.stories.tsx @@ -23,12 +23,27 @@ for (let i = 0; i < 50; i++) { manyItems.push(item); } +function ResizableContainer({children}) { + return ( +
+ {children} +
+ ); +} + +function render(props: SpectrumTagGroupProps) { + return ( + + Cool Tag 1 + Cool Tag 2 + Cool Tag 3 + + ); +} + export default { title: 'TagGroup', component: TagGroup, - args: { - onRemove: action('onRemove') - }, argTypes: { onRemove: { table: { @@ -36,24 +51,13 @@ export default { } }, maxRows: {type: 'number'} - } + }, + render: args => render(args) } as ComponentMeta; export type TagGroupStory = ComponentStoryObj; -export const Default: TagGroupStory = { - render: (args) => render(args) -}; - -function render(props: SpectrumTagGroupProps) { - return ( - - Cool Tag 1 - Cool Tag 2 - Cool Tag 3 - - ); -} +export const Default: TagGroupStory = {}; export const WithIcons: TagGroupStory = { args: {items: [{key: '1', label: 'Cool Tag 1'}, {key: '2', label: 'Cool Tag 2'}]}, @@ -75,18 +79,7 @@ export const OnRemove: TagGroupStory = { }; export const Wrapping: TagGroupStory = { - render: (args) => ( -
- - Cool Tag 1 - Another cool tag - This tag - What tag? - This tag is cool too - Shy tag - -
- ) + decorators: [(Story) => {}] }; export const LabelTruncation: TagGroupStory = { @@ -98,41 +91,46 @@ export const LabelTruncation: TagGroupStory = { This tag
- ) + ) }; export const MaxRows: TagGroupStory = { args: {maxRows: 2}, render: (args) => ( -
- - Cool Tag 1 - Another cool tag - This tag - What tag? - This tag is cool too - Shy tag - -
- ), + + Cool Tag 1 + Another cool tag + This tag + What tag? + This tag is cool too + Shy tag + + ), + decorators: [(Story) => {}], storyName: 'maxRows' }; export const MaxRowsManyTags: TagGroupStory = { args: {maxRows: 2}, render: (args) => ( -
- - {(item: any) => ( - {`Tag ${item.key}`} - )} - -
- ), + + {(item: any) => ( + {`Tag ${item.key}`} + )} + + ), + decorators: [(Story) => {}], storyName: 'maxRows with many tags' }; -function OnRemoveExample() { +export const MaxRowsOnRemove: TagGroupStory = { + args: {maxRows: 2}, + render: (args) => , + decorators: [(Story) => {}], + storyName: 'maxRows + onRemove' +}; + +function OnRemoveExample(props) { let [items, setItems] = useState([ {id: 1, label: 'Cool Tag 1'}, {id: 2, label: 'Another cool tag'}, @@ -148,8 +146,8 @@ function OnRemoveExample() { }; return ( - onRemove(key)}> - {item => ( + onRemove(key)} {...props}> + {(item: any) => ( {item.label} )} From bbbd110dcc28a7a46306d4df56e6a8855027e9e3 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 10 Jan 2023 14:16:42 -0600 Subject: [PATCH 36/42] move button outside of grid --- .../components/tags/index.css | 28 ++++++--- .../@react-aria/gridlist/src/useGridList.ts | 12 +--- packages/@react-aria/tag/src/useTagGroup.ts | 6 +- packages/@react-spectrum/tag/src/TagGroup.tsx | 57 +++++++++---------- 4 files changed, 49 insertions(+), 54 deletions(-) diff --git a/packages/@adobe/spectrum-css-temp/components/tags/index.css b/packages/@adobe/spectrum-css-temp/components/tags/index.css index 28d36d50e42..1bab2430a37 100644 --- a/packages/@adobe/spectrum-css-temp/components/tags/index.css +++ b/packages/@adobe/spectrum-css-temp/components/tags/index.css @@ -12,13 +12,12 @@ governing permissions and limitations under the License. @import '../commons/index.css'; -.spectrum-Tags { - display: flex; - flex-wrap: wrap; +:root { + --spectrum-tag-margin: calc(var(--spectrum-taggroup-tag-gap-y) / 2) calc(var(--spectrum-taggroup-tag-gap-x) / 2); +} - margin: 0; - padding: 0; - list-style: none; +.spectrum-Tags { + display: inline; } .spectrum-Tags-item { @@ -26,15 +25,15 @@ governing permissions and limitations under the License. --spectrum-focus-ring-border-radius: var(--spectrum-tag-border-radius); --spectrum-focus-ring-border-size: var(--spectrum-tag-border-size); - display: grid; + display: inline-grid; grid-template-columns: auto 1fr auto; grid-template-areas: "icon content action"; align-items: center; box-sizing: border-box; position: relative; - margin: calc(var(--spectrum-taggroup-tag-gap-y) / 2) calc(var(--spectrum-taggroup-tag-gap-x) / 2); - padding-inline-start:calc(var(--spectrum-tag-padding-x) - var(--spectrum-tag-border-size)); + margin: var(--spectrum-tag-margin); + padding-inline-start: calc(var(--spectrum-tag-padding-x) - var(--spectrum-tag-border-size)); block-size: var(--spectrum-tag-height); max-inline-size: 100%; @@ -91,3 +90,14 @@ governing permissions and limitations under the License. } } +.spectrum-Tags-actionButton { + height: var(--spectrum-tag-height); + font-size: var(--spectrum-tag-text-size); + margin: var(--spectrum-tag-margin); + + > span { + padding-inline-start: var(--spectrum-tag-padding-x); + padding-inline-end: var(--spectrum-tag-padding-x); + } +} + diff --git a/packages/@react-aria/gridlist/src/useGridList.ts b/packages/@react-aria/gridlist/src/useGridList.ts index a65d0822746..c35350a480a 100644 --- a/packages/@react-aria/gridlist/src/useGridList.ts +++ b/packages/@react-aria/gridlist/src/useGridList.ts @@ -45,11 +45,7 @@ export interface AriaGridListOptions extends Omit, 'chil * An optional keyboard delegate implementation for type to select, * to override the default. */ - keyboardDelegate?: KeyboardDelegate, - /** - * Whether navigation through tab key is enabled. - */ - allowsTabNavigation?: boolean + keyboardDelegate?: KeyboardDelegate } export interface GridListAria { @@ -68,8 +64,7 @@ export function useGridList(props: AriaGridListOptions, state: ListState(props: AriaGridListOptions, state: ListState extends TagGroupProps, DOMProps, AriaLa * An optional keyboard delegate to handle arrow key navigation, * to override the default. */ - keyboardDelegate?: TagKeyboardDelegate, - /** - * Whether navigation through tab key is enabled. - */ - allowsTabNavigation?: boolean + keyboardDelegate?: TagKeyboardDelegate } /** diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index dd335e6d3bf..2f142f35464 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -46,7 +46,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef ? new TagKeyboardDelegate(new ListCollection([...state.collection].slice(0, tagState.visibleTagCount)), direction) : new TagKeyboardDelegate(new ListCollection([...state.collection]), direction) ), [direction, isCollapsed, state.collection, tagState.visibleTagCount]) as TagKeyboardDelegate; - let {tagGroupProps} = useTagGroup({...props, keyboardDelegate, allowsTabNavigation: tagState.showCollapseButton}, state, domRef); + let {tagGroupProps} = useTagGroup({...props, keyboardDelegate}, state, domRef); let updateVisibleTagCount = useCallback(() => { if (maxRows > 0) { @@ -57,14 +57,8 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef return; } - let [tags, button] = [...currDomRef.children].reduce((acc, el) => { - if (el.tagName !== 'BUTTON') { - acc[0].push(el); - } else { - acc[1] = el; - } - return acc; - }, [[], null]); + let tags = [...currDomRef.children]; + let button = currDomRef.parentElement.querySelector('button'); let currY = -Infinity; let rowCount = 0; let index = 0; @@ -130,29 +124,30 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef return (
- {visibleTags.map(item => ( - - {item.rendered} - - ))} + {...styleProps} + className={classNames(styles, 'spectrum-Tags-container', styleProps.className)}> +
+ {visibleTags.map(item => ( + + {item.rendered} + + ))} +
{tagState.showCollapseButton && - + {isCollapsed ? `Show all (${state.collection.size})` : 'Show less '} } From d3a0f99d0112068bc50a42079996b67eaf4b9b9a Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 10 Jan 2023 14:23:30 -0600 Subject: [PATCH 37/42] lint --- packages/@react-spectrum/tag/src/TagGroup.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 2f142f35464..52fac6ff05a 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -16,10 +16,10 @@ import {classNames, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {DOMRef, StyleProps} from '@react-types/shared'; import {FocusScope} from '@react-aria/focus'; import {ListCollection} from '@react-stately/list'; -import {mergeProps, useLayoutEffect, useResizeObserver, useValueEffect} from '@react-aria/utils'; import React, {ReactElement, useCallback, useMemo, useState} from 'react'; import styles from '@adobe/spectrum-css-temp/components/tags/vars.css'; import {Tag} from './Tag'; +import {useLayoutEffect, useResizeObserver, useValueEffect} from '@react-aria/utils'; import {useLocale} from '@react-aria/i18n'; import {useProviderProps} from '@react-spectrum/provider'; import {useTagGroupState} from '@react-stately/tag'; From 3096d2cbd579fa0691a2ec78cb8d7d9ae147bcbb Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 12 Jan 2023 12:15:27 -0600 Subject: [PATCH 38/42] fix refs for overflow detection --- packages/@react-spectrum/tag/src/TagGroup.tsx | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 52fac6ff05a..0d81339795a 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -16,7 +16,7 @@ import {classNames, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {DOMRef, StyleProps} from '@react-types/shared'; import {FocusScope} from '@react-aria/focus'; import {ListCollection} from '@react-stately/list'; -import React, {ReactElement, useCallback, useMemo, useState} from 'react'; +import React, {ReactElement, useCallback, useMemo, useRef, useState} from 'react'; import styles from '@adobe/spectrum-css-temp/components/tags/vars.css'; import {Tag} from './Tag'; import {useLayoutEffect, useResizeObserver, useValueEffect} from '@react-aria/utils'; @@ -36,6 +36,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef ...otherProps } = props; let domRef = useDOMRef(ref); + let containerRef = useRef(null); let {styleProps} = useStyleProps(otherProps); let {direction} = useLocale(); let [isCollapsed, setIsCollapsed] = useState(maxRows != null); @@ -53,12 +54,13 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef let computeVisibleTagCount = () => { // Refs can be null at runtime. let currDomRef: HTMLDivElement | null = domRef.current; - if (!currDomRef) { + let currContainerRef: HTMLDivElement | null = containerRef.current; + if (!currDomRef || !currContainerRef) { return; } let tags = [...currDomRef.children]; - let button = currDomRef.parentElement.querySelector('button'); + let button = currContainerRef.querySelector('button'); let currY = -Infinity; let rowCount = 0; let index = 0; @@ -82,7 +84,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef // Remove tags until there is space for the collapse button on the last row. let buttonWidth = button.getBoundingClientRect().width; let end = direction === 'ltr' ? 'right' : 'left'; - let containerEnd = currDomRef.getBoundingClientRect()[end]; + let containerEnd = currContainerRef.getBoundingClientRect()[end]; let lastTagEnd = tags[index - 1]?.getBoundingClientRect()[end]; let availableWidth = containerEnd - lastTagEnd; for (let tagWidth of tagWidths.reverse()) { @@ -103,9 +105,9 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef yield computeVisibleTagCount(); }); } - }, [direction, domRef, state.collection.size, maxRows, setTagState]); + }, [maxRows, setTagState, domRef, direction, state.collection.size]); - useResizeObserver({ref: domRef, onResize: updateVisibleTagCount}); + useResizeObserver({ref: containerRef, onResize: updateVisibleTagCount}); // eslint-disable-next-line react-hooks/exhaustive-deps useLayoutEffect(updateVisibleTagCount, [children]); @@ -124,6 +126,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef return (
(props: SpectrumTagGroupProps, ref: DOMRef {isCollapsed ? `Show all (${state.collection.size})` : 'Show less '} From f248c6e7ed20bfc84a8b8bd776d87997306d3c55 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 12 Jan 2023 12:29:02 -0600 Subject: [PATCH 39/42] add translation string --- packages/@react-spectrum/tag/intl/en-US.json | 4 ++++ packages/@react-spectrum/tag/src/TagGroup.tsx | 7 +++++-- 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 packages/@react-spectrum/tag/intl/en-US.json diff --git a/packages/@react-spectrum/tag/intl/en-US.json b/packages/@react-spectrum/tag/intl/en-US.json new file mode 100644 index 00000000000..872c357ed6e --- /dev/null +++ b/packages/@react-spectrum/tag/intl/en-US.json @@ -0,0 +1,4 @@ +{ + "showAllButtonLabel": "Show all ({tagCount})", + "hideButtonLabel": "Show less" +} \ No newline at end of file diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 0d81339795a..714cef10f38 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -15,12 +15,14 @@ import {AriaTagGroupProps, TagKeyboardDelegate, useTagGroup} from '@react-aria/t import {classNames, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {DOMRef, StyleProps} from '@react-types/shared'; import {FocusScope} from '@react-aria/focus'; +// @ts-ignore +import intlMessages from '../intl/*.json'; import {ListCollection} from '@react-stately/list'; import React, {ReactElement, useCallback, useMemo, useRef, useState} from 'react'; import styles from '@adobe/spectrum-css-temp/components/tags/vars.css'; import {Tag} from './Tag'; import {useLayoutEffect, useResizeObserver, useValueEffect} from '@react-aria/utils'; -import {useLocale} from '@react-aria/i18n'; +import {useLocale, useLocalizedStringFormatter} from '@react-aria/i18n'; import {useProviderProps} from '@react-spectrum/provider'; import {useTagGroupState} from '@react-stately/tag'; @@ -39,6 +41,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef let containerRef = useRef(null); let {styleProps} = useStyleProps(otherProps); let {direction} = useLocale(); + let stringFormatter = useLocalizedStringFormatter(intlMessages); let [isCollapsed, setIsCollapsed] = useState(maxRows != null); let state = useTagGroupState(props); let [tagState, setTagState] = useValueEffect({visibleTagCount: state.collection.size, showCollapseButton: false}); @@ -152,7 +155,7 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef onPress={handlePressCollapse} UNSAFE_style={{display: 'inline'}} UNSAFE_className={classNames(styles, 'spectrum-Tags-actionButton')}> - {isCollapsed ? `Show all (${state.collection.size})` : 'Show less '} + {isCollapsed ? stringFormatter.format('showAllButtonLabel', {tagCount: state.collection.size}) : stringFormatter.format('hideButtonLabel')} }
From df30f447bae50a8928cea4a506982c0b1f23ae7f Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 12 Jan 2023 12:29:54 -0600 Subject: [PATCH 40/42] remove extraneous style --- packages/@react-spectrum/tag/src/TagGroup.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index 714cef10f38..dcfbf300048 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -153,7 +153,6 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef {isCollapsed ? stringFormatter.format('showAllButtonLabel', {tagCount: state.collection.size}) : stringFormatter.format('hideButtonLabel')} From 1c14b1be755b5a52f9bb6d8138a8da19cbc51c91 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 12 Jan 2023 12:43:26 -0600 Subject: [PATCH 41/42] fix actionbutton style --- packages/@adobe/spectrum-css-temp/components/tags/index.css | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@adobe/spectrum-css-temp/components/tags/index.css b/packages/@adobe/spectrum-css-temp/components/tags/index.css index 1bab2430a37..40a43edd662 100644 --- a/packages/@adobe/spectrum-css-temp/components/tags/index.css +++ b/packages/@adobe/spectrum-css-temp/components/tags/index.css @@ -91,6 +91,7 @@ governing permissions and limitations under the License. } .spectrum-Tags-actionButton { + display: inline; height: var(--spectrum-tag-height); font-size: var(--spectrum-tag-text-size); margin: var(--spectrum-tag-margin); From e8829a8ddafb52122a9b00a9a37727a55c07c495 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Thu, 12 Jan 2023 13:55:19 -0600 Subject: [PATCH 42/42] recalculate after fonts load --- packages/@react-spectrum/tag/src/TagGroup.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tag/src/TagGroup.tsx b/packages/@react-spectrum/tag/src/TagGroup.tsx index dcfbf300048..3e6ae36102f 100644 --- a/packages/@react-spectrum/tag/src/TagGroup.tsx +++ b/packages/@react-spectrum/tag/src/TagGroup.tsx @@ -18,7 +18,7 @@ import {FocusScope} from '@react-aria/focus'; // @ts-ignore import intlMessages from '../intl/*.json'; import {ListCollection} from '@react-stately/list'; -import React, {ReactElement, useCallback, useMemo, useRef, useState} from 'react'; +import React, {ReactElement, useCallback, useEffect, useMemo, useRef, useState} from 'react'; import styles from '@adobe/spectrum-css-temp/components/tags/vars.css'; import {Tag} from './Tag'; import {useLayoutEffect, useResizeObserver, useValueEffect} from '@react-aria/utils'; @@ -115,6 +115,12 @@ function TagGroup(props: SpectrumTagGroupProps, ref: DOMRef // eslint-disable-next-line react-hooks/exhaustive-deps useLayoutEffect(updateVisibleTagCount, [children]); + useEffect(() => { + // Recalculate visible tags when fonts are loaded. + document.fonts?.ready.then(() => updateVisibleTagCount()); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + let visibleTags = [...state.collection]; if (maxRows != null && isCollapsed) { visibleTags = visibleTags.slice(0, tagState.visibleTagCount);