diff --git a/craft/src/editor/Canvas.tsx b/craft/src/editor/Canvas.tsx index e50ec05..e4ff5b5 100644 --- a/craft/src/editor/Canvas.tsx +++ b/craft/src/editor/Canvas.tsx @@ -6,6 +6,7 @@ import { DeviceMode } from '../types'; import { DEVICE_WIDTHS } from '../constants/presets'; import { exportBodyHtml } from '../utils/html-export'; import { useIsMobile } from '../hooks/useIsMobile'; +import { useMobileChrome } from '../state/MobileChromeContext'; interface CanvasProps { device: DeviceMode; @@ -135,6 +136,42 @@ export const Canvas: React.FC = ({ device, showGuides }) => { const isEditingRegularPage = !isEditingHeader && !isEditingFooter; + // Fast-follow item 4: while `MobileSelectionToolbar` is up (fixed, ~53px + // tall, just above the tab bar), it covers whatever content was at the + // very bottom of the canvas's own scroll area -- on a short page, the + // last section could sit permanently under the toolbar with no way to + // scroll past it. Mirrors that toolbar's own visibility condition + // (`selectedId && activeSheet === null`) exactly, computed independently + // here since Canvas has no other reason to depend on the toolbar + // component itself. + const isMobile = useIsMobile(); + const { activeSheet } = useMobileChrome(); + const { selectedId } = useEditor((state) => { + const selected = state.events.selected; + const id = selected && selected.size > 0 ? (Array.from(selected)[0] as string) : null; + return { selectedId: id && id !== 'ROOT' ? id : null }; + }); + const mobileToolbarVisible = isMobile && !!selectedId && activeSheet === null; + + // Fast-follow item 3: `MobileSelectionToolbar`'s "Style" tap scrolls the + // selected node up towards the top of the canvas so it stays visible + // above the Styles sheet (~65dvh tall) -- but that scroll is still bound + // by the canvas's own natural scroll range. For a selected node near the + // END of the content (very plausibly the last section on the page -- + // exactly the kind of node someone just added/duplicated and wants to + // style), there may not be enough scrollable distance below it to bring + // its top all the way up to the visible band above the sheet; the browser + // simply clamps at its existing max scrollTop, leaving the node's top + // stuck behind the sheet with nothing anyone can do about it (verified + // live: the last section of a page landed under the sheet even after the + // scroll-into-view ran). Pad the canvas with a full extra viewport's + // worth of scroll room while the sheet is open with a selection, exactly + // as `has-mobile-selection-toolbar` (item 4) already pads it for the + // fixed toolbar -- generous enough that ANY node, including the very + // last one, can always be scrolled with its top reaching the very top of + // the canvas (comfortably within the "top ~30%" target). + const mobileStylesSheetPad = isMobile && !!selectedId && activeSheet === 'styles'; + const frameStyle = isEditingHeader ? { minHeight: '60px', backgroundColor: '#ffffff', padding: '12px 24px', display: 'flex', alignItems: 'center' } : isEditingFooter @@ -144,7 +181,11 @@ export const Canvas: React.FC = ({ device, showGuides }) => { const frameTag = isEditingHeader ? 'header' : isEditingFooter ? 'footer' : 'div'; return ( -
+
{ expect(moveMock).not.toHaveBeenCalled(); }); - test('duplicate regenerates ids before calling actions.addNodeTree with the parent id', () => { + test('duplicate inserts the copy immediately after the source (parentId, sourceIndex + 1) and selects it', () => { render('child-1'); - act(() => captured!.duplicate()); + let returned: string | null | undefined; + act(() => { + returned = captured!.duplicate(); + }); expect(regenerateTreeIdsMock).toHaveBeenCalledTimes(1); expect(addNodeTreeMock).toHaveBeenCalledTimes(1); - const [tree, parentId] = addNodeTreeMock.mock.calls[0]; + const [tree, parentId, index] = addNodeTreeMock.mock.calls[0]; + // child-1 is at index 1 among ['child-0', 'child-1', 'child-2'] -> insert at 2. expect(parentId).toBe('parent-1'); + expect(index).toBe(2); expect(tree.rootNodeId).not.toBe(COPIED_TREE.rootNodeId); + + expect(selectNodeMock).toHaveBeenCalledTimes(1); + expect(selectNodeMock).toHaveBeenCalledWith(tree.rootNodeId); + expect(returned).toBe(tree.rootNodeId); }); test('selectParent calls actions.selectNode with the parent id', () => { @@ -197,6 +213,14 @@ describe('useNodeActions', () => { expect(selectNodeMock).toHaveBeenCalledWith('parent-1'); }); + test('canSelectParent is false at a top-level node (parent is ROOT), true one level deeper', () => { + render('parent-1'); // parent-1's own parent is 'ROOT' + expect(captured!.canSelectParent).toBe(false); + + rerender('child-1'); // child-1's parent is 'parent-1', a real node + expect(captured!.canSelectParent).toBe(true); + }); + test('deleteNode calls actions.delete with the node id when deletable', () => { render('child-1'); expect(captured!.canDelete).toBe(true); @@ -209,6 +233,7 @@ describe('useNodeActions', () => { expect(captured!.canMoveUp).toBe(false); expect(captured!.canMoveDown).toBe(false); expect(captured!.canDelete).toBe(false); + expect(captured!.canSelectParent).toBe(false); act(() => { captured!.moveUp(); @@ -228,5 +253,6 @@ describe('useNodeActions', () => { expect(captured!.canMoveUp).toBe(false); expect(captured!.canMoveDown).toBe(false); expect(captured!.canDelete).toBe(false); + expect(captured!.canSelectParent).toBe(false); }); }); diff --git a/craft/src/hooks/useNodeActions.ts b/craft/src/hooks/useNodeActions.ts index 63b3869..e1cc080 100644 --- a/craft/src/hooks/useNodeActions.ts +++ b/craft/src/hooks/useNodeActions.ts @@ -5,7 +5,12 @@ import { findDeletableTarget } from '../utils/craft-helpers'; export interface NodeActions { moveUp: () => void; moveDown: () => void; - duplicate: () => 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 @@ -18,6 +23,12 @@ export interface NodeActions { * `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; } /** @@ -74,11 +85,13 @@ export function useNodeActions(nodeId: string | null | undefined): NodeActions { 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; @@ -102,16 +115,45 @@ export function useNodeActions(nodeId: string | null | undefined): NodeActions { } }; - const duplicate = () => { - if (!nodeId || nodeId === 'ROOT') return; + const duplicate = (): string | null => { + if (!nodeId || nodeId === 'ROOT') return null; try { const parentId = getParentId(); - if (!parentId) return; + if (!parentId) return null; const tree = regenerateTreeIds(query.node(nodeId).toNodeTree()); - actions.addNodeTree(tree, parentId); + + // 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; } }; @@ -165,5 +207,15 @@ export function useNodeActions(nodeId: string | null | undefined): NodeActions { } }; - return { moveUp, moveDown, duplicate, deleteNode, selectParent, canMoveUp, canMoveDown, canDelete }; + return { + moveUp, + moveDown, + duplicate, + deleteNode, + selectParent, + canMoveUp, + canMoveDown, + canDelete, + canSelectParent, + }; } diff --git a/craft/src/panels/mobile/MobileSelectionToolbar.tsx b/craft/src/panels/mobile/MobileSelectionToolbar.tsx index ab21db3..09e8502 100644 --- a/craft/src/panels/mobile/MobileSelectionToolbar.tsx +++ b/craft/src/panels/mobile/MobileSelectionToolbar.tsx @@ -34,6 +34,11 @@ export const MobileSelectionToolbar: React.FC = () => { const id = selected && selected.size > 0 ? (Array.from(selected)[0] as string) : null; return { selectedId: id && id !== 'ROOT' ? id : null }; }); + // Collector-free -- an always-live reference (same pattern as + // GuidedStyles.tsx/BlocksPanel.tsx), used only for imperative DOM lookups + // below (scroll-into-view), never for anything that needs to react to + // store changes on its own. + const { query } = useEditor(); const nodeActions = useNodeActions(selectedId); const [confirmingDelete, setConfirmingDelete] = useState(false); @@ -76,6 +81,24 @@ export const MobileSelectionToolbar: React.FC = () => { useEffect(() => clearConfirmTimeout, [clearConfirmTimeout]); + // Fast-follow item 3: the Styles bottom sheet covers the bottom ~65dvh of + // the canvas, so a selected node sitting anywhere in that band goes + // invisible the instant the sheet opens -- live style edits then have no + // visible effect until the user closes the sheet to check. Whenever the + // sheet becomes 'styles' (however it got opened -- currently only this + // toolbar's own "Style" button, but this is keyed on the sheet/selection + // state rather than the button tap so it stays correct if another entry + // point is added later) scroll the selected node to the top of the + // canvas's scroll area, into the sliver of screen the sheet leaves clear. + useEffect(() => { + if (activeSheet !== 'styles' || !selectedId) return; + try { + query.node(selectedId).get()?.dom?.scrollIntoView({ behavior: 'smooth', block: 'start' }); + } catch { + // Best-effort -- not fatal if the node/DOM isn't found. + } + }, [activeSheet, selectedId, query]); + const handleDeleteTap = useCallback(() => { if (confirmingDelete) { clearConfirmTimeout(); @@ -104,9 +127,24 @@ export const MobileSelectionToolbar: React.FC = () => { }, [nodeActions, bumpTick]); const handleDuplicate = useCallback(() => { - nodeActions.duplicate(); + const newId = nodeActions.duplicate(); bumpTick(); - }, [nodeActions, bumpTick]); + if (!newId) return; + // Mirror BlocksPanel's tap-to-add pattern: the new node's real DOM + // element isn't attached yet the instant `duplicate()` returns (Craft.js + // mounts it asynchronously after this handler's synchronous state + // update), so defer two animation frames before reading `.dom` off the + // query, same as `handleTapToAdd` does. + requestAnimationFrame(() => { + 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. + } + }); + }); + }, [nodeActions, bumpTick, query]); if (!selectedId || activeSheet !== null) return null; @@ -145,6 +183,7 @@ export const MobileSelectionToolbar: React.FC = () => { type="button" className="mobile-selection-toolbar-btn" onClick={nodeActions.selectParent} + disabled={!nodeActions.canSelectParent} aria-label="Select parent" >