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.

Sign up for GitHub

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

Jump to bottom

feat: improve detecting missing translations #659

Merged
merged 6 commits into from
Jun 10, 2021
Merged

feat: improve detecting missing translations #659

merged 6 commits into from
Jun 10, 2021

Conversation

dhhyi
Copy link
Collaborator

@dhhyi dhhyi commented Apr 18, 2021 •
edited
Loading

PR Type

[x] Feature

What Is the Current Behavior?

  • Missing translations are silently replaced with the ones from the ngx-translate default lang or just the translation key is inserted
  • The default lang is always pulled for client applications

What Is the New Behavior?

  • Missing translations trigger an error in the console, so that they can be detected (SSR logs, Sentry, Browser console)
  • Pulling default fallbacks can be customized
  • final SSR locale is transferred to the client to be used for initialization

Does this PR Introduce a Breaking Change?

[ ] Yes
[x] No

@dhhyi dhhyi added enhancement Enhancement to an existing feature wait Waiting for something (e.g. new ICM release) labels Apr 18, 2021
@dhhyi
Copy link
Collaborator Author

dhhyi commented Apr 18, 2021

wait for integration of #655

@dhhyi dhhyi marked this pull request as ready for review April 18, 2021 14:14
This was referenced Apr 18, 2021
Closed
Closed
Closed
@dhhyi dhhyi force-pushed the feature/translations-output-hashed branch from 828dc50 to 4b07610 Compare April 18, 2021 19:09
@dhhyi dhhyi force-pushed the feature/translations-fallback-handling branch from d3a8f27 to 933adfe Compare April 18, 2021 19:27
@MaxKless MaxKless self-assigned this Apr 28, 2021
@MaxKless MaxKless self-requested a review April 28, 2021 11:43
@MaxKless
Copy link
Collaborator

MaxKless commented Jun 2, 2021

Hi Danilo, thanks for the PR.
During our localization rework (#695) we adopted some of your SSR-specific code. Thank you very much for your contribution.
The missing translation handling is definitely a nice-to-have but not a priority for us right now. You're very welcome to rebase and integrate your changes with the current internationalization module and I will look at it ASAP! Otherwise, we will close this PR and track the enhancement ourselves.

@dhhyi dhhyi force-pushed the feature/translations-output-hashed branch 2 times, most recently from 658afe8 to ba32a09 Compare June 3, 2021 12:09
@dhhyi dhhyi force-pushed the feature/translations-fallback-handling branch from 933adfe to c811354 Compare June 3, 2021 13:11
@dhhyi
Copy link
Collaborator Author

dhhyi commented Jun 3, 2021

@MaxKless I went for the rebasing. Please have a look 😄

@shauke shauke added this to the 0.32 milestone Jun 4, 2021
MaxKless
MaxKless suggested changes Jun 7, 2021
View reviewed changes
Copy link
Collaborator

@MaxKless MaxKless left a comment

Choose a reason for hiding this comment

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

Thanks for the work but I think the fallback handling can still be improved.

src/app/core/internationalization.module.ts Outdated Show resolved Hide resolved
src/app/core/store/core/configuration/configuration.effects.ts Show resolved Hide resolved
Base automatically changed from feature/translations-output-hashed to develop June 7, 2021 13:03
@dhhyi dhhyi removed the wait Waiting for something (e.g. new ICM release) label Jun 7, 2021
@dhhyi dhhyi force-pushed the feature/translations-fallback-handling branch from c811354 to 8b59ef3 Compare June 7, 2021 18:51
@dhhyi dhhyi marked this pull request as draft June 7, 2021 20:57
@dhhyi dhhyi force-pushed the feature/translations-fallback-handling branch 3 times, most recently from a56ab3e to 8811ec7 Compare June 9, 2021 13:08
@dhhyi dhhyi marked this pull request as ready for review June 9, 2021 14:47
@dhhyi dhhyi requested a review from MaxKless June 9, 2021 14:47
MaxKless
MaxKless approved these changes Jun 10, 2021
View reviewed changes
Copy link
Collaborator

@MaxKless MaxKless left a comment

Choose a reason for hiding this comment

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

All around great changes!!

@MaxKless MaxKless merged commit 32c57bd into develop Jun 10, 2021
@MaxKless MaxKless deleted the feature/translations-fallback-handling branch June 10, 2021 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaxKless MaxKless MaxKless approved these changes

Assignees

@MaxKless MaxKless

Labels
enhancement Enhancement to an existing feature
Projects
None yet
Milestone
1.0
Development

Successfully merging this pull request may close these issues.
3 participants
@dhhyi @MaxKless @shauke