Fix for the issue of selectively disabling start or end date

[dart-wakar]

#593
The above mentioned issue is fixed by introducing a new prop into the DateRangePicker component: selectivelyDisable. selectivelyDisable is a string proptype,which is none by default,but if defined as 'startDate' in DateRangePickerWrapper it disables the start-date and so is the case with end-date.

[ljharb]

instead of a string, let's use PropTypes.oneOf(['none', 'startDate', 'endDate']), and let's have that be imported from the same shared module.

[timhwang21]

@ljharb what do you think about instead providing two boolean props, disableStart and disableEnd?

It might make the logic a little easier for users. Rather than

let selectivelyDisabled;

if (condition1) {
  selectivelyDisabled = 'startDate';
}
if (condition2) {
  selectivelyDisabled = 'endDate';
}
<DatePicker selectivelyDisabled={selectivelyDisabled}/>

it could be

<DatePicker disableStart={condition1} disableEnd={condition2}/>

Would also simplify the other code a bit as well.

I guess a potential gotcha would be that behavior of <DatePicker disableStart disableEnd/> would probably need to be identical to <DatePicker disabled/>.

[ljharb]

Generally we've been moving in the direction, for > 2-state props, of having a single enum prop instead of multiple boolean props. (With your suggestion, we'd want to specify a mutuallyExclusiveTrueProp propType to prevent both from being true anyways)

[dart-wakar]

I have made the changes as suggested.Have a look

[coveralls]

Coverage increased (+0.02%) to 86.15% when pulling 43174a7 on dart-wakar:selectiveDisable into 8eca5d0 on airbnb:master.

[coveralls]

Coverage increased (+0.02%) to 86.15% when pulling 43174a7 on dart-wakar:selectiveDisable into 8eca5d0 on airbnb:master.

[coveralls]

Coverage increased (+0.02%) to 86.15% when pulling b958ad3 on dart-wakar:selectiveDisable into 8eca5d0 on airbnb:master.

[coveralls]

Coverage increased (+0.02%) to 86.15% when pulling 6b975bd on dart-wakar:selectiveDisable into 8eca5d0 on airbnb:master.

[coveralls]

Coverage decreased (-0.03%) to 86.457% when pulling 3680f11 on dart-wakar:selectiveDisable into 8d0c96e on airbnb:master.

[ajwild]

Is there anything I can do to help get this merged? It would be very useful for a project I'm currently working on.

[majapw]

If you'd like to take over this PR @dart-wakar, that would be great.

I think that I'd personally prefer to roll this functionality up into the disabled prop and have it be able to be equal to true, false, 'startDate' or 'endDate'. I'd also like to see the prop type definition use the constants instead of hardcoding the strings.

what do you think @ljharb? i know you had some comments on the original PR.

[ljharb]

I agree on constants; but I’m hesitant to roll up more meaning into an existing prop - but maybe it makes sense here for “disabled”.

If @ajwild or anyone else wants to update this PR, please comment here with a link to your branch and we can pull in the commits and rebase for you (rather than making a new PR).

[ajwild]

@majapw @ljharb I've added a commit to the master branch of my fork: https://github.com/ajwild/react-dates

I had to pass through the disabled property from DateRangePicker to DayPickerRangeController to avoid updating or focusing on a disabled date when clicking on a calendar date.

It's also necessary to provide an isOutsideRange function to get the blocked-out-of-range CSS classes on the calendar, but the functionality still works without this. I think it would be messy trying to do this by updating the default isOutsideRange function.

Let me know if there's anything else required once you've had a chance to review/test it.

[majapw]

Awesome @ajwild I'll pull it into this branch and take a look. Thank you for the work!

Can you clarify a bit about what you mean here? I'm not sure I understand.

It's also necessary to provide an isOutsideRange function to get the blocked-out-of-range CSS classes on the calendar, but the functionality still works without this. I think it would be messy trying to do this by updating the default isOutsideRange function.

[ajwild]

Sorry if I wasn't clear, I was referring to the "greyed-out" effect on the calendar dates.

Here's the code from the stories I added:

  .addWithInfo('disabled start date', () => (
    <DateRangePickerWrapper
      initialStartDate={moment().add(3, 'months')}
      initialEndDate={moment().add(3, 'months').add(10, 'days')}
      disabled="startDate"
      isOutsideRange={day => !isInclusivelyAfterDay(day, moment().add(3, 'months'))}
    />
  ))
  .addWithInfo('disabled end date', () => (
    <DateRangePickerWrapper
      initialStartDate={moment().add(3, 'months')}
      initialEndDate={moment().add(3, 'months').add(10, 'days')}
      disabled="endDate"
      isOutsideRange={day => !isInclusivelyAfterDay(day, moment()) ||
        !isInclusivelyBeforeDay(day, moment().add(3, 'months').add(10, 'days'))}
    />
  ))

A screenshot of the "disabled end date" example without isOutsideRange:

A screenshot of the "disabled end date" example with isOutsideRange:

[majapw]

We could probably pull this out into its own variable

[ajwild]

Should I reference it from "DateRangePickerShape.js" in the three other locations it is used, and leave it as it is (or simplify it) here in the readme?

[majapw]

I cleaned it up! :P But left it in the README for clarity.

[majapw]

Ah @ajwild you are right. I think we want to leave it up to the developer to block out days when disabling if that's the UI they want. I don't think we should make any attempt to mess with it automagically.