Skip to content

fix(bricks): SF colour panel broken for border/box-shadow + missing admin inputs - #208

Merged
jackgranatowski merged 3 commits into
mainfrom
claude/bricks-border-sf-color-bug-f33Qj
Jun 3, 2026
Merged

fix(bricks): SF colour panel broken for border/box-shadow + missing admin inputs#208
jackgranatowski merged 3 commits into
mainfrom
claude/bricks-border-sf-color-bug-f33Qj

Conversation

@jackgranatowski

@jackgranatowski jackgranatowski commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

$(cat <<'EOF'

Summary

  • SF colour panel — border/box-shadow fix: clicking a swatch closed the Bricks settings panel and showed "input not found" for border and box-shadow colour controls
  • Admin panel — missing header/sticky height inputs: the four token inputs added in PR Add contextual color cascade and fluid layout dimension controls #203 were invisible because the admin-app bundle was never rebuilt

Root cause (border/box-shadow)

Svelte 5 delegates all onclick/onmousedown attributes to document, which fires after Bricks' document-level handlers (registered earlier at init time). For border and box-shadow controls, Bricks shows a colour popover with its own click-outside handler. Event sequence on swatch click:

  1. Click bubbles to document
  2. Bricks' handler fires first → detects click is outside the popover → closes it → colorInputEl.isConnected = false
  3. Svelte delegation firesonPickValue()isConnected = false → returns false → error toast

Previous attempts (stopPropagation / preventDefault via Svelte attributes) failed because they also fire at document level — too late to intercept Bricks.

Fix

Replace the Svelte-delegated onmousedown with a direct addEventListener attached in onMount. This fires at element level, before the event reaches document, so:

  • stopPropagation() actually prevents Bricks from ever seeing the event
  • preventDefault() actually prevents focus from leaving the Bricks colour input
  • onPickValue() is called on mousedown while colorInputEl is still connected

Every pickable swatch button gets a data-cp-pick-var attribute so the element-level handler can identify which swatch was pressed via e.target.closest('button[data-cp-pick-var]') without needing a swatch→object map.

Changes

  • ColorPanel.svelte — replace Svelte onmousedown with direct listener in onMount; add data-cp-pick-var to Quick Use, family banner, scale strip, alpha strip buttons
  • ColorSwatch.svelte — add data-cp-pick-var={swatch.var}
  • assets/editor-app/app.js — rebuilt
  • assets/admin-app/app.js / app.css — rebuilt to expose header/sticky height token inputs

Test plan

  • Open Bricks editor, select an element, open border settings → colour picker → click SF button → pick a swatch → colour applied, no error, no panel close
  • Same for box-shadow colour control
  • Text and background colour controls still work as before
  • Reference mode (SF Colors launcher badge) still copies to clipboard
  • Admin panel → Spacing/Layouts tab → Header & Sticky section shows Mobile/Desktop height and sticky offset inputs

https://claude.ai/code/session_016aL3vh5nHHa2QbmAxyFCGy
EOF
)


Generated by Claude Code

Summary by CodeRabbit

  • Style

    • Regenerated and optimized CSS styling for the Bricks editor admin interface, maintaining visual consistency across all UI components
  • Bug Fixes

    • Enhanced color picker event handling in the editor to improve the reliability and responsiveness of color selection

claude added 2 commits June 3, 2026 13:59
…de handler

Svelte 5 delegates onclick/onmousedown to document, which fires after Bricks'
own document-level handlers. For border and box-shadow colour controls, Bricks'
click-outside handler closes the sub-panel (disconnecting colorInputEl) before
our Svelte delegation runs — causing the "input not found" error.

Fix: attach a direct (non-delegated) mousedown listener on the ColorPanel
element. It fires at element level, before the event bubbles to document, so:
  - e.stopPropagation() actually prevents Bricks from seeing the event
  - e.preventDefault() actually prevents focus loss from the colour input
  - onPickValue() is called while colorInputEl is still connected

Add data-cp-pick-var={swatch.var} to every pickable swatch button (ColorSwatch,
Quick Use buttons, family banners, scale and alpha strip buttons) so the
element-level mousedown handler can identify the swatch without a swatch→object
map lookup.

https://claude.ai/code/session_016aL3vh5nHHa2QbmAxyFCGy
PR #203 added NumberField components for header_height_mobile,
header_height_desktop, sticky_offset_mobile and sticky_offset_desktop in
LayoutsTab.svelte, but the admin-app assets were never rebuilt. The admin panel
showed no input fields for these tokens until now.

https://claude.ai/code/session_016aL3vh5nHHa2QbmAxyFCGy
@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@jackgranatowski, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 59 minutes and 1 second. 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 542d9591-09be-4593-94af-ac617bce60a9

📥 Commits

Reviewing files that changed from the base of the PR and between 46eab83 and 18303c0.

📒 Files selected for processing (2)
  • plugins/SLASHED-for-WP/integrations/bricks/assets/editor-app/app.js
  • plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorPanel.svelte
📝 Walkthrough

Walkthrough

ColorPanel and ColorSwatch components refactored to support data-attribute-driven color token selection through a direct DOM mousedown listener instead of inline handlers. Quick Use and palette buttons annotated with data-cp-pick-var markers. Admin stylesheet minified with equivalent styling rules preserved.

Changes

Color picker refactor

Layer / File(s) Summary
Data attribute markers for color token selection
plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorSwatch.svelte, plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorPanel.svelte
ColorSwatch button and Quick Use buttons in ColorPanel annotated with data-cp-pick-var attributes to expose swatch variable identifiers for external listener logic.
Direct mousedown listener refactor
plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorPanel.svelte
ColorPanel adds panelEl reference binding and registers a direct onMount mousedown listener that reads data-cp-pick-var to construct and apply color token values in picker mode; replaces prior inline onmousedown handler.
CSS stylesheet minification
plugins/SLASHED-for-WP/integrations/bricks/assets/admin-app/app.css
Admin app stylesheet regenerated in minified form with equivalent styling rules for admin UI components, previews, and layout container.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • codeslash-dev/SLASHED#198: The redesigned two-mode color panel and picker/reference flow that this PR builds upon with data-attribute selection.
  • codeslash-dev/SLASHED#201: Modifies ColorPanel.svelte event handling for the SF colors panel interaction behavior.
  • codeslash-dev/SLASHED#206: Changes ColorPanel.svelte mousedown handling to prevent Bricks popovers from closing during color picker interactions.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: fixing a broken color panel for border/box-shadow and adding missing admin inputs, which are the primary objectives of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/bricks-border-sf-color-bug-f33Qj

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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
`@plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorPanel.svelte`:
- Around line 129-147: handlePanelMouseDown currently invokes onPickValue on
mousedown which causes a duplicate invocation when the subsequent click path in
ColorSwatch.svelte also runs; fix this by adding a transient suppression flag on
the element when you successfully handle the mousedown (e.g., set
btn.dataset.cpSuppressClick = '1' or a WeakMap entry) and then update the click
handler(s) in ColorSwatch.svelte (and any click handlers in ColorPanel.svelte)
to early-return if that suppression flag is present, clearing the flag after
reading it so keyboard-initiated clicks still work; reference
handlePanelMouseDown and onPickValue in ColorPanel.svelte and the click handler
in ColorSwatch.svelte when making these edits.
🪄 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: 311a938b-9121-452f-85e0-6069328b5118

📥 Commits

Reviewing files that changed from the base of the PR and between 2cbd586 and 46eab83.

📒 Files selected for processing (5)
  • plugins/SLASHED-for-WP/integrations/bricks/assets/admin-app/app.css
  • plugins/SLASHED-for-WP/integrations/bricks/assets/admin-app/app.js
  • plugins/SLASHED-for-WP/integrations/bricks/assets/editor-app/app.js
  • plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorPanel.svelte
  • plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorSwatch.svelte

The direct mousedown listener applies the colour value before Bricks'
click-outside handler fires. The Svelte-delegated onclick path on swatch
buttons stays live, so if the component is still mounted when the subsequent
click event fires (e.g. on a failed pick that shows an error toast), onPickValue
would be called a second time.

Add a data-cp-suppress-click flag to the button in handlePanelMouseDown before
calling onPickValue. The pick() click path checks for the flag and early-returns
if present, clearing it. Successful picks delete the flag before onPick() closes
the panel; failed picks leave it for the click to consume. Keyboard-activated
clicks never set the flag so they continue to work normally.

https://claude.ai/code/session_016aL3vh5nHHa2QbmAxyFCGy
@jackgranatowski
jackgranatowski merged commit a85bc73 into main Jun 3, 2026
9 checks passed
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