Skip to content
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

Drawer component only styled properly when using bottom-opening direction #1273

Open
camball opened this issue Sep 17, 2024 · 1 comment
Open

Comments

@camball
Copy link

camball commented Sep 17, 2024

TL;DR

The direction prop from the vaul-svelte component is honoured in shadcn-svelte's Drawer component, but has some hard-coded styling making any direction other than a bottom-opening drawer unusable.

Motivation

vaul-svelte's Drawer.Root takes a direction prop:

direction: Direction of the drawer. Can be topbottomleft, or right. Defaults to bottom.

When attempting to pass a direction to shadcn-svelte's Drawer.Root

<Drawer.Root direction="top">
  ...
</Drawer.Root>

, the prop is honoured, yet there are fixed default classes that mandate the direction coming from the bottom, making the style appear incorrectly

"bg-background fixed inset-x-0 bottom-0 z-50 mt-24 flex h-auto flex-col rounded-t-[10px] border",

Expected behaviour here is to honour the prop and style the component correctly, as shown in the vaul-svelte website examples.

Solution

If we dynamically apply the proper directional tailwind classes, based on the passed direction, the shadcn-svelte Drawer wrapper will work correctly for any passed direction, out of the box.

Drawer Handle Considerations

One thing that I feel would look wrong is the "drawer handle" being present on left- and right-opening drawers. IMO, from a design perspective, it should only be present on top and bottom drawers.

Proposed PR Implementation

Will need to modify shadcn-svelte's drawer-content.svelte:

  • To use one of {bottom, top, left, right}-0 instead of a fixed bottom-0 class, as well as one of rounded-{b, t, l, r}-[10px] instead of the fixed rounded-t-[10px].
  • Remove the "drawer handle" for left and right, only keeping it for top and bottom. Provide an option to remove the drawer handle entirely as well. Currently, there's an mt-24 class applied (to account for the size of the drawer handle), which would also need to be changed dynamically, based on drawer direction, but since I believe the handle should only be present on the top and bottom drawers, it should only every either be mt-24 or mb-24 (only applied when the drawer is present).
  • See if we can reuse the DrawerDirection type straight from vaul-svelte in the shadcn component.

I'm more than happy to own this issue and open a PR with this proposed implementation if this issue is accepted.

Reproduction

Working Reproduction in StackBlitz: Shadcn-Svelte Drawer Direction Styling Bug


<script lang="ts">
  import { Button } from '$lib/components/ui/button';
  import * as Drawer from '$lib/components/ui/drawer';
</script>

<!-- or substitute `direction` with 'left' or 'right' to observe the wrong styling -->
<Drawer.Root direction="top">
  <Drawer.Trigger>
    <Button>Open Drawer</Button>
  </Drawer.Trigger>
  <Drawer.Content>
    <Drawer.Close class="m-10">Close Drawer</Drawer.Close>
  </Drawer.Content>
</Drawer.Root>

Logs

No response

System Info

System:
  OS: macOS 14.6.1
  CPU: (12) arm64 Apple M3 Pro
  Memory: 68.31 MB / 36.00 GB
  Shell: 5.9 - /bin/zsh
Binaries:
  Node: 20.14.0 - ~/.nvm/versions/node/v20.14.0/bin/node
  npm: 10.8.2 - ~/.nvm/versions/node/v20.14.0/bin/npm
Browsers:
  Chrome: 128.0.6613.138
  Safari: 17.6

Severity

annoyance

@yuriko192
Copy link

yuriko192 commented Dec 18, 2024

hey, not a maintainer here, but i just encountered the same issue as you. my solution is to use drawer solely for bottom direction, while other direction i try and use the Sheet component, hope this might help you too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants