Drawer design migration #1646
Drawer design migration #1646
Conversation
3em
commented
May 20, 2025
•
3em
commented
- migrated component to the new design
- API spec
- resolved animation issue with nested drawers
- documented Drawer usage with Portal
- corrected the Drawer component implementation
3em
requested review from
abhinavrastogi-harness,
knagurski and
vardanbansal-harness
as code owners
✅ Deploy Preview for harness-xd-review ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for harness-design ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| @@ -65,7 +65,7 @@ const Logo: FC<LogoProps> = ({ name, size = 24, original = true, ...props }) => | |||
| <svg | |||
| width={size} | |||
| height={size} | |||
| viewBox="0 0 24 24" | |||
| viewBox={`0 0 ${size} ${size}`} | |||
There was a problem hiding this comment.
Why do we want to change the view box like this? This doesn't change the size of the logo, only the vector canvas it's drawn on. Changing the view box without changing the vectors that are being drawn can cause the logo to be drawn small up in the top left corner (when the view box is larger than the expected size) or render outside of visible area (when the view box is smaller than the expected size). There is no connection between viewBox and width/height in SVG
There was a problem hiding this comment.
This is what happens when size is set to 48
| type DrawerHeaderBaseProps = Omit<HTMLAttributes<HTMLDivElement>, 'children'> & { | ||
| children: ReactNode | ||
| } | ||
|
|
||
| type DrawerHeaderIconOnlyProps = { | ||
| icon: IconProps['name'] | ||
| logo?: never | ||
| } | ||
|
|
||
| type DrawerHeaderLogoOnlyProps = { | ||
| logo: LogoProps['name'] | ||
| icon?: never | ||
| } | ||
|
|
||
| type DrawerHeaderNoIconOrLogoProps = { | ||
| icon?: never | ||
| logo?: never | ||
| } | ||
|
|
||
| export type DrawerHeaderProps = DrawerHeaderBaseProps & | ||
| (DrawerHeaderIconOnlyProps | DrawerHeaderLogoOnlyProps | DrawerHeaderNoIconOrLogoProps) |
There was a problem hiding this comment.
This stuff is a pain in the ass with props on a component. Much easier with arguments on a standard function. Makes a big difference to the dev using the component though 👍🏻
| return ( | ||
| <DrawerContext.Provider value={{ direction, nested }}> | ||
| <RootComponent {...rootProps}> | ||
| {nested && FakeTrigger} |
There was a problem hiding this comment.
What if they use a real trigger?
| {isCreate && ( | ||
| <Button variant="outline" onClick={() => onBack?.()}> | ||
| Back | ||
| </Button> | ||
| )} |
There was a problem hiding this comment.
Probably not much use in showing a back button if it won't do anything.
| {isCreate && ( | |
| <Button variant="outline" onClick={() => onBack?.()}> | |
| Back | |
| </Button> | |
| )} | |
| {isCreate && onBack && ( | |
| <Button variant="outline" onClick={onBack}> | |
| Back | |
| </Button> | |
| )} |
| </Button> | ||
| </div> | ||
| {!!editStepIntention && ( | ||
| <Button |
There was a problem hiding this comment.
Looks like this component will be "button" to a screen reader. Can we add aria-label?
| </Button> | ||
| </ButtonGroup> | ||
| {!!editStageIntention && ( | ||
| <Button |
There was a problem hiding this comment.
Same here. Not part of this PR's changes, so not a blocker, but worth doing if you're in the area.
| @@ -4,57 +4,192 @@ description: Drawer component with stacking | |||
| beta: true | |||
| --- | |||
|
|
|||
| The Drawer component works similar to a Sheet component, but allows animated stacking as well. | |||
| The Drawer component provides a way to display content in a panel that slides in from the edge of the screen. It is built on top of the Vaul library and supports multiple configuration options, including direction (left, right, top, bottom), size constraints, and nested drawers for building complex, multi-level interfaces. | |||
There was a problem hiding this comment.
| The Drawer component provides a way to display content in a panel that slides in from the edge of the screen. It is built on top of the Vaul library and supports multiple configuration options, including direction (left, right, top, bottom), size constraints, and nested drawers for building complex, multi-level interfaces. | |
| The `Drawer` component provides a way to display content in a panel that slides in from the edge of the screen. It is built on top of the Vaul library and supports multiple configuration options, including direction (left, right, top, bottom), size constraints, and nested drawers for building complex, multi-level interfaces. |
There was a problem hiding this comment.
Not sure we need to call out that it's built on top of Vaul. It won't mean anything to the majority of our audience and is just an implementation detail.
| The Drawer component works similar to a Sheet component, but allows animated stacking as well. | ||
| The Drawer component provides a way to display content in a panel that slides in from the edge of the screen. It is built on top of the Vaul library and supports multiple configuration options, including direction (left, right, top, bottom), size constraints, and nested drawers for building complex, multi-level interfaces. | ||
|
|
||
| It is composed of several subcomponents such as Drawer.Root, Drawer.Trigger, Drawer.Content, Drawer.Header, Drawer.Tagline, Drawer.Title, Drawer.Description, Drawer.Body, Drawer.Footer and Drawer.Close to offer a structured and customizable interface. |
There was a problem hiding this comment.
| It is composed of several subcomponents such as Drawer.Root, Drawer.Trigger, Drawer.Content, Drawer.Header, Drawer.Tagline, Drawer.Title, Drawer.Description, Drawer.Body, Drawer.Footer and Drawer.Close to offer a structured and customizable interface. | |
| It is composed of several subcomponents such as `Drawer.Root`, `Drawer.Trigger`, `Drawer.Content`, `Drawer.Header`, `Drawer.Tagline`, `Drawer.Title`, `Drawer.Description`, `Drawer.Body`, `Drawer.Footer` and `Drawer.Close` to offer a structured and customizable interface. |
| </Drawer.Content> | ||
| </Drawer.Root>`} | ||
| code={` | ||
| <Drawer.Root direction="left"> |
There was a problem hiding this comment.
Best to keep the main examples in the typical right alignment.
| <Drawer.Root direction="left"> | |
| <Drawer.Root> |
| </Drawer.Root>`} | ||
| code={` | ||
| <Drawer.Root direction="left"> | ||
| <Drawer.Trigger> |
There was a problem hiding this comment.
This will render as a button inside a button if we don't use asChild. Can we either use asChild, or remove the inner button?
| <Drawer.Trigger> | |
| <Drawer.Trigger asChild> |
|
|
||
| ## Anatomy | ||
|
|
||
| All parts of the `Alert` component can be imported and composed as required. |
There was a problem hiding this comment.
| All parts of the `Alert` component can be imported and composed as required. | |
| All parts of the `Drawer` component can be imported and composed as required. |
| ```typescript jsx | ||
| <Drawer.Root> | ||
| <Drawer.Trigger /> |
There was a problem hiding this comment.
We only need to use 4-space indent in the ExampleComponent blocks.
| ```typescript jsx | |
| <Drawer.Root> | |
| <Drawer.Trigger /> | |
| ```typescript jsx | |
| <Drawer.Root> | |
| <Drawer.Trigger /> |
|
|
||
| Contains `Drawer.Trigger` and `Drawer.Content` components. | ||
|
|
||
| It follows the [Vaul API](https://vaul.emilkowal.ski/api#root). |
There was a problem hiding this comment.
Instead of redirecting the viewer to another website, let's add what they need to know here. The fact that we use Vaul under the hood is an implementation detail we don't need to publicise.
| `} | ||
| /> | ||
|
|
||
| <DocsPage.ComponentExample |
There was a problem hiding this comment.
Can we take this example and create a new section below "Anatomy", perhaps called "Nested drawers" to explain the concept and implementation. Would be nice to have the same for size and direction too.
|
|
||
| An optional button that opens the drawer. | ||
|
|
||
| It follows the [Vaul API](https://vaul.emilkowal.ski/api#trigger). |
There was a problem hiding this comment.
Let's remove all links out to Vaul's docs and instead document things here.
|
|
||
| ### `Root` | ||
|
|
||
| Contains `Drawer.Trigger` and `Drawer.Content` components. |
There was a problem hiding this comment.
Would be nice to link these to the appropriate part of the doc.
