refactor: move token infrastructure to global includes/ - #169
Merged
Conversation
Token overrides (--sf-* CSS variables), REST API, admin page, and supporting classes are framework-level concerns shared by all builder integrations — not Bricks-specific. Moved from integrations/bricks/includes/ → includes/: class-token-store.php → Slashed_Token_Store (was Slashed_Bricks_Token_Store) class-token-sanitizer.php → Slashed_Token_Sanitizer class-token-defaults.php → Slashed_Token_Defaults class-tab-registry.php → Slashed_Tab_Registry class-css-generator.php → Slashed_CSS_Generator class-rest-controller.php → Slashed_REST_Controller (namespace: slashed/v1) class-admin-page-svelte.php → class-token-page.php (Slashed_Token_Page) slashed.php now owns: token class loading, REST route registration (slashed/v1), global override CSS injection (wp_enqueue_scripts + enqueue_block_editor_assets at priority 20), and admin page bootstrap. slashed-bricks.php: standalone-mode fallback loads shared classes from ../../includes/; Bricks REST endpoints (rebemer, fonts) updated to slashed/v1 namespace; override CSS injection removed from Enqueue class (now handled globally). Slashed_Token_Store performs a one-time migration from the legacy slashed_bricks_tokens option to slashed_tokens on first read. https://claude.ai/code/session_01DU3r3kT7GqH7w7jeadV2DT
|
Warning Review limit reached
More reviews will be available in 46 minutes and 40 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR consolidates Bricks-specific token management infrastructure (store, sanitizer, REST API, admin page) into unified, shared plugin components registered under the ChangesUnified Token Infrastructure
Possibly related PRs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 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 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.
Actionable comments posted: 6
🧹 Nitpick comments (2)
integrations/bricks/includes/class-rebemer-rest.php (1)
71-72: 💤 Low valueStale docblock reference to the "legacy admin REST controller."
register_routes()is no longer called "alongside the legacy admin REST controller" —slashed_bricks_rest_routes_init()now registers only the Bricks-specific endpoints, with the token CRUD controller registered globally byslashed.php. Update this note for accuracy.🤖 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-rebemer-rest.php` around lines 71 - 72, Docblock text is stale: update the comment for register_routes() so it no longer claims it runs "alongside the legacy admin REST controller"; instead state that slashed_bricks_rest_routes_init() registers only Bricks-specific endpoints and that the token CRUD controller is registered globally by slashed.php. Edit the docblock on register_routes() in class-rebemer-rest.php to replace the legacy admin controller reference with this accurate description and keep function names exactly as mentioned (register_routes(), slashed_bricks_rest_routes_init(), slashed.php, token CRUD controller) for clarity.integrations/bricks/slashed-bricks.php (1)
138-152: 💤 Low valueStale docblock references the removed
slashed_bricks_admin_init().Lines 146-151 still describe a now-deleted admin-init path ("ALSO instantiated inside
slashed_bricks_admin_init()"), which no longer exists after this refactor. Trim this paragraph to avoid misleading future readers, since the controller is now registered globally byslashed.php.🤖 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/slashed-bricks.php` around lines 138 - 152, The docblock still mentions a removed admin-init path; remove or trim the paragraph referencing slashed_bricks_admin_init() so it no longer claims the REST controller is "ALSO instantiated inside `slashed_bricks_admin_init()`"; keep the explanation that routes are registered via rest_api_init and note that the controller is now registered globally by slashed.php (and that NAMESPACE is available for wp_localize_script) to avoid misleading readers.
🤖 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.
Inline comments:
In `@includes/class-css-generator.php`:
- Around line 32-33: The code is directly calling get_option(...) which bypasses
the legacy migration in Slashed_Token_Store; replace those direct reads with
Slashed_Token_Store::get_settings() so the migration path in
includes/class-token-store.php runs; specifically update the has_overrides()
method and the other read sites around the 64-69 region to call
Slashed_Token_Store::get_settings() (instead of
get_option(Slashed_Token_Store::OPTION_NAME, array())) and use the returned
array for checks so existing installs trigger the "first load" migration.
In `@includes/class-rest-controller.php`:
- Around line 300-316: The import currently seeds $all from
Slashed_Token_Store::get_settings(), causing a merge instead of replacing the
token state; change the logic so you build a fresh settings array from
$body['tokens'] (use Slashed_Tab_Registry::is_token_tab and
Slashed_Token_Sanitizer::sanitize_section to validate and sanitize each section)
and then call Slashed_Token_Store::update_settings with only those sanitized
sections so any sections not present in the import are removed rather than
preserved.
In `@includes/class-token-page.php`:
- Around line 41-61: The register_menu() method (and the class-token-page usage)
assumes unified-plugin constants like SLASHED_VERSION, SLASHED_URL,
SLASHED_PATH, and SLASHED_CSS_REF are always defined; when SLASHED_VERSION is
undefined the standalone branch still later dereferences those constants and can
fatal or load wrong assets. Fix by adding defensive checks and fallbacks: change
any unconditional uses of SLASHED_URL, SLASHED_PATH, SLASHED_VERSION,
SLASHED_CSS_REF to use defined(...) ? constant : sensible default (or properties
injected by the bootstrap), and/or update the class constructor to accept and
store injected baseUrl/basePath/version/cssRef values so register_menu(),
render_page(), and asset-resolution code use the injected properties instead of
raw constants.
In `@includes/class-token-sanitizer.php`:
- Around line 45-51: The current array handling in the token sanitizer only
sanitizes one level via array_map and casts nested arrays to "Array", losing
nested token maps; replace that one-level array_map callback with a recursive
sanitizer method (e.g., sanitize_value) that: when $value is an array iterates
keys, uses sanitize_key() and skips empty keys, recursively calls
self::sanitize_value() for nested values, and for non-arrays returns
self::sanitize_css_value( sanitize_text_field( (string) $value ) ); then use
this sanitize_value function wherever the one-level array_map/static callback
was used (the existing array handling in the class-token-sanitizer.php) so
nested token maps like typography.font_sizes.* and spacing.space_sizes.* are
preserved.
In `@includes/class-token-store.php`:
- Around line 113-114: delete_settings() currently only deletes
self::OPTION_NAME which leaves the legacy option 'slashed_bricks_tokens' intact
and causes it to be re-migrated on next get_settings(); update delete_settings()
to also delete the legacy option by calling
delete_option('slashed_bricks_tokens') (or the stored legacy constant/name) so
both the current option (self::OPTION_NAME) and the legacy option are removed to
make “Reset all” durable across upgraded installs; reference delete_settings(),
get_settings(), and self::OPTION_NAME when making the change.
In `@integrations/bricks/includes/class-inventory.php`:
- Around line 192-193: get_admin_color_overrides() currently reads tokens
directly via get_option(Slashed_Token_Store::OPTION_NAME) which bypasses the
token-store API and prevents the legacy migration in
Slashed_Token_Store::get_settings() from running; update
get_admin_color_overrides() in class-inventory.php to call
Slashed_Token_Store::get_settings() (or the appropriate accessor on
Slashed_Token_Store) and use the returned settings/tokens instead of calling
get_option() directly so the legacy migration path
(Slashed_Token_Store::LEGACY_OPTION_NAME) is honored and admin color overrides
reflect migrated values.
---
Nitpick comments:
In `@integrations/bricks/includes/class-rebemer-rest.php`:
- Around line 71-72: Docblock text is stale: update the comment for
register_routes() so it no longer claims it runs "alongside the legacy admin
REST controller"; instead state that slashed_bricks_rest_routes_init() registers
only Bricks-specific endpoints and that the token CRUD controller is registered
globally by slashed.php. Edit the docblock on register_routes() in
class-rebemer-rest.php to replace the legacy admin controller reference with
this accurate description and keep function names exactly as mentioned
(register_routes(), slashed_bricks_rest_routes_init(), slashed.php, token CRUD
controller) for clarity.
In `@integrations/bricks/slashed-bricks.php`:
- Around line 138-152: The docblock still mentions a removed admin-init path;
remove or trim the paragraph referencing slashed_bricks_admin_init() so it no
longer claims the REST controller is "ALSO instantiated inside
`slashed_bricks_admin_init()`"; keep the explanation that routes are registered
via rest_api_init and note that the controller is now registered globally by
slashed.php (and that NAMESPACE is available for wp_localize_script) to avoid
misleading readers.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: eef7974a-a0d8-43a7-9d67-f31431603c14
📒 Files selected for processing (18)
includes/class-css-generator.phpincludes/class-rest-controller.phpincludes/class-tab-registry.phpincludes/class-token-defaults.phpincludes/class-token-page.phpincludes/class-token-sanitizer.phpincludes/class-token-store.phpintegrations/bricks/includes/class-admin-page-svelte.phpintegrations/bricks/includes/class-enqueue.phpintegrations/bricks/includes/class-fonts-rest.phpintegrations/bricks/includes/class-inventory.phpintegrations/bricks/includes/class-rebemer-enqueue.phpintegrations/bricks/includes/class-rebemer-rest.phpintegrations/bricks/includes/class-rest-controller.phpintegrations/bricks/includes/class-token-sanitizer.phpintegrations/bricks/includes/class-token-store.phpintegrations/bricks/slashed-bricks.phpslashed.php
💤 Files with no reviewable changes (5)
- integrations/bricks/includes/class-token-store.php
- integrations/bricks/includes/class-rest-controller.php
- integrations/bricks/includes/class-enqueue.php
- integrations/bricks/includes/class-admin-page-svelte.php
- integrations/bricks/includes/class-token-sanitizer.php
Comment on lines
+300
to
+316
| $all = Slashed_Token_Store::get_settings(); | ||
| $imported = 0; | ||
|
|
||
| foreach ( $body['tokens'] as $section => $values ) { | ||
| if ( ! Slashed_Tab_Registry::is_token_tab( $section ) || ! is_array( $values ) ) { | ||
| continue; | ||
| } | ||
| $sanitized = Slashed_Token_Sanitizer::sanitize_section( $section, $values ); | ||
| if ( ! empty( $sanitized ) ) { | ||
| $all[ $section ] = $sanitized; | ||
| ++$imported; | ||
| } else { | ||
| unset( $all[ $section ] ); | ||
| } | ||
| } | ||
|
|
||
| Slashed_Token_Store::update_settings( $all ); |
There was a problem hiding this comment.
Import should replace the saved token state, not merge into it.
Line 300 seeds the import with the site's existing overrides, so an export file that only contains colors will leave any old spacing, layouts, etc. in place on the target site. That means export → import does not faithfully reproduce the source configuration.
Suggested fix
- $all = Slashed_Token_Store::get_settings();
+ $all = array();🤖 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 `@includes/class-rest-controller.php` around lines 300 - 316, The import
currently seeds $all from Slashed_Token_Store::get_settings(), causing a merge
instead of replacing the token state; change the logic so you build a fresh
settings array from $body['tokens'] (use Slashed_Tab_Registry::is_token_tab and
Slashed_Token_Sanitizer::sanitize_section to validate and sanitize each section)
and then call Slashed_Token_Store::update_settings with only those sanitized
sections so any sections not present in the import are removed rather than
preserved.
Comment on lines
+45
to
+51
| if ( is_array( $value ) ) { | ||
| $sanitized[ $key ] = array_map( | ||
| static function ( $v ) { | ||
| return self::sanitize_css_value( sanitize_text_field( (string) $v ) ); | ||
| }, | ||
| $value | ||
| ); |
There was a problem hiding this comment.
Preserve nested token maps when sanitizing non-color sections.
This only sanitizes one array level. Shapes like typography.font_sizes.2xs = ['min' => ..., 'max' => ...] and spacing.space_sizes.* will hit the callback as arrays, get cast to "Array", and lose their bounds on save.
Suggested fix
- if ( is_array( $value ) ) {
- $sanitized[ $key ] = array_map(
- static function ( $v ) {
- return self::sanitize_css_value( sanitize_text_field( (string) $v ) );
- },
- $value
- );
- } else {
- $sanitized[ $key ] = self::sanitize_css_value( sanitize_text_field( (string) $value ) );
- }
+ $sanitized[ $key ] = self::sanitize_value( $value );private static function sanitize_value( $value ) {
if ( is_array( $value ) ) {
$sanitized = array();
foreach ( $value as $key => $nested_value ) {
$key = sanitize_key( $key );
if ( '' === $key ) {
continue;
}
$sanitized[ $key ] = self::sanitize_value( $nested_value );
}
return $sanitized;
}
return self::sanitize_css_value( sanitize_text_field( (string) $value ) );
}🤖 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 `@includes/class-token-sanitizer.php` around lines 45 - 51, The current array
handling in the token sanitizer only sanitizes one level via array_map and casts
nested arrays to "Array", losing nested token maps; replace that one-level
array_map callback with a recursive sanitizer method (e.g., sanitize_value)
that: when $value is an array iterates keys, uses sanitize_key() and skips empty
keys, recursively calls self::sanitize_value() for nested values, and for
non-arrays returns self::sanitize_css_value( sanitize_text_field( (string)
$value ) ); then use this sanitize_value function wherever the one-level
array_map/static callback was used (the existing array handling in the
class-token-sanitizer.php) so nested token maps like typography.font_sizes.* and
spacing.space_sizes.* are preserved.
- class-css-generator: use Slashed_Token_Store::get_settings() instead of direct get_option() in both has_overrides() and get_override_css() so the legacy slashed_bricks_tokens → slashed_tokens migration always runs on first access regardless of which read path fires first - class-token-store: delete_settings() now also deletes the legacy LEGACY_OPTION_NAME option so "Reset all" is durable on upgraded installs (without this, the migration would repopulate tokens on the next get_settings() call) - class-inventory: get_admin_color_overrides() routed through Slashed_Token_Store::get_settings() for the same migration reason - class-token-page: enqueue_assets() and get_class_hints() now use defined() checks with SLASHED_BRICKS_* fallbacks so the class is safe to instantiate when only the standalone Bricks plugin is active - slashed-bricks.php (standalone): add missing bootstrap block — registers Slashed_REST_Controller routes, instantiates Slashed_Token_Page (admin only), and hooks slashed_inject_token_overrides so token override CSS is injected in standalone mode the same way slashed.php does it - Stale docblocks in class-rebemer-rest.php and slashed-bricks.php updated to reflect the current architecture https://claude.ai/code/session_01DU3r3kT7GqH7w7jeadV2DT
This was referenced May 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
$(cat <<'EOF'
Summary
integrations/bricks/includes/toincludes/and renamed fromSlashed_Bricks_*toSlashed_*slashed.phpowns the global token pipeline: loads shared classes, registersslashed/v1REST routes, injects override CSS on bothwp_enqueue_scriptsandenqueue_block_editor_assets(priority 20), and bootstraps the Tokens admin page../../includes/; Bricks-specific REST endpoints updated toslashed/v1namespace; override CSS injection removed fromSlashed_Bricks_Enqueue(now handled globally)What moved
Slashed_Bricks_Token_StoreSlashed_Token_StoreSlashed_Bricks_Token_SanitizerSlashed_Token_SanitizerSlashed_Bricks_Token_DefaultsSlashed_Token_DefaultsSlashed_Bricks_Tab_RegistrySlashed_Tab_RegistrySlashed_Bricks_CSS_GeneratorSlashed_CSS_GeneratorSlashed_Bricks_REST_Controller(slashed-bricks/v1)Slashed_REST_Controller(slashed/v1)Slashed_Bricks_Admin_Page_SvelteSlashed_Token_PageMigration
Slashed_Token_Store::get_settings()performs a one-time silent migration: ifslashed_tokensdoesn't exist butslashed_bricks_tokenshas data, it copies to the new option and deletes the old one. Existing token configurations are preserved automatically.Test plan
slashed-bricks.phpdirectly still works — loads global classes from../../includes/slashed/v1(tokens, settings, rebemer/unused, bricks-fonts)slashed_bricks_tokensoption is migrated toslashed_tokenson first loadhttps://claude.ai/code/session_01DU3r3kT7GqH7w7jeadV2DT
EOF
)
Generated by Claude Code
Summary by CodeRabbit
Refactor
slashed/v1)Chores