Properly handle move feedback if mouse up happens outside window

- Handle mouse down if we should be dragging as mouse up - Reset feedback if selection has changed -- Ensure element does not have to be selected to revert move -- Add method for listeners to selection service Fixes eclipse-glsp/glsp#1426
eclipse-glsp · Nov 22, 2024 · 2757855 · 2757855
1 parent 352e764
commit 2757855
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 27 deletions.
diff --git a/packages/client/src/base/selection-service.ts b/packages/client/src/base/selection-service.ts
@@ -15,6 +15,7 @@
  ********************************************************************************/
 import {
     Action,
+    AnyObject,
     Command,
     CommandExecutionContext,
     Disposable,
@@ -32,6 +33,7 @@ import {
     SprottySelectCommand,
     TYPES,
     hasArrayProp,
+    hasFunctionProp,
     isSelectable,
     pluck
 } from '@eclipse-glsp/sprotty';
@@ -45,6 +47,12 @@ export interface ISelectionListener {
     selectionChanged(root: Readonly<GModelRoot>, selectedElements: string[], deselectedElements?: string[]): void;
 }
 
+export namespace ISelectionListener {
+    export function is(object: unknown): object is ISelectionListener {
+        return AnyObject.is(object) && hasFunctionProp(object, 'selectionChanged');
+    }
+}
+
 export interface SelectionChange {
     root: Readonly<GModelRoot>;
     selectedElements: string[];
@@ -73,13 +81,7 @@ export class SelectionService implements IGModelRootListener, Disposable, IDiagr
     }
 
     preLoadDiagram(): void {
-        this.lazyInjector
-            .getAll<ISelectionListener>(TYPES.ISelectionListener)
-            .forEach(listener =>
-                this.onSelectionChanged(change =>
-                    listener.selectionChanged(change.root, change.selectedElements, change.deselectedElements)
-                )
-            );
+        this.lazyInjector.getAll<ISelectionListener>(TYPES.ISelectionListener).forEach(listener => this.addListener(listener));
     }
 
     @preDestroy()
@@ -92,6 +94,12 @@ export class SelectionService implements IGModelRootListener, Disposable, IDiagr
         return this.onSelectionChangedEmitter.event;
     }
 
+    addListener(listener: ISelectionListener): Disposable {
+        return this.onSelectionChanged(change =>
+            listener.selectionChanged(change.root, change.selectedElements, change.deselectedElements)
+        );
+    }
+
     modelRootChanged(root: Readonly<GModelRoot>): void {
         this.updateSelection(root, [], []);
     }

diff --git a/packages/client/src/features/helper-lines/helper-line-manager-default.ts b/packages/client/src/features/helper-lines/helper-line-manager-default.ts
@@ -112,9 +112,7 @@ export class HelperLineManager implements IActionHandler, ISelectionListener, IH
             dynamicOptions.minimumMoveDelta = Point.multiplyScalar(this.grid, 2);
         }
         this.options = { ...DEFAULT_HELPER_LINE_OPTIONS, ...dynamicOptions, ...this.userOptions };
-        this.selectionService.onSelectionChanged(change =>
-            this.selectionChanged(change.root, change.selectedElements, change.deselectedElements)
-        );
+        this.selectionService.addListener(this);
     }
 
     handle(action: Action): void {

diff --git a/packages/client/src/features/tools/change-bounds/change-bounds-tool-move-feedback.ts b/packages/client/src/features/tools/change-bounds/change-bounds-tool-move-feedback.ts
@@ -13,16 +13,18 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import { Action, ElementMove, GModelElement, GModelRoot, MoveAction, Point, findParentByFeature } from '@eclipse-glsp/sprotty';
+import { Action, ElementMove, GModelElement, GModelRoot, MoveAction, Point, TypeGuard, findParentByFeature } from '@eclipse-glsp/sprotty';
 
 import { DebouncedFunc, debounce } from 'lodash';
 import { DragAwareMouseListener } from '../../../base/drag-aware-mouse-listener';
 import { CursorCSS, cursorFeedbackAction } from '../../../base/feedback/css-feedback';
 import { FeedbackEmitter } from '../../../base/feedback/feedback-emitter';
+import { ISelectionListener } from '../../../base/selection-service';
 import {
     MoveableElement,
     filter,
     getElements,
+    isNonRoutableMovableBoundsAware,
     isNonRoutableSelectedMovableBoundsAware,
     removeDescendants
 } from '../../../utils/gmodel-util';
@@ -38,7 +40,7 @@ import { ChangeBoundsTracker, TrackedMove } from './change-bounds-tracker';
  * the visual feedback but also the basis for sending the change to the server
  * (see also `tools/MoveTool`).
  */
-export class FeedbackMoveMouseListener extends DragAwareMouseListener {
+export class FeedbackMoveMouseListener extends DragAwareMouseListener implements ISelectionListener {
     protected rootElement?: GModelRoot;
     protected tracker: ChangeBoundsTracker;
     protected elementId2startPos = new Map<string, Point>();
@@ -58,6 +60,11 @@ export class FeedbackMoveMouseListener extends DragAwareMouseListener {
     override mouseDown(target: GModelElement, event: MouseEvent): Action[] {
         super.mouseDown(target, event);
         if (event.button === 0) {
+            if (this.tracker.isTracking()) {
+                // we have a move in progress that was not resolved yet (e.g., user may have triggered a mouse up outside the window)
+                this.draggingMouseUp(target, event);
+                return [];
+            }
             this.initializeMove(target, event);
             return [];
         }
@@ -105,6 +112,10 @@ export class FeedbackMoveMouseListener extends DragAwareMouseListener {
         return !!element && isNonRoutableSelectedMovableBoundsAware(element) && !(element instanceof GResizeHandle);
     }
 
+    protected isValidRevertable(element?: GModelElement): element is MoveableElement {
+        return !!element && isNonRoutableMovableBoundsAware(element) && !(element instanceof GResizeHandle);
+    }
+
     override nonDraggingMouseUp(element: GModelElement, event: MouseEvent): Action[] {
         // should reset everything that may have happend on mouse down
         this.moveInitializedFeedback.dispose();
@@ -161,8 +172,8 @@ export class FeedbackMoveMouseListener extends DragAwareMouseListener {
         return Array.from(topLevelElements);
     }
 
-    protected getElementsToMove(context: GModelElement): MoveableElement[] {
-        return getElements(context.root.index, Array.from(this.elementId2startPos.keys()), this.isValidMoveable);
+    protected getElementsToMove(context: GModelElement, moveable: TypeGuard<MoveableElement> = this.isValidMoveable): MoveableElement[] {
+        return getElements(context.root.index, Array.from(this.elementId2startPos.keys()), moveable);
     }
 
     protected resetElementPositions(context: GModelElement): MoveAction | undefined {
@@ -173,7 +184,7 @@ export class FeedbackMoveMouseListener extends DragAwareMouseListener {
     protected revertElementMoves(context?: GModelElement): ElementMove[] {
         const elementMoves: ElementMove[] = [];
         if (context?.root?.index) {
-            const movableElements = this.getElementsToMove(context);
+            const movableElements = this.getElementsToMove(context, this.isValidRevertable);
             movableElements.forEach(element =>
                 elementMoves.push({ elementId: element.id, toPosition: this.elementId2startPos.get(element.id)! })
             );
@@ -185,14 +196,14 @@ export class FeedbackMoveMouseListener extends DragAwareMouseListener {
         if (!this.tracker.isTracking()) {
             return [];
         }
-        const elementsToMove = this.getElementsToMove(target);
+        const elementToMove = this.getElementsToMove(target);
         if (!this.tool.movementOptions.allElementsNeedToBeValid) {
             // only reset the move of invalid elements, the others will be handled by the change bounds tool itself
-            elementsToMove
+            elementToMove
                 .filter(element => this.tool.changeBoundsManager.isValid(element))
                 .forEach(element => this.elementId2startPos.delete(element.id));
         } else {
-            if (elementsToMove.every(element => this.tool.changeBoundsManager.isValid(element))) {
+            if (elementToMove.length > 0 && elementToMove.every(element => this.tool.changeBoundsManager.isValid(element))) {
                 // do not reset any element as all are valid
                 this.elementId2startPos.clear();
             }
@@ -201,6 +212,10 @@ export class FeedbackMoveMouseListener extends DragAwareMouseListener {
         return [];
     }
 
+    selectionChanged(root: Readonly<GModelRoot>, selectedElements: string[], deselectedElements?: string[]): void {
+        this.dispose();
+    }
+
     override dispose(): void {
         this.pendingMoveInitialized?.cancel();
         this.moveInitializedFeedback.dispose();

diff --git a/packages/client/src/features/tools/change-bounds/change-bounds-tool.ts b/packages/client/src/features/tools/change-bounds/change-bounds-tool.ts
@@ -98,21 +98,23 @@ export class ChangeBoundsTool extends BaseEditTool {
     enable(): void {
         // install feedback move mouse listener for client-side move updates
         const feedbackMoveMouseListener = this.createMoveMouseListener();
+        this.toDisposeOnDisable.push(this.mouseTool.registerListener(feedbackMoveMouseListener));
         if (Disposable.is(feedbackMoveMouseListener)) {
             this.toDisposeOnDisable.push(feedbackMoveMouseListener);
         }
+        if (ISelectionListener.is(feedbackMoveMouseListener)) {
+            this.toDisposeOnDisable.push(this.selectionService.addListener(feedbackMoveMouseListener));
+        }
 
         // install change bounds listener for client-side resize updates and server-side updates
         const changeBoundsListener = this.createChangeBoundsListener();
+        this.toDisposeOnDisable.push(this.mouseTool.registerListener(changeBoundsListener));
         if (Disposable.is(changeBoundsListener)) {
             this.toDisposeOnDisable.push(changeBoundsListener);
         }
-
-        this.toDisposeOnDisable.push(
-            this.mouseTool.registerListener(feedbackMoveMouseListener),
-            this.mouseTool.registerListener(changeBoundsListener),
-            this.selectionService.onSelectionChanged(change => changeBoundsListener.selectionChanged(change.root, change.selectedElements))
-        );
+        if (ISelectionListener.is(changeBoundsListener)) {
+            this.toDisposeOnDisable.push(this.selectionService.addListener(changeBoundsListener));
+        }
     }
 
     createChangeBoundsTracker(): ChangeBoundsTracker {
@@ -123,7 +125,7 @@ export class ChangeBoundsTool extends BaseEditTool {
         return new FeedbackMoveMouseListener(this);
     }
 
-    protected createChangeBoundsListener(): MouseListener & ISelectionListener {
+    protected createChangeBoundsListener(): MouseListener {
         return new ChangeBoundsListener(this);
     }
 }

diff --git a/packages/client/src/features/tools/edge-edit/edge-edit-tool.ts b/packages/client/src/features/tools/edge-edit/edge-edit-tool.ts
@@ -82,7 +82,7 @@ export class EdgeEditTool extends BaseEditTool {
             this.feedbackEdgeSourceMovingListener,
             this.feedbackEdgeTargetMovingListener,
             this.feedbackMovingListener,
-            this.selectionService.onSelectionChanged(change => this.edgeEditListener.selectionChanged(change.root, change.selectedElements))
+            this.selectionService.addListener(this.edgeEditListener)
         );
     }
 

diff --git a/packages/client/src/utils/gmodel-util.ts b/packages/client/src/utils/gmodel-util.ts
@@ -196,14 +196,28 @@ export function isNonRoutableSelectedMovableBoundsAware(element: GModelElement):
     return isNonRoutableSelectedBoundsAware(element) && isMoveable(element);
 }
 
+export function isNonRoutableMovableBoundsAware(element: GModelElement): element is BoundsAwareModelElement {
+    return isNonRoutableBoundsAware(element) && isMoveable(element);
+}
+
 /**
  * A typeguard function to check wether a given {@link GModelElement} implements the {@link BoundsAware} model feature,
  * the {@link Selectable} model feature and is actually selected. In addition, the element must not be a {@link GRoutableElement}.
  * @param element The element to check.
  * @returns A type predicate indicating wether the element is of type {@link SelectableBoundsAware}.
  */
 export function isNonRoutableSelectedBoundsAware(element: GModelElement): element is SelectableBoundsAware {
-    return isBoundsAware(element) && isSelected(element) && !isRoutable(element);
+    return isNonRoutableBoundsAware(element) && isSelected(element);
+}
+
+/**
+ * A typeguard function to check wether a given {@link GModelElement} implements the {@link BoundsAware} model feature.
+ * In addition, the element must not be a {@link GRoutableElement}.
+ * @param element The element to check.
+ * @returns A type predicate indicating wether the element is of type {@link BoundsAwareModelElement}.
+ */
+export function isNonRoutableBoundsAware(element: GModelElement): element is BoundsAwareModelElement {
+    return isBoundsAware(element) && !isRoutable(element);
 }
 
 /**