From 2c8425ffb02378cf107c01463912c9837be9c3c0 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Mon, 13 Jul 2026 08:07:51 -0700 Subject: [PATCH] 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) --- craft/src/editor/EditorShell.tsx | 2 + craft/src/hooks/useNodeActions.test.tsx | 232 ++++++++++++++++++ craft/src/hooks/useNodeActions.ts | 169 +++++++++++++ craft/src/hooks/useVisualViewport.ts | 55 +++++ craft/src/panels/context-menu/ContextMenu.tsx | 74 ++---- craft/src/panels/left/BlocksPanel.tsx | 122 +++++++-- craft/src/panels/left/LayersPanel.tsx | 1 + craft/src/panels/mobile/BottomSheet.tsx | 124 +++++++++- .../panels/mobile/MobileSelectionToolbar.tsx | 174 +++++++++++++ craft/src/panels/topbar/HeadCodeModal.tsx | 14 +- craft/src/styles/editor.css | 91 ++++++- craft/src/utils/craft-tree.test.ts | 31 +++ craft/src/utils/craft-tree.ts | 27 +- 13 files changed, 1023 insertions(+), 93 deletions(-) create mode 100644 craft/src/hooks/useNodeActions.test.tsx create mode 100644 craft/src/hooks/useNodeActions.ts create mode 100644 craft/src/hooks/useVisualViewport.ts create mode 100644 craft/src/panels/mobile/MobileSelectionToolbar.tsx diff --git a/craft/src/editor/EditorShell.tsx b/craft/src/editor/EditorShell.tsx index 59b2866..d09b81f 100644 --- a/craft/src/editor/EditorShell.tsx +++ b/craft/src/editor/EditorShell.tsx @@ -4,6 +4,7 @@ import { TopBar } from '../panels/topbar/TopBar'; import { LeftPanel } from '../panels/left/LeftPanel'; import { RightPanel } from '../panels/right/RightPanel'; import { MobilePanelBar } from '../panels/mobile/MobilePanelBar'; +import { MobileSelectionToolbar } from '../panels/mobile/MobileSelectionToolbar'; import { Canvas } from './Canvas'; import { ContextMenu } from '../panels/context-menu/ContextMenu'; import { useContextMenu } from '../hooks/useContextMenu'; @@ -97,6 +98,7 @@ export const EditorShell: React.FC = () => { {!isMobile && } {isMobile && } + {isMobile && } = {}; + +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(); + 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(); + }); +} + +function rerender(nodeId: string | null) { + act(() => { + root.render(); + }); +} + +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); + }); +}); diff --git a/craft/src/hooks/useNodeActions.ts b/craft/src/hooks/useNodeActions.ts new file mode 100644 index 0000000..63b3869 --- /dev/null +++ b/craft/src/hooks/useNodeActions.ts @@ -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 }; +} diff --git a/craft/src/hooks/useVisualViewport.ts b/craft/src/hooks/useVisualViewport.ts new file mode 100644 index 0000000..013660e --- /dev/null +++ b/craft/src/hooks/useVisualViewport.ts @@ -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(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; +} diff --git a/craft/src/panels/context-menu/ContextMenu.tsx b/craft/src/panels/context-menu/ContextMenu.tsx index 7e89445..1d31e0a 100644 --- a/craft/src/panels/context-menu/ContextMenu.tsx +++ b/craft/src/panels/context-menu/ContextMenu.tsx @@ -1,10 +1,10 @@ import React, { useEffect, useCallback, useRef } from 'react'; import { useEditor } from '@craftjs/core'; -import { findDeletableTarget } from '../../utils/craft-helpers'; import { useSitesmithModal } from '../../state/SitesmithContext'; import { buildSitesmithTarget } from '../../utils/sitesmith-target'; import { regenerateTreeIds } from '../../utils/craft-tree'; import { getClipboardNodeId, setClipboardNodeId } from '../../hooks/clipboard'; +import { useNodeActions } from '../../hooks/useNodeActions'; interface ContextMenuProps { visible: boolean; @@ -65,19 +65,16 @@ export const ContextMenu: React.FC = ({ } }, [nodeId, query]); - const duplicate = useCallback(() => { - if (!nodeId || nodeId === 'ROOT') return; - try { - const parentId = getParentId(); - if (!parentId) return; + // Shared move/duplicate/delete/select-parent logic (item 1, Phase B) -- + // extracted into `useNodeActions` so the mobile selection toolbar drives + // the exact same behavior. Wrapped here purely to also `onClose()` the + // menu after each action, same as before the extraction. + const nodeActions = useNodeActions(nodeId); - const tree = regenerateTreeIds(query.node(nodeId).toNodeTree()); - actions.addNodeTree(tree, parentId); - } catch (e) { - console.error('Duplicate failed:', e); - } + const duplicate = useCallback(() => { + nodeActions.duplicate(); onClose(); - }, [nodeId, actions, query, getParentId, onClose]); + }, [nodeActions, onClose]); const copyNode = useCallback(() => { if (!nodeId || nodeId === 'ROOT') return; @@ -118,47 +115,19 @@ export const ContextMenu: React.FC = ({ }, [nodeId, actions, query, onClose]); const moveUp = useCallback(() => { - 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); - } + nodeActions.moveUp(); onClose(); - }, [nodeId, actions, query, getParentId, onClose]); + }, [nodeActions, onClose]); const moveDown = useCallback(() => { - 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); - } + nodeActions.moveDown(); onClose(); - }, [nodeId, actions, query, getParentId, onClose]); + }, [nodeActions, onClose]); const selectParent = useCallback(() => { - if (!nodeId || nodeId === 'ROOT') return; - const parentId = getParentId(); - if (parentId) { - actions.selectNode(parentId); - } + nodeActions.selectParent(); onClose(); - }, [nodeId, actions, getParentId, onClose]); + }, [nodeActions, onClose]); const askSitesmith = useCallback(() => { if (!nodeId || nodeId === 'ROOT') return; @@ -172,18 +141,9 @@ export const ContextMenu: React.FC = ({ }, [nodeId, query, openSitesmith, onClose]); const deleteNode = useCallback(() => { - const target = findDeletableTarget(query, nodeId); - if (!target) { - onClose(); - return; - } - try { - actions.delete(target); - } catch (e) { - console.error('Delete failed:', e); - } + nodeActions.deleteNode(); onClose(); - }, [nodeId, actions, query, onClose]); + }, [nodeActions, onClose]); if (!visible) return null; diff --git a/craft/src/panels/left/BlocksPanel.tsx b/craft/src/panels/left/BlocksPanel.tsx index 4ab34f6..d4351b4 100644 --- a/craft/src/panels/left/BlocksPanel.tsx +++ b/craft/src/panels/left/BlocksPanel.tsx @@ -1,5 +1,7 @@ -import React, { useState } from 'react'; +import React, { useCallback, useState } from 'react'; import { useEditor } from '@craftjs/core'; +import { useIsMobile } from '../../hooks/useIsMobile'; +import { useMobileChrome } from '../../state/MobileChromeContext'; import { Container } from '../../components/layout/Container'; import { Section } from '../../components/layout/Section'; import { ColumnLayout } from '../../components/layout/ColumnLayout'; @@ -146,6 +148,8 @@ const categories: CategoryDef[] = [ export const BlocksPanel: React.FC = () => { const { connectors, actions, query } = useEditor(); + const isMobile = useIsMobile(); + const { closeSheet } = useMobileChrome(); const [collapsed, setCollapsed] = useState>(() => { const initial: Record = {}; categories.forEach((cat, index) => { @@ -158,6 +162,101 @@ export const BlocksPanel: React.FC = () => { setCollapsed((prev) => ({ ...prev, [categoryId]: !prev[categoryId] })); }; + /** Default insertion target when there's no usable selection to anchor + * to: the first real Craft.js canvas in the document (falls back to + * ROOT). Extracted from the pre-existing onDoubleClick handler so + * tap-to-add (mobile, item 3) and double-click (desktop, unchanged) share + * the exact same fallback. */ + const findDefaultCanvasId = useCallback((): string => { + try { + const serialized = JSON.parse(query.serialize()); + const nodeIds = Object.keys(serialized); + for (const id of nodeIds) { + if (serialized[id].isCanvas && id !== 'ROOT') return id; + } + } catch { + // Fall through to ROOT below. + } + return 'ROOT'; + }, [query]); + + /** + * Builds a fresh node tree for `block` and inserts it into the canvas, + * returning the new node's id (or null on failure). Shared by desktop's + * double-click (unchanged behavior/position: always appended to + * `findDefaultCanvasId()`) and mobile's tap-to-add (item 3), which instead + * prefers inserting as a sibling right after the current selection -- + * `insertAfterSelection: true` only from the mobile tap handler below. + */ + const addBlockNode = useCallback((block: BlockDef, insertAfterSelection: boolean): string | null => { + try { + const tree = query.parseReactElement(React.cloneElement(block.component)).toNodeTree(); + + let inserted = false; + if (insertAfterSelection) { + try { + const selectedIds = query.getEvent('selected').all(); + const selectedId = selectedIds.length > 0 ? selectedIds[0] : null; + if (selectedId && selectedId !== 'ROOT') { + const selectedNode = query.node(selectedId).get(); + const parentId = selectedNode?.data?.parent; + if (parentId) { + const parent = query.node(parentId).get(); + const siblings: string[] = parent?.data?.nodes || []; + const idx = siblings.indexOf(selectedId); + if (idx !== -1) { + actions.addNodeTree(tree, parentId, idx + 1); + inserted = true; + } + } + } + } catch { + // Selection isn't a valid sibling target (e.g. lives in a + // linkedNodes slot) -- fall through to the default canvas below. + } + } + + if (!inserted) { + actions.addNodeTree(tree, findDefaultCanvasId()); + } + + return tree.rootNodeId; + } catch (e) { + console.error('Failed to add block:', e); + return null; + } + }, [query, actions, findDefaultCanvasId]); + + /** + * Mobile tap-to-add (item 3): a single tap on a block tile inserts it, + * closes the Blocks sheet, then selects the new node and scrolls it into + * view so the user immediately sees it (and gets the selection toolbar). + * The select/scroll step is deferred two animation frames past the + * `addNodeTree` call -- Craft.js's own state update (and thus the new + * node's real DOM element) lands asynchronously after this handler + * returns, so `query.node(id).get().dom` isn't populated yet if read + * synchronously here. + */ + const handleTapToAdd = useCallback((block: BlockDef) => { + const newId = addBlockNode(block, true); + closeSheet(); + if (!newId) return; + requestAnimationFrame(() => { + try { + actions.selectNode(newId); + } catch { + // Node may have failed to mount -- nothing to select. + } + requestAnimationFrame(() => { + try { + query.node(newId).get()?.dom?.scrollIntoView({ behavior: 'smooth', block: 'center' }); + } catch { + // Best-effort scroll -- not fatal if the node/DOM isn't found. + } + }); + }); + }, [addBlockNode, closeSheet, actions, query]); + return (
{categories.map((category) => { @@ -178,24 +277,9 @@ export const BlocksPanel: React.FC = () => { key={block.id} className="block-item" ref={(ref) => { if (ref) connectors.create(ref, block.component); }} - onDoubleClick={() => { - try { - const serialized = JSON.parse(query.serialize()); - const nodeIds = Object.keys(serialized); - let canvasId = 'ROOT'; - for (const nodeId of nodeIds) { - if (serialized[nodeId].isCanvas && nodeId !== 'ROOT') { - canvasId = nodeId; - break; - } - } - const tree = query.parseReactElement(React.cloneElement(block.component)).toNodeTree(); - actions.addNodeTree(tree, canvasId); - } catch (e) { - console.error('Failed to add block:', e); - } - }} - title={`Drag or double-click to add ${block.label}`} + onDoubleClick={() => addBlockNode(block, false)} + onClick={isMobile ? () => handleTapToAdd(block) : undefined} + title={isMobile ? `Tap to add ${block.label}` : `Drag or double-click to add ${block.label}`} > {block.label} diff --git a/craft/src/panels/left/LayersPanel.tsx b/craft/src/panels/left/LayersPanel.tsx index ad23cfe..b057af6 100644 --- a/craft/src/panels/left/LayersPanel.tsx +++ b/craft/src/panels/left/LayersPanel.tsx @@ -146,6 +146,7 @@ const LayerNode: React.FC = ({ nodeId, depth }) => { {...clickableProps(handleActivate)} onMouseEnter={handleMouseEnter} onMouseLeave={handleMouseLeave} + className="layer-node-row" style={{ position: 'relative', display: 'flex', diff --git a/craft/src/panels/mobile/BottomSheet.tsx b/craft/src/panels/mobile/BottomSheet.tsx index aed4f59..bf191d6 100644 --- a/craft/src/panels/mobile/BottomSheet.tsx +++ b/craft/src/panels/mobile/BottomSheet.tsx @@ -1,4 +1,5 @@ -import React, { useEffect } from 'react'; +import React, { useCallback, useEffect, useRef, useState } from 'react'; +import { useVisualViewportInsets } from '../../hooks/useVisualViewport'; export interface BottomSheetProps { open: boolean; @@ -7,6 +8,10 @@ export interface BottomSheetProps { children: React.ReactNode; } +/** Vertical drag distance (px) past which releasing the handle/header + * commits to closing the sheet, rather than snapping back open. */ +const SWIPE_DISMISS_THRESHOLD_PX = 80; + /** * Mobile-only bottom sheet (Phase A). Slides up from the bottom of the * viewport to host one of the existing side-panel components @@ -18,6 +23,22 @@ export interface BottomSheetProps { * dialog chrome) -- this is anchored to the bottom, sized to ~65dvh, and * has its own drag-handle affordance + internally scrollable body, all of * which Modal doesn't need for its centered use cases. + * + * Phase B adds two more deferred behaviors on top of Phase A's static sheet: + * - **Swipe-to-dismiss**: a touchstart/move/end drag tracked on the handle + * row + header (not the scrollable body, so it never fights a panel's own + * vertical scroll) that follows the finger while dragging down and either + * commits to `onClose()` past `SWIPE_DISMISS_THRESHOLD_PX` or snaps back + * with a short transition otherwise. + * - **On-screen-keyboard clearance**: `useVisualViewportInsets` reports how + * much the visual viewport has shrunk from the bottom (i.e. the + * keyboard's height); that's surfaced as a `--keyboard-inset` CSS custom + * property (see editor.css) which both lifts the whole backdrop/sheet + * clear of the keyboard AND caps the sheet's own max-height so it never + * extends into the space the keyboard occupies. A `focusin` listener also + * scrolls the newly-focused input into view once the keyboard has had a + * moment to animate in, as a second line of defense for a field that's + * still off-screen after the resize alone (e.g. deep in a long panel). */ export const BottomSheet: React.FC = ({ open, onClose, title, children }) => { useEffect(() => { @@ -35,6 +56,78 @@ export const BottomSheet: React.FC = ({ open, onClose, title, }; }, [open, onClose]); + const { keyboardInset } = useVisualViewportInsets(); + + const bodyRef = useRef(null); + useEffect(() => { + if (!open) return; + const bodyEl = bodyRef.current; + if (!bodyEl) return; + const handleFocusIn = (e: FocusEvent) => { + const target = e.target as HTMLElement | null; + if (!target) return; + const tag = target.tagName; + if (tag === 'INPUT' || tag === 'TEXTAREA' || tag === 'SELECT' || target.isContentEditable) { + // Give the on-screen keyboard a moment to finish animating in + // before scrolling -- doing it immediately measures the pre-keyboard + // layout and can undershoot. + window.setTimeout(() => { + target.scrollIntoView({ block: 'center', behavior: 'smooth' }); + }, 300); + } + }; + bodyEl.addEventListener('focusin', handleFocusIn); + return () => bodyEl.removeEventListener('focusin', handleFocusIn); + }, [open]); + + // Swipe-to-dismiss: tracked only on the handle row + header (never the + // scrollable `.mobile-sheet-body`, which needs its own vertical touch + // scrolling to keep working unimpeded). + // + // The authoritative drag distance lives in a REF (`dragOffsetRef`), + // updated synchronously and imperatively on every touchmove -- NOT solely + // in the `dragOffset` state used for rendering the drag transform. Two + // touchmoves can fire back-to-back inside the same synchronous event + // dispatch (e.g. a fast real swipe, or synthetic touch events fired + // without a yield between them), and React batches their `setDragOffset` + // calls into a single pending update that hasn't committed yet by the + // time `touchend` runs in that same tick -- reading the `dragOffset` + // STATE value directly in `handleTouchEnd` would then see a stale + // (pre-drag) value and wrongly decide the swipe didn't clear the + // threshold. The ref sidesteps that entirely (plain synchronous mutation, + // no batching). Relatedly, `onClose()` mutates an ANCESTOR component's + // state (MobileChromeContext's `closeSheet`) -- it must be called as a + // plain statement in the event handler, never from inside a `setState` + // functional updater (that runs during this component's own render phase + // and trips React's "Cannot update a component while rendering a + // different component" warning). + const dragStartYRef = useRef(null); + const dragOffsetRef = useRef(0); + const [dragOffset, setDragOffset] = useState(0); + + const handleTouchStart = useCallback((e: React.TouchEvent) => { + if (e.touches.length !== 1) return; + dragStartYRef.current = e.touches[0].clientY; + }, []); + + const handleTouchMove = useCallback((e: React.TouchEvent) => { + if (dragStartYRef.current === null) return; + const delta = e.touches[0].clientY - dragStartYRef.current; + // Only follow downward drags -- dragging up shouldn't do anything (the + // sheet is already fully open; there's no "expand further" state). + const next = Math.max(0, delta); + dragOffsetRef.current = next; + setDragOffset(next); + }, []); + + const handleTouchEnd = useCallback(() => { + dragStartYRef.current = null; + const finalOffset = dragOffsetRef.current; + dragOffsetRef.current = 0; + setDragOffset(0); + if (finalOffset > SWIPE_DISMISS_THRESHOLD_PX) onClose(); + }, [onClose]); + if (!open) return null; return ( @@ -43,16 +136,37 @@ export const BottomSheet: React.FC = ({ open, onClose, title, onClick={(e) => { if (e.target === e.currentTarget) onClose(); }} + style={{ ['--keyboard-inset' as any]: `${keyboardInset}px` }} > {/* Deliberately NOT aria-modal: the tab bar stays reachable/operable while a sheet is open (tapping another tab swaps sheets directly), so the rest of the screen must not be marked inert to assistive tech the way a true modal dialog would be. */} -
-
+
+
-
+
{title}