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] marks type is invalid #19335

Closed
2 tasks done
LorenzHenk opened this issue Jan 21, 2020 · 1 comment · Fixed by #19350
Closed
2 tasks done

[Slider] marks type is invalid #19335

LorenzHenk opened this issue Jan 21, 2020 · 1 comment · Fixed by #19350
Labels
component: slider This is the name of the generic UI component, not the React module! ready to take Help wanted. Guidance available. There is a high chance the change will be accepted

Comments

@LorenzHenk
Copy link

The slider prop type and typescript type allows the value false for marks, but using this value will lead to the error "marks.map is not a function".

The problem seems to be here: https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/Slider/Slider.js#L386

Examining this code block, I was able to trigger the same error with the following properties as well: marks={true} step={null} (see here).
marks={true} step={null} is an invalid configuration, so it's ok to throw. But I think it should be checked explicitly and there should be a descriptive error message. Alternatively, the typescript and prop types can be adapted to not allow these two values together, see the expected behavior section for more information.

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

Current Behavior 😯

When supplying the prop marks={false}, the Slider component throws an error.

This also happens with marks={false} step={null}.

Expected Behavior 🤔

I should not be able to set false as value in the first place - it should be prohibited by prop types and typescript.

Alternative solutions:

  • there should be some special behavior with marks={false}
    or
  • it should fall back to the default value ([]).

Steps to Reproduce 🕹

https://codesandbox.io/s/material-demo-kg4fn

Steps:

  1. set marks={false} on the Slider component

Context 🔦

Nothing special, just noticed this problem by accident.

Your Environment 🌎

Tech Version
Material-UI v4.8.3
@oliviertassinari oliviertassinari added component: slider This is the name of the generic UI component, not the React module! ready to take Help wanted. Guidance available. There is a high chance the change will be accepted labels Jan 21, 2020
@oliviertassinari
Copy link
Member

@LorenzHenk Thanks for raising. I have no idea what would be the best fix here. The fix you have proposed sounds good. Maybe somebody else has a preference?

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! ready to take Help wanted. Guidance available. There is a high chance the change will be accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants