Repository navigation
feat: rendered-block + canvas-styles extension filters - #651
Merged
Merged
Conversation
…vas-styles filters Two extension points third-party packages need to decorate rendered output without editing the visual-editor package. - `ap.visual-editor.rendered-block` (PHP) fires in `BlockRenderer::renderBlock` after each block (static or dynamic) finishes rendering. Callbacks receive the HTML, the block name, and the normalized attributes, and must return an HTML string. Lets cross-cutting effects (frosted glass, contrast overlays, motion wrappers) post-process a block's markup without every host having to modify the renderer. - `ap.visual-editor.canvas-styles` (JS) filters the ordered `CanvasStyle[]` handed to `BlockCanvas`'s `__unstableEditorStyles` prop. Callbacks return a `CanvasStyle[]`; non-array returns fall through to the base list and non-object entries are dropped. Lets packages push their stylesheets into the Gutenberg iframe (which is sandboxed from the parent document's Vite-injected styles) without touching `canvas-styles.ts` for every new integration. Motivated by the artisanpack-ui/liquid-glass integration — the package now uses both filters to (1) decorate dynamic-block HTML with the frosted-glass class + CSS vars and (2) inject its stylesheet into the iframe canvas. No behavioral change for hosts that don't hook them. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…eir contracts Follow-up to b1a8fa0. A code-review pass on the filter feature surfaced that neither extension point had test coverage and that three real implementer footguns were undocumented. This commit closes both gaps: Docs - `BlockRenderer::renderBlock`: call out the RECURSION behavior (the filter fires once per block at every level of the tree, so a container-wrapping callback also wraps every descendant unless it branches on `$name`/`$attributes`) and the ATTRIBUTES SHAPE contract (the array already carries the internal `_resolved*` keys stamped by `stampSiteMeta` / `stampLoginout`, which are a package-internal side channel, not stable public attributes). - `canvas-styles.ts`: fold the stranded pre-existing docblock into a single `baseCanvasStyles` block, and add a TIMING callout to the `canvasStyles` export — the IIFE runs `applyFilters` synchronously at module-load, so callbacks registered after the first import silently no-op. Tests - `BlockRendererTest`: new `describe('ap.visual-editor.rendered-block filter')` covering static wrap, dynamic wrap, per-level recursion order (`[paragraph, paragraph, group]`), non-string-return discard, stamped-attribute pass-through, and no-op with zero callbacks. - `canvas-styles.filter.test.ts`: new file, mocks `@wordpress/hooks` per the sibling `fork-class-name-alias.test.ts` pattern and covers the callback receiving a mutable copy of the base list, appended entries landing in the export, non-array fallback to base, and drop-on-shape-mismatch for both non-string `css` and non-object entries. Verified: pest BlockRendererTest → 89 passed (was 83, +6); vitest editor/__tests__ → 252 passed across 28 files (was 247, +5). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR documents a Font Library feature and adds extension filters for rendered blocks and canvas styles. Both filters validate callback results and include focused automated tests. ChangesFont Library specification
Editor filter hooks
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant BlockRenderer
participant DynamicBlockRegistry
participant applyFilters
BlockRenderer->>DynamicBlockRegistry: resolve dynamic block
DynamicBlockRegistry-->>BlockRenderer: rendered HTML
BlockRenderer->>applyFilters: apply ap.visual-editor.rendered-block
applyFilters-->>BlockRenderer: filtered or original HTML
sequenceDiagram
participant canvasStyles
participant WordpressHooks
participant BlockCanvas
canvasStyles->>WordpressHooks: apply ap.visual-editor.canvas-styles
WordpressHooks-->>canvasStyles: filtered stylesheet list
canvasStyles->>BlockCanvas: export validated canvasStyles
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 6
🧹 Nitpick comments (1)
docs/plans/font-library.md (1)
172-172: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftHarden or avoid the bespoke font parser.
If a minimal parser is shipped, require strict bounds checks, malformed-table rejection, resource limits, and fuzz testing. Prefer a maintained parser where practical.
🤖 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 `@docs/plans/font-library.md` at line 172, Update the VariableFontMetadataParser plan to prefer a maintained OTF/TTF parser where practical; if a bespoke reader remains necessary, require strict bounds validation, malformed-table rejection, resource limits, and fuzz testing for its fvar and name table handling.
🤖 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 `@docs/plans/font-library.md`:
- Line 70: Add an idempotency constraint for font_faces using a composite unique
key that matches the intended font-face cardinality, then update the font
installation persistence flow to use an upsert against that key so retries and
concurrent installs cannot create duplicate rows or `@font-face` rules.
- Line 55: Update the Google-tab preview flow in the font-library plan to avoid
loading the provider’s stylesheet in the browser before installation: specify a
backend or locally hosted preview asset path, or explicitly document the runtime
request as a GDPR/no-CDN exception requiring user consent. Apply the same
correction to the corresponding second preview reference.
- Around line 162-165: Update the upload configuration and handling associated
with the 'upload' settings to validate actual font signatures and container
structure server-side rather than trusting allowed_mime_types. Enforce the 5120
KB limit using the uploaded file’s actual size, reject malformed font tables
before storage or metadata parsing, and only continue for structurally valid
WOFF2, WOFF, TTF, or OTF files.
- Line 84: Update the generated fonts.css requirement to specify cache
invalidation after install, uninstall, or upload by enqueueing the stylesheet
with a content-hash or modification-time revision. Ensure the editor iframe and
public site use the revised URL, and add tests covering cache-warm scenarios so
regenerated font rules are served.
- Around line 157-159: Update the providers configuration around the google
entry so it does not unconditionally enable Google without a key policy: gate
enabled on VE_GOOGLE_FONTS_API_KEY being present and document the
disabled/diagnostic behavior, or explicitly define startup failure when the key
is missing. Keep the bunny provider configuration unchanged.
In `@resources/js/visual-editor/editor/canvas-styles.ts`:
- Around line 242-258: Update the canvasStyles module-load initialization around
applyFilters for the ap.visual-editor.canvas-styles hook to catch thrown filter
callbacks and return baseCanvasStyles. Preserve the existing non-array fallback
and CanvasStyle validation for successfully returned filter results, ensuring
initialization cannot fail because a callback throws.
---
Nitpick comments:
In `@docs/plans/font-library.md`:
- Line 172: Update the VariableFontMetadataParser plan to prefer a maintained
OTF/TTF parser where practical; if a bespoke reader remains necessary, require
strict bounds validation, malformed-table rejection, resource limits, and fuzz
testing for its fvar and name table handling.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2ba6eb9e-e883-4be7-9fdd-687ff69b71f6
📒 Files selected for processing (5)
docs/plans/font-library.mdpackages/visual-editor-renderer-blade/src/BlockRenderer.phppackages/visual-editor-renderer-blade/tests/Unit/BlockRendererTest.phpresources/js/visual-editor/editor/__tests__/canvas-styles.filter.test.tsresources/js/visual-editor/editor/canvas-styles.ts
|
|
||
| 1. Admin opens Site Editor → clicks **Styles → Typography → Manage Fonts**. Font Library modal opens. | ||
| 2. Modal shows tabs: **Installed**, **Google Fonts**, **Bunny Fonts**, **Upload**, plus any provider registered via the registry. | ||
| 3. Admin searches "Inter" in the Google tab. Provider returns catalog results with a live preview rendered in the browser using the provider's preview stylesheet (loaded only for the modal session). |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Reconcile preview loading with the no-CDN/GDPR contract.
Loading a provider stylesheet in the browser is still a third-party runtime request before installation, contradicting the “no runtime requests” guarantee. Use a backend/local preview path, or explicitly document and consent to this exception.
Also applies to: 177-177
🤖 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 `@docs/plans/font-library.md` at line 55, Update the Google-tab preview flow in
the font-library plan to avoid loading the provider’s stylesheet in the browser
before installation: specify a backend or locally hosted preview asset path, or
explicitly document the runtime request as a GDPR/no-CDN exception requiring
user consent. Apply the same correction to the corresponding second preview
reference.
| ### Edge cases | ||
|
|
||
| - **Provider network failure during install**: rollback the DB row, remove partial files, surface a user-visible error with retry. | ||
| - **Font already installed at a different weight set**: adding new weights merges into the existing `Font`; removing weights that a block references warns before delete. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add an idempotency constraint for font_faces.
The merge behavior is not enforced by the schema, so retries or concurrent installs can create duplicate rows and duplicate @font-face rules. Add a composite unique key matching the intended face cardinality and use an upsert.
Also applies to: 123-123
🤖 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 `@docs/plans/font-library.md` at line 70, Add an idempotency constraint for
font_faces using a composite unique key that matches the intended font-face
cardinality, then update the font installation persistence flow to use an upsert
against that key so retries and concurrent installs cannot create duplicate rows
or `@font-face` rules.
| - [ ] `FontSourceRegistry` registers providers and is seeded via `ap.visual-editor.register-font-sources` filter. | ||
| - [ ] Google Fonts and Bunny Fonts providers register themselves in the package service provider. | ||
| - [ ] Installing a font from any provider downloads files to the configured disk and creates DB rows. | ||
| - [ ] A single generated `fonts.css` file is rebuilt after every install/uninstall/upload and is enqueued in both the editor iframe and the public site. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Specify cache invalidation for regenerated fonts.css.
After install or uninstall, the same URL may remain cached, leaving the editor and public site with stale font rules. Enqueue a content-hash or modification-time revision and cover cache-warm scenarios in tests.
🤖 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 `@docs/plans/font-library.md` at line 84, Update the generated fonts.css
requirement to specify cache invalidation after install, uninstall, or upload by
enqueueing the stylesheet with a content-hash or modification-time revision.
Ensure the editor iframe and public site use the revised URL, and add tests
covering cache-warm scenarios so regenerated font rules are served.
Comment on lines
+157
to
+159
| 'providers' => [ | ||
| 'google' => ['enabled' => true, 'api_key' => env('VE_GOOGLE_FONTS_API_KEY')], | ||
| 'bunny' => ['enabled' => true], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Relevant section ==\n'
nl -ba docs/plans/font-library.md | sed -n '145,200p'
printf '\n== Search for VE_GOOGLE_FONTS_API_KEY ==\n'
rg -n "VE_GOOGLE_FONTS_API_KEY|google.*api_key|api_key.*google|enabled'.*google|providers' =>" docs src . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'
printf '\n== Nearby plan references ==\n'
rg -n "Google Fonts|Bunny|provider|validation|disabled|diagnostic|api key|VE_GOOGLE" docs/plans/font-library.mdRepository: ArtisanPack-UI/visual-editor
Length of output: 230
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Relevant section ==\n'
sed -n '145,200p' docs/plans/font-library.md | cat -n
printf '\n== Search for VE_GOOGLE_FONTS_API_KEY ==\n'
rg -n "VE_GOOGLE_FONTS_API_KEY|google.*api_key|api_key.*google|enabled'.*google|providers' =>" docs src . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' || true
printf '\n== Nearby plan references ==\n'
rg -n "Google Fonts|Bunny|provider|validation|disabled|diagnostic|api key|VE_GOOGLE" docs/plans/font-library.md || trueRepository: ArtisanPack-UI/visual-editor
Length of output: 8342
Don’t default Google to enabled without a key policy. VE_GOOGLE_FONTS_API_KEY is still unresolved here, so either gate enabled on the key’s presence and define the disabled/diagnostic state, or state that startup fails fast when the key is missing.
🤖 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 `@docs/plans/font-library.md` around lines 157 - 159, Update the providers
configuration around the google entry so it does not unconditionally enable
Google without a key policy: gate enabled on VE_GOOGLE_FONTS_API_KEY being
present and document the disabled/diagnostic behavior, or explicitly define
startup failure when the key is missing. Keep the bunny provider configuration
unchanged.
Comment on lines
+162
to
+165
| 'upload' => [ | ||
| 'max_file_size' => 5120, // KB | ||
| 'allowed_mime_types' => ['font/woff2', 'font/woff', 'font/ttf', 'font/otf'], | ||
| ], |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate font contents, not only MIME labels.
Require server-side signature/container validation, size enforcement after upload, and malformed-table rejection before storage or metadata parsing. Client-provided MIME types are not trustworthy.
🤖 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 `@docs/plans/font-library.md` around lines 162 - 165, Update the upload
configuration and handling associated with the 'upload' settings to validate
actual font signatures and container structure server-side rather than trusting
allowed_mime_types. Enforce the 5120 KB limit using the uploaded file’s actual
size, reject malformed font tables before storage or metadata parsing, and only
continue for structurally valid WOFF2, WOFF, TTF, or OTF files.
Comment on lines
+242
to
+258
| export const canvasStyles: readonly CanvasStyle[] = ( (): CanvasStyle[] => { | ||
| const filtered = applyFilters( | ||
| 'ap.visual-editor.canvas-styles', | ||
| [...baseCanvasStyles], | ||
| ); | ||
|
|
||
| if ( ! Array.isArray( filtered ) ) { | ||
| return baseCanvasStyles; | ||
| } | ||
|
|
||
| return filtered.filter( | ||
| ( entry ): entry is CanvasStyle => | ||
| entry !== null && | ||
| typeof entry === 'object' && | ||
| typeof ( entry as { css?: unknown } ).css === 'string', | ||
| ); | ||
| } )(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Guard the module-load-time filter call against thrown callbacks.
applyFilters('ap.visual-editor.canvas-styles', …) runs during module evaluation, so a throwing callback can abort the import path and break the canvas bundle. Wrap it and fall back to baseCanvasStyles so one bad filter cannot take down initialization.
🤖 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 `@resources/js/visual-editor/editor/canvas-styles.ts` around lines 242 - 258,
Update the canvasStyles module-load initialization around applyFilters for the
ap.visual-editor.canvas-styles hook to catch thrown filter callbacks and return
baseCanvasStyles. Preserve the existing non-array fallback and CanvasStyle
validation for successfully returned filter results, ensuring initialization
cannot fail because a callback throws.
github-project-automation
Bot
moved this from Backlog
to Done
in ArtisanPack UI Overview
18 of 21 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
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
Adds two extension points third-party packages need to decorate rendered output without editing this package, plus test coverage and contract documentation from a follow-up code review.
ap.visual-editor.rendered-block(PHP,BlockRenderer::renderBlock) — fires after each block (static or dynamic) renders. Callbacks receive(string $html, string $name, array $attributes)and must return an HTML string. Non-string returns are discarded. Lets cross-cutting effects (frosted glass, contrast overlays, motion wrappers) post-process markup without every host modifying the renderer.ap.visual-editor.canvas-styles(JS,editor/canvas-styles.ts) — filters the orderedCanvasStyle[]handed toBlockCanvas's__unstableEditorStylesprop. Non-array returns fall through to the base list; non-object entries and entries without a stringcssare dropped. Lets packages push stylesheets into the sandboxed Gutenberg iframe without touchingcanvas-styles.tsfor every new integration.Motivated by the
artisanpack-ui/liquid-glassintegration, which now uses both filters to (1) decorate dynamic-block HTML with the frosted-glass class + CSS vars and (2) inject its stylesheet into the iframe canvas. No behavioral change for hosts that don't hook them.Documented contracts (added in the follow-up commit)
$name/$attributes.$attributesalready carries the internal_resolved*keys stamped bystampSiteMeta/stampLoginout. Treated as a package-internal side channel, not stable public attributes.applyFiltersruns synchronously at module-load inside an IIFE and the resultingcanvasStylesexport is frozen for the session. Callbacks must be registered before the first import.Test coverage
BlockRendererTest→ newdescribe('ap.visual-editor.rendered-block filter')block, 6 tests covering static wrap, dynamic wrap, per-level recursion order, non-string-return discard, stamped-attribute pass-through, and no-op with zero callbacks. Matches theBreadcrumbsResolverTestguard pattern.canvas-styles.filter.test.ts→ new file, mocks@wordpress/hooksper the siblingfork-class-name-alias.test.tspattern; 5 tests covering the callback receiving a mutable copy of the base list, appended entries landing in the export, non-array fallback to base, and drop-on-shape-mismatch for non-stringcssand non-object entries.Heads up before merging
Commit
43a2dc8(docs: add feature plan for font library) is also on this branch but is unrelated to the filter feature. If it shouldn't ship together, cherry-pick it onto its own branch and drop it from here before merging.Test plan
pest packages/visual-editor-renderer-blade/tests/Unit/BlockRendererTest.php→ 89 passed / 264 assertions (was 83, +6)vitest run resources/js/visual-editor/editor/__tests__/→ 252 passed across 28 files (was 247, +5)ap.visual-editor.rendered-blockfrom a host app; verify HTML mutation reaches the frontendap.visual-editor.canvas-stylesfrom a host app entry that loads before the editor bundle; verify the injected stylesheet appears inside theBlockCanvasiframeartisanpack-ui/liquid-glass— frosted-glass class + CSS vars land on dynamic blocks, and liquid-glass CSS renders inside the editor iframe🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Bug Fixes