Skip to content

feat: block.json architecture, registries, RichText & InnerBlocks primitives - #306

Merged
ViewFromTheBox merged 3 commits into
add/phase-twofrom
enhancement/302-block-json-architecture-registries-primitives
Apr 16, 2026
Merged

ViewFromTheBox merged 3 commits into
add/phase-twofrom
enhancement/302-block-json-architecture-registries-primitives

Conversation

@ViewFromTheBox

@ViewFromTheBox ViewFromTheBox commented Apr 16, 2026 •

Copy link
Copy Markdown
Member

Description

Rebuild the block registration and rendering architecture to follow a WordPress Gutenberg-inspired block.json-driven, SOLID-principled model. Replaces hardcoded block wiring with a fully extensible system that third-party Composer packages can plug into.

Closes: #302

Type of Change

  • New feature (adds new functionality)
  • Enhancement (improves existing functionality)
  • Refactoring (code improvement, no behavior change)

Related Issue

Issue: #302

Motivation and Context

The previous block system hardcoded block names in shared code (toolbar visibility, split/merge logic) and wired blocks in main.tsx, making it impossible for third-party packages to add custom blocks. This overhaul follows WordPress's block.json pattern where blocks declare their capabilities via manifests, and the editor queries them dynamically via blockSupports().

Changes Made

Architecture

  • Created registry/types.ts with full WordPress-parity BlockJsonMetadata, BlockSupports, BlockAttributeSchema types
  • Added registerBlockType(metadata, settings) as primary registration API
  • Added blockSupports() and getBlockSupport() for dot-path feature queries
  • Refactored inserterRegistry to derive its list from unified block registry
  • Removed all hardcoded block name sets (TEXT_BLOCK_NAMES, toolbar name checks)
  • Standardized block namespace from ve/ to artisanpack/

Primitives

  • Created <RichText> component wrapping Tiptap with focus/selection sync
  • Heading and paragraph edit components refactored to use <RichText>

PHP

  • VisualEditor::registerBlock($path) reads block.json and registers in BlockTypeRegistry
  • Removed hardcoded seeds from BlockTypeRegistry constructor
  • Service provider registers core blocks via block.json in boot()
  • REST endpoint response key: { data } → { blocks }
  • Facade updated to VisualEditor::class accessor

ES Module Export

  • Public API barrel index.ts exports registry, primitives, store types
  • Vite library build (npm run build:lib) produces 18.69 kB ES module
  • package.json exports field configured

New Blocks (using new architecture)

  • List, Quote, Code, Preformatted — all with block.json, registerBlockType(), <RichText>

How Has This Been Tested?

Testing Environment:

  • Operating System: macOS
  • PHP Version: 8.4.3
  • Node: v22

Tests Performed:

  1. Vitest suite: 215 JS tests passing (26 test files)
  2. PHP Pest suite: 27 tests passing (76 assertions)
  3. Manual: All 6 blocks insertable, editable, slash command works
  4. Library build: npm run build:lib produces valid ES module

Accessibility Tests Run

  • Keyboard navigation tested
  • Screen reader tested
  • Color contrast verified
  • ARIA labels checked

Details: Code textarea has aria-label, quote citation has aria-label, list type toolbar buttons have aria-labels, language select has aria-label.

Tests Added

  • Unit tests added/updated
  • Integration tests added/updated
  • All tests passing

Test details:

  • 4 new block test files (list, quote, code, preformatted)
  • Updated inserterRegistry, insertBlockActions, InserterPanel, slashCommand tests for new namespace
  • Updated PHP BlockTypeRegistryTest and BlocksControllerTest for new architecture

Documentation

  • Inline code documentation added
  • README updated
  • Wiki updated
  • API documentation updated

Documentation details: JSDoc added to key new APIs. Full documentation deferred to docs issue.

Pre-Submission Checklist

  • Code passes all tests
  • Code follows project style guide
  • Self-review completed
  • No new warnings generated

Summary by CodeRabbit

  • New Features

    • Adds Code, List (bulleted/numbered), Quote (with citation), Preformatted, Heading, and Paragraph blocks to the visual editor; introduces a shared rich-text editor primitive and a public editor API surface.
    • Toolbar now exposes list-type controls for quick toggling between bulleted/numbered lists.
    • Inserter panel now surfaces core blocks from manifests and respects registry-sourced blocks.
  • Tests

    • Added comprehensive block-level tests covering registration, rendering, editing, and attribute persistence.
  • Chores

    • Build tooling updated to produce a library bundle and expose the editor entry with TypeScript typings.

…mitives

Rebuild the block registration and rendering architecture to follow a
WordPress Gutenberg-inspired block.json-driven, SOLID-principled model.
This replaces hardcoded block wiring with a fully extensible system that
third-party Composer packages can plug into.

Architecture:
- block.json manifests per block (WordPress-parity schema with supports,
  attributes, metadata, nesting, assets, styles, variations)
- registerBlockType(metadata, settings) merges block.json + JS edit
  component, replacing the old registerBlock() API
- blockSupports() utility for dot-path feature queries, replacing all
  hardcoded block name sets (TEXT_BLOCK_NAMES, toolbar checks)
- Unified block registry with observer pattern; inserter derives its
  list from the block registry instead of a separate metadata map
- Block namespace standardized from ve/ to artisanpack/

Primitives:
- <RichText> component wrapping Tiptap with focus/selection sync,
  heading level reactivity, and Enter/Backspace delegation
- Paragraph and heading refactored to use <RichText>
- <InnerBlocks> (existing) exported via public API

PHP:
- VisualEditor::registerBlock($blockJsonPath) reads block.json manifests
- BlockTypeRegistry no longer seeds blocks in constructor
- Service provider registers core blocks via block.json in boot()
- REST endpoint response key changed from {data} to {blocks}
- Facade updated to VisualEditor::class accessor

ES Module:
- Public API barrel (index.ts) exports registry, primitives, store types
- Vite library build mode (npm run build:lib) produces 18.69 kB ES module
- package.json exports field configured

New blocks (using new architecture):
- List: BulletList/OrderedList with toolbar toggle, Enter-on-empty exits
- Quote: Blockquote with citation input
- Code: Plain textarea with language dropdown, controlled Tab-indent
- Preformatted: Custom Tiptap node with inline formatting

Closes #302
@coderabbitai

coderabbitai Bot commented Apr 16, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c4be6c16-e078-4dfb-a1e5-73fca8f29b06

📥 Commits

Reviewing files that changed from the base of the PR and between e4991a9 and 9cbd4c7.

📒 Files selected for processing (1)
  • resources/js/visual-editor/editor/blocks/paragraph/edit.tsx

📝 Walkthrough

Walkthrough

Implements a block.json-driven visual editor: adds Gutenberg-style block manifests and refactored block editors using a new RichText primitive; introduces typed JS registry (registerBlockType, subscribeRegistry, blockSupports) and inserter overhaul; exposes a public JS API, PHP VisualEditor registration methods, and library build output.

Changes

Cohort / File(s) Summary
Package / Build
package.json, vite.config.ts
Add build:lib npm script and exports entry; Vite gains mode==='lib' path producing dist/lib/visual-editor.js with externals for React/Zustand/TipTap.
Public JS API Entry
resources/js/visual-editor/editor/index.ts
New JS entry exporting registry APIs, primitives, store/hooks, block editor helpers, and split/merge handlers.
Registry Types & Runtime
resources/js/visual-editor/editor/registry/*
Add types.ts, expand blockRegistry with registerBlockType, snapshot caching, getRegisteredBlocks, subscribeRegistry, new BlockDefinition/BlockFactory types; add blockSupports/getBlockSupport.
Inserter
resources/js/visual-editor/editor/inserter/*
Inserter now sources blocks from registry, supports supplemental inserter blocks, adds registerBlockFactory/clearFactories, and deprecates builtin inserter seeding (no-op).
RichText Primitive
resources/js/visual-editor/editor/primitives/RichText.tsx, .../index.ts
Introduce RichText controlled Tiptap primitive (tag→node mapping, focus/cursor, onChange/onEnter/onBackspace props) and exported props type.
Core Blocks (manifests + editors + registration)
resources/js/visual-editor/editor/blocks/*/block.json, .../edit.tsx, .../index.ts
Add block.json manifests and refactor paragraph, heading, list, quote, code, preformatted blocks to derive metadata.name, use RichText/node wrappers, export normalization helpers, and provide registerXBlock() + factory wiring.
Toolbar & Shared Logic
resources/js/visual-editor/editor/components/RichTextToolbar.tsx, .../shared/blockSplitMerge.ts
Replace hardcoded block-name gates with registry queries (getBlock/blockSupports); add list-type controls and dynamic heading gating.
Block Editor Primitives / Registry Consumers
resources/js/visual-editor/editor/primitives/*, .../blocks/index.ts
Re-exports updated; registerCoreBlocks() extended to register new blocks; blocks re-export names/helpers from their modules.
Tests (Vitest & PHP)
resources/js/visual-editor/editor/blocks/__tests__/*, .../inserter/__tests__/*, .../components/__tests__/*, tests/*
Add many block-specific Vitest suites; update tests to use registerCoreBlocks() and metadata constants; PHP tests updated for artisanpack/* names and changed REST payload key blocks.
PHP Backend & Service Provider
src/VisualEditor.php, src/Registries/BlockTypeRegistry.php, src/VisualEditorServiceProvider.php, src/Facades/VisualEditor.php, src/Http/Controllers/VisualEditorBlocksController.php
Add PHP VisualEditor API (registerBlock, registerBlockType, getRegistry), remove registry auto-seed, add get() on registry, change service binding to inject BlockTypeRegistry, add registerCoreBlocks() on boot, and change controller response key to blocks.

Sequence Diagram(s)

sequenceDiagram
    participant NPM as package.json
    participant Vite as Vite (mode=lib)
    participant Dist as dist/lib/visual-editor.js
    participant ThirdParty as Third-Party Package

    NPM->>Vite: npm run build:lib (--mode lib)
    Vite->>Vite: library build (externals, es output)
    Vite->>Dist: emit ES module `visual-editor.js` + types
    ThirdParty->>Dist: import { registerBlockType, RichText, InnerBlocks }
    ThirdParty->>ThirdParty: call registerBlockType(metadata, { edit })
Loading
sequenceDiagram
    participant PHP as ServiceProvider
    participant VE as VisualEditor (PHP)
    participant Registry as BlockTypeRegistry (PHP)
    participant REST as BlocksController
    participant JS as JS runtime / Editor

    PHP->>VE: boot() → registerCoreBlocks()
    VE->>Registry: registerBlock(path/block.json)
    REST->>Registry: all()
    Registry-->>REST: [{ name, title, attributes, supports }]
    JS->>REST: GET /visual-editor/api/blocks
    JS->>JS: subscribeRegistry() → populate client registry
    UI->>JS: blockSupports(name, 'feature')
    JS-->>UI: true/false
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

"A Rabbit's Registry Rhyme"
I nibble manifests from json beds,
Hop into code where RichText treads,
Registries hum and blocks parade,
Third-party friends join the meadow shade,
I thump my paw — hooray for every new block made! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main architectural changes: introducing block.json-driven architecture with registries, RichText and InnerBlocks primitives.
Linked Issues check ✅ Passed The PR comprehensively implements all acceptance criteria from issue #302: block.json manifests for core blocks, registerBlockType() API, RichText/InnerBlocks primitives, PHP registry with registerBlock(), blockSupports() utility, ES module exports, and full test coverage.
Out of Scope Changes check ✅ Passed All changes align with issue #302 objectives: block.json architecture, JS/PHP registries, primitives, and core block migrations. No unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch enhancement/302-block-json-architecture-registries-primitives

Comment @coderabbitai help to get the list of available commands and usage tips.

@ViewFromTheBox ViewFromTheBox moved this from Backlog to In Review in ArtisanPack UI Overview Apr 16, 2026
@ViewFromTheBox
ViewFromTheBox marked this pull request as ready for review April 16, 2026 19:34

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/Registries/BlockTypeRegistry.php (1)

45-60: ⚠️ Potential issue | 🟠 Major

Keep the stored block name canonical.

Line 60 lets $definition['name'] override the already-validated $normalized key. That means a caller can register one block name and persist a different one, so get()/all() can return metadata that no longer matches the registry identity.

Suggested fix
-		$this->blocks[ $normalized ] = array_merge( ['name' => $normalized], $definition );
+		if ( isset( $definition['name'] ) && $definition['name'] !== $normalized ) {
+			throw new InvalidArgumentException(
+				sprintf(
+					'Block type definition name "%s" must match the registered name "%s".',
+					(string) $definition['name'],
+					$normalized
+				)
+			);
+		}
+
+		$definition['name']       = $normalized;
+		$this->blocks[ $normalized ] = $definition;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Registries/BlockTypeRegistry.php` around lines 45 - 60, The register()
method currently allows $definition['name'] to override the validated
$normalized key because array_merge(['name' => $normalized], $definition) lets
later keys win; change the merge so the canonical $normalized wins (e.g. merge
$definition first then ['name' => $normalized], or explicitly set
$this->blocks[$normalized]['name'] = $normalized) to ensure the stored block
name always matches the registry key in BlockTypeRegistry::register and cannot
be overridden by $definition.
resources/js/visual-editor/editor/inserter/__tests__/inserterRegistry.test.ts (1)

184-195: ⚠️ Potential issue | 🟡 Minor

Assert the fallback state for non-ok responses too.

This test only checks that fetchImpl ran, so it would still pass if a non-ok response emptied the inserter registry. Since the failure-path contract here is “keep core blocks available,” please assert the post-load contents as well.

Suggested assertion update
     await loadInserterBlocks({ apiBase: 'https://example.test/api', fetchImpl });

     expect(fetchImpl).toHaveBeenCalledOnce();
+    const names = getInserterBlocks().map((block) => block.name);
+    expect(names).toContain(PARAGRAPH_BLOCK_NAME);
+    expect(names).toContain(HEADING_BLOCK_NAME);
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@resources/js/visual-editor/editor/inserter/__tests__/inserterRegistry.test.ts`
around lines 184 - 195, After calling registerCoreBlocks() and invoking
loadInserterBlocks(...) with a non-ok fetchImpl, assert that the inserter still
contains the core fallback blocks instead of being emptied; specifically, after
loadInserterBlocks completes, verify the registry includes known core block IDs
(e.g., 'core/paragraph' or other core IDs your test suite registers) or that the
inserter block list length remains > 0. Use the same test setup
(registerCoreBlocks and loadInserterBlocks) and add an assertion that inspects
the inserter registry's post-load contents to confirm core blocks are preserved.
🧹 Nitpick comments (9)
resources/js/visual-editor/editor/inserter/builtinInserterBlocks.ts (1)

2-8: Consider delegating to registerCoreBlocks() in registerBuiltinInserterBlocks() to avoid implicit dependencies.

The function is documented as a no-op but called by loadInserterBlocks() without verifying that blocks are registered. While the production bootstrap (main.tsx) correctly calls registerCoreBlocks() before loadInserterBlocks(), the implicit ordering dependency is fragile. Since registerBlockType() is idempotent (duplicate registrations are silently ignored), delegating to registerCoreBlocks() would make the contract explicit and safer for future callers:

Suggested change
+import { registerCoreBlocks } from '../blocks';
+
 export function registerBuiltinInserterBlocks(): void {
-    // Intentionally empty — registration happens in registerCoreBlocks()
+    // Backward-compatible bridge: ensure core blocks are registered.
+    registerCoreBlocks();
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@resources/js/visual-editor/editor/inserter/builtinInserterBlocks.ts` around
lines 2 - 8, The current no-op registerBuiltinInserterBlocks() relies on an
implicit call to registerCoreBlocks() elsewhere; modify
registerBuiltinInserterBlocks() to explicitly call registerCoreBlocks() (which
is idempotent because registerBlockType() ignores duplicates) so callers like
loadInserterBlocks() no longer depend on external ordering. Locate the
registerBuiltinInserterBlocks() function and add a call to registerCoreBlocks()
at the start (or before any early returns), keeping behavior unchanged for
existing callers while making the registration contract explicit and safe.
resources/js/visual-editor/editor/blocks/code/block.json (1)

14-21: Avoid duplicating the language allow-list.

This enum now has to stay in sync with the runtime list behind normalizeLanguage(). If one side changes first, the editor and the published block metadata will disagree. Prefer deriving the runtime options from parsed metadata, or generating this manifest from the shared list.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@resources/js/visual-editor/editor/blocks/code/block.json` around lines 14 -
21, The enum in block.json duplicates the runtime language allow-list and can
drift from the single source used by normalizeLanguage(); fix by removing the
hard-coded enum and instead generate or derive the block's language options from
the shared runtime list used by normalizeLanguage() (or import that shared list
when building the manifest), ensuring the editor metadata is produced from the
same source of truth so normalizeLanguage() and the block.json options always
match.
resources/js/visual-editor/editor/blocks/paragraph/index.ts (1)

10-15: Keep factory defaults aligned with block.json defaults.

block.json says content.default is "", but the factory seeds "<p></p>". That creates a second attribute shape for the same block depending on whether it was instantiated from manifest defaults or from this factory. If the wrapper is only needed by the editor, build it in ParagraphEdit; otherwise derive the factory default from metadata so the schema stays authoritative. The same drift exists in the quote/preformatted rich-text factories.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@resources/js/visual-editor/editor/blocks/paragraph/index.ts` around lines 10
- 15, The factory for the paragraph block is seeding attributes.content with
"<p></p>" which drifts from the authoritative metadata/block.json default
(content.default == ""), causing two attribute shapes; update the factory in
registerBlockType (the factory that returns the object for PARAGRAPH_BLOCK_NAME)
to derive its default from the metadata (e.g.,
metadata.attributes.content.default or fall back to "") instead of hardcoding
"<p></p>", and if the paragraph wrapper is only an editor concern move that
wrapper construction into ParagraphEdit so the serialized attribute matches
block.json; apply the same pattern to the quote/preformatted rich-text
factories.
resources/js/visual-editor/editor/blocks/quote/index.ts (1)

3-18: Make block.json the canonical source for QUOTE_BLOCK_NAME.

This module registers from metadata, but the factory and re-export still depend on QUOTE_BLOCK_NAME from ./edit. If the manifest name changes and that constant does not, inserted blocks will no longer match the registered definition. Mirroring the paragraph block’s metadata.name pattern would remove that drift risk here and in the other migrated blocks using the same pattern.

Suggested cleanup
-import QuoteEdit, { QUOTE_BLOCK_NAME } from './edit';
+import QuoteEdit from './edit';

 const metadata = rawMetadata as unknown as BlockJsonMetadata;
+export const QUOTE_BLOCK_NAME = metadata.name;

 export function registerQuoteBlock(): void {
     registerBlockType(metadata, {
         edit: QuoteEdit,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@resources/js/visual-editor/editor/blocks/quote/index.ts` around lines 3 - 18,
The code imports QUOTE_BLOCK_NAME from './edit' but uses metadata (block.json)
as the canonical source; to avoid drift, stop relying on the constant from edit
and instead use metadata.name everywhere: in registerQuoteBlock() set the
factory name to metadata.name (replace QUOTE_BLOCK_NAME), remove the import of
QUOTE_BLOCK_NAME from './edit', and re-export the canonical name from this
module by exporting metadata.name (e.g., export const QUOTE_BLOCK_NAME =
metadata.name) so the factory, registration and exported symbol all come from
block.json.
src/VisualEditorServiceProvider.php (1)

75-89: Avoid a second hardcoded core-block list.

This duplicates resources/js/visual-editor/editor/blocks/index.ts::registerCoreBlocks(), so adding or removing a core block can desync the PHP registry from the client registry and REST payload. Scanning */block.json under $blocksDir keeps discovery manifest-driven.

♻️ Possible simplification
-        $coreBlocks = [
-            'paragraph',
-            'heading',
-            'list',
-            'quote',
-            'code',
-            'preformatted',
-        ];
-
-        foreach ( $coreBlocks as $block ) {
-            $blockJsonPath = $blocksDir . '/' . $block . '/block.json';
-
-            if ( file_exists( $blockJsonPath ) ) {
-                $editor->registerBlock( $blockJsonPath );
-            }
-        }
+        $blockJsonPaths = glob( $blocksDir . '/*/block.json' ) ?: [];
+        sort( $blockJsonPaths );
+
+        foreach ( $blockJsonPaths as $blockJsonPath ) {
+            $editor->registerBlock( $blockJsonPath );
+        }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/VisualEditorServiceProvider.php` around lines 75 - 89, The PHP provider
currently hardcodes $coreBlocks and iterates to call $editor->registerBlock(...)
which duplicates
resources/js/visual-editor/editor/blocks/index.ts::registerCoreBlocks() and
risks desync; change VisualEditorServiceProvider.php to drop the $coreBlocks
array and instead scan $blocksDir for subdirectories containing block.json
(e.g., glob or DirectoryIterator for "$blocksDir/*/block.json") and call
$editor->registerBlock($path) for each discovered block.json so the PHP registry
becomes manifest-driven and stays in sync with the client registry.
resources/js/visual-editor/editor/registry/blockSupports.ts (1)

17-70: Centralize the support-path resolver.

Both exported helpers walk the same dot-path separately. Extracting one internal resolver will keep their null/object handling aligned as support types grow.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@resources/js/visual-editor/editor/registry/blockSupports.ts` around lines 17
- 70, Both blockSupports and getBlockSupport duplicate the dot-path resolution
logic; extract a single internal resolver (e.g., resolveBlockSupportPath) that
accepts (blockName: string, feature: string) and returns the raw value or
undefined, preserving the same null/object/type checks currently used in both
functions. Replace the loop logic in blockSupports and getBlockSupport to call
this resolver: blockSupports should use the resolver result to perform the
true/array/object/length checks currently implemented, and getBlockSupport
should return the resolver result directly. Keep existing function names
(blockSupports, getBlockSupport) and ensure the resolver handles
definition?.supports, part iteration, and returns undefined when the path is
invalid.
resources/js/visual-editor/editor/blocks/code/edit.tsx (1)

95-109: Minor accessibility redundancy.

The <label> wraps a <span className="ve-sr-only"> (line 96) and the <select> also has aria-label="Code language" (line 99). Both convey the same information. Consider removing one — either the aria-label or the ve-sr-only span — to simplify the DOM while maintaining accessibility.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@resources/js/visual-editor/editor/blocks/code/edit.tsx` around lines 95 -
109, The label element (className "ve-block-code__language") currently includes
a visually-hidden span ("ve-sr-only") and the select also has an aria-label
("Code language"), which is redundant; choose one method and remove the other to
simplify DOM while preserving accessibility—either remove the aria-label on the
select (keeping the visible label/ve-sr-only span) or remove the ve-sr-only span
and keep aria-label on the select; update the JSX around the <label> / <select>
in edit.tsx accordingly (look for CODE_LANGUAGES map, the select with
data-testid "ve-code-language", and the surrounding label with className
"ve-block-code__language").
resources/js/visual-editor/editor/registry/types.ts (1)

186-186: Consider making apiVersion required.

In WordPress Gutenberg, apiVersion is effectively required for modern blocks (version 2+). While keeping it optional maintains backward compatibility, you may want to enforce it for new registrations to ensure blocks declare their API level explicitly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@resources/js/visual-editor/editor/registry/types.ts` at line 186, The
apiVersion property is currently optional (apiVersion?: number) which allows
registrations without declaring an API level; change it to a required property
by removing the optional marker (make it apiVersion: number) in the type
declaration so new blocks must specify their API version, and then update any
call sites, tests, or defaults that relied on the optionality (e.g., ensure
block registration objects and any factory functions/constructors that create
these types supply apiVersion).
resources/js/visual-editor/editor/blocks/list/edit.tsx (1)

156-169: Potential update loop between ensureListContent and toggleList.

When ordered changes, two things happen:

  1. content memo (line 61-67) re-runs ensureListContent, wrapping with <ol> or <ul>
  2. This useEffect calls editor.commands.toggleList if the doc doesn't match

If toggleList emits an update (via onUpdate), it may persist HTML with the toggled wrapper, then ensureListContent runs again on the next render. The context snippet shows useBlockTiptap uses emitUpdate: false when setting content, which should prevent infinite loops, but ensure the sequencing is deterministic to avoid brief content mismatches.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@resources/js/visual-editor/editor/blocks/list/edit.tsx` around lines 156 -
169, There’s a potential update loop between the content memo/ensureListContent
and the useEffect that calls editor.commands.toggleList when `ordered` changes;
fix this by adding a one-shot guard so we don’t react to the edit we just
applied: when ensureListContent (the memo that calls useBlockTiptap/setContent
with emitUpdate: false) runs, set a ref flag like `skipNextToggleRef.current =
true`, and in the useEffect before calling
`editor.commands.toggleList(expectedType, 'listItem')` check and clear that flag
(if true, skip the toggle). This uses the existing symbols ensureListContent,
useBlockTiptap/setContent (emitUpdate: false), ordered, and
editor.commands.toggleList to make sequencing deterministic and avoid the brief
mismatch/update loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@package.json`:
- Around line 6-10: Update the package export map and TypeScript build settings
so consumers get declaration files: enable declaration output in tsconfig.json
by setting "declaration": true and "declarationDir": "./dist/lib" (or equivalent
in your build config), and update the package.json "exports" entry for "." to
add a "types" condition pointing to the emitted declaration (e.g.
"./dist/lib/visual-editor.d.ts") alongside the existing "import":
"./dist/lib/visual-editor.js" so tools resolve types for the public API exported
from resources/js/visual-editor/editor/index.ts.

In `@resources/js/visual-editor/editor/blocks/list/block.json`:
- Around line 14-17: The block.json currently defines the "content" property as
a plain string which loses rich-text metadata; update the "content" field in
resources/js/visual-editor/editor/blocks/list/block.json to use the same
rich-text metadata schema used by other RichText-based blocks (include keys such
as "type": "rich-text" or equivalent, and nested metadata like "source",
"selector", and "role") so consumers can read source/selector/role from the
manifest and serializers/parsers can handle list content consistently; locate
the "content" property in that file and replace the simple string schema with
the rich-text metadata shape used elsewhere in your blocks.

In `@resources/js/visual-editor/editor/blocks/list/edit.tsx`:
- Around line 84-123: The onEnter handler currently inserts a new paragraph
block but leaves the empty <li> in the editor document; fix by deleting the
empty list item node from the ProseMirror document before creating the new
block: using the activeEditor (from getBlockEditor(clientId)) derive the node
positions for the list item (e.g. via $from.before(listItemDepth) and
$from.end(listItemDepth) or similar), build a transaction
(activeEditor.state.tr) that deletes that node range (or uses node deletion API)
and dispatch it with activeEditor.dispatch(...) to remove the empty list item,
then continue with the existing logic that creates newBlock and calls
state.replaceBlocks/state.select; reference symbols: onEnter, getBlockEditor,
findListItemDepth, $from.node(listItemDepth), activeEditor.state.tr,
activeEditor.dispatch, state.replaceBlocks, state.select.

In `@resources/js/visual-editor/editor/blocks/paragraph/edit.tsx`:
- Around line 101-107: The onEnter handler currently returns true whenever
slashActiveRef.current is set, which swallows Enter even when
filteredRef.current has no matches; update onEnter (and its early-return logic)
so that when slashActiveRef.current is true but filteredRef.current.length === 0
the function returns false (allowing the default paragraph split), and only
calls selectSlashBlockAtIndex(slashIndexRef.current) and returns true when
filteredRef.current.length > 0.

In `@resources/js/visual-editor/editor/blocks/shared/blockSplitMerge.ts`:
- Line 7: The current import of PARAGRAPH_BLOCK_NAME from the side-effectful
module "../paragraph" introduces init-order/circular coupling; replace that
import in blockSplitMerge.ts with a side-effect-free export that only exposes
the name constant (e.g., a dedicated metadata/constant module that exports
PARAGRAPH_BLOCK_NAME or ParagraphBlock.metadata.name). If such a module doesn’t
exist, create a small side-effect-free file that exports the paragraph block
name constant and update the import in blockSplitMerge.ts to use that constant
instead of importing the full registration entrypoint.

In `@resources/js/visual-editor/editor/components/RichTextToolbar.tsx`:
- Around line 59-61: The toolbar must not infer semantics from attribute names;
replace the heuristics in RichTextToolbar.tsx (where getBlock, blockDefinition,
hasLevelAttribute, hasOrderedAttribute are used and similarly in the 70-81
range) with explicit capability checks on block metadata (e.g.,
blockDefinition.editor?.supportsHeadingLevel and
blockDefinition.editor?.supportsListOrdering or a unified
blockDefinition.editor?.toolbarCapabilities object). Update the toolbar to check
those capability flags before exposing heading-level or ordered-list controls,
and update block registration/docs so third-party blocks can opt into these
capabilities instead of relying on attribute name collisions.

In `@resources/js/visual-editor/editor/inserter/inserterRegistry.ts`:
- Around line 29-31: registerBlockFactory currently only mutates blockFactories
without triggering a rebuild, so cachedInserterBlocks (used by
rebuildInserterSnapshot) can remain stale; likewise clearFactories can leave
stale entries visible. Update registerBlockFactory and clearFactories to call
rebuildInserterSnapshot() (or otherwise invalidate cachedInserterBlocks) after
mutating blockFactories so the inserter snapshot is rebuilt immediately;
reference the registerBlockFactory and clearFactories functions and the
rebuildInserterSnapshot/cachedInserterBlocks cache when making the change.
- Around line 108-119: registerInserterBlock currently stores server-only blocks
in supplementalInserterBlocks but those entries get filtered out later (in the
getInserterBlocks/rebuildInserterSnapshot flow) when a factory check expects a
client-side definition; update the logic so supplementalInserterBlocks entries
are included in the inserter snapshot when there is no client-side factory:
locate registerInserterBlock, supplementalInserterBlocks, getBlock,
getInserterBlocks and rebuildInserterSnapshot and change the filter/merge step
to union supplementalInserterBlocks into the final inserter list for names where
getBlock(name) is falsy or no factory exists, ensuring server-only blocks are
returned by getInserterBlocks while preserving existing behavior when a
client-side factory does exist.

In `@resources/js/visual-editor/editor/primitives/RichText.tsx`:
- Line 101: The placeholder prop is being discarded by mapping to _placeholder;
update RichText.tsx so the incoming placeholder prop is forwarded to the
underlying editor/field instead of being ignored: remove the throwaway renaming
(placeholder: _placeholder) and pass the actual placeholder value down to the
editor instance or placeholder-aware component (search for the placeholder prop
usage and the mapping line in RichText.tsx), and ensure any defaulting logic
uses the real placeholder variable so consumers see the empty-state hint.
- Around line 34-38: The code returns docContentSpec using the raw tagName which
is brittle for custom extensions; update the branch that handles extraExtensions
(the block using extraExtensions[0]) to set docContentSpec to the extension's
node name (e.g., extraExtensions[0].name or the extension's declared node name)
instead of tagName so Document.extend({ content: docContentSpec }) receives the
correct schema node name; adjust the returned object in RichText.tsx where node
and docContentSpec are constructed.

In `@src/VisualEditor.php`:
- Around line 63-69: The current validation only checks that $metadata['name']
is a string but allows empty or whitespace-only names; update the guard in
VisualEditor.php to trim $metadata['name'] and require the trimmed value to be
non-empty before calling $this->registry->register($metadata['name'],
$metadata). If the trimmed name is empty, throw the same
InvalidArgumentException (or update the message) indicating the block.json is
missing a valid non-empty "name" field so invalid manifests aren't registered
under blank keys.
- Around line 80-82: The registerBlockType function can let callers supply a
conflicting 'name' inside $definition that overrides the normalized key; update
registerBlockType to enforce consistency by either validating that if
$definition['name'] is set it equals the $name (or $normalized) and throw or
normalize it, or ensure the registry call uses the left-precedence array union
(['name' => $normalized] + $definition) instead of array_merge so the provided
key cannot override the lookup key; reference the registerBlockType method and
the registry->register call (and mirror the registerBlock validation behavior)
when making the change.

---

Outside diff comments:
In
`@resources/js/visual-editor/editor/inserter/__tests__/inserterRegistry.test.ts`:
- Around line 184-195: After calling registerCoreBlocks() and invoking
loadInserterBlocks(...) with a non-ok fetchImpl, assert that the inserter still
contains the core fallback blocks instead of being emptied; specifically, after
loadInserterBlocks completes, verify the registry includes known core block IDs
(e.g., 'core/paragraph' or other core IDs your test suite registers) or that the
inserter block list length remains > 0. Use the same test setup
(registerCoreBlocks and loadInserterBlocks) and add an assertion that inspects
the inserter registry's post-load contents to confirm core blocks are preserved.

In `@src/Registries/BlockTypeRegistry.php`:
- Around line 45-60: The register() method currently allows $definition['name']
to override the validated $normalized key because array_merge(['name' =>
$normalized], $definition) lets later keys win; change the merge so the
canonical $normalized wins (e.g. merge $definition first then ['name' =>
$normalized], or explicitly set $this->blocks[$normalized]['name'] =
$normalized) to ensure the stored block name always matches the registry key in
BlockTypeRegistry::register and cannot be overridden by $definition.

---

Nitpick comments:
In `@resources/js/visual-editor/editor/blocks/code/block.json`:
- Around line 14-21: The enum in block.json duplicates the runtime language
allow-list and can drift from the single source used by normalizeLanguage(); fix
by removing the hard-coded enum and instead generate or derive the block's
language options from the shared runtime list used by normalizeLanguage() (or
import that shared list when building the manifest), ensuring the editor
metadata is produced from the same source of truth so normalizeLanguage() and
the block.json options always match.

In `@resources/js/visual-editor/editor/blocks/code/edit.tsx`:
- Around line 95-109: The label element (className "ve-block-code__language")
currently includes a visually-hidden span ("ve-sr-only") and the select also has
an aria-label ("Code language"), which is redundant; choose one method and
remove the other to simplify DOM while preserving accessibility—either remove
the aria-label on the select (keeping the visible label/ve-sr-only span) or
remove the ve-sr-only span and keep aria-label on the select; update the JSX
around the <label> / <select> in edit.tsx accordingly (look for CODE_LANGUAGES
map, the select with data-testid "ve-code-language", and the surrounding label
with className "ve-block-code__language").

In `@resources/js/visual-editor/editor/blocks/list/edit.tsx`:
- Around line 156-169: There’s a potential update loop between the content
memo/ensureListContent and the useEffect that calls editor.commands.toggleList
when `ordered` changes; fix this by adding a one-shot guard so we don’t react to
the edit we just applied: when ensureListContent (the memo that calls
useBlockTiptap/setContent with emitUpdate: false) runs, set a ref flag like
`skipNextToggleRef.current = true`, and in the useEffect before calling
`editor.commands.toggleList(expectedType, 'listItem')` check and clear that flag
(if true, skip the toggle). This uses the existing symbols ensureListContent,
useBlockTiptap/setContent (emitUpdate: false), ordered, and
editor.commands.toggleList to make sequencing deterministic and avoid the brief
mismatch/update loop.

In `@resources/js/visual-editor/editor/blocks/paragraph/index.ts`:
- Around line 10-15: The factory for the paragraph block is seeding
attributes.content with "<p></p>" which drifts from the authoritative
metadata/block.json default (content.default == ""), causing two attribute
shapes; update the factory in registerBlockType (the factory that returns the
object for PARAGRAPH_BLOCK_NAME) to derive its default from the metadata (e.g.,
metadata.attributes.content.default or fall back to "") instead of hardcoding
"<p></p>", and if the paragraph wrapper is only an editor concern move that
wrapper construction into ParagraphEdit so the serialized attribute matches
block.json; apply the same pattern to the quote/preformatted rich-text
factories.

In `@resources/js/visual-editor/editor/blocks/quote/index.ts`:
- Around line 3-18: The code imports QUOTE_BLOCK_NAME from './edit' but uses
metadata (block.json) as the canonical source; to avoid drift, stop relying on
the constant from edit and instead use metadata.name everywhere: in
registerQuoteBlock() set the factory name to metadata.name (replace
QUOTE_BLOCK_NAME), remove the import of QUOTE_BLOCK_NAME from './edit', and
re-export the canonical name from this module by exporting metadata.name (e.g.,
export const QUOTE_BLOCK_NAME = metadata.name) so the factory, registration and
exported symbol all come from block.json.

In `@resources/js/visual-editor/editor/inserter/builtinInserterBlocks.ts`:
- Around line 2-8: The current no-op registerBuiltinInserterBlocks() relies on
an implicit call to registerCoreBlocks() elsewhere; modify
registerBuiltinInserterBlocks() to explicitly call registerCoreBlocks() (which
is idempotent because registerBlockType() ignores duplicates) so callers like
loadInserterBlocks() no longer depend on external ordering. Locate the
registerBuiltinInserterBlocks() function and add a call to registerCoreBlocks()
at the start (or before any early returns), keeping behavior unchanged for
existing callers while making the registration contract explicit and safe.

In `@resources/js/visual-editor/editor/registry/blockSupports.ts`:
- Around line 17-70: Both blockSupports and getBlockSupport duplicate the
dot-path resolution logic; extract a single internal resolver (e.g.,
resolveBlockSupportPath) that accepts (blockName: string, feature: string) and
returns the raw value or undefined, preserving the same null/object/type checks
currently used in both functions. Replace the loop logic in blockSupports and
getBlockSupport to call this resolver: blockSupports should use the resolver
result to perform the true/array/object/length checks currently implemented, and
getBlockSupport should return the resolver result directly. Keep existing
function names (blockSupports, getBlockSupport) and ensure the resolver handles
definition?.supports, part iteration, and returns undefined when the path is
invalid.

In `@resources/js/visual-editor/editor/registry/types.ts`:
- Line 186: The apiVersion property is currently optional (apiVersion?: number)
which allows registrations without declaring an API level; change it to a
required property by removing the optional marker (make it apiVersion: number)
in the type declaration so new blocks must specify their API version, and then
update any call sites, tests, or defaults that relied on the optionality (e.g.,
ensure block registration objects and any factory functions/constructors that
create these types supply apiVersion).

In `@src/VisualEditorServiceProvider.php`:
- Around line 75-89: The PHP provider currently hardcodes $coreBlocks and
iterates to call $editor->registerBlock(...) which duplicates
resources/js/visual-editor/editor/blocks/index.ts::registerCoreBlocks() and
risks desync; change VisualEditorServiceProvider.php to drop the $coreBlocks
array and instead scan $blocksDir for subdirectories containing block.json
(e.g., glob or DirectoryIterator for "$blocksDir/*/block.json") and call
$editor->registerBlock($path) for each discovered block.json so the PHP registry
becomes manifest-driven and stays in sync with the client registry.
🪄 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: d08956e3-50c4-470b-9add-1f257f0e5dfa

📥 Commits

Reviewing files that changed from the base of the PR and between b2d83d9 and f2e81b1.

📒 Files selected for processing (49)
  • package.json
  • resources/js/visual-editor/editor/blocks/__tests__/code.test.tsx
  • resources/js/visual-editor/editor/blocks/__tests__/list.test.tsx
  • resources/js/visual-editor/editor/blocks/__tests__/preformatted.test.tsx
  • resources/js/visual-editor/editor/blocks/__tests__/quote.test.tsx
  • resources/js/visual-editor/editor/blocks/__tests__/slashCommand.test.tsx
  • resources/js/visual-editor/editor/blocks/code/block.json
  • resources/js/visual-editor/editor/blocks/code/edit.tsx
  • resources/js/visual-editor/editor/blocks/code/index.ts
  • resources/js/visual-editor/editor/blocks/heading/block.json
  • resources/js/visual-editor/editor/blocks/heading/edit.tsx
  • resources/js/visual-editor/editor/blocks/heading/index.ts
  • resources/js/visual-editor/editor/blocks/index.ts
  • resources/js/visual-editor/editor/blocks/list/block.json
  • resources/js/visual-editor/editor/blocks/list/edit.tsx
  • resources/js/visual-editor/editor/blocks/list/index.ts
  • resources/js/visual-editor/editor/blocks/paragraph/block.json
  • resources/js/visual-editor/editor/blocks/paragraph/edit.tsx
  • resources/js/visual-editor/editor/blocks/paragraph/index.ts
  • resources/js/visual-editor/editor/blocks/preformatted/block.json
  • resources/js/visual-editor/editor/blocks/preformatted/edit.tsx
  • resources/js/visual-editor/editor/blocks/preformatted/index.ts
  • resources/js/visual-editor/editor/blocks/quote/block.json
  • resources/js/visual-editor/editor/blocks/quote/edit.tsx
  • resources/js/visual-editor/editor/blocks/quote/index.ts
  • resources/js/visual-editor/editor/blocks/shared/blockSplitMerge.ts
  • resources/js/visual-editor/editor/components/RichTextToolbar.tsx
  • resources/js/visual-editor/editor/components/__tests__/InserterPanel.test.tsx
  • resources/js/visual-editor/editor/index.ts
  • resources/js/visual-editor/editor/inserter/__tests__/insertBlockActions.test.ts
  • resources/js/visual-editor/editor/inserter/__tests__/inserterRegistry.test.ts
  • resources/js/visual-editor/editor/inserter/builtinInserterBlocks.ts
  • resources/js/visual-editor/editor/inserter/index.ts
  • resources/js/visual-editor/editor/inserter/inserterRegistry.ts
  • resources/js/visual-editor/editor/primitives/RichText.tsx
  • resources/js/visual-editor/editor/primitives/index.ts
  • resources/js/visual-editor/editor/registry/blockRegistry.ts
  • resources/js/visual-editor/editor/registry/blockSupports.ts
  • resources/js/visual-editor/editor/registry/index.ts
  • resources/js/visual-editor/editor/registry/types.ts
  • src/Facades/VisualEditor.php
  • src/Http/Controllers/VisualEditorBlocksController.php
  • src/Registries/BlockTypeRegistry.php
  • src/VisualEditor.php
  • src/VisualEditorServiceProvider.php
  • tests/Feature/VisualEditor/BlocksControllerTest.php
  • tests/Feature/VisualEditor/PostsControllerTest.php
  • tests/Unit/VisualEditor/BlockTypeRegistryTest.php
  • vite.config.ts

Comment thread package.json
Comment on lines +6 to +10
"exports": {
".": {
"import": "./dist/lib/visual-editor.js"
}
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check package export/type metadata.
rg -n '"exports"|"types"|"typings"' package.json

# Check tsconfig declaration settings.
fd -i 'tsconfig*.json' | xargs -r -I{} sh -c 'echo "--- {} ---"; rg -n "\"declaration\"|\"emitDeclarationOnly\"|\"declarationDir\"|\"types\"" "{}"'

# Check whether declaration artifacts currently exist.
fd -i '*.d.ts'

Repository: ArtisanPack-UI/visual-editor

Length of output: 484


🏁 Script executed:

#!/bin/bash
# Correct fd syntax for finding .d.ts files
echo "=== Looking for .d.ts files ==="
fd -e d.ts

echo ""
echo "=== Full package.json content ==="
cat -n package.json

echo ""
echo "=== Full tsconfig.json content ==="
cat -n tsconfig.json

echo ""
echo "=== Looking for build:lib script definition ==="
rg -A 2 '"build:lib"' package.json

Repository: ArtisanPack-UI/visual-editor

Length of output: 3526


🏁 Script executed:

fd -e 'vite.config.ts' -e 'vite.config.js' | xargs -r cat -n

Repository: ArtisanPack-UI/visual-editor

Length of output: 54


🏁 Script executed:

# Check if vite.config exists anywhere and look at dist directory structure
echo "=== Checking for vite config ==="
find . -name "vite.config*" -o -name "vite.config.*"

echo ""
echo "=== Checking dist directory structure ==="
ls -la dist/ 2>/dev/null || echo "dist directory not found or empty"

echo ""
echo "=== Checking what index.ts exports ==="
rg -n "export" resources/js/visual-editor/editor/index.ts | head -20

Repository: ArtisanPack-UI/visual-editor

Length of output: 769


🏁 Script executed:

cat -n vite.config.ts

Repository: ArtisanPack-UI/visual-editor

Length of output: 3240


Add TypeScript declaration exports to the library configuration.

The export map exposes only the ES module entrypoint without a types field. Since the public API exports many TypeScript types (from resources/js/visual-editor/editor/index.ts), external TypeScript consumers will lack type information.

Enable declaration emission in tsconfig.json ("declaration": true and "declarationDir": "./dist/lib") and add a types condition to the exports in package.json:

"exports": {
    ".": {
        "types": "./dist/lib/visual-editor.d.ts",
        "import": "./dist/lib/visual-editor.js"
    }
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` around lines 6 - 10, Update the package export map and
TypeScript build settings so consumers get declaration files: enable declaration
output in tsconfig.json by setting "declaration": true and "declarationDir":
"./dist/lib" (or equivalent in your build config), and update the package.json
"exports" entry for "." to add a "types" condition pointing to the emitted
declaration (e.g. "./dist/lib/visual-editor.d.ts") alongside the existing
"import": "./dist/lib/visual-editor.js" so tools resolve types for the public
API exported from resources/js/visual-editor/editor/index.ts.

Comment thread resources/js/visual-editor/editor/blocks/list/block.json
Comment thread resources/js/visual-editor/editor/blocks/list/edit.tsx
Comment thread resources/js/visual-editor/editor/blocks/paragraph/edit.tsx
Comment thread resources/js/visual-editor/editor/blocks/shared/blockSplitMerge.ts Outdated
Comment on lines +108 to +119
/**
* @deprecated Use `registerBlockType(metadata, settings)` from the registry
* module instead. This function remains for backward compatibility during
* migration and for REST-API-loaded blocks that don't have a JS edit component.
*/
export function registerInserterBlock(block: InserterBlock): void {
// For blocks registered via REST API without a client-side definition,
// we don't have a full BlockDefinition. Keep these in a supplemental map.
if (!getBlock(block.name)) {
supplementalInserterBlocks.set(block.name, block);
}
rebuildInserterSnapshot();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

The REST/API fallback currently drops the server-only blocks it claims to support.

Lines 109-111 say registerInserterBlock() remains for REST/API-loaded blocks without a JS edit component, but Line 135 filters every supplemental entry unless a factory already exists. As implemented, those blocks never make it into getInserterBlocks() for the exact migration case described here.

Also applies to: 127-136

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@resources/js/visual-editor/editor/inserter/inserterRegistry.ts` around lines
108 - 119, registerInserterBlock currently stores server-only blocks in
supplementalInserterBlocks but those entries get filtered out later (in the
getInserterBlocks/rebuildInserterSnapshot flow) when a factory check expects a
client-side definition; update the logic so supplementalInserterBlocks entries
are included in the inserter snapshot when there is no client-side factory:
locate registerInserterBlock, supplementalInserterBlocks, getBlock,
getInserterBlocks and rebuildInserterSnapshot and change the filter/merge step
to union supplementalInserterBlocks into the final inserter list for names where
getBlock(name) is falsy or no factory exists, ensuring server-only blocks are
returned by getInserterBlocks while preserving existing behavior when a
client-side factory does exist.

Comment thread resources/js/visual-editor/editor/primitives/RichText.tsx
Comment thread resources/js/visual-editor/editor/primitives/RichText.tsx Outdated
Comment thread src/VisualEditor.php Outdated
Comment thread src/VisualEditor.php Outdated
- package.json: add types condition to exports map for declaration files
- list/block.json: use rich-text type with source/selector/role for content
- list/edit.tsx: delete empty list item from ProseMirror doc before exiting
  to new paragraph block on Enter
- paragraph/edit.tsx: don't swallow Enter when slash popover has no matches
- blockSplitMerge.ts: import paragraph name from block.json to avoid
  circular import through side-effectful paragraph/index.ts
- RichTextToolbar.tsx: check attribute schema type (number/boolean) not
  just existence for heading level and list type switchers
- inserterRegistry.ts: call rebuildInserterSnapshot() in registerBlockFactory
  and clearFactories to keep cached list in sync
- RichText.tsx: forward placeholder as data-placeholder; use extension name
  for docContentSpec in custom extension fallback path
- VisualEditor.php: trim and validate non-empty name in registerBlock();
  use array union in registerBlockType() so $name param takes precedence

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
resources/js/visual-editor/editor/inserter/inserterRegistry.ts (1)

134-138: ⚠️ Potential issue | 🟠 Major

Server-only blocks are still filtered out despite the stated use case.

The deprecation note on lines 111-113 states this function "remains for REST-API-loaded blocks that don't have a JS edit component." However, line 137 requires supplemental blocks to have a factory (either in blockFactories or via getBlock(b.name)?.factory). Server-only blocks without a JS edit component won't have a factory, so they'll never appear in the inserter.

If server-only blocks should be insertable (perhaps rendering via a generic server-block component or placeholder), remove the factory requirement for supplemental blocks. If they genuinely cannot be inserted without a factory, update the deprecation note to clarify the actual supported use case.

Suggested fix (if server-only blocks should be insertable)
     // Build combined list — registry blocks take precedence
     const registryNames = new Set(cachedInserterBlocks.map((b) => b.name));
     const supplemental = Array.from(supplementalInserterBlocks.values()).filter(
-        (b) => !registryNames.has(b.name) && (blockFactories.has(b.name) || getBlock(b.name)?.factory)
+        (b) => !registryNames.has(b.name)
     );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@resources/js/visual-editor/editor/inserter/inserterRegistry.ts` around lines
134 - 138, The supplemental inserter filtering erroneously excludes server-only
blocks because the supplemental array building requires a factory (check uses
blockFactories.has(b.name) || getBlock(b.name)?.factory), so remove the factory
requirement when assembling supplementalInserterBlocks so server-only blocks can
be included; specifically update the supplemental variable/filter logic that
constructs supplemental from supplementalInserterBlocks.values() to only filter
out names already in cachedInserterBlocks (cachedInserterBlocks.map(b => b.name)
/ registryNames) and any other truly incompatible criteria, or alternatively
adjust the deprecation comment to state that supplemental blocks must have a JS
factory if you decide not to include server-only blocks (reference symbols:
cachedInserterBlocks, supplementalInserterBlocks, blockFactories, getBlock, and
the supplemental variable).
resources/js/visual-editor/editor/primitives/RichText.tsx (1)

185-190: ⚠️ Potential issue | 🟠 Major

placeholder still isn't wired into actual Tiptap placeholder behavior.

Setting data-placeholder on EditorContent only annotates the wrapper. With the current useBlockTiptap extension list, callers can pass placeholder and still never get an empty-state hint rendered inside the editor. Please wire this through the editor extensions/plugin layer instead of only the wrapper attribute.

In Tiptap v3, does setting `data-placeholder` on `<EditorContent>` render placeholder text by itself, or is `@tiptap/extension-placeholder` (or equivalent ProseMirror plugin/node-level handling) required?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@resources/js/visual-editor/editor/primitives/RichText.tsx` around lines 185 -
190, The placeholder prop is only set as a data attribute on the EditorContent
wrapper and not wired into Tiptap's placeholder plugin; update the extension
list returned by useBlockTiptap (or where extensions are assembled) to include
`@tiptap/extension-placeholder` when a placeholder string is passed and configure
it with the placeholder text (and sensible options like showOnlyWhenEditable or
showOnlyWhenEditable: false if you want it visible when empty), so EditorContent
receives an editor instance whose extensions include the Placeholder extension
instead of relying solely on data-placeholder on the DOM node; keep the
data-placeholder attribute if you want the wrapper annotated but ensure the
actual placeholder rendering comes from the Placeholder extension.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@resources/js/visual-editor/editor/blocks/paragraph/edit.tsx`:
- Line 12: The effect that attaches the blur listener currently only depends on
clientId so if getBlockEditor(clientId) (populated by registerBlockEditor in
useBlockTiptap) is undefined on first render the handler is never attached;
change the effect to track the actual editor instance (call
getBlockEditor(clientId) and include that editor value in the dependency array
or bail and subscribe when it becomes non-null) and attach the blur listener
only when the returned editor exists, and ensure you remove the listener in
cleanup; reference getBlockEditor(clientId), registerBlockEditor, and
useBlockTiptap to locate the registry interaction and update the effect to
depend on the editor presence rather than only clientId.

---

Duplicate comments:
In `@resources/js/visual-editor/editor/inserter/inserterRegistry.ts`:
- Around line 134-138: The supplemental inserter filtering erroneously excludes
server-only blocks because the supplemental array building requires a factory
(check uses blockFactories.has(b.name) || getBlock(b.name)?.factory), so remove
the factory requirement when assembling supplementalInserterBlocks so
server-only blocks can be included; specifically update the supplemental
variable/filter logic that constructs supplemental from
supplementalInserterBlocks.values() to only filter out names already in
cachedInserterBlocks (cachedInserterBlocks.map(b => b.name) / registryNames) and
any other truly incompatible criteria, or alternatively adjust the deprecation
comment to state that supplemental blocks must have a JS factory if you decide
not to include server-only blocks (reference symbols: cachedInserterBlocks,
supplementalInserterBlocks, blockFactories, getBlock, and the supplemental
variable).

In `@resources/js/visual-editor/editor/primitives/RichText.tsx`:
- Around line 185-190: The placeholder prop is only set as a data attribute on
the EditorContent wrapper and not wired into Tiptap's placeholder plugin; update
the extension list returned by useBlockTiptap (or where extensions are
assembled) to include `@tiptap/extension-placeholder` when a placeholder string is
passed and configure it with the placeholder text (and sensible options like
showOnlyWhenEditable or showOnlyWhenEditable: false if you want it visible when
empty), so EditorContent receives an editor instance whose extensions include
the Placeholder extension instead of relying solely on data-placeholder on the
DOM node; keep the data-placeholder attribute if you want the wrapper annotated
but ensure the actual placeholder rendering comes from the Placeholder
extension.
🪄 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: 47641f37-9900-4331-a2f3-aae3dd5012f9

📥 Commits

Reviewing files that changed from the base of the PR and between f2e81b1 and e4991a9.

📒 Files selected for processing (9)
  • package.json
  • resources/js/visual-editor/editor/blocks/list/block.json
  • resources/js/visual-editor/editor/blocks/list/edit.tsx
  • resources/js/visual-editor/editor/blocks/paragraph/edit.tsx
  • resources/js/visual-editor/editor/blocks/shared/blockSplitMerge.ts
  • resources/js/visual-editor/editor/components/RichTextToolbar.tsx
  • resources/js/visual-editor/editor/inserter/inserterRegistry.ts
  • resources/js/visual-editor/editor/primitives/RichText.tsx
  • src/VisualEditor.php
✅ Files skipped from review due to trivial changes (2)
  • resources/js/visual-editor/editor/components/RichTextToolbar.tsx
  • resources/js/visual-editor/editor/blocks/list/block.json
🚧 Files skipped from review as they are similar to previous changes (4)
  • resources/js/visual-editor/editor/blocks/shared/blockSplitMerge.ts
  • package.json
  • resources/js/visual-editor/editor/blocks/list/edit.tsx
  • src/VisualEditor.php

Comment thread resources/js/visual-editor/editor/blocks/paragraph/edit.tsx Outdated
The blur listener effect only depended on clientId, so if the Tiptap
editor wasn't registered yet on first render the listener was never
attached. Now uses useSyncExternalStore to subscribe to the block editor
registry and re-runs the effect when the editor instance appears.
@ViewFromTheBox
ViewFromTheBox merged commit 54a90f1 into add/phase-two Apr 16, 2026
1 check passed
@github-project-automation github-project-automation Bot moved this from In Review to Done in ArtisanPack UI Overview Apr 16, 2026
@ViewFromTheBox
ViewFromTheBox deleted the enhancement/302-block-json-architecture-registries-primitives branch April 16, 2026 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

Overhaul A: block.json architecture, PHP/JS registries, RichText & InnerBlocks primitives

1 participant