fix: Bricks swatch coverage + lightweight CSS normalize - #191
Merged
Conversation
SLASHED's reset (@layer slashed.reset) zeroes UA defaults that the WP admin bar relies on but doesn't explicitly redeclare in its own CSS. Author layers beat the UA in the cascade, so the admin bar icons (SVG display:block), spacing (margin/padding), box model, and button font-family were all being affected by the reset. Adds unlayered inline CSS (unlayered author > layered author in the cascade) using `revert` on the impacted properties. `revert` defers each property to the user/UA stylesheet value, making the SLASHED reset a no-op inside #wpadminbar without touching the reset itself. https://claude.ai/code/session_01JLvpeyEzNZtgc7TVjVMhzu
…ormalize The old reset zeroed margin/padding on *, stripped list markers, forced display:block on all media, and changed form-element font inheritance globally. This created cascade conflicts with any third-party UI (WP admin bar, builder chrome, plugins) that relied on UA defaults for properties it did not explicitly redeclare -- requiring element-specific patches rather than good architecture. New approach: core/reset.css — true normalize only Retains: box-sizing:border-box on *, html baselines (text-size-adjust, color-scheme, scroll-padding-top, scrollbar-gutter, hanging-punctuation, interpolate-size), body min-height/min-inline-size, and genuine bug fixes (button text-transform:none, search display:block, table border-collapse, [hidden], dialog/popover, fieldset min-width:0, [inert] cursor). Removes: * margin/padding zero, ul/ol list-style:none, img/svg display:block, iframe border:none, button/input font:inherit, touch-action, fieldset border/padding, legend padding. core/base.css — absorbs the opinionated rules it needs body, h1-h6, p now carry explicit margin:0 (UA 8px body margin and heading/paragraph margin-block are removed where we own those elements). Form-element normalization (font:inherit, cursor:pointer, touch-action, textarea resize:vertical) moves here. Media display:block and iframe border:none move here with a comment explaining why they are base-layer rather than reset-layer concerns. plugins/SLASHED-for-WP/class-core-enqueue.php Reverts the #wpadminbar element-specific patch -- no longer needed because the reset no longer touches the properties that caused the conflict. The right fix was the architecture, not the bandage. https://claude.ai/code/session_01JLvpeyEzNZtgc7TVjVMhzu
The rule was redundant — .sf-icon already sets fill: currentcolor explicitly (core/layout.css:271). Globally it caused silent breakage for stroke-based icon sets, multi-colour logos, and third-party SVGs (WP admin bar Dashicons, builder chrome icons) that did not declare their own fill. Removing it keeps the same principle as the reset refactor: don't assert global opinions; let components own their needs. https://claude.ai/code/session_01JLvpeyEzNZtgc7TVjVMhzu
|
Warning Review limit reached
More reviews will be available in 52 minutes and 48 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 (4)
📝 WalkthroughWalkthroughLighten reset.css into a lightweight normalize; add explicit typography margins and form-control inheritance in base.css; make media elements block-level responsive; update prose blockquote/table sizing in macros; change header token from 8rem to 0px affecting scroll/sticky offsets. ChangesCSS Normalization Restructuring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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: 1
🧹 Nitpick comments (1)
core/base.css (1)
154-159: ⚡ Quick winUse logical properties for consistency.
Lines 148-152 use logical properties (
max-inline-size,block-size) for img/picture/video/canvas, but these iframe/embed/object rules use physical properties (max-width,height). The rest of the file consistently uses logical properties (e.g.,border-block-startat line 163,border-block-endat line 248).♻️ Align with logical property usage
iframe, embed, object { display: block; - max-width: 100%; - height: auto; + max-inline-size: 100%; + block-size: auto; border: none; }🤖 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 `@core/base.css` around lines 154 - 159, The iframe/embed/object CSS rule uses physical properties (max-width, height) while the rest of the file uses logical properties; update the iframe, embed, object selector to use max-inline-size instead of max-width and block-size instead of height (leave display: block and border: none as-is) so the rule aligns with the logical-property convention used elsewhere.
🤖 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 `@core/reset.css`:
- Around line 77-83: The rule targeting dialog and [popover] currently strips UA
visuals by setting background:none and border:none, which is an opinionated
reset rather than a lightweight normalize; either update the comment to state
this is an intentional, opinionated component reset (so keep dialog, [popover]
with padding:0, border:none, background:none and change the comment to mention
"opinionated reset" or "component reset"), or change the CSS to preserve visible
defaults for normalization (remove background:none and border:none or set them
to initial/transparent-preserving values) so dialog and [popover] remain
visible; locate the selector block for dialog and [popover] to apply the chosen
fix and adjust the comment accordingly.
---
Nitpick comments:
In `@core/base.css`:
- Around line 154-159: The iframe/embed/object CSS rule uses physical properties
(max-width, height) while the rest of the file uses logical properties; update
the iframe, embed, object selector to use max-inline-size instead of max-width
and block-size instead of height (leave display: block and border: none as-is)
so the rule aligns with the logical-property convention used elsewhere.
🪄 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: 0a4a4fcd-8083-4590-88ba-251bae43eaf0
📒 Files selected for processing (2)
core/base.csscore/reset.css
address/cite/dfn/var { font-style: normal }
Removes semantic italic from HTML elements whose UA italic is
intentional — <cite> marks titles of works, <dfn> marks
definitions, <var> marks code variables. Stripping it globally
removes a visual semantic cue sighted users rely on (WCAG 1.3.3).
blockquote { font-style: italic }
Global italic is a readability hazard for users with dyslexia and
applied unconditionally to third-party plugin content and builder
widgets. The rule moves to .sf-prose blockquote in macros.css where
it is intentional. The border-start treatment remains global.
body { font-synthesis: none }
When a font is loaded as Regular-only (common with custom web fonts)
this silently renders <strong> and <em> identically to normal text,
failing WCAG 1.4.1 / 1.3.1 for users who rely on bold/italic to
distinguish emphasis. Removed; browsers will synthesise bold/italic
when variants are absent, which is the safer universal default.
https://claude.ai/code/session_01JLvpeyEzNZtgc7TVjVMhzu
- reset.css: dialog/[popover] background: none → Canvas so bare dialogs adapt to color-scheme and forced-colors instead of being transparent - reset.css + base.css: scroll-padding-top / scroll-margin-top fallbacks changed from 8rem to 0px — sites without a sticky header no longer get a 128px anchor offset out of the box - tokens.css: --sf-header-height default 8rem → 0px for the same reason - base.css: removed width:100% from global table rule — full-width is now scoped to .sf-prose table (macros.css) where it belongs https://claude.ai/code/session_01JLvpeyEzNZtgc7TVjVMhzu
There was a problem hiding this comment.
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 `@core/tokens.css`:
- Line 1151: Update the public docs and release notes to reflect the new default
of --sf-header-height: 0px by changing the documented default in docs/tokens.md
to 0px (and update related mentions of --sf-sticky-offset and
scroll-margin-top/scroll-padding-top fallbacks), add a clear breaking-change
entry in CHANGELOG.md and a migration note in docs/migration.md explaining that
sites with fixed/sticky headers must explicitly set --sf-header-height to their
header height (and provide a brief example and guidance), and flag this as a
potential major-version impact so maintainers can reassess bumping the version.
🪄 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: ba57178a-ec57-43c8-81cd-40f0726ba997
📒 Files selected for processing (4)
core/base.csscore/macros.csscore/reset.csscore/tokens.css
🚧 Files skipped from review as they are similar to previous changes (1)
- core/reset.css
| Layout & a11y | ||
| ---------------------------------------------------------- */ | ||
| --sf-header-height: 8rem; | ||
| --sf-header-height: 0px; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for CHANGELOG, migration docs, or breaking change notices
echo "==> Searching for CHANGELOG or migration documentation"
fd -t f -i 'changelog|migrat|upgrade|breaking' --max-depth 3
echo ""
echo "==> Checking for header-height mentions in docs"
rg -i 'header-height' -g '*.md' -C2Repository: codeslash-dev/SLASHED
Length of output: 886
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==> Locate occurrences of --sf-header-height in repo"
rg -n --hidden --no-ignore-vcs --stats --fixed-strings "--sf-header-height" .
echo "==> Check what docs/tokens.md says around --sf-header-height"
rg -n --fixed-strings "--sf-header-height" docs/tokens.md -C3
echo "==> Look for breaking-change/migration references to sf-header-height in CHANGELOG and docs/migration"
for f in CHANGELOG.md docs/migration.md; do
echo ""
echo "---- $f (sf-header-height/header-height keywords) ----"
rg -n -i -C2 "sf-header-height|header-height" "$f" || true
done
echo "==> Search CHANGELOG/migration for explicit token default value mentions"
for f in CHANGELOG.md docs/migration.md; do
echo ""
echo "---- $f (8rem vs 0px mentions) ----"
rg -n -i -C1 "8rem|0px" "$f" || true
doneRepository: codeslash-dev/SLASHED
Length of output: 557
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==> Occurrences of --sf-header-height in repo"
rg -n --hidden --no-ignore-vcs --fixed-strings -- --sf-header-height .
echo ""
echo "==> docs/tokens.md around --sf-header-height"
rg -n --fixed-strings -- --sf-header-height docs/tokens.md -C3
echo ""
echo "==> CHANGELOG.md and docs/migration.md for header-height / sf-header-height"
for f in CHANGELOG.md docs/migration.md; do
echo ""
echo "---- $f ----"
rg -n -i -C2 -- "sf-header-height|header-height" "$f" || true
done
echo ""
echo "==> CHANGELOG.md and docs/migration.md for 8rem vs 0px mentions"
for f in CHANGELOG.md docs/migration.md; do
echo ""
echo "---- $f ----"
rg -n -i -C1 -- "8rem|0px" "$f" || true
done
echo ""
echo "==> core/tokens.css around the modified token (near line 1151)"
sed -n '1140,1165p' core/tokens.cssRepository: codeslash-dev/SLASHED
Length of output: 25914
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==> core/tokens.css around modified token"
sed -n '1120,1175p' core/tokens.css
echo ""
echo "==> docs/tokens.md around --sf-header-height entry"
sed -n '450,490p' docs/tokens.md
echo ""
echo "==> docs/migration.md and CHANGELOG.md for sf-header-height/header-height + 8rem/0px"
for f in CHANGELOG.md docs/migration.md; do
echo "---- $f (keywords) ----"
rg -n -i -C2 -- 'sf-header-height|header-height' "$f" || true
echo "---- $f (8rem/0px mentions) ----"
rg -n -i -C1 -- '8rem|0px' "$f" || true
done
echo ""
echo "==> docs/registry.json token entry (default/value references if any)"
rg -n --fixed-strings -- "--sf-header-height" docs/registry.json -C2Repository: codeslash-dev/SLASHED
Length of output: 5572
Major breaking change: --sf-header-height default is now 0px (was 8rem)
core/tokens.cssnow sets--sf-header-height: 0px, which also changes--sf-sticky-offsetand thescroll-margin-top/scroll-padding-topfallbacks for fixed/sticky headers and:targetanchor behavior.docs/tokens.mdstill documents--sf-header-heightas8rem, so the public token docs are out of sync with the actual default.- No migration/breaking-change notes found in
CHANGELOG.mdordocs/migration.mdforsf-header-height/header-heightor the8rem→0pxdefault change.
Fix: Update docs/tokens.md to 0px and add explicit migration/changelog documentation telling users (fixed/sticky header sites) to set --sf-header-height to their header height; re-assess major version bump.
🤖 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 `@core/tokens.css` at line 1151, Update the public docs and release notes to
reflect the new default of --sf-header-height: 0px by changing the documented
default in docs/tokens.md to 0px (and update related mentions of
--sf-sticky-offset and scroll-margin-top/scroll-padding-top fallbacks), add a
clear breaking-change entry in CHANGELOG.md and a migration note in
docs/migration.md explaining that sites with fixed/sticky headers must
explicitly set --sf-header-height to their header height (and provide a brief
example and guidance), and flag this as a potential major-version impact so
maintainers can reassess bumping the version.
Remove the blanket `a { display: inline-block; min-size }` rule — forcing
inline-block on every anchor breaks flex/grid layouts and stretches inline
text links, which is worse than having no touch-target floor.
Navigational links already meet target size from their container.
Add input[type="checkbox"] and input[type="radio"] — these are the most
commonly under-sized touch targets and were previously missing. No display
change needed for replaced elements; min-block/inline-size expands the
hit area without affecting surrounding layout.
https://claude.ai/code/session_01JLvpeyEzNZtgc7TVjVMhzu
- base.css: align iframe/embed/object with the logical-property convention used throughout the file — max-width/height → max-inline-size/block-size - docs/tokens.md: regenerate to reflect --sf-header-height default change (8rem → 0px), fixing the 'Verify generated docs' CI failure https://claude.ai/code/session_01JLvpeyEzNZtgc7TVjVMhzu
0px caused anchor scroll targets to land behind sticky headers on the majority of sites that use SLASHED with page builders. 5rem (80px) covers most sticky nav heights without over-compensating on headerless pages. Also removes the now-unnecessary stylelint-disable comments that were needed for the zero-unit 0px fallback values. https://claude.ai/code/session_01JLvpeyEzNZtgc7TVjVMhzu
8 tasks
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.
Summary
Bricks variable-picker swatches
--sf-color-{family}-lightsource tokens — the resolver read them as inputs but never wrote them to the hex map, leaving checkerboard swatches--sf-color-{success,warning,error,info,danger}-strongtokens which were not generated at all--sf-color-selection-textand--sf-color-mark-text(bothinheritin CSS → approximate with dark-text constant)--sf-color-schemefromget_color_variables()— it holds the stringlight dark, not a colour value, and leaked into the picker via the prefix-only filterCSS reset → lightweight normalize (
core/reset.css)The previous reset used
* { margin: 0; padding: 0 }, stripped list markers globally, forceddisplay: blockon all media, and changed form-element font inheritance site-wide. Any third-party UI (page builder chrome, plugins) that relied on UA defaults for properties it did not explicitly redeclare was silently broken — requiring element-specific patches rather than good architecture.core/reset.cssis now a true normalize:box-sizing: border-box, HTML baselines (text-size-adjust,color-scheme,scroll-padding-top,scrollbar-gutter,hanging-punctuation,interpolate-size),bodymin dimensions, and genuine browser bug fixes only (button text-transform: none,search display: block,table border-collapse,[hidden],dialog/popover,fieldset min-width,[inert]).What moved to
core/base.cssElements SLASHED explicitly styles now carry their own margin/padding resets:
body { margin: 0 },h1–h6 { margin: 0 },p { margin: 0 }. Form-element normalization (font: inherit,cursor: pointer,touch-action,textarea resize) and mediadisplay: blockrules move here too — still in@layer slashed.base(beatable by any unlayered CSS), but no longer zeroing properties of elements we don't own.svg { fill: currentcolor }removed frombase.cssThe global rule was redundant —
.sf-iconalready setsfill: currentcolorexplicitly. Globally it silently broke stroke-based icon sets, multi-colour logos, and any third-party SVG that did not carry its own fill declaration.Test plan
--sf-color-{family}-lighttokens show a filled swatch--sf-color-{success,warning,error,info,danger}-strongshow correct darker swatches--sf-color-schemeno longer appears in the colour variables listbase.cssexplicit declarationshttps://claude.ai/code/session_01JLvpeyEzNZtgc7TVjVMhzu
Generated by Claude Code
Summary by CodeRabbit
Refactor
Style
Accessibility
Documentation