Bind the menu item form widget so its AJAX handlers can run - #57
Conversation
Any handler belonging to a widget inside the menu item editor popup fails with "A widget with class name formMenu<hash>ItemsMenuItemTitle has not been bound to the controller". With Winter.Translate installed that makes the ML controls' copy-from-locale and auto-translate handlers unusable on a static menu item. Two things kept the widget from ever being bound: The item form widget was made in prepareVars(), which only runs from render(). On an AJAX request Form::defineFormFields() binds the menuitems widget itself but never reaches the nested item form, so the widgets that Winter.Translate swaps in for `title` and `url` do not exist server-side. Make it on init() and bind it there, the way Backend\FormWidgets\Repeater already does for its items. The popup is a detached modal outside the editor's own form, so its requests reached the controller without formWidgetAlias / objectType / objectPath / theme. Index::index() gates bindFormWidgetToController() on those, so nothing was rebuilt at all. Clone them into the popup form when it opens. Reported by @mjauvin as a follow-up on wintercms#55. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe change caches the menu item form widget during initialization and reuses it when preparing variables. The widget receives a new Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change binds menu item form widgets and preserves the required popup request context so AJAX locale and editor actions can run; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Tested as a working solution. @LukeTowers any objection for this fix ? |
2 participants
Follow-up on #55, reported by @mjauvin: locale switching works now, but copying a value from another locale in the menu item editor throws.
Any AJAX handler belonging to a widget inside the Static Menus item editor popup fails with
"A widget with class name 'formMenu<hash>ItemsMenuItemTitle' has not been bound to the controller". With Winter.Translate installed that takes out the ML controls' copy-from-locale and auto-translate handlers ontitleandurl. Not a regression from #55 — that fix just got people far enough to hit it.Two reasons the widget is never bound:
MenuItems::prepareVars()only runs fromrender(), so on an AJAX request the nested item form is never made. Fixed by making it ininit()and binding it there, asBackend\FormWidgets\Repeateralready does for its items.formWidgetAlias/objectType/objectPath/themeandIndex::index()skipsbindFormWidgetToController(). Fixed by cloning those inputs into the popup form.Verified on Winter 1.2 / PHP 8.4 with Winter.Translate
dev-main: the copy popup opens and a Google-backed copyen→nl-NLlands correctly. Locale switching,onGetMenuItemTypeInfo, Apply, saving, and a new unsaved menu (emptyobjectPath) all still work.Summary by CodeRabbit