Refresh Bricks fonts dropdown on mount via REST endpoint - #231
Conversation
|
Warning Review limit reached
More reviews will be available in 46 minutes and 7 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 (3)
📝 WalkthroughWalkthroughBricks font selection now fetches live data from the backend on component mount instead of relying solely on PHP bootstrap. A new ChangesLive Bricks Font Refresh
Sequence DiagramsequenceDiagram
participant FontFamilyField as FontFamilyField Component
participant fetchBricksFonts as fetchBricksFonts()
participant Backend as /bricks-fonts Endpoint
FontFamilyField->>FontFamilyField: Initialize with bootstrap bricksFonts
Note over FontFamilyField: onMount triggers
FontFamilyField->>fetchBricksFonts: call fetchBricksFonts()
fetchBricksFonts->>Backend: GET /bricks-fonts with nonce
Backend-->>fetchBricksFonts: data.fonts array
fetchBricksFonts-->>FontFamilyField: updated fonts
FontFamilyField->>FontFamilyField: Update reactive state
alt fetch fails
fetchBricksFonts-->>FontFamilyField: error thrown
FontFamilyField->>FontFamilyField: Retain bootstrap data
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
Previously bricksFonts was a const read once from the PHP bootstrap (window.slashedApp.bricksFonts), so fonts added after the page loaded were invisible until a full reload — and CPT fonts could lag by up to an hour due to the transient cache. Now FontFamilyField initialises from the bootstrap (no flash) and calls GET /slashed/v1/bricks-fonts on mount to pick up any fonts added since the page was served. The bootstrap snapshot is kept as a fallback if the fetch fails. https://claude.ai/code/session_012RcH93KjHBJ7ABJJ1yvyrS
48992dd to
b2dbedd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/SLASHED-for-WP/integrations/bricks/admin-app/src/lib/api.js (1)
96-99: ⚡ Quick winNormalize the REST payload shape before returning.
Line 98 currently returns
data.fonts ?? []; a non-array value can still leak through and break downstream.some/.find/.lengthusage. Coerce to an array at the boundary.Proposed patch
export async function fetchBricksFonts() { const { url, nonce } = meta.rest; if (!url) { console.info('[slashed-admin] (dev) would GET /bricks-fonts'); - return meta.bricksFonts; + return Array.isArray(meta.bricksFonts) ? meta.bricksFonts : []; } const res = await fetch(url + '/bricks-fonts', { credentials: 'same-origin', headers: { 'X-WP-Nonce': nonce }, }); if (!res.ok) throw new Error(await res.text() || `HTTP ${res.status}`); const data = await res.json(); - return data.fonts ?? []; + return Array.isArray(data?.fonts) ? data.fonts : []; }🤖 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 `@plugins/SLASHED-for-WP/integrations/bricks/admin-app/src/lib/api.js` around lines 96 - 99, Normalize the REST payload by coercing data.fonts into an array before returning: in the block that currently does `const data = await res.json(); return data.fonts ?? [];`, replace that return with logic that checks data.fonts and returns an array — e.g., if Array.isArray(data.fonts) return it, if it's a single truthy item wrap it as [data.fonts], otherwise return [] — so downstream callers using .some/.find/.length always get an array.
🤖 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/admin-app/src/components/FontFamilyField.svelte`:
- Around line 61-65: The source inference runs before the async fetchBricksFonts
completes, so if the current font is only present in the refreshed bricksFonts
the UI remains stuck on "manual"; after updating bricksFonts in onMount (and
after the live refresh path around the code at line ~93) call the same function
that infers/sets source (e.g., inferFontSource or the reactive assignment that
computes source from selectedFont and bricksFonts) so the source is recalculated
against the new bricksFonts list; locate the onMount callback, the
fetchBricksFonts call, the bricksFonts assignment, and the place at line ~93 and
invoke the inference routine right after updating bricksFonts.
---
Nitpick comments:
In `@plugins/SLASHED-for-WP/integrations/bricks/admin-app/src/lib/api.js`:
- Around line 96-99: Normalize the REST payload by coercing data.fonts into an
array before returning: in the block that currently does `const data = await
res.json(); return data.fonts ?? [];`, replace that return with logic that
checks data.fonts and returns an array — e.g., if Array.isArray(data.fonts)
return it, if it's a single truthy item wrap it as [data.fonts], otherwise
return [] — so downstream callers using .some/.find/.length always get an array.
🪄 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: b2bc8f25-002d-404a-b198-4e1ccaa2f6ed
📒 Files selected for processing (3)
plugins/SLASHED-for-WP/integrations/bricks/admin-app/src/components/FontFamilyField.svelteplugins/SLASHED-for-WP/integrations/bricks/admin-app/src/lib/api.jsplugins/SLASHED-for-WP/integrations/bricks/assets/admin-app/app.js
- Re-run detectSource after the live font refresh so a font that only appears in the updated list isn't left stuck on 'manual'. - Coerce fetchBricksFonts return to Array.isArray guards at both the dev-harness and live REST paths. https://claude.ai/code/session_012RcH93KjHBJ7ABJJ1yvyrS
Previously bricksFonts was a const read once from the PHP bootstrap
(window.slashedApp.bricksFonts), so fonts added after the page loaded
were invisible until a full reload — and CPT fonts could lag by up to
an hour due to the transient cache.
Now FontFamilyField initialises from the bootstrap (no flash) and calls
GET /slashed/v1/bricks-fonts on mount to pick up any fonts added since
the page was served. The bootstrap snapshot is kept as a fallback if the
fetch fails.
https://claude.ai/code/session_012RcH93KjHBJ7ABJJ1yvyrS
Summary by CodeRabbit