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: month gets skipped when the current date is the 31st and the user clicks next month button #862

Merged
merged 2 commits into from Jun 18, 2019

Conversation

mikerodonnell89
Copy link
Member

@mikerodonnell89 mikerodonnell89 commented Jun 4, 2019

fixes #855

Fixed bug where if the 31st day of the month is selected and the user clicks the next month button, and the next month has fewer than 31 days, the next month will be skipped and the one after that will be displayed. So for example if Jan 31 is selected and the next month button is clicked, March will be displayed.

@netlify
Copy link

netlify bot commented Jun 4, 2019

Deploy preview for fundamental-ngx ready!

Built with commit 2c256d2

https://deploy-preview-862--fundamental-ngx.netlify.com

// get the current date of the month
const currentDate = this.date.getDate();
// get the number of days in the new month
const daysInNewMonth = new Date(this.date.getFullYear(), month + 1, 0).getDate();
Copy link
Contributor

Choose a reason for hiding this comment

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

if this is december, will the Date object automatically go to January of next year?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

new Date(2019, 12, 1) = Wed Jan 01 2020 00:00:00 GMT-0700 (Mountain Standard Time)

in the event that the currentDate is 31 and the next month has 30 days
*/
if (currentDate > daysInNewMonth) {
this.date.setDate(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this go to the next month?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it sets the date for the current month to 1, but sets it back later

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm not sure how friendly this for ux

Copy link
Member Author

Choose a reason for hiding this comment

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

This is all happening internally, not with the actual ngModel the user sees

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I know, but so if the user is on the 31st and clicks next, it goes to date(1) of the same month that was currently displayed? Or the next month?

Copy link
Member Author

Choose a reason for hiding this comment

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

The month that is currently displayed, but I don't think this really matters because it reverts back a few lines later

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you describe the PR in the description? And also exactly the flow of the bug? Like for instance, if the user presses the next month button, this and this happens.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a little description. The original ticket is linked in the description with a gif demonstration

@droshev droshev added this to the sprint 13 - Hot Pie milestone Jun 14, 2019
@MattL75
Copy link
Contributor

MattL75 commented Jun 17, 2019

Why does it skip February if you press next in January 31st?

@mikerodonnell89
Copy link
Member Author

mikerodonnell89 commented Jun 17, 2019

Why does it skip February if you press next in January 31st?

It's just the way dates in JS work... if date===jan 31 and you say date.setMonth(feb) it'll skip over Feb and go straight to March. Because there is no Feb 31.

You can easily test this bug on master, and the big fix on this branch, by setting a specific selectedDay.date in the calendar-single-example.component.ts example code

https://stackoverflow.com/questions/23976513/javascript-setmonth-shows-improper-date

@MattL75
Copy link
Contributor

MattL75 commented Jun 18, 2019

Yeah but I'm saying instead of doing that, why not set it to Feb 1st?

@mikerodonnell89 mikerodonnell89 merged commit 24d7b9f into master Jun 18, 2019
@droshev droshev deleted the bug/#855-calendar-next-month-click branch June 18, 2019 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
board
  
Awaiting triage
Development

Successfully merging this pull request may close these issues.

Bug: Calendar component next month click
3 participants