implement Slider component #38
Conversation
|
|
||
| const valueString = value!.toString(); | ||
|
|
||
| return ( |
There was a problem hiding this comment.
it might be worth adding some console.warn here to alert the developer as well?
There was a problem hiding this comment.
leaving out for now since this validator is called on every value change; don't want to pollute the console too much
| * Type alias for a predicate function. | ||
| */ | ||
| export type Validator = (value: string | number | null | undefined) => boolean; | ||
|
|
There was a problem hiding this comment.
Maybe adding a validator so that if value is an array it has length of 1-2?
There was a problem hiding this comment.
addressed validation via component wrappers
| multi?: boolean; | ||
| onChange?: (value: number | number[]) => void; | ||
| step?: number; | ||
| value?: number | number[]; |
There was a problem hiding this comment.
is there ever a case where value as an array will have one value instead of two?
There was a problem hiding this comment.
it shouldn't, but it'll probably happen at some point. I'll add some validation
There was a problem hiding this comment.
added SingleValueSlider and MultiValueSlider variants which enforce type restrictions; Slider now exists as BaseSlider and can still be used in the same way, but we should advocate for using one of the strongly-typed variants whenever possible
src/components/Slider/Slider.tsx
Outdated
| max: number; | ||
| maxLabel?: ReactNode; | ||
| multi?: boolean; | ||
| onChange?: (value: number | number[]) => void; |
There was a problem hiding this comment.
I'm not sure I've been consistent with this - I've been working under the assumption that these components are always "controlled". Do we want to support cases for uncontrolled?
There was a problem hiding this comment.
I think assuming everything is controlled is fine for now, I'm not aware of any cases where we need uncontrolled behavior.
I'm guessing we'll want it at some point, so I'm adding uncontrolled whenever I feel motivated to do the work. Not a priority right now though
Rationale
Implement a
Slidercomponent which supports:Changes
Sliderand supporting componentsTesting
storybook,
npm run build && npm run test