feat(slider): add new slider component draft version #328
Conversation
There was a problem hiding this comment.
@tsironis13 - Added one comment but this looks great, appreciate the hard work on this!
Add directionality support, basic focus behavior and appearance, and resize behavior.
|
Slider is fully accessibly under the slider WAI aria patterns: |
There was a problem hiding this comment.
LGTM! @tsironis13 we should probably create a separate ticket for e2e tests!
There was a problem hiding this comment.
Pretty nice work!
Just wanted to point to things out.
Styling
The styling is not completely matching the shadcn style.
I just mention it, because I know, there are teams using angular and react in one app and use spartan to match shadcn styles.
spartan
vs.
shadcn
Transitions
To me the transitions feel a little bit clunky when dragging the slider.
@elite-benni the styles are adapted to shadcn ui. In general, I think the separation between styling and functionality makes styles changes really trivial. The idea behind spartan UI primitives is for us as the creators of the components to provide a default style and the consumer of them to adapt the styling according to their needs, isn't it? Or do we need to create initially all the primitives matching the shadcn ui styling? |
|
@tsironis13 the goal is mimic the shadcn styling out of the box and then let people adjust them as they need in their own repositories |
|
@thatsamsonkid @goetzrobin @elite-benni I added 2 last core functional features at least for the first version (step and tick marks). Also the styles match the shadcn ones now, but it is very easy to adapt according to our needs in any case.
I am not going to add any more functionality on this PR unless you would like to do so. |
|
@tsironis13 sounds good. If you want to add e2e tests as part of this ticket feel free to do so! I was just saying that it's not a blocker IMO if we create a new ticket specifically for e2e tests |
Just to be on the same page when we are talking about storybook e2e tests, we are referring to component tests that will be placed under the brain folder(like the select component tests)? |
|
I am talking about adding them here: https://github.com/goetzrobin/spartan/tree/main/apps/ui-storybook-e2e/src/integration |
Ok perfect, I was talking about component tests only. I think, we can implement e2e tests in a separate ticket. I will only add component testing on this PR. Thanks for the clarification! |
|
Awesome @tsironis13! I'll wait for you to mark the PR as ready for review and then we can merge as soon as you're ready :) |
@goetzrobin The PR is ready for review after basic component tests addition. |
* feat(slider): add new slider component draft version * feat(slider): remove non used imports * feat(slider): adapt code to biome linting * feat(slider): adapt code to biome linting * fix(tooltip): when toggleVisibility give nativeElement display:none property (spartan-ng#327) * feat(slider): features addition and code optimization Add directionality support, basic focus behavior and appearance, and resize behavior. * feat(slider): implement full accessible slider according to WAI * feat(slider): add step and tick mark features, adapt styles to shadcn ui * feat(slider): add component tests * feat(slider): remove circular dependencies --------- Co-authored-by: elite-lucas <115133536+elite-lucas@users.noreply.github.com>
PR Checklist
Please check if your PR fulfills the following requirements:
guidelines: https://github.com/goetzrobin/spartan/blob/main/CONTRIBUTING.md#-commit-message-guidelines
PR Type
What kind of change does this PR introduce?
Which package are you modifying?
What is the current behavior?
What is the new behavior?
This is the draft version of the new slider component. Since it is my first contribution to this project any feedback, omitted best practices would be really appreciate it, if pointed out.
It contains the backbone infrastructure of the component where each separate component element has their associated functionality and public API. The brain lib is split into 4 main parts (slider, input, track and thumb directives). The most crucial directive is the input one, which is responsible for interacting with the native input element.
Currently, the slider supports only 4 different states (default, disabled, min and max) with the possibility for more to be added in the official first version.
I have added the related Storybook stories; further tests will be added when this draft PR is approved.
The component supports only updating the UI when the value is changed and needs more polishing in order to handle pointer events, directionality changes and be user accessible.
It would be helpful if you give me some insights what else in terms of functionality is needed for the first version (e.g rtl, step).
Does this PR introduce a breaking change?
Other information