TagGroup: fix action being hidden

[reidbarber]

Fixes the case where

• maxRows and an action were provided
• there was enough room to show all tags within maxRows, but not enough for the action button to fit on the last line

Since we detected that all tags were shown, we wrongly assumed we could stop removing tags. We still need to remove some tags in order to show the action button on the last line.

✅ 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:

Test 'with action and maxRows' story. Resize container and make sure action button never gets hidden.

Test other maxRows stories.

🧢 Your Project:

[rspbot]

Build successful! 🎉

• View the storybook
• View the storybook-17
• View the storybook-16
• View the documentation

[snowystinger]

maybe a little too far in the other direction? I can do this now with maxRows=2
I don't think I'm supposed to see the show less button?

[reidbarber]

maybe a little too far in the other direction? I can do this now with maxRows=2

@snowystinger I'm not able to get into that state. What is the browser and repro steps?

[snowystinger]

Ah, ok, I see what is happening. I got a little confused. I went to that story, then clicked "Show All" which should rightfully extend beyond the maxRows. However, where it gets weird is that if I expand the container, then eventually everything shows on two rows and the "Show less" button gets hidden. So I was seeing that button appear and disappear even though I hadn't altered the props or interacted with the button again. I'm not sure if it's right or wrong. If it was shown when everything fits, then clicking that button wouldn't do anything anyways.

I think it's probably fine as it is because of that last point.

[yihuiliao]

not sure if this is an issue but with the action + maxRows story, if i don't have it fully expanded with all the tags displayed, there is a specific width where the action button gets hidden

[rspbot]

Build successful! 🎉

• View the storybook
• View the storybook-17
• View the storybook-16
• View the documentation

[reidbarber]

not sure if this is an issue but with the action + maxRows story, if i don't have it fully expanded with all the tags displayed, there is a specific width where the action button gets hidden

@yihuiliao Good catch! Looks like in the buttons width calculation, I forgot to multiply the margin values we're adding by the number of buttons.

I also noticed that since buttons have the same margin as tags in our css, I can use the static values we have instead of calling window.getComputedStyle.

[rspbot]

Build successful! 🎉

• View the storybook
• View the storybook-17
• View the storybook-16
• View the documentation

[ktabors]

The math now seems too generous/strict. The first image shows all the items and no buttons, but I decrease the width by just a little and the show/hide button shows up when the last tag would still fit, see second image. They are from "maxRows + onRemove".

[rspbot]

Build successful! 🎉

• View the storybook
• View the storybook-17
• View the storybook-16
• View the documentation

[snowystinger]

None of the buttons we need access to are hidden, and in general it's working well. There is still one thing I'm able to do. In the with-action-and-max-rows story, I can get the Show All button to disappear when it should be wrapping and not showing. However, I think it's pretty minor.

[LFDanLu]

I was able to get it into the following state with maxRows=3 in the maxRows story:
but it happens at a very specific width and layouts properly if resized a little bit more.

I'm fine with the current state since the actions seems to appear/disappear appropriately

[rspbot]

Build successful! 🎉

• View the storybook
• View the storybook-17
• View the storybook-16
• View the documentation

[rspbot]

Build successful! 🎉

• View the storybook
• View the storybook-17
• View the storybook-16
• View the documentation

[rspbot]

## 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' }

[ktabors]

LGTM!