Create FAQ Accordion Component#13840
Conversation
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Currently, using a tailwind accordion code copy with few modifications. |
saurabhburade
requested review from
corwintines,
minimalsm,
pettinarip and
wackerow
as code owners
wackerow
left a comment
wackerow
left a comment
There was a problem hiding this comment.
Thanks so much for the help here @saurabhburade! Just left some questions, requests, etc =)
There was a problem hiding this comment.
accordiOn not accordiAn
Need to fix the file paths here: FaqAccordion/index.tsx
(Same with the story: FaqAccordion/FaqAccordion.stories.tsx)
There was a problem hiding this comment.
But, this also makes me wonder if we should even be calling this "FaqAccordion", since in the DS it's just called FAQ... @pettinarip Any thoughts here? Simply calling this component/FAQ/index.tsx would work for me, and could just swap "FAQ" for "Accordion" in the exported sub-components in this file.
| React.ComponentPropsWithoutRef<typeof AccordionPrimitive.Trigger> | ||
| >(({ className, children, ...props }, ref) => ( | ||
| // p-4 hover:bg-background-highlight | ||
| <AccordionPrimitive.Header className="flex w-full"> |
There was a problem hiding this comment.
Is there a reason we didn't go with the existing AccordionTrigger from tailwind/ui/accordion.tsx, instead of rebuilding from the Radix-UI primitive?
There was a problem hiding this comment.
It's used for grouping styles with MdChevronRight ==> "group-hover:border-primary group-hover:text-primary group-hover:shadow-md group-hover:shadow-primary-low-contrast"
There was a problem hiding this comment.
Screen.Recording.2024-10-22.at.11.14.51.mov
Functioning pretty well, just noting we should handle the width. In the DS we have max-width: 52rem; Ultimately though, the width shouldn't change when open vs closed.
There was a problem hiding this comment.
We can implement max-width: 52rem; or near valuemax-w-4xl ==> 56rem. One thing I notice is, its taking parent width if mentioned and fits it to full width (trigger & content both)
There was a problem hiding this comment.
Screen.Recording.2024-10-23.at.12.38.55.AM.mov
|
This issue is stale because it has been open 30 days with no activity. |
github-actions
Bot
added
the
Status: Stale
Component Changes: - Renamed from FaqAccordion to Faq - Updated to use ui/accordion components instead of direct @radix-ui/react-accordion imports - Removed react-icons dependency - the ChevronNext icon is now handled by the ui/accordion components - Fixed import path from tailwind/ui/accordion to ui/accordion - Simplified component structure while maintaining custom styling Story Changes: - Renamed file from FaqAccordion.stories.tsx to Faq.stories.tsx - Updated all component references to use new names (Faq, FaqContent, FaqItem, FaqTrigger) - Changed story name from FaqAccordionStory to Faq following standard conventions. categorized in Molecules / Display Content / Faq
wackerow
removed
the
Status: Stale
wackerow
left a comment
wackerow
left a comment
There was a problem hiding this comment.
Hey folks, circling back.
Pushed a couple commits to merge with latest dev and update code conventions since this was last touched.
cc: @pettinarip for another set of eyes
wackerow
left a comment
wackerow
left a comment
There was a problem hiding this comment.
Screen.Recording.2025-07-21.at.17.09.18.mov
Polished this up a bit more; should match DS fully. Bringing in, thanks again for getting this going @saurabhburade!
|
Congrats, your important contribution to this open-source project has earned you a GitPOAP! GitPOAP: 2025 Ethereum.org Contributor: Head to gitpoap.io & connect your GitHub account to mint!Keep buidling, keep learning, and let's grow the Ethereum open-source community together 🌱 Learn more about GitPOAPs here. |
2 participants
Description
Create
FaqAccordionComponentRelated Issue