Repository navigation
feat(blocks): fork content cluster as artisanpack/* (I1, #409) - #476
Merged
Merged
Conversation
Phase I1 of the block fork (#409): port the 8 content-cluster blocks — heading, list (with list-item), quote, code, preformatted, pullquote, verse, table — into the artisanpack/* namespace under resources/js/visual-editor/blocks/. Each fork ships block.json, edit.tsx, save.tsx, deprecated.tsx, transforms.ts, inserter-icon.tsx, combined CSS, upstream-state.json, and a tests/ suite covering save + transforms + deprecations + edit. Round-trip block transforms wire each fork bidirectionally with its core/* counterpart so mixed documents convert losslessly during the V1 rollout. Two shared primitives (migrate-text-align, migrate-font- family) move to blocks/_shared/ to support the heading, list, verse, and quote deprecation chains. A blocks.getBlockDefaultClassName filter (_shared/fork-class-name- alias.ts) remaps the I1 fork class names back to their wp-block-{slug} upstream forms so the bundled gutenberg CSS applies. Without it the table cells in particular render at 0×0 because every upstream rule targets .wp-block-table, not .wp-block-artisanpack-table. Renderer parity (Blade/React/Vue) registers the new artisanpack/* names against the existing core/* renderer components — the saved markup is byte-equivalent so one renderer covers both namespaces. Closes #409 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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 (14)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughAdds artisanpack/* forks for core text blocks with manifests, edit/save/transforms/deprecations, CSS, tests; synchronizes renderer parity across Blade/React/Vue; adds class-name aliasing and vendored migrations; records upstream-state/diff reports. ChangesI1 Content Cluster Fork
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
resources/js/visual-editor/blocks/preformatted/__tests__/transforms.test.ts (1)
24-57: ⚡ Quick winAssert transform lookup results before dereferencing.
These tests currently rely on type assertions and can fail with a less-informative runtime error when a mapping is missing. Add explicit
toBeDefined()guards before calling.transform(...).Proposed test hardening
const fromBlock = transforms.from.find( (t: { type: string; blocks?: string[] }) => t.type === 'block' && t.blocks?.includes('core/paragraph') - ) as { + ); + expect(fromBlock).toBeDefined(); + const typedFromBlock = fromBlock as { transform: (a: Record<string, unknown>) => { name: string; attributes: Record<string, unknown>; }; }; - const result = fromBlock.transform({ content: 'x', anchor: 'a' }); + const result = typedFromBlock.transform({ content: 'x', anchor: 'a' }); @@ const fromBlock = transforms.from.find( (t: { type: string; blocks?: string[] }) => t.type === 'block' && t.blocks?.length === 1 && t.blocks[0] === 'core/preformatted' - ) as { + ); + expect(fromBlock).toBeDefined(); + const typedFromBlock = fromBlock as { transform: (a: Record<string, unknown>) => { name: string }; }; - expect(fromBlock.transform({}).name).toBe('artisanpack/preformatted'); + expect(typedFromBlock.transform({}).name).toBe('artisanpack/preformatted'); @@ const toBlock = transforms.to.find( (t: { type: string; blocks?: string[] }) => t.type === 'block' && t.blocks?.[0] === 'core/preformatted' - ) as { + ); + expect(toBlock).toBeDefined(); + const typedToBlock = toBlock as { transform: (a: Record<string, unknown>) => { name: string }; }; - expect(toBlock.transform({}).name).toBe('core/preformatted'); + expect(typedToBlock.transform({}).name).toBe('core/preformatted');🤖 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/blocks/preformatted/__tests__/transforms.test.ts` around lines 24 - 57, The test dereferences .transform on lookups (fromBlock/toBlock) without asserting the lookup succeeded; add explicit guards like expect(fromBlock).toBeDefined() and expect(toBlock).toBeDefined() immediately after each find result to ensure the objects exist before calling fromBlock.transform(...) and toBlock.transform(...), and keep the existing type assertions for the transform call sites (references: transforms.from, transforms.to, fromBlock, toBlock, .transform).resources/js/visual-editor/blocks/heading/__tests__/edit.test.tsx (1)
20-25: ⚡ Quick winAdd coverage for anchor-generation and level-clamping paths.
Current tests force
canGenerateAnchors: false, so the new anchor side-effect/update branch is untested. Please add cases for out-of-rangelevelvalues (e.g., 0/7) and forcanGenerateAnchors: trueto verify anchor updates remain stable.Also applies to: 63-100
🤖 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/blocks/heading/__tests__/edit.test.tsx` around lines 20 - 25, The tests currently hardcode the `@wordpress/data` mock to return useSelect -> { canGenerateAnchors: false }, so add new test cases that change the mock to return canGenerateAnchors: true and assert the anchor-update side effect runs/stabilizes (use the existing __unstableMarkNextChangeAsNotPersistent mock to observe dispatch calls); also add tests that render the heading edit component with out-of-range level props (e.g., level: 0 and level: 7) and assert the component clamps to the valid range and does not crash or produce invalid anchors. Locate and update the mock in resources/js/visual-editor/blocks/heading/__tests__/edit.test.tsx (the useSelect stub) and add tests around the component under test (the Heading edit/render function used in existing tests) to verify both anchor generation when canGenerateAnchors is true and clamping behavior for level values 0 and 7.resources/js/visual-editor/blocks/table/__tests__/transforms.test.ts (1)
47-58: ⚡ Quick winMake transform lookup order-independent.
Using
t.blocks?.[0]is brittle; a harmless reorder can break this test. Match by membership instead.♻️ Suggested change
- const fromBlock = transforms.from.find( - (t: { type: string; blocks?: string[] }) => - t.type === 'block' && t.blocks?.[0] === 'core/table' - ) as { transform: (a: Record<string, unknown>) => { name: string } }; + const fromBlock = transforms.from.find( + (t: { type: string; blocks?: string[] }) => + t.type === 'block' && !!t.blocks?.includes('core/table') + ) as { transform: (a: Record<string, unknown>) => { name: string } }; - const toBlock = transforms.to.find( - (t: { type: string; blocks?: string[] }) => - t.type === 'block' && t.blocks?.[0] === 'core/table' - ) as { transform: (a: Record<string, unknown>) => { name: string } }; + const toBlock = transforms.to.find( + (t: { type: string; blocks?: string[] }) => + t.type === 'block' && !!t.blocks?.includes('core/table') + ) as { transform: (a: Record<string, unknown>) => { name: string } };🤖 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/blocks/table/__tests__/transforms.test.ts` around lines 47 - 58, The test's transform lookup is brittle because it checks t.blocks?.[0] === 'core/table'; change the predicate in the finds for both transforms.from (fromBlock) and transforms.to (toBlock) to check membership (e.g., use t.blocks?.includes('core/table')) so the lookup is order-independent and still matches any transform that targets 'core/table'.
🤖 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 `@resources/js/visual-editor/blocks/code/edit.tsx`:
- Around line 45-50: The handler for __unstableOnSplitAtDoubleLineEnd currently
performs a side-effect by calling insertBlocksAfter and returns void; instead,
change it to return the created block(s) so RichText can handle insertion: call
getDefaultBlockName(), and if it yields a name return [createBlock(defaultName)]
(do not call insertBlocksAfter here); if no defaultName, return undefined/null
as the split result. Ensure you update the function body for
__unstableOnSplitAtDoubleLineEnd accordingly and remove the insertBlocksAfter
call.
In `@resources/js/visual-editor/blocks/code/save.tsx`:
- Around line 31-35: The code risks dereferencing attributes.content when it's
null/undefined before calling toHTMLString; update the expression that renders
content so you first guard content (e.g. attributes.content != null) or use
optional chaining on the content object itself and on the method: replace
attributes.content.toHTMLString?.(...) with
attributes.content?.toHTMLString?.(...) (or wrap in a check like typeof
attributes.content === 'string' ? attributes.content : attributes.content !=
null ? attributes.content.toHTMLString?.({ preserveWhiteSpace: true }) ?? '' :
'') so toHTMLString is only invoked when content exists and is not a string.
In `@resources/js/visual-editor/blocks/code/utils.ts`:
- Around line 14-18: The function escapeProtocolInIsolatedUrls only replaces the
first isolated URL line because the regex lacks the global flag; update the
regex in escapeProtocolInIsolatedUrls to use the global modifier (e.g., add /g)
so content.replace applies to every matching line in the multiline string while
keeping the multiline flag (/m) intact.
In `@resources/js/visual-editor/blocks/list/deprecated.tsx`:
- Line 184: The deprecated block registration sets the support flag
__experimentalOnMerge as the string 'true'; change it to the boolean true
instead. Locate the object literal containing __experimentalOnMerge (in the
deprecated block definition in deprecated.tsx) and replace
"__experimentalOnMerge: 'true'" with "__experimentalOnMerge: true" so the
supports flag is a proper boolean for v3 handling.
In `@resources/js/visual-editor/blocks/list/transforms.ts`:
- Around line 205-215: Transforms that pass innerBlocks straight into
createBlock (the transform arrow returning createBlock(name, attributes,
innerBlocks)) fail to remap child list-item block names, leaving core/list-item
inside artisanpack/list (and vice versa); update the transform to map over
innerBlocks and replace each block.name from 'core/list-item' to
'artisanpack/list-item' (and in the reverse transform replace
'artisanpack/list-item' to 'core/list-item') producing a new innerBlocks array
of { name, attributes, innerBlocks } before calling createBlock(name,
attributes, remappedInnerBlocks), and apply the same remapping logic to the
other similar transform that currently passes innerBlocks through unchanged.
In `@resources/js/visual-editor/blocks/quote/edit.tsx`:
- Around line 6-8: The top-of-file header incorrectly claims the hook
useMigrateOnLoad is preserved even though this file doesn't import or call it;
fix by either (A) actually importing and invoking useMigrateOnLoad from its
module within this file (so the preservation claim is accurate) or (B) update
the header comment to remove the preservation claim and instead state that this
file does not call useMigrateOnLoad and only mirrors the deprecation chain for
migrateToQuoteV2; reference the hook name useMigrateOnLoad and the upstream
symbol migrateToQuoteV2 when updating the comment so the intent is clear.
In `@resources/js/visual-editor/blocks/table/edit.tsx`:
- Around line 194-205: The onChange handlers for initial column/row inputs allow
values < 1 which later causes createTable to throw RangeError; update the
handlers for setInitialColumnCount and setInitialRowCount to parse the incoming
value and clamp it with Math.max(1, parsedNumber) (or equivalent) so only
numbers >= 1 are stored, ensuring createTable always receives valid counts.
In `@resources/js/visual-editor/blocks/table/transforms.ts`:
- Around line 74-97: The transform is dropping the TH scope attribute; update
the column mapping in transforms.ts (the reducer that builds col objects from
col variable inside the (colAcc, col) => { ... } block) to read
col.getAttribute('scope') (or similar) and include a scope property on the
pushed object alongside tag, content, rowspan, colspan, and align so header
semantics are preserved; use normalizeRowColSpan and the existing styledCol
handling unchanged and only add the scope value when present.
In `@resources/js/visual-editor/blocks/verse/verse.css`:
- Line 12: Replace the deprecated CSS value by changing the rule that currently
sets "word-break: break-word;" in
resources/js/visual-editor/blocks/verse/verse.css: set "word-break: normal;" and
add "overflow-wrap: anywhere;" to preserve wrapping behavior and satisfy
stylelint (update the same selector that contains the existing word-break rule).
---
Nitpick comments:
In `@resources/js/visual-editor/blocks/heading/__tests__/edit.test.tsx`:
- Around line 20-25: The tests currently hardcode the `@wordpress/data` mock to
return useSelect -> { canGenerateAnchors: false }, so add new test cases that
change the mock to return canGenerateAnchors: true and assert the anchor-update
side effect runs/stabilizes (use the existing
__unstableMarkNextChangeAsNotPersistent mock to observe dispatch calls); also
add tests that render the heading edit component with out-of-range level props
(e.g., level: 0 and level: 7) and assert the component clamps to the valid range
and does not crash or produce invalid anchors. Locate and update the mock in
resources/js/visual-editor/blocks/heading/__tests__/edit.test.tsx (the useSelect
stub) and add tests around the component under test (the Heading edit/render
function used in existing tests) to verify both anchor generation when
canGenerateAnchors is true and clamping behavior for level values 0 and 7.
In `@resources/js/visual-editor/blocks/preformatted/__tests__/transforms.test.ts`:
- Around line 24-57: The test dereferences .transform on lookups
(fromBlock/toBlock) without asserting the lookup succeeded; add explicit guards
like expect(fromBlock).toBeDefined() and expect(toBlock).toBeDefined()
immediately after each find result to ensure the objects exist before calling
fromBlock.transform(...) and toBlock.transform(...), and keep the existing type
assertions for the transform call sites (references: transforms.from,
transforms.to, fromBlock, toBlock, .transform).
In `@resources/js/visual-editor/blocks/table/__tests__/transforms.test.ts`:
- Around line 47-58: The test's transform lookup is brittle because it checks
t.blocks?.[0] === 'core/table'; change the predicate in the finds for both
transforms.from (fromBlock) and transforms.to (toBlock) to check membership
(e.g., use t.blocks?.includes('core/table')) so the lookup is order-independent
and still matches any transform that targets 'core/table'.
🪄 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: 9e112d1b-9387-49dd-9ac4-a92802279738
📒 Files selected for processing (147)
packages/renderer-parity.jsonpackages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/code.blade.phppackages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/heading.blade.phppackages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/list-item.blade.phppackages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/list.blade.phppackages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/preformatted.blade.phppackages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/pullquote.blade.phppackages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/quote.blade.phppackages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/table.blade.phppackages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/verse.blade.phppackages/visual-editor-renderer-react/src/blocks/registerCoreBlocks.tspackages/visual-editor-renderer-vue/src/blocks/registerCoreBlocks.tsresources/js/visual-editor/blocks/_shared/__tests__/fork-class-name-alias.test.tsresources/js/visual-editor/blocks/_shared/fork-class-name-alias.tsresources/js/visual-editor/blocks/_shared/migrate-font-family.tsresources/js/visual-editor/blocks/_shared/migrate-text-align.tsresources/js/visual-editor/blocks/code/__tests__/deprecated.test.tsxresources/js/visual-editor/blocks/code/__tests__/edit.test.tsxresources/js/visual-editor/blocks/code/__tests__/save.test.tsxresources/js/visual-editor/blocks/code/__tests__/transforms.test.tsresources/js/visual-editor/blocks/code/block.jsonresources/js/visual-editor/blocks/code/code.cssresources/js/visual-editor/blocks/code/deprecated.tsxresources/js/visual-editor/blocks/code/edit.tsxresources/js/visual-editor/blocks/code/index.tsresources/js/visual-editor/blocks/code/inserter-icon.tsxresources/js/visual-editor/blocks/code/save.tsxresources/js/visual-editor/blocks/code/transforms.tsresources/js/visual-editor/blocks/code/upstream-diff-report.jsonresources/js/visual-editor/blocks/code/upstream-state.jsonresources/js/visual-editor/blocks/code/utils.tsresources/js/visual-editor/blocks/heading/__tests__/deprecated.test.tsxresources/js/visual-editor/blocks/heading/__tests__/edit.test.tsxresources/js/visual-editor/blocks/heading/__tests__/save.test.tsxresources/js/visual-editor/blocks/heading/__tests__/transforms.test.tsresources/js/visual-editor/blocks/heading/autogenerate-anchors.tsresources/js/visual-editor/blocks/heading/block.jsonresources/js/visual-editor/blocks/heading/deprecated.tsxresources/js/visual-editor/blocks/heading/edit.tsxresources/js/visual-editor/blocks/heading/heading.cssresources/js/visual-editor/blocks/heading/index.tsresources/js/visual-editor/blocks/heading/inserter-icon.tsxresources/js/visual-editor/blocks/heading/save.tsxresources/js/visual-editor/blocks/heading/transforms.tsresources/js/visual-editor/blocks/heading/upstream-diff-report.jsonresources/js/visual-editor/blocks/heading/upstream-state.jsonresources/js/visual-editor/blocks/list-item/__tests__/deprecated.test.tsxresources/js/visual-editor/blocks/list-item/__tests__/edit.test.tsxresources/js/visual-editor/blocks/list-item/__tests__/save.test.tsxresources/js/visual-editor/blocks/list-item/__tests__/transforms.test.tsresources/js/visual-editor/blocks/list-item/block.jsonresources/js/visual-editor/blocks/list-item/deprecated.tsxresources/js/visual-editor/blocks/list-item/edit.tsxresources/js/visual-editor/blocks/list-item/index.tsresources/js/visual-editor/blocks/list-item/inserter-icon.tsxresources/js/visual-editor/blocks/list-item/list-item.cssresources/js/visual-editor/blocks/list-item/save.tsxresources/js/visual-editor/blocks/list-item/transforms.tsresources/js/visual-editor/blocks/list-item/upstream-diff-report.jsonresources/js/visual-editor/blocks/list-item/upstream-state.jsonresources/js/visual-editor/blocks/list/__tests__/deprecated.test.tsxresources/js/visual-editor/blocks/list/__tests__/edit.test.tsxresources/js/visual-editor/blocks/list/__tests__/save.test.tsxresources/js/visual-editor/blocks/list/__tests__/transforms.test.tsresources/js/visual-editor/blocks/list/block.jsonresources/js/visual-editor/blocks/list/deprecated.tsxresources/js/visual-editor/blocks/list/edit.tsxresources/js/visual-editor/blocks/list/index.tsresources/js/visual-editor/blocks/list/inserter-icon.tsxresources/js/visual-editor/blocks/list/list.cssresources/js/visual-editor/blocks/list/save.tsxresources/js/visual-editor/blocks/list/transforms.tsresources/js/visual-editor/blocks/list/upstream-diff-report.jsonresources/js/visual-editor/blocks/list/upstream-state.jsonresources/js/visual-editor/blocks/preformatted/__tests__/deprecated.test.tsxresources/js/visual-editor/blocks/preformatted/__tests__/edit.test.tsxresources/js/visual-editor/blocks/preformatted/__tests__/save.test.tsxresources/js/visual-editor/blocks/preformatted/__tests__/transforms.test.tsresources/js/visual-editor/blocks/preformatted/block.jsonresources/js/visual-editor/blocks/preformatted/deprecated.tsxresources/js/visual-editor/blocks/preformatted/edit.tsxresources/js/visual-editor/blocks/preformatted/index.tsresources/js/visual-editor/blocks/preformatted/inserter-icon.tsxresources/js/visual-editor/blocks/preformatted/preformatted.cssresources/js/visual-editor/blocks/preformatted/save.tsxresources/js/visual-editor/blocks/preformatted/transforms.tsresources/js/visual-editor/blocks/preformatted/upstream-diff-report.jsonresources/js/visual-editor/blocks/preformatted/upstream-state.jsonresources/js/visual-editor/blocks/pullquote/__tests__/deprecated.test.tsxresources/js/visual-editor/blocks/pullquote/__tests__/edit.test.tsxresources/js/visual-editor/blocks/pullquote/__tests__/save.test.tsxresources/js/visual-editor/blocks/pullquote/__tests__/transforms.test.tsresources/js/visual-editor/blocks/pullquote/block.jsonresources/js/visual-editor/blocks/pullquote/deprecated.tsxresources/js/visual-editor/blocks/pullquote/edit.tsxresources/js/visual-editor/blocks/pullquote/index.tsresources/js/visual-editor/blocks/pullquote/inserter-icon.tsxresources/js/visual-editor/blocks/pullquote/pullquote.cssresources/js/visual-editor/blocks/pullquote/save.tsxresources/js/visual-editor/blocks/pullquote/shared.tsresources/js/visual-editor/blocks/pullquote/transforms.tsresources/js/visual-editor/blocks/pullquote/upstream-diff-report.jsonresources/js/visual-editor/blocks/pullquote/upstream-state.jsonresources/js/visual-editor/blocks/quote/__tests__/deprecated.test.tsxresources/js/visual-editor/blocks/quote/__tests__/edit.test.tsxresources/js/visual-editor/blocks/quote/__tests__/save.test.tsxresources/js/visual-editor/blocks/quote/__tests__/transforms.test.tsresources/js/visual-editor/blocks/quote/block.jsonresources/js/visual-editor/blocks/quote/deprecated.tsxresources/js/visual-editor/blocks/quote/edit.tsxresources/js/visual-editor/blocks/quote/index.tsresources/js/visual-editor/blocks/quote/inserter-icon.tsxresources/js/visual-editor/blocks/quote/quote.cssresources/js/visual-editor/blocks/quote/save.tsxresources/js/visual-editor/blocks/quote/transforms.tsresources/js/visual-editor/blocks/quote/upstream-diff-report.jsonresources/js/visual-editor/blocks/quote/upstream-state.jsonresources/js/visual-editor/blocks/table/__tests__/deprecated.test.tsxresources/js/visual-editor/blocks/table/__tests__/edit.test.tsxresources/js/visual-editor/blocks/table/__tests__/save.test.tsxresources/js/visual-editor/blocks/table/__tests__/transforms.test.tsresources/js/visual-editor/blocks/table/block.jsonresources/js/visual-editor/blocks/table/deprecated.tsxresources/js/visual-editor/blocks/table/edit.tsxresources/js/visual-editor/blocks/table/index.tsresources/js/visual-editor/blocks/table/inserter-icon.tsxresources/js/visual-editor/blocks/table/save.tsxresources/js/visual-editor/blocks/table/table.cssresources/js/visual-editor/blocks/table/transforms.tsresources/js/visual-editor/blocks/table/upstream-diff-report.jsonresources/js/visual-editor/blocks/table/upstream-state.jsonresources/js/visual-editor/blocks/table/utils.tsresources/js/visual-editor/blocks/verse/__tests__/deprecated.test.tsxresources/js/visual-editor/blocks/verse/__tests__/edit.test.tsxresources/js/visual-editor/blocks/verse/__tests__/save.test.tsxresources/js/visual-editor/blocks/verse/__tests__/transforms.test.tsresources/js/visual-editor/blocks/verse/block.jsonresources/js/visual-editor/blocks/verse/deprecated.tsxresources/js/visual-editor/blocks/verse/edit.tsxresources/js/visual-editor/blocks/verse/index.tsresources/js/visual-editor/blocks/verse/inserter-icon.tsxresources/js/visual-editor/blocks/verse/save.tsxresources/js/visual-editor/blocks/verse/transforms.tsresources/js/visual-editor/blocks/verse/upstream-diff-report.jsonresources/js/visual-editor/blocks/verse/upstream-state.jsonresources/js/visual-editor/blocks/verse/verse.cssresources/js/visual-editor/editor/custom-blocks.ts
Comment on lines
+45
to
+50
| __unstableOnSplitAtDoubleLineEnd={() => { | ||
| const defaultName = getDefaultBlockName(); | ||
| if (defaultName) { | ||
| insertBlocksAfter?.(createBlock(defaultName)); | ||
| } | ||
| }} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @wordpress/block-editorRichText (matching Gutenberg around@wordpress/block-editor15.16.0), what is the expected handler contract for__unstableOnSplitAtDoubleLineEnd—should it return created block(s), or is manual insertBlocksAfter side-effect the intended usage?
💡 Result:
In the @wordpress/block-editor RichText component, unstableOnSplitAtDoubleLineEnd is an unstable, internal callback used for handling specific splitting behavior, particularly when detecting a double line ending. The expected contract for this handler is to return the new block(s) created as a result of the split [1][2][3]. The RichText component handles the state orchestration (such as replacing the current block or updating its content) using the returned value from your callback in conjunction with the onReplace prop [1][2]. You should not manually invoke insertBlocksAfter inside this handler; instead, the intended pattern is to return the block object(s) generated by createBlock, which the parent RichText component then processes to perform the block insertion or replacement [3]. This follows the established pattern of other splitting callbacks (like onSplit), where the component expects the developer to return a representation of the new block structure, rather than executing side-effectful block insertion methods directly [1][2][3]. Because this is an unstable API, be aware that it may change, and the standard practice for modern block development is to favor block composition and InnerBlocks where possible rather than relying on unstable RichText splitting handlers [4].
Citations:
- 1: https://github.com/WordPress/gutenberg/blob/4faba613bc30df69618a191ebabb63fa01787599/packages/block-editor/src/components/rich-text/README.md
- 2: https://github.com/WordPress/gutenberg/blob/a84c037a0e62a344005054102544c34d7b970a6b/packages/block-editor/src/components/rich-text/README.md
- 3: https://wordpress.stackexchange.com/questions/363479/implementing-gutenberg-richtext-onsplit-onreplace
- 4: https://wordpress.stackexchange.com/questions/422094/replace-multiple-richtext-components-in-a-single-block-wp-blockeditor-richtext
🌐 Web query:
"unstableOnSplitAtDoubleLineEnd" return type RichText callback should return block(s) array
💡 Result:
The prop unstableOnSplitAtDoubleLineEnd (often found in the context of WordPress Gutenberg's RichText component) is an experimental/internal API used to handle splitting behavior when a user performs a split (such as pressing Enter) at the end of a block with a double line break [1][2]. Regarding its callback requirements: 1. Return Type: The callback typically needs to handle the logic of splitting the block into new entities [1][2]. In the WordPress Gutenberg architecture, when you implement custom splitting logic, you are generally expected to facilitate the creation of a new block or blocks to replace the existing content [1][2]. 2. Block Array: While standard onSplit handlers often take before and after strings to create two blocks, more complex splitting scenarios—or those involving multiple splits—expect the resulting operation to effectively manage the block list [1][2]. If you are manually handling the split, you should aim to return or invoke a function that adds the resulting block(s) into the editor's block array (often via insertBlocksAfter or similar block-editor-provided helpers) [2]. Note: These unstable props are internal WordPress Gutenberg APIs [2]. They are subject to change without notice and should ideally be avoided in favor of stable APIs like onSplit if your requirements allow [2]. If you must use them, be aware that their signatures, including exact parameter order and expected return values, are not part of the public stable documentation and may vary by version [2]. Always ensure you have a fallback or a way to replace the existing block via onReplace when implementing these handlers [1].
Citations:
- 1: https://github.com/WordPress/gutenberg/blob/4faba613bc30df69618a191ebabb63fa01787599/packages/block-editor/src/components/rich-text/README.md
- 2: https://wordpress.stackexchange.com/questions/384130/how-to-resolve-warning-for-unstableonsplit-prop-on-a-dom-element-in-block-edit
Return the split result from __unstableOnSplitAtDoubleLineEnd (don’t manually insertBlocksAfter inside the handler).
The current callback in resources/js/visual-editor/blocks/code/edit.tsx (lines 45-50) performs a side-effect via insertBlocksAfter and returns void, but Gutenberg’s RichText unstable split handlers are expected to return the created block(s) so RichText can orchestrate insertion/replacement. This can break split behavior (and also won’t work when insertBlocksAfter is unavailable).
Proposed fix
- __unstableOnSplitAtDoubleLineEnd={() => {
- const defaultName = getDefaultBlockName();
- if (defaultName) {
- insertBlocksAfter?.(createBlock(defaultName));
- }
- }}
+ __unstableOnSplitAtDoubleLineEnd={() => {
+ const defaultName = getDefaultBlockName();
+ return defaultName ? createBlock(defaultName) : undefined;
+ }}If RichText expects an array of blocks for this callback, return [createBlock(defaultName)] instead.
🤖 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/blocks/code/edit.tsx` around lines 45 - 50, The
handler for __unstableOnSplitAtDoubleLineEnd currently performs a side-effect by
calling insertBlocksAfter and returns void; instead, change it to return the
created block(s) so RichText can handle insertion: call getDefaultBlockName(),
and if it yields a name return [createBlock(defaultName)] (do not call
insertBlocksAfter here); if no defaultName, return undefined/null as the split
result. Ensure you update the function body for __unstableOnSplitAtDoubleLineEnd
accordingly and remove the insertBlocksAfter call.
Comment on lines
+14
to
+18
| function escapeProtocolInIsolatedUrls(content: string): string { | ||
| return content.replace( | ||
| /^(\s*https?:)\/\/([^\s<>"]+\s*)$/m, | ||
| '$1//$2' | ||
| ); |
There was a problem hiding this comment.
Escape all isolated URL lines, not just the first.
The regex currently rewrites only the first isolated URL in multiline content. Add the global flag so every matching line is escaped.
Suggested fix
function escapeProtocolInIsolatedUrls(content: string): string {
return content.replace(
- /^(\s*https?:)\/\/([^\s<>"]+\s*)$/m,
+ /^(\s*https?:)\/\/([^\s<>"]+\s*)$/gm,
'$1&`#47`;&`#47`;$2'
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function escapeProtocolInIsolatedUrls(content: string): string { | |
| return content.replace( | |
| /^(\s*https?:)\/\/([^\s<>"]+\s*)$/m, | |
| '$1//$2' | |
| ); | |
| function escapeProtocolInIsolatedUrls(content: string): string { | |
| return content.replace( | |
| /^(\s*https?:)\/\/([^\s<>"]+\s*)$/gm, | |
| '$1&`#47`;&`#47`;$2' | |
| ); | |
| } |
🤖 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/blocks/code/utils.ts` around lines 14 - 18, The
function escapeProtocolInIsolatedUrls only replaces the first isolated URL line
because the regex lacks the global flag; update the regex in
escapeProtocolInIsolatedUrls to use the global modifier (e.g., add /g) so
content.replace applies to every matching line in the multiline string while
keeping the multiline flag (/m) intact.
| spacing: { margin: true, padding: true }, | ||
| __unstablePasteTextInline: true, | ||
| __experimentalSelector: 'ol,ul', | ||
| __experimentalOnMerge: 'true', |
There was a problem hiding this comment.
Use a boolean for __experimentalOnMerge in v3 supports.
Line 184 sets __experimentalOnMerge to the string 'true' instead of boolean true, which can fail strict handling of support flags.
🔧 Proposed fix
- __experimentalOnMerge: 'true',
+ __experimentalOnMerge: true,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| __experimentalOnMerge: 'true', | |
| __experimentalOnMerge: true, |
🤖 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/blocks/list/deprecated.tsx` at line 184, The
deprecated block registration sets the support flag __experimentalOnMerge as the
string 'true'; change it to the boolean true instead. Locate the object literal
containing __experimentalOnMerge (in the deprecated block definition in
deprecated.tsx) and replace "__experimentalOnMerge: 'true'" with
"__experimentalOnMerge: true" so the supports flag is a proper boolean for v3
handling.
Comment on lines
+74
to
+97
| (colAcc, col) => { | ||
| const rowspan = normalizeRowColSpan( | ||
| col.getAttribute('rowspan') | ||
| ); | ||
| const colspan = normalizeRowColSpan( | ||
| col.getAttribute('colspan') | ||
| ); | ||
| const styledCol = col as HTMLElement; | ||
| const { textAlign } = styledCol.style || {}; | ||
| let align; | ||
| if ( | ||
| textAlign === 'left' || | ||
| textAlign === 'center' || | ||
| textAlign === 'right' | ||
| ) { | ||
| align = textAlign; | ||
| } | ||
| (colAcc as unknown[]).push({ | ||
| tag: col.nodeName.toLowerCase(), | ||
| content: col.innerHTML, | ||
| rowspan, | ||
| colspan, | ||
| align, | ||
| }); |
There was a problem hiding this comment.
Preserve <th scope> when transforming raw table HTML.
scope is allowed by schema but discarded in the raw transform payload, so pasted header semantics are lost.
Suggested fix
(colAcc as unknown[]).push({
tag: col.nodeName.toLowerCase(),
content: col.innerHTML,
+ scope:
+ col.nodeName.toLowerCase() === 'th'
+ ? col.getAttribute('scope') ?? undefined
+ : undefined,
rowspan,
colspan,
align,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| (colAcc, col) => { | |
| const rowspan = normalizeRowColSpan( | |
| col.getAttribute('rowspan') | |
| ); | |
| const colspan = normalizeRowColSpan( | |
| col.getAttribute('colspan') | |
| ); | |
| const styledCol = col as HTMLElement; | |
| const { textAlign } = styledCol.style || {}; | |
| let align; | |
| if ( | |
| textAlign === 'left' || | |
| textAlign === 'center' || | |
| textAlign === 'right' | |
| ) { | |
| align = textAlign; | |
| } | |
| (colAcc as unknown[]).push({ | |
| tag: col.nodeName.toLowerCase(), | |
| content: col.innerHTML, | |
| rowspan, | |
| colspan, | |
| align, | |
| }); | |
| (colAcc, col) => { | |
| const rowspan = normalizeRowColSpan( | |
| col.getAttribute('rowspan') | |
| ); | |
| const colspan = normalizeRowColSpan( | |
| col.getAttribute('colspan') | |
| ); | |
| const styledCol = col as HTMLElement; | |
| const { textAlign } = styledCol.style || {}; | |
| let align; | |
| if ( | |
| textAlign === 'left' || | |
| textAlign === 'center' || | |
| textAlign === 'right' | |
| ) { | |
| align = textAlign; | |
| } | |
| (colAcc as unknown[]).push({ | |
| tag: col.nodeName.toLowerCase(), | |
| content: col.innerHTML, | |
| scope: | |
| col.nodeName.toLowerCase() === 'th' | |
| ? col.getAttribute('scope') ?? undefined | |
| : undefined, | |
| rowspan, | |
| colspan, | |
| align, | |
| }); |
🤖 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/blocks/table/transforms.ts` around lines 74 - 97,
The transform is dropping the TH scope attribute; update the column mapping in
transforms.ts (the reducer that builds col objects from col variable inside the
(colAcc, col) => { ... } block) to read col.getAttribute('scope') (or similar)
and include a scope property on the pushed object alongside tag, content,
rowspan, colspan, and align so header semantics are preserved; use
normalizeRowColSpan and the existing styledCol handling unchanged and only add
the scope value when present.
| overflow: auto; | ||
| white-space: pre-wrap; | ||
| min-width: 1em; | ||
| word-break: break-word; |
There was a problem hiding this comment.
Replace deprecated word-break: break-word to satisfy stylelint and keep wrapping behavior.
Line 12 uses a deprecated value and is currently flagged by stylelint. Prefer overflow-wrap: anywhere (and keep word-break: normal) for equivalent intent without linter failure.
Proposed fix
pre.wp-block-verse {
box-sizing: border-box;
overflow: auto;
white-space: pre-wrap;
min-width: 1em;
- word-break: break-word;
+ overflow-wrap: anywhere;
+ word-break: normal;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| word-break: break-word; | |
| pre.wp-block-verse { | |
| box-sizing: border-box; | |
| overflow: auto; | |
| white-space: pre-wrap; | |
| min-width: 1em; | |
| overflow-wrap: anywhere; | |
| word-break: normal; | |
| } |
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 12-12: Deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
🤖 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/blocks/verse/verse.css` at line 12, Replace the
deprecated CSS value by changing the rule that currently sets "word-break:
break-word;" in resources/js/visual-editor/blocks/verse/verse.css: set
"word-break: normal;" and add "overflow-wrap: anywhere;" to preserve wrapping
behavior and satisfy stylelint (update the same selector that contains the
existing word-break rule).
Real fixes: - code/save.tsx: extract resolveContent() helper that guards attributes.content === null/undefined before reaching for toHTMLString. Legacy serialized attributes can arrive null for empty code blocks. - list/transforms.ts: introduce remapListItemNames() recursive helper and apply it on the bidirectional core/list ↔ artisanpack/list block transforms so nested list-item children are remapped to the matching namespace. Without it, the transform produced artisanpack/list trees containing core/list-item children, which violates the block's allowedBlocks contract. - table/edit.tsx: clamp the column-count + row-count text inputs to >= 1 via Math.max so a zero/negative entry can't reach createTable. - quote/edit.tsx: correct the header comment — useMigrateOnLoad is NOT ported in the fork; the deprecation chain handles legacy `value` → inner-blocks migration via migrateToQuoteV2 instead. Test hardening: - preformatted/__tests__/transforms.test.ts: add toBeDefined() guards before dereferencing the transform lookups. - table/__tests__/transforms.test.ts: switch the from/to lookups from position-based (blocks[0]) to membership (blocks.includes), so a harmless reorder of the transforms array can't false-fail the test. - heading/__tests__/edit.test.tsx: add coverage for level clamping (level=0 → h1, level=7 → h6). - list/__tests__/transforms.test.ts: replace the "preserves innerBlocks" test with two cases that lock in the new remapping (core → artisanpack and artisanpack → core, including nested list-items). Findings skipped because they match upstream verbatim and changing them would break drift parity: - code/edit.tsx __unstableOnSplitAtDoubleLineEnd side-effect. - code/utils.ts escapeProtocolInIsolatedUrls without /g flag. - list/deprecated.tsx v3 supports.__experimentalOnMerge: 'true' (string, upstream uses the same). - table/transforms.ts raw paste transform not collecting `scope` attribute on TH cells. - verse/verse.css word-break: break-word (stylelint deprecation warning, but the value is what upstream ships). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-project-automation
Bot
moved this from In Review
to Done
in ArtisanPack UI Overview
This was referenced May 28, 2026
Merged
12 tasks done
1 task done
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.
Description
Phase I1 of the V1 block fork (plan 13 §2.2.1 + §3): port the 8 content-cluster blocks —
heading,list(+list-item),quote,code,preformatted,pullquote,verse,table— into theartisanpack/*namespace underresources/js/visual-editor/blocks/, following the I0 paragraph-pilot reference layout.Closes: #409
Type of Change
Related Issue
Issue: #409 (umbrella: #331)
Motivation and Context
Plan 13 inverts the V1 bet to "adopt @wordpress/block-library at runtime" by porting each
core/*block in the visual-editor's allow-list into the package underartisanpack/*. I0 shipped the paragraph pilot + the diff/parity tooling. I1 is the content cluster, the first of the parallelizable cluster milestones. Forking inside 1.0.0 (before any host app persistscore/*trees) is permanently migration-free.Changes Made
resources/js/visual-editor/blocks/— each shipsblock.json,edit.tsx,save.tsx,deprecated.tsx,transforms.ts,inserter-icon.tsx, combined CSS,upstream-state.json, and a__tests__/suite covering save + transforms + deprecations + edit.core/*counterpart (and acceptartisanpack/*peers as from-sources where it makes sense — e.g.artisanpack/quoteaccepts bothcore/paragraphandartisanpack/paragraph).blocks/_shared/:migrate-text-align.tsandmigrate-font-family.ts(used by heading/list/quote/verse deprecation chains).blocks.getBlockDefaultClassNamefilter (_shared/fork-class-name-alias.ts) remaps the I1 fork class names back to theirwp-block-{slug}upstream forms so the bundled gutenberg CSS applies. Without it, e.g. table cells render at 0×0 because every upstream rule targets.wp-block-table, not.wp-block-artisanpack-table.artisanpack/*namespace registers against the existingcore/*renderer component (saved markup is byte-equivalent, so one renderer covers both namespaces).packages/renderer-parity.jsonupdated.How Has This Been Tested?
Testing Environment:
jmwd-keystone-cms(the V1 host app) on Chrome/SafariTests Performed:
npm test— full suite, 1000+ tests passingnpm run verify:parity— all 3 renderers register the full setnpm run upstream-diff— all forks in sync with their pinned upstream (9.43.0)Accessibility Tests Run
aria-label="Cell", headings/lists/quotes carry their upstream labels)Details: Each fork keeps the upstream
aria-labelstrings —RichText.isEmptyplaceholders match upstream copy.Tests Added
Test details: 4-file tests per fork (
save,transforms,deprecated,edit) + a_shared/__tests__/fork-class-name-alias.test.tsfor the new filter. Save tests assert byte-equivalent markup shape against upstream; deprecation tests lock the chain entry counts and migration semantics; transforms tests cover the bidirectionalcore/* ↔ artisanpack/*round-trip.Documentation
upstream-state.jsondocuments per-filestatus+knownDivergences)Documentation details: Per the I0 pilot's convention,
docs/block-fork-workflow.mdalready covers the fork shape — no doc rewrites needed for I1. Theupstream-state.jsonfiles are the per-block source of truth for drift triage.Pre-Submission Checklist
Known divergences from upstream
Captured per-block in
upstream-state.json. The notable ones for this cluster:table/edit.tsxships a simpler edit experience than upstream (no row/col add/delete toolbar, no per-cell scope/alignment picker, no header/footer toggles). Deferred to the post-I7 customization phase. Save shape and deprecation chain are byte-equivalent, so existing host content continues to deserialize correctly.list/edit.tsxandlist-item/edit.tsxomit the upstream indent/outdent hooks (useIndentListItem,useOutdentListItem,useEnter,useSpace,useMerge) — they reach into block-editor private dispatch surface. Same deferral.pullquote/deprecated.tsxv2 drops the upstreamselect(blockEditorStore).getSettings().colorsnamed-color lookup in itssave(). Calling a store insidesaveis impure; legacy pullquote blocks persisted with default style + a namedmainColor(nocustomMainColor) may fall through to v1 instead of v2 on first re-edit.blocks.getBlockDefaultClassNamefilter, the bundled gutenberg CSS doesn't apply to forks.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests