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

Days selection fix when using RTL languages #172

Merged
merged 2 commits into from
Nov 12, 2016
Merged

Conversation

DorWeid
Copy link
Contributor

@DorWeid DorWeid commented Sep 15, 2016

When trying to select a range of days ( when in Months view ) and using a RTL language (such as Hebrew) you'll find it buggy. It'll select the opposite days when in the current row and will do some weird stuff when passing between rows. I could not find any open issues or pull requests regarding that matter, so I decided to do it myself. I also added some documentation to the code to help me better understand it while finding a solution (I can take it down if it's a problem).

TL;DR

  • The problem: Wrong selection indexes when using RTL language.
  • Ways to reproduce the bug: Use the i18n example. Edit cultures.js by adding the selectable prop. Run it and select ar-AE.

p.s
This is my first time creating a pull request.

@jquense
Copy link
Owner

jquense commented Sep 15, 2016

thanks for the contribution! I'll take a look at the code, its a bit tough for me to see what's new with the comments but that's ok, just take me a bit longer to read through

@jquense
Copy link
Owner

jquense commented Oct 18, 2016

hey there 👋 sorry for the delay, can you rebase this to fix the conflict?

@DorWeid
Copy link
Contributor Author

DorWeid commented Oct 24, 2016

Hey, I actually found a better solution than the current one, as it applies only to drag selection event, and not single cell selection. I will update soon.

@DorWeid
Copy link
Contributor Author

DorWeid commented Oct 24, 2016

Hey @jquense
Project should be OK now and is up to date with master branch.
Also removed documentation for your convenience :p

@jquense jquense merged commit 5b8c99d into jquense:master Nov 12, 2016
@jquense
Copy link
Owner

jquense commented Nov 12, 2016

thanks a bunch for this! sorry for the delay

@JonathanGilyadov
Copy link

Hey,

Still having this issue now, any idea why?

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.

3 participants