Skip to content
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

[Slider] Allow generic value type to narrow onChange value type #37854

Open
2 tasks done
eloiqs opened this issue Jul 7, 2023 · 4 comments
Open
2 tasks done

[Slider] Allow generic value type to narrow onChange value type #37854

eloiqs opened this issue Jul 7, 2023 · 4 comments
Assignees
Labels
component: slider This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature ready to take Help wanted. Guidance available. There is a high chance the change will be accepted typescript waiting for 👍 Waiting for upvotes

Comments

@eloiqs
Copy link

eloiqs commented Jul 7, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Summary 💡

Giving a number type as value should narrow onChange value param to number, and giving a number[] type as value should narrow onChange value param to number[]

Examples 🌈

<Slider value={1} onChange={(e, value:number) => {}} />
<Slider value={[1,2]} onChange={(e, value:number[]) => {}} />

Motivation 🔦

We currently have to do something like this
<Slider value={1} onChange{(e, value) => typeof value === 'number' ? ... : ...what do i do here? throw an error? } /> to make typescript happy

@eloiqs eloiqs added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 7, 2023
@zannager zannager added the component: slider This is the name of the generic UI component, not the React module! label Jul 7, 2023
@DiegoAndai DiegoAndai added enhancement This is not a bug, nor a new feature typescript and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 10, 2023
@DiegoAndai DiegoAndai self-assigned this Jul 10, 2023
@DiegoAndai
Copy link
Member

Hey @eloiqs, thanks for the report!

A workaround would be using a Type Assertion, for example:

function SliderExample() {
  const [value, setValue] = React.useState(20);

  const handleChange = (event, newValue) => {
    setValue(newValue as number); // or number[] for a range slider
  };

  return (
    <Box sx={{ width: 300 }}>
      <Slider value={value} onChange={handleChange} />
    </Box>
  );
}

Using the as keyword, you manually assure Typescript that newValue is a number, of which you can be sure as the provided value is a number as well.

Implementing a generic to narrow down the callback would be nice though. If you (or anyone) would like to work on a PR, that would be great as well 😊🚀

@DiegoAndai DiegoAndai added waiting for 👍 Waiting for upvotes ready to take Help wanted. Guidance available. There is a high chance the change will be accepted labels Jul 10, 2023
@gitstart
Copy link
Contributor

@DiegoAndai @eloiqs I would like to pick this up

@DiegoAndai
Copy link
Member

@gitstart sure! let me know if you need any help

@good-jinu
Copy link

good-jinu commented Dec 15, 2024

Hi there, I've also encountered this issue, so understand the concern. I've tried to work on a solution.

Here's some modifications to the proposed by @sai6855 . - #38753

Could you please review them?

It has worked on the latest master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: slider This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature ready to take Help wanted. Guidance available. There is a high chance the change will be accepted typescript waiting for 👍 Waiting for upvotes
Projects
None yet
7 participants