Skip to content

[Accordion] Support multiple values #527

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 3, 2021
Merged

[Accordion] Support multiple values #527

merged 5 commits into from
Mar 3, 2021

Conversation

jjenzz
Copy link
Contributor

@jjenzz jjenzz commented Mar 2, 2021

  • Adds support for multiple values
  • Toggles items by default (clicking button again will close item)
  • To prevent items from closing, consumer now needs to control it
  • Updates tests
  • Updates stories

@jjenzz jjenzz requested a review from benoitgrelard as a code owner March 2, 2021 21:10
@jjenzz jjenzz force-pushed the accordion-multi-values branch 2 times, most recently from 9082dc8 to e199b5f Compare March 2, 2021 21:13
@jjenzz jjenzz force-pushed the accordion-multi-values branch from e199b5f to 5410542 Compare March 2, 2021 21:14
const handleItemClose = React.useCallback(
(itemValue) => setValue((prevValue = []) => prevValue.filter((value) => value !== itemValue)),
[setValue]
);
Copy link
Contributor Author

@jjenzz jjenzz Mar 2, 2021

Choose a reason for hiding this comment

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

I thought this was a nice example of why it's good to have the callback implementation in the component that provides the context. I have the same context API provided by AccordionSingle and AccordionMultiple but they have different handler implementations 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Love that

Copy link
Contributor

@benoitgrelard benoitgrelard left a comment

Choose a reason for hiding this comment

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

This is great! Love the approach!
Left a few minor comments.

const handleItemClose = React.useCallback(
(itemValue) => setValue((prevValue = []) => prevValue.filter((value) => value !== itemValue)),
[setValue]
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Love that

Jenna Smith and others added 2 commits March 3, 2021 12:41
@jjenzz jjenzz merged commit 35de4d6 into main Mar 3, 2021
@jjenzz jjenzz deleted the accordion-multi-values branch March 3, 2021 13:39
benoitgrelard added a commit that referenced this pull request Mar 3, 2021
# New features

Breaking changes are indicated with a 🔥 icon.

- Add support for SSR
- [Accordion] Support multiple values (#527) 🔥
- [Tabs] Add RTL support (`dir` prop) (#497)
- Remove `selector` prop and `data-radix-*` atributes 🔥

# Fixes

- [Slider] Fix step rounding issue (#463)
- [Presence] Improve `Presence` perf (#465)
- Fixed potential issue with overriding attributes in certain primitives

# Maintenance

- Improved internal context handling
- Better package/dependency split
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants