Repository navigation
chore: M0 · branch + issue housekeeping for Gutenberg adoption - #326
Conversation
Migrate the visual editor's UI chrome to use @artisanpack-ui/react primitives where compatible, aligning the editor with the broader ArtisanPack UI design system. Declare daisyui and tailwindcss as peer dependencies since the react package's DaisyUI styling is now required at runtime. Swapped to @artisanpack-ui/react: - Button: TopBar, BlockToolbar pin, InspectorSidebar breakpoint buttons, EditorBoot retry, RichTextToolbar format/more-menu/link actions - Input: InserterPanel search, RichTextToolbar link URL - Select: InspectorSidebar state, RichTextToolbar heading level - Tabs: LeftSidebar (Blocks/Patterns/Layers), InspectorSidebar (Settings/Styles) Kept bespoke: - Icon.tsx (FontAwesome adapter; react package's Icon takes raw SVG paths) - Drag handles in BlockWrapper/LayersPanel (dnd-kit ref fragility) - Block tiles, between-block inserter, SlashCommandPopover (layout-specific) - Popover and Dropdown (triggered a main-thread freeze with Tiptap; filed upstream; reverted to inline manual patterns) Workarounds: - tabListClassName="tabs-box" — @artisanpack-ui/react Tabs emits DaisyUI 4 variant names which are dead in DaisyUI 5 - Always subpath imports (/form, /layout) — root import pulls in Chart chunk which needs react-apexcharts that's an optional peer Removed 263 lines of orphaned chrome CSS that was replaced by DaisyUI classes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the custom-React editor tree under resources/js/visual-editor/_legacy/ as reference-only code while the Gutenberg adoption (umbrella #309) plays out across M1–M15. No runtime behavior change — path updates only. Changes: - `resources/js/visual-editor/editor/` → `resources/js/visual-editor/_legacy/editor/` Entire custom editor tree (block registry, Zustand store, Tiptap wiring, editor shell, per-block edit.tsx, REST client, tests). - Add `resources/js/visual-editor/_legacy/README.md` explaining scope and pointers to the new strategy doc. - Add `docs/gutenberg-adoption.md` as the canonical V1 strategy document linking umbrella #309 and milestones #310–#325. - Update path references in `vite.config.ts`, `vitest.config.ts`, `tsconfig.json`, `resources/views/editor/mount.blade.php`, `src/VisualEditorServiceProvider.php`, `tests/Feature/EditorMountViewTest.php` to point at `_legacy/editor/`. Out of scope (per issue #310): - The cherry-pick targets 77b5da0 and 2bb64d3 are dev-app commits (they live in the Laravel dev app repo, not this package), so no cherry-pick happens here. Noted in the umbrella for M15 docs. Verification: - `npm test`: 215/215 tests pass - `composer test`: 27/27 tests pass - `npx tsc --noEmit`: clean Closes #310 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 (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughRelocates the active editor code to a legacy directory, updates build/test/module aliases and service/provider paths to point at the legacy tree, integrates Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/VisualEditorServiceProvider.php (1)
73-76: Consider extracting the legacy blocks path into a class constantLine 76 introduces another hardcoded editor path. Given planned milestone swaps, a single constant here will reduce churn and typo risk.
♻️ Suggested refactor
class VisualEditorServiceProvider extends ServiceProvider { + private const LEGACY_BLOCKS_DIR = __DIR__ . '/../resources/js/visual-editor/_legacy/editor/blocks'; + @@ - $blocksDir = __DIR__ . '/../resources/js/visual-editor/_legacy/editor/blocks'; + $blocksDir = self::LEGACY_BLOCKS_DIR;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/VisualEditorServiceProvider.php` around lines 73 - 76, Extract the hardcoded legacy blocks path assigned to $blocksDir in VisualEditorServiceProvider into a class constant (e.g., LEGACY_BLOCKS_PATH) on the VisualEditorServiceProvider class and replace the inline string usage with that constant; ensure the constant value equals __DIR__ . '/../resources/js/visual-editor/_legacy/editor/blocks' and update any other references/new uses to read from self::LEGACY_BLOCKS_PATH so future milestone/path swaps only require a single constant change.resources/js/visual-editor/_legacy/editor/components/SlashCommandPopover.tsx (1)
45-54: Consider using<button>or addingrole="menuitem"for better accessibility.The
<a>element without anhrefattribute is a common DaisyUI pattern, but it may trigger accessibility warnings since anchors withouthreflack keyboard focusability by default and don't convey the correct semantic role. Since these items trigger actions rather than navigation, a<button>or addingrole="menuitem"withtabindex="0"would be more semantic.♻️ Optional: Use button for action semantics
- <a className={isSelected ? 'active' : undefined}> + <a + role="menuitem" + tabIndex={0} + className={isSelected ? 'active' : undefined} + >Or alternatively, if DaisyUI styling permits:
- <a className={isSelected ? 'active' : undefined}> + <button type="button" className={isSelected ? 'active' : undefined}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/_legacy/editor/components/SlashCommandPopover.tsx` around lines 45 - 54, The anchor element used to render each command item (in SlashCommandPopover.tsx around the element that renders isSelected, block.title and block.description) is non-semantic for an action; replace the <a> with a <button> (preserving DaisyUI classes and the isSelected styling) so items are keyboard-focusable and have correct action semantics, or if you must keep an anchor add role="menuitem" and tabindex="0" plus appropriate keyboard handlers to emulate button behavior; ensure any onClick/onKeyDown handlers currently on the anchor are moved/kept on the new element and that aria-selected or aria-current is applied consistent with isSelected.resources/js/visual-editor/_legacy/editor/components/RichTextToolbar.tsx (1)
415-422: TheonClick={disabled ? undefined : onClick}pattern is redundant.Since the
<button>element has thedisabledattribute set, the browser will prevent click events automatically. SettingonClicktoundefinedwhen disabled is unnecessary.♻️ Simplify onClick handling
- onClick={disabled ? undefined : onClick} + onClick={onClick}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/_legacy/editor/components/RichTextToolbar.tsx` around lines 415 - 422, The onClick ternary is redundant because the native disabled attribute prevents clicks; simplify the button in RichTextToolbar by removing the conditional and using onClick={onClick} directly (leave onMouseDown, disabled, data-testid and className as-is) so the handler is defined consistently without changing behavior.resources/js/visual-editor/_legacy/editor/components/InspectorSidebar.tsx (1)
13-30: Avoid repeated full-tree scans in the store selector.
findBlockInTree(...)inside the Zustand selector can become a hot path as document size grows. Consider indexing blocks byclientIdin store state (or memoizing an index) and doing O(1) lookup for the selected block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/_legacy/editor/components/InspectorSidebar.tsx` around lines 13 - 30, The selector used in InspectorSidebar currently calls findBlockInTree over state.blocks on every render (via useStore), which will become expensive; instead add an O(1) lookup in the editor store (e.g., maintain a blocksById map updated whenever state.blocks change) or expose a getter like getBlockByClientId(clientId) on useEditorStore, then change the selector in InspectorSidebar to use state.blocksById?.[state.selection.clientId] or call state.getBlockByClientId(state.selection.clientId) rather than invoking findBlockInTree; update store mutation points to keep the index in sync and remove the full-tree scan from the selector.resources/js/visual-editor/_legacy/editor/components/editor.css (1)
441-449: Consider tightening tab role selectors to avoid style bleed.Using broad descendant selectors can affect nested tab widgets if they’re ever rendered inside these roots. Prefer a stricter scope (e.g., direct child or a library-specific class hook) to keep styles isolated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/_legacy/editor/components/editor.css` around lines 441 - 449, The current descendant selectors (.ve-inspector__tabs-root [role="tablist"], .ve-left-sidebar__tabs-root [role="tablist"], and their [role="tab"] descendants) are too broad and may style nested tab widgets; tighten them by scoping to direct children or a dedicated class hook (e.g., target .ve-inspector__tabs-root > [role="tablist"] and .ve-inspector__tabs-root > [role="tablist"] > [role="tab"], or add library-specific class hooks like .ve-tablist / .ve-tab on the elements) so styles apply only to the immediate tablist/tab instances used by these roots.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/gutenberg-adoption.md`:
- Line 39: The fenced architecture code block in docs/gutenberg-adoption.md is
missing a language specifier and triggers markdownlint MD040; update the opening
fence for that ASCII-art block from ``` to ```text (i.e., add the language
identifier "text" to the fenced code block surrounding the box diagram) so the
block is explicitly marked and linting passes.
- Around line 123-125: Update the Markdown link target so it points to the
stated directory: change the href that currently links to
"../resources/js/visual-editor/_legacy/" to
"../resources/js/visual-editor/_legacy/editor/" so the visible path
`resources/js/visual-editor/_legacy/editor/` and the actual link target match.
---
Nitpick comments:
In `@resources/js/visual-editor/_legacy/editor/components/editor.css`:
- Around line 441-449: The current descendant selectors
(.ve-inspector__tabs-root [role="tablist"], .ve-left-sidebar__tabs-root
[role="tablist"], and their [role="tab"] descendants) are too broad and may
style nested tab widgets; tighten them by scoping to direct children or a
dedicated class hook (e.g., target .ve-inspector__tabs-root > [role="tablist"]
and .ve-inspector__tabs-root > [role="tablist"] > [role="tab"], or add
library-specific class hooks like .ve-tablist / .ve-tab on the elements) so
styles apply only to the immediate tablist/tab instances used by these roots.
In `@resources/js/visual-editor/_legacy/editor/components/InspectorSidebar.tsx`:
- Around line 13-30: The selector used in InspectorSidebar currently calls
findBlockInTree over state.blocks on every render (via useStore), which will
become expensive; instead add an O(1) lookup in the editor store (e.g., maintain
a blocksById map updated whenever state.blocks change) or expose a getter like
getBlockByClientId(clientId) on useEditorStore, then change the selector in
InspectorSidebar to use state.blocksById?.[state.selection.clientId] or call
state.getBlockByClientId(state.selection.clientId) rather than invoking
findBlockInTree; update store mutation points to keep the index in sync and
remove the full-tree scan from the selector.
In `@resources/js/visual-editor/_legacy/editor/components/RichTextToolbar.tsx`:
- Around line 415-422: The onClick ternary is redundant because the native
disabled attribute prevents clicks; simplify the button in RichTextToolbar by
removing the conditional and using onClick={onClick} directly (leave
onMouseDown, disabled, data-testid and className as-is) so the handler is
defined consistently without changing behavior.
In
`@resources/js/visual-editor/_legacy/editor/components/SlashCommandPopover.tsx`:
- Around line 45-54: The anchor element used to render each command item (in
SlashCommandPopover.tsx around the element that renders isSelected, block.title
and block.description) is non-semantic for an action; replace the <a> with a
<button> (preserving DaisyUI classes and the isSelected styling) so items are
keyboard-focusable and have correct action semantics, or if you must keep an
anchor add role="menuitem" and tabindex="0" plus appropriate keyboard handlers
to emulate button behavior; ensure any onClick/onKeyDown handlers currently on
the anchor are moved/kept on the new element and that aria-selected or
aria-current is applied consistent with isSelected.
In `@src/VisualEditorServiceProvider.php`:
- Around line 73-76: Extract the hardcoded legacy blocks path assigned to
$blocksDir in VisualEditorServiceProvider into a class constant (e.g.,
LEGACY_BLOCKS_PATH) on the VisualEditorServiceProvider class and replace the
inline string usage with that constant; ensure the constant value equals __DIR__
. '/../resources/js/visual-editor/_legacy/editor/blocks' and update any other
references/new uses to read from self::LEGACY_BLOCKS_PATH so future
milestone/path swaps only require a single constant change.
🪄 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: 515f6303-2475-4dd1-a5f4-39392208697a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (109)
README.mddocs/gutenberg-adoption.mdpackage.jsonresources/js/visual-editor/_legacy/README.mdresources/js/visual-editor/_legacy/editor/blocks/__tests__/code.test.tsxresources/js/visual-editor/_legacy/editor/blocks/__tests__/heading.test.tsxresources/js/visual-editor/_legacy/editor/blocks/__tests__/list.test.tsxresources/js/visual-editor/_legacy/editor/blocks/__tests__/paragraph.test.tsxresources/js/visual-editor/_legacy/editor/blocks/__tests__/preformatted.test.tsxresources/js/visual-editor/_legacy/editor/blocks/__tests__/quote.test.tsxresources/js/visual-editor/_legacy/editor/blocks/__tests__/slashCommand.test.tsxresources/js/visual-editor/_legacy/editor/blocks/code/block.jsonresources/js/visual-editor/_legacy/editor/blocks/code/edit.tsxresources/js/visual-editor/_legacy/editor/blocks/code/index.tsresources/js/visual-editor/_legacy/editor/blocks/heading/block.jsonresources/js/visual-editor/_legacy/editor/blocks/heading/edit.tsxresources/js/visual-editor/_legacy/editor/blocks/heading/index.tsresources/js/visual-editor/_legacy/editor/blocks/index.tsresources/js/visual-editor/_legacy/editor/blocks/list/block.jsonresources/js/visual-editor/_legacy/editor/blocks/list/edit.tsxresources/js/visual-editor/_legacy/editor/blocks/list/index.tsresources/js/visual-editor/_legacy/editor/blocks/paragraph/block.jsonresources/js/visual-editor/_legacy/editor/blocks/paragraph/edit.tsxresources/js/visual-editor/_legacy/editor/blocks/paragraph/index.tsresources/js/visual-editor/_legacy/editor/blocks/preformatted/block.jsonresources/js/visual-editor/_legacy/editor/blocks/preformatted/edit.tsxresources/js/visual-editor/_legacy/editor/blocks/preformatted/index.tsresources/js/visual-editor/_legacy/editor/blocks/quote/block.jsonresources/js/visual-editor/_legacy/editor/blocks/quote/edit.tsxresources/js/visual-editor/_legacy/editor/blocks/quote/index.tsresources/js/visual-editor/_legacy/editor/blocks/shared/blockEditorRegistry.tsresources/js/visual-editor/_legacy/editor/blocks/shared/blockSplitMerge.tsresources/js/visual-editor/_legacy/editor/blocks/shared/index.tsresources/js/visual-editor/_legacy/editor/blocks/shared/splitContent.tsresources/js/visual-editor/_legacy/editor/blocks/shared/useBlockTiptap.tsresources/js/visual-editor/_legacy/editor/components/BlockList.tsxresources/js/visual-editor/_legacy/editor/components/BlockToolbar.tsxresources/js/visual-editor/_legacy/editor/components/BlockWrapper.tsxresources/js/visual-editor/_legacy/editor/components/Canvas.tsxresources/js/visual-editor/_legacy/editor/components/EditorBoot.tsxresources/js/visual-editor/_legacy/editor/components/EditorShell.tsxresources/js/visual-editor/_legacy/editor/components/Icon.tsxresources/js/visual-editor/_legacy/editor/components/InserterPanel.tsxresources/js/visual-editor/_legacy/editor/components/InspectorSidebar.tsxresources/js/visual-editor/_legacy/editor/components/LayersPanel.tsxresources/js/visual-editor/_legacy/editor/components/LeftSidebar.tsxresources/js/visual-editor/_legacy/editor/components/RichTextToolbar.tsxresources/js/visual-editor/_legacy/editor/components/SlashCommandPopover.tsxresources/js/visual-editor/_legacy/editor/components/StatusBar.tsxresources/js/visual-editor/_legacy/editor/components/TopBar.tsxresources/js/visual-editor/_legacy/editor/components/__tests__/BlockList.test.tsxresources/js/visual-editor/_legacy/editor/components/__tests__/BlockListDnd.test.tsxresources/js/visual-editor/_legacy/editor/components/__tests__/BlockListDndDispatch.test.tsxresources/js/visual-editor/_legacy/editor/components/__tests__/BlockWrapper.test.tsxresources/js/visual-editor/_legacy/editor/components/__tests__/EditorBoot.test.tsxresources/js/visual-editor/_legacy/editor/components/__tests__/EditorShell.test.tsxresources/js/visual-editor/_legacy/editor/components/__tests__/InserterPanel.test.tsxresources/js/visual-editor/_legacy/editor/components/__tests__/RichTextToolbar.test.tsxresources/js/visual-editor/_legacy/editor/components/blockIconMap.tsresources/js/visual-editor/_legacy/editor/components/editor.cssresources/js/visual-editor/_legacy/editor/components/index.tsresources/js/visual-editor/_legacy/editor/index.htmlresources/js/visual-editor/_legacy/editor/index.tsresources/js/visual-editor/_legacy/editor/inserter/__tests__/insertBlockActions.test.tsresources/js/visual-editor/_legacy/editor/inserter/__tests__/inserterRegistry.test.tsresources/js/visual-editor/_legacy/editor/inserter/builtinInserterBlocks.tsresources/js/visual-editor/_legacy/editor/inserter/filterInserterBlocks.tsresources/js/visual-editor/_legacy/editor/inserter/index.tsresources/js/visual-editor/_legacy/editor/inserter/insertBlockActions.tsresources/js/visual-editor/_legacy/editor/inserter/inserterRegistry.tsresources/js/visual-editor/_legacy/editor/inserter/loadInserterBlocks.tsresources/js/visual-editor/_legacy/editor/main.tsxresources/js/visual-editor/_legacy/editor/primitives/BlockContext.tsxresources/js/visual-editor/_legacy/editor/primitives/EditorStoreContext.tsxresources/js/visual-editor/_legacy/editor/primitives/ReadOnlyContext.tsxresources/js/visual-editor/_legacy/editor/primitives/RichText.tsxresources/js/visual-editor/_legacy/editor/primitives/__tests__/BlockContext.test.tsxresources/js/visual-editor/_legacy/editor/primitives/__tests__/EditorStoreContext.test.tsxresources/js/visual-editor/_legacy/editor/primitives/__tests__/useBlockPreview.test.tsxresources/js/visual-editor/_legacy/editor/primitives/__tests__/useInnerBlocksProps.test.tsxresources/js/visual-editor/_legacy/editor/primitives/index.tsresources/js/visual-editor/_legacy/editor/primitives/useBlockPreview.tsxresources/js/visual-editor/_legacy/editor/primitives/useInnerBlocksProps.tsxresources/js/visual-editor/_legacy/editor/registry/blockRegistry.tsresources/js/visual-editor/_legacy/editor/registry/blockSupports.tsresources/js/visual-editor/_legacy/editor/registry/index.tsresources/js/visual-editor/_legacy/editor/registry/types.tsresources/js/visual-editor/_legacy/editor/rest/__tests__/postRestClient.test.tsresources/js/visual-editor/_legacy/editor/rest/__tests__/useAutosave.test.tsxresources/js/visual-editor/_legacy/editor/rest/index.tsresources/js/visual-editor/_legacy/editor/rest/postRestClient.tsresources/js/visual-editor/_legacy/editor/rest/useAutosave.tsresources/js/visual-editor/_legacy/editor/store/__tests__/editorStore.test.tsresources/js/visual-editor/_legacy/editor/store/__tests__/editorStoreHistory.test.tsresources/js/visual-editor/_legacy/editor/store/__tests__/editorStoreSelectors.test.tsxresources/js/visual-editor/_legacy/editor/store/editorStore.tsresources/js/visual-editor/_legacy/editor/store/ids.tsresources/js/visual-editor/_legacy/editor/store/index.tsresources/js/visual-editor/_legacy/editor/store/treeUtils.tsresources/js/visual-editor/_legacy/editor/store/types.tsresources/js/visual-editor/_legacy/editor/test-setup.tsresources/js/visual-editor/editor/components/InspectorSidebar.tsxresources/js/visual-editor/editor/components/LeftSidebar.tsxresources/views/editor/mount.blade.phpsrc/VisualEditorServiceProvider.phptests/Feature/EditorMountViewTest.phptsconfig.jsonvite.config.tsvitest.config.ts
💤 Files with no reviewable changes (2)
- resources/js/visual-editor/editor/components/LeftSidebar.tsx
- resources/js/visual-editor/editor/components/InspectorSidebar.tsx
- Add `text` language identifier to the architecture fenced code block so markdownlint MD040 passes. - Fix the legacy-tree link href to point at `_legacy/editor/` to match the visible link text. Closes #310 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Description
M0 of the Gutenberg adoption epic (#309): branch + issue housekeeping. Moves the custom-React editor tree under
resources/js/visual-editor/_legacy/as reference-only code while M1–M15 rebuild the editor on top of Gutenberg's npm packages. No runtime behavior change — path updates only.Closes: #310
Type of Change
Related Issue
Issue: #310
Part of the Gutenberg adoption epic: #309.
Motivation and Context
The package pivoted from rebuilding block-editor primitives in React from scratch to adopting Gutenberg's React packages as a library dependency. M0 sets up the workspace so M1–M15 can proceed against a clean baseline:
feature/gutenberg-adoptionbranch offadd/phase-two._legacy/as a reference (not as active code).docs/gutenberg-adoption.mdlinked from the active memory.See
docs/gutenberg-adoption.mdand#309for the full rationale.Changes Made
resources/js/visual-editor/editor/→resources/js/visual-editor/_legacy/editor/(entire custom editor tree: blocks, components, store, primitives, inserter, REST client, tests).resources/js/visual-editor/_legacy/README.mdexplaining what's there and what happens to it.docs/gutenberg-adoption.md— canonical V1 strategy document with architecture diagram, locked decisions, and milestone pointers.vite.config.ts,vitest.config.ts,tsconfig.jsonresources/views/editor/mount.blade.phpsrc/VisualEditorServiceProvider.phptests/Feature/EditorMountViewTest.phpOut of scope: the cherry-pick targets
77b5da0and2bb64d3referenced in the M0 acceptance criteria are commits in the Laravel dev-app repo, not the package repo. They do not apply to this branch.How Has This Been Tested?
Testing Environment:
feature/gutenberg-adoptionoffadd/phase-twoTests Performed:
npm test— 215/215 tests passcomposer test— 27/27 tests passnpx tsc --noEmit— clean, no errors_legacy/tree present; no stale path references viagrep -r resources/js/visual-editor/editor(excluding_legacy)Accessibility Tests Run
N/A — no UI changes. File relocation only.
Details: This is a housekeeping PR. No user-facing UI surfaces are modified.
Tests Added
Test details: No new tests. Existing tests validate the path updates — if any reference was missed, the test suite would fail. All 215 JS and 27 PHP tests pass.
Documentation
Documentation details:
docs/gutenberg-adoption.mdas the canonical V1 strategy doc.resources/js/visual-editor/_legacy/README.mdexplaining the legacy tree.Screenshots
N/A.
Pre-Submission Checklist
Summary by CodeRabbit
New Features
Documentation
Style