MosheBerman/MBCalendarKit

CalendarView doesn't take in account firstDayOfWeek

iChernov opened this issue · 10 comments

Summary

CalendarView doesn't take in account firstDayOfWeek, so if (like in EU) first day of the week is Monday, then some months (like October 2017) are shown without the first day (as it is Sunday and it is hidden in the previous month's last line).

Steps To Reproduce

  1. Set calendarView locale as "de_DE" or "ru_RU" and open October 2017

Expected Results

The whole month should be shown at once

Actual Results

Month is shown starting 2nd of October, but one extra line (with only November dates) is added at the end.

Notes

Please let me know where I can fix it. Very annoying bug.

The problem is here, as I can see:

- (NSDate *)firstDayOfTheWeekUsingReferenceDate:(NSDate *)date andStartDay:(NSInteger)day
{
    NSInteger weekday = [self weekdayInDate:date]-day; //it could be below 0
    newStartDate = [self dateBySubtractingDays:weekday fromDate:date];
    
    return newStartDate;
}

the proper variant should be:

- (NSDate *)firstDayOfTheWeekUsingReferenceDate:(NSDate *)date andStartDay:(NSInteger)day
{
    NSInteger weekday = [self weekdayInDate:date]-day;
    if (weekday < 0) {
        weekday = weekday + 7;
    }
    NSDate *newStartDate = [self dateBySubtractingDays:weekday fromDate:date];
    
    return newStartDate;
}

Thanks for reporting this, and for diving into the code. It looks like you found a solution. Thanks for sharing it back.

Root Cause

Thinking about this, I'm not sure that I properly considered all of the possible cases for the weekday of date relative to day. (If day is less than "weekday of date", the same as it, or great than it.)

I'm not sure if the weekday comes from NSCalendar itself or the calendar's locale, which is, if I'm remembering right, is what determines the start weekday.

I'm fairly certain it's the locale, because if you look at some of the calendar systems in the demo app, they work just fine with the device locale set to the US, but if you change the language to French or Arabic as in #73 or #110, you'll see this issue with German and Russian, as you noted.

I'm also fairly certain that the language itself isn't the issue, but the language is defined the locale, which also defines a default firstWeekday.

Unit Tests

I'm making a note here for some test cases, when I have a little more time to dive in:

Calendar Identifier Start Day Locale
.gregorian Sunday (1) US (English)
.gregorian non-Sunday (2...7) US (English)
.islamicTabular non-Sunday (2...7) US (English)
.gregorian Sunday (1) EU/German, Russian, French, Arabic
.gregorian non-Sunday (2...7) EU (German, Russian, French, Arabic)
.islamic... non-Sunday (2...7) AR (Arabic)

The tests would construct a reference date representing a known calendar day, and then call firstDayOfWeekUsingReferenceDate:andStartDay:, and compare against an expected date for the start of the week.

I think some of these test cases will overlap, but since we know at least several locales to exhibit this bug, I'm noting the ones we know expose the broken behavior.

Still rather bad.

Okay, it ignores my firstWeekday I explicitly assign to calendar and uses the system's calendar firstWeekday. That's still somehow understandable. But the problem I've filed is still there: (missing 1st day):
screen shot 2017-09-06 at 19 10 19

5.0.4 is strictly a performance release. This hasn't been addressed yet.

Working on this - found another place in NSCalendar+Juncture that needs to account for this.

Fix incoming. (Related: #73 #110.)

Out. Let me know if this is better for you.

Great. I'm going to close this.