-
Notifications
You must be signed in to change notification settings - Fork 162
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
Fix bug on slider when dragging in rtl
mode
#457
Conversation
Thanks for the PR! So instead of passing a prop users will now need to wrap the entire Carousel Provider in a div with dir=RTL ? |
Typically (99+% of cases?) I would expect See here for example. |
@Conor-Hinchee I'm happy to take your lead on a couple of points here. It's possible the code doesn't need to be quite as defensive as it now is (I added the guards in the face of failing tests and runtime errors). It's also possible there may be a better place to perform this What are your thoughts? |
Hi @Conor-Hinchee, thanks for your work on this project. Are you able to provide a rough timeline of when this PR might be reviewed, merged and released? |
Hi guys, thank you for your work on this! Is this really fixed? it's seem to slide the wrong way on both the |
Actually, this comment was helpful. |
What:
Fix bug on slider when dragging in
rtl
mode. If you drag to the left, the slider animates to the right (and vice-versa). This broken behaviour can be observed on the examples page.See also here and here.
Why:
Dragging on the carousel is broken for
rtl
pages.How:
Rather than require a prop to be passed into the component, the
Slider
component detects whether thedirection
isrtl
and invertsdeltaX
during drag accordingly.Checklist: