-
-
Notifications
You must be signed in to change notification settings - Fork 46
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.
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
Date Fix #7
Date Fix #7
Conversation
Can you explain the issue this fixes with perhaps some example code? |
As in Issue #6 explained, I fixed the today bug. When you initalize the calendar to the current month it doesnt hightlight the today item correctly. It highlights the very first of the month each time. This was caused by not correctly referencing to the current date in the each day loop. |
Oh, I see. Thank you very much for the explanation as I didn't understand what was the actual complaint in #6. So you're calling new
where "April 2017" is the current month and year without specifying the day of the month. Hmm… The ability to specify the day is technically intended as a "feature", but it's kind of ham handedly implemented. I had not considered that someone wouldn't just omit the parameter for the current month. Ala new \donatj\SimpleCalendar() which will highlight the current day correctly. because if you specify for instance April 22 2017 it picks April 22nd. Current month and highlighted day should be seperate parameters not jammed together. I will accept your PR, but I'm afraid I'm not going to tag a release right now as it's a breaking change. This evening when I get home from work I will start deving current date as an optional separate parameter so you can still control the "current date". |
I thought that the Class can be called like this |
@ben182 It can, yes. The issue is when the month and year are the current month and year, it also uses the day of that date to select which day to display. It's a bad behaviour, but also a "feature". As noted, I should break that into a separate parameter. I just have not had a lot of time of late, but that's coming soon. |
This was merged into the 1.x development branch. The final fix however is a little different, I've added a seperate "Today" date value from the current calendar value, which before were kinda of the same value. |
@ben182 It's sad on my part but this finally made it into a release - see: https://github.com/donatj/SimpleCalendar/releases/tag/v0.7.0 |
Issue #6 fixed