fix(assets): restore admin localization of package vars - #1380
fix(assets): restore admin localization of package vars#1380marklchaves wants to merge 1 commit into
Conversation
autoload_styles_for_scripts() is only ever hooked to wp_print_scripts (frontend) and admin_print_scripts (wp-admin). The added 'admin_print_scripts' !== current_filter() guard from #1316 meant the localization branch could never run in wp-admin at all, since that's the only admin hook it's called on. This silently stopped wp_localize_script()/pum_localize_script() from running for every non-bundled admin package (block-editor, admin-bar, cta-admin, cta-editor, dashboard), leaving window.popupMakerBlockEditor and similar globals undefined. In the block editor this crashed block-editor.js at module-evaluation time (destructuring popupTriggerExcludedBlocks off an undefined object), which halted the rest of that bundle's top-level code, including the Popup Title panel's registerPlugin() call. The $this->localized_packages per-request cache added in the same commit already prevents double-localizing, making the hook-name check redundant as well as wrong. Drop it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. WalkthroughThe package-variable localization guard no longer skips ChangesAdmin package localization
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This localized change restores admin package variables while preserving existing enqueue and duplicate-localization safeguards; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Linked Issues checkExplanation The change removes the admin hook guard that prevented package localization. This restores window.popupMakerBlockEditor and other admin package variables, which addresses the missing Popup Title panel and related error in issue ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Summary
Fixes #1379.
autoload_styles_for_scripts()inclasses/Controllers/Assets.phpis only ever hooked towp_print_scripts(front end) andadmin_print_scripts(wp-admin). A guard added in52a17cb1(#1316) skipped the localization branch whenever the current hook wasadmin_print_scripts. Since that's the only wp-admin hook this method runs on, the guard stoppedwp_localize_script()/pum_localize_script()from ever running in wp-admin, for every non-bundled admin package (block editor, admin bar, CTA admin, CTA editor, dashboard).In the block editor, this left
window.popupMakerBlockEditorundefined. A module readspopupTriggerExcludedBlocksoff that object at the top of the file, with no fallback, so it throws right away. The build bundles every module into one file, so the error stopped the rest of that file from running, including the line that registers the Popup Title sidebar panel.Fix
Drop the
'admin_print_scripts' !== current_filter()check. The$this->localized_packagesper-request cache added in the same commit already stops duplicate localization, so the hook-name check was redundant, and wrong.Test plan
pnpm run release) from this branch and installed on a local test site.varsName/vars(admin bar, CTA admin, CTA editor, dashboard) still localize correctly in wp-admin.Summary by CodeRabbit