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

feat(slider): add vertical slider support and update docs #2586

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

xmliszt
Copy link

@xmliszt xmliszt commented Jan 26, 2024

This PR adds vertical slider support for the <Slider> component. It closes #2186

Intuition

radix/primitive slider comes with both "horizontal" and "vertical" support, it injects the orientation prop as data-orientation attribute to the primitive components. Hence, I think it is straight-forward and simpler for us to just tap on the data-orientation attribute to change our component classes accordingly using Tailwind's built-in data-* modifier (see: https://tailwindcss.com/docs/hover-focus-and-other-states#attribute-selectors )

Changes

  • The previous slider demo is refactored into slider-horizontal-demo and I created a slider-vertical-demo
  • Slider component is updated in both default and new-york to support vertical orientation
  • We follow the original slider implementation, that is, the slider itself does not hard-code its width / height, but it should be following its parent container. (e.g. a horizontal slider has w-full, or a vertical slider has h-full) So it is parent's container's responsibility to determine the width of the horizontal slider, or the height of the vertical slider.
  • Updated docs/component/slider.mdx to showcase both horizontal and vertical slider sample codes, as well as explaining the usage of setting optional orientation="vertical" to create a vertical slider. Also added examples to our sink

CleanShot 2024-01-27 at 03 28 11

Demo

shadcn slider demo

video demo link:
https://github.com/shadcn-ui/ui/assets/44829092/dd3f5e3f-b37a-4ac1-8b4a-6bf213c69a51

Copy link

vercel bot commented Jan 26, 2024

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

A member of the Team first needs to authorize it.

@jogamod
Copy link

jogamod commented Feb 4, 2024

Can you merge this please? @shadcn

@DenizUgur
Copy link

DenizUgur commented Mar 11, 2024

kindly bumping @shadcn

@xmliszt
Copy link
Author

xmliszt commented Mar 11, 2024

Rebased to resolve conflicts

@xmliszt xmliszt changed the title feat(slider): add vertical slider and update docs feat(slider): add vertical slider support and update docs Mar 11, 2024
@magoz
Copy link

magoz commented Mar 22, 2024

Bump

Copy link

@ruru-m07 ruru-m07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work 👍

@xmliszt
Copy link
Author

xmliszt commented Apr 15, 2024

Resolved conflicts & comments. Updated sink to have a better use case of vertical slider like this shown below (maybe like music app equalizer)

CleanShot 2024-04-15 at 15 43 32@2x

@DenizUgur
Copy link

A volume slider on a custom video player might be a use case as well.

@xmliszt
Copy link
Author

xmliszt commented Apr 15, 2024

Thanks for approval. Anyone knows how to kickstart the workflows? Seems like it still needs authorization to proceed. 🙏🏻

@xmliszt
Copy link
Author

xmliszt commented May 16, 2024

Hi @shadcn , may I kindly ask for your review and approval to run the deployment pipeline please? 🙏🏻 Thank you so much!

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

Successfully merging this pull request may close these issues.

Bug(Slider): Slider in vertical orientation does not work
6 participants