Fixes bugs with Min/MaxMonth and adds a lot of new tests#26
Merged
Conversation
Passing in any type to Min/MaxMonth except for a month string would: 1. Prevent January (1) of any year from being enabled and in some cases would lead the an entire year being disabled. 2. If the current day of month is greater than 28 and the max/max month would evaluate to February (2) then March (3) would become enabled. These bugs were fixed by switching the internal representation of the Min/Max month values from a JavaScript Date objects to a number corresponding to ( (Year * 12) + (Month - 1) ) which rules out any kind of issues caused by the date of month and results in a smaller minified file.
Adds the Relative month periods test Adds the JavaScript Date objects test Adds the Right to left test
This commit also fixes ans issue were the event was not passed to OnBeforeMenuOpen.
KidSysco
added a commit
that referenced
this pull request
Nov 2, 2015
Fixes bugs with Min/MaxMonth and adds a lot of new tests
Owner
|
I just merged after looking at it on my machine for a bit. Great work once again, I will update the fiddle next. You are the best thing to happen to this plugin in a very long time Benjamin, I thank you for your efforts! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While writing unit tests for the Min/MaxMonth options I discovered and fixed a couple of bugs with those options.
The bugs were a result of storing and calculating the min/max month values using JavaScript Date objects where the unnecessary day of month could cause the calculated date object to end up in the next month.
For example if today is October 31 and the
MinMonthis 1 (+1 month a.k.a November) the calculated date would end up being December 1 because November doesn't have a 31'st day of month.I fixed this and many other day of month related bugs by switching the internal representation of the Min/Max month values from a JavaScript Date objects to a number corresponding to
( (Year * 12) + (Month - 1) )which rules out any kind of issues caused by the day of month and results in a smaller minified file.I've also fixed a bug that caused the
_showYearsmethod to unnecessarily expose two globals.I've prevented most of these common errors by enabling strict mode.
Luckily we can completely prevent polluting the global namespace by checking the 'Check for globals' option in QUnit.
I've added:
I've also fixed some small issues with the README.md and the fiddle.