huntabyte / shadcn-svelte

shadcn/ui, but for Svelte. ✨
https://next.shadcn-svelte.com
MIT License
5.37k stars 335 forks source link

toggle icon when sidebar is open to use LeftPanelClose #1422

Closed burggraf closed 3 days ago

burggraf commented 3 days ago

Prerequisites

Describe the feature

I've changed the code of sidebar-trigger.svelte so that:

Here's the new code (since I don't have access to create a PR):

<script lang="ts">
    import { Button } from "$lib/components/ui/button/index.js";
    import { cn } from "$lib/utils.js";
    import PanelLeft from "lucide-svelte/icons/panel-left";
    import PanelLeftClose from "lucide-svelte/icons/panel-left-close";
    import type { ComponentProps } from "svelte";
    import { useSidebar } from "./context.svelte.js";

    let {
        ref = $bindable(null),
        class: className,
        onclick,
        ...restProps
    }: ComponentProps<typeof Button> & {
        onclick?: (e: MouseEvent) => void;
    } = $props();

    const sidebar = useSidebar();
    let isOpen = $derived(sidebar.open);
</script>

<Button
    type="button"
    onclick={(e) => {
        onclick?.(e);
        sidebar.toggle();
    }}
    data-sidebar="trigger"
    variant="ghost"
    size="icon"
    class={cn("h-7 w-7", className)}
    aria-expanded={isOpen}
    {...restProps}
>
    {#if isOpen}
        <PanelLeftClose />
    {:else}
        <PanelLeft />
    {/if}
    <span class="sr-only">Toggle Sidebar</span>
</Button>
huntabyte commented 3 days ago

This feature already exists in shadcn/ui - if not, it won't be considered here so don't continue with your issue.

Where does this exist currently in shadcn/ui? I try to avoid adding things that aren't in the original as we're trying to maintain consistency in this port. If it gets approved/merged there, I'm more than happy to have the same here 😁

burggraf commented 3 days ago

Ah, I didn't look to see if it's in shadcn/ui -- it just made perfect sense from a UX POV. I guess I misunderstood that checkbox :(

huntabyte commented 3 days ago

No worries at all @burggraf! Perhaps open the PR over there first and if it lands we can add it here as well! Until then going to close this issue 😁