-
Notifications
You must be signed in to change notification settings - Fork 21
feat(component): add raw slider #6435
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
base: slider-component-utils
Are you sure you want to change the base?
Conversation
|
|
Related Previews |
fe09fda to
a6960b5
Compare
a6960b5 to
e9fbcc3
Compare
e9fbcc3 to
bd09654
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw in the specs of the ticket mentions of ticks within a datalist. Do you plan on adding the examples and CSS in another PR?
| } | ||
|
|
||
| constructor(node: Node | EventTarget, host: HTMLElement, orientation: Orientation) { | ||
| if (!isThumb(node)) throw Error('An active thumb must be an HTML element with a slider role.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The constructor is only called onPointerDown of the div with the role="slider", therefore isThumb(node) should always be true so I'm not sure it's worth checking here.
| @Watch('value') | ||
| validateValue() { | ||
| if (this.range) { | ||
| checkEmptyOrArrayOf(this, 'value', 'number'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a more precise check that there is 2 elements within the array?
| * The greatest value in the range of permitted values. | ||
| */ | ||
| @Prop() max = 100; | ||
| @Watch('max') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part of the specs is not verified:
min, max, and step must be numbers. min must be lower than max. step must be lower than max - min.
Same goes for the value: it is not checked whether the defined value is between the min and max
The result is a broken slider if I try to set wrong data but I get no errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I try to fill in some fields to test it, it breaks the slider in the demo.
I think defining default args for the different values should fix it.
https://github.com/user-attachments/assets/ff0f5525-5996-4362-93ac-64e1ea7729ef
|
Also, last bug I found. Sometimes I can't move the slider and I get the 🚫 icon that appears. It seems to be random and happens pretty rarely though and I always manage to slide it again when I select the slider again. |
I tried the |



📄 Description
This PR add a slider component to our raw components.
🚀 Demo
https://preview-6435--swisspost-design-system-next.netlify.app/?path=/docs/d6bc3b88-050b-4ed7-af0d-c01d22a2605a--docs
🔮 Design review
📝 Checklist