Admin panel: share WCAG/scale/font logic with the configurator + DX polish - #25
Merged
Merged
Conversation
Port patterns from the framework configurator into the Bricks admin SPA:
- Add sanitizeValue() (from configurator/src/lib/css.js) and apply it to
every raw, user-entered value interpolated into generated CSS and the
live-preview inline style. A pasted Advanced value containing ';' or '}'
can no longer break out of the declaration block / style attribute.
- Extend generateExportCSS() with { mode, banner, version }: a @layer vs
bare :root output-framing toggle and a generated-by header comment.
Defaults preserve existing behaviour for LivePreview/SaveBar.
- ExportImportTab: @layer/:root segmented toggle, a Copy CSS button with
copied/blocked feedback, and a live CSS preview.
- Add ui.outputMode to the store.
- Cover sanitizeValue and generateExportCSS options with unit tests.
Co-authored-by: Jack Granatowski <contact@codeslash.net>
…r + DX polish Extract the framework-agnostic logic the admin panel shares with the SLASHED configurator into three modules that are byte-for-byte identical across both repos, so the two tools can never drift on the behaviours they have in common: - src/lib/color.js — WCAG maths, rgb<->hsl, pure hslToRgb, resolveToRgb, and the accessible-palette optimizer (now pure, no canvas round-trip). - src/lib/scale.js — named ratios + modular-scale maths + clamp() builder. - src/lib/fonts.js — curated system font stacks + detection. Refactor the components to consume them (behaviour unchanged): - WcagTab: drops its local contrast/HSL/optimizer maths. - ScaleGenerator: uses shared RATIOS + modularValue/round (keeps the WP step list incl. display-* and the display-offset control). - FontFamilyField: uses the shared system-stack catalogue + detection. Tests: mirrored color/scale/fonts suites plus a byte-identity drift guard against the framework checkout (skips when absent). 107 tests pass. DX polish: branded header mark, softer body card; tab nav focus-visible outline, active-tab accent and smoother transitions. Admin-app assets rebuilt.
|
Warning Review limit reached
More reviews will be available in 51 minutes and 25 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 (5)
📝 WalkthroughWalkthroughThis PR consolidates color/WCAG, font stack, and modular-scale utilities into shared library modules; adds CSS value sanitization and extends the export API with framing modes; refactors components to use shared utilities; implements clipboard-based CSS export with ChangesShared utilities and export enhancement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
SLASHED-for-WP/integrations/bricks/admin-app/src/components/FontFamilyField.svelte (1)
79-95:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the same normalized system-stack matcher in
switchSource('system').
detectSource()now accepts whitespace/case variants viadetectSystemStack, butswitchSource('system')still does stricttoLowerCase()equality. This mismatch can treat a valid system stack as “no match” and overwrite it withSYSTEM_STACKS[0]when switching tabs.💡 Suggested patch
function switchSource(next) { source = next; // Pre-select sensible first value when switching into a dropdown mode. if (next === 'system') { - const match = SYSTEM_STACKS.find(s => s.value.toLowerCase() === effectiveValue.toLowerCase()); - if (!match) writeField(section, fieldKey, SYSTEM_STACKS[0].value); + const match = detectSystemStack(effectiveValue); + if (!match) { + writeField(section, fieldKey, SYSTEM_STACKS[0].value); + } else if (match.value !== effectiveValue) { + // Canonicalize equivalent values (spacing/case) to the curated string. + writeField(section, fieldKey, match.value); + } } else if (next === 'bricks') { const match = bricksFonts.find(f => f.family.toLowerCase() === effectiveValue.toLowerCase()); if (!match && bricksFonts.length > 0) writeField(section, fieldKey, bricksFonts[0].family); }🤖 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 `@SLASHED-for-WP/integrations/bricks/admin-app/src/components/FontFamilyField.svelte` around lines 79 - 95, The system-stack matching in switchSource should use the same normalization as detectSource/detectSystemStack: replace the strict equality search (SYSTEM_STACKS.find(s => s.value.toLowerCase() === effectiveValue.toLowerCase())) with a call to detectSystemStack(effectiveValue) (or use its returned match) and only call writeField(section, fieldKey, SYSTEM_STACKS[0].value) when that normalized match is falsy; update switchSource (and any local variable name) to rely on detectSystemStack so whitespace/case variants are preserved the same way as detectSource.
🧹 Nitpick comments (1)
SLASHED-for-WP/integrations/bricks/admin-app/src/lib/export.js (1)
452-457: ⚡ Quick winSanitize
versionbefore interpolating it into the banner comment.
versionis currently inserted raw into/* ... */. If bootstrap metadata ever contains*/, it can terminate the comment and inject extra CSS into exported/copied output.Proposed patch
if (banner) { - const v = version ? ` v${version}` : ''; + const safeVersion = sanitizeValue(version); + const v = safeVersion ? ` v${safeVersion}` : ''; const n = declarations.length; css += `/* SLASHED override tokens${v} — generated by the SLASHED for WordPress admin.\n` + ` Load this AFTER the SLASHED stylesheet. ${n} declaration${n === 1 ? '' : 's'}. */\n`; }🤖 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 `@SLASHED-for-WP/integrations/bricks/admin-app/src/lib/export.js` around lines 452 - 457, The banner assembly inserts the raw version into a block comment (variables: banner, version, css, declarations), which allows a malicious version like "*/" to terminate the comment and inject CSS; sanitize version before interpolation by stripping or escaping any closing-comment sequences and control characters (e.g., remove or replace "*/" and non-printable chars) and then use that sanitizedVersion when building the banner string so the comment cannot be prematurely closed or contain unexpected content.
🤖 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
`@SLASHED-for-WP/integrations/bricks/admin-app/src/components/ExportImportTab.svelte`:
- Around line 145-159: The segmented control buttons in ExportImportTab.svelte
don't expose pressed state to assistive tech; update each button (the `@layer`
button bound to ui.outputMode === 'layer' and the :root button bound to
ui.outputMode === 'root') to include an aria-pressed attribute that reflects the
current selection (e.g., aria-pressed={ui.outputMode === 'layer'} and
aria-pressed={ui.outputMode === 'root'}) so screen readers can perceive the
binary toggle state.
In
`@SLASHED-for-WP/integrations/bricks/admin-app/src/components/LivePreview.svelte`:
- Around line 133-134: Guard emissions by the sanitized result rather than raw
truthiness: call sanitizeValue(typography.font_body) and
sanitizeValue(typography.font_heading) into local variables, check those
sanitized values are non-empty, and only then push the CSS custom property
strings via pairs.push(`--sf-font-body:${sanitized}`) /
pairs.push(`--sf-font-heading:${sanitized}`). Update the logic that currently
uses if (typography.font_body) / if (typography.font_heading) so it references
the sanitized variables and skips emitting `--sf-font-*:` when
sanitizeValue(...) returns an empty string.
In `@tests/shared-parity.test.js`:
- Around line 21-27: The test currently only checks haveFramework before reading
files, so if pluginLib is missing the readFileSync calls will throw; add a guard
for the plugin path (e.g., compute havePlugin = existsSync(pluginLib) or check
each resolved plugin path) and use that in the test skip condition (for MODULES
loop) so tests are skipped when either the framework or plugin parity path is
absent; update the skip message to reflect missing plugin or framework and
ensure readFileSync is only executed when both haveFramework and havePlugin are
true.
---
Outside diff comments:
In
`@SLASHED-for-WP/integrations/bricks/admin-app/src/components/FontFamilyField.svelte`:
- Around line 79-95: The system-stack matching in switchSource should use the
same normalization as detectSource/detectSystemStack: replace the strict
equality search (SYSTEM_STACKS.find(s => s.value.toLowerCase() ===
effectiveValue.toLowerCase())) with a call to detectSystemStack(effectiveValue)
(or use its returned match) and only call writeField(section, fieldKey,
SYSTEM_STACKS[0].value) when that normalized match is falsy; update switchSource
(and any local variable name) to rely on detectSystemStack so whitespace/case
variants are preserved the same way as detectSource.
---
Nitpick comments:
In `@SLASHED-for-WP/integrations/bricks/admin-app/src/lib/export.js`:
- Around line 452-457: The banner assembly inserts the raw version into a block
comment (variables: banner, version, css, declarations), which allows a
malicious version like "*/" to terminate the comment and inject CSS; sanitize
version before interpolation by stripping or escaping any closing-comment
sequences and control characters (e.g., remove or replace "*/" and non-printable
chars) and then use that sanitizedVersion when building the banner string so the
comment cannot be prematurely closed or contain unexpected content.
🪄 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: 86a15026-5486-4464-8dc9-66a91b9f1cde
📒 Files selected for processing (19)
SLASHED-for-WP/integrations/bricks/admin-app/src/App.svelteSLASHED-for-WP/integrations/bricks/admin-app/src/components/ExportImportTab.svelteSLASHED-for-WP/integrations/bricks/admin-app/src/components/FontFamilyField.svelteSLASHED-for-WP/integrations/bricks/admin-app/src/components/LivePreview.svelteSLASHED-for-WP/integrations/bricks/admin-app/src/components/ScaleGenerator.svelteSLASHED-for-WP/integrations/bricks/admin-app/src/components/TabNav.svelteSLASHED-for-WP/integrations/bricks/admin-app/src/components/WcagTab.svelteSLASHED-for-WP/integrations/bricks/admin-app/src/lib/color.jsSLASHED-for-WP/integrations/bricks/admin-app/src/lib/export.jsSLASHED-for-WP/integrations/bricks/admin-app/src/lib/fonts.jsSLASHED-for-WP/integrations/bricks/admin-app/src/lib/scale.jsSLASHED-for-WP/integrations/bricks/admin-app/src/lib/stores.svelte.jsSLASHED-for-WP/integrations/bricks/assets/admin-app/app.cssSLASHED-for-WP/integrations/bricks/assets/admin-app/app.jstests/export-css.test.jstests/shared-color.test.jstests/shared-fonts.test.jstests/shared-parity.test.jstests/shared-scale.test.js
…ed_locally Replace the value-seeded `let source = $state(detectSource(...))` with a `userSource` override (null = auto): `source` is now $derived from the value + the live Bricks list until the user explicitly picks a tab. Removes the build warning, drops the manual onMount re-sync, and fixes a latent bug where a font present only in the freshly-fetched Bricks list stayed misclassified. Behaviour for the user is unchanged. Asset rebuilt.
- ExportImportTab: expose pressed state on the @layer/:root framing toggle (aria-pressed) so screen readers perceive the binary selection. - LivePreview: gate font-var emission on the sanitized result so a value that sanitizes to empty no longer emits a valueless --sf-font-*: declaration. Admin-app assets rebuilt.
…el-shared-parity-modules # Conflicts: # SLASHED-for-WP/integrations/bricks/assets/admin-app/app.js
Guard `havePlugin` alongside `haveFramework` so a misresolved/absent plugin lib makes the byte-identity check skip deterministically instead of throwing in readFileSync. (review feedback)
5 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.
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 Web
Summary
Extracts the framework-agnostic logic the admin panel shares with the SLASHED configurator into three modules that are byte-for-byte identical across both repos, so the two tools can never drift on the behaviours they have in common. Behaviour of the existing tabs is unchanged.
Companion PR (framework side):
codeslash-dev/SLASHED→ Configurator: Accessibility + Scales views and shared parity modules.Shared logic modules (
admin-app/src/lib/)rgb↔hsl, purehslToRgb,resolveToRgb, and the accessible-palette optimizer (now pure — no canvas round-trip).clamp()builder.Component refactor (no behaviour change)
RATIOS+modularValue/round(keeps the WP-specific 12-step list incl.display-*and the display-offset control).Tests
shared-color/shared-scale/shared-fontssuites, plusshared-parity— a byte-identity drift guard against the framework checkout (skips cleanly when the framework isn't present).node --test tests/*.test.js).check-admin-appreports no cssVar/default drift.DX polish
focus-visibleoutline, an active-tab accent bar and smoother transitions.assets/admin-app/app.js+app.cssrebuilt from source.Known limitations
state_referenced_locallybuild warning inFontFamilyField.svelteis unrelated to this change and left as-is.Summary by CodeRabbit
New Features
Bug Fixes
Style
Tests