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] Value label position if disabled #19329

Closed
2 tasks done
LorenzHenk opened this issue Jan 21, 2020 · 5 comments · Fixed by #19330 or #26257
Closed
2 tasks done

[Slider] Value label position if disabled #19329

LorenzHenk opened this issue Jan 21, 2020 · 5 comments · Fixed by #19330 or #26257
Labels
bug 🐛 Something doesn't work component: slider This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@LorenzHenk
Copy link

If the Slider is disabled, the value label is slightly off to the right.

image

  • 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 😯

The value label for a disabled Slider is positioned slightly to the right.

The problem is at this line: https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/Slider/ValueLabel.js#L23
Removing the line fixed the problem, can you explain why it is needed?

Expected Behavior 🤔

The value label should be centered.

image

Steps to Reproduce 🕹

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

Steps:

  1. Use valueLabelDisplay="on" to make the value label visible all the time
  2. Use disabled={true}

Context 🔦

I want to display the value of a disabled slider.

Your Environment 🌎

Tech Version
Material-UI v4.8.3
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: slider This is the name of the generic UI component, not the React module! labels Jan 21, 2020
@oliviertassinari
Copy link
Member

Removing the line fixed the problem, can you explain why it is needed?

I have no idea. I suspect it originates from iteration on the initial implementation.

@davidfdriscoll
Copy link
Contributor

Sorry to reopen this issue but this problem is back. On a disabled slider the value label is again positioned slightly to the right.

See a code sandbox here. The same steps as above reproduce the problem:

  • valueLabelDisplay="on"
  • disabled={true}

This time I think the problem is at this line. The value label is being centered by a manual calculation of left: 'calc(-50% - 4px)', to fix an IE bug. That manual calculation centers the value label properly in the case of an active thumb (width 12 pixels) but not in the case of a disabled thumb (width 8 pixels).

If interested I could try to put together a PR to fix the issue. It would probably involve applying the disabled pseudo-class to the value label element in addition to the root and thumb and styling accordingly.

@oliviertassinari
Copy link
Member

@davidfdriscoll Thanks for the report. I think that we should remove the IE 11 fix, in the source and in the customization demos and add a visual regression test. This is mainly motivated by https://next.material-ui.com/getting-started/supported-platforms/#ie-11.

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label May 10, 2021
@davidfdriscoll
Copy link
Contributor

davidfdriscoll commented May 10, 2021

That sounds good -- would you mind if I try tackling that and submit a pull request to remove the IE 11 fix and add a test?

@oliviertassinari
Copy link
Member

@davidfdriscoll Please do :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: slider This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
3 participants