Skip to content

Conversation

@NataliiaPodp
Copy link
Contributor

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked my code and corrected any misspellings
  • My changes take different screen sizes into account
  • My changes are backward compatible
    • I have not changed any parameter names
    • I have not changed any translation keys
  • My changes conform to the contributing guidelines (CONTRIBUTING.md)

@NataliiaPodp NataliiaPodp added the enhancement New feature or request label Apr 24, 2025
@NataliiaPodp NataliiaPodp self-assigned this Apr 24, 2025
@NataliiaPodp NataliiaPodp linked an issue Apr 24, 2025 that may be closed by this pull request
@NataliiaPodp NataliiaPodp marked this pull request as ready for review April 28, 2025 13:20
@NataliiaPodp NataliiaPodp requested a review from lukket April 28, 2025 13:20
Comment on lines +12 to +23
const availableLangs = [
{{- range $index, $lang := .Site.Languages -}}
"{{ $lang.Lang }}"{{ if ne (add $index 1) (len $.Site.Languages) }}, {{ end }}
{{- end -}}
];

const langMap = {
{{- $total := len .Site.Home.AllTranslations -}}
{{- range $index, $lang := .Site.Home.AllTranslations -}}
"{{ $lang.Language.Lang }}": "{{ $lang.RelPermalink }}"{{ if ne (add $index 1) $total }},{{ end }}
{{- end -}}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation isn't consistent here, isn't it?

Comment on lines +8 to +10
const isVisited = localStorage.getItem('visited');
if (isVisited) return;
localStorage.setItem('visited', '1');
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really the expected behavior @ThomasPe? Or should we store the browser language here and set it every time? In other words, what happens when the user visits the page the next time? Then the language is still wrong, isn't it?


Depending on @ThomasPe answer, let's put 'visited' into a constant to avoid magic string side effects and I would suggest a more explicit name (e.g., 'BrowserLanguageChecked' or similar).

Copy link
Collaborator

Choose a reason for hiding this comment

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

we need to allow the user to switch (back) to any language, so auto-redirect makes only sense on the first page visit? open to other suggestions

Copy link
Contributor

Choose a reason for hiding this comment

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

That would result in an inconsistent behavior, wouldn't it? Let's assume the browser language is English and the default language German. The first time, the user gets redirected to the English version. The second and every time after, the user stays on the German page. I don't see a benefit in the behavior.

Suggestion: we store the preferred language. Initially, the preferred language is the browser language. When the users use the language switcher to change the language, this language is stored as preferred language. Finally, if the preferred language is not the current language, we redirect the user to the preferred language.

This way, users can switch the language with the language chooser and will always end up on their preferred language.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds good 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automatically switch to correct language

4 participants