Skip to content

fix(bricks): use official bricks/builder/color_palette filter for color injection - #92

Merged
jackgranatowski merged 4 commits into
mainfrom
claude/bricks-auto-load-framework-assets-3STxk
May 25, 2026
Merged

fix(bricks): use official bricks/builder/color_palette filter for color injection#92
jackgranatowski merged 4 commits into
mainfrom
claude/bricks-auto-load-framework-assets-3STxk

Conversation

@jackgranatowski

@jackgranatowski jackgranatowski commented May 25, 2026

Copy link
Copy Markdown
Contributor

The previous option_bricks_color_palette filter only fires when WordPress
reads the DB option directly; Bricks fires its own bricks/builder/color_palette
filter when assembling the builder color picker, bypassing get_option.
Colors were never appearing in the builder because the wrong hook was used.

The option_* filters are kept as fallbacks for REST/import/older Bricks paths.

Summary by CodeRabbit

  • New Features
    • Improved Bricks builder integration: colors, variables and global classes are initialized earlier so palettes and tokens appear and stay synchronized more reliably in the builder.
  • Bug Fixes
    • Reduced timing-related issues where palettes, variables or classes could be missing or out of sync during initial theme/plugin load.
  • Documentation
    • Internal docs clarified lifecycle expectations to ensure consistent behavior across loads.

Review Change Stack

claude added 2 commits May 25, 2026 13:57
…or injection

The previous option_bricks_color_palette filter only fires when WordPress
reads the DB option directly; Bricks fires its own bricks/builder/color_palette
filter when assembling the builder color picker, bypassing get_option.
Colors were never appearing in the builder because the wrong hook was used.

The option_* filters are kept as fallbacks for REST/import/older Bricks paths.
…olor_palette

The bricks/builder/color_palette filter expects a flat array of {hex, rgb?}
objects (per Bricks Academy docs), not the {id, name, colors:[]} palette-group
shape used by the option. The previous fix wired the wrong callback to the
filter, so nothing appeared in the builder color picker.

Added inject_builder_colors() with the correct flat structure, using
var(--sf-color-*) references so swatches track the live theme via CSS
variable resolution in the editor iframe.
@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c13daf7d-2b3c-40d5-8d59-2318854376f2

📥 Commits

Reviewing files that changed from the base of the PR and between 8f71ffa and e54abba.

📒 Files selected for processing (4)
  • integrations/bricks/includes/class-classes.php
  • integrations/bricks/includes/class-colors.php
  • integrations/bricks/includes/class-variables.php
  • integrations/bricks/slashed-bricks.php
✅ Files skipped from review due to trivial changes (3)
  • integrations/bricks/includes/class-variables.php
  • integrations/bricks/includes/class-colors.php
  • integrations/bricks/includes/class-classes.php

📝 Walkthrough

Walkthrough

Adds a new plugins_loaded initializer (slashed_bricks_data_init) that conditionally instantiates Bricks data managers (variables, classes, colors) early; slashed_bricks_init() is simplified to perform late enqueue/CSS setup. Class docblocks for the Bricks integrations are updated to document Bricks option-read timing and the managed/virtual inject-on-read / strip-on-save lifecycle.

Changes

Bricks integration bootstrap and docs

Layer / File(s) Summary
Early data initializer (plugins_loaded)
integrations/bricks/slashed-bricks.php
Adds slashed_bricks_data_init() hooked on plugins_loaded (priority 20) to conditionally require CSS parser/inventory and instantiate Variables, Classes, and Colors managers; slashed_bricks_init() is reduced to late enqueue/CSS setup and Slashed_Bricks_Enqueue instantiation.
Integration docblock updates
integrations/bricks/includes/class-colors.php, integrations/bricks/includes/class-variables.php, integrations/bricks/includes/class-classes.php
Rewrites class-level documentation to clarify Bricks reads bricks_global_* options during Database::__construct() (theme load) and that integrations must be instantiated at plugins_loaded; describes SLASHED entries as managed/virtual (inject on read, strip on save) and tweaks Colors constructor comments about which Bricks filter is intentionally not used.

Sequence Diagram

sequenceDiagram
  participant WordPress
  participant Slashed_Bricks_Data_Init
  participant Slashed_Bricks_Variables
  participant Slashed_Bricks_Classes
  participant Slashed_Bricks_Colors
  participant Slashed_Bricks_Init
  participant Slashed_Bricks_Enqueue

  WordPress->>Slashed_Bricks_Data_Init: plugins_loaded (priority 20)
  Slashed_Bricks_Data_Init->>Slashed_Bricks_Variables: instantiate Variables manager
  Slashed_Bricks_Data_Init->>Slashed_Bricks_Classes: instantiate Classes manager
  Slashed_Bricks_Data_Init->>Slashed_Bricks_Colors: instantiate Colors manager
  WordPress->>Slashed_Bricks_Init: after_setup_theme
  Slashed_Bricks_Init->>Slashed_Bricks_Enqueue: instantiate Enqueue (late)
Loading

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • codeslash-dev/SLASHED#81: Modifies integrations/bricks/slashed-bricks.php bootstrap and inventory-driven injection wiring that intersects with this PR's lifecycle changes.
  • codeslash-dev/SLASHED#77: Also changes WordPress lifecycle wiring in slashed-bricks.php, overlapping with this PR's relocation of data-manager initialization.
  • codeslash-dev/SLASHED#83: Adjusts Bricks integration bootstrap and inventory/palette wiring related to when data managers are loaded vs deferred.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title claims to use the official bricks/builder/color_palette filter for color injection, but the actual changes show this filter was removed and replaced with earlier initialization of data managers at plugins_loaded. The title does not reflect the main structural change. Update the title to accurately describe the main change: something like 'fix(bricks): initialize data managers at plugins_loaded for proper option filter timing' to reflect the actual implementation.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/bricks-auto-load-framework-assets-3STxk

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

claude added 2 commits May 25, 2026 14:45
…p_theme

Root cause: Bricks' Database::__construct() reads bricks_global_variables,
bricks_global_classes, and bricks_color_palette via get_option() during theme
functions.php load — which happens AFTER plugins_loaded but BEFORE
after_setup_theme. Our option filters were registered in after_setup_theme,
so Bricks had already read and internally cached the options before our filters
were ever in place. Nothing appeared in any of the three managers.

Fix: split initialization into two phases.
- plugins_loaded (priority 20): instantiate Variables, Classes, Colors so their
  option filters are registered before the theme loads and Bricks reads options.
- after_setup_theme: instantiate Enqueue only (needs theme + Bricks version check).

Also removes the bricks/builder/color_palette flat-swatch hook added in the
previous two commits. That filter cannot assign names to injected colors per
Bricks' own docs ("id and name are generated after it is applied"), making it
unsuitable for Color Manager integration. Named palette groups via the option
filter are the correct mechanism and now fire at the right time.
get_option('template') returns false when the option is absent; casting to
string and using strtolower() makes the Bricks check robust against missing
options and any case variation in the theme directory name.
@jackgranatowski
jackgranatowski merged commit fd3d1e3 into main May 25, 2026
5 checks passed
@jackgranatowski
jackgranatowski deleted the claude/bricks-auto-load-framework-assets-3STxk branch May 31, 2026 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants