feat(bricks): separate color palette + HEX picker + Contrast tab in admin - #83
Merged
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>
…dmin
Three user-facing GUI improvements based on direct feedback:
1. SLASHED colors as a separate, named Bricks palette
Switch class-colors.php from globalColors injection (which mixed our
tokens into the site's flat global color list) to the Bricks color
palette API via the option_bricks_color_palette filter. Eight
palettes appear under the Bricks color picker palette dropdown:
SLASHED Primary/Secondary/Tertiary/Action/Neutral/Base, plus
SLASHED Status and SLASHED Semantic. Each swatch is a var(--sf-*)
reference (both 'hex' and 'raw' fields) so it tracks the live
theme, including dark mode and admin token overrides.
Read-injection + write-strip cycle keeps the database clean: we
inject our palettes whenever Bricks reads bricks_color_palette,
and strip them on pre_update_option_bricks_color_palette so user-
created palettes still persist but ours never get baked in.
2. Real HEX color picker in the SLASHED admin Colors tab
Each color row now has two paired inputs - a HEX picker wired to
wpColorPicker, and a collapsible Advanced field that accepts any
CSS color string (oklch, rgb, hsl, var(), ...). The active mode
is auto-detected on load from the saved value's shape, so existing
oklch overrides keep their format and aren't truncated by the
HEX-only picker. On save, raw wins when filled; otherwise the HEX
value is stored. The framework consumes whatever ends up in the
option (CSS Color 4 oklch-from-* operations work on any input
color space).
3. New Contrast admin tab for cross-cutting tuning knobs
Adds sliders/inputs for tokens that don't fit any one section:
- --sf-contrast-bias (-0.2 .. +0.2)
- --sf-contrast-threshold (0 .. 1)
- --sf-opacity-disabled (0 .. 1)
- --sf-focus-ring-width (px)
- --sf-focus-ring-offset (px)
- --sf-focus-ring-style (enum: solid/dashed/dotted/double/none)
focus_ring_style is rendered as a select and validated against an
enum in the CSS generator so arbitrary input never reaches output.
Per-section scale multipliers (text/space/radius/motion) keep their
own tabs - one home per setting, no duplicate sources of truth.
Validated end-to-end via mock-WP harness: 41/41 checks passing across
palette injection idempotency + write-strip cycle, sanitizer matrix
(HEX/raw/both/neither + multi-row + CSS-injection guard + shape
recognition for 3/4/6/8-digit hex), CSS generator output (units,
@layer wrap, enum guard, locale-safe float formatting).
Stacks on top of PR #81 (depends on Slashed_Bricks_Inventory).
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 introduces a dynamic CSS parsing and inventory system for the SLASHED Bricks integration, replacing hardcoded variable/class lists with runtime discovery from the active CSS bundle (local, CDN, or fallback JSON), and redesigns the admin UI to support paired color input modes and new contrast settings. ChangesDynamic Inventory-Driven Bricks Integration
Sequence DiagramsequenceDiagram
participant Admin as Admin Settings Page
participant Inventory as Slashed_Bricks_Inventory
participant Variables as Slashed_Bricks_Variables
participant Classes as Slashed_Bricks_Classes
participant Colors as Slashed_Bricks_Colors
participant Parser as Slashed_Bricks_CSS_Parser
participant Cache as Transient Cache
Admin->>Inventory: get() on page load
Inventory->>Cache: Check per-request cache
alt Cache miss
Inventory->>Inventory: find_local_bundle_path()
alt Local bundle found
Inventory->>Parser: parse(bundle_css)
Parser-->>Inventory: {variables, sf_classes, is_classes}
else Fall back to remote/fallback
Inventory->>Inventory: parse_url_with_cache() or fallback_inventory()
end
Inventory->>Cache: Store in per-request cache
end
Variables->>Inventory: get_variables_by_category()
Inventory-->>Variables: {Category: [var1, var2, ...]}
Classes->>Inventory: get_sf_classes() + get_is_classes()
Inventory-->>Classes: [sf-layout, is-state, ...]
Colors->>Inventory: get_color_variables()
Inventory-->>Colors: [--sf-color-primary, ...]
Colors->>Colors: build_palettes()
Colors->>Colors: inject_palettes() via option filter
Admin-->>Admin: Render Variables, Classes, Color palettes from inventory
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 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
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-variables.php (1)
47-55:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate
registered_variablesfilter output before iteration.
get_variables()returns filtered data unvalidated, but callers assumearray<string, string[]>. A malformed callback can break both i18n and code-signature registration paths. Normalize to an empty-safe shape first.Suggested fix
public function get_variables() { - $variables = Slashed_Bricks_Inventory::get_variables_by_category(); + $variables = Slashed_Bricks_Inventory::get_variables_by_category(); /** * Filter the registered CSS variables. * * `@param` array $variables Associative array of category => variable names. */ - return apply_filters( 'slashed_bricks/registered_variables', $variables ); + $variables = apply_filters( 'slashed_bricks/registered_variables', $variables ); + if ( ! is_array( $variables ) ) { + return array(); + } + + $normalized = array(); + foreach ( $variables as $category => $vars ) { + if ( ! is_string( $category ) || ! is_array( $vars ) ) { + continue; + } + $normalized[ $category ] = array_values( + array_filter( + $vars, + static function ( $v ) { + return is_string( $v ) && '' !== $v; + } + ) + ); + } + + return $normalized; }Also applies to: 68-69, 87-88
🤖 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-variables.php` around lines 47 - 55, Normalize and validate the output of the 'slashed_bricks/registered_variables' filter before returning/iterating: in the method that returns apply_filters('slashed_bricks/registered_variables', $variables) (and the other filter usages around the other occurrences mentioned), ensure the filtered $variables is an array, iterate keys and cast each value to an array (e.g., $value = (array) $value), filter each item to strings (or discard non-strings) and if the final structure is invalid produce an empty associative array (array<string, string[]>). Replace direct return/use of the filtered value with this normalized, empty-safe shape so callers (i18n and code-signature registration) always receive array<string, string[]>.
🤖 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-colors.php`:
- Around line 107-110: The strip_palettes() method currently returns the
original value when $palettes is not an array, which violates its expected array
return and can corrupt the bricks_color_palette shape; update strip_palettes()
(the method name) to normalize non-array inputs by returning an empty array ([])
instead of the original mixed value, then continue processing and returning an
array so any downstream use of the bricks_color_palette filter always receives
an array.
In `@integrations/bricks/includes/class-inventory.php`:
- Around line 74-76: Ensure the inventory payload is normalized to a strict
variables->sf_classes->is_classes => string[] shape before caching/returning:
validate that $inventory is an array, ensure $inventory['variables'] and
$inventory['variables']['sf_classes'] are arrays, set
$inventory['variables']['sf_classes']['is_classes'] to an array (fallback to
[]), and map every element to string (e.g. via array_map('strval',
array_values(...))). Do this just before calling self::$cache =
apply_filters('slashed_bricks/inventory', $inventory); and before the other
return site referenced around lines 492-499 so downstream offset
access/array_values() always sees a string[].
---
Outside diff comments:
In `@integrations/bricks/includes/class-variables.php`:
- Around line 47-55: Normalize and validate the output of the
'slashed_bricks/registered_variables' filter before returning/iterating: in the
method that returns apply_filters('slashed_bricks/registered_variables',
$variables) (and the other filter usages around the other occurrences
mentioned), ensure the filtered $variables is an array, iterate keys and cast
each value to an array (e.g., $value = (array) $value), filter each item to
strings (or discard non-strings) and if the final structure is invalid produce
an empty associative array (array<string, string[]>). Replace direct return/use
of the filtered value with this normalized, empty-safe shape so callers (i18n
and code-signature registration) always receive array<string, string[]>.
🪄 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: dedd645d-50c9-47f0-a0ec-7fbf35c63053
📒 Files selected for processing (15)
integrations/bricks/README.mdintegrations/bricks/assets/admin-page.cssintegrations/bricks/assets/admin-page.jsintegrations/bricks/data/inventory.jsonintegrations/bricks/includes/class-admin-page.phpintegrations/bricks/includes/class-classes.phpintegrations/bricks/includes/class-colors.phpintegrations/bricks/includes/class-css-generator.phpintegrations/bricks/includes/class-css-parser.phpintegrations/bricks/includes/class-inventory.phpintegrations/bricks/includes/class-token-defaults.phpintegrations/bricks/includes/class-variables.phpintegrations/bricks/slashed-bricks.phppackage.jsonscripts/gen-bricks-inventory.js
Comment on lines
+107
to
110
| public function strip_palettes( $palettes ) { | ||
| if ( ! is_array( $palettes ) ) { | ||
| return $palettes; | ||
| } |
There was a problem hiding this comment.
Normalize strip_palettes() return type to array.
Line 109 currently returns mixed for non-array input, which breaks the method’s declared contract and can leave bricks_color_palette in an invalid shape. Normalize to an empty array here.
Suggested patch
public function strip_palettes( $palettes ) {
if ( ! is_array( $palettes ) ) {
- return $palettes;
+ return array();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public function strip_palettes( $palettes ) { | |
| if ( ! is_array( $palettes ) ) { | |
| return $palettes; | |
| } | |
| public function strip_palettes( $palettes ) { | |
| if ( ! is_array( $palettes ) ) { | |
| return array(); | |
| } |
🧰 Tools
🪛 PHPStan (2.1.54)
[error] 109-109: Method Slashed_Bricks_Colors::strip_palettes() should return array but returns mixed.
Type array<mixed, mixed> has already been eliminated from mixed.
(return.type)
🤖 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 107 - 110, The
strip_palettes() method currently returns the original value when $palettes is
not an array, which violates its expected array return and can corrupt the
bricks_color_palette shape; update strip_palettes() (the method name) to
normalize non-array inputs by returning an empty array ([]) instead of the
original mixed value, then continue processing and returning an array so any
downstream use of the bricks_color_palette filter always receives an array.
Comment on lines
+74
to
+76
| self::$cache = apply_filters( 'slashed_bricks/inventory', $inventory ); | ||
|
|
||
| return self::$cache; |
There was a problem hiding this comment.
Normalize inventory shape before caching/returning.
The filtered and decoded payloads are trusted as well-formed arrays. If either contains an unexpected type, downstream offset access/array_values() can raise runtime errors. Normalize once to a strict variables/sf_classes/is_classes => string[] shape with empty-array fallbacks.
Suggested fix
- self::$cache = apply_filters( 'slashed_bricks/inventory', $inventory );
+ $filtered = apply_filters( 'slashed_bricks/inventory', $inventory );
+ self::$cache = self::normalize_inventory( $filtered );
@@
- if ( is_array( $decoded )
- && isset( $decoded['variables'], $decoded['sf_classes'], $decoded['is_classes'] )
- ) {
- return array(
- 'variables' => array_values( $decoded['variables'] ),
- 'sf_classes' => array_values( $decoded['sf_classes'] ),
- 'is_classes' => array_values( $decoded['is_classes'] ),
- );
- }
+ if ( is_array( $decoded ) ) {
+ return self::normalize_inventory( $decoded );
+ }
@@
+ /**
+ * Normalize arbitrary inventory-like input into the expected shape.
+ *
+ * `@param` mixed $inventory Candidate inventory.
+ * `@return` array{variables: string[], sf_classes: string[], is_classes: string[]}
+ */
+ private static function normalize_inventory( $inventory ) {
+ if ( ! is_array( $inventory ) ) {
+ return Slashed_Bricks_CSS_Parser::empty_inventory();
+ }
+
+ $normalize_list = static function( $value ) {
+ if ( ! is_array( $value ) ) {
+ return array();
+ }
+ return array_values(
+ array_filter(
+ $value,
+ static function ( $item ) {
+ return is_string( $item ) && '' !== $item;
+ }
+ )
+ );
+ };
+
+ return array(
+ 'variables' => $normalize_list( isset( $inventory['variables'] ) ? $inventory['variables'] : array() ),
+ 'sf_classes' => $normalize_list( isset( $inventory['sf_classes'] ) ? $inventory['sf_classes'] : array() ),
+ 'is_classes' => $normalize_list( isset( $inventory['is_classes'] ) ? $inventory['is_classes'] : array() ),
+ );
+ }Also applies to: 492-499
🤖 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 74 - 76,
Ensure the inventory payload is normalized to a strict
variables->sf_classes->is_classes => string[] shape before caching/returning:
validate that $inventory is an array, ensure $inventory['variables'] and
$inventory['variables']['sf_classes'] are arrays, set
$inventory['variables']['sf_classes']['is_classes'] to an array (fallback to
[]), and map every element to string (e.g. via array_map('strval',
array_values(...))). Do this just before calling self::$cache =
apply_filters('slashed_bricks/inventory', $inventory); and before the other
return site referenced around lines 492-499 so downstream offset
access/array_values() always sees a string[].
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.
CodeRabbit flagged strip_palettes() returning the original mixed value when its input wasn't an array - violating the documented @return array contract and risking a corrupted bricks_color_palette option if a malformed value (scalar, null, object) ever reaches the pre_update_option_bricks_color_palette filter. Fix: collapse any non-array input to array(). Bricks' own loader iterates over the option, so the option must always be an array even in degenerate cases (fresh install, bad migration, third-party corruption). Validated 5 input types (null/false/int/string/object) plus the full pre_update write pipeline via mock-WP harness; all 20 checks pass.
This was referenced May 25, 2026
fix(bricks): register variables/classes via canonical options and stop CSS bleed into builder UI
#84
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
What
Three GUI/integration improvements to the SLASHED ↔ Bricks integration, addressing user feedback (PL):
Changes by area
1. Separate color palette in Bricks (
integrations/bricks/includes/class-colors.php)Switched from
globalColorsinjection (flat list, mixed with the user's other site colors) to the proper Bricks color palette API via:option_bricks_color_palette/default_option_bricks_color_palette— inject SLASHED palettes whenever Bricks reads palettes from the option.pre_update_option_bricks_color_palette— strip SLASHED palettes before persisting, so the database never bakes them in. User-created palettes still save normally.The result: the Bricks color picker shows a palette dropdown with eight SLASHED entries:
Each swatch references the framework variable directly (
var(--sf-color-X)in bothhexandrawfields), so swatches track live theming, dark mode, and admin token overrides.The integration becomes the single source of truth: bumping the framework or changing the active bundle automatically updates what Bricks shows, without leaving stale palette rows on the site.
2. Real HEX color picker in admin Colors tab (
includes/class-admin-page.php,assets/admin-page.{js,css})Each color row is now two paired inputs:
wpColorPickerbound to a HEX<input>. Curated palette of approximate sRGB equivalents of SLASHED's default oklch tokens shown as quick-pick chips.Active mode is auto-detected on render: if the saved value is a HEX literal we start in HEX mode; if it's anything else (oklch, named, etc.) we start in Advanced mode — so existing oklch overrides keep their format and aren't truncated by the HEX-only picker.
On save,
sanitize_color_section()merges the paired inputs into a single stored value at the base key (brand_primary,status_success, ...). Raw wins when filled; otherwise the HEX value is stored. The CSS generator emits whatever is stored — the framework consumes any CSS color (HEX, oklch, rgb, ...) thanks to CSS Color 4oklch(from ...)operations.3. New Contrast admin tab (
includes/class-admin-page.php+ token-defaults + css-generator)Houses cross-cutting visual fine-tuning knobs that don't fit any one token family:
--sf-contrast-bias--sf-contrast-threshold--sf-opacity-disabled--sf-focus-ring-width--sf-focus-ring-offset--sf-focus-ring-stylePer-section scale multipliers (text/space/radius/motion) intentionally stay in their own tabs — one home per setting, no duplicate sources of truth.
A new
render_range_field()helper emits the paired range+number markup that the existinginitRangeSync()JS already mirrors, and the CSS generator'sgenerate_contrast_declarations()emits the new properties with proper units, validatesfocus_ring_styleagainst a strict enum, and uses a locale-safe float formatter so a Polish/GermanLC_NUMERICcan't smuggle,into the output.Validation
End-to-end harness with a minimal WP mock — 41/41 checks passing:
update_option)@layer slashed.overrideswrap, enum guard blocks evil input, all five whitelisted ring styles passformat_floatlocale-safety (explicit'.'decimal separator)PHP
-lclean across all touched files; JSnode --checkclean.Backwards compat
Slashed_Bricks_Colors::get_colors()retained for tests/filters; existingslashed_bricks/registered_colorsandslashed_bricks/color_categoriesfilters still work.slashed_bricks/registered_paletteslets sites alter the injected palette set.slashed_tokensoption shape preserved — color values still stored at base keys (brand_primary,status_success, ...) regardless of which input was used.Summary by CodeRabbit