Skip to content

Content Workspace: Add variant sync when switching app culture (closes #16853)#22566

Open
engijlr wants to merge 3 commits intomainfrom
v17/bugfix/content-workspace-change-with-app-culture
Open

Content Workspace: Add variant sync when switching app culture (closes #16853)#22566
engijlr wants to merge 3 commits intomainfrom
v17/bugfix/content-workspace-change-with-app-culture

Conversation

@engijlr
Copy link
Copy Markdown
Contributor

@engijlr engijlr commented Apr 22, 2026

Summary

Adds a check in the appLanguageCulture observer in document-workspace-editor.element.ts. When the culture changes, if the user is on a single-variant view and the document has a variant for the new culture, it updates the URL to the new culture.

Test plan

  • Create two languages (e.g. English and Spanish)
  • Create a document type with "Allow vary by culture" enabled
  • Create content with values in both languages
  • Open the content node
  • Switch the language in the sidebar picker
  • Verify the workspace content switches to the selected language

Fixes #16853

@claude
Copy link
Copy Markdown

claude Bot commented Apr 22, 2026

Claude finished @engijlr's task in 2m 45s —— View job


PR Review

Target: origin/main · Based on commit: 37b25a38

Fixes the language-switch UX bug (#16853): when the app language context changes and the user is viewing a single-variant document route matching the previous culture, the URL is synchronously updated via history.replaceState to the new culture — provided a variant for the new culture exists.


Suggestions

  • document-workspace-editor.element.ts:44: The replaceState call preserves window.location.search but omits window.location.hash. The existing fallback in #generateRoutes() (line 150) also omits it, so this is consistent with the existing code — but since you're already threading through the query string, it would be worth rounding this out with + window.location.hash for correctness in edge cases where a fragment is present.

  • document-workspace-editor.element.ts:40–45: The new lines use + string concatenation (this.#workspaceRoute + '/' + previousCulture) while the equivalent line already in #generateRoutes() uses a template literal (\${this.#workspaceRoute}/${path}...``). Consider switching to template literals for consistency with the surrounding code:

    const expectedOldPath = `${this.#workspaceRoute}/${previousCulture}`;
    // ...
    history.replaceState(null, '', `${this.#workspaceRoute}/${appCulture}${window.location.search}`);
  • No tests added. Per project conventions, new behavior should have accompanying tests. This is a browser-routing concern (making it hard to cover with a pure unit test), but if there's an existing workspace-editor or router test harness, it would be worth adding a case that verifies the URL is updated on culture change and is left unchanged on split-view paths.


Notes

Ordering: #generateRoutes() is called before history.replaceState. Because Lit batches re-renders as microtasks, both the _routes update and the URL update complete synchronously before any re-render occurs. The router slot sees the correct routes and URL together — no timing issue here.

Split-view paths are correctly left alone: currentPath === expectedOldPath will not match a culture1_&_culture2 URL.

Media Workspace Editor: The TODO comment on line 10 notes that this element is "fully identical" with the Media Workspace Editor. However, UmbMediaWorkspaceEditorElement does not consume UMB_APP_LANGUAGE_CONTEXT at all, so this fix is not needed there.

No breaking changes detected. The change modifies internal behavior only — no public API surface, no exported types, no custom element interface changes.


Approved with Suggestions for improvement

Good to go, but please carefully consider the importance of the suggestions.

@claude claude Bot added area/frontend category/ux User experience category/ui User interface labels Apr 22, 2026
@AndyButland
Copy link
Copy Markdown
Contributor

I was just looking to tackle the testing part of this @engijlr. Looks a nice addition and editor friendly update. It mostly works fine, but I found a few edge cases.

  • Go to a variant page, switch language - the document language switches. ✅
  • Go to an invariant page, switch language - no change. ✅
  • Go to a variant page, click to the "Info" view, switch language - this now doesn't work. It also doesn't work when I switch back to the "Content" view. I have to click to another page for it to start working again. ❌
  • Go to a page that varies by culture and segment, open the default segment in one language, switch language - the document language switches. ✅
  • Go to a page that varies by culture and segment, open a non-default segment in one language, switch language - no change. I'd expect to go to the same segment, or at least the default segment, on the selected language ❌

const currentPath = window.location.pathname;
const expectedOldPath = `${this.#workspaceRoute}/${previousCulture}`;
if (currentPath === expectedOldPath && this.#variants?.some((v) => v.unique === appCulture)) {
history.replaceState(null, '', `${this.#workspaceRoute}/${appCulture}${window.location.search}`);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could this avoid removing the rest of the URL, so if you have navigated to the info workspace view, then it would be nice to keep that part of the URL. in other words keeping the rest of the url. :-)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed the two edge cases by extracting the URL sync into a #syncUrlToCulture method. The main issue was that the old code did an exact path match, so it broke when there was extra stuff in the URL like /view/info.

Now it splits the variant part from the rest of the path and keeps whatever comes after. Also, instead of comparing the full variant string, it looks up the variant and matches by culture. I tested all the scenarios manually except segments since I don't have a setup for that.

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

Labels

area/frontend category/ui User interface category/ux User experience

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Content Workspace: Does not change variant when switching app culture

3 participants