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

[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]

Hmm...that seems like an odd design, but the patch makes sense based on
that.

Why was one of the tests commented out?

On Wed, May 18, 2011 at 11:45 AM, fracmak <
reply@reply.github.com>wrote:

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.

Reply to this email directly or view it on GitHub:
#271 (comment)

[fracmak]

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

[gnarf]

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]

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]

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

[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]

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!