Conversation
🤖 Pull request artifacts
|
📝 WalkthroughWalkthroughAdds three admin options to control a floating “disable optimization” toggle (including a frontend-only visibility flag and persistence), exposes them via localized settings, adds toggle UI/CSS, guards media upload handlers to respect the toggle, and wires new settings into the admin settings page. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin UI
participant Server as Backend (class-admin / class-script-loader)
participant Page as Frontend Page (localized cimoSettings)
participant Media as Media Handlers (drop-zone / select-files)
Admin->>Server: Save options (`show_optimization_toggle`, `show_optimization_toggle_frontend`, `persist_optimization_toggle`)
Server-->>Admin: Persist via REST `cimo_options`
Server->>Page: Localize `cimoSettings` including show/persist flags
Page->>Page: Render floating toggle if `showOptimizationToggle === 1` and `showOptimizationToggleFrontend` allows
Page->>Page: Apply persisted state from localStorage if `persistOptimizationToggle === 1`
Page->>Media: Set `window.cimoSettings.disableOptimization` from toggle state
Media->>Media: If `disableOptimization` truthy → early-return (skip optimization)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/admin/css/index.css`:
- Around line 150-151: The hover selector currently targets
.cimo-optimization-toggle:hover but the markup uses
.cimo-optimization-toggle-container, so the hover rule never applies; update the
CSS selector to target .cimo-optimization-toggle-container:hover (or include
both .cimo-optimization-toggle:hover, .cimo-optimization-toggle-container:hover)
and keep the existing background declaration so the intended hover background is
applied to the container element.
In `@src/admin/js/page/admin-settings.js`:
- Around line 495-496: The disabled premium toggles currently force the checked
prop to 0 for free builds (e.g. the JSX using checked={ buildType === 'premium'
? settings.optimizeAllMedia === 1 : 0 }), which causes the UI to misrepresent
the stored value; change the checked expression to always reflect the stored
setting (e.g. checked={ settings.optimizeAllMedia === 1 }) and keep disabled={
buildType === 'free' } so the control is non-interactive but still shows the
true state; apply the same fix to the other affected toggle usages referenced in
this file (the other checked props at the locations noted).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1085ea1d-85fd-4d70-b5f1-a60cbcae4b8d
📒 Files selected for processing (7)
src/admin/class-admin.phpsrc/admin/class-script-loader.phpsrc/admin/css/admin-page.csssrc/admin/css/index.csssrc/admin/js/media-manager/drop-zone.jssrc/admin/js/media-manager/select-files.jssrc/admin/js/page/admin-settings.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/admin/js/media-manager/drop-zone.js`:
- Around line 42-44: The front-end guard
window.cimoSettings?.disableOptimization never triggers because the localization
payload built in src/admin/class-script-loader.php doesn't include a
disableOptimization key; update the payload generator in that file (the method
that builds/localizes cimoSettings) to add a boolean "disableOptimization"
property (derived from the plugin/config option or default false) so that
drop-zone.js and select-files.js can read
window.cimoSettings.disableOptimization and correctly disable optimization when
true.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8db2b13-bc0e-4684-9ee3-02ba36be0874
📒 Files selected for processing (2)
src/admin/class-script-loader.phpsrc/admin/js/media-manager/drop-zone.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/admin/class-script-loader.php
| if ( window.cimoSettings?.disableOptimization ) { | ||
| return | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Find every read/write of disableOptimization:"
rg -n -C2 --type=js --type=php '\bdisableOptimization\b'
echo
echo "2) Inspect cimoSettings localization payload(s):"
rg -n -C6 --type=php "wp_localize_script\\(|['\"]cimoSettings['\"]"
echo
echo "Expected:"
echo "- At least one initialization/assignment path for window.cimoSettings.disableOptimization"
echo " that runs before drop/select listeners are attached."Repository: gambitph/Cimo
Length of output: 2782
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Searching for dynamic assignments to cimoSettings or disableOptimization ==="
rg -n 'cimoSettings\s*=|cimoSettings\[|window\.cimoSettings' --type=js --type=php
echo ""
echo "=== Checking admin settings page for disable optimization toggle ==="
rg -n -i 'disable.*optim|optim.*disable|toggle.*disable' --type=js --type=php
echo ""
echo "=== Searching for any REST endpoint that might set this state ==="
rg -n -i 'disableOptimization|disable_optimization' --type=phpRepository: gambitph/Cimo
Length of output: 3007
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for floating toggle implementation ==="
rg -n -i 'floating.*toggle|toggle.*optim|showOptimizationToggle' --type=js -A5 -B5
echo ""
echo "=== Search for files in admin/js that might contain toggle logic ==="
fd -e js --path 'admin/js' | head -20
echo ""
echo "=== Check if there's a separate toggle component or widget ==="
rg -n 'class.*Toggle|function.*[Tt]oggle|const.*Toggle' --type=js --path 'admin' | head -30Repository: gambitph/Cimo
Length of output: 8480
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for toggle widget or component ==="
fd -e js -e jsx --path 'admin' | xargs rg -l -i 'toggle.*widget|toggle.*component|floating.*toggle' | head -10
echo ""
echo "=== List all files in admin/js to understand structure ==="
find src/admin/js -type f -name '*.js' -o -name '*.jsx' | sort
echo ""
echo "=== Search for any localStorage or sessionStorage writes related to optimize ==="
rg -n 'localStorage|sessionStorage' --type=js --path 'admin' -B2 -A2
echo ""
echo "=== Check for window mutation patterns on cimoSettings ==="
rg -n 'window\.' --type=js --path 'admin/js' | grep -i 'optim\|disable' | head -20Repository: gambitph/Cimo
Length of output: 389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== List all files in admin/js ==="
find src/admin/js -type f \( -name '*.js' -o -name '*.jsx' \) | sort
echo ""
echo "=== Search for localStorage or sessionStorage ==="
rg -n 'localStorage|sessionStorage' --type=js | grep -v node_modules
echo ""
echo "=== Search for any toggle or optimization-related JS files ==="
find src -type f \( -name '*toggle*' -o -name '*optimize*' \) | grep -E '\.(js|jsx)$'
echo ""
echo "=== Check if toggle state is managed inline in drop-zone or select-files ==="
cat src/admin/js/media-manager/drop-zone.jsRepository: gambitph/Cimo
Length of output: 501
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Read full drop-zone.js to understand context ==="
wc -l src/admin/js/media-manager/drop-zone.js
echo ""
echo "=== Read the file ==="
cat -n src/admin/js/media-manager/drop-zone.js
echo ""
echo "=== Read full select-files.js ==="
cat -n src/admin/js/media-manager/select-files.jsRepository: gambitph/Cimo
Length of output: 17353
disableOptimization guard will never activate — property is never initialized
The guards at Lines 42 (drop-zone.js) and 34–37 (select-files.js) check window.cimoSettings?.disableOptimization, but this property is never added to the localization payload in src/admin/class-script-loader.php. Since nothing initializes or sets this value, the guards remain inert and the intended disable-optimization behavior is non-functional.
Suggested fix
--- a/src/admin/class-script-loader.php
+++ b/src/admin/class-script-loader.php
@@ -84,6 +84,7 @@ public function enqueue_scripts() {
'restUrl' => rest_url( 'cimo/v1/' ),
'nonce' => wp_create_nonce( 'wp_rest' ),
'isFrontend' => ! is_admin(),
+ 'disableOptimization' => 0,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/admin/js/media-manager/drop-zone.js` around lines 42 - 44, The front-end
guard window.cimoSettings?.disableOptimization never triggers because the
localization payload built in src/admin/class-script-loader.php doesn't include
a disableOptimization key; update the payload generator in that file (the method
that builds/localizes cimoSettings) to add a boolean "disableOptimization"
property (derived from the plugin/config option or default false) so that
drop-zone.js and select-files.js can read
window.cimoSettings.disableOptimization and correctly disable optimization when
true.
|
@Arukuen add another toggle for "Show in frontend" that appears when "Show Optimization Toggle" is enabled. By default, the "Show optimization toggle" should not be present only in the frontend and should only show in the backend. The new toggle for "Show in frontend" should show the toggle in the frontend only when this is toggled on. Important: Make sure that the toggle is only visible when there is an upload form in the current page in the frontend. Use the title: Show toggle in frontend |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/admin/class-script-loader.php (1)
13-29:⚠️ Potential issue | 🔴 CriticalFrontend script enqueue required for advertised feature. The
showOptimizationToggleFrontendsetting is exposed in the admin UI with explicit messaging about showing the optimization toggle on frontend pages and user-facing forms. However, the script is only enqueued in admin and page-builder contexts—there's nowp_enqueue_scriptshook for general frontend pages. Additionally, the frontend build entry in webpack is disabled (marked TODO "Not currently used. Remove"), andsrc/frontend/index.jsis just a placeholder. This creates a non-functional feature: users can enable the setting, but the toggle will never appear on frontend pages. Either implement frontend enqueue viawp_enqueue_scriptshook, or remove the frontend-facing setting from the UI if this feature is deferred.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/admin/class-script-loader.php` around lines 13 - 29, The frontend toggle setting (showOptimizationToggleFrontend) is never applied because assets are only enqueued for admin/page-builder hooks; add a frontend enqueue or remove the setting. Fix by either (A) registering the frontend enqueue in the class constructor (add_action('wp_enqueue_scripts', [ __CLASS__, 'enqueue_cimo_assets' ])), enable the frontend webpack entry, and implement the frontend behavior in src/frontend/index.js so the toggle renders on public pages, or (B) if the frontend feature is deferred, remove the showOptimizationToggleFrontend UI option and any frontend-related copy. Target symbols: the class constructor, enqueue_cimo_assets, showOptimizationToggleFrontend, src/frontend/index.js, and the frontend webpack entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/admin/class-script-loader.php`:
- Around line 13-29: The frontend toggle setting
(showOptimizationToggleFrontend) is never applied because assets are only
enqueued for admin/page-builder hooks; add a frontend enqueue or remove the
setting. Fix by either (A) registering the frontend enqueue in the class
constructor (add_action('wp_enqueue_scripts', [ __CLASS__, 'enqueue_cimo_assets'
])), enable the frontend webpack entry, and implement the frontend behavior in
src/frontend/index.js so the toggle renders on public pages, or (B) if the
frontend feature is deferred, remove the showOptimizationToggleFrontend UI
option and any frontend-related copy. Target symbols: the class constructor,
enqueue_cimo_assets, showOptimizationToggleFrontend, src/frontend/index.js, and
the frontend webpack entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6a38d04b-3c62-495e-a26e-c6401985cd80
📒 Files selected for processing (3)
src/admin/class-admin.phpsrc/admin/class-script-loader.phpsrc/admin/js/page/admin-settings.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/admin/js/page/admin-settings.js
fixes #36
Summary by CodeRabbit
New Features
Bug Fixes
Style