fix(ui): canvas scroll (#7) and cross-group drag/drop (#9)

packages/builder-ui/src/app/ExpressionBuilderShell.tsx

@@ -10,6 +10,7 @@ import {
   deleteNode,
   duplicateRule,
   focusGroup,
+  moveNode,
   reorderNode,
   selectRule,
   updateRule,
@@ -324,6 +325,14 @@ export function ExpressionBuilderShell({
     );
   };
 
+  const moveConditionNode = (
+    nodeId: string,
+    targetGroupId: string,
+    index: number,
+  ) => {
+    setDocument((current) => moveNode(current, nodeId, targetGroupId, index));
+  };
+
   const toggleWrapper = (wrapperId: string) =>
     setSelectedWrappers((current) =>
       current.includes(wrapperId) ? current.filter((id) => id !== wrapperId) : [...current, wrapperId],
@@ -417,6 +426,7 @@ export function ExpressionBuilderShell({
           root={document.root}
           onInsertField={insertFieldAtPosition}
           onReorderNode={reorderConditionNode}
+          onMoveNode={moveConditionNode}
         >
           <main
             className="eb-workspace"

packages/builder-ui/src/theme/tokens.css

@@ -485,7 +485,16 @@ body {
 
 /* Preview Card */
 .eb-preview-card {
+  display: flex;
+  flex-direction: column;
   flex: 0 0 auto;
+  /* Content-sized with no ceiling, this card grows with the expression and the
+     canvas — the only flexible sibling — pays for every pixel. An empty document
+     measures 178px, so the cap is inert until the expression runs past roughly
+     four lines, after which .eb-preview-body scrolls instead. The flex column
+     above is what makes that body shrink; with max-height alone it keeps its
+     natural height and the overflow is silently clipped. */
+  max-height: 220px;
   border-radius: var(--r-panel);
   border: 1px solid var(--border);
   background: var(--surface);
@@ -531,6 +540,8 @@ body {
   padding: 14px;
   display: flex;
   flex-direction: column;
+  flex: 1 1 auto;
+  min-height: 0;
   gap: 14px;
   overflow: auto;
   scrollbar-gutter: stable;
@@ -564,10 +575,18 @@ body {
   flex-shrink: 0;
 }
 
+/* The root card is the exception to the flex-shrink:0 above. It is the pane's
+   only child, so refusing to shrink makes it grow to its full content height:
+   .eb-group-children then sizes to that expanded parent, its overflow:auto never
+   engages, and the scroll falls outward to .eb-pane-body. Measured in PPTB at a
+   420px frame with six rules, the list rendered 1334px tall inside a 398px
+   canvas. flex:1 1 auto restores shrink; min-height:0 defeats the automatic
+   minimum that would otherwise still block it. Nested cards keep shrink:0. */
 .eb-group-card.is-root {
   display: flex;
   flex-direction: column;
-  flex-grow: 1;
+  flex: 1 1 auto;
+  min-height: 0;
 }
 
 .eb-group-card.is-empty {
@@ -579,6 +598,9 @@ body {
 .eb-group-card.is-root > .eb-group-children,
 .eb-group-card.is-empty > .eb-group-children {
   flex: 1;
+  /* Pairs with .is-root's min-height:0 above: once the card can shrink, this is
+     what lets the list shrink with it and scroll internally. */
+  min-height: 0;
 }
 
 .eb-group-card.is-root > .eb-group-children > .eb-condition-drop-target.is-terminal,
@@ -1723,6 +1745,10 @@ body {
 
   .eb-preview-card {
     flex-basis: auto;
+    /* The desktop cap keeps a long expression from eating the canvas they share
+       a column with. Stacked, they no longer compete and the page scrolls as
+       one, so an inner scroll region here would only add a second scrollbar. */
+    max-height: none;
   }
 
   /* Stacked layout scrolls as one page. The desktop chain nests two scroll

packages/builder-ui/src/workbench/BuilderDragDropProvider.tsx

@@ -52,6 +52,7 @@ export function BuilderDragDropProvider({
   root,
   onInsertField,
   onReorderNode,
+  onMoveNode,
 }: BuilderDragDropProviderProps) {
   const initiatingHandle = useRef<HTMLElement | null>(null);
   const [dragAnnouncement, setDragAnnouncement] = useState('');
@@ -92,6 +93,8 @@ export function BuilderDragDropProvider({
           onInsertField(command.fieldId, command.groupId, command.index);
         } else if (command?.kind === 'reorder-node') {
           onReorderNode(command.nodeId, command.parentGroupId, command.index);
+        } else if (command?.kind === 'move-node') {
+          onMoveNode(command.nodeId, command.targetGroupId, command.index);
         }
 
         const handle = initiatingHandle.current;

packages/builder-ui/src/workbench/ConditionGroupCard.tsx

@@ -13,6 +13,8 @@ interface ConditionGroupCardProps {
   group: QueryGroup;
   fields: FieldDefinition[];
   isRoot?: boolean;
+  /** Ids of every group above this one, root first. See ConditionPositionTarget. */
+  ancestorGroupIds?: readonly string[];
   parentGroupId?: string;
   sourceIndex?: number;
   siblingCount?: number;
@@ -35,6 +37,7 @@ export function ConditionGroupCard({
   fields,
   group,
   isRoot = false,
+  ancestorGroupIds = [],
   parentGroupId,
   sourceIndex,
   siblingCount,
@@ -57,6 +60,9 @@ export function ConditionGroupCard({
   const ruleCount = countRules(group);
   const isFocused = group.id === activeGroupId;
   const groupLabel = `${group.conjunction.toUpperCase()} group ${group.id}`;
+  // Separators rendered here sit inside this group, so the chain they guard
+  // against includes it.
+  const targetAncestorIds = [...ancestorGroupIds, group.id];
 
   const renderCard = ({
     sourceRef,
@@ -153,6 +159,7 @@ export function ConditionGroupCard({
             <ConditionPositionTarget
               groupId={group.id}
               groupLabel={groupLabel}
+              ancestorGroupIds={targetAncestorIds}
               index={childIndex}
               beforeNodeId={child.id}
               positionCount={group.children.length + 1}
@@ -161,6 +168,7 @@ export function ConditionGroupCard({
               <ConditionGroupCard
                 group={child}
                 fields={fields}
+                ancestorGroupIds={targetAncestorIds}
                 parentGroupId={group.id}
                 sourceIndex={childIndex}
                 siblingCount={group.children.length}
@@ -200,6 +208,7 @@ export function ConditionGroupCard({
         <ConditionPositionTarget
           groupId={group.id}
           groupLabel={groupLabel}
+          ancestorGroupIds={targetAncestorIds}
           index={group.children.length}
           positionCount={group.children.length + 1}
           terminal

packages/builder-ui/src/workbench/ConditionPositionTarget.tsx

@@ -13,6 +13,14 @@ interface ConditionPositionTargetProps {
   groupLabel: string;
   index: number;
   positionCount: number;
+  /**
+   * This separator's own group and every group above it, root first. A node can
+   * never land inside itself, and only the ancestor chain can tell: this
+   * component sees one group, and resolveDragDropCommand sees only the drag
+   * metadata, so without it a group dragged onto its own descendants would light
+   * up as a valid target and then be rejected on drop.
+   */
+  ancestorGroupIds: readonly string[];
   /** Id of the node this separator sits before; omitted for the terminal one. */
   beforeNodeId?: string;
   terminal?: boolean;
@@ -23,6 +31,7 @@ export function ConditionPositionTarget({
   groupLabel,
   index,
   positionCount,
+  ancestorGroupIds,
   beforeNodeId,
   terminal = false,
 }: ConditionPositionTargetProps) {
@@ -33,17 +42,18 @@ export function ConditionPositionTarget({
   };
   const { source } = useDragOperation();
   const isActive = source !== null && source !== undefined;
+  const landsInsideItself =
+    isActive &&
+    isConditionNodeDragMetadata(source.data) &&
+    ancestorGroupIds.includes(source.data.nodeId);
   const isValidDrop =
     isActive &&
+    !landsInsideItself &&
     resolveDragDropCommand({
       source: source.data,
       target: metadata,
     }) !== undefined;
-  const isIneligibleGroup =
-    isActive &&
-    !isValidDrop &&
-    isConditionNodeDragMetadata(source.data) &&
-    source.data.parentGroupId !== groupId;
+  const isIneligibleGroup = isActive && !isValidDrop && landsInsideItself;
   const { isDropTarget, ref } = useDroppable({
     id: conditionPositionDropId(groupId, beforeNodeId),
     data: metadata,
@@ -56,6 +66,10 @@ export function ConditionPositionTarget({
     // on hit-box geometry.
     collisionDetector: closestCenter,
     accept: (draggable) =>
+      !(
+        isConditionNodeDragMetadata(draggable.data) &&
+        ancestorGroupIds.includes(draggable.data.nodeId)
+      ) &&
       resolveDragDropCommand({
         source: draggable.data,
         target: metadata,

packages/builder-ui/src/workbench/dragDropModel.ts

@@ -34,6 +34,12 @@ export type DragDropCommand =
       nodeId: string;
       parentGroupId: string;
       index: number;
+    }
+  | {
+      kind: 'move-node';
+      nodeId: string;
+      targetGroupId: string;
+      index: number;
     };
 
 export type DragDropResolutionInput = {
@@ -113,10 +119,29 @@ export const resolveDragDropCommand = ({
     };
   }
 
-  if (!isConditionNodeDragMetadata(source) || source.parentGroupId !== target.groupId) {
+  if (!isConditionNodeDragMetadata(source)) {
     return undefined;
   }
 
+  if (source.parentGroupId !== target.groupId) {
+    /**
+     * A move into another group. The index is used as-is: unlike a reorder, the
+     * node is removed from a *different* list, so nothing shifts in the target
+     * and the -1 correction below would land it one position too high.
+     *
+     * Legality beyond shape — the target group existing, and a group never
+     * landing inside itself — needs the document tree, so it is enforced by
+     * resolveCurrentDragDropCommand and mirrored in the drop target's own
+     * ancestor check.
+     */
+    return {
+      kind: 'move-node',
+      nodeId: source.nodeId,
+      targetGroupId: target.groupId,
+      index: target.index,
+    };
+  }
+
   if (target.index === source.sourceIndex || target.index === source.sourceIndex + 1) {
     return undefined;
   }
@@ -155,10 +180,31 @@ export const resolveCurrentDragDropCommand = ({
   }
 
   const source = findNodeLocation(root, command.nodeId);
-  return source?.parent?.id === command.parentGroupId &&
-    source.index === input.source.sourceIndex &&
-    command.index >= 0 &&
-    command.index < targetGroup.children.length
+  if (
+    !source ||
+    source.parent?.id !== input.source.parentGroupId ||
+    source.index !== input.source.sourceIndex
+  ) {
+    return undefined;
+  }
+
+  if (command.kind === 'move-node') {
+    // Appending past the last child is legal for a move but not for a reorder,
+    // so the bound is inclusive here. findNodeLocation matches the subtree root
+    // as well as its descendants, which covers both dropping a group onto its
+    // own separators and onto one nested deeper inside it.
+    const landsInsideItself =
+      source.node.kind === 'group' &&
+      findNodeLocation(source.node, command.targetGroupId) !== undefined;
+
+    return command.index >= 0 &&
+      command.index <= targetGroup.children.length &&
+      !landsInsideItself
+      ? command
+      : undefined;
+  }
+
+  return command.index >= 0 && command.index < targetGroup.children.length
     ? command
     : undefined;
 };

packages/builder-ui/src/workbench/types.ts

@@ -134,4 +134,5 @@ export interface BuilderDragDropProviderProps {
   root: QueryGroup;
   onInsertField: (fieldId: string, groupId: string, index: number) => void;
   onReorderNode: (nodeId: string, parentGroupId: string, finalIndex: number) => void;
+  onMoveNode: (nodeId: string, targetGroupId: string, index: number) => void;
 }