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

Add functionality to see previous months for vertical scrollable calendar #1894

Conversation

nkinser
Copy link
Contributor

@nkinser nkinser commented Jan 10, 2020

This PR adds the ability to view months before the current month when the calendar is vertically scrollable. There's a small issue where when you click to view more dates above the calendar shifts down and you see the farthest date in the past rather than staying on the month you were just viewing. I will try to fix this in a followup PR.

Before

react-dates-scroll-before

After

react-dates-scroll-after

@nkinser nkinser added the semver-major: breaking change A non-backwards-compatible change; anything that breaks code - including adding a peerDep. label Jan 10, 2020
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Looks great, pending comments! Perhaps add some stories as well?

src/components/DayPicker.jsx Outdated Show resolved Hide resolved
src/components/DayPickerNavigation.jsx Outdated Show resolved Hide resolved
src/components/DayPickerNavigation.jsx Show resolved Hide resolved
src/components/DayPickerNavigation.jsx Show resolved Hide resolved
src/components/DayPickerNavigation.jsx Show resolved Hide resolved
src/components/DayPickerNavigation.jsx Outdated Show resolved Hide resolved
src/components/DayPickerNavigation.jsx Outdated Show resolved Hide resolved
src/components/DayPickerNavigation.jsx Outdated Show resolved Hide resolved
src/components/DayPickerNavigation.jsx Show resolved Hide resolved
src/components/DayPickerNavigation.jsx Outdated Show resolved Hide resolved
@ljharb
Copy link
Member

ljharb commented Jan 10, 2020

What makes this a breaking change exactly? It seems like a semver-minor but I might be missing something.

@nkinser
Copy link
Contributor Author

nkinser commented Jan 10, 2020

What makes this a breaking change exactly? It seems like a semver-minor but I might be missing something.

@ljharb Actually, I think you're right. I was thinking it was incompatible because it could mess up a user's calendar navigation, but I guess that's not really incompatibility

@nkinser
Copy link
Contributor Author

nkinser commented Jan 10, 2020

@ljharb are there any additional stories you think would be helpful? Since this change doesn't rely on any props from the user I thought change to the existing vertical scrollable stories was enough

@ljharb
Copy link
Member

ljharb commented Jan 10, 2020

oh if it's already accessible in existing stories, then there's def no point in adding more :-D i just saw no story files in the diff :-p

@ljharb ljharb added semver-minor: new stuff Any feature or API addition. and removed semver-major: breaking change A non-backwards-compatible change; anything that breaks code - including adding a peerDep. labels Jan 10, 2020
const newVisibleDays = getVisibleDays(firstPreviousMonth, numberOfMonths, enableOutsideDays, true);

this.setState({
currentMonth: firstPreviousMonth.clone(),
Copy link
Contributor

Choose a reason for hiding this comment

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

@nkinser Is this why the calendar resets to the farthest date in the past? You're setting the currentMonth to the firstPreviousMonth

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kevinthepan no, currentMonth has to be set to firstPreviousMonth in order for the previous months to be rendered. I think the reason it shows the earliest month is because when the past months are rendered they push the rest of the months down. I'm not sure how to get around that though

@nkinser nkinser force-pushed the nk--vertically-scrollable-cal-view-prev-months branch 5 times, most recently from b6eaefa to a4226db Compare January 11, 2020 00:16
@nkinser nkinser force-pushed the nk--vertically-scrollable-cal-view-prev-months branch 9 times, most recently from 4ca51e0 to fab0e1c Compare January 13, 2020 21:23
@nkinser nkinser force-pushed the nk--vertically-scrollable-cal-view-prev-months branch from fab0e1c to a3e76fe Compare January 13, 2020 21:50
@nkinser nkinser merged commit 9ebad7f into react-dates:master Jan 13, 2020
@nkinser nkinser deleted the nk--vertically-scrollable-cal-view-prev-months branch January 13, 2020 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor: new stuff Any feature or API addition.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants