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

Reload the application on language switch #681

Merged
merged 4 commits into from
Nov 11, 2023
Merged

Reload the application on language switch #681

merged 4 commits into from
Nov 11, 2023

Conversation

fgravin
Copy link
Member

@fgravin fgravin commented Nov 11, 2023

For multilingual concerns, after that the user switches the language, the application should

  • translate all the UI
  • translate all the multingual content of the metadata
  • use the language as a boost factor within the search

The easiest way to achieve all of that is to reload the page for now.

Note: the language-switcher is a UI component and should not interact with the localStorage IMO. I didn't want to refactor so I added the reload along the storage process.

The PR also modifies the content of the language switcher, to translate the entries from the targeted language.

@fgravin fgravin requested a review from jahow November 11, 2023 09:16
Copy link
Contributor

github-actions bot commented Nov 11, 2023

Affected libs: ui-catalog, feature-catalog, feature-record, feature-router,
Affected apps: metadata-editor, datahub, demo, webcomponents, search,

  • 🚀 Build and deploy storybook and demo on GitHub Pages
  • 📦 Build and push affected docker images

@coveralls
Copy link

coveralls commented Nov 11, 2023

Coverage Status

coverage: 87.74% (+3.8%) from 83.925%
when pulling ad9fbe9 on lang-reload
into 61cb3a9 on main.

Copy link
Collaborator

@jahow jahow left a comment

Choose a reason for hiding this comment

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

Looking good, thanks for changing the translations of the language switcher option, it's indeed more logical now. I think the changes to the default config should be reverted, was that intentional?

The language switcher is a very simple component so it wasn't made into a smart/presentation couple. On the other hand it might be useful to abstract the logic for reading/writing the current language into a separate service.

conf/default.toml Outdated Show resolved Hide resolved
conf/default.toml Outdated Show resolved Hide resolved
@fgravin
Copy link
Member Author

fgravin commented Nov 11, 2023

Thanks @jahow yes I unintentionally pushed the configuration.

@fgravin fgravin merged commit 54ff36a into main Nov 11, 2023
7 checks passed
@fgravin fgravin deleted the lang-reload branch November 11, 2023 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants