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.

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

wired-calendar wrong day names #129

Closed
martinKumhera opened this issue Apr 2, 2020 · 5 comments · Fixed by #140
Closed

wired-calendar wrong day names #129

martinKumhera opened this issue Apr 2, 2020 · 5 comments · Fixed by #140

Comments

@martinKumhera
Copy link

martinKumhera commented Apr 2, 2020

Using locale !== EN, the day names are wrong.
Issue is in function localizeCalendarHeaders

should be:
...
if (l !== 'en-us' && l !== 'en') {
const d = new Date();
// Compute weekday header texts (like "Sun", "Mon", "Tue", ...)
const weekDayOffset = d.getUTCDay();
const daySunday = new Date(d.getTime() - DAY * weekDayOffset);
for (let i = 0; i < 7; i++) {
let weekdayDate = new Date(daySunday);

            weekdayDate.setDate(daySunday.getDate() + i);
            this.weekdays_short[i] = weekdayDate.toLocaleString(this.locale, { weekday: 'short' });
        }
@martinKumhera martinKumhera changed the title wired-calendar wrond day names wired-calendar wrong day names Apr 2, 2020
@apennamen
Copy link
Contributor

Hello,
I can't reproduce the issue with locale fr, day names are correct. What was your locale ?

Besides, the modification you propose has no effect on behaviour, as you assign the same value for weekdayDate each loop.
Regards

@martinKumhera
Copy link
Author

Hello, the issue occurs only in the last week of a month. You start with the prev Sunday and add 7 days. If you jump to the next month, implicitly within the date variable, the month will be increased and this will be done for all remaining days.
Regards

@apennamen
Copy link
Contributor

I think I begin to understand what is wrong here...

Problem is you can have (depending at which time you launch this code):

const day = new Date();
day.getUTCDay() => 0 (Sunday)
day.toLocaleString('fr-FR', {weekday: 'short'}) => 'lun.' (Monday)

Normally, we COULD write this to have the localized days:

const days = [];
const d = new Date();
for (let i=0; i < 7; i++) {
  days[d.getUTCDate()] = d.toLocaleString('fr-FR', {weekday: 'short'}); // replace fr-FR by the locale
}

Indeed, d.getUTCDate() will always have a 0 index for Sunday according to doc. So we SHOULD have :
["dim.", "lun.",...]
But as there is a possible gap between the UTCDate and the localeString, you can have:
["lun.", "mar.", ... "dim."]

I think the solution is to pick a date in the past, that was a sunday. I'm trying to rewrite those parts in a more functionnal style to be more testable:

  const localizedDays = function(locale :string = 'en-US', variant: string = 'short') {
    const d = new Date(0,0,0); // 31 Dec 1899 was a sunday
    const days = [];

    // Compute weekday header texts (like "Sun", "Mon", "Tue", ...)
    for (let i = 0; i < 7; i++) {
      days[i] = d.toLocaleString(locale, { weekday: variant });
      d.setDate(d.getDate() +1)
    }
    return days;
}

@apennamen
Copy link
Contributor

@martinKumhera in order to fix this, I need to reproduce the issue consistently.

Can you give me the following information:

  • Month and Year on which you observed the issue
  • Locale used
  • A screenshot

Regards,

@martinKumhera
Copy link
Author

martinKumhera commented Apr 16, 2020

If you set your local computer date to e.g. 28th april 2020 and open the component showcase,
you will see in locale DE - Mo, Di, Mi, Do, Fr, Mo -- and the last Mo should be Sa !!

The issue can be solved, if you just move the line "let weekdayDate = new Date(daySunday);" from outside the for loop into the for loop. Then the date (including Month) is new initialized and your calculation is correct.

BR
Martin
Wired Elements Showcase.pdf

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

Successfully merging a pull request may close this issue.

2 participants