Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Datepicker displays wrong week numbers #2506

Closed
guenter-wolf opened this issue Jul 24, 2014 · 20 comments
Closed

Datepicker displays wrong week numbers #2506

guenter-wolf opened this issue Jul 24, 2014 · 20 comments

Comments

@guenter-wolf
Copy link

The datepicker displays wrong week numbers if the starting-day is 0 (Sunday).
Please compare the week numbers of angularjs-ui bootstrap Inline Datepicker at:
http://angular-ui.github.io/bootstrap/
with the ISO 8601 conform week numbers at:
http://www.epochconverter.com/date-and-time/weeknumbers-by-year.php

The angularjs-ui datepicker displays e.g. for Tue Aug 19 2014 the week number 33,
but the ISO 8601 conform week number is 34.

The problem seems to be that the datepicker uses the first day of the row in the calendar grid to determine the week number of the corresponding row.
This leads to wrong results if the first day of the row is Sunday (which is the datepicker default starting-day).
According to ISO 8601 the week starts on Monday and ends on Sunday.
Consequently the datapicker will display 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.
However, please note that the Popup Datepicker at
http://angular-ui.github.io/bootstrap/ displays the correct week number, since the startingDay is 1 (Monday).

Suggested Solution:
Use the day in the middle of the row to determine the week number of the corresponding row, i.e. replace in datepicker.js the line:
var weekNumber = getISO8601WeekNumber( scope.rows[0][0].date )
with:
var weekNumber = getISO8601WeekNumber( scope.rows[0][3].date )
This will display the correct week number regardless of whether the week display starts with a Sunday or a Monday.

guenter-wolf added a commit to guenter-wolf/bootstrap that referenced this issue Jul 26, 2014
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.
@guenter-wolf
Copy link
Author

I have created a pull request for this bug fix:
#2517

guenter-wolf added a commit to guenter-wolf/bootstrap that referenced this issue Jul 26, 2014
fix bug angular-ui#2506 Datepicker displays wrong week numbers.
also fixed some white space problems jshint was complaining about.
guenter-wolf added a commit to guenter-wolf/bootstrap that referenced this issue Jul 26, 2014
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
@stephent
Copy link

Not sure if this is a related issue, but December 2015 shows weeks 49 through 54, January 2016 shows weeks 53 through 58 in the popup.

screen shot 2014-08-27 at 18 31 25

screen shot 2014-08-27 at 18 34 12

Observed in 0.11.0

guenter-wolf added a commit to guenter-wolf/bootstrap that referenced this issue Aug 28, 2014
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.
@guenter-wolf
Copy link
Author

This is another bug:
It calculates the ISO 8601 Week Number only for the first row of a month and for the following rows of the month it just increments the week number.
This generally works, but it may create wrong week numbers for December and January.
The correct week numbers for December 2015 calendar display should be:
49,50,51,52,53,1
and for the January 2016 calendar:
53,1,2,3,4,5
I have changed the pull request #2517 to fix also this bug.

@guenter-wolf
Copy link
Author

@pkozlowski-opensource @chrisirhc
Why is this simple bug fix still not merged?
Sorry, this is my first attempt to contribute. Is there anything wrong with my pull request?
Shouldn't bugs like this have highest priority?
I have noticed that memory leak issues with the datepicker seem to get a lot more attention.
But who cares about memory leaks? If the datepicker displays wrong information, it is useless.

@mforkin
Copy link

mforkin commented Nov 19, 2014

+1 Need this fix

@DannyDouglass
Copy link

+1 on this item - any chance we get that pull request accepted if it fixes these issues?

@wilgert
Copy link

wilgert commented Dec 6, 2014

+1 need this very badly

@Burgov
Copy link

Burgov commented Dec 7, 2014

+1 much needed here as well!

@LooMan
Copy link

LooMan commented Dec 8, 2014

+1 Need this fix! :)

@mhaamann
Copy link

+1 - It's a bug fix, why not merge it?

@chrisirhc
Copy link
Contributor

I've reviewed the PR and it needs some work before we can get it merged in.

@stephanmullerNL
Copy link

Since I had no idea of knowing when the adjustments would be made here I made a new PR that covers the issues that were still relevant: #3158. Starting day seemed to have already been fixed in the meantime.

@yuyokk
Copy link
Contributor

yuyokk commented Feb 25, 2015

Hey guys,

Do you have any updates on this bug? When do you plan to fix it?
Thank you!

@karianna
Copy link
Contributor

@stephanmullerNL
Copy link

@yuyokk I still have a PR outstanding for this, but as you can read in the comments I'm confused by the conflict in specs (must adhere to ISO standards vs must allow sunday as the week's first day). I don't know how to solve that, it seems more like a business decision to me than something code can easily fix.

#3158

@yuyokk
Copy link
Contributor

yuyokk commented Feb 26, 2015

@picodealion I see what you mean. I appreciate your answer. Thanks.

@chrisirhc chrisirhc self-assigned this Apr 26, 2015
@LiamK
Copy link

LiamK commented Nov 9, 2016

Is there any way to get rid of that last week? For example, in the last image above, the last row on the January 2016 calendar is not even January, it's February.

@jkevlorayna
Copy link

Replace this one
screenshot_70
with this code
screenshot_71

on ui-bootstrap-tpls.js

Think this one will solve your problem!

@chinchan
Copy link

chinchan commented Jan 6, 2017

wow. Thanks for this fix. I just make your code into more elegant looks. I just removed the index since it does not needed. Thanks.
image

@jkevlorayna
Copy link

@chinchan awesome !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.