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

fix: Localisation-matching algorithm missing some edgecase #4692

Merged
merged 13 commits into from
Apr 21, 2024

Conversation

CommanderStorm
Copy link
Collaborator

@CommanderStorm CommanderStorm commented Apr 19, 2024

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

  • I have read and understand the pull request rules.

Description

Resolves #4691
Resolves #4690
Resolves #4697

There were these minor bugs in the new language selection mechanism introduced in 1.23.12:

  • we only fall back to the generic locale (for en-US this would be en) for the first entry of the languages
  • We never match en if other (less prefered) alternatives exist in the users preferences, as this is only in the messages and not in the languageList.
  • We assumed that the generic locales are only 2 characters, not 3

Our testcases missed this edgecase

thanks @CirnoT for the bug report

Type of change

Please delete any options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas
    (including JSDoc for methods)
  • My changes generate no new warnings
  • My code needed automated testing. I have added them (this is optional task)

@louislam louislam added this to the 1.23.13 milestone Apr 19, 2024
@CommanderStorm CommanderStorm marked this pull request as ready for review April 19, 2024 13:05
@CommanderStorm CommanderStorm linked an issue Apr 19, 2024 that may be closed by this pull request
1 task
@CommanderStorm CommanderStorm changed the title Localisation fix: Localisation of en-detection if there are other languages with lower priority Apr 19, 2024
@CommanderStorm CommanderStorm marked this pull request as draft April 20, 2024 12:53
@CommanderStorm CommanderStorm linked an issue Apr 20, 2024 that may be closed by this pull request
1 task
@CommanderStorm CommanderStorm changed the title fix: Localisation of en-detection if there are other languages with lower priority fix: Localisation-matching algorithm missing some edgecase Apr 20, 2024
@CommanderStorm CommanderStorm linked an issue Apr 20, 2024 that may be closed by this pull request
1 task
src/i18n.js Outdated Show resolved Hide resolved
src/i18n.js Outdated Show resolved Hide resolved
src/i18n.js Outdated Show resolved Hide resolved
@CommanderStorm CommanderStorm marked this pull request as ready for review April 20, 2024 15:46
@louislam louislam merged commit add5c12 into louislam:1.23.X Apr 21, 2024
14 checks passed
@CommanderStorm CommanderStorm deleted the localisation branch April 21, 2024 12:24
@CommanderStorm CommanderStorm added the area:core issues describing changes to the core of uptime kuma label Apr 21, 2024
expect(currentLocale()).equal("en");

setLanguage('zz-ZZ');
setLanguages(['zz-ZZ']);
expect(currentLocale()).equal("en");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same test case as in line 35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:core issues describing changes to the core of uptime kuma
Projects
None yet
3 participants