GLSP-394: NoOverlapMovementRestrictor support for nested nodes (#452)

Adapt the  implementation of the NoOverlapMovementRestrictor to properly
handle nested nodes

Fixes eclipse-glsp/glsp#394

Author: tortmayr

examples/workflow-glsp/src/workflow-diagram-module.ts

@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (c) 2019-2024 EclipseSource and others.
+ * Copyright (c) 2019-2025 EclipseSource and others.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0 which is available at
@@ -30,6 +30,7 @@ import {
     GLabelView,
     IHelperLineOptions,
     LogLevel,
+    NoOverlapMovementRestrictor,
     RectangularNodeView,
     RevealNamedElementActionProvider,
     RoundedCornerNodeView,
@@ -64,7 +65,7 @@ export const workflowDiagramModule = new FeatureModule(
         bindOrRebind(context, TYPES.LogLevel).toConstantValue(LogLevel.warn);
         bindAsService(context, TYPES.ICommandPaletteActionProvider, RevealNamedElementActionProvider);
         bindAsService(context, TYPES.IContextMenuItemProvider, DeleteElementContextMenuItemProvider);
-
+        bind(TYPES.IMovementRestrictor).to(NoOverlapMovementRestrictor).inSingletonScope();
         configureDefaultModelElements(context);
         configureModelElement(context, 'task:automated', TaskNode, RoundedCornerNodeView);
         configureModelElement(context, 'task:manual', TaskNode, RoundedCornerNodeView);

packages/client/src/features/change-bounds/movement-restrictor.ts

@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (c) 2019-2024 EclipseSource and others.
+ * Copyright (c) 2019-2025 EclipseSource and others.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0 which is available at
@@ -13,11 +13,23 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import { Bounds, Dimension, GModelElement, GNode, GParentElement, Point, isBoundsAware, isMoveable } from '@eclipse-glsp/sprotty';
+import {
+    Bounds,
+    Dimension,
+    GChildElement,
+    GModelElement,
+    GNode,
+    GParentElement,
+    isBoundsAware,
+    isMoveable,
+    Point,
+    toTypeGuard
+} from '@eclipse-glsp/sprotty';
 import { injectable } from 'inversify';
-import { ModifyCSSFeedbackAction } from '../../base/feedback/css-feedback';
-import { BoundsAwareModelElement } from '../../utils/gmodel-util';
+import { CSS_GHOST_ELEMENT, ModifyCSSFeedbackAction } from '../../base/feedback/css-feedback';
+import { BoundsAwareModelElement, getChildren, getParents } from '../../utils/gmodel-util';
 import { toAbsoluteBounds } from '../../utils/viewpoint-util';
+import { ContainerElement, isContainable } from '../hints/model';
 import { GResizeHandle } from './model';
 
 /**
@@ -39,34 +51,75 @@ export interface IMovementRestrictor {
     cssClasses?: string[];
 }
 
+export interface MoveElementContext {
+    element: GModelElement;
+    elementAtNewLocation: BoundsAwareModelElement;
+    parentContainers: ContainerElement[];
+    childNodes: GNode[];
+}
+
 /**
  *  A `IMovementRestrictor` that checks for overlapping elements. Move operations
- *  are only valid if the element does not collide with another element after moving.
+ *  are only valid if the element does not collide with another element/node after moving.
  */
 @injectable()
 export class NoOverlapMovementRestrictor implements IMovementRestrictor {
     cssClasses = ['movement-not-allowed'];
 
     validate(element: GModelElement, newLocation?: Point): boolean {
-        if (!isMoveable(element) || !newLocation) {
+        if (!(element instanceof GChildElement) || !isMoveable(element) || !newLocation) {
             return false;
         }
-        // Create ghost element at the newLocation
+
+        const moveContext = this.createMoveElementContext(element, newLocation);
+        const elementsToValidate = Array.from(element.root.index.all()).filter(e =>
+            this.isBoundsRelevant(e, moveContext)
+        ) as BoundsAwareModelElement[];
+
+        const valid = !elementsToValidate.some(e => this.areOverlapping(e, moveContext.elementAtNewLocation));
+        return valid;
+    }
+
+    protected createMoveElementContext(element: GModelElement, newLocation: Point): MoveElementContext {
+        const parentContainers = getParents(element, isContainable);
+        const childNodes = getChildren(element, toTypeGuard(GNode));
+        // Create a mock element at the newLocation for overlap checking
         const dimensions: Dimension = isBoundsAware(element) ? element.bounds : { width: 1, height: 1 };
-        const ghostElement = Object.create(element) as BoundsAwareModelElement;
-        ghostElement.bounds = { ...dimensions, ...newLocation };
-        ghostElement.type = 'Ghost';
-        ghostElement.id = element.id;
-        return !Array.from(
-            element.root.index
-                .all()
-                .filter(node => node.id !== ghostElement.id && node !== ghostElement.root && node instanceof GNode)
-                .map(node => node as BoundsAwareModelElement)
-        ).some(e => this.areOverlapping(e, ghostElement));
+        const elementAtNewLocation = Object.create(element) as BoundsAwareModelElement as BoundsAwareModelElement;
+        elementAtNewLocation.bounds = { ...dimensions, ...newLocation };
+
+        return {
+            element,
+            elementAtNewLocation,
+            parentContainers,
+            childNodes
+        };
     }
 
-    protected isBoundsRelevant(element: GModelElement, ghostElement: BoundsAwareModelElement): element is BoundsAwareModelElement {
-        return element.id !== ghostElement.id && element !== ghostElement.root && element instanceof GNode && isBoundsAware(element);
+    protected isBoundsRelevant(element: GModelElement, moveContext: MoveElementContext): element is BoundsAwareModelElement {
+        // Only consider GNodes that are not the element being moved (or one of its children)
+        if (
+            !(element instanceof GNode) ||
+            element.id === moveContext.element.id ||
+            moveContext.childNodes.some(child => child.id === element.id)
+        ) {
+            return false;
+        }
+
+        // Do not consider parent containers of the element being moved
+        if (moveContext.parentContainers.length > 0 && moveContext.parentContainers.some(container => container.id === element.id)) {
+            return false;
+        }
+
+        // If the element is a ghost element (node creation), don't consider overlap checks for potential parent containers
+        if (
+            moveContext.element.cssClasses?.includes(CSS_GHOST_ELEMENT) &&
+            isContainable(element) &&
+            element.isContainableElement(moveContext.element.type)
+        ) {
+            return false;
+        }
+        return true;
     }
 
     protected areOverlapping(element1: BoundsAwareModelElement, element2: BoundsAwareModelElement): boolean {

packages/client/src/utils/gmodel-util.ts

@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (c) 2019-2024 EclipseSource and others.
+ * Copyright (c) 2019-2025 EclipseSource and others.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0 which is available at
@@ -86,6 +86,63 @@ export function getMatchingElements<T>(index: ModelIndexImpl, predicate: ModelFi
     return Array.from(filter(index, predicate));
 }
 
+/**
+ * Retrieves an array of all parent elements for the given element that match the given type guard.
+ * @param element The {@link GModelElement} for which the parent elements should be retrieved.
+ * @param guard The type guard that should be used.
+ * @returns An array of all parent elements that match the type guard
+ * (correctly casted to also include the additional type of the guard).
+ */
+export function getParents<T extends GParentElement>(element: GModelElement, guard: TypeGuard<T>): T[];
+/**
+ * Retrieves an array of all parent elements for the given element that match the given {@link ModelFilterPredicate}.
+ * @param element The {@link GModelElement} for which the parent elements should be retrieved.
+ * @param predicate The filter predicate that should be used.
+ * @returns An array of all parent elements that match the filter predicate
+ * (correctly casted to also include the additional type of the predicate).
+ */
+export function getParents<T>(element: GModelElement, predicate: ModelFilterPredicate<T>): (GParentElement & T)[];
+export function getParents(element: GModelElement, filterPredicate: (parent: GParentElement) => boolean): GParentElement[] {
+    const parents: GParentElement[] = [];
+    let current: GModelElement | undefined = element;
+    while (current instanceof GChildElement) {
+        const parent: GParentElement = current.parent;
+        if (filterPredicate(parent)) {
+            parents.push(parent);
+        }
+        current = parent;
+    }
+    return parents;
+}
+
+/** * Retrieves an array of all child elements for the given element that match the given type guard.
+ * @param element The {@link GModelElement} for which the child elements should be retrieved.
+ * @param guard The type guard that should be used.
+ * @returns An array of all child elements that match the type guard
+ * (correctly casted to also include the additional type of the guard).
+ */
+export function getChildren<T extends GChildElement>(element: GModelElement, guard: TypeGuard<T>): T[];
+/** * Retrieves an array of all child elements for the given element that match the given {@link ModelFilterPredicate}.
+ * @param element The {@link GModelElement} for which the child elements should be retrieved.
+ * @param predicate The filter predicate that should be used.
+ * @returns An array of all child elements that match the filter predicate
+ * (correctly casted to also include the additional type of the predicate).
+ */
+export function getChildren<T>(element: GModelElement, predicate: ModelFilterPredicate<T>): (GChildElement & T)[];
+export function getChildren(element: GModelElement, filterPredicate: (parent: GChildElement) => boolean): GChildElement[] {
+    const children: GChildElement[] = [];
+    if (element instanceof GParentElement) {
+        for (const child of element.children) {
+            if (filterPredicate(child)) {
+                children.push(child);
+            }
+            if (child instanceof GParentElement) {
+                children.push(...getChildren(child, filterPredicate as any));
+            }
+        }
+    }
+    return children;
+}
 /**
  * Invokes the given model index to retrieve the corresponding model elements for the given set of ids. Ids that
  * have no corresponding element in the index will be ignored.
@@ -104,6 +161,7 @@ export function getElements<S extends GModelElement>(index: ModelIndexImpl, elem
     };
     return elementsIDs.map(id => index.getById(id)).filter(filterFn);
 }
+
 /**
  * Retrieves the amount of currently selected elements in the given {@link ModelIndexImpl}.
  * @param index The {@link ModelIndexImpl}.