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

Fix for initialVisibleMonth in DayPickerRangeController (#613) #617

Merged
merged 2 commits into from
Jul 7, 2017

Conversation

aleib
Copy link
Contributor

@aleib aleib commented Jul 5, 2017

Original issue:
is initialVisibleMonth now required for DayPickerRangeController

Looking at the commit logs I think this commit broke the component when initialVisibleMonth is not set on DayPickerRangeController

By changing the default prop from:
initialVisibleMonth: DayPickerDefaultProps.initialVisibleMonth
to
initialVisibleMonth: null,
as it will pass null down to the DayPicker component on render

So made the prop a function as in DateRangePicker

@aleib aleib changed the title Fix for initialVisibleMonth in DayPickerRangeController #613 Fix for initialVisibleMonth in DayPickerRangeController Jul 5, 2017
@aleib aleib changed the title Fix for initialVisibleMonth in DayPickerRangeController Fix for initialVisibleMonth in DayPickerRangeController (#613) Jul 5, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 86.276% when pulling 6308746 on aleib:al-613-initial-visible-month into c384dc1 on airbnb:master.

Copy link
Collaborator

@majapw majapw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! If you could make the change to the DayPickerSingleDateController as well, that would be awesome. :)

@@ -819,6 +820,8 @@ export default class DayPickerRangeController extends React.Component {
} = this.props;

const { phrases, visibleDays } = this.state;
const initialVisibleMonthThunk =
initialVisibleMonth || (startDate ? () => startDate : () => moment());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! This is definitely a bug (I originally didn't catch is because the wrapper I'm using has an initialVisibleMonth default prop).

@majapw majapw force-pushed the al-613-initial-visible-month branch from 6308746 to 82ffc8f Compare July 7, 2017 22:19
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 86.202% when pulling 82ffc8f on aleib:al-613-initial-visible-month into a05a63f on airbnb:master.

@majapw majapw merged commit c9c0227 into react-dates:master Jul 7, 2017
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