Revert "removes check if initial month was set before"

[viktoriyavulfson]

This reverts commit 68e991a.

[viktoriyavulfson]

@ljharb this PR reverts previous changes to initial month logic. Sorry about that, please merge this and I'll investigate a better way to solve my use case

[coveralls]

Coverage decreased (-0.1%) to 86.293% when pulling 478acbc on plangrid:master into c108674 on airbnb:master.

[majapw]

So conceivably, since we want this to happen only on open we could make our check the following:
if (this.props.hidden && !nextProps.hidden) {

However, this means that the initialVisibleMonth will be reset every time the DayPicker is opened and closed unless the parent changes the prop on open or close. Which is feasible, but puts a lot of responsibility and leaves us in this kind of half-controlled state.

What do you think @ljharb?

[ljharb]

Hmm - I think it makes sense for initialVisibleMonth to only matter on mount, or when it transitions from hidden to visible. All other times it would be ignored.

[majapw]

well that's the current behavior. :/

[ljharb]

In that case I'm not sure what @ViktoriyaSavkina's change is meant to accomplish. @ViktoriyaSavkina, could you elaborate?

[majapw]

I mean, I think she explained it pretty thoroughly here @ljharb - #176 (comment)

I was brainstorming alternative solutions given that we had to revert the initial change.

[ljharb]

Right, but in that case, you'd render it hidden, and then change the initialVisibleMonth and render it visible in the new location, no?

[majapw]

The DayPicker is always rendered to the screen (it's not getting mounted and unmounted every time you open it for reasons) which is why this more problematic than it seems. I think that is part of the problem. The other is that then the onus is on the parent to update initialVisibleMonth on close so that it doesn't always reopen to the same month.

[ljharb]

right but doesn't componentWillReceiveProps get called with different props when the calendar is shown in a different location?

[majapw]

So I was brainstorming ideas for how to handle both @ViktoriyaSavkina's usecase while not introducing unnecessary complication to the users of DateRangePicker, and one idea that we came up with was adding a forceUpdate type prop. Basically, if you provide an initialVisibleMonth prop, that would get set exactly once and then the rest of the time the DayPicker would continue relying on internal state. If you needed to reset the visible month for some reason (you have some sort of external data that is changing and you would like the month it opens to to change), you would pass in forceVisibleMonthToUpdate or something similar and this would change the DayPicker view. What do you think?

[viktoriyavulfson]

@majapw @ljharb to build on the idea of forceVisibleMonthToUpdate, do we really need an extra property for this? Maybe we could just check if previous value of initialMonth is different from the next value in componentWillRecieveProps?

componentWillReceiveProps(nextProps) {
    if (this.state.currentMonth && this.state.currentMonth != nextProps.initialVisibleMonth()) {
      this.setState({
        currentMonth: nextProps.initialVisibleMonth(),
      });
    }
  }

This way every time a parent component passes in different value (based on whatever logic) current month for the DayPicker will be updated. Passing the same value in won't make any changes to the initial value of the current month though.

[ljharb]

@ViktoriyaSavkina hm, that's not a bad suggestion.

Overall, I'm beginning to think that the concept of "initial visible month" is problematic overall, because of the "initial" - because a prop is being used to initialize state, only once, and because this component needs to be concerned with the business logic about when that prop is copied to state and when not.

I'm beginning to think it might be better to deprecate/eliminate the initialVisibleMonth prop, and then replace it with a getVisibleMonth function prop - called on every render (with the currently about-to-be displayed month), and when it returns a moment object (including if it just returns the provided moment object), force that month to be visible - otherwise (when it returns a falsy value) do nothing to override the state. The prop would default to x => x.

That way, the parent fully controls when the visible month changes, is initially set, or does not change - and the datepicker doesn't need to be concerned with "initial", or with when the prop should be copied to state.

Thoughts?

[viktoriyavulfson]

I like this idea! This way we can maintain existing behavior for the day picker navigation and customize what we want the initial month to be when getVisibleMonth is called without a parameter