(react-slider) - Adding Marks #19427
Conversation
…eat/slider-mark # Conflicts: # packages/react-slider/src/Slider.stories.tsx # packages/react-slider/src/components/Slider/Slider.types.ts # packages/react-slider/src/components/Slider/useSliderState.tsx # packages/react-slider/src/components/Slider/useSliderStyles.ts
…eat/slider-mark # Conflicts: # packages/react-slider/src/components/Slider/Slider.types.ts
…eat/slider-basic-marks # Conflicts: # packages/react-slider/etc/react-slider.api.md # packages/react-slider/src/components/Slider/useSlider.ts # packages/react-slider/src/components/Slider/useSliderState.tsx # packages/react-slider/src/components/Slider/useSliderStyles.ts
| outline: 'none', | ||
| zIndex: '1', | ||
| whiteSpace: 'nowrap', | ||
| '& .ms-Slider-mark': { |
There was a problem hiding this comment.
I'm not seeing ms-Slider-mark applied anywhere? Also it seems like in converged it's more common to have a separate style mark and apply that where needed rather than hardcoding class names.
There was a problem hiding this comment.
It is applied in useSliderState since it goes into a JSX element that is being dynamically generated.
Having a separate mark style, while possible, would require having a makeStyles call in a non use[Component]Styles file, which I don't know if it's something we want either, although I guess it could still be defined and exported here and just imported in the other file.
There was a problem hiding this comment.
Got it. Maybe if the class name could be shared in a const with a comment about why it's needed?
There was a problem hiding this comment.
@czearing would you mind maybe moving that to a const in the other file and importing it here? Or maybe putting the const in here and importing it in useSliderState might be the better thing to do? Either way, you'd have to individually specify the exports from these files in index.ts to ensure you're not exporting something you don't want to.
…eat/slider-basic-marks
…eat/slider-basic-marks # Conflicts: # packages/react-slider/src/components/Slider/Slider.test.tsx
| outline: 'none', | ||
| zIndex: '1', | ||
| whiteSpace: 'nowrap', | ||
| '& .ms-Slider-mark': { |
There was a problem hiding this comment.
It is applied in useSliderState since it goes into a JSX element that is being dynamically generated.
Having a separate mark style, while possible, would require having a makeStyles call in a non use[Component]Styles file, which I don't know if it's something we want either, although I guess it could still be defined and exported here and just imported in the other file.
…eat/slider-basic-marks
| outline: 'none', | ||
| zIndex: '1', | ||
| whiteSpace: 'nowrap', | ||
| '& .ms-Slider-mark': { |
There was a problem hiding this comment.
@czearing would you mind maybe moving that to a const in the other file and importing it here? Or maybe putting the const in here and importing it in useSliderState might be the better thing to do? Either way, you'd have to individually specify the exports from these files in index.ts to ensure you're not exporting something you don't want to.
…eat/slider-basic-marks
…eat/slider-basic-marks # Conflicts: # packages/react-slider/src/components/Slider/Slider.test.tsx # packages/react-slider/src/components/Slider/useSliderState.tsx
Pull request checklist
$ yarn changeDescription of changes
Adding the basic
marksprop to the Slider component.In this pull request the
marksprop accepts:CodeSandbox Demo
This is the full demo of the
marksprop.https://codesandbox.io/s/marked-slider-component-q5bpd?file=/src/App.js