Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Datepicker: Ticket 7362, range checks allow you to go past yearRange, which causes weird glitches #271

Closed
wants to merge 3 commits into from

4 participants

@fracmak

Updated the range tests so you can't navigate past the yearRange which causes weird behavior where the UI where you're actually in a year that's not displayable, so clicking on a year gives you a date you aren't expecting. Unit tests included

@scottgonzalez

Shouldn't _getMinMaxDate() take the year range into account?

@fracmak

Well, I was kind of torn by this, because the documentation for yearRange is as follows: "Note that this option only affects what appears in the drop-down, to restrict which dates may be selected use the minDate and/or maxDate options." But the fact you can navigate left and right past the range of drop downs and end up thinking you're selecting one date and you really are selecting a different date is problematic. The _isInRange() function was a nice middle ground that didn't affect too much code. If you want to include yearRange into the minMaxDate, I'll gladly update the code and fix the documentation.

@scottgonzalez
@fracmak

Oops, sorry about that, slipped into the checkin by accident. Pull request updated

@gnarf
Owner

Might it be a smarter approach to make sure that minDate / maxDate get set to the start/end of the year range in the drop down if they aren't already set? And also make sure that the yearRange doesn't go outside of minDate/maxDate?

@scottgonzalez

Unfortunately any change like this would be a breaking change and therefore can't be done (it wouldn't make sense to make a change that can't go into 1.8.x since datepicker is being rewritten from scratch).

@scottgonzalez

The test fails when run in the full suite. It passes when run on its own (or if it's the first test to run, which happens after a failure).

@fracmak

is this something you want me to look into? or hold off on since it's not going into the 1.8 branch and the datepicker is being rewritten from scratch?

@scottgonzalez

You must've misunderstood my comment. When I said it would be a breaking change, I was replying to @gnarf37's comment about the more logical interaction of options. If you fix the failing unit test, this will land in the 1-8-stable branch.

@fracmak

Ok, the unit test is fixed. This checkin in a little bigger than before because I realized while investigating the problem that I hadn't done the yearRange calculation correctly, so I refactored the other year range code into it's own function that I could call. Ended up simplifying a lot of code. But this has had a minor side effect where if you set yearRange = "", the year range suddenly becomes the current year and you can't navigate anymore. This is how the existing year range calculation works, but this change did break the options:miscellaneous unit test because it was relying on html being generated by a successful call to _isInRange(). I've updated the test so it doesn't fail anymore. Let me know if this isn't intended behavior or if I missed anything.

@mikesherov
Collaborator

Jay, can you rebase this? I know it's very old at this point, but I'm trying to clean up the pull request queue. I got rid of datepicker_tickets.js, so now the test should live in the corresponding file the bug effects (either, methods or options or events). Thanks man!

@mikesherov
Collaborator

@fracmak, I just realized I never pinged you when commenting on this. Ping!

fracmak added some commits
@fracmak fracmak Updated the range tests so you can't navigate past the yearRange whic…
…h causes weird behavior where the UI and what you select become out of sync. Unit tests included
4eeab67
@fracmak fracmak made unit test description better d3f4ad2
@fracmak

Imagine my surprise when rebasing a branch that was over a year old and it seriously merged cleanly except for that one deleted file. I moved the unit test to the minMax unit test. All datepicker tests are passing.

@mikesherov
Collaborator

Thanks @fracmak . Landed in eca5abd

A small note: I had to make a minor edit because JSHint was failing, but this is understandable but when this PR was authoring, we weren't yet JSHinting this file!

@mikesherov mikesherov closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 13, 2012
  1. @fracmak

    Updated the range tests so you can't navigate past the yearRange whic…

    fracmak authored
    …h causes weird behavior where the UI and what you select become out of sync. Unit tests included
  2. @fracmak
  3. @fracmak

    fix whitespace

    fracmak authored
This page is out of date. Refresh to see the latest.
View
8 tests/unit/datepicker/datepicker_options.js
@@ -322,9 +322,10 @@ test('miscellaneous', function() {
});
test('minMax', function() {
- expect( 17 );
+ expect( 19 );
var date,
inp = TestHelpers.datepicker.init('#inp'),
+ dp = $('#ui-datepicker-div'),
lastYear = new Date(2007, 6 - 1, 4),
nextYear = new Date(2009, 6 - 1, 4),
minDate = new Date(2008, 2 - 1, 29),
@@ -404,6 +405,11 @@ test('minMax', function() {
TestHelpers.datepicker.equalsDate(inp.datepicker('getDate'), new Date(2008, 6 - 1, 4), 'Min/max - setDate > min, < max');
inp.datepicker('option', {maxDate: null}).val('01/04/2009').datepicker('option', {minDate: minDate, maxDate: maxDate});
TestHelpers.datepicker.equalsDate(inp.datepicker('getDate'), maxDate, 'Min/max - setDate > max');
+
+ inp.datepicker('option', {yearRange: '-0:+1'}).val('01/01/' + new Date().getFullYear());
+ ok(dp.find(".ui-datepicker-prev").hasClass("ui-state-disabled"), "Year Range Test - previous button disabled at 1/1/minYear");
+ inp.datepicker("setDate", "12/30/" + new Date().getFullYear());
+ ok(dp.find(".ui-datepicker-next").hasClass("ui-state-disabled"), "Year Range Test - next button disabled at 12/30/maxYear");
});
test('setDate', function() {
View
14 ui/jquery.ui.datepicker.js
@@ -1742,8 +1742,20 @@ $.extend(Datepicker.prototype, {
_isInRange: function(inst, date) {
var minDate = this._getMinMaxDate(inst, 'min');
var maxDate = this._getMinMaxDate(inst, 'max');
+ var minYear = null;
+ var maxYear = null;
+ var years = this._get(inst, 'yearRange');
+ if (years){
+ var yearSplit = years.split(':');
+ var currentYear = new Date().getFullYear();
+ minYear = parseInt(yearSplit[0]) + currentYear;
+ maxYear = parseInt(yearSplit[1]) + currentYear;
+ }
+
return ((!minDate || date.getTime() >= minDate.getTime()) &&
- (!maxDate || date.getTime() <= maxDate.getTime()));
+ (!maxDate || date.getTime() <= maxDate.getTime()) &&
+ (!minYear || date.getFullYear() >= minYear) &&
+ (!maxYear || date.getFullYear() <= maxYear));
},
/* Provide the configuration settings for formatting/parsing. */
Something went wrong with that request. Please try again.