Repository navigation
feat(blocks): fork I4 widgets cluster (search, latest-posts) (#412) - #479
Conversation
Forks the 2-block widgets cluster from `core/*` → `artisanpack/*` per docs/plans/13-block-fork.md §2.2.1 + §3. artisanpack/search — static fork (I1–I3 pattern): block.json, TS-ported edit (drops the block-library-internal ToolsPanel dropdown hook), null save (search is rendered outside save), empty deprecation chain, bidirectional core/search ↔ artisanpack/search transforms, the default variation, inline inserter icon, combined CSS, upstream-state.json, and the four-test suite. The Blade view delegates to core/search and the React/Vue renderers reuse the core/search component, so V1's #338 button-icon a11y fix carries forward unchanged — covered by a carry-forward test in each renderer. The Vue renderer never received the #338 fix, so it's brought into parity here (has-icon + aria-label + svg) to match Blade/React. artisanpack/latest-posts — dynamic fork (artisanpack/form pattern): block.json (full upstream attribute schema), edit previews through the package's <ServerSideRender> seam with a ported PanelBody inspector, null save, the category-string deprecation, bidirectional transforms, inline icon, and tests. Server rendering is a new LatestPostsBlock DynamicBlock querying cms-framework's Post model (modeled on ArchivesBlock), registered in the service provider behind the BlogManager guard. As a dynamic block it is served by the Blade DynamicBlockRegistry, so — like categories/archives/ form — it is intentionally absent from renderer-parity.json. Pinned upstream: @wordpress/block-library v9.43.0. Closes #412 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…over) Now that the content (I1), media (I2), layout (I3), and widgets (I4) clusters ship `artisanpack/*` forks, hide their upstream `core/*` counterparts from the editor inserter so authors only reach for the forks — generalizing the existing I0 paragraph cutover to the whole forked set. - editor/forked-block-cutover.ts: install a `blocks.registerBlockType` filter (before registerCoreBlocks) that stamps `supports.inserter = false` on each forked core block. The blocks stay registered, so legacy `<!-- wp:* -->` markup still deserializes through its deprecation chain and the from:core/* transforms keep working — only the inserter surface changes. Idempotent via the hooks registry; no post-registration mutation of the block registry. core/paragraph stays with cutoverParagraphFork (it also re-homes the default block); core/row + core/stack are covered by suppressing core/group. - config/visual-editor.php: reflect the forks as the canonical names in the enabled_blocks allow-list (artisanpack/* in, forked core/* out; unforked entity/taxonomy/query blocks stay core/*). - Tests for the cutover filter + the updated allow-list. This is the inserter-level cutover only; dropping registerCoreBlocks and removing the core/* renderers is the full I7 cutover, gated on all clusters being forked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds artisanpack/search and artisanpack/latest-posts forks: editor entrypoints, metadata, transforms, styles, variations, tests, three-renderer parity (Blade/React/Vue) with a11y fixes for search, PHP dynamic rendering for latest-posts, and cutover/config updates to prefer artisanpack/* block names. ChangesSearch Block Widget Fork
Latest Posts Block Widget Fork
Block Registration, Cutover, and Configuration
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/Blocks/Core/LatestPostsBlock.php (1)
79-101: ⚡ Quick winValidate attributes at
render()entry to prevent undefined-key paths.
render()currently assumes attrs are already normalized, but downstream helpers dereference required keys directly. Normalizing once at entry makes this method safe and consistent withsearchableText().Proposed patch
public function render( array $attrs ): string { - $posts = $this->fetchPosts( $attrs ); - $classes = $this->wrapperClasses( $attrs ); + $attrs = $this->validateAttrs( $attrs ); + $posts = $this->fetchPosts( $attrs ); + $classes = $this->wrapperClasses( $attrs ); @@ - $items = $posts->map( - fn ( object $post ): string => $this->renderItem( $post, $attrs ) - )->implode( '' ); + $items = $posts->map( + fn ( object $post ): string => $this->renderItem( $post, $attrs ) + )->implode( '' );🤖 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 `@src/Blocks/Core/LatestPostsBlock.php` around lines 79 - 101, render() assumes $attrs are normalized which lets downstream helpers like fetchPosts() and wrapperClasses() dereference keys directly; validate and normalize $attrs at the start of render() (the same approach used by searchableText())—call an existing normalizer (e.g. $this->normalizeAttributes($attrs)) or add one if missing, then use the normalized $attrs for fetchPosts($attrs), wrapperClasses($attrs) and renderItem calls so undefined-key access is prevented.
🤖 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 `@packages/visual-editor-renderer-vue/tests/BlockTree.test.ts`:
- Around line 576-584: The test named "carries the `#338` a11y fix forward to
artisanpack/search (I4 fork)" creates a tree via makeBlock and renders it with
renderTree into html but is missing the negative assertion present in the other
tests; add a negative assertion after the existing positive expects that ensures
the empty search button element with class "wp-block-search__button" is not
present in html (i.e., assert html does not contain an empty button element for
that class) to match parity with the core/search, React, and Blade tests.
In `@resources/js/visual-editor/blocks/latest-posts/deprecated.ts`:
- Around line 32-39: The migrate function may produce categories with id: NaN
because it blindly uses Number(oldAttributes.categories); update migrate (in
deprecated.ts) to parse and validate the legacy value before emitting a category
object: parse the legacy id (e.g., const parsed =
parseInt(oldAttributes.categories as string, 10)), check with
Number.isFinite(parsed) or !Number.isNaN(parsed), and only return categories: [{
id: parsed }] when valid; otherwise return a safe fallback such as categories:
[] (or omit the category) so malformed legacy values don’t become NaN IDs.
In `@resources/js/visual-editor/blocks/search/upstream-state.json`:
- Line 23: The transforms.ts entry in upstream-state.json incorrectly uses
status: "extended" while upstream: "n/a"; update that entry to status: "added"
(same treatment as the deprecated.ts entry) so the entry correctly reflects a
file unique to the fork and matches the schema legend.
In `@src/Blocks/Core/LatestPostsBlock.php`:
- Around line 271-273: In LatestPostsBlock (the branch handling $media),
currently returning (string) $media->url can produce an empty string; change the
guard so it checks that $media is an object and $media->url is non-empty (e.g.
using !empty($media->url) or trim check) and return null for empty values
instead of an empty string so empty featuredImageMedia->url is treated the same
as other null URL branches.
---
Nitpick comments:
In `@src/Blocks/Core/LatestPostsBlock.php`:
- Around line 79-101: render() assumes $attrs are normalized which lets
downstream helpers like fetchPosts() and wrapperClasses() dereference keys
directly; validate and normalize $attrs at the start of render() (the same
approach used by searchableText())—call an existing normalizer (e.g.
$this->normalizeAttributes($attrs)) or add one if missing, then use the
normalized $attrs for fetchPosts($attrs), wrapperClasses($attrs) and renderItem
calls so undefined-key access is prevented.
🪄 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: 2da259f0-a8ee-471c-9697-92da60d5fd6d
📒 Files selected for processing (44)
config/visual-editor.phppackages/renderer-parity.jsonpackages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/search.blade.phppackages/visual-editor-renderer-blade/tests/Unit/BlockRendererTest.phppackages/visual-editor-renderer-react/src/blocks/registerCoreBlocks.tspackages/visual-editor-renderer-react/tests/BlockTree.test.tsxpackages/visual-editor-renderer-vue/src/blocks/core/design.tspackages/visual-editor-renderer-vue/src/blocks/registerCoreBlocks.tspackages/visual-editor-renderer-vue/tests/BlockTree.test.tsresources/js/visual-editor/blocks/latest-posts/__tests__/deprecated.test.tsresources/js/visual-editor/blocks/latest-posts/__tests__/edit.test.tsxresources/js/visual-editor/blocks/latest-posts/__tests__/save.test.tsxresources/js/visual-editor/blocks/latest-posts/__tests__/transforms.test.tsresources/js/visual-editor/blocks/latest-posts/block.jsonresources/js/visual-editor/blocks/latest-posts/constants.tsresources/js/visual-editor/blocks/latest-posts/deprecated.tsresources/js/visual-editor/blocks/latest-posts/edit.tsxresources/js/visual-editor/blocks/latest-posts/index.tsresources/js/visual-editor/blocks/latest-posts/inserter-icon.tsxresources/js/visual-editor/blocks/latest-posts/save.tsxresources/js/visual-editor/blocks/latest-posts/transforms.tsresources/js/visual-editor/blocks/latest-posts/upstream-state.jsonresources/js/visual-editor/blocks/search/__tests__/deprecated.test.tsresources/js/visual-editor/blocks/search/__tests__/edit.test.tsxresources/js/visual-editor/blocks/search/__tests__/save.test.tsxresources/js/visual-editor/blocks/search/__tests__/transforms.test.tsresources/js/visual-editor/blocks/search/block.jsonresources/js/visual-editor/blocks/search/constants.tsresources/js/visual-editor/blocks/search/deprecated.tsresources/js/visual-editor/blocks/search/edit.tsxresources/js/visual-editor/blocks/search/index.tsresources/js/visual-editor/blocks/search/inserter-icon.tsxresources/js/visual-editor/blocks/search/save.tsxresources/js/visual-editor/blocks/search/search.cssresources/js/visual-editor/blocks/search/transforms.tsresources/js/visual-editor/blocks/search/upstream-state.jsonresources/js/visual-editor/blocks/search/variations.tsresources/js/visual-editor/editor/__tests__/forked-block-cutover.test.tsresources/js/visual-editor/editor/editor-app.tsxresources/js/visual-editor/editor/forked-block-cutover.tssrc/Blocks/Core/LatestPostsBlock.phpsrc/VisualEditorServiceProvider.phptests/Unit/VisualEditor/Blocks/Core/LatestPostsBlockTest.phptests/Unit/VisualEditor/EnabledBlockNamesTest.php
- latest-posts deprecation: guard the legacy string→id conversion so a
malformed value drops the category instead of migrating to { id: NaN }.
- LatestPostsBlock::featuredImageUrl(): treat an empty media-relation url
as null so the renderer never emits src="".
- Vue artisanpack/search a11y test: add the missing negative assertion
(no empty submit button), matching the core/React/Blade tests.
- upstream-state.json (search + latest-posts): transforms.ts is `added`
(no upstream file), not `extended`, per the schema legend.
- Tests for the NaN-guard and the media-relation url branches.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Description
Forks the I4 widgets cluster (
core/search,core/latest-posts) into theartisanpack/*namespace perdocs/plans/13-block-fork.md§2.2.1 + §3, and (as a follow-on cleanup) hides the now-supersededcore/*blocks from the I1–I4 clusters from the editor inserter.Closes: #412
Type of Change
Related Issue
Issue: #412
Motivation and Context
#331inverts V1's "adopt@wordpress/block-librarywholesale" bet by owning the block source underartisanpack/*. I4 is the 2-block widgets cluster. Shipping the fork inside 1.0.0 means no host has persistedcore/*trees, so there is no migration to perform — only a namespace swap with full three-renderer parity.Changes Made
artisanpack/search(static fork — I1–I3 pattern)block.json, TS edit, nullsave, empty deprecations, bidirectionalcore/search ↔ artisanpack/searchtransforms, default variation, inline icon, CSS,upstream-state.json, 4 tests).core/search; React/Vue reuse thecore/searchcomponent — so V1's core/searchbuttonUseIconrenders an unlabeled submit button (a11y) #338 button-icon a11y fix carries forward unchanged (covered by a carry-forward test in each renderer).core/searchrenderer into parity with Blade/React on the core/searchbuttonUseIconrenders an unlabeled submit button (a11y) #338 fix (it had never received it).artisanpack/latest-posts(dynamic fork —artisanpack/formpattern)<ServerSideRender>-backed editor preview + ported inspector, nullsave, category-string deprecation, bidirectional transforms, tests.LatestPostsBlockDynamicBlock(PHP) querying cms-framework'sPostmodel (modeled onArchivesBlock), registered behind theBlogManagerguard. As a dynamic block it's served by the BladeDynamicBlockRegistry, so — like categories/archives/form — it's intentionally absent fromrenderer-parity.json.Cleanup (separate commit): hide forked
core/*from the inserter (I1–I4 cutover)blocks.registerBlockTypefilter stampssupports.inserter = falseon every forkedcore/*block (installed beforeregisterCoreBlocks, so no registry mutation). Blocks stay registered → legacy markup still deserializes andfrom:core/*transforms keep working; only the inserter surface changes.enabled_blocksreorganized so the forks are the canonical names; unforked entity/taxonomy/query blocks staycore/*.registerCoreBlocks/ removingcore/*renderers is the full I7 cutover, gated on all clusters being forked.Pinned upstream:
@wordpress/block-libraryv9.43.0.How Has This Been Tested?
Testing Environment:
jmwd-keystone-cmshost apprelease/1.0)Tests Performed:
1400 passing(200 files); PHP suite485 passing;npm run buildsucceeds;verify:parityOK.has-icon/aria-label/<svg>on the front end) and Latest Posts (server-rendered preview lists real posts; inspector toggles update it); verifiedcore/*↔artisanpack/*transforms.artisanpack/*versions for all forked clusters; existingcore/*content still opens and renders.Accessibility Tests Run
Details: The #338 fix (carried forward to
artisanpack/searchacross all three renderers) ensures the icon-only search submit button always exposes an accessible name (buttonText→ label → "Search") plus the magnifier SVG withhas-icon. Verified the labelled button output in Blade/React/Vue via tests and in-canvas.Tests Added
Test details: Per-block save/edit/transforms/deprecated suites for both forks;
LatestPostsBlockTest(PHP) for the dynamic renderer; #338 carry-forward assertions in all three renderer suites;forked-block-cutovertest for the cutover filter + allow-list. UpdatedEnabledBlockNamesTestto the new allow-list.Documentation
Documentation details: Each forked block ships an
upstream-state.jsondocumenting the per-file fork status and divergences from upstream v9.43.0.Pre-Submission Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests