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

Date Localization Issues with dayjs #391

Open
davide-ruota opened this issue Jan 13, 2025 · 12 comments
Open

Date Localization Issues with dayjs #391

davide-ruota opened this issue Jan 13, 2025 · 12 comments

Comments

@davide-ruota
Copy link

Description

Hi, I have some issues with date localization. I initially thought it was due to localization settings, but I realized it depends on the localization provided by dayjs.

I also checked on https://try.vikunja.io and noticed that other languages besides Italian have localization issues.
In practice, only German, French, and Russian are translated correctly, and maybe a few others (I haven't checked everything).

Vikunja Version

v0.24.1-657-8ba9ded3e2

Browser and version

No response

Can you reproduce the bug on the Vikunja demo site?

Yes

Screenshots

mXcBhDF

@kolaente
Copy link
Member

This seems to be a problem with our loading of these translations. The italian translation files seem correct. @dpschen Do you have an idea what could be wrong here?

@kolaente kolaente added the bug Something isn't working label Jan 13, 2025
@dpschen
Copy link
Contributor

dpschen commented Jan 14, 2025

I would assume that the dayjs language mapping is the issue here, since it doesn't seem like it's in sync.

Screenshot 2025-01-14 at 10 53 38

EDIT

Also SUPPORTED_LOCALES need to be updated accordingly. I thought that they would be up-to-date, but they don't seem to be?

Screenshot 2025-01-14 at 11 00 06

@kolaente
Copy link
Member

Crowdin syncs all languages, but only those with translated strings of more than 50% are added to SUPPORTED_LOCALES. That's why not all languages we have files for are included.

Italian is included correctly, but the date strings are not used?

@dpschen
Copy link
Contributor

dpschen commented Jan 14, 2025

I see, will check further, thanks for clarification!

I wasn't aware that Crowdin also changed the SUPPORTED_LOCALES list. Maybe we should add a comment there. Or to make it even more obvious we could generate the list in an external file that gets imported in i18n/index.ts.

Why do we add json files of languages that have less than 50%?

@kolaente
Copy link
Member

I wasn't aware that Crowdin also changed the SUPPORTED_LOCALES list

It doesn't, the crowdin sync will only create the files. Adding a language to SUPPORTED_LOCALES is a manual process.

Why do we add json files of languages that have less than 50%?

That's a limitation of the crowdin sync job.

@kolaente
Copy link
Member

It looks like the part of Vikunja from the screenshot in the first comment does not use dayjs, but date-fns (a different library) 🙃

https://github.com/go-vikunja/vikunja/blob/main/frontend/src/helpers/time/formatDate.ts#L4-L5

Why again do we have two date libraries?

@kolaente
Copy link
Member

@kolaente kolaente added area/frontend and removed bug Something isn't working labels Jan 21, 2025
@jon4god
Copy link

jon4god commented Feb 19, 2025

I think it's a pretty critical bug. I get everything related to dates in English, although I use Russian in v0.24.6. And besides I have other inserts from English, although everything is translated in Crowdin.

Image

@kolaente
Copy link
Member

And besides I have other inserts from English, although everything is translated in Crowdin.

Not all translated strings are approved. Only approved strings will show up in Vikunja.

@kolaente
Copy link
Member

Fixed in 021d71b, please check with the next unstable build (should be ready for deployment in ~45min, also on try).

@jon4god
Copy link

jon4god commented Feb 22, 2025

From what I understand the changes can be viewed at https://try.vikunja.io/. Checked it out. There's still a problem with the dates.
Gone are the remnants of English from the settings.

Image Image

iworklee pushed a commit to iworklee/vikunja that referenced this issue Feb 24, 2025
This removes date-fns and replaces it with the already used dayjs library. It does not make sense to have two libraries for the same purpose, and dayjs seems to be smaller and its translations are already integrated. Since we have to use dayjs because it is used by the gantt chart, this was the obvious way to go (instead of replacing dayjs with date-fns).

Resolves go-vikunja#391

Reviewed-on: https://kolaente.dev/vikunja/vikunja/pulls/3039
Co-authored-by: kolaente <[email protected]>
Co-committed-by: kolaente <[email protected]>
@kolaente
Copy link
Member

This seems to affect a handful of languages, but it's completely unclear to me why. Dayjs has all the required translations.

@kolaente kolaente reopened this Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants