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

feat: language switch with multisite support #722

Merged
merged 23 commits into from
Jul 1, 2021

Conversation

MaxKless
Copy link
Collaborator

@MaxKless MaxKless commented Jun 9, 2021

PR Type

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no API changes)
[ ] Build-related changes
[ ] CI-related changes
[ ] Documentation content changes
[ ] Application / infrastructure changes
[ ] Other:

What Is the Current Behavior?

Currently, the language switch only changes the language and completely ignores any multisite config.

What Is the New Behavior?

Now the language switch works with our new default multi site setup and can be configured/overridden to work with arbitrary multi site configurations.

Does this PR Introduce a Breaking Change?

[ ] Yes
[ x No

Other Information

@MaxKless MaxKless changed the base branch from develop to chore/new-demo-multisite-config June 9, 2021 12:37
@MaxKless MaxKless changed the title Feat/language switch multisite feat: language switch with multisite support Jun 9, 2021
@MaxKless MaxKless self-assigned this Jun 10, 2021
@shauke shauke self-requested a review June 14, 2021 07:09
@shauke shauke added the feature New feature or request label Jun 14, 2021
@shauke shauke added this to the 0.32 milestone Jun 14, 2021
Base automatically changed from chore/new-demo-multisite-config to develop June 16, 2021 07:32
@shauke shauke force-pushed the feat/language-switch-multisite branch from f0c1329 to 27741e9 Compare June 17, 2021 17:47
Copy link
Collaborator

@shauke shauke left a comment

Choose a reason for hiding this comment

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

The generated links in the language switch component look like this now for our standard multi-site configuration: http://localhost:4200/de/home;redirect=1;lang=de_DE
I would have expected http://localhost:4200/de/home. What is the reason for keeping the ;redirect=1;lang=de_DE part. Isn't this still doing things that would no longer be necessary?

docs/guides/multi-site-configurations.md Outdated Show resolved Hide resolved
docs/guides/multi-site-configurations.md Show resolved Hide resolved
docs/guides/multi-site-configurations.md Outdated Show resolved Hide resolved
src/environments/environment.model.ts Outdated Show resolved Hide resolved
@shauke

This comment has been minimized.

@shauke shauke force-pushed the feat/language-switch-multisite branch from 27741e9 to 223e097 Compare June 18, 2021 12:25
dhhyi
dhhyi previously requested changes Jun 20, 2021
Copy link
Collaborator

@dhhyi dhhyi left a comment

Choose a reason for hiding this comment

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

I think the multiSiteLocaleMap must not only be a environment.ts property, but an environment property. It should also be possible to define complete URLs to switch to so that this feature also works when the multi-site setup is using different domains for each channel.

@MaxKless MaxKless force-pushed the feat/language-switch-multisite branch from 223e097 to 628f24e Compare June 23, 2021 15:43
src/environments/environment.model.ts Outdated Show resolved Hide resolved
src/app/core/utils/multi-site/multi-site.service.ts Outdated Show resolved Hide resolved
package-lock.json Outdated Show resolved Hide resolved
src/environments/environment.model.ts Outdated Show resolved Hide resolved
@MaxKless MaxKless requested a review from shauke June 29, 2021 14:07
@shauke shauke requested a review from dhhyi June 30, 2021 12:05
@MaxKless MaxKless requested a review from shauke July 1, 2021 08:43
@shauke shauke dismissed dhhyi’s stale review July 1, 2021 12:16

partly resolved

@MaxKless MaxKless merged commit 4949be1 into develop Jul 1, 2021
@MaxKless MaxKless deleted the feat/language-switch-multisite branch July 1, 2021 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants