The interface ValueLabelProps used in ValueLabelComponent in Slider only exposes these fields:
export interface ValueLabelProps extends React.HTMLAttributes<HTMLSpanElement> {
value: number;
open: boolean;
children: React.ReactElement;
}
However, there are many other props that could be used as we can see here:
<ValueLabelComponent
key={index}
valueLabelFormat={valueLabelFormat}
valueLabelDisplay={valueLabelDisplay}
className={classes.valueLabel}
value={...}
index={index}
open={...}
disabled={disabled}
>
For example, I need to use index to have different Tooltips per index.
This is a request to extend ValueLabelProps to include more fields (at least index, other could be beneficial too)
TypeScript error Property 'index' does not exist on type 'ValueLabelProps'
No TypeScript error when using index in a ValueLabelComponent
https://codesandbox.io/s/elated-johnson-ecn0b?file=/src/Demo.tsx
Steps:
Demo.tsx fileJust as I how in the CodeSandbox, I am trying to have a different ValueLabelComponent for each index (handler) of the range Slider
| Tech | Version |
| ----------- | ------- |
| Material-UI | 4.9.4 |
| React | ^16.12.0 |
| TypeScript | ^3.9.3 |
@CarlosGines Sounds great, I can track the prop from the initial pull request, I think I added it for this use case in mind.
What do you think about this diff?
diff --git a/packages/material-ui/src/Slider/Slider.d.ts b/packages/material-ui/src/Slider/Slider.d.ts
index 94ce09146..3b6b5fcae 100644
--- a/packages/material-ui/src/Slider/Slider.d.ts
+++ b/packages/material-ui/src/Slider/Slider.d.ts
@@ -7,9 +7,10 @@ export interface Mark {
}
export interface ValueLabelProps extends React.HTMLAttributes<HTMLSpanElement> {
- value: number;
- open: boolean;
children: React.ReactElement;
+ index: number;
+ open: boolean;
+ value: number;
}
export interface SliderTypeMap<P = {}, D extends React.ElementType = 'span'> {
diff --git a/packages/material-ui/src/Slider/Slider.js b/packages/material-ui/src/Slider/Slider.js
index 1abb49565..31cf036bb 100644
--- a/packages/material-ui/src/Slider/Slider.js
+++ b/packages/material-ui/src/Slider/Slider.js
@@ -743,8 +743,6 @@ const Slider = React.forwardRef(function Slider(props, ref) {
return (
<ValueLabelComponent
key={index}
- valueLabelFormat={valueLabelFormat}
- valueLabelDisplay={valueLabelDisplay}
className={classes.valueLabel}
value={
typeof valueLabelFormat === 'function'
Do you want to work on a pull request? :) I would also keep #17905 in mind.
@oliviertassinari I was just doing a PR for this and wanted to clarify that about your reference to #17905. And the changes you have listed for Slider.js
Let's wait feedback and give priority to the author of the issue.
Hey folks! Thank you for the quick reply.
Adding index to ValueLabelProps would be the minimum for me. Then, now that we are here, wouldn't it make sense to add the rest of props to? disabled, className... it would be just matching the types with the actual implementation.
Regarding valueLabelFormat and valueLabelDisplay from #17905, I am not sure how you want to proceed. My take would be, if you keep passing them them to ValueLabelComponent, then add them to ValueLabelProps too!
I can write a PR if you want to, and it is more than ok for me if some of you wants to create it!
Then, now that we are here, wouldn't it make sense to add the rest of props to?
It should already be supported by the extension of the HTML attribute.
My take would be, if you keep passing them them to ValueLabelComponent, then add them to ValueLabelProps too!
We don't need them, let's drop them.
I can write a PR if you want to, and it is more than ok for me if some of you wants to create it!
Please do 馃檹
@oliviertassinari This is my first PR, I think I can do it. Can I claim this issue?
Most helpful comment
@oliviertassinari This is my first PR, I think I can do it. Can I claim this issue?