feat(bricks): inject all framework classes and variables into Bricks UI - #81
Merged
Conversation
The integration previously hand-curated Variables, Classes, and Colors via
hardcoded lists in PHP, leaving roughly half the framework's tokens
(animations, blur, body/headings, borders, focus rings, gradients, line,
opacity, optical, perspective, ratio, scroll, scrollbar, stroke, the alpha
color scale a5-a95, etc.) and several class selectors absent from the
Bricks UI. The lists also drifted out of sync with each release.
Replace the hardcoded enumeration with runtime parsing of the loaded CSS
bundle, so registrations always match the framework exactly:
- class-css-parser.php pure parser: declared --sf-* properties + .sf-/
.is- selectors from any CSS string
- class-inventory.php resolves the active bundle (local file > CDN URL
with transient cache > built-in JSON fallback),
categorizes variables by prefix family
- data/inventory.json fallback inventory (regenerated at release time
by scripts/gen-bricks-inventory.js, hooked into
npm run build)
Refactor class-variables/classes/colors to delegate to the inventory.
Public APIs and existing filters (slashed_bricks/registered_*,
slashed_bricks/color_categories) are preserved; new filters
slashed_bricks/inventory and slashed_bricks/inventory_local_path let
sites override resolution.
Validation against dist/slashed.optimal.css confirms 100% coverage:
603 variables (was 332), 123 .sf-* classes (was 141 with several stale
entries), 40 .is-* classes, and 275 color swatches in the global palette
(was ~76).
Co-authored-by: Jack Granatowski <contact@codeslash.net>
Contributor
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR refactors the Bricks integration to register classes, colors, and variables dynamically from a centralized CSS bundle inventory rather than hardcoded lists. A new ChangesBricks Integration Inventory-Driven Registration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🧹 Nitpick comments (1)
integrations/bricks/includes/class-colors.php (1)
28-40: ⚡ Quick winDerive color family buckets from the inventory instead of static prefix lists.
build_categories()still depends on the fixed$brands/$statusesarrays, so a new--sf-color-<family>-*palette will be registered but silently grouped underSLASHED Semanticuntil this file is updated. That reintroduces the drift this PR is trying to remove. Consider deriving first-segment families from the inventory itself, or at least making the recognized family lists filterable.Also applies to: 111-139, 176-188
🤖 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-colors.php` around lines 28 - 40, The current static lists private static $brands and private static $statuses cause build_categories() to misgroup newly-registered palettes; update class-colors.php so family buckets are derived from the actual inventory rather than hard-coded prefixes (or at minimum make these lists filterable). Specifically, modify the logic in build_categories() (and any helper that references $brands/$statuses) to iterate the inventory keys (first segment of the palette slug) to compute canonical families dynamically, falling back to the existing arrays only if no inventory-derived families exist; alternatively expose $brands and $statuses via apply_filters so consumers can add families. Ensure any references at the other noted regions (lines around 111-139 and 176-188) use the new dynamic/filerable family source rather than the static arrays.
🤖 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 `@integrations/bricks/includes/class-inventory.php`:
- Around line 329-349: The resolve() logic currently prefers a local bundle
(find_local_bundle_path() -> parse_path_with_cache()) even when the enqueued
bundle URL (slashed_bricks_get_css_url() or the slashed_bricks/css_bundle_url
override) points at a different remote bundle; update resolve() so it first
determines the final bundle URL (use slashed_bricks_get_css_url() and apply the
slashed_bricks/css_bundle_url override) and then choose the inventory source
that matches that URL — if the final URL is a remote URL call
parse_url_with_cache($url), otherwise if it points to a local path or the local
bundle name matches use parse_path_with_cache($local_path); ensure both
resolve() and the same logic area around lines 369-392 are aligned to avoid
reading variables from the wrong bundle.
- Around line 66-76: The filter runs before the cache is primed which allows
re-entrant calls to Slashed_Bricks_Inventory::get_*() to recurse; to fix, prime
the cache with the resolved inventory before calling apply_filters so recursive
get() calls read the primed value: set self::$cache (or an "in-progress"
sentinel) to $inventory returned by self::resolve() first, then call
apply_filters( 'slashed_bricks/inventory', self::$cache ) and assign its result
back to self::$cache; update the get()/resolve() logic accordingly to respect
the primed cache and avoid infinite recursion.
---
Nitpick comments:
In `@integrations/bricks/includes/class-colors.php`:
- Around line 28-40: The current static lists private static $brands and private
static $statuses cause build_categories() to misgroup newly-registered palettes;
update class-colors.php so family buckets are derived from the actual inventory
rather than hard-coded prefixes (or at minimum make these lists filterable).
Specifically, modify the logic in build_categories() (and any helper that
references $brands/$statuses) to iterate the inventory keys (first segment of
the palette slug) to compute canonical families dynamically, falling back to the
existing arrays only if no inventory-derived families exist; alternatively
expose $brands and $statuses via apply_filters so consumers can add families.
Ensure any references at the other noted regions (lines around 111-139 and
176-188) use the new dynamic/filerable family source rather than the static
arrays.
🪄 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: 2d96c044-6a29-47e0-b452-97ed21fe9996
📒 Files selected for processing (10)
integrations/bricks/README.mdintegrations/bricks/data/inventory.jsonintegrations/bricks/includes/class-classes.phpintegrations/bricks/includes/class-colors.phpintegrations/bricks/includes/class-css-parser.phpintegrations/bricks/includes/class-inventory.phpintegrations/bricks/includes/class-variables.phpintegrations/bricks/slashed-bricks.phppackage.jsonscripts/gen-bricks-inventory.js
Comment on lines
+66
to
+76
| $inventory = self::resolve(); | ||
|
|
||
| /** | ||
| * Filter the resolved inventory before it's used to register | ||
| * variables, classes, and colors with Bricks. | ||
| * | ||
| * @param array $inventory ['variables', 'sf_classes', 'is_classes']. | ||
| */ | ||
| self::$cache = apply_filters( 'slashed_bricks/inventory', $inventory ); | ||
|
|
||
| return self::$cache; |
There was a problem hiding this comment.
Prevent re-entrant recursion during inventory filtering.
At Line 74, apply_filters( 'slashed_bricks/inventory', ...) runs before self::$cache is primed. If a filter callback calls Slashed_Bricks_Inventory::get_*(), get() re-enters and can recurse indefinitely.
Suggested fix
public static function get() {
if ( null !== self::$cache ) {
return self::$cache;
}
- $inventory = self::resolve();
+ $inventory = self::resolve();
+ // Prime cache before filters to prevent re-entrant recursion.
+ self::$cache = self::sanitize_inventory( $inventory );
/**
* Filter the resolved inventory before it's used to register
* variables, classes, and colors with Bricks.
*
* `@param` array $inventory ['variables', 'sf_classes', 'is_classes'].
*/
- self::$cache = apply_filters( 'slashed_bricks/inventory', $inventory );
+ $filtered = apply_filters( 'slashed_bricks/inventory', self::$cache );
+ self::$cache = self::sanitize_inventory( $filtered );
return self::$cache;
}
+
+private static function sanitize_inventory( $inventory ) {
+ $base = Slashed_Bricks_CSS_Parser::empty_inventory();
+ if ( ! is_array( $inventory ) ) {
+ return $base;
+ }
+ foreach ( array( 'variables', 'sf_classes', 'is_classes' ) as $key ) {
+ $base[ $key ] = isset( $inventory[ $key ] ) && is_array( $inventory[ $key ] )
+ ? array_values( array_filter( $inventory[ $key ], 'is_string' ) )
+ : array();
+ $base[ $key ] = array_values( array_unique( $base[ $key ] ) );
+ sort( $base[ $key ] );
+ }
+ return $base;
+}🤖 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-inventory.php` around lines 66 - 76, The
filter runs before the cache is primed which allows re-entrant calls to
Slashed_Bricks_Inventory::get_*() to recurse; to fix, prime the cache with the
resolved inventory before calling apply_filters so recursive get() calls read
the primed value: set self::$cache (or an "in-progress" sentinel) to $inventory
returned by self::resolve() first, then call apply_filters(
'slashed_bricks/inventory', self::$cache ) and assign its result back to
self::$cache; update the get()/resolve() logic accordingly to respect the primed
cache and avoid infinite recursion.
Comment on lines
+329
to
+349
| private static function resolve() { | ||
| // 1. Local file - cheapest, also handles offline development. | ||
| $local_path = self::find_local_bundle_path(); | ||
| if ( '' !== $local_path ) { | ||
| $inventory = self::parse_path_with_cache( $local_path ); | ||
| if ( ! empty( $inventory['variables'] ) ) { | ||
| return $inventory; | ||
| } | ||
| } | ||
|
|
||
| // 2. Remote URL (CDN). Cached as a transient. | ||
| $url = function_exists( 'slashed_bricks_get_css_url' ) | ||
| ? slashed_bricks_get_css_url() | ||
| : ''; | ||
|
|
||
| if ( '' !== $url && self::is_remote_url( $url ) ) { | ||
| $inventory = self::parse_url_with_cache( $url ); | ||
| if ( ! empty( $inventory['variables'] ) ) { | ||
| return $inventory; | ||
| } | ||
| } |
There was a problem hiding this comment.
Keep inventory source aligned with the final CSS bundle URL.
Lines 331-337 always prefer local dist/slashed.optimal.css, but the actual enqueued bundle can be overridden via slashed_bricks/css_bundle_url (integrations/bricks/slashed-bricks.php Line 76). In environments where local dist/ exists, this can register tokens from optimal while the site loads essential, full, or a custom URL.
Direction for fix
private static function find_local_bundle_path() {
$override = apply_filters( 'slashed_bricks/inventory_local_path', null );
@@
if ( is_string( $override ) ) {
return ( '' !== $override && file_exists( $override ) ) ? $override : '';
}
+
+ // If the final CSS URL was overridden away from default local bundle URLs,
+ // skip implicit local candidates to avoid inventory/CSS drift.
+ $resolved_url = function_exists( 'slashed_bricks_get_css_url' )
+ ? slashed_bricks_get_css_url()
+ : '';
+ $default_local_urls = array(
+ SLASHED_BRICKS_URL . '../../dist/slashed.optimal.css',
+ SLASHED_BRICKS_URL . 'dist/slashed.optimal.css',
+ );
+ if ( '' !== $resolved_url && ! in_array( $resolved_url, $default_local_urls, true ) ) {
+ return '';
+ }Also applies to: 369-392
🤖 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-inventory.php` around lines 329 - 349, The
resolve() logic currently prefers a local bundle (find_local_bundle_path() ->
parse_path_with_cache()) even when the enqueued bundle URL
(slashed_bricks_get_css_url() or the slashed_bricks/css_bundle_url override)
points at a different remote bundle; update resolve() so it first determines the
final bundle URL (use slashed_bricks_get_css_url() and apply the
slashed_bricks/css_bundle_url override) and then choose the inventory source
that matches that URL — if the final URL is a remote URL call
parse_url_with_cache($url), otherwise if it points to a local path or the local
bundle name matches use parse_path_with_cache($local_path); ensure both
resolve() and the same logic area around lines 369-392 are aligned to avoid
reading variables from the wrong bundle.
Two bugs surfaced by CodeRabbit review on PR #81, both Major: 1. Re-entrant recursion in Slashed_Bricks_Inventory::get() The 'slashed_bricks/inventory' filter ran before self::$cache was primed. A filter callback calling Slashed_Bricks_Inventory::get_*() from inside itself (a legitimate use case for plugins extending the token list) would re-enter get(), call resolve() again, and recurse indefinitely. Fix: prime the cache with the resolved+sanitised inventory FIRST, then apply the filter, then re-sanitise and re-cache the filter's return. Recursive get() calls now short-circuit on the cache check in the first three lines. 2. Inventory drifted from the active CSS bundle URL find_local_bundle_path() always probed dist/slashed.optimal.css, ignoring whatever URL slashed_bricks/css_bundle_url resolved to. Sites that filtered the URL to load 'essential' or 'full' got tokens registered from 'optimal' - so Bricks UI showed tokens that didn't exist in the loaded CSS. Fix: derive the local path from slashed_bricks_get_css_url(). Map the URL back to a filesystem path when it lives under the plugin's URL space (covers both copy-install and symlink-in-repo modes); fall through to remote fetch otherwise. inventory_local_path filter still wins over both, and returning false still skips local resolution entirely. Also adds defensive sanitize_inventory() that normalises arbitrary filter outputs (null, partial arrays, non-string entries) into the canonical {variables, sf_classes, is_classes} shape so downstream registration code never sees malformed data. Validated end-to-end via mock-WP harness covering both fixes plus PR #81 coverage baseline: 22/22 checks pass. - Recursion test: filter callback calls get_variables() inside itself, returns 603 vars, callback runs exactly once, resolves in <2ms. - URL drift test: switching slashed_bricks/css_bundle_url between essential/optimal/full and CDN paths makes the inventory follow. - Sanitiser tests: dedup + sort + drop-non-strings + handle-null. - Coverage sanity: 603 vars, 123 sf, 40 is, 17 categories, all unchanged from PR #81 baseline.
jackgranatowski
pushed a commit
that referenced
this pull request
May 25, 2026
Two bugs surfaced by CodeRabbit review on PR #81, both Major: 1. Re-entrant recursion in Slashed_Bricks_Inventory::get() The 'slashed_bricks/inventory' filter ran before self::$cache was primed. A filter callback calling Slashed_Bricks_Inventory::get_*() from inside itself (a legitimate use case for plugins extending the token list) would re-enter get(), call resolve() again, and recurse indefinitely. Fix: prime the cache with the resolved+sanitised inventory FIRST, then apply the filter, then re-sanitise and re-cache the filter's return. Recursive get() calls now short-circuit on the cache check in the first three lines. 2. Inventory drifted from the active CSS bundle URL find_local_bundle_path() always probed dist/slashed.optimal.css, ignoring whatever URL slashed_bricks/css_bundle_url resolved to. Sites that filtered the URL to load 'essential' or 'full' got tokens registered from 'optimal' - so Bricks UI showed tokens that didn't exist in the loaded CSS. Fix: derive the local path from slashed_bricks_get_css_url(). Map the URL back to a filesystem path when it lives under the plugin's URL space (covers both copy-install and symlink-in-repo modes); fall through to remote fetch otherwise. inventory_local_path filter still wins over both, and returning false still skips local resolution entirely. Also adds defensive sanitize_inventory() that normalises arbitrary filter outputs (null, partial arrays, non-string entries) into the canonical {variables, sf_classes, is_classes} shape so downstream registration code never sees malformed data. Validated end-to-end via mock-WP harness covering both fixes plus PR #81 coverage baseline: 22/22 checks pass. - Recursion test: filter callback calls get_variables() inside itself, returns 603 vars, callback runs exactly once, resolves in <2ms. - URL drift test: switching slashed_bricks/css_bundle_url between essential/optimal/full and CDN paths makes the inventory follow. - Sanitiser tests: dedup + sort + drop-non-strings + handle-null. - Coverage sanity: 603 vars, 123 sf, 40 is, 17 categories, all unchanged from PR #81 baseline.
This was referenced May 25, 2026
fix(bricks): register variables/classes via canonical options and stop CSS bleed into builder UI
#84
Merged
jackgranatowski
pushed a commit
that referenced
this pull request
May 25, 2026
Brings main (post PR #86 chevron change, PR #81-85 bricks integration work, PR #87-90 Dependabot bumps) into the v0.4 branch so PR #91 can be merged cleanly. The 16 *.min.css and *.min.css.map files conflicted on git merge because they're single-line bundles where every change collides; resolved by rebuilding from the merged source via npm run build. The unminified .css and .flat.css bundles auto-merged cleanly because the changes are in non-overlapping line ranges (chevron region in optional/forms.css from main vs new tokens/macros/themes/layout from v0.4). Regenerated artefacts: - dist/* via scripts/bundle.js - docs/tokens.md via scripts/gen-token-reference.js (608 tokens) - integrations/bricks/data/inventory.json via scripts/gen-bricks-inventory.js Validation: npm run lint:css clean, 149/149 chromium playwright tests pass (token-API snapshot lock, bundle-size budget, link-contrast suite, macros, tokens resolution, layers ordering, behavior, demo coverage, demo visual).
This was referenced May 25, 2026
This was referenced Jun 1, 2026
Merged
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.
This pull request was created by @kiro-agent on behalf of @jackgranatowski 👻
Comment with /kiro fix to address specific feedback or /kiro all to address everything.
Learn about Kiro autonomous agent
Why
The Bricks integration was supposed to populate the Bricks Builder UI with every SLASHED token and class so users get a complete picker / autocomplete experience. In practice it was hand-curating those lists in PHP and missing roughly half of them:
--sf-*variables registered.sf-*classes registered.is-*classes registered--sf-color-*swatches in paletteEntire token families were absent — animations, blur, body/headings, borders, focus rings, gradients, line, opacity, optical, perspective, ratio, scroll, scrollbar, stroke, the whole alpha color scale (
-a5…-a95), etc. Every framework release silently broke parity with the Bricks UI.What changed
Replace the hardcoded enumeration with runtime parsing of the loaded CSS bundle, so registrations always match the framework exactly.
New files
integrations/bricks/includes/class-css-parser.php— pure parser. Strips comments, returns declared--sf-*properties and.sf-*/.is-*selectors.integrations/bricks/includes/class-inventory.php— resolves the active bundle (local file → CDN URL with transient cache → built-in JSON fallback) and categorizes variables by prefix family.integrations/bricks/data/inventory.json— fallback inventory (603 vars / 123.sf-/ 40.is-) generated fromdist/slashed.optimal.css. Keeps the plugin functional on hosts that block outbound HTTP.scripts/gen-bricks-inventory.js— regenerates the fallback at release time. Wired intonpm run build.Refactored
class-variables.php,class-classes.php,class-colors.php— all now delegate to the inventory. Removed several hundred lines of stale hardcoded lists.Preserved
slashed_bricks/registered_variables,slashed_bricks/registered_classes,slashed_bricks/registered_colors,slashed_bricks/color_categories).New filters
slashed_bricks/inventory— replace the resolved inventory wholesale.slashed_bricks/inventory_local_path— authoritative local-path override (stringuse that path ·falseskip local ·nulluse defaults).How it works at runtime
The inventory tries sources in order:
dist/slashed.optimal.cssnext to the plugin). Cached as a transient keyed bymtime, so edits invalidate automatically.slashed_bricks_get_css_url(). Fetched viawp_remote_get, cached for one day.data/inventory.json).The first source that succeeds wins. All three registration classes share one process-local cache so a page load incurs at most one parse.
Validation
A harness mocking the WP functions the integration uses was run against the real
dist/slashed.optimal.css. All 27 assertions pass:--sf-*(no misses)--sf-truncate-suffixlands in "Misc" (correct — it's a content fragment, not a typed token)locked: trueraw: var(--sf-color-…)references; brand categories appear in canonical order; labels render correctly (e.g. "SF Primary 50", "SF Primary A20")WP_Error) confirmsdata/inventory.jsonis loaded successfullyTested
php -lclean across all six PHP filesKnown limitations
slashed_bricks/css_bundle_urlfilter to point at a non-SLASHED stylesheet, the inventory will reflect that file (not the framework). This is the correct behavior — the Bricks UI should match what's actually applied — but worth flagging for users with unusual setups.slashed.optimal.css. Sites runningslashed.essential.csswill still see the optimal-superset inventory if both local and remote resolution fail. In practice this only affects offline-only installs that are also using a non-default bundle.Summary by CodeRabbit
New Features
Documentation