feat(bricks): CSS bundle selector in admin panel, remove legacy admin page - #134
Conversation
… page - Add `css_bundle` plugin setting (essential / optimal / full) so admins can choose which SLASHED CSS file is loaded on the frontend - Add `slashed_bricks_get_css_bundle()` helper; update `slashed_bricks_get_css_url()` and `class-enqueue.php` to use the configured bundle instead of hardcoded optimal - Extend REST `/settings` endpoint to accept and persist `css_bundle`; both `html_font_size` and `css_bundle` are now optional (partial-update semantics) - Add CSS Bundle selector to BundleTab.svelte, send both settings on save - Promote Svelte admin page to the top-level SLASHED menu (slug `slashed-bricks`, position 59, dashicons-art); remove the legacy jQuery admin page from the load chain - Rebuild admin-app bundle https://claude.ai/code/session_01Gq4QNEeg6sk74oSWqatzH4
|
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 (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR migrates the Bricks admin integration from a PHP-based form to a Svelte-based SPA mounted as a top-level menu page, and adds a CSS bundle selection (essential, optimal, full) that is validated, persisted, surfaced in the Svelte UI, and used to choose/cache-bust the frontend CSS URL. ChangesCSS Bundle Selection & Admin Page Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
- Use Slashed_Bricks_Token_Store::get_plugin_settings() in slashed_bricks_get_css_bundle() instead of direct get_option() call, consistent with the Token Store's single-source-of-truth design - Move class-token-store.php require_once to the top of the file so it is available before the helper functions that depend on it - Eliminate duplicate settings read in enqueue_frontend_styles() by deriving the CSS filename from the already-resolved URL via wp_parse_url/basename instead of calling get_css_bundle() a second time - Guard REST POST /settings against empty requests: return current settings without a write when neither field is present - Delete orphaned class-admin-page.php (was no longer loaded; keeping it on disk risked "class not found" errors for any code using its backwards-compat OPTION_NAME / SETTINGS_OPTION_NAME constants) - Fix trailing double space in slashed_bricks_get_css_url() docblock - Update stale slashed_bricks_admin_init() docblock - Restore explanatory comments in class-admin-page-svelte.php (hook suffix capture rationale, type=module regex reasoning, missing-bundle dev hint) https://claude.ai/code/session_01Gq4QNEeg6sk74oSWqatzH4
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)
integrations/bricks/includes/class-enqueue.php (1)
67-75:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard empty filename before local
filemtime()checks.When a filtered CSS URL has no file path,
$filenamecan be empty andfile_exists()/filemtime()may run against a directory path. Add a filename guard and fall back toSLASHED_BRICKS_VERSION.Suggested patch
- $filename = basename( (string) wp_parse_url( $css_url, PHP_URL_PATH ) ); - $repo_path = SLASHED_BRICKS_PATH . '../../dist/' . $filename; - $local_path = SLASHED_BRICKS_PATH . 'dist/' . $filename; + $filename = basename( (string) wp_parse_url( $css_url, PHP_URL_PATH ) ); + if ( '' === $filename ) { + $version = SLASHED_BRICKS_VERSION; + } else { + $repo_path = SLASHED_BRICKS_PATH . '../../dist/' . $filename; + $local_path = SLASHED_BRICKS_PATH . 'dist/' . $filename; - if ( file_exists( $repo_path ) ) { - $version = (string) filemtime( $repo_path ); - } elseif ( file_exists( $local_path ) ) { - $version = (string) filemtime( $local_path ); + if ( file_exists( $repo_path ) ) { + $version = (string) filemtime( $repo_path ); + } elseif ( file_exists( $local_path ) ) { + $version = (string) filemtime( $local_path ); + } else { + $version = SLASHED_BRICKS_VERSION; + } - } else { - $version = SLASHED_BRICKS_VERSION; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/bricks/includes/class-enqueue.php` around lines 67 - 75, Add a guard to ensure $filename is non-empty before calling file_exists()/filemtime() on $repo_path/$local_path: if empty($filename) set $version = SLASHED_BRICKS_VERSION and skip the file checks; otherwise proceed with the existing file_exists($repo_path)/file_exists($local_path) branches and set $version = (string) filemtime(...) as before (variables: $filename, $repo_path, $local_path, SLASHED_BRICKS_VERSION).
🧹 Nitpick comments (1)
integrations/bricks/includes/class-rest-controller.php (1)
126-133: ⚡ Quick winConsolidate CSS bundle allowlist into one source of truth.
The
essential|optimal|fullallowlist is duplicated here and in runtime resolution logic. Please centralize it (shared constant/helper) to prevent contract drift across REST validation and URL resolution.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/bricks/includes/class-rest-controller.php` around lines 126 - 133, Extract the CSS bundle allowlist ('essential','optimal','full') into a single shared constant or helper (e.g., ALLOWED_CSS_BUNDLES or get_allowed_css_bundles()) and replace the duplicated literal array in the 'css_bundle' field's validate_callback (the anonymous function in class-rest-controller.php) and in the runtime URL resolution logic to reference that constant/helper; keep the existing strict in_array casting/true flags and ensure the validate_callback still returns a boolean and sanitize as string before validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@integrations/bricks/includes/class-enqueue.php`:
- Around line 67-75: Add a guard to ensure $filename is non-empty before calling
file_exists()/filemtime() on $repo_path/$local_path: if empty($filename) set
$version = SLASHED_BRICKS_VERSION and skip the file checks; otherwise proceed
with the existing file_exists($repo_path)/file_exists($local_path) branches and
set $version = (string) filemtime(...) as before (variables: $filename,
$repo_path, $local_path, SLASHED_BRICKS_VERSION).
---
Nitpick comments:
In `@integrations/bricks/includes/class-rest-controller.php`:
- Around line 126-133: Extract the CSS bundle allowlist
('essential','optimal','full') into a single shared constant or helper (e.g.,
ALLOWED_CSS_BUNDLES or get_allowed_css_bundles()) and replace the duplicated
literal array in the 'css_bundle' field's validate_callback (the anonymous
function in class-rest-controller.php) and in the runtime URL resolution logic
to reference that constant/helper; keep the existing strict in_array
casting/true flags and ensure the validate_callback still returns a boolean and
sanitize as string before validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e1bef3ed-34a9-441e-b504-a6e65bf1c3a2
📒 Files selected for processing (8)
integrations/bricks/admin-app/src/components/BundleTab.svelteintegrations/bricks/assets/admin-app/app.cssintegrations/bricks/assets/admin-app/app.jsintegrations/bricks/includes/class-admin-page-svelte.phpintegrations/bricks/includes/class-admin-page.phpintegrations/bricks/includes/class-enqueue.phpintegrations/bricks/includes/class-rest-controller.phpintegrations/bricks/slashed-bricks.php
💤 Files with no reviewable changes (1)
- integrations/bricks/includes/class-admin-page.php
- Guard empty $filename in enqueue_frontend_styles() so a filtered CSS URL with no file path falls back to SLASHED_BRICKS_VERSION without running file_exists()/filemtime() against an unintended path - Centralise the css_bundle allowlist as Slashed_Bricks_Token_Store::ALLOWED_CSS_BUNDLES and replace the duplicated literal arrays in slashed_bricks_get_css_bundle() and the REST validate_callback to reference the single constant https://claude.ai/code/session_01Gq4QNEeg6sk74oSWqatzH4
css_bundleplugin setting (essential / optimal / full) so adminscan choose which SLASHED CSS file is loaded on the frontend
slashed_bricks_get_css_bundle()helper; updateslashed_bricks_get_css_url()and
class-enqueue.phpto use the configured bundle instead of hardcoded optimal/settingsendpoint to accept and persistcss_bundle;both
html_font_sizeandcss_bundleare now optional (partial-update semantics)slashed-bricks,position 59, dashicons-art); remove the legacy jQuery admin page from the load chain
https://claude.ai/code/session_01Gq4QNEeg6sk74oSWqatzH4
Summary by CodeRabbit
New Features
UI Changes
Settings