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

Improve browser language detection and set nl-be fallback #621

Merged
merged 2 commits into from
Mar 9, 2024

Conversation

elpoelma
Copy link
Contributor

Overview

This PR applies the same set of changes as lblod/ember-rdfa-editor#1117, improving the detection of the user's browser language.

connected issues and PRs:

lblod/ember-rdfa-editor#1117

How to test/reproduce

Use a locale switcher to try different combinations of locales and that it always translates the UI as expected. In that add-on you can select multiple locales by clicking the switch at the top:
image

Checks PR readiness

  • UI: works on smaller screen sizes
  • UI: feedback for any loading/error states
  • Check if dummy app is correctly updated
  • Check cancel/go-back flows
  • changelog
  • npm lint
  • no new deprecations

@elpoelma elpoelma added the enhancement New feature or request label Jan 22, 2024
@elpoelma elpoelma requested a review from piemonkey January 22, 2024 10:20
Copy link
Contributor

@piemonkey piemonkey left a comment

Choose a reason for hiding this comment

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

Looks good, but not sure about importing vs duplication.

@@ -1,6 +1,7 @@
import Route from '@ember/routing/route';
import { service } from '@ember/service';
import ENV from 'frontend-gelinkt-notuleren/config/environment';
import { decentLocaleMatch } from '../utils/intl';
Copy link
Contributor

Choose a reason for hiding this comment

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

My intent was that we would import the function from the editor repo rather than having to duplicate it in every codebase. Is there a good reason not to do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I see, the main reason I did not import it from the editor is that the function is not really inherent to the editor codebase (it is quite generic).

@abeforgit abeforgit enabled auto-merge March 9, 2024 14:12
@abeforgit abeforgit merged commit d78cae4 into master Mar 9, 2024
3 checks passed
@abeforgit abeforgit deleted the improve-locale-matching branch March 9, 2024 14:16
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.

3 participants