-
Notifications
You must be signed in to change notification settings - Fork 995
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
update expandable component #6920
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Hey @mirnawong1 nice refactor here!
I just left one small request adjusting the CSS for the open/closed state on the chevron. Let me know if you have any questions with that.
Re: search box not working - I'm okay with this. Since it's using algolia we have much less control of how it functions when using that vs. in-browser search.
<div id={anchorId} className={`${styles.expandableContainer}`}> | ||
<div className={styles.header} onClick={handleToggleClick}> | ||
<span className={`${styles.toggle} ${isOn ? styles.toggleDown : styles.toggleRight}`}></span> | ||
<details ref={detailsRef} id={anchorId} className={styles.expandableContainer} open={isOpen}> |
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.
Nice work setting the open
attribute programmatically here.
Once small adjustment we need to make is adjusting the CSS for the chevron when ctrl + f searching.
You'll see in the screenshot even though the expandable is open, the chevron still is in the closed state.
We should be able to clean this up by reading the open
attribute within the CSS.
For example,
details[open] .toggle {transform: rotate(225deg);}
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.
oh great catch! thank you so much @john-rock ! ok updated it now and used ::before
since the chevron arrow is rendered through a pseudo-element.
@@ -59,6 +59,10 @@ | |||
margin-right: 8px; | |||
} | |||
|
|||
details[open] :local(.toggle)::before { | |||
transform: rotate(225deg); /* this rotates the chevron down */ |
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.
hey @john-rock , added this to fix the chevron detail. can you please re-review when you have a chance?
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 work @mirnawong1 lgtm!
awesome, thank yoU! |
this pr updates the Expandable component to allow the contents within the expandable to be searchable. changes code from
div
todetails
to allow this and changes/updates css to ensure light and dark mode are supportedinternal slack thread
note this isn't perfect as i can't seem to get it to work when searching via the search box, but it does appear as a result in algolia search. and then it does work and auto expand when you control/cmd f on the page
🚀 Deployment available! Here are the direct links to the updated files: