Skip to content
New issue

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

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

Already on GitHub? # to your account

Add Accordion and AccordionItem Components #27

Merged
merged 3 commits into from
Sep 5, 2023

Conversation

silentbaws
Copy link

@silentbaws silentbaws commented Sep 2, 2023

Using functional components, but if you'd like me to refactor with normal components let me know. Also no doc comments as it's not finalized and doesn't include all Accordion functionality. Want to make sure it looks good before finalizing.

Please don't merge as is.

If there's any changes you want made let me know. If not I'll finalize in the same style as it is now with a proper commit message, doc comments, and complete functionality.

silentbaws added 2 commits September 2, 2023 10:06
- Remove `HeadingSize`
- Add `AccordionHeader` private component
- Add `AccordionCollapse` private component
- Change String properties to AttrValue
- Added `stay_open` property to `Accordion`
- Added `flush` property to `Accordion`
@silentbaws
Copy link
Author

Added the version bump and all of the additional functionality(other than styling classes) for Accordion components.

Included doc comments for using the Accordion and AccordionItem components.

There may still be things to do for a fully fledged implementation but this gets most of the basics implemented.

A noteable design decision was made when determining which components and properties were public/private. The thought process here is that ideally we want to abstract away the inner workings of the underlying bootstrap components otherwise users would just write it themselves. So assigning id's to children and attaching them to their parents was done programatically. Additionally users can't create AccordionHeader or AccordionCollapse components as Accordion components only take in AccordionItem children as well as the above consideration.

These decisions may be a hindrance in the future and should then be re-evaluated at that time by future users/editors.

@isosphere isosphere self-requested a review September 4, 2023 13:24
@isosphere isosphere assigned isosphere and unassigned isosphere Sep 4, 2023
@isosphere
Copy link
Owner

Looks good, thank you for your contribution!

I especially appreciate that accessibility recommendations from Bootstrap have been implemented (aria-expanded). I think you've been reasonable re: public/private, we can revisit if/when necessary.

I've added an example to our examples/basics sub-crate; I find similar example pages helpful for getting started with component libraries and like to keep it updated.

I'm ready to merge this code; it's an improvement. Please confirm re:

Please don't merge as is.

@silentbaws
Copy link
Author

I'm ready to merge this code; it's an improvement. Please confirm re:

Please don't merge as is.

yup forgot to remove that line. I’m also happy to merge it with the updated commit

@isosphere isosphere merged commit a8b48fb into isosphere:main Sep 5, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants