Merge issue-39 (Refs #39)

# Conflicts:
#	craft/src/styles/editor.css

craft/src/editor/EditorShell.tsx

@@ -15,6 +15,7 @@ import { LayerFocusProvider } from '../panels/left/LayerFocusContext';
 import { LoadGate } from './LoadGate';
 import { DirtyTracker } from './DirtyTracker';
 import { Breadcrumb } from './Breadcrumb';
+import { ShortcutsModalHost } from '../panels/topbar/ShortcutsModal';
 import { DeviceProvider } from '../state/DeviceContext';
 
 const SHOW_GUIDES_STORAGE_KEY = 'craft-show-guides';
@@ -58,7 +59,7 @@ const EditorShellBody: React.FC = () => {
   const { menuState, show: showMenu, hide: hideMenu } = useContextMenu();
   const { query } = useEditor();
 
-  // Register keyboard shortcuts
+  // Register the core commands and mount the single keydown dispatcher.
   useKeyboardShortcuts();
 
   const handleContextMenu = useCallback((e: React.MouseEvent) => {
@@ -101,6 +102,7 @@ const EditorShellBody: React.FC = () => {
           </div>
           {isMobile && <MobilePanelBar />}
           {isMobile && <MobileSelectionToolbar />}
+          <ShortcutsModalHost />
           <ContextMenu
             visible={menuState.visible}
             x={menuState.x}

craft/src/editor/commands.ts

@@ -36,6 +36,8 @@ export interface Command {
   keywords?: string[];
   /** Shortcut spec string, see utils/shortcuts.ts ('mod+d', 'alt+arrowup'). */
   shortcut?: string;
+  /** Extra shortcut specs that trigger the same command ('mod+y' for redo). */
+  shortcuts?: string[];
   when?(ctx: CommandContext): boolean;
   run(ctx: CommandContext): void | Promise<void>;
   /** Not shown in the palette / menu (still runnable by shortcut and id). */
@@ -97,7 +99,7 @@ export function useCommandContext(): CommandContext {
   // Collect a string key (not an array) so the hook only re-renders when the
   // selection actually changes.
   const { actions, query, selKey } = useEditor((state) => ({
-    selKey: Array.from(state.events.selected || []).join('\n'),
+    selKey: Array.from(state.events?.selected || []).join('\n'),
   }));
   const { device, setDevice } = useDevice();
   const pages = usePages();
@@ -158,11 +160,25 @@ export function useRunCommand(): (id: string) => Promise<boolean> {
   }, []);
 }
 
+/** Keys that are also how focus / dialogs are driven: Tab walks focus and Esc
+ *  dismisses. They only belong to the canvas when focus is on it (or the body),
+ *  never while a button, link or dialog holds focus. */
+function focusOutsideCanvas(el: Element | null): boolean {
+  // An open modal owns Esc / Tab even when focus has not moved into it.
+  if (document.querySelector('[role="dialog"][aria-modal="true"]')) return true;
+  if (!el || el === document.body) return false;
+  if (el.closest('[role="dialog"], [aria-modal="true"], .modal-backdrop')) return true;
+  return !!el.closest('button, a[href], summary, [role="button"], [role="menuitem"], [role="tab"]');
+}
+
+function shortcutsOf(cmd: Command): string[] {
+  return cmd.shortcut ? [cmd.shortcut, ...(cmd.shortcuts ?? [])] : [];
+}
+
 /**
  * Document-level keydown dispatcher: first registered command whose shortcut
- * matches and whose `when()` passes runs, with preventDefault. NOT mounted by
- * W0-commands -- issue #39 mounts it (and retires useKeyboardShortcuts) so two
- * listeners never double-fire.
+ * matches and whose `when()` passes runs, with preventDefault. Mounted once, by
+ * useKeyboardShortcuts (#39), so two listeners never double-fire.
  */
 export function useCommandShortcuts(): void {
   const ctx = useCommandContext();
@@ -174,7 +190,9 @@ export function useCommandShortcuts(): void {
       const typing = isTypingTarget(document.activeElement);
       const c = liveContext(ref.current);
       for (const cmd of getCommands()) {
-        if (!cmd.shortcut || !matchShortcut(e, cmd.shortcut)) continue;
+        const hit = shortcutsOf(cmd).find((spec) => matchShortcut(e, spec));
+        if (!hit) continue;
+        if ((hit === 'tab' || hit === 'shift+tab' || hit === 'escape') && focusOutsideCanvas(document.activeElement)) continue;
         if (typing && !cmd.allowWhileTyping) continue;
         if (!isEnabled(cmd, c)) continue;
         e.preventDefault();

craft/src/editor/core-commands.test.tsx

@@ -0,0 +1,111 @@
+import { describe, test, expect, beforeEach, afterEach, vi } from 'vitest';
+import { renderEditorHarness, EditorHarness } from '../test-utils/editorHarness';
+import { SELECTION_STATE } from './selection-fixture';
+import { CORE_COMMANDS, topLevelSelection } from './core-commands';
+import { setClipboardTree } from '../hooks/clipboard';
+import type { Command, CommandContext } from './commands';
+
+let h: EditorHarness;
+beforeEach(() => {
+  setClipboardTree(null);
+  h = renderEditorHarness({ initialState: SELECTION_STATE });
+});
+afterEach(() => { h.unmount(); setClipboardTree(null); });
+
+const cmd = (id: string): Command => CORE_COMMANDS.find((c) => c.id === id)!;
+const ctx = (ids: string[], extra: Partial<CommandContext> = {}): CommandContext =>
+  ({ query: h.query, actions: h.actions, selectedIds: ids, setDevice: vi.fn(), ...extra }) as any;
+const kids = (id: string) => h.query.node(id).get().data.nodes as string[];
+const NODE_CMDS = ['edit.delete', 'edit.duplicate', 'edit.copy', 'edit.move-up', 'edit.move-down'];
+
+describe('when() guards', () => {
+  test.each(NODE_CMDS)('%s is disabled with no selection or only ROOT selected', (id) => {
+    expect(cmd(id).when!(ctx([]))).toBe(false);
+    expect(cmd(id).when!(ctx(['ROOT']))).toBe(false);
+    expect(cmd(id).when!(ctx(['b1']))).toBe(true);
+  });
+
+  test('ROOT selected: running a node command changes nothing', () => {
+    const before = h.getSerialized();
+    for (const id of NODE_CMDS) h.act(() => { cmd(id).run(ctx(['ROOT'])); });
+    expect(h.getSerialized()).toBe(before);
+  });
+
+  test('paste requires a clipboard tree', () => {
+    expect(cmd('edit.paste').when!(ctx(['b1']))).toBe(false);
+    setClipboardTree(h.query.node('b1').toNodeTree());
+    expect(cmd('edit.paste').when!(ctx(['b1']))).toBe(true);
+  });
+
+  test('undo / redo follow the history', () => {
+    expect(cmd('edit.redo').when!(ctx([]))).toBe(false);
+    h.act(() => { cmd('edit.delete').run(ctx(['b1'])); });
+    expect(cmd('edit.undo').when!(ctx([]))).toBe(true);
+    h.act(() => { cmd('edit.undo').run(ctx([])); });
+    expect(kids('col-0')).toEqual(['b1', 'b2']);
+    expect(cmd('edit.redo').when!(ctx([]))).toBe(true);
+  });
+});
+
+describe('registry shape', () => {
+  test('ids and shortcuts', () => {
+    const spec = (id: string) => [cmd(id).shortcut, ...(cmd(id).shortcuts ?? [])];
+    expect(spec('edit.undo')).toEqual(['mod+z']);
+    expect(spec('edit.redo')).toEqual(['mod+shift+z', 'mod+y']);
+    expect(spec('edit.delete')).toEqual(['delete', 'backspace']);
+    expect(spec('edit.duplicate')).toEqual(['mod+d']);
+    expect(spec('edit.move-up')).toEqual(['alt+arrowup']);
+    expect(spec('view.device-mobile')).toEqual(['3']);
+    expect(spec('help.shortcuts')[0]).toBe('mod+/');
+    expect(new Set(CORE_COMMANDS.map((c) => c.id)).size).toBe(CORE_COMMANDS.length);
+  });
+});
+
+describe('multi-select', () => {
+  test('topLevelSelection drops ROOT and nodes under another selected node', () => {
+    expect(topLevelSelection(h.query, ['ROOT', 'sec', 'b1', 'sec2'])).toEqual(['sec', 'sec2']);
+  });
+
+  test('a selected node plus its own child resolves to the ancestor only (a populated column is refused)', () => {
+    h.act(() => { cmd('edit.delete').run(ctx(['b1', 'col-0'])); });
+    expect(kids('col-0')).toEqual(['b1', 'b2']);
+  });
+
+  test('delete of two siblings', () => {
+    h.act(() => { cmd('edit.delete').run(ctx(['b1', 'b2'])); });
+    expect(kids('col-0')).toEqual([]);
+  });
+
+  test('duplicate copies each after its source and selects all copies', () => {
+    h.act(() => { cmd('edit.duplicate').run(ctx(['b1', 'b2'])); });
+    const order = kids('col-0');
+    expect(order).toHaveLength(4);
+    expect([order[0], order[2]]).toEqual(['b1', 'b2']);
+    expect(h.query.getEvent('selected').all().sort()).toEqual([order[1], order[3]].sort());
+  });
+
+  test('move down keeps a selected run in order', () => {
+    h.act(() => { cmd('edit.duplicate').run(ctx(['b2'])); });
+    const [, , c] = [0, 1, 2].map((i) => kids('col-0')[i]);
+    expect(kids('col-0')[2]).toBe(c);
+    h.act(() => { cmd('edit.move-down').run(ctx(['b1', 'b2'])); });
+    // b2 was followed by its copy; b2 steps down first, then b1
+    expect(kids('col-0')).toHaveLength(3);
+    expect(kids('col-0').indexOf('b1')).toBeGreaterThan(-1);
+    expect(kids('col-0').indexOf('b1')).toBeLessThan(kids('col-0').indexOf('b2'));
+  });
+
+  test('linked nodes (columns) are not duplicated or moved', () => {
+    const before = h.getSerialized();
+    h.act(() => { cmd('edit.duplicate').run(ctx(['col-0'])); cmd('edit.move-down').run(ctx(['col-0'])); });
+    expect(h.getSerialized()).toBe(before);
+  });
+});
+
+describe('device commands', () => {
+  test('call setDevice with their mode', () => {
+    const setDevice = vi.fn();
+    cmd('view.device-tablet').run(ctx([], { setDevice }));
+    expect(setDevice).toHaveBeenCalledWith('tablet');
+  });
+});

craft/src/editor/core-commands.ts

@@ -0,0 +1,227 @@
+import { registerCommands, type Command, type CommandContext } from './commands';
+import { findDeletableTarget, getAncestorIds, getParentId } from '../utils/craft-helpers';
+import { regenerateTreeIds } from '../utils/craft-tree';
+import { getClipboardTree, setClipboardTree } from '../hooks/clipboard';
+import type { DeviceMode } from '../types';
+
+/**
+ * The editor's core keyboard commands (undo/redo, delete, duplicate,
+ * copy/paste, move, device, shortcuts help) as registry entries. The one
+ * dispatcher (`useCommandShortcuts`) runs them; the context menu and the
+ * cheat sheet read their labels from here. Node commands act on EVERY selected
+ * node (multi-select), skipping ROOT and any node whose ancestor is also
+ * selected (it travels with the ancestor).
+ */
+
+/** Selected ids minus ROOT and minus descendants of other selected ids, in selection order. */
+export function topLevelSelection(query: any, ids: string[]): string[] {
+  const set = new Set(ids);
+  return ids.filter((id) => {
+    if (!id || id === 'ROOT') return false;
+    try {
+      return !getAncestorIds(query, id).some((a) => set.has(a));
+    } catch {
+      return false;
+    }
+  });
+}
+
+const hasSelection = (ctx: CommandContext) => topLevelSelection(ctx.query, ctx.selectedIds).length > 0;
+
+function siblingsOf(query: any, id: string): { parentId: string; siblings: string[] } | null {
+  const parentId = getParentId(query, id);
+  if (!parentId) return null;
+  try {
+    return { parentId, siblings: query.node(parentId).get()?.data?.nodes || [] };
+  } catch {
+    return null;
+  }
+}
+
+/** Insert `tree` under `parentId` at `index` (append when omitted); returns its root id. */
+function insertTree(actions: any, tree: any, parentId: string, index?: number): string {
+  if (index !== undefined) actions.addNodeTree(tree, parentId, index);
+  else actions.addNodeTree(tree, parentId);
+  return tree.rootNodeId;
+}
+
+function duplicateAll({ query, actions, selectedIds }: CommandContext): void {
+  const created: string[] = [];
+  for (const id of topLevelSelection(query, selectedIds)) {
+    try {
+      const sib = siblingsOf(query, id);
+      const idx = sib ? sib.siblings.indexOf(id) : -1;
+      // Linked nodes (columns, section inner) are not in `nodes` and cannot be copied alone.
+      if (!sib || idx === -1) continue;
+      const tree = regenerateTreeIds(query.node(id).toNodeTree());
+      created.push(insertTree(actions, tree, sib.parentId, idx + 1));
+    } catch (err) {
+      console.error('Duplicate failed:', err);
+    }
+  }
+  if (created.length) actions.selectNode(created.length === 1 ? created[0] : created);
+}
+
+function deleteAll({ query, actions, selectedIds }: CommandContext): void {
+  const targets = new Set<string>();
+  for (const id of topLevelSelection(query, selectedIds)) {
+    const t = findDeletableTarget(query, id);
+    if (t) targets.add(t);
+  }
+  targets.forEach((t) => {
+    try {
+      actions.delete(t);
+    } catch (err) {
+      console.error('Delete failed:', err);
+    }
+  });
+}
+
+function pasteClipboard({ query, actions, selectedIds }: CommandContext): void {
+  const clip = getClipboardTree();
+  if (!clip) return;
+  const sel = topLevelSelection(query, selectedIds);
+  let parentId = 'ROOT';
+  let index: number | undefined;
+  const last = sel[sel.length - 1];
+  if (last) {
+    const sib = siblingsOf(query, last);
+    if (sib) {
+      parentId = sib.parentId;
+      const idx = sib.siblings.indexOf(last);
+      if (idx !== -1) index = idx + 1;
+    }
+  }
+  const tree = regenerateTreeIds(clip);
+  actions.selectNode(insertTree(actions, tree, parentId, index));
+}
+
+/** One step among siblings. Moving down walks the selection in reverse so a run of
+ *  adjacent nodes keeps its order; a node already at the edge stays put. */
+function moveAll({ query, actions, selectedIds }: CommandContext, dir: -1 | 1): void {
+  const ids = topLevelSelection(query, selectedIds);
+  for (const id of dir === 1 ? [...ids].reverse() : ids) {
+    try {
+      const sib = siblingsOf(query, id);
+      if (!sib) continue;
+      const idx = sib.siblings.indexOf(id);
+      if (idx === -1) continue;
+      if (dir === -1 && idx > 0) actions.move(id, sib.parentId, idx - 1);
+      // Craft's move index is the slot BEFORE removal, hence idx + 2.
+      if (dir === 1 && idx < sib.siblings.length - 1) actions.move(id, sib.parentId, idx + 2);
+    } catch (err) {
+      console.error('Move failed:', err);
+    }
+  }
+}
+
+/** Cheat-sheet visibility: a module-level channel (like clipboard.ts) so a command can open it. */
+const shortcutsListeners = new Set<() => void>();
+export function subscribeShortcutsModal(cb: () => void): () => void {
+  shortcutsListeners.add(cb);
+  return () => {
+    shortcutsListeners.delete(cb);
+  };
+}
+export function openShortcutsModal(): void {
+  shortcutsListeners.forEach((cb) => cb());
+}
+
+const deviceCommand = (device: DeviceMode, key: string): Command => ({
+  id: `view.device-${device}`,
+  title: `${device.charAt(0).toUpperCase()}${device.slice(1)} view`,
+  group: 'view',
+  shortcut: key,
+  run: ({ setDevice }) => setDevice(device),
+});
+
+export const CORE_COMMANDS: Command[] = [
+  {
+    id: 'edit.undo',
+    title: 'Undo',
+    group: 'edit',
+    shortcut: 'mod+z',
+    when: ({ query }) => query.history.canUndo(),
+    run: ({ actions }) => actions.history.undo(),
+  },
+  {
+    id: 'edit.redo',
+    title: 'Redo',
+    group: 'edit',
+    shortcut: 'mod+shift+z',
+    shortcuts: ['mod+y'],
+    when: ({ query }) => query.history.canRedo(),
+    run: ({ actions }) => actions.history.redo(),
+  },
+  {
+    id: 'edit.delete',
+    title: 'Delete',
+    group: 'edit',
+    shortcut: 'delete',
+    shortcuts: ['backspace'],
+    when: hasSelection,
+    run: deleteAll,
+  },
+  {
+    id: 'edit.duplicate',
+    title: 'Duplicate',
+    group: 'edit',
+    shortcut: 'mod+d',
+    when: hasSelection,
+    run: duplicateAll,
+  },
+  {
+    id: 'edit.copy',
+    title: 'Copy',
+    group: 'edit',
+    shortcut: 'mod+c',
+    when: hasSelection,
+    // A detached tree snapshot, not an id: it must survive a page switch (see clipboard.ts).
+    // v1: with several nodes selected, the first is copied.
+    run: ({ query, selectedIds }) => {
+      const [id] = topLevelSelection(query, selectedIds);
+      if (id) setClipboardTree(query.node(id).toNodeTree());
+    },
+  },
+  {
+    id: 'edit.paste',
+    title: 'Paste',
+    group: 'edit',
+    shortcut: 'mod+v',
+    when: () => !!getClipboardTree(),
+    run: pasteClipboard,
+  },
+  {
+    id: 'edit.move-up',
+    title: 'Move up',
+    group: 'edit',
+    shortcut: 'alt+arrowup',
+    when: hasSelection,
+    run: (ctx) => moveAll(ctx, -1),
+  },
+  {
+    id: 'edit.move-down',
+    title: 'Move down',
+    group: 'edit',
+    shortcut: 'alt+arrowdown',
+    when: hasSelection,
+    run: (ctx) => moveAll(ctx, 1),
+  },
+  deviceCommand('desktop', '1'),
+  deviceCommand('tablet', '2'),
+  deviceCommand('mobile', '3'),
+  {
+    id: 'help.shortcuts',
+    title: 'Keyboard shortcuts',
+    group: 'help',
+    shortcut: 'mod+/',
+    // '?' is Shift+/ on most layouts.
+    shortcuts: ['mod+shift+?', 'mod+shift+/'],
+    allowWhileTyping: true,
+    run: () => openShortcutsModal(),
+  },
+];
+
+export function registerCoreCommands(): () => void {
+  return registerCommands(CORE_COMMANDS);
+}

craft/src/editor/selection-commands.test.tsx

@@ -78,7 +78,7 @@ describe('command definitions', () => {
     expect(byId['select.next-sibling'].shortcut).toBe('tab');
     expect(byId['select.prev-sibling'].shortcut).toBe('shift+tab');
     expect(byId['select.first-child'].shortcut).toBeUndefined();
-    const ctx = (ids: string[]) => ({ selectedIds: ids }) as any;
+    const ctx = (ids: string[]) => ({ selectedIds: ids, query: h.query }) as any;
     for (const c of selectionCommands) {
       expect(c.group).toBe('select');
       expect(c.when!(ctx(['b1']))).toBe(true);
@@ -86,6 +86,10 @@ describe('command definitions', () => {
       expect(c.when!(ctx(['ROOT']))).toBe(false);
       expect(c.when!(ctx(['a', 'b']))).toBe(false);
     }
+    // Sibling cycling only claims Tab when it can move: a lone node ('cols') is left to the browser.
+    expect(byId['select.next-sibling'].when!(ctx(['cols']))).toBe(false);
+    expect(byId['select.prev-sibling'].when!(ctx(['cols']))).toBe(false);
+    expect(byId['select.parent'].when!(ctx(['cols']))).toBe(true);
     __resetCommandsForTests();
     expect(getCommand('select.parent')).toBeUndefined();
   });

craft/src/editor/selection-commands.ts

@@ -80,6 +80,11 @@ export function selectFirstChild(query: any, actions: any, id: string): string |
 const oneNonRootSelected = (ctx: { selectedIds: string[] }) =>
   ctx.selectedIds.length === 1 && ctx.selectedIds[0] !== 'ROOT';
 
+// Sibling cycling only claims Tab when it will move the selection, so focus
+// navigation still works with a lone node.
+const hasSiblings = (ctx: { query: any; selectedIds: string[] }) =>
+  oneNonRootSelected(ctx) && getSiblingIds(ctx.query, ctx.selectedIds[0]).length >= 2;
+
 export const selectionCommands: Command[] = [
   {
     id: 'select.parent',
@@ -94,7 +99,7 @@ export const selectionCommands: Command[] = [
     title: 'Select next sibling',
     group: 'select',
     shortcut: 'tab',
-    when: oneNonRootSelected,
+    when: hasSiblings,
     run: ({ query, actions, selectedIds }) => void selectNextSibling(query, actions, selectedIds[0]),
   },
   {
@@ -102,7 +107,7 @@ export const selectionCommands: Command[] = [
     title: 'Select previous sibling',
     group: 'select',
     shortcut: 'shift+tab',
-    when: oneNonRootSelected,
+    when: hasSiblings,
     run: ({ query, actions, selectedIds }) => void selectPrevSibling(query, actions, selectedIds[0]),
   },
   {