feat: Create a reusable component for expandable section #2903
Conversation
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: src/services/navigation/useNavLinks/useNavLinks.js
Did you find this useful? React with a 👍 or 👎 |
RulaKhaled
force-pushed
the
expandable-section
branch
from
2ad31bc to
9d0228b
Compare
RulaKhaled
force-pushed
the
expandable-section
branch
from
9d0228b to
189b7ea
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #2903 +/- ##
==========================================
+ Coverage 98.46% 98.48% +0.02%
==========================================
Files 879 882 +3
Lines 13028 13077 +49
Branches 3494 3511 +17
==========================================
+ Hits 12828 12879 +51
+ Misses 196 194 -2
Partials 4 4
... and 9 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found @@ Coverage Diff @@
## main #2903 +/- ##
==========================================
+ Coverage 98.46% 98.48% +0.02%
==========================================
Files 879 882 +3
Lines 13028 13077 +49
Branches 3499 3506 +7
==========================================
+ Hits 12828 12879 +51
+ Misses 196 194 -2
Partials 4 4
... and 9 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #2903 +/- ##
================================================
+ Coverage 98.46000 98.48000 +0.02000
================================================
Files 879 882 +3
Lines 13028 13077 +49
Branches 3429 3441 +12
================================================
+ Hits 12828 12879 +51
+ Misses 196 194 -2
Partials 4 4
... and 9 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #2903 +/- ##
=======================================
Coverage ? 98.48%
=======================================
Files ? 882
Lines ? 13077
Branches ? 3506
=======================================
Hits ? 12879
Misses ? 194
Partials ? 4
Continue to review full report in Codecov by Sentry.
|
Bundle ReportChanges will increase total bundle size by 25.34kB ⬆️
|
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
There was a problem hiding this comment.
This seems very similar to the SummaryDropdown. Are you alright refactoring this to follow that style, as well as using the primitives from Radix? Refactoring to this style will also increase the composability of the component so it's super nice to work with and style, bringing it up to the GV2 style of writing composable components, as well as making it fully accessible.
There was a problem hiding this comment.
There's also a couple of components that Spencer has also done up already in the newer style that you can also reference.
There was a problem hiding this comment.
Card, RadioTileGroup, CopyClipboard, and CodeSnippet are the ones I've done/redone, for reference! :)
There was a problem hiding this comment.
The SummaryDropdown is using the Accordion which may not be best for what we're trying to accomplish unless you're looking to stack a couple of this drop downs together, an alternative if they're going to all be separate or only one would be the Collapsible.
src/ui/ExpandableSection/index.ts
Outdated
There was a problem hiding this comment.
Can we swap this from a default export to a named export similar to this, as this is the kinda style we're switching to make things a little nicer on the import.
| @@ -0,0 +1,32 @@ | |||
| import * as Collapsible from '@radix-ui/react-collapsible' | |||
There was a problem hiding this comment.
You'll probably need to install this as a dependency as with Radix you install the different primitives individually, as they're packaged individually.
| <div className="my-2 border border-gray-200"> | ||
| <Collapsible.Root open={isExpanded} onOpenChange={setIsExpanded}> | ||
| <Collapsible.Trigger asChild> | ||
| <button className="flex w-full items-center justify-between p-4 text-left font-semibold hover:bg-gray-100"> | ||
| <span>{title}</span> | ||
| <Icon name={isExpanded ? 'chevronUp' : 'chevronDown'} size="sm" /> | ||
| </button> | ||
| </Collapsible.Trigger> | ||
| <Collapsible.Content className="border-t border-gray-200 p-4"> | ||
| {children} | ||
| </Collapsible.Content> | ||
| </Collapsible.Root> | ||
| </div> |
There was a problem hiding this comment.
Can you break this up into different components similar to the SummaryDropdown that way it will enable us to compose them together and make any stylistic changes inline.
| @@ -0,0 +1,67 @@ | |||
| import * as Collapsible from '@radix-ui/react-collapsible' | |||
| import cs from 'classnames' | |||
There was a problem hiding this comment.
Should use new cn wrapper instead of cs
There was a problem hiding this comment.
It's a drop in replacement
There was a problem hiding this comment.
Updated. Why did we replace cs though?
There was a problem hiding this comment.
cn is a mix of classnames, and another utility called tailwind-merge, tailwind-merge will remove duplicate and potentially conflicting classnames for us, which has caused some issues beforehand.
| Expandable Section with HTML | ||
| </ExpandableSection.Trigger> | ||
| <ExpandableSection.Content> | ||
| <div> |
There was a problem hiding this comment.
I think ExpandableSection.Content should functionally be a div, so could probably remove this div here. Doesn't really matter though
Description
This was requested by @Adal3n3 as we foresee this being used more in the future, this will be used in several areas in new onboarding for failed tests.
Code Example
Notable Changes
Create new ui component and stories.
Screenshots
Screen.Recording.2024-05-28.at.3.32.17.PM.mov
Link to Sample Entry
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.