Skip to content

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

Merged
merged 4 commits into from
Apr 25, 2015

Conversation

fnagel
Copy link
Member

@fnagel fnagel commented Mar 24, 2015

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).

  • demos/calendar/multiple-months.html: Clicking a date on the second or third month changes the display to show that month as the first month. Only clicking the next/prev buttons should change that.
  • demos/calendar/other-months.html: Very similar problem as the previous one, clicking lead days changes the month

#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.

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(
Copy link
Member

Choose a reason for hiding this comment

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

What is this checking?

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 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.

Copy link
Member

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 andthis.viewDate` match, but the day isn't rendered?

Copy link
Member Author

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).

Copy link
Member

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@fnagel fnagel merged commit a3df6dd into jquery:datepicker Apr 25, 2015
@fnagel fnagel deleted the datepicker-fix-jumping branch October 28, 2016 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants