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

[@mantine/core] fix slider drag and hover behavior #4000

Merged

Conversation

jvdsande
Copy link
Contributor

@jvdsande jvdsande commented Apr 4, 2023

Fix some Slider behavior:

  • Remove unneeded stopPropagation on mouseDownCapture on the thumb, making it buggy in Preact
  • Fix marks label behavior: now you can only click on them to set the value, but they do not trigger hover effect or drag
  • Fix area in which drag and hover effect detection is active, to include the whole root without the marks label, by introducing a secondary level container "trackContainer" handling the events

@rtivital
Copy link
Member

rtivital commented Apr 6, 2023

@jvdsande focus styles are incorrect, focus ring should appear only when user navigates with keyboard. In storybook it appears when slider is dragged. It works correctly only after mark was clicked (not sure why).

2023-04-06.13.43.51.mov

@jvdsande
Copy link
Contributor Author

jvdsande commented Apr 6, 2023

Hm, I missed that while testing. I'll look into it today!

@jvdsande jvdsande force-pushed the feat/slider-drag-and-hover-behavior branch from f495622 to e488323 Compare April 6, 2023 21:32
@jvdsande
Copy link
Contributor Author

jvdsande commented Apr 6, 2023

@rtivital I've found the issue and fixed it

@rtivital rtivital merged commit 8bee2e5 into mantinedev:master Apr 7, 2023
1 check passed
@rtivital
Copy link
Member

rtivital commented Apr 7, 2023

Thanks!

@rtivital
Copy link
Member

@jvdsande after this PR, slider does not respect container width, can you send a PR with a fix?
Снимок экрана 2023-04-15 в 20 13 55

You can compare it with v5:

@jvdsande
Copy link
Contributor Author

I'll look into it tonight, but as far as I know it was already like that in other V6, I'll see what I can do

rtivital added a commit that referenced this pull request Apr 15, 2023
@rtivital
Copy link
Member

I've reverted this PR commit in a separate branch and the issue is gone – https://github.com/mantinedev/mantine/tree/revert-8bee2e5
So the regression was introduced in this PR.

I've already migrated slider in v7 and faced the same issue. I wasn't able to make mark clicks work the same way as they work now while keeping click behavior, so not really sure if that is possible. I've just disabled pointer-events on marks to remove confusing behavior.

@jvdsande
Copy link
Contributor Author

I actually remember adding a weird negative margin to fix this the other way around. I think it might have fixed it in storybook specifically and broke it everywhere else. When I'm home I'll find the proper way to fix this in all situation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants