i18n: update coverage to auto-detect browser languages#946
Merged
xiashtra merged 5 commits intoOverlayPlugin:mainfrom Jan 20, 2026
Merged
i18n: update coverage to auto-detect browser languages#946xiashtra merged 5 commits intoOverlayPlugin:mainfrom
xiashtra merged 5 commits intoOverlayPlugin:mainfrom
Conversation
Author
Collaborator
|
I think that option 2 is better from a logic standpoint, with a wrapper for pulling from browser export const localesToLang = (locales: readonly string[]): Lang => {
// ...
};
export const browserLanguagesToLang = (): Lang => localesToLang(navigator.languages);But this is pretty subjective. |
Author
valarnin
approved these changes
Jan 19, 2026
Collaborator
valarnin
left a comment
There was a problem hiding this comment.
Because you cannot change the language setting of the ACT web browser
Can you file an issue on the OverlayPlugin repo for this and I'll see about tackling it towards the end of the expansion, if it's even possible?
github-actions bot
pushed a commit
that referenced
this pull request
Jan 20, 2026
#946) f285d76: `coverage.ts` detects language from the web browser. c0cf1de: fix Korean translation 8cbefb1: remove `languagesArr` parameter from `browserLanguagesToLang` function. ## Removing `languagesArr` parameter ### Current Situation The `browserLanguagesToLang` function has a comment saying it only takes `navigator.languages` as input. But it still receives this value as a parameter, which is unnecessary. ### Option 1: Remove the parameter If the function only handles `navigator.languages`, we can remove the parameter and use `navigator.languages` directly inside the function. ### Option 2: Generalize the function If we want to support other locale arrays beyond `navigator.languages`, we could keep the parameter and rename the function to `localesToLang`. This makes it clear that the function converts any locale array to a cactbot language. --- This PR implements Option 1. Let me know if Option 2 is preferred. e14eec1
github-actions bot
pushed a commit
to ShadyWhite/cactbot
that referenced
this pull request
Jan 20, 2026
OverlayPlugin#946) f285d76: `coverage.ts` detects language from the web browser. c0cf1de: fix Korean translation 8cbefb1: remove `languagesArr` parameter from `browserLanguagesToLang` function. ## Removing `languagesArr` parameter ### Current Situation The `browserLanguagesToLang` function has a comment saying it only takes `navigator.languages` as input. But it still receives this value as a parameter, which is unnecessary. ### Option 1: Remove the parameter If the function only handles `navigator.languages`, we can remove the parameter and use `navigator.languages` directly inside the function. ### Option 2: Generalize the function If we want to support other locale arrays beyond `navigator.languages`, we could keep the parameter and rename the function to `localesToLang`. This makes it clear that the function converts any locale array to a cactbot language. --- This PR implements Option 1. Let me know if Option 2 is preferred. e14eec1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


f285d76:
coverage.tsdetects language from the web browser.c0cf1de: fix Korean translation
8cbefb1: remove
languagesArrparameter frombrowserLanguagesToLangfunction.Removing
languagesArrparameterCurrent Situation
The
browserLanguagesToLangfunction has a comment saying it only takesnavigator.languagesas input. But it still receives this value as a parameter, which is unnecessary.Option 1: Remove the parameter
If the function only handles
navigator.languages, we can remove the parameter and usenavigator.languagesdirectly inside the function.Option 2: Generalize the function
If we want to support other locale arrays beyond
navigator.languages, we could keep the parameter and rename the function tolocalesToLang. This makes it clear that the function converts any locale array to a cactbot language.This PR implements Option 1. Let me know if Option 2 is preferred.