TagGroup: add `maxRows` prop to limit initially shown tags, and include 'Show all' button by reidbarber · Pull Request #3759 · adobe/react-spectrum

reidbarber · 2022-11-16T20:10:18Z

Closes #3660

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Check maxRows stories under TagGroup. Resize container and confirm behavior.

🧢 Your Project:

RSP

adobe-bot · 2022-11-16T20:17:08Z

Build successful! 🎉

adobe-bot · 2022-11-16T21:15:25Z

Build successful! 🎉

adobe-bot · 2022-11-17T16:52:58Z

Build successful! 🎉

adobe-bot · 2022-12-02T20:26:38Z

Build successful! 🎉

adobe-bot · 2022-12-05T18:51:48Z

Build successful! 🎉

adobe-bot · 2022-12-06T16:31:16Z

Build successful! 🎉

adobe-bot · 2022-12-06T17:20:18Z

Build successful! 🎉

snowystinger

We need a FocusScope around the TagGroup. See https://github.com/adobe/react-spectrum/blob/main/packages/%40react-spectrum/actiongroup/src/ActionGroup.tsx#L219 for an example

reidbarber · 2023-01-10T23:03:36Z

Reidbot: https://reactspectrum.blob.core.windows.net/reactspectrum/d3a0f99d0112068bc50a42079996b67eaf4b9b9a/storybook/index.html?path=/story/taggroup--max-rows&providerSwitcher-express=false&providerSwitcher-toastPosition=bottom

devongovett · 2023-01-10T23:10:56Z

Seems like when you make the width bigger, it doesn't display more tags now?

reidbarber · 2023-01-10T23:16:00Z

Seems like when you make the width bigger, it doesn't display more tags now?

@devongovett If you refresh, it behaves correctly. Looking into why it's different on first load....

devongovett · 2023-01-10T23:24:10Z

Hmm I tried refreshing and it still happened for me. Actually when I make the element smaller, it also wraps to show more than 2 lines:

reidbarber · 2023-01-12T00:08:54Z

@devongovett Yeah, looks like the new inline styles I introduced broke the overflow logic. Working on it now.

rspbot · 2023-01-12T20:06:07Z

Build successful! 🎉

reidbarber · 2023-01-12T20:12:50Z

Should be fixed now. One issue was with font loading, and I needed to set the resize observer to watch the new container. Also went ahead and added in translation strings for the button.

devongovett

Working well for me in Safari.

rspbot · 2023-01-12T23:32:34Z

Build successful! 🎉

rspbot · 2023-01-13T00:44:08Z

Build successful! 🎉

rspbot · 2023-01-13T00:46:46Z

Build successful! 🎉

rspbot · 2023-01-13T00:46:48Z

## API Changes

unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }

@react-aria/tag

AriaTagGroupProps

-
+AriaTagGroupProps<T> {
+  keyboardDelegate?: TagKeyboardDelegate<T>
+}

it changed:

useTagGroup

@react-spectrum/tag

Item

 SpectrumTagGroupProps<T> {
-  allowsRemoving?: boolean
-  onRemove?: (Key) => void
+
 }

SpectrumTagGroupProps

+TagGroup<T extends {}> {
 
+}

snowystinger

Working well for me in Chrome and on iPhone Safari

reidbarber added 2 commits November 16, 2022 14:07

add defaultVisibleRows and initial functionality

1a39b79

cleanup

aba67e9

rename to maxRows

31eeeb1

reidbarber changed the title ~~WIP: TagGroup: add defaultVisibleRows prop to limit initially shown tags, and include 'Show all' button~~ WIP: TagGroup: add maxRows prop to limit initially shown tags, and include 'Show all' button Nov 16, 2022

adobe deleted a comment from adobe-bot Nov 16, 2022

reidbarber added 2 commits November 16, 2022 17:30

switch to use quiet ActionButton

2937e02

include collapse button on last row

c93a145

adobe deleted a comment from adobe-bot Nov 17, 2022

reidbarber added 3 commits December 1, 2022 12:10

revert cleanup

5b1d073

Merge branch 'main' into tagroup-show-all

146e8c9

update logic

888562d

reidbarber added 2 commits December 5, 2022 12:17

update to use generator

3a97857

Merge branch 'main' into tagroup-show-all

ae21d32

reidbarber added 3 commits December 6, 2022 10:10

include show all button on last row

4b2ecbe

update story to fill width for resizing

49b75a3

add test

bba7bac

reidbarber changed the title ~~WIP: TagGroup: add maxRows prop to limit initially shown tags, and include 'Show all' button~~ TagGroup: add maxRows prop to limit initially shown tags, and include 'Show all' button Dec 6, 2022

adobe deleted a comment from adobe-bot Dec 6, 2022

cleanup

4639f4d

snowystinger reviewed Dec 6, 2022

View reviewed changes

Comment thread packages/@react-spectrum/tag/test/TagGroup.test.js

snowystinger reviewed Dec 6, 2022

View reviewed changes

Comment thread packages/@react-spectrum/tag/src/TagGroup.tsx Outdated

Comment thread packages/@react-spectrum/tag/src/TagGroup.tsx Outdated

Comment thread packages/@react-spectrum/tag/src/TagGroup.tsx Outdated

reidbarber added 2 commits December 6, 2022 17:37

add FocusScope

af7d23e

improve performance

9e7bd1c

devongovett assigned devongovett and unassigned devongovett Jan 10, 2023

reidbarber added 2 commits January 10, 2023 17:56

Merge branch 'main' into tagroup-show-all

0bb236e

Merge branch 'main' into tagroup-show-all

c05009b

reidbarber added 7 commits January 12, 2023 12:15

fix refs for overflow detection

3096d2c

add translation string

f248c6e

remove extraneous style

df30f44

fix actionbutton style

1c14b1b

Merge branch 'main' into tagroup-show-all

5874296

recalculate after fonts load

e8829a8

Merge branch 'main' into tagroup-show-all

a4c3f5e

devongovett approved these changes Jan 12, 2023

View reviewed changes

Merge branch 'main' into tagroup-show-all

2bcfc57

adobe deleted a comment from adobe-bot Jan 13, 2023

snowystinger approved these changes Jan 13, 2023

View reviewed changes

reidbarber merged commit 1faa59e into main Jan 13, 2023

reidbarber deleted the tagroup-show-all branch January 13, 2023 00:53

reidbarber added the needs testing label Jan 13, 2023

dannify removed the needs testing label Feb 16, 2023

Conversation

reidbarber commented Nov 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

Uh oh!

adobe-bot commented Nov 16, 2022

Uh oh!

adobe-bot commented Nov 16, 2022

Uh oh!

adobe-bot commented Nov 17, 2022

Uh oh!

adobe-bot commented Dec 2, 2022

Uh oh!

adobe-bot commented Dec 5, 2022

Uh oh!

adobe-bot commented Dec 6, 2022

Uh oh!

adobe-bot commented Dec 6, 2022

Uh oh!

snowystinger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

reidbarber commented Jan 10, 2023

Uh oh!

devongovett commented Jan 10, 2023

Uh oh!

reidbarber commented Jan 10, 2023

Uh oh!

devongovett commented Jan 10, 2023

Uh oh!

reidbarber commented Jan 12, 2023

Uh oh!

rspbot commented Jan 12, 2023

Uh oh!

reidbarber commented Jan 12, 2023

Uh oh!

devongovett left a comment

Choose a reason for hiding this comment

Uh oh!

rspbot commented Jan 12, 2023

Uh oh!

rspbot commented Jan 13, 2023

Uh oh!

rspbot commented Jan 13, 2023

Uh oh!

rspbot commented Jan 13, 2023

@react-aria/tag

AriaTagGroupProps

@react-spectrum/tag

Item

SpectrumTagGroupProps

Uh oh!

snowystinger left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

reidbarber commented Nov 16, 2022 •

edited

Loading