77f35c4e9e
Ship-blocking fix (Fable consult): useNodeActions.duplicate() appended the regenerated tree at the end of the parent while leaving the ORIGINAL selected, so duplicating a top-level section landed the copy off-screen at the bottom of the page with no visible change -- shared by both the mobile selection toolbar and the desktop right-click ContextMenu. Now inserts the copy immediately after the source (actions.addNodeTree(tree, parentId, sourceIndex + 1)) and selects it (actions.selectNode(tree.rootNodeId)), falling back to append-at-end if the source's index can't be resolved. Mobile also scrolls the new node into view. Three cheap fast-follows: - canSelectParent on useNodeActions (false when the node's parent is ROOT or missing); MobileSelectionToolbar disables "Select Parent" instead of dead-ending on a page-wide ROOT outline with no toolbar of its own. - Opening the Styles sheet on mobile now scrolls the selected node above the 65dvh sheet; a temporary generous bottom-padding class handles the case where the node is the last thing on the page and there'd otherwise be no room left to scroll it into view. - Canvas gets bottom padding equal to the fixed selection toolbar's height while it's visible, so the last section of a short page isn't stuck permanently underneath it. Desktop duplicate behavior improves (inserts after + selects) via the shared hook; no toHtml changes. Verified live via Playwright at 375px and 1280px (screenshots in craft/scratchpad/mobileB2/). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
222 lines
8.8 KiB
TypeScript
222 lines
8.8 KiB
TypeScript
import { useEditor } from '@craftjs/core';
|
|
import { regenerateTreeIds } from '../utils/craft-tree';
|
|
import { findDeletableTarget } from '../utils/craft-helpers';
|
|
|
|
export interface NodeActions {
|
|
moveUp: () => void;
|
|
moveDown: () => void;
|
|
/** Duplicates `nodeId`, inserting the copy immediately after the source in
|
|
* the parent's children and selecting it. Returns the new node's id (so
|
|
* callers -- e.g. `MobileSelectionToolbar` -- can scroll it into view),
|
|
* or null if the duplicate could not be performed (no-op for ROOT/null,
|
|
* or a caught error). */
|
|
duplicate: () => string | null;
|
|
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;
|
|
/** True if `nodeId` has a real parent to select -- i.e. the parent is
|
|
* neither ROOT nor missing. `selectParent` on a top-level section (whose
|
|
* parent IS 'ROOT') would only select the page-wide ROOT node, which has
|
|
* no on-canvas outline and no toolbar of its own -- a dead end for a
|
|
* mobile user with no way back. False for ROOT/null/no-parent too. */
|
|
canSelectParent: 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;
|
|
let canSelectParent = false;
|
|
if (!isRootOrNull) {
|
|
try {
|
|
const node = query.node(nodeId).get();
|
|
const parentId: string | null | undefined = node?.data?.parent;
|
|
if (parentId) {
|
|
canSelectParent = parentId !== 'ROOT';
|
|
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 = (): string | null => {
|
|
if (!nodeId || nodeId === 'ROOT') return null;
|
|
try {
|
|
const parentId = getParentId();
|
|
if (!parentId) return null;
|
|
|
|
const tree = regenerateTreeIds(query.node(nodeId).toNodeTree());
|
|
|
|
// Insert the copy IMMEDIATELY AFTER the source node, not appended at
|
|
// the end of the parent -- on a top-level section, "append at end"
|
|
// landed the copy at the bottom of the page, off-screen, with the
|
|
// ORIGINAL still selected, so a duplicate looked like nothing had
|
|
// happened at all. Resolve the source's index among its siblings so
|
|
// the copy lands right next to what it was copied from; if that can't
|
|
// be resolved for any reason, fall back to the old append behavior
|
|
// rather than guessing at an index (or throwing).
|
|
let insertIndex: number | undefined;
|
|
try {
|
|
const siblings: string[] = query.node(parentId).get()?.data?.nodes || [];
|
|
const sourceIndex = siblings.indexOf(nodeId);
|
|
if (sourceIndex !== -1) insertIndex = sourceIndex + 1;
|
|
} catch {
|
|
// Leave insertIndex undefined -- addNodeTree appends when omitted.
|
|
}
|
|
|
|
if (insertIndex !== undefined) {
|
|
actions.addNodeTree(tree, parentId, insertIndex);
|
|
} else {
|
|
actions.addNodeTree(tree, parentId);
|
|
}
|
|
|
|
// Select the new copy (not the original) so the toolbar/context menu
|
|
// and any on-canvas outline immediately reflect what was just created.
|
|
actions.selectNode(tree.rootNodeId);
|
|
|
|
return tree.rootNodeId;
|
|
} catch (e) {
|
|
console.error('Duplicate failed:', e);
|
|
return null;
|
|
}
|
|
};
|
|
|
|
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,
|
|
canSelectParent,
|
|
};
|
|
}
|