-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Calendar: fix month jumping WIP #1519
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
Conversation
Fixes multiple-month demo and other-month demo.
Do not use value option to pass in a string but value method.
this.grid.focus(); | ||
} | ||
|
||
this._setActiveDescendant(); | ||
}, | ||
|
||
_needsRefresh: function() { | ||
if ( this.date.month() !== this.viewDate.month() || this.date.year() !== this.viewDate.year() ) { | ||
return !this.grid.find( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this checking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does a basic checking if anything has changed at all (as before, see fnagel@4c2229f#diff-563a5392a566d1a29a68b1b5a37c4cb6L139) and if so, checks if the wanted date is already available within our grid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, but why is this extra check needed? That is, under what circumstance do this.date and
this.viewDate` match, but the day isn't rendered?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what is meant here. The find() part is needed to make sure we do not have the day available due to eachDay changes (like in other-months demo).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Can you add that explanation as a comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
This PR addresses @jzaefferer bug reports regarding the use of multiple-month and other-months demo / option. Some other related issues (with keyboard control) should be fixed now, too. See Datepicker Wiki (ToDo section).
#1316 (comment)
This PR introduces a couple of changes. Mainly removing the selected property from$.date and splitting the $ .date object for ongoing modifications of the current value and the $.date object for the rendering of the grid. The view date object is only updated if we really want to show another month. This split helps fixing issues when dealing with multiple month grids, too.
This is work in progress but I wanted to get some feedback. Not sure if this is a good solution or if I'm missing a simpler way to fix these issues.