Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.In the month view, the header attribute for the the date has the wrong day name listed if the start of the week isn't set to Sunday. In my case the start of the week is set to Monday to correspond to the ISO-8601 standard for weeks. This is because we use a week view and PHP 7+ uses this standard for weeks.
I changed two for loops into foreach loops to properly deal with how Drupal reorders the week day names. I then pull the index and use that for $week_day in calendarBuildMonth and $i in calendarBuildWeek. I also changed how the multiday and single day buckets are initialized so they keep the weekDaysOredered key order.
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | Screenshot from 2020-09-16 14-37-34.png | 33.91 KB | ankithashetty |
| #19 | Screenshot from 2020-09-16 14-38-30.png | 32.29 KB | ankithashetty |
| #19 | diff_reroll_2937387-2_19.txt | 3.21 KB | ankithashetty |
| #19 | 2937387-19.patch | 2.5 KB | ankithashetty |
| #14 | interdiff_11-14.txt | 6.75 KB | vebrovski |
Comments
Comment #2
jazzfiction CreditAttribution: jazzfiction as a volunteer commentedComment #3
jazzfiction CreditAttribution: jazzfiction as a volunteer commentedComment #4
eigentor CreditAttribution: eigentor commentedComment #5
eigentor CreditAttribution: eigentor commentedI can confirm the issue and that the patch provided by jazzfiction solves it.
I don't feel fit to do a code review though.
Comment #6
Neslee Canil PintoComment #7
jedsaet CreditAttribution: jedsaet as a volunteer commentedStraight reroll of #2.
Comment #9
strozx CreditAttribution: strozx at Agiledrop - Your Trusted Drupal Teammates commentedI just rerolled the patch and fixed the code standard violation. I also tested the solution and it fixes the problem. I'm going to leave this on needs review if anyone else could also test.
Comment #10
ngkoutsaik CreditAttribution: ngkoutsaik at Agiledrop - Your Trusted Drupal Teammates commentedComment #11
ngkoutsaik CreditAttribution: ngkoutsaik at Agiledrop - Your Trusted Drupal Teammates commentedHi,
The patch from @jedsaet applies cleanly for me. I inspected the file with code sniffer and fixed errors and warnings.
I am going to set this to review cause I made quite a few changes to the file.
However, as said above the patch works.
Comment #13
ngkoutsaik CreditAttribution: ngkoutsaik at Agiledrop - Your Trusted Drupal Teammates commentedComment #14
vebrovski CreditAttribution: vebrovski at Agiledrop - Your Trusted Drupal Teammates commentedRerolled the patch. Applying it solves the issue.
Comment #16
vebrovski CreditAttribution: vebrovski at Agiledrop - Your Trusted Drupal Teammates commentedComment #17
rokzabukovec CreditAttribution: rokzabukovec at Agiledrop - Your Trusted Drupal Teammates commentedThe patch from #14 solves the issue of the wrong headers information. I tested it with every day as the start of the week and now the information is correct.
Comment #18
joelpittetThe code standard fixes, though appreciated, make it really hard to review this patch before committing. And make the patch fragile when someone does fix the Coding standards in a slightly different way.
Could someone undo the coding standards so we can just see the fix? I'll gladly review and commit this then.
Comment #19
ankithashettyI don't think this issue exists anymore... Even without the patch the calendar month view's day column headers displayed correctly as we change the 'First day of week'. But looks like some did face this issue, so I have re-rolled the patch in #2 and removed the extra drupal code standard changes from it as requested in #18...
Before and After applying the patch (same result):
When 'First day of week' is set to Sunday:
When 'First day of week' is set to Wednesday:
Thank you!
Comment #21
joelpittetThank you @ankithashetty and the screenshots are a nice touch!