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] Show value tooltip when hovering a slider thumb with disableSwap true #30753

Open
2 tasks done
NPetz opened this issue Jan 24, 2022 · 4 comments
Open
2 tasks done
Labels
component: slider This is the name of the generic UI component, not the React module! new feature New feature or request

Comments

@NPetz
Copy link

NPetz commented Jan 24, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Summary 💡

Hovering on a slider thumb should make a tooltip with the thumb's value pop out when valueLabelDisplay is set to auto and disableSwap is set. Just like it does when disableSwap is not set.

Examples 🌈

https://codesandbox.io/s/pensive-dream-w91mx?file=/src/App.js

In this example, the only difference between the two sliders is that disableSwap is set on the second one.
A tooltip showing the value of the thumb should pop out when hovering on the thumb regardless of whether disableSwap is set or not.

Motivation 🔦

I don't think that setting disableSwap should change the behavior of valueLabelDisplay.
It is unexpected and makes disableSwap unusable when you need to maintain the ability to show the thumbs' value on hover.

I thought it was a bug at first, but it seem it may have been the intended behavior (#25547 (review)), so I'm filing a feature request instead.

I could take a stab at it.

@NPetz NPetz added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 24, 2022
@mnajdova
Copy link
Member

I agree with your expectation. I think that if valueLabelDisplay is 'auto' it should show the value label when hovering over the thumb, regardless of the disableSwap value. Maybe we should handle the hover separately. Feel free to start with the changes, I can help with the review.

@mnajdova mnajdova added component: slider This is the name of the generic UI component, not the React module! new feature New feature or request and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 24, 2022
@mnajdova mnajdova changed the title show value tooltip when hovering a slider thumb with disableSwap true [Slider] Show value tooltip when hovering a slider thumb with disableSwap true Jan 24, 2022
@NPetz
Copy link
Author

NPetz commented Jan 24, 2022

@mnajdova

So, I created a draft pull request!

#30761

I'm not so sure what is the intended behavior of the labels.
I made it so that when disabledSwap is set you can only ever see one label at a time. Should this behavior be true at anytime?

Thanks in advance!

@NPetz
Copy link
Author

NPetz commented Feb 2, 2022

@mnajdova

Hello!

I've made another change to the pull request and marked it ready for review.

I would appreciate it if you could check when you have time!

Thanks in advance!

@totszwai
Copy link

Maybe you could also fix the broken disableSwap here too
#32381

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! new feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants