This repository has been archived by the owner on May 29, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
bug #2506 Datepicker displays wrong week numbers #2517
Closed
Closed
Conversation
This file contains 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
The datepicker displays wrong week numbers if the starting-day is 0 (Sunday), because it uses the first of the row to determine the weeknumber of the corresponding row. According to ISO 8601 the week starts on Monday and ends on Sunday. Consequently the datapicker will display e.g. for the week Sun Aug 17 - Sat Aug 23 the number of the week that ended on Sun Aug 17 and not the week that starts on Mon Aug 18. Using the date at middle of the row will display the correct week number regardless of whether the week display starts with a Sunday or a Monday.
fix bug angular-ui#2506 Datepicker displays wrong week numbers. also fixed some white space problems jshint was complaining about.
fix bug angular-ui#2506 Datepicker displays wrong week numbers also fixing the unit test. correct ISO week numbers are here: http://www.epochconverter.com/date-and-time/weeknumbers-by-year.php?year=2010
fix bug angular-ui#2506 Datepicker displays wrong week numbers. also fixed another week number bug: It calculates the ISO 8601 Week Number only for the first row of a month and for the following rows of this month it just increments the week number. This generally works, but it may create wrong week numbers for December and January.
@@ -259,9 +259,11 @@ angular.module('ui.bootstrap.datepicker', ['ui.bootstrap.dateparser', 'ui.bootst | |||
|
|||
if ( scope.showWeeks ) { | |||
scope.weekNumbers = []; | |||
var weekNumber = getISO8601WeekNumber( scope.rows[0][0].date ), | |||
numWeeks = scope.rows.length; | |||
while( scope.weekNumbers.push(weekNumber++) < numWeeks ) {} |
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.
There's no need to change the while loop logic once the first week number (weekNumber
) is determined correctly.
Did a review, if you don't mind making those changes, we can get this merged! Thank you! |
Also, this PR should add tests to test this change by changing the startingDay. |
Closing in favor of #2306 which is getting merged. Thanks for your work! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The datepicker displays wrong week numbers if the starting-day is 0 (Sunday),
because it uses the first day of the row to determine the weeknumber of the corresponding row.
According to ISO 8601 the week starts on Monday and ends on Sunday.
Consequently the datapicker will display e.g. for the week Sun Aug 17 - Sat Aug 23 the number of the week that ended on Sun Aug 17 and not the week that starts on Mon Aug 18.
Using the date at middle of the row will display the correct week number regardless of whether the week display starts with a Sunday or a Monday.
[@chrisirhc edit: FIxes #2506]