ui icon indicating copy to clipboard operation
ui copied to clipboard

feat(slider): vertical orientation support for slider with example in docs

Open rishabhbizzle opened this issue 1 year ago • 9 comments

This PR:

  1. Add Vertical orientation support for the Slider component.
  2. Add a Vertical Slider example in Documentation.

Fixes: #2186

rishabhbizzle avatar Dec 26 '23 16:12 rishabhbizzle

@rishabhbizzle is attempting to deploy a commit to the shadcn-pro Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Dec 26 '23 16:12 vercel[bot]

@shadcn please look into this PR. It would be nice to have this feature available for use.

rishabhbizzle avatar Jan 03 '24 06:01 rishabhbizzle

image I seem to get this when using the vertical orientation. The range portion doesn't cover the background fully

I checked it and it's working fine please re-check your code with the example I added in docs.

dark mode:

image

light mode:

image

rishabhbizzle avatar Jan 08 '24 11:01 rishabhbizzle

Please review this PR @shadcn

rishabhbizzle avatar Jan 08 '24 11:01 rishabhbizzle

interesting. inspecting the element seems to show this for the range span element image

Adding right-0 left-0 to

 <SliderPrimitive.Range
        className={cn(
          "absolute h-full bg-primary",
          orientation === "vertical" ? "w-full right-0 left-0" : "h-full"
        )}
      />

fixed it for me. I'm curious if the misaligned span element is there for you as well.

image I seem to get this when using the vertical orientation. The range portion doesn't cover the background fully

I checked it and it's working fine please re-check your code with the example I added in docs.

dark mode:

image light mode: image

jp-hedilog avatar Jan 08 '24 16:01 jp-hedilog

Nope.. the span element is correctly aligned for me @jp-hedilog

image

rishabhbizzle avatar Jan 08 '24 18:01 rishabhbizzle

@rishabhbizzle , thank you for this PR.

alamenai avatar Jan 14 '24 02:01 alamenai

any reason this is not merged yet?

sonyarianto avatar Jan 20 '24 12:01 sonyarianto

any reason this is not merged yet?

@shadcn 👀

rishabhbizzle avatar Jan 20 '24 20:01 rishabhbizzle

I think a better approach would be to use radix/primitive exposed data-orientation attribute to differentiate the horizontal and vertical case directly using tailwindcss data-* modifier. And another small nit is better to use w-2 following what the original shadcn slider (for default, and for new-york is w-1.5 does instead of hard code the w-[20px] there. I have another implementation here in PR can take a look: https://github.com/shadcn-ui/ui/pull/2586

xmliszt avatar Jan 26 '24 19:01 xmliszt