fix(assets): enqueue wu-admin styles on addon pages for wu-form modals#891
fix(assets): enqueue wu-admin styles on addon pages for wu-form modals#891superdav42 merged 1 commit intomainfrom
Conversation
PR #433 scoped admin asset enqueues to pages whose hook suffix contains `wp-ultimo`. Addon pages (e.g. multinetwork's `wu-networks`) have hook suffixes like `toplevel_page_wu-networks-network` that do not contain `wp-ultimo`, so wu-admin.css and wu-styling.css were never loaded. When those pages opened a wu-form modal (injected inline by wubox), the modal rendered with no styling. Changes: - Broaden wu_is_wu_page() to also match `_page_wu-`, covering both `toplevel_page_wu-*` and `{parent}_page_wu-*` slugs. The trailing hyphen avoids false positives on slugs like `wunderground`. - Add a new `wu_is_wu_page` filter so addons with non-standard slugs can explicitly opt in (or opt out). - Update PHPDoc with recognized patterns and filter usage example. - Add tests covering core, addon, false-positive, and filter cases. - Fix stale Scripts_Test.php assertions still referencing admin_init after the hook was moved to admin_enqueue_scripts in PR #433.
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/WP_Ultimo/Functions/Assets_Functions_Test.php (1)
139-172: Filter cleanup is not exception-safe.If an assertion in either filter test fails,
remove_filter()never runs and the callback leaks into subsequent tests in the same process, which can make later failures confusing. Consider moving the removal intotear_down()or wrapping withtry/finally. Minor — not blocking.Proposed refactor
public function test_wu_is_wu_page_filter_opt_in(): void { $callback = function ($is_wu_page, $hook_suffix) { if ('toplevel_page_my-custom-addon' === $hook_suffix) { return true; } return $is_wu_page; }; add_filter('wu_is_wu_page', $callback, 10, 2); - - $this->assertTrue(wu_is_wu_page('toplevel_page_my-custom-addon')); - - remove_filter('wu_is_wu_page', $callback, 10); + try { + $this->assertTrue(wu_is_wu_page('toplevel_page_my-custom-addon')); + } finally { + remove_filter('wu_is_wu_page', $callback, 10); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Functions/Assets_Functions_Test.php` around lines 139 - 172, The tests test_wu_is_wu_page_filter_opt_in and test_wu_is_wu_page_filter_opt_out add a transient filter with add_filter but call remove_filter only at the end, so a failed assertion can leak the callback; fix by making cleanup exception-safe: either move removal into the test class tear_down() (implement tear_down() to call remove_filter for the same callback) or wrap each add_filter/remove_filter pair in a try/finally so remove_filter always runs; reference the existing callback closures and the add_filter/remove_filter calls to ensure the exact same callback and priority (10) are removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/WP_Ultimo/Functions/Assets_Functions_Test.php`:
- Around line 139-172: The tests test_wu_is_wu_page_filter_opt_in and
test_wu_is_wu_page_filter_opt_out add a transient filter with add_filter but
call remove_filter only at the end, so a failed assertion can leak the callback;
fix by making cleanup exception-safe: either move removal into the test class
tear_down() (implement tear_down() to call remove_filter for the same callback)
or wrap each add_filter/remove_filter pair in a try/finally so remove_filter
always runs; reference the existing callback closures and the
add_filter/remove_filter calls to ensure the exact same callback and priority
(10) are removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dfffc000-59b3-4fd7-8092-fdc3d72afa3e
📒 Files selected for processing (3)
inc/functions/assets.phptests/WP_Ultimo/Functions/Assets_Functions_Test.phptests/WP_Ultimo/Scripts_Test.php
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
|
Performance Test Results Performance test results for 7ba9e58 are in 🛎️! Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown. URL:
|
1 participant
Summary
/wp-admin/network/admin.php?page=wu-networkswu_is_wu_page()to also match hook suffixes containing_page_wu-, covering addon slug conventionswu_is_wu_pagefilter so addons with non-standard slugs can opt inRoot cause
PR #433 (commit cb5a40f) introduced
wu_is_wu_page()which only returned true for hook suffixes containingwp-ultimo. This correctly scoped 150KB+ of CSS/JS away from unrelated admin pages, but also broke addon pages whose slugs start withwu-(the long-standing convention for addons).On the multinetwork page, clicking "Add New Network" opens a wu-form via wubox's
createAjaxBox, which injects the form content inline into the parent document. Withoutwu-admin.cssloaded on the parent page, the modal rendered completely un-styled.Changes
inc/functions/assets.php—wu_is_wu_page()wp-ultimo(core) and_page_wu-(addons) in the hook suffix._page_wu-prevents false positives on unrelated slugs likewunderground.wu_is_wu_pagefilter for explicit opt-in/opt-out.tests/WP_Ultimo/Functions/Assets_Functions_Test.php— 8 new test cases covering core, addon (toplevel / network / submenu), unrelated pages, the false-positive guard, and the filter opt-in/opt-out.tests/WP_Ultimo/Scripts_Test.php— bonus: fix stale assertions still referencingadmin_initafter PR #433 moved the enqueue hook toadmin_enqueue_scripts.Testing
Backward compatibility
No breaking changes. Addon developers using non-
wu-slugs can register via the filter:Summary by CodeRabbit
New Features
Improvements
Tests