Fix Bricks font tab always visible + WCAG oklch color resolution - #173
Conversation
- FontFamilyField: replace async REST fetch with PHP bootstrap data (window.slashedApp.bricksFonts); Bricks tab now always shows when the Bricks integration is enabled — even when no fonts are registered yet (shows a "No Bricks fonts found" hint instead of hiding the tab) - class-token-page.php: add get_bricks_fonts() static method that collects custom/Google/Adobe/CPT fonts at page-load time (same logic as Slashed_Bricks_Fonts_REST) and passes the list as bricksFonts in the slashedApp bootstrap; includes draft CPT posts so Font Manager uploads appear immediately - stores.svelte.js: expose meta.bricksFonts from bootstrap - WcagTab: replace DOM+regex resolveToRgb with canvas-based approach so oklch() colors (returned by modern browsers from getComputedStyle) resolve correctly; fixes WCAG grid showing only fail cells in red while all other levels rendered white https://claude.ai/code/session_01DU3r3kT7GqH7w7jeadV2DT
|
Warning Review limit reached
More reviews will be available in 54 minutes and 35 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 (2)
📝 WalkthroughWalkthroughThe PR refactors Bricks font loading from asynchronous REST requests to server-side bootstrap injection. A new ChangesBootstrap Bricks fonts for admin app
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
integrations/bricks/admin-app/src/components/WcagTab.svelte (1)
14-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTop-level color-resolution docs are now outdated.
Lines 14-16 still describe hidden DOM +
getComputedStyle, but implementation moved to canvas. Please align the comment to avoid future maintenance confusion.Proposed fix
- * Color resolution uses getComputedStyle() on a hidden proxy element - * so hex, oklch, hsl — anything the browser understands — is handled - * without a custom parser. + * Color resolution uses an offscreen canvas (fillStyle + getImageData) + * so hex, rgb/hsl, oklch — anything the browser parser accepts — + * is handled without a custom parser.🤖 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/admin-app/src/components/WcagTab.svelte` around lines 14 - 16, Update the top-level color-resolution comment in WcagTab.svelte (the doc block that currently mentions using a hidden proxy element and getComputedStyle) to reflect the current implementation which uses an offscreen canvas for color parsing/resolution; replace references to "hidden proxy element" and "getComputedStyle()" with "offscreen/canvas-based parsing" and note that the browser's canvas is used to handle hex, oklch, hsl, etc., so future readers look to the canvas-based functions in this component for the actual logic.
🤖 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/admin-app/src/components/WcagTab.svelte`:
- Around line 52-61: The canvas color parsing can silently fall back to the
previous fillStyle (often black) when cssValue is invalid; before drawing in the
block that creates canvas/ctx and uses ctx.fillStyle, validate cssValue with
CSS.supports('color', cssValue) and return null if it fails, keeping the
existing try/catch and document guards; update the routine that reads
getImageData (references: cssValue, ctx, getImageData) so it only draws when
CSS.supports indicates a valid color.
---
Outside diff comments:
In `@integrations/bricks/admin-app/src/components/WcagTab.svelte`:
- Around line 14-16: Update the top-level color-resolution comment in
WcagTab.svelte (the doc block that currently mentions using a hidden proxy
element and getComputedStyle) to reflect the current implementation which uses
an offscreen canvas for color parsing/resolution; replace references to "hidden
proxy element" and "getComputedStyle()" with "offscreen/canvas-based parsing"
and note that the browser's canvas is used to handle hex, oklch, hsl, etc., so
future readers look to the canvas-based functions in this component for the
actual logic.
🪄 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: f1be0a41-8a85-4fa3-8334-3325a5f235cd
📒 Files selected for processing (6)
includes/class-token-page.phpintegrations/bricks/admin-app/src/components/FontFamilyField.svelteintegrations/bricks/admin-app/src/components/WcagTab.svelteintegrations/bricks/admin-app/src/lib/stores.svelte.jsintegrations/bricks/assets/admin-app/app.cssintegrations/bricks/assets/admin-app/app.js
- Update file-level JSDoc to reference canvas/offscreen approach instead
of the removed getComputedStyle/hidden-element implementation
- Add CSS.supports('color', cssValue) guard before canvas draw to avoid
silent black fallback when an invalid color string is assigned to
fillStyle (invalid assignments are ignored by the browser, keeping the
default black fill)
https://claude.ai/code/session_01DU3r3kT7GqH7w7jeadV2DT
2 participants
Summary
class-token-page.phpnow collects all Bricks fonts (custom, Google, Adobe, Font Manager CPT including draft posts) via a newget_bricks_fonts()static method and passes them aswindow.slashedApp.bricksFontsat page load, eliminating the async REST fetch fromFontFamilyFieldresolveToRgbimplementation with a canvas-based one; modern browsers returnoklch()syntax fromgetComputedStyle, which the old/rgb\(\d+, \d+, \d+\)/regex couldn't parse — causing every non-fail WCAG cell to render whiteChanges
includes/class-token-page.phpget_bricks_fonts()static method; passesbricksFontsinwp_localize_scriptbootstrapstores.svelte.jsmeta.bricksFontsfrom bootstrap dataFontFamilyField.sveltemeta.bricksFontssynchronously; Bricks tab always rendered whenbricksEnabled; empty-state message shown when no fonts registeredWcagTab.svelteresolveToRgbreplaces DOM+regex; correctly resolves hex, rgb, hsl, oklchassets/admin-app/app.{js,css}Test plan
https://claude.ai/code/session_01DU3r3kT7GqH7w7jeadV2DT
Generated by Claude Code
Summary by CodeRabbit
Bug Fixes
Refactor