Skip to content

Fix Winter.Translate default locale detection in the menu item editor - #55

Merged
LukeTowers merged 1 commit into
wintercms:mainfrom
AIC-BV:fix/menu-item-default-locale
Aug 25, 2026
Merged

Fix Winter.Translate default locale detection in the menu item editor#55
LukeTowers merged 1 commit into
wintercms:mainfrom
AIC-BV:fix/menu-item-default-locale

Conversation

@AIC-BV

@AIC-BV AIC-BV commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #56 — the issue is filed on Winter.Translate, but the broken code lives here.

Problem

With Winter.Translate installed, editing a Static Menu item and switching the locale back and forth blanks the title, and the default locale's hidden data locker is never filled in.

menu-items-editor.js locates the active ML control in three places with:

var defaultLocale = $('[data-control="multilingual"]').data('default-locale')

data-control="multilingual" is only rendered by Winter.Translate's mlurl widget. EventRegistry::registerMenuItemTranslation() turns the menu item's title and url into mltext, whose root element is data-control="mltext". So inside the Static Menus popup that selector matches nothing and defaultLocale is undefined, which silently disables all three code paths:

  • loadProperties() never copies the item's own title into RLTranslate[<default>][title], so the default locale locker stays empty;
  • applyMenuItem() never reads the default locale back, so the item's title is taken from whatever locale is active at the time;
  • the select[name=reference] handler never seeds the default locale locker with the auto-filled title.

Observed on a site with default locale nl-BE, item title "Maatkasten", en → "Our closets", fr-BE → "Nos armoires":

$('[data-control="multilingual"]').length  ->  0
RLTranslate[nl-BE][title]                  ->  ""     <-- default locale locker empty
RLTranslate[en][title]                     ->  "Our closets"
RLTranslate[fr-BE][title]                  ->  "Nos armoires"

switch to en     -> input shows "Our closets"
switch back nl-BE-> input shows ""                    <-- title wiped

Applying the popup while a non-default locale was active then wrote that locale's title (or an empty one) into the item's title property.

Fix

Select on [data-default-locale] instead — all nine of Winter.Translate's ML widget partials carry that attribute, so this works whichever ML control the field was replaced with — and scope the lookup to the popup container.

loadProperties() also gets the real $popupContainer passed to it: that method has no self in scope, so self.$popupContainer was resolving to the global window.self and the lookups were silently falling back to a document-wide search.

Testing

Same menu item, after the change:

RLTranslate[nl-BE][title]  ->  "Maatkasten"
en -> "Our closets" | fr-BE -> "Nos armoires" | back to nl-BE -> "Maatkasten"

Applying the popup with en active now keeps title: "Maatkasten" and leaves viewBag.locale intact.

Verified against Winter.Translate dev-main, Winter 1.2, PHP 8.4.

Summary by CodeRabbit

  • Bug Fixes
    • Improved menu item editing in multilingual setups by consistently using the default locale configured for the current editor popup.
    • Fixed locale data lookup when loading and applying menu item properties.

The editor located the active ML control with [data-control="multilingual"],
an attribute only the mlurl widget renders. Winter.Translate turns the menu
item's `title` and `url` fields into mltext (data-control="mltext"), so in the
Static Menus popup the selector matched nothing and `defaultLocale` was
undefined. As a result the RLTranslate[<default>][...] data lockers were never
populated, and the item's own properties were read from whichever locale
happened to be active - switching the locale back and forth wiped the title.

Select on [data-default-locale] instead: all nine ML widget partials carry it.
Scope the lookup to the popup container while we're here.

Also pass the real $popupContainer in loadProperties(). That method has no
`self` in scope, so `self.$popupContainer` resolved to the global `window.self`
and the lookups silently fell back to a document-wide search.

Fixes wintercms/wn-translate-plugin#120

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: acbe0a47-0bab-46cd-a2bc-5150421a5c38

📥 Commits

Reviewing files that changed from the base of the PR and between b9655e1 and 003ef47.

📒 Files selected for processing (1)
  • formwidgets/menuitems/assets/js/menu-items-editor.js

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The menu item editor now reads the default locale from the active popup container in onItemClick, loadProperties, and applyMenuItem. The Winter.Translate default-locale data locker lookup in loadProperties also uses the provided popup container.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 003ef

This change corrects locale detection and popup scoping in the menu item editor so default-language titles are preserved when switching locales; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address issue #120 by locating the default locale data within the popup and passing the correct popup container to property loading. This supports preservation of default and translated St…
Out of Scope Changes check ✅ Passed The changes are limited to default locale detection and popup-container scoping in the menu item editor. No unrelated code changes are identified.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: fixing Winter.Translate default locale detection in the menu item editor.
Full details: Linked Issues check

Explanation

The changes address issue #120 by locating the default locale data within the popup and passing the correct popup container to property loading. This supports preservation of default and translated Static Menu item values when switching locales.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@LukeTowers
LukeTowers merged commit b770396 into wintercms:main Aug 25, 2026
5 checks passed
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.

StaticMenu items translation is broken when switching the locale back and forth in the backend.

2 participants