Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Week update #152

Closed
ghost opened this issue Nov 18, 2016 · 54 comments
Closed

Week update #152

ghost opened this issue Nov 18, 2016 · 54 comments

Comments

@ghost
Copy link

ghost commented Nov 18, 2016 •
edited by ghost

When going to the calendar page I have it set at weekly starting on Monday, when switching between next and previous links the week number doesn't change once, when just using one of the two it does.
So if hitting next then previous I would end up with two weeks numbered at 47

screenshot-1
@excid3
Copy link
Owner

excid3 commented Nov 18, 2016

Hey @wgailey24,

Interesting find. So you are setting Date.beginning_of_week = :monday somewhere in a config so it's globally applied?

@ghost
Copy link
Author

ghost commented Nov 18, 2016 •
edited by ghost

Not that I can find and ive been through the whole of the config directory and controller but I'm not the one that implemented that change so its possible. (my next response will be :monday)

@excid3
Copy link
Owner

excid3 commented Nov 18, 2016

I can't remember what the Rails defaults are if they're Monday (they might be) so it might not be in your codebase. Was trying to think of what might be the cause of it, and thought maybe that if it wasn't globally set, maybe you'd see this discrepancy?

I'm not entirely sure, probably need to build an example and app fiddle with it before I can give you an answer on that.

@ghost
Copy link
Author

ghost commented Nov 19, 2016

I would really appreciate that!

On Nov 18, 2016 4:43 PM, "Chris Oliver" notifications@github.com wrote:

I can't remember what the Rails defaults are if they're Monday (they might
be) so it might not be in your codebase. Was trying to think of what might
be the cause of it, and thought maybe that if it wasn't globally set, maybe
you'd see this discrepancy?

I'm not entirely sure, probably need to build an example and app fiddle
with it before I can give you an answer on that.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#152 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AOkR8RXw2Xyo1OxWZstLt4hlICaWCqxWks5q_hv1gaJpZM4K2e_M
.

excid3 added a commit that referenced this issue Nov 20, 2016
@excid3
Potential fix for #152 week number incorrect
392609e
@excid3
Copy link
Owner

excid3 commented Nov 20, 2016

Yo @wgailey24 I think I figured it out. There are 2 different strftime codes for week number, one is for beginning of week being Sunday and the other is for Monday.

I just patched this in the fix-week-number branch. You mind installing that and trying it out to see if that fixes it for you?

gem 'simple_calendar', git: 'https://github.com/excid3/simple_calendar', branch: 'fix-week-number'

@wgailey
Copy link
Contributor

wgailey commented Nov 20, 2016

Thanks Chris,

Yeah I'll try it out about 2pm est tomorrow. Getting the last of the days sun and some decent waves tonight.

Best regards,

wgailey24

http://imgur.com/a/iGne9

@excid3
Copy link
Owner

excid3 commented Nov 21, 2016

Super jealous. 🛥🏄🌊

@ghost
Copy link
Author

ghost commented Nov 21, 2016 •
edited by ghost

Hey Chris,
Yes, that branch fixed it completely!
Are you going to be merging that branch into the master?
Thanks much!

@excid3
Copy link
Owner

excid3 commented Nov 21, 2016

Great! I'll do a version bump and then release this on rubygems.

@ghost
Copy link
Author

ghost commented Nov 21, 2016

perfect!

@excid3
Copy link
Owner

excid3 commented Nov 21, 2016

v2.2.1 is out!

@excid3 excid3 closed this as completed Nov 21, 2016
@ghost
Copy link
Author

ghost commented Nov 22, 2016

Hey chris,

Sorry to be a bother but just read some user feedback and didn't get a
chance to investigate, does simple calendar have issues with year
transitions like going to week 0 instead of going from week one to week 52,
and when hitting next going to week 53?

Best regards,

Wgailey24

On Nov 21, 2016 12:03 PM, "Chris Oliver" notifications@github.com wrote:

Closed #152 #152.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#152 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AOkR8ZpAKErKnRczYt6BhPfGmYzGqhYHks5rAc59gaJpZM4K2e_M
.

@excid3
Copy link
Owner

excid3 commented Nov 22, 2016

Oh you know what I forgot to check year transitions. What are you seeing? Might be a fix needed for that. I remember someone said that one of the two started at 0 instead of 1.

Technically there are more than 52 weeks in a year, so that might be where the 53 came from.

There are exactly 52.2857142857 weeks in the year 2016. This is equivalent to 52 weeks and 2 extra days, since there are 366 total days in 2016. Most years have 365 days, but a leap year has 366 days. That adds up to 52 weeks (where each week is exactly 7 days) PLUS 1 or 2 additional days.

@wgailey
Copy link
Contributor

wgailey commented Nov 23, 2016

Like I said I won't be able to investigate until tomarrow, but from what I was getting in feedback just before I left, it goes to week 0 and I just thought checking if it goes beyond the end of the year might be a good idea too.
Best,
Will
On Nov 22, 2016 6:59 PM, Chris Oliver notifications@github.com wrote:Oh you know what I forgot to check year transitions. What are you seeing? Might be a fix needed for that. I remember someone said that one of the two started at 0 instead of 1.
Technically there are more than 52 weeks in a year, so that might be where the 53 came from.

There are exactly 52.2857142857 weeks in the year 2016. This is equivalent to 52 weeks and 2 extra days, since there are 366 total days in 2016. Most years have 365 days, but a leap year has 366 days. That adds up to 52 weeks (where each week is exactly 7 days) PLUS 1 or 2 additional days.

—You are receiving this because you commented.Reply to this email directly, view it on GitHub, or mute the thread.

@excid3
Copy link
Owner

excid3 commented Nov 23, 2016

I discovered there's also a %V which I overlooked before which is the ISO 8601 standard.

ISO 8601 week-based year and week number:
The week 1 of YYYY starts with a Monday and includes YYYY-01-04.
The days in the year before the first week are in the last week of
the previous year.
  %G - The week-based year
  %g - The last 2 digits of the week-based year (00..99)
  %V - Week number of the week-based year (01..53)

Week number:
The week 1 of YYYY starts with a Sunday or Monday (according to %U
or %W).  The days in the year before the first week are in week 0.
  %U - Week number of the year.  The week starts with Sunday.  (00..53)
  %W - Week number of the year.  The week starts with Monday.  (00..53)

Still not sure what makes the most sense.

@wgailey
Copy link
Contributor

wgailey commented Nov 23, 2016

To me what makes the most sense would be grey placeholders, ie if the year starts on a Friday; Monday or Sunday through Thursday would simply be replaced with grey blanks, same for the end of the year if it ends on a Wednesday; Thursday through sat or sun would be grey blanks.
Best,
Will
On Nov 22, 2016 8:07 PM, Chris Oliver notifications@github.com wrote:I discovered there's also a %V which I overlooked before which is the ISO 8601 standard.
ISO 8601 week-based year and week number:
The week 1 of YYYY starts with a Monday and includes YYYY-01-04.
The days in the year before the first week are in the last week of
the previous year.
%G - The week-based year
%g - The last 2 digits of the week-based year (00..99)
%V - Week number of the week-based year (01..53)

Week number:
The week 1 of YYYY starts with a Sunday or Monday (according to %U
or %W). The days in the year before the first week are in week 0.
%U - Week number of the year. The week starts with Sunday. (00..53)
%W - Week number of the year. The week starts with Monday. (00..53)

Still not sure what makes the most sense.

—You are receiving this because you commented.Reply to this email directly, view it on GitHub, or mute the thread.

@ghost
Copy link
Author

ghost commented Nov 23, 2016

One of the things I'm actually including in my calander is the ability to
choose weekly or monthly calander and what day of the week it starts on
using some of the simple calendar options.

Biweekly would be nice too with the first week always being the current.

best,

-Will

On Nov 22, 2016 8:13 PM, "William Gailey" notifications@github.com wrote:

To me what makes the most sense would be grey placeholders, ie if the year
starts on a Friday; Monday or Sunday through Thursday would simply be
replaced with grey blanks, same for the end of the year if it ends on a
Wednesday; Thursday through sat or sun would be grey blanks.
Best,
Will
On Nov 22, 2016 8:07 PM, Chris Oliver notifications@github.com wrote:I
discovered there's also a %V which I overlooked before which is the ISO
8601 standard.
ISO 8601 week-based year and week number:
The week 1 of YYYY starts with a Monday and includes YYYY-01-04.
The days in the year before the first week are in the last week of
the previous year.
%G - The week-based year
%g - The last 2 digits of the week-based year (00..99)
%V - Week number of the week-based year (01..53)

Week number:
The week 1 of YYYY starts with a Sunday or Monday (according to %U
or %W). The days in the year before the first week are in week 0.
%U - Week number of the year. The week starts with Sunday. (00..53)
%W - Week number of the year. The week starts with Monday. (00..53)

Still not sure what makes the most sense.

—You are receiving this because you commented.Reply to this email
directly, view it on GitHub, or mute the thread.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#152 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AOkR8aPXyj3ZLTl3-ugkmqjVVy7BHTlEks5rA5M_gaJpZM4K2e_M
.

@excid3
Copy link
Owner

excid3 commented Nov 23, 2016 •
edited

Yeah, one thing you can do is configure the number_of_weeks on the week calendar.

You'll probably have to customize the title or whatever you want it to look like. I'm trying to figure out what the best default will be for the title for the single week, whether that's %V, %U, or %W. I'm wondering if %V was actually the right default to use for the gem as it seems to start counting at 1 instead of 0.

But according to the docs it seems to only be for when the week starts in Monday? I'm not sure if that is going to affect when the week start is set to Sunday.

@wgailey
Copy link
Contributor

wgailey commented Nov 23, 2016

Yeah,
%V is good, weeks 1-53 is all I use and my dates are configured with regex.
Best,
-Will
On Nov 22, 2016 8:20 PM, William Gailey notifications@github.com wrote:One of the things I'm actually including in my calander is the ability to
choose weekly or monthly calander and what day of the week it starts on
using some of the simple calendar options.

Biweekly would be nice too with the first week always being the current.

best,

-Will

On Nov 22, 2016 8:13 PM, "William Gailey" notifications@github.com wrote:

To me what makes the most sense would be grey placeholders, ie if the year
starts on a Friday; Monday or Sunday through Thursday would simply be
replaced with grey blanks, same for the end of the year if it ends on a
Wednesday; Thursday through sat or sun would be grey blanks.
Best,
Will
On Nov 22, 2016 8:07 PM, Chris Oliver notifications@github.com wrote:I
discovered there's also a %V which I overlooked before which is the ISO
8601 standard.
ISO 8601 week-based year and week number:
The week 1 of YYYY starts with a Monday and includes YYYY-01-04.
The days in the year before the first week are in the last week of
the previous year.
%G - The week-based year
%g - The last 2 digits of the week-based year (00..99)
%V - Week number of the week-based year (01..53)

Week number:
The week 1 of YYYY starts with a Sunday or Monday (according to %U
or %W). The days in the year before the first week are in week 0.
%U - Week number of the year. The week starts with Sunday. (00..53)
%W - Week number of the year. The week starts with Monday. (00..53)

Still not sure what makes the most sense.

—You are receiving this because you commented.Reply to this email
directly, view it on GitHub, or mute the thread.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#152 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AOkR8aPXyj3ZLTl3-ugkmqjVVy7BHTlEks5rA5M_gaJpZM4K2e_M
.

—You are receiving this because you commented.Reply to this email directly, view it on GitHub, or mute the thread.

@wgailey
Copy link
Contributor

wgailey commented Nov 23, 2016

Yeah,
%V is good, weeks 1-53 is all I use and my dates are configured with regex.
Best,
-Will
On Nov 22, 2016 8:20 PM, William Gailey notifications@github.com wrote:One of the things I'm actually including in my calander is the ability to
choose weekly or monthly calander and what day of the week it starts on
using some of the simple calendar options.

Biweekly would be nice too with the first week always being the current.

best,

-Will

On Nov 22, 2016 8:13 PM, "William Gailey" notifications@github.com wrote:

To me what makes the most sense would be grey placeholders, ie if the year
starts on a Friday; Monday or Sunday through Thursday would simply be
replaced with grey blanks, same for the end of the year if it ends on a
Wednesday; Thursday through sat or sun would be grey blanks.
Best,
Will
On Nov 22, 2016 8:07 PM, Chris Oliver notifications@github.com wrote:I
discovered there's also a %V which I overlooked before which is the ISO
8601 standard.
ISO 8601 week-based year and week number:
The week 1 of YYYY starts with a Monday and includes YYYY-01-04.
The days in the year before the first week are in the last week of
the previous year.
%G - The week-based year
%g - The last 2 digits of the week-based year (00..99)
%V - Week number of the week-based year (01..53)

Week number:
The week 1 of YYYY starts with a Sunday or Monday (according to %U
or %W). The days in the year before the first week are in week 0.
%U - Week number of the year. The week starts with Sunday. (00..53)
%W - Week number of the year. The week starts with Monday. (00..53)

Still not sure what makes the most sense.

—You are receiving this because you commented.Reply to this email
directly, view it on GitHub, or mute the thread.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#152 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AOkR8aPXyj3ZLTl3-ugkmqjVVy7BHTlEks5rA5M_gaJpZM4K2e_M
.

—You are receiving this because you commented.Reply to this email directly, view it on GitHub, or mute the thread.

@wgailey
Copy link
Contributor

wgailey commented Nov 23, 2016

Yeah,
%V is good, weeks 1-53 is all I use and my dates are configured with regex.
Best,
-Will
On Nov 22, 2016 8:23 PM, Chris Oliver notifications@github.com wrote:Yeah, one thing you can do is configure the number_of_weeks on the week calendar.
You'll probably have to customize the title or whatever you want it to look like. I'm trying to figure out what the best default will be for the title for the single week, whether that's %V, %U, or %W. I'm wondering if %V was actually the right default to use for the gem as it seems to start counting at 1 instead of 0.

—You are receiving this because you commented.Reply to this email directly, view it on GitHub, or mute the thread.

@wgailey
Copy link
Contributor

wgailey commented Nov 23, 2016

Hey Chris,
So, for week numbers:
Behavior observed:
for 2016 it goes from 0 to 52 then back to 1 in 2017, hitting previous from there it goes to 0 then 51.
Expected behavior:
For 2016 go from 1 to 53 then back to 1 in 2017, hitting previous from there go to 53.
Best,
Will
On Nov 22, 2016 8:13 PM, William Gailey wgailey24@yahoo.com wrote:To me what makes the most sense would be grey placeholders, ie if the year starts on a Friday; Monday or Sunday through Thursday would simply be replaced with grey blanks, same for the end of the year if it ends on a Wednesday; Thursday through sat or sun would be grey blanks.
Best,
Will
On Nov 22, 2016 8:07 PM, Chris Oliver notifications@github.com wrote:I discovered there's also a %V which I overlooked before which is the ISO 8601 standard.
ISO 8601 week-based year and week number:
The week 1 of YYYY starts with a Monday and includes YYYY-01-04.
The days in the year before the first week are in the last week of
the previous year.
%G - The week-based year
%g - The last 2 digits of the week-based year (00..99)
%V - Week number of the week-based year (01..53)

Week number:
The week 1 of YYYY starts with a Sunday or Monday (according to %U
or %W). The days in the year before the first week are in week 0.
%U - Week number of the year. The week starts with Sunday. (00..53)
%W - Week number of the year. The week starts with Monday. (00..53)

Still not sure what makes the most sense.

—You are receiving this because you commented.Reply to this email directly, view it on GitHub, or mute the thread.

@wgailey
Copy link
Contributor

wgailey commented Nov 23, 2016

PS grey areas seem to be already implemented as expected. Nice, very nice!
On Nov 22, 2016 8:13 PM, William Gailey wgailey24@yahoo.com wrote:To me what makes the most sense would be grey placeholders, ie if the year starts on a Friday; Monday or Sunday through Thursday would simply be replaced with grey blanks, same for the end of the year if it ends on a Wednesday; Thursday through sat or sun would be grey blanks.
Best,
Will
On Nov 22, 2016 8:07 PM, Chris Oliver notifications@github.com wrote:I discovered there's also a %V which I overlooked before which is the ISO 8601 standard.
ISO 8601 week-based year and week number:
The week 1 of YYYY starts with a Monday and includes YYYY-01-04.
The days in the year before the first week are in the last week of
the previous year.
%G - The week-based year
%g - The last 2 digits of the week-based year (00..99)
%V - Week number of the week-based year (01..53)

Week number:
The week 1 of YYYY starts with a Sunday or Monday (according to %U
or %W). The days in the year before the first week are in week 0.
%U - Week number of the year. The week starts with Sunday. (00..53)
%W - Week number of the year. The week starts with Monday. (00..53)

Still not sure what makes the most sense.

—You are receiving this because you commented.Reply to this email directly, view it on GitHub, or mute the thread.

@excid3
Copy link
Owner

excid3 commented Nov 23, 2016

You wanna try using the master branch? I am still using %U and %W, but I changed it to always use the first day of the week to calculate the week number.

I don't know if this is better or worse than using %V across the board.

@ghost
Copy link
Author

ghost commented Nov 23, 2016

Yeah,

I'll update it to the master branch on Friday, half day today and off
tomarrow.

Best,

Will

On Nov 23, 2016 2:46 PM, "Chris Oliver" notifications@github.com wrote:

You wanna try using the master branch? I am still using %U and %W, but I
changed it to always use the first day of the week to calculate the week
number.

I don't know if this is better or worse than using %V across the board.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#152 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AOkR8bx3dSnYNyzHSyRRhnLpIfrmb8Z1ks5rBJgBgaJpZM4K2e_M
.

@excid3
Copy link
Owner

excid3 commented Nov 24, 2016

Take your time, no rush. And enjoy the turkey! :) 🦃 🦃 🦃

@ghost
Copy link
Author

ghost commented Nov 25, 2016 via email

@excid3
Copy link
Owner

excid3 commented Nov 26, 2016

Are you on the master branch?

I just tested again and I can't reproduce week 0 at the beginning or end of 2015, 2016, or 2017. I'm using Monday as start of week.

Also I don't see what your issue is with cut off months. This is what I get for week 48:

week 48

@ghost
Copy link
Author

ghost commented Nov 26, 2016 via email

@wgailey
Copy link
Contributor

wgailey commented Nov 28, 2016 via email

@excid3
Copy link
Owner

excid3 commented Nov 28, 2016

First week of 2017 does that for you? I tried it a bunch of times and it wasn't saying Week 0 for me. grumble

@ghost
Copy link
Author

ghost commented Nov 28, 2016 via email

@ghost
Copy link
Author

ghost commented Nov 28, 2016 via email

@excid3
Copy link
Owner

excid3 commented Nov 28, 2016

Oh hmm, I think you may have to specify the git url for the gem to make sure you're using it from github.

gem 'simple_calendar', :git => 'https://github.com/excid3/simple_calendar', :branch => 'master'

You should see it in the bundle logs to show that it's coming from Github.

@wgailey
Copy link
Contributor

wgailey commented Nov 28, 2016 via email

@ghost
Copy link
Author

ghost commented Nov 28, 2016 via email

@wgailey
Copy link
Contributor

wgailey commented Nov 29, 2016 via email

@excid3
Copy link
Owner

excid3 commented Nov 29, 2016

You sure those aren't being hidden with CSS or something? I can't reproduce the weeks with missing days. Every week displays the full 7 days of dates for me.

@wgailey
Copy link
Contributor

wgailey commented Nov 29, 2016 via email

@ghost
Copy link
Author

ghost commented Nov 29, 2016 via email

@ghost
Copy link
Author

ghost commented Nov 29, 2016 via email

@wgailey
Copy link
Contributor

wgailey commented Dec 5, 2016 via email

@excid3
Copy link
Owner

excid3 commented Dec 5, 2016

Hah, no worries. :)

Yeah you should be able to highlight those links. You could style against .calendar-heading a which you can see here: https://github.com/excid3/simple_calendar/blob/master/app/views/simple_calendar/_week_calendar.html.erb

Or you could just add the views to your app and then add some classes in.

@ghost
Copy link
Author

ghost commented Dec 5, 2016 via email

@wgailey
Copy link
Contributor

wgailey commented Dec 7, 2016 via email

@excid3
Copy link
Owner

excid3 commented Dec 7, 2016

Feel free to shoot me a PR for it, I don't have any time to add it myself at the moment.

@wgailey
Copy link
Contributor

wgailey commented Dec 7, 2016 via email

@wgailey
Copy link
Contributor

wgailey commented Dec 7, 2016 via email

@wgailey
Copy link
Contributor

wgailey commented Dec 9, 2016 via email

@wgailey
Copy link
Contributor

wgailey commented Dec 18, 2016 via email

@wgailey
Copy link
Contributor

wgailey commented Mar 2, 2017 via email

@wgailey
Copy link
Contributor

wgailey commented Mar 2, 2017 via email

@wgailey
Copy link
Contributor

wgailey commented Mar 2, 2017 via email

@excid3
Copy link
Owner

excid3 commented Mar 6, 2017

Just merged it, sorry for the delay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@excid3 @wgailey