feat(builder): mobile-B touch editing -- selection toolbar, tap-to-add, swipe-dismiss
Phase B makes the Craft.js editor genuinely usable by touch on top of Phase A's responsive shell, gated entirely behind useIsMobile()/<=768px: - Extract useNodeActions(nodeId) out of ContextMenu.tsx (move/duplicate/ delete/select-parent), shared by the desktop right-click menu (behavior unchanged) and the new mobile MobileSelectionToolbar. - MobileSelectionToolbar: bottom-fixed selection toolbar (Move Up/Down, Duplicate, Select Parent, Edit Styles, two-tap Delete confirm), hidden while a sheet is open. - BlocksPanel: tap-to-add on mobile (insert after selection, close sheet, select + scroll the new node into view); desktop drag/double-click unchanged. - LayersPanel rows >=44px on mobile; HeadCodeModal portaled to document.body (same fix TemplateModal already had); BottomSheet gets swipe-to-dismiss and on-screen-keyboard clearance via a new useVisualViewportInsets hook. Also fixes two pre-existing bugs surfaced only by driving a real Craft.js document with Playwright touch input (masked by tests that mock @craftjs/core): regenerateTreeIds structuredClone'd a live node's whole data object, including the component function reference in data.type, throwing DataCloneError and silently breaking Duplicate/Paste for every node type; and an earlier useNodeActions draft cached canMoveUp/canMoveDown inside a useEditor collector closed over nodeId, which goes stale for one render whenever the selection changes without an unrelated store event. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,232 @@
|
||||
import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import React from 'react';
|
||||
import { createRoot, Root } from 'react-dom/client';
|
||||
import { act } from 'react-dom/test-utils';
|
||||
import type { NodeTree, Node } from '@craftjs/core';
|
||||
import { useNodeActions, type NodeActions } from './useNodeActions';
|
||||
|
||||
/**
|
||||
* Phase B, item 1: `useNodeActions` was extracted out of `ContextMenu.tsx` so
|
||||
* the desktop right-click menu and the mobile selection toolbar
|
||||
* (`MobileSelectionToolbar`) share one implementation. This suite exercises
|
||||
* the hook directly (mocking `@craftjs/core`'s `useEditor`, mirroring the
|
||||
* pattern in `useKeyboardShortcuts.test.tsx`), covering:
|
||||
* - moveUp/moveDown call `actions.move` with the right target index, and
|
||||
* are no-ops at the respective boundary
|
||||
* - canMoveUp/canMoveDown reflect the node's position among its siblings
|
||||
* - duplicate regenerates ids before calling `actions.addNodeTree`
|
||||
* - selectParent calls `actions.selectNode` with the parent id
|
||||
* - deleteNode routes through `findDeletableTarget` (also covering the
|
||||
* "no deletable target" no-op)
|
||||
* - ROOT/null nodeId disables every action safely (no-ops, all
|
||||
* canMoveUp/canMoveDown/canDelete false)
|
||||
*/
|
||||
|
||||
const moveMock = vi.fn();
|
||||
const addNodeTreeMock = vi.fn();
|
||||
const selectNodeMock = vi.fn();
|
||||
const deleteMock = vi.fn();
|
||||
|
||||
interface FakeNodeData {
|
||||
parent: string | null;
|
||||
nodes: string[];
|
||||
}
|
||||
|
||||
let fakeNodes: Record<string, { data: FakeNodeData }> = {};
|
||||
|
||||
const COPIED_TREE: NodeTree = {
|
||||
rootNodeId: 'child-1',
|
||||
nodes: {
|
||||
'child-1': {
|
||||
id: 'child-1',
|
||||
data: { props: {}, type: { resolvedName: 'Container' }, name: 'Container', displayName: 'Container', isCanvas: false, parent: 'parent-1', linkedNodes: {}, nodes: [], hidden: false },
|
||||
info: {},
|
||||
events: { selected: false, dragged: false, hovered: false },
|
||||
dom: null,
|
||||
related: {},
|
||||
rules: {},
|
||||
_hydrationTimestamp: 0,
|
||||
} as unknown as Node,
|
||||
},
|
||||
};
|
||||
|
||||
function makeQuery() {
|
||||
return {
|
||||
node: (id: string) => ({
|
||||
get: () => fakeNodes[id] ?? null,
|
||||
toNodeTree: () => {
|
||||
if (id !== 'child-1') throw new Error(`unexpected toNodeTree() for "${id}"`);
|
||||
return COPIED_TREE;
|
||||
},
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
vi.mock('@craftjs/core', () => ({
|
||||
useEditor: (collector?: (state: any, query: any) => any) => {
|
||||
const state = {
|
||||
nodes: Object.fromEntries(Object.entries(fakeNodes).map(([id, n]) => [id, n])),
|
||||
};
|
||||
const query = makeQuery();
|
||||
const collected = collector ? collector(state, query) : {};
|
||||
return {
|
||||
...collected,
|
||||
actions: {
|
||||
move: moveMock,
|
||||
addNodeTree: addNodeTreeMock,
|
||||
selectNode: selectNodeMock,
|
||||
delete: deleteMock,
|
||||
},
|
||||
query,
|
||||
};
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('../utils/craft-tree', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('../utils/craft-tree')>();
|
||||
return { ...actual, regenerateTreeIds: vi.fn(actual.regenerateTreeIds) };
|
||||
});
|
||||
|
||||
import { regenerateTreeIds } from '../utils/craft-tree';
|
||||
const regenerateTreeIdsMock = vi.mocked(regenerateTreeIds);
|
||||
|
||||
let container: HTMLDivElement;
|
||||
let root: Root;
|
||||
let captured: NodeActions | null = null;
|
||||
|
||||
const Probe: React.FC<{ nodeId: string | null }> = ({ nodeId }) => {
|
||||
captured = useNodeActions(nodeId);
|
||||
return null;
|
||||
};
|
||||
|
||||
function render(nodeId: string | null) {
|
||||
container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
act(() => {
|
||||
root = createRoot(container);
|
||||
root.render(<Probe nodeId={nodeId} />);
|
||||
});
|
||||
}
|
||||
|
||||
function rerender(nodeId: string | null) {
|
||||
act(() => {
|
||||
root.render(<Probe nodeId={nodeId} />);
|
||||
});
|
||||
}
|
||||
|
||||
function unmount() {
|
||||
act(() => {
|
||||
root.unmount();
|
||||
});
|
||||
container.remove();
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
moveMock.mockClear();
|
||||
addNodeTreeMock.mockClear();
|
||||
selectNodeMock.mockClear();
|
||||
deleteMock.mockClear();
|
||||
regenerateTreeIdsMock.mockClear();
|
||||
captured = null;
|
||||
fakeNodes = {
|
||||
ROOT: { data: { parent: null, nodes: ['parent-1'] } },
|
||||
'parent-1': { data: { parent: 'ROOT', nodes: ['child-0', 'child-1', 'child-2'] } },
|
||||
'child-0': { data: { parent: 'parent-1', nodes: [] } },
|
||||
'child-1': { data: { parent: 'parent-1', nodes: [] } },
|
||||
'child-2': { data: { parent: 'parent-1', nodes: [] } },
|
||||
};
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (root) unmount();
|
||||
});
|
||||
|
||||
describe('useNodeActions', () => {
|
||||
test('a middle child can move both up and down', () => {
|
||||
render('child-1');
|
||||
expect(captured!.canMoveUp).toBe(true);
|
||||
expect(captured!.canMoveDown).toBe(true);
|
||||
});
|
||||
|
||||
test('the first child cannot move up, but can move down', () => {
|
||||
render('child-0');
|
||||
expect(captured!.canMoveUp).toBe(false);
|
||||
expect(captured!.canMoveDown).toBe(true);
|
||||
});
|
||||
|
||||
test('the last child can move up, but not down', () => {
|
||||
render('child-2');
|
||||
expect(captured!.canMoveUp).toBe(true);
|
||||
expect(captured!.canMoveDown).toBe(false);
|
||||
});
|
||||
|
||||
test('moveUp calls actions.move with idx - 1, moveDown with idx + 2 (Craft.js index semantics)', () => {
|
||||
render('child-1');
|
||||
act(() => captured!.moveUp());
|
||||
expect(moveMock).toHaveBeenCalledWith('child-1', 'parent-1', 0);
|
||||
|
||||
moveMock.mockClear();
|
||||
act(() => captured!.moveDown());
|
||||
expect(moveMock).toHaveBeenCalledWith('child-1', 'parent-1', 3);
|
||||
});
|
||||
|
||||
test('moveUp/moveDown at a boundary are no-ops', () => {
|
||||
render('child-0');
|
||||
act(() => captured!.moveUp());
|
||||
expect(moveMock).not.toHaveBeenCalled();
|
||||
|
||||
rerender('child-2');
|
||||
act(() => captured!.moveDown());
|
||||
expect(moveMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('duplicate regenerates ids before calling actions.addNodeTree with the parent id', () => {
|
||||
render('child-1');
|
||||
act(() => captured!.duplicate());
|
||||
|
||||
expect(regenerateTreeIdsMock).toHaveBeenCalledTimes(1);
|
||||
expect(addNodeTreeMock).toHaveBeenCalledTimes(1);
|
||||
const [tree, parentId] = addNodeTreeMock.mock.calls[0];
|
||||
expect(parentId).toBe('parent-1');
|
||||
expect(tree.rootNodeId).not.toBe(COPIED_TREE.rootNodeId);
|
||||
});
|
||||
|
||||
test('selectParent calls actions.selectNode with the parent id', () => {
|
||||
render('child-1');
|
||||
act(() => captured!.selectParent());
|
||||
expect(selectNodeMock).toHaveBeenCalledWith('parent-1');
|
||||
});
|
||||
|
||||
test('deleteNode calls actions.delete with the node id when deletable', () => {
|
||||
render('child-1');
|
||||
expect(captured!.canDelete).toBe(true);
|
||||
act(() => captured!.deleteNode());
|
||||
expect(deleteMock).toHaveBeenCalledWith('child-1');
|
||||
});
|
||||
|
||||
test('ROOT nodeId disables every action and is a safe no-op', () => {
|
||||
render('ROOT');
|
||||
expect(captured!.canMoveUp).toBe(false);
|
||||
expect(captured!.canMoveDown).toBe(false);
|
||||
expect(captured!.canDelete).toBe(false);
|
||||
|
||||
act(() => {
|
||||
captured!.moveUp();
|
||||
captured!.moveDown();
|
||||
captured!.duplicate();
|
||||
captured!.selectParent();
|
||||
captured!.deleteNode();
|
||||
});
|
||||
expect(moveMock).not.toHaveBeenCalled();
|
||||
expect(addNodeTreeMock).not.toHaveBeenCalled();
|
||||
expect(selectNodeMock).not.toHaveBeenCalled();
|
||||
expect(deleteMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('null nodeId disables every action and is a safe no-op', () => {
|
||||
render(null);
|
||||
expect(captured!.canMoveUp).toBe(false);
|
||||
expect(captured!.canMoveDown).toBe(false);
|
||||
expect(captured!.canDelete).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,169 @@
|
||||
import { useEditor } from '@craftjs/core';
|
||||
import { regenerateTreeIds } from '../utils/craft-tree';
|
||||
import { findDeletableTarget } from '../utils/craft-helpers';
|
||||
|
||||
export interface NodeActions {
|
||||
moveUp: () => void;
|
||||
moveDown: () => void;
|
||||
duplicate: () => void;
|
||||
deleteNode: () => void;
|
||||
selectParent: () => void;
|
||||
/** True if `nodeId` has an earlier sibling under the same parent (so
|
||||
* `moveUp` would actually move it). False for ROOT/null/no-parent. */
|
||||
canMoveUp: boolean;
|
||||
/** True if `nodeId` has a later sibling under the same parent (so
|
||||
* `moveDown` would actually move it). False for ROOT/null/no-parent. */
|
||||
canMoveDown: boolean;
|
||||
/** True if `findDeletableTarget` resolves to a real, deletable node (either
|
||||
* `nodeId` itself, or an ancestor when `nodeId` is an empty linked-node
|
||||
* slot whose siblings are all also empty -- see `craft-helpers.ts`). */
|
||||
canDelete: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Shared node-action logic (move up/down, duplicate, delete, select parent)
|
||||
* extracted from `ContextMenu.tsx` (Phase B) so both the desktop right-click
|
||||
* menu AND the mobile on-canvas selection toolbar (`MobileSelectionToolbar`)
|
||||
* drive the exact same behavior from one place instead of two independent
|
||||
* copies drifting apart.
|
||||
*
|
||||
* IMPORTANT gotcha this hook works around: `@craftjs/core`'s `useEditor`
|
||||
* collector (`@craftjs/utils`' `useCollector`) only synchronously computes
|
||||
* the collector function ONCE, on this hook's very first mount. After that,
|
||||
* it updates purely in reaction to the underlying store's OWN change
|
||||
* notifications, invoking whatever collector closure is current AT THAT
|
||||
* NOTIFICATION -- so a collector that closes over `nodeId` (an argument that
|
||||
* changes across renders of the SAME mounted hook instance, e.g. every time
|
||||
* `MobileSelectionToolbar` re-renders with a newly-selected node) goes stale
|
||||
* for exactly one render: the cached value from the last store notification
|
||||
* (computed against the PREVIOUS nodeId) is what gets returned, until some
|
||||
* unrelated store event happens to trigger a fresh computation. An earlier
|
||||
* version of this hook computed `canMoveUp`/`canMoveDown`/`canDelete` inside
|
||||
* such a collector and was caught showing the PREVIOUS selection's move
|
||||
* boundaries in the mobile toolbar for one render after tapping a new node
|
||||
* (verified with real Playwright touch taps against a real Craft.js
|
||||
* document -- a plain mocked `useEditor` in a unit test doesn't reproduce
|
||||
* this, since a hand-rolled mock has no reason to replicate the real
|
||||
* library's caching).
|
||||
*
|
||||
* The fix: get `actions`/`query` from a collector-FREE `useEditor()` call
|
||||
* (an always-live reference, same pattern `ContextMenu.tsx` used before this
|
||||
* extraction) and compute `canMoveUp`/`canMoveDown`/`canDelete` as plain
|
||||
* synchronous code during render using that live `query` -- never cached.
|
||||
*
|
||||
* That still leaves one gap: `nodeId` staying the SAME across renders while
|
||||
* its position changes (e.g. tapping "Move Up" repeatedly on the same
|
||||
* still-selected node) needs SOMETHING to trigger a re-render so the boundary
|
||||
* flags below get recomputed. An earlier version of this hook forced that
|
||||
* via a second `useEditor((state) => ({ _: state.nodes }))` subscription --
|
||||
* but subscribing to the WHOLE node map reacts to every `dom` ref
|
||||
* assignment too (Craft's `connectors.connect(ref)` calls `actions.setDOM`
|
||||
* synchronously from a React ref callback during COMMIT, i.e. while some
|
||||
* OTHER component is still mounting), which trips React's "Cannot update a
|
||||
* component while rendering a different component" warning the moment a
|
||||
* freshly-added container with children mounts. Callers that need
|
||||
* `canMoveUp`/`canMoveDown` to refresh after a move they themselves
|
||||
* triggered (`MobileSelectionToolbar`) should instead bump their OWN local
|
||||
* state right after calling `moveUp`/`moveDown` -- a plain, local,
|
||||
* event-handler-triggered re-render, not a store-wide subscription.
|
||||
*/
|
||||
export function useNodeActions(nodeId: string | null | undefined): NodeActions {
|
||||
const { actions, query } = useEditor();
|
||||
|
||||
const isRootOrNull = !nodeId || nodeId === 'ROOT';
|
||||
|
||||
let canMoveUp = false;
|
||||
let canMoveDown = false;
|
||||
if (!isRootOrNull) {
|
||||
try {
|
||||
const node = query.node(nodeId).get();
|
||||
const parentId: string | null | undefined = node?.data?.parent;
|
||||
if (parentId) {
|
||||
const siblings: string[] = query.node(parentId).get()?.data?.nodes || [];
|
||||
const idx = siblings.indexOf(nodeId);
|
||||
canMoveUp = idx > 0;
|
||||
canMoveDown = idx !== -1 && idx < siblings.length - 1;
|
||||
}
|
||||
} catch {
|
||||
// Node no longer exists (e.g. deleted out from under a stale
|
||||
// reference) -- leave both false.
|
||||
}
|
||||
}
|
||||
|
||||
const canDelete = !isRootOrNull && !!findDeletableTarget(query, nodeId);
|
||||
|
||||
const getParentId = (): string | null => {
|
||||
if (!nodeId) return null;
|
||||
try {
|
||||
const node = query.node(nodeId).get();
|
||||
return node?.data?.parent || null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
};
|
||||
|
||||
const duplicate = () => {
|
||||
if (!nodeId || nodeId === 'ROOT') return;
|
||||
try {
|
||||
const parentId = getParentId();
|
||||
if (!parentId) return;
|
||||
|
||||
const tree = regenerateTreeIds(query.node(nodeId).toNodeTree());
|
||||
actions.addNodeTree(tree, parentId);
|
||||
} catch (e) {
|
||||
console.error('Duplicate failed:', e);
|
||||
}
|
||||
};
|
||||
|
||||
const moveUp = () => {
|
||||
if (!nodeId || nodeId === 'ROOT') return;
|
||||
try {
|
||||
const parentId = getParentId();
|
||||
if (!parentId) return;
|
||||
const parent = query.node(parentId).get();
|
||||
const children = parent.data.nodes || [];
|
||||
const idx = children.indexOf(nodeId);
|
||||
if (idx > 0) {
|
||||
actions.move(nodeId, parentId, idx - 1);
|
||||
}
|
||||
} catch (e) {
|
||||
console.error('Move up failed:', e);
|
||||
}
|
||||
};
|
||||
|
||||
const moveDown = () => {
|
||||
if (!nodeId || nodeId === 'ROOT') return;
|
||||
try {
|
||||
const parentId = getParentId();
|
||||
if (!parentId) return;
|
||||
const parent = query.node(parentId).get();
|
||||
const children = parent.data.nodes || [];
|
||||
const idx = children.indexOf(nodeId);
|
||||
if (idx < children.length - 1) {
|
||||
actions.move(nodeId, parentId, idx + 2);
|
||||
}
|
||||
} catch (e) {
|
||||
console.error('Move down failed:', e);
|
||||
}
|
||||
};
|
||||
|
||||
const selectParent = () => {
|
||||
if (!nodeId || nodeId === 'ROOT') return;
|
||||
const parentId = getParentId();
|
||||
if (parentId) {
|
||||
actions.selectNode(parentId);
|
||||
}
|
||||
};
|
||||
|
||||
const deleteNode = () => {
|
||||
const target = findDeletableTarget(query, nodeId);
|
||||
if (!target) return;
|
||||
try {
|
||||
actions.delete(target);
|
||||
} catch (e) {
|
||||
console.error('Delete failed:', e);
|
||||
}
|
||||
};
|
||||
|
||||
return { moveUp, moveDown, duplicate, deleteNode, selectParent, canMoveUp, canMoveDown, canDelete };
|
||||
}
|
||||
@@ -0,0 +1,55 @@
|
||||
import { useEffect, useState } from 'react';
|
||||
|
||||
export interface VisualViewportInsets {
|
||||
/** The visual viewport's current height (shrinks when the on-screen
|
||||
* keyboard opens). Falls back to `window.innerHeight` when
|
||||
* `visualViewport` isn't supported. */
|
||||
height: number;
|
||||
/** Extra inset a `position: fixed`, bottom-anchored element should add to
|
||||
* its own `bottom` offset to stay clear of the on-screen keyboard --
|
||||
* `window.innerHeight` minus the visual viewport's bottom edge (its
|
||||
* height + offsetTop). Zero whenever no keyboard is open, or
|
||||
* `visualViewport` isn't supported (a safe no-op fallback). */
|
||||
keyboardInset: number;
|
||||
}
|
||||
|
||||
const ZERO_INSETS: VisualViewportInsets = { height: 0, keyboardInset: 0 };
|
||||
|
||||
function computeInsets(): VisualViewportInsets {
|
||||
if (typeof window === 'undefined') return ZERO_INSETS;
|
||||
const vv = window.visualViewport;
|
||||
if (!vv) return { height: window.innerHeight, keyboardInset: 0 };
|
||||
const keyboardInset = Math.max(0, window.innerHeight - (vv.height + vv.offsetTop));
|
||||
return { height: vv.height, keyboardInset };
|
||||
}
|
||||
|
||||
/**
|
||||
* Tracks `window.visualViewport`'s height/offset (item 5, Phase B) so
|
||||
* `BottomSheet` can stay clear of the on-screen keyboard. `position: fixed`
|
||||
* elements are positioned against the LAYOUT viewport, which does NOT shrink
|
||||
* when a mobile keyboard opens -- only the visual viewport does -- so a
|
||||
* bottom-anchored sheet's inputs can otherwise end up hidden underneath the
|
||||
* keyboard with no visual indication anything is wrong.
|
||||
*
|
||||
* Guards for browsers without `visualViewport` (older WebViews): falls back
|
||||
* to `{ height: window.innerHeight, keyboardInset: 0 }`, i.e. a no-op, so
|
||||
* the sheet just keeps its existing (keyboard-unaware) sizing there.
|
||||
*/
|
||||
export function useVisualViewportInsets(): VisualViewportInsets {
|
||||
const [insets, setInsets] = useState<VisualViewportInsets>(computeInsets);
|
||||
|
||||
useEffect(() => {
|
||||
if (typeof window === 'undefined' || !window.visualViewport) return;
|
||||
const vv = window.visualViewport;
|
||||
const handleChange = () => setInsets(computeInsets());
|
||||
handleChange();
|
||||
vv.addEventListener('resize', handleChange);
|
||||
vv.addEventListener('scroll', handleChange);
|
||||
return () => {
|
||||
vv.removeEventListener('resize', handleChange);
|
||||
vv.removeEventListener('scroll', handleChange);
|
||||
};
|
||||
}, []);
|
||||
|
||||
return insets;
|
||||
}
|
||||
Reference in New Issue
Block a user