Material-ui: Missing fields in interface `ValueLabelProps` in `Slider`

Created on 23 Jul 2020  路  6Comments  路  Source: mui-org/material-ui

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)

  • [x] The issue is present in the latest release.
  • [x] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 馃槸

TypeScript error Property 'index' does not exist on type 'ValueLabelProps'

Expected Behavior 馃

No TypeScript error when using index in a ValueLabelComponent

Steps to Reproduce 馃暪

https://codesandbox.io/s/elated-johnson-ecn0b?file=/src/Demo.tsx

Steps:

  1. Open the CodeSandbox
  2. Check the TS error on the Demo.tsx file

Context 馃敠

Just as I how in the CodeSandbox, I am trying to have a different ValueLabelComponent for each index (handler) of the range Slider

Your Environment 馃寧

| Tech | Version |
| ----------- | ------- |
| Material-UI | 4.9.4 |
| React | ^16.12.0 |
| TypeScript | ^3.9.3 |

Slider good first issue

Most helpful comment

@oliviertassinari This is my first PR, I think I can do it. Can I claim this issue?

All 6 comments

@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?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ericraffin picture ericraffin  路  3Comments

chris-hinds picture chris-hinds  路  3Comments

sys13 picture sys13  路  3Comments

ghost picture ghost  路  3Comments

newoga picture newoga  路  3Comments