Feature / RTL Support #454

sag1v · 2017-04-18T14:32:50Z

I've added support for RTL languages.
The consumer should supply a prop isRTL : bool.
I've also added example stories to see it in action (on both horizontal and vertical mode ).
I haven't created tests for it yet as i'm not that familiar with those kind of tests, it will be nice to get help with those tests if you think is needed.

ljharb · 2017-04-18T20:44:10Z

css/DayPicker.scss

@@ -56,6 +56,12 @@
  }
 }

+.DayPicker__week-header--rtl {
+  ul{


needs a space before {

ljharb · 2017-04-18T20:44:23Z

css/DayPickerNavigation.scss

@@ -42,10 +42,20 @@
    left: 22px;
  }

+  .DayPickerNavigation__prev--rtl{ // sag1v rtl


space before {

ljharb · 2017-04-18T20:44:27Z

css/DayPickerNavigation.scss

  .DayPickerNavigation__next {
    right: 22px;
  }

+  .DayPickerNavigation__next--rtl{ // sag1v rtl


space before {

ljharb · 2017-04-18T20:44:39Z

css/DayPickerNavigation.scss

@@ -42,10 +42,20 @@
    left: 22px;
  }

+  .DayPickerNavigation__prev--rtl{ // sag1v rtl
+      left: auto;
+      right:22px


space after :; missing a semicolon

ljharb · 2017-04-18T20:44:42Z

css/DayPickerNavigation.scss

  .DayPickerNavigation__next {
    right: 22px;
  }

+  .DayPickerNavigation__next--rtl{ // sag1v rtl
+    right: auto;
+    left:22px


space after :; missing a semicolon

ljharb · 2017-04-18T20:46:24Z

src/components/DateRangePicker.jsx

@@ -195,14 +196,17 @@ export default class DateRangePicker extends React.Component {
      withPortal,
      withFullScreenPortal,
      anchorDirection,
+      isRTL // sag1v rtl


missing a trailing comma

ljharb · 2017-04-18T20:46:31Z

src/components/DateRangePicker.jsx

@@ -86,6 +86,7 @@ const defaultProps = {
  displayFormat: () => moment.localeData().longDateFormat('L'),
  monthFormat: 'MMMM YYYY',
  phrases: DateRangePickerPhrases,
+  isRTL:false, // sag1v rtl


needs a space after :

ljharb · 2017-04-18T20:46:39Z

src/components/DateRangePicker.jsx

    const dayPickerClassName = cx('DateRangePicker__picker', {
      'DateRangePicker__picker--direction-left': anchorDirection === ANCHOR_LEFT,
      'DateRangePicker__picker--direction-right': anchorDirection === ANCHOR_RIGHT,
      'DateRangePicker__picker--horizontal': orientation === HORIZONTAL_ORIENTATION,
      'DateRangePicker__picker--vertical': orientation === VERTICAL_ORIENTATION,
      'DateRangePicker__picker--portal': withPortal || withFullScreenPortal,
      'DateRangePicker__picker--full-screen-portal': withFullScreenPortal,
+      'DateRangePicker__picker--rtl' : isRTL, // sag1v rtl


should not have a space before :

ljharb · 2017-04-18T20:47:45Z

src/components/DayPicker.jsx

@@ -621,7 +631,7 @@ export default class DayPicker extends React.Component {

    return (
      <div
-        className="DayPicker__week-header"
+        className={cx("DayPicker__week-header", { "DayPicker__week-header--rtl": isRTL })} // sag1v rtl


these double quotes need to be single quotes when in a JS context (previously it was in a jsx context, which requires double)

ljharb · 2017-04-18T20:48:55Z

src/components/DayPickerNavigation.jsx

+  return ( // sag1v rtl
+    <div className={navClassNames}>
+      {!isVerticalScrollable && !isRTL ? prevButton() : nextButton()}
+      {!isRTL ? nextButton() : prevButton()}


instead of making these functions that are only executed conditionally, let's use the typical jsx pattern if isRTL && nextButtonElements etc.

but they render in different order

ljharb · 2017-04-18T20:49:34Z

I'm not sure why tests aren't running, but if you rebase latest master and force push i expect them to run.

sag1v · 2017-04-18T21:46:36Z

I'm not sure why tests aren't running, but if you rebase latest master and force push i expect them to run.

@ljharb
I've tried pull (rebase) (i'm working with vs code) and it undo all my latest changes so i had to do a git reset --hard ORIG_HEAD to get it back. as i said, it's my first time using git-hub so I'll need some guidance with this one. :)

ljharb · 2017-04-18T22:15:18Z

@sag1v assuming origin is the remote for your fork, and source is the remove for react-dates, you'd do git checkout feature/RTL-support && git fetch source && git rebase source/master and when that was completed and conflicts resolved, git push origin -f

sag1v · 2017-04-19T07:07:13Z

@ljharb
when i run it i'm getting this:

Your branch is up-to-date with 'origin/feature/RTL-support'.
Already on 'feature/RTL-support'
fatal: 'source' does not appear to be a git repository
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

obviously source is not the correct name. is there a way to know what is the correct name?
i've tried git remote show origin and got this:

* remote origin
  Fetch URL: https://github.com/sag1v/react-dates.git
  Push  URL: https://github.com/sag1v/react-dates.git
  HEAD branch: master
  Remote branches:
    feature/RTL-support            tracked
    gh-pages                       tracked
    implement-with-styles          tracked
    maja-a11y-work                 tracked
    maja-add-calendarday-size-prop tracked
    maja-fix-tweaks-to-a11y-pr     tracked
    master                         tracked
  Local branches configured for 'git pull':
    feature/RTL-support merges with remote feature/RTL-support
    master              merges with remote master
  Local refs configured for 'git push':
    feature/RTL-support pushes to feature/RTL-support (up to date)
    master              pushes to master              (up to date)

ljharb · 2017-04-19T07:21:07Z

@sag1v you need to add one - i think the command is git remote add source https://github.com/airbnb/react-dates.git?

sag1v · 2017-04-19T07:28:14Z

@ljharb ok, after adding the source and running the command again i get this:

Your branch is up-to-date with 'origin/feature/RTL-support'.
Already on 'feature/RTL-support'
From https://github.com/airbnb/react-dates
 * [new branch]      gh-pages              -> source/gh-pages
 * [new branch]      implement-with-styles -> source/implement-with-styles
 * [new branch]      maja-a11y-work        -> source/maja-a11y-work
 * [new branch]      maja-add-calendarday-size-prop -> source/maja-add-calendarday-size-prop
 * [new branch]      maja-prototype-perf-idea -> source/maja-prototype-perf-idea
 * [new branch]      master                -> source/master
First, rewinding head to replay your work on top of it...
Applying: rtl support
.git/rebase-apply/patch:9: trailing whitespace.
.DateRangePicker__picker--rtl{
.git/rebase-apply/patch:167: trailing whitespace.

warning: 2 lines add whitespace errors.
error: failed to open 'src/components/DateRangePicker.jsx': Permission denied
Using index info to reconstruct a base tree...
M       examples/DateRangePickerWrapper.jsx
M       examples/DayPickerRangeControllerWrapper.jsx
M       examples/SingleDatePickerWrapper.jsx
M       src/components/DateRangePicker.jsx
M       src/components/DateRangePickerInput.jsx
M       src/components/DateRangePickerInputController.jsx
M       src/components/DayPicker.jsx
M       src/components/DayPickerNavigation.jsx
M       src/components/DayPickerRangeController.jsx
M       src/shapes/DateRangePickerShape.js
Falling back to patching base and 3-way merge...
Auto-merging src/shapes/DateRangePickerShape.js
Auto-merging src/components/DayPickerRangeController.jsx
CONFLICT (content): Merge conflict in src/components/DayPickerRangeController.jsx
Auto-merging src/components/DayPickerNavigation.jsx
Auto-merging src/components/DayPicker.jsx
Auto-merging src/components/DateRangePickerInputController.jsx
Auto-merging src/components/DateRangePickerInput.jsx
Auto-merging src/components/DateRangePicker.jsx
CONFLICT (content): Merge conflict in src/components/DateRangePicker.jsx
Patch failed at 0001 rtl support
The copy of the patch that failed is found in: .git/rebase-apply/patch

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".

error: Failed to merge in the changes.

ljharb · 2017-04-19T07:29:15Z

I'm not sure about the "permission denied", but that means there's conflicts, and you'll need to manually resolve them.

sag1v · 2017-04-19T08:07:57Z

@ljharb i think i resolved them but it still showing that there are conflicts here

sag1v · 2017-04-19T11:55:32Z

@ljharb does it look ok now?

ljharb · 2017-04-19T16:08:51Z

There's still a merge commit on the end; you'll want to avoid clicking "update branch" on the github UI because it means you need another rebase.

sag1v · 2017-04-19T16:35:02Z

@ljharb seems that i cant merge nore push because i need to merge. Im on a loop lol 😂

ljharb · 2017-04-19T16:37:00Z

You need to fetch from source, rebase locally onto source/master, and then force push. No merging or your local master involved.

sag1v · 2017-04-19T16:41:05Z

@ljharb can you please provide a set of commands to do what you suggested?

ljharb · 2017-04-19T16:41:37Z

#454 (comment) has them.

sag1v · 2017-04-19T16:50:06Z

@ljharb tried it and getting this (i dont know why the difference all of a sudden):

Your branch and 'origin/feature/RTL-support' have diverged,
and have 13 and 19 different commits each, respectively.
  (use "git pull" to merge the remote branch into yours)
Already on 'feature/RTL-support'
Current branch feature/RTL-support is up to date.

sag1v · 2017-04-19T17:06:25Z

@ljharb i think i managed to resolve all conflicts. now when i run the commands you mentioned i get this:

Your branch is up-to-date with 'origin/feature/RTL-support'.
Already on 'feature/RTL-support'
Current branch feature/RTL-support is up to date.

and i executed this as well:
git merge origin/master

sag1v · 2017-04-19T17:13:52Z

hmm tests failing again

coveralls · 2017-04-19T17:34:26Z

Coverage decreased (-0.004%) to 83.957% when pulling 38c7672 on sag1v:feature/RTL-support into 3815516 on airbnb:master.

coveralls · 2017-04-19T18:40:52Z

Coverage decreased (-0.4%) to 83.581% when pulling 5931c00 on sag1v:feature/RTL-support into 3815516 on airbnb:master.

ljharb · 2017-04-19T18:54:26Z

@sag1v the idea is that you never ever ever run git merge, ever - that's the mistake.

ljharb

Looking much better! A few more comments, plus it'll need extensive tests.

ljharb · 2017-04-19T19:15:32Z

src/components/DayPicker.jsx

@@ -37,6 +37,7 @@ const MONTH_PADDING = 23;
 const DAY_PICKER_PADDING = 9;
 const PREV_TRANSITION = 'prev';
 const NEXT_TRANSITION = 'next';
+const RTL_TRANSLATION_VALUE = -295;


where does this number come from?

hmm i was sure i changed that magic number. anyway it should get the negative size of getCalendarMonthWidth().

ljharb · 2017-04-19T19:15:46Z

src/components/DayPicker.jsx

@@ -80,7 +84,7 @@ export const defaultProps = {
  numberOfMonths: 2,
  orientation: HORIZONTAL_ORIENTATION,
  withPortal: false,
-  onOutsideClick() {},
+  onOutsideClick() { },


please revert the addition of a space inside the body of these empty methods, throughout the file

ljharb · 2017-04-19T19:16:59Z

src/components/DayPickerNavigation.jsx

-          {navPrevIcon}
-        </button>
-      )}
+  const prevButton = () => {


i still don't think these should be functions; they should be conditionally rendered inline instead.

ljharb

There's an excessive amount of extra whitespace added in this diff; i've tried to comment on all of them, but please in general ensure that there is never more than one blank line in a row, and that blank lines aren't added arbitrarily.

ljharb · 2017-04-19T20:43:15Z

examples/SingleDatePickerWrapper.jsx

@@ -22,6 +22,10 @@ const propTypes = {
    'focused',
    'onFocusChange',
  ]),
+


there shouldn't be two empty lines next to each other; let's collapse this to one

ljharb · 2017-04-19T20:43:18Z

examples/DateRangePickerWrapper.jsx

@@ -77,6 +80,10 @@ const defaultProps = {
  displayFormat: () => moment.localeData().longDateFormat('L'),
  monthFormat: 'MMMM YYYY',
  phrases: DateRangePickerPhrases,
+


there shouldn't be two empty lines next to each other; let's collapse this to one

ljharb · 2017-04-19T20:43:34Z

examples/SingleDatePickerWrapper.jsx

+
+  isRTL: false,
+
+


there shouldn't be any empty lines inside curly braces; please remove these two

ljharb · 2017-04-19T20:43:49Z

examples/SingleDatePickerWrapper.jsx

@@ -65,6 +69,10 @@ const defaultProps = {
  displayFormat: () => moment.localeData().longDateFormat('L'),
  monthFormat: 'MMMM YYYY',
  phrases: SingleDatePickerPhrases,
+


this line should be removed; properties in object literals aren't padded by blank lines.

ljharb · 2017-04-19T20:44:25Z

src/components/DateRangePicker.jsx

@@ -88,6 +88,9 @@ const defaultProps = {
  displayFormat: () => moment.localeData().longDateFormat('L'),
  monthFormat: 'MMMM YYYY',
  phrases: DateRangePickerPhrases,
+
+  isRTL: false,
+


remove this line

ljharb · 2017-04-19T20:47:36Z

examples/DateRangePickerWrapper.jsx

@@ -25,6 +25,9 @@ const propTypes = {
    'focusedInput',
    'onFocusChange',
  ]),
+


remove this line

ljharb · 2017-04-19T20:47:42Z

examples/DateRangePickerWrapper.jsx

+
+
+  isRTL: false,
+


remove this line

ljharb · 2017-04-19T20:47:46Z

examples/DayPickerRangeControllerWrapper.jsx

+
+
+  isRTL: PropTypes.bool,
+


remove this line

ljharb · 2017-04-19T20:47:56Z

examples/DayPickerRangeControllerWrapper.jsx

@@ -77,6 +81,9 @@ const defaultProps = {

  // internationalization
  monthFormat: 'MMMM YYYY',
+
+  isRTL: false,
+


remove this line

ljharb · 2017-04-19T20:48:01Z

examples/SingleDatePickerWrapper.jsx

+
+
+  isRTL: PropTypes.bool,
+


remove this line

ljharb · 2017-04-19T21:07:18Z

src/components/DayPicker.jsx

+    const convertedTranslationValueRTL = (this.dayPickerWidth + this.state.translationValueRTL);
+    let translationValue = this.props.isRTL ? convertedTranslationValueRTL : -this.dayPickerWidth;
+
+    if(this.isVertical()){


always a space after if, always a space between ) and {.

ljharb · 2017-04-19T21:07:23Z

src/components/DayPicker.jsx

+    let translationValue = this.props.isRTL ? convertedTranslationValueRTL : -this.dayPickerWidth;
+
+    if(this.isVertical()){
+      translationValue = this.getMonthHeightByIndex(1) * -1


missing a semicolon

coveralls · 2017-04-19T21:15:57Z

Coverage decreased (-0.6%) to 83.333% when pulling 1293a92 on sag1v:feature/RTL-support into 3815516 on airbnb:master.

coveralls · 2017-04-19T21:37:54Z

Coverage decreased (-0.6%) to 83.333% when pulling 83e7e85 on sag1v:feature/RTL-support into 3815516 on airbnb:master.

coveralls · 2017-04-19T22:22:45Z

Coverage decreased (-0.6%) to 83.333% when pulling 4a15100 on sag1v:feature/RTL-support into 3815516 on airbnb:master.

coveralls · 2017-04-19T22:30:43Z

Coverage decreased (-0.6%) to 83.333% when pulling 4a15100 on sag1v:feature/RTL-support into 3815516 on airbnb:master.

sag1v · 2017-04-24T17:03:13Z

@majapw Hi, any idea when will it be merged to the master? we want to do an npm update on react-dates in our project instead of using my version :)

majapw · 2017-04-27T18:56:25Z

Taking a look at this today! :) Hope to get it merged asap.

sag1v · 2017-04-27T19:00:54Z

Awesome news, Yippee ki-yay! :)

majapw

There are a handful of changes I'd like to see before this is ready, but overall looking good.

majapw · 2017-04-26T16:15:56Z

src/components/DayPicker.jsx

    if (e) e.preventDefault();

    if (this.props.onPrevMonthClick) {
      this.props.onPrevMonthClick(e);
    }

-    const translationValue =
-      this.isVertical() ? this.getMonthHeightByIndex(0) : this.dayPickerWidth;
+    let translationValue = this.isVertical() ? this.getMonthHeightByIndex(0) : this.dayPickerWidth;


Can you make this and the conditional in onNextMonthClick match? I don't really care what pattern you use, but it'd be nice if they were the same.

you mean the extra const in the onNextMonthClick?

More that this function does:

let translationValue = this.isVertical() ? this.getMonthHeightByIndex(0) : this.dayPickerWidth; if (isRTL && this.isHorizontal()) { translationValue = translationValueRTL - this.dayPickerWidth; }

and onNextMonthClick does

const convertedTranslationValueRTL = this.dayPickerWidth + this.state.translationValueRTL; let translationValue = this.props.isRTL ? convertedTranslationValueRTL : -this.dayPickerWidth; if (this.isVertical()) { translationValue = -this.getMonthHeightByIndex(1); }

These two sets of ternaries/conditionals do pretty much the exact same thing, but look totally different. They should look the same. You could accomplish this by changing onNextMonthClick to have:

let translationValue = this.isVertical() ? -this.getMonthHeightByIndex(1) : -this.dayPickerWidth; if (isRTL && this.isHorizontal()) { translationValue = this.dayPickerWidth + this.state.translationValueRTL; }

yeah remember i had to change it because of the "maximum line length of 100" lint rule. it exceeds it by 2 chars.
The difference between this and the other block from onPrevMonthClick is the - chars on this line:
let translationValue = this.isVertical() ? -this.getMonthHeightByIndex(1) : -this.dayPickerWidth;
should i disable this rule for this line only?

you know what forget the rule disabling. i'll just add a new line (i've seen you guys doing it before i think, so i guess that's legit by you)

majapw · 2017-05-01T16:31:59Z

examples/DateRangePickerWrapper.jsx

@@ -17,6 +17,7 @@ const propTypes = {
  autoFocusEndDate: PropTypes.bool,
  initialStartDate: momentPropTypes.momentObj,
  initialEndDate: momentPropTypes.momentObj,
+  isRTL: PropTypes.bool,


I don't think this needs to go here since it probably should be added to DateRangePickerShape

majapw · 2017-05-01T16:32:36Z

examples/SingleDatePickerWrapper.jsx

@@ -15,6 +15,7 @@ const propTypes = {
  // example props for the demo
  autoFocus: PropTypes.bool,
  initialDate: momentPropTypes.momentObj,
+  isRTL: PropTypes.bool,


Same. This should be a part of SingleDatePickerShape so you don't need to add it explicitly to the wrapper proptypes

I think you removed the defaultProp instead of the propType declaration in this file by accident!

majapw · 2017-05-01T16:35:24Z

src/components/CalendarMonth.jsx

@@ -43,6 +43,8 @@ const propTypes = forbidExtraProps({
  // i18n
  monthFormat: PropTypes.string,
  phrases: PropTypes.shape(getPhrasePropTypes(CalendarDayPhrases)),
+
+  isRTL: PropTypes.bool,


This never gets used?

majapw · 2017-05-01T16:37:03Z

src/components/DayPicker.jsx

@@ -516,7 +535,7 @@ export default class DayPicker extends React.Component {
    this.setState({
      currentMonth: newMonth,
      monthTransition: null,
-      translationValue: 0,
+      translationValue: (this.props.isRTL && this.isHorizontal()) ? translationValueRTL : 0,


I don't know that this should be non-zero here even if it is RTL since this is after the transition.

when it's 0 on RTL the transition to the right is super fast and the transition to the left is cluttering a bit (not smooth)

That's ... surprising. Let me look into that.

majapw · 2017-05-01T16:38:05Z

src/shapes/DateRangePickerShape.js

@@ -66,4 +66,6 @@ export default {
  displayFormat: PropTypes.oneOfType([PropTypes.string, PropTypes.func]),
  monthFormat: PropTypes.string,
  phrases: PropTypes.shape(getPhrasePropTypes(DateRangePickerPhrases)),
+
+  isRTL: PropTypes.bool,


Should add this prop to SingleDatePickerShape as well.

majapw · 2017-05-01T16:39:41Z

test/components/DateRangePicker_spec.jsx

@@ -30,6 +30,11 @@ describe('DateRangePicker', () => {
      expect(wrapper.find('.DateRangePicker__picker')).to.have.length(1);
    });

+    it('renders .DateRangePicker__picker--rtl class', () => {


test for SingleDatePicker as well?

sag1v · 2017-05-01T20:29:38Z

@majapw what about adding the isRTL flag to the documentation readme?

majapw · 2017-05-01T20:31:55Z

We probably want that as well! (adding it to the README I mean)

sag1v · 2017-05-01T20:36:30Z

We probably want that as well! (adding it to the README I mean)

@majapw on what section? i was thinking maybe under this section
// calendar presentation and interaction related props

sag1v · 2017-05-01T23:02:47Z

@majapw i think i'm done

ljharb · 2017-05-01T23:25:02Z

@sag1v it'll need another rebase whenever you can too :-)

sag1v · 2017-05-02T15:02:27Z

@ljharb @majapw i did, should be fine now (no conflicts)

coveralls · 2017-05-02T15:14:25Z

Coverage decreased (-0.4%) to 83.527% when pulling 747fc4e on sag1v:feature/RTL-support into 767bebc on airbnb:master.

majapw · 2017-05-02T00:05:23Z

css/SingleDatePicker.scss

@@ -12,6 +12,10 @@
  top: $react-dates-spacing-vertical-picker;
 }

+.SingleDatePicker__picker--rtl{


nit: space after the class name

majapw · 2017-05-02T00:05:39Z

css/SingleDatePickerInput.scss

@@ -4,7 +4,9 @@
  background-color: $react-dates-color-white;
  border: 1px solid $react-dates-color-border;
 }
-
+.SingleDatePickerInput--rtl{


nit: space after the class name

majapw · 2017-05-02T00:05:51Z

css/SingleDatePickerInput.scss

-
+.SingleDatePickerInput--rtl{
+  direction: rtl;
+}


empty line between the class declarations?

majapw · 2017-05-02T00:06:27Z

examples/SingleDatePickerWrapper.jsx

@@ -15,6 +15,7 @@ const propTypes = {
  // example props for the demo
  autoFocus: PropTypes.bool,
  initialDate: momentPropTypes.momentObj,
+  isRTL: PropTypes.bool,


I think you removed the defaultProp instead of the propType declaration in this file by accident!

majapw · 2017-05-02T00:06:55Z

src/components/CalendarMonth.jsx

@@ -63,6 +64,7 @@ const defaultProps = {
  // i18n
  monthFormat: 'MMMM YYYY', // english locale
  phrases: CalendarDayPhrases,
+


Unnecessary line additions. We can probs checkout the master CalendarMonth version to avoid this

majapw · 2017-05-02T00:07:13Z

src/components/CalendarMonthGrid.jsx

@@ -46,6 +46,7 @@ const propTypes = forbidExtraProps({
  // i18n
  monthFormat: PropTypes.string,
  phrases: PropTypes.shape(getPhrasePropTypes(CalendarDayPhrases)),
+


Same here. Let's check out master's CalendarMonthGrid to avoid this diff

majapw · 2017-05-02T15:33:28Z

src/components/DayPicker.jsx

@@ -516,7 +535,7 @@ export default class DayPicker extends React.Component {
    this.setState({
      currentMonth: newMonth,
      monthTransition: null,
-      translationValue: 0,
+      translationValue: (this.props.isRTL && this.isHorizontal()) ? translationValueRTL : 0,


That's ... surprising. Let me look into that.

majapw · 2017-05-02T15:36:05Z

Hi @sag1v, this is looking close to ready to go! I'm going to check out the branch and play around with it a bit. Can you squash down your commits into 1?

sag1v · 2017-05-02T16:20:32Z

@majapw sure, how do i do that?

majapw · 2017-05-02T16:25:12Z

I've helped to address some of my comments! This is pretty much ready to go right now so maybe @ljharb can take a look?

I was also thinking that part of the reason we have to do some weird calculations in the DayPicker (instead of just negative the current translationValues) is that we're still orienting the CalendarMonthGrid along the left instead of the right. If we fix that, the logic in the DayPicker might be a bit cleaner. I'm going to explore that option and see if it's possible... but in general, I plan to get this PR and #449 merged in today (I want to get it in ahead of #450 (comment) since that's a breaking change).

coveralls · 2017-05-02T16:30:48Z

Coverage decreased (-0.5%) to 83.492% when pulling 528f74b on sag1v:feature/RTL-support into 767bebc on airbnb:master.

majapw · 2017-05-02T16:38:45Z

Oh also @sag1v, sorry I just aggressively squashed for you... but in the future you can do:
git rebase -i parent_commit_sha and squash everything but the first commit. In my case, I ended up running git rebase -i HEAD~40 because you had 40 commits. :)

coveralls · 2017-05-02T16:40:46Z

Coverage decreased (-0.5%) to 83.492% when pulling 528f74b on sag1v:feature/RTL-support into 767bebc on airbnb:master.

coveralls · 2017-05-02T16:40:47Z

Coverage decreased (-0.5%) to 83.492% when pulling 528f74b on sag1v:feature/RTL-support into 767bebc on airbnb:master.

ljharb · 2017-05-02T17:42:04Z

src/components/DayPicker.jsx

    this.hasSetInitialVisibleMonth = !props.hidden;
    this.state = {
      currentMonth,
      monthTransition: null,
-      translationValue: 0,


ljharb · 2017-05-02T17:43:08Z

src/components/SingleDatePicker.jsx

@@ -60,6 +60,7 @@ const defaultProps = {
  renderCalendarInfo: null,
  hideKeyboardShortcutsPanel: false,
  daySize: DAY_SIZE,
+  isRTL: false,


where's the propType for this?

It's in SingleDatePickerShape

ljharb · 2017-05-02T17:43:52Z

src/components/DateRangePickerInput.jsx

@@ -52,6 +53,8 @@ const propTypes = forbidExtraProps({

  // i18n
  phrases: PropTypes.shape(getPhrasePropTypes(DateRangePickerInputPhrases)),
+
+  isRTL: PropTypes.bool,


does this still belong here?

Yes! Wait why wouldn't it?

ljharb · 2017-05-02T17:44:36Z

src/components/DateRangePickerInputController.jsx

@@ -58,6 +58,8 @@ const propTypes = forbidExtraProps({

  // i18n
  phrases: PropTypes.shape(getPhrasePropTypes(DateRangePickerInputPhrases)),
+
+  isRTL: PropTypes.bool,


does this still belong here?

The Input needs the isRTL prop to select the correct direction of decorative arrow.

ljharb · 2017-05-02T17:44:42Z

src/components/DayPicker.jsx

@@ -50,6 +50,7 @@ const propTypes = forbidExtraProps({
  renderCalendarInfo: PropTypes.func,
  hideKeyboardShortcutsPanel: PropTypes.bool,
  daySize: nonNegativeInteger,
+  isRTL: PropTypes.bool,


does this still belong here?

ljharb · 2017-05-02T17:44:48Z

src/components/DayPickerNavigation.jsx

@@ -27,6 +27,8 @@ const propTypes = forbidExtraProps({

  // internationalization
  phrases: PropTypes.shape(getPhrasePropTypes(DayPickerNavigationPhrases)),
+
+  isRTL: PropTypes.bool,


does this still belong here?

ljharb · 2017-05-02T17:45:19Z

src/components/DayPickerRangeController.jsx

@@ -66,6 +66,8 @@ const propTypes = forbidExtraProps({
  // i18n
  monthFormat: PropTypes.string,
  phrases: PropTypes.shape(getPhrasePropTypes(DayPickerPhrases)),
+
+  isRTL: PropTypes.bool,


does this still belong here?

ljharb · 2017-05-02T17:51:52Z

src/components/SingleDatePickerInput.jsx

@@ -22,7 +22,7 @@ const propTypes = forbidExtraProps({
  showCaret: PropTypes.bool,
  showClearDate: PropTypes.bool,
  customCloseIcon: PropTypes.node,
-
+  isRTL: PropTypes.bool,


does this still belong here?

sag1v · 2017-05-02T19:47:40Z

Hey i got a happy e-mail about merging and stuff. :)
@ljharb @majapw this was fun and thanks for your help!
when can i update react-dates from npm and get the new features?

majapw · 2017-05-02T19:48:48Z

Thanks for your contribution @sag1v! :D Always great to work with other folks to make the library better. :) I'm doing a release of v10.2.0 right now, so it should be available in about 15 minutes :D

majapw · 2017-05-02T19:56:52Z

All set! v10.2.0 has been released! :)

sag1v · 2017-05-03T05:44:53Z

@ljharb @majapw As this is my first branch (and PR) on GitHub i hope you don't mind that i will keep it as a souvenir and wont delete it. lol

ljharb · 2017-05-03T05:46:42Z

the branch is in your fork, not this repo, so go nuts! :-D

ljharb mentioned this pull request Apr 18, 2017

RTL languages support #431

Closed

ljharb requested changes Apr 18, 2017

View reviewed changes

ljharb added the semver-minor: new stuff Any feature or API addition. label Apr 18, 2017

ljharb requested changes Apr 19, 2017

View reviewed changes

majapw requested changes May 1, 2017

View reviewed changes

majapw requested changes May 2, 2017

View reviewed changes

Adds RTL support to both the DRP and the SDP with a isRTL prop

0d70b06

Address PR comments and clean up translationValue state a little bit

528f74b

majapw approved these changes May 2, 2017

View reviewed changes

ljharb approved these changes May 2, 2017

View reviewed changes

majapw merged commit 61acbcf into react-dates:master May 2, 2017

sag1v mentioned this pull request Oct 16, 2017

Right to left keyboard navigation #774

Closed

Feature / RTL Support #454

Feature / RTL Support #454

Conversation

sag1v commented Apr 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljharb commented Apr 18, 2017

sag1v commented Apr 18, 2017 • edited Loading

ljharb commented Apr 18, 2017

sag1v commented Apr 19, 2017

ljharb commented Apr 19, 2017

sag1v commented Apr 19, 2017

ljharb commented Apr 19, 2017

sag1v commented Apr 19, 2017 • edited Loading

sag1v commented Apr 19, 2017

ljharb commented Apr 19, 2017

sag1v commented Apr 19, 2017

ljharb commented Apr 19, 2017

sag1v commented Apr 19, 2017

ljharb commented Apr 19, 2017

sag1v commented Apr 19, 2017

sag1v commented Apr 19, 2017 • edited Loading

sag1v commented Apr 19, 2017

coveralls commented Apr 19, 2017 • edited Loading

coveralls commented Apr 19, 2017 • edited Loading

ljharb commented Apr 19, 2017

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Apr 19, 2017 • edited Loading

coveralls commented Apr 19, 2017 • edited Loading

coveralls commented Apr 19, 2017 • edited Loading

coveralls commented Apr 19, 2017 • edited Loading

sag1v commented Apr 24, 2017

majapw commented Apr 27, 2017

sag1v commented Apr 27, 2017

majapw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sag1v commented May 1, 2017

majapw commented May 1, 2017

sag1v commented Apr 18, 2017 •

edited

Loading

sag1v commented Apr 19, 2017 •

edited

Loading

sag1v commented Apr 19, 2017 •

edited

Loading

coveralls commented Apr 19, 2017 •

edited

Loading

coveralls commented Apr 19, 2017 •

edited

Loading

coveralls commented Apr 19, 2017 •

edited

Loading

coveralls commented Apr 19, 2017 •

edited

Loading

coveralls commented Apr 19, 2017 •

edited

Loading

coveralls commented Apr 19, 2017 •

edited

Loading

coveralls commented May 2, 2017 •

edited

Loading

coveralls commented May 2, 2017 •

edited

Loading