-
Notifications
You must be signed in to change notification settings - Fork 63
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
Feature/accordion molecule #985
Conversation
✔️ Deploy Preview for storeui ready! 🔨 Explore the source changes: 0cf6e86 🔍 Inspect the deploy log: https://app.netlify.com/sites/storeui/deploys/61685024598a9600075446f7 😎 Browse the preview: https://deploy-preview-985--storeui.netlify.app |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 0cf6e86:
|
20cd7ad
to
0b1fd18
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job 👏 . I have a suggestion: instead of using number[]
to type indices
, I think we should use Iterable<number>
. By doing this, we can use Sets to make all operations O(1). We would also accept arrays that we could internally convert to Sets: new Set(indices)
. What do you think?
Great suggestion! I'll make the necessary changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only some punctuations 😆 but LGTM, really great job!
2d66491
to
82c5b3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, @lariciamota! This PR is excellent 👏 🚀
I left some comments.
82c5b3c
to
257b080
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!! 👏🏻
I just left a naming suggestion :)
d44b6c2
to
7dbbfdf
Compare
What's the purpose of this pull request?
Implement the Accordion molecule
How it works?
It exports these components: Accordion, AccordionItem, AccordionButton, and AccordionPanel.
In Accordion's context you'll have access to which panels should be opened, the function that should be triggered when opening/closing panels and the number of accordion items.
In AccordionItem's context you'll have access to its index and its button/panel ids.
The structure is:
How to test it?
References
https://www.w3.org/TR/wai-aria-practices-1.2/#accordion