Improve handling of change bounds in several areas

packages/client/css/change-bounds.css

@@ -2,14 +2,48 @@
     cursor: nw-resize;
 }
 
+.sprotty-resize-handle[data-kind='top'] {
+    cursor: n-resize;
+}
+
 .sprotty-resize-handle[data-kind='top-right'] {
     cursor: ne-resize;
 }
 
-.sprotty-resize-handle[data-kind='bottom-left'] {
-    cursor: sw-resize;
+.sprotty-resize-handle[data-kind='right'] {
+    cursor: e-resize;
 }
 
 .sprotty-resize-handle[data-kind='bottom-right'] {
     cursor: se-resize;
 }
+
+.sprotty-resize-handle[data-kind='bottom'] {
+    cursor: s-resize;
+}
+
+.sprotty-resize-handle[data-kind='bottom-left'] {
+    cursor: sw-resize;
+}
+
+.sprotty-resize-handle[data-kind='left'] {
+    cursor: w-resize;
+}
+
+.sprotty-resize-handle.resize-not-allowed {
+    fill: var(--glsp-error-foreground);
+}
+
+.sprotty g .resize-not-allowed > .sprotty-node {
+    stroke: var(--glsp-error-foreground);
+    stroke-width: 1.5px;
+}
+
+.move-mode .sprotty-projection-bar,
+.resize-mode .sprotty-projection-bar {
+    /**
+     * We are using mouse events (offsetX, offsetY) in the GLSPMousePositionTracker to calculate the diagram position relative to the parent.
+     * Other elements result in relative coordinates different from the graph and will therefore interfere with the correct position calculation.
+     */
+    pointer-events: none;
+}

packages/client/css/glsp-sprotty.css

@@ -152,18 +152,34 @@
     cursor: nw-resize;
 }
 
+.sprotty .resize-w-mode {
+    cursor: n-resize;
+}
+
 .sprotty .resize-ne-mode {
     cursor: ne-resize;
 }
 
-.sprotty .resize-sw-mode {
-    cursor: sw-resize;
+.sprotty .resize-e-mode {
+    cursor: e-resize;
 }
 
 .sprotty .resize-se-mode {
     cursor: se-resize;
 }
 
+.sprotty .resize-s-mode {
+    cursor: s-resize;
+}
+
+.sprotty .resize-sw-mode {
+    cursor: sw-resize;
+}
+
+.sprotty .resize-w-mode {
+    cursor: w-resize;
+}
+
 .sprotty .element-deletion-mode {
     cursor: pointer;
 }

packages/client/css/helper-lines.css

@@ -16,8 +16,8 @@
 
 .helper-line {
     pointer-events: none;
-    stroke: red;
-    stroke-width: 1px;
+    stroke: #f7086c;
+    stroke-width: 0.5;
     opacity: 1;
 }
 
@@ -28,6 +28,6 @@
     stroke-linejoin: miter;
     stroke-linecap: round;
     stroke: darkblue;
-    stroke-width: 1px;
+    stroke-width: 0.5;
     stroke-dasharray: 2;
 }

packages/client/src/base/default.module.ts

@@ -18,6 +18,7 @@ import {
     FeatureModule,
     KeyTool,
     LocationPostprocessor,
+    MousePositionTracker,
     MouseTool,
     MoveCommand,
     SetDirtyStateAction,
@@ -47,6 +48,7 @@ import { DiagramLoader } from './model/diagram-loader';
 import { GLSPModelSource } from './model/glsp-model-source';
 import { DefaultModelInitializationConstraint, ModelInitializationConstraint } from './model/model-initialization-constraint';
 import { GModelRegistry } from './model/model-registry';
+import { GLSPMousePositionTracker } from './mouse-position-tracker';
 import { SelectionClearingMouseListener } from './selection-clearing-mouse-listener';
 import { SelectionService } from './selection-service';
 import { EnableDefaultToolsAction, EnableToolsAction } from './tool-manager/tool';
@@ -85,7 +87,8 @@ export const defaultModule = new FeatureModule((bind, unbind, isBound, rebind, .
     bind(GLSPMouseTool).toSelf().inSingletonScope();
     bindOrRebind(context, MouseTool).toService(GLSPMouseTool);
     bind(TYPES.IDiagramStartup).toService(GLSPMouseTool);
-
+    bind(GLSPMousePositionTracker).toSelf().inSingletonScope();
+    bindOrRebind(context, MousePositionTracker).toService(GLSPMousePositionTracker);
     bind(GLSPKeyTool).toSelf().inSingletonScope();
     bindOrRebind(context, KeyTool).toService(GLSPKeyTool);
     bind(TYPES.IDiagramStartup).toService(GLSPKeyTool);

packages/client/src/base/feedback/css-feedback.ts

@@ -85,14 +85,18 @@ export enum CursorCSS {
     RESIZE_NESW = 'resize-nesw-mode',
     RESIZE_NWSE = 'resize-nwse-mode',
     RESIZE_NW = 'resize-nw-mode',
+    RESIZE_N = 'resize-n-mode',
     RESIZE_NE = 'resize-ne-mode',
-    RESIZE_SW = 'resize-sw-mode',
+    RESIZE_E = 'resize-e-mode',
     RESIZE_SE = 'resize-se-mode',
+    RESIZE_S = 'resize-s-mode',
+    RESIZE_SW = 'resize-sw-mode',
+    RESIZE_W = 'resize-w-mode',
     MOVE = 'move-mode',
     MARQUEE = 'marquee-mode'
 }
 
-export function cursorFeedbackAction(cursorCss?: CursorCSS): ModifyCSSFeedbackAction {
+export function cursorFeedbackAction(cursorCss?: string): ModifyCSSFeedbackAction {
     const add = [];
     if (cursorCss) {
         add.push(cursorCss);
@@ -107,3 +111,7 @@ export function applyCssClasses(element: GModelElement, ...add: string[]): Modif
 export function deleteCssClasses(element: GModelElement, ...remove: string[]): ModifyCSSFeedbackAction {
     return ModifyCSSFeedbackAction.create({ elements: [element], remove });
 }
+
+export function toggleCssClasses(element: GModelElement, add: boolean, ...cssClasses: string[]): ModifyCSSFeedbackAction {
+    return add ? applyCssClasses(element, ...cssClasses) : deleteCssClasses(element, ...cssClasses);
+}

packages/client/src/base/feedback/feeback-emitter.ts

@@ -32,9 +32,12 @@ export class FeedbackEmitter implements IFeedbackEmitter, Disposable {
      * once the {@link submit} method is called.
      *
      * @param action feedback action
-     * @param cleanupAction action that undoes the feedback action. This is only triggered when {@link revert} is called.
+     * @param cleanupAction action that undoes the feedback action. This is only triggered when {@link revert} or {@link dispose} is called.
      */
-    add(action: Action, cleanupAction?: MaybeActions): this {
+    add(action?: Action, cleanupAction?: MaybeActions): this {
+        if (!action && !cleanupAction) {
+            return this;
+        }
         const idx = this.feedbackActions.length;
         this.feedbackActions[idx] = action;
         if (cleanupAction) {
@@ -73,7 +76,7 @@ export class FeedbackEmitter implements IFeedbackEmitter, Disposable {
      * Registers any pending actions as feedback. Any previously submitted feedback becomes invalid.
      */
     submit(): this {
-        // with 'arrayOf' we skip undefined entries that are created for non-cleanup actions
+        // with 'arrayOf' we skip undefined entries that are created for non-cleanup actions or cleanup-only actions
         const actions = arrayOf(...this.feedbackActions);
         const cleanupActions = arrayOf(...this.cleanupActions);
         this.deregistration = this.feedbackDispatcher.registerFeedback(this, actions, () => cleanupActions.flatMap(MaybeActions.asArray));

packages/client/src/base/index.ts

@@ -24,6 +24,7 @@ export * from './editor-context-service';
 export * from './feedback';
 export * from './focus';
 export * from './model';
+export * from './mouse-position-tracker';
 export * from './ranked';
 export * from './selection-clearing-mouse-listener';
 export * from './selection-service';

packages/client/src/base/mouse-position-tracker.ts

@@ -0,0 +1,25 @@
+/********************************************************************************
+ * Copyright (c) 2024 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
+    rank: number;
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+import { MousePositionTracker } from '@eclipse-glsp/sprotty';
+import { injectable } from 'inversify';
+import { Ranked } from './ranked';
+
+@injectable()
+export class GLSPMousePositionTracker extends MousePositionTracker implements Ranked {
+    /* we want to be executed before all default mouse listeners since we are just tracking the position and others may need it */
+    rank = Ranked.DEFAULT_RANK - 200;
+}

packages/client/src/features/accessibility/view-key-tools/movement-key-tool.ts

@@ -20,8 +20,8 @@ import { matchesKeystroke } from 'sprotty/lib/utils/keyboard';
 import { GLSPActionDispatcher } from '../../../base/action-dispatcher';
 import { SelectionService } from '../../../base/selection-service';
 import { Tool } from '../../../base/tool-manager/tool';
-import { unsnapModifier, useSnap } from '../../change-bounds/snap';
 import { Grid } from '../../grid';
+import { ChangeBoundsManager } from '../../tools';
 import { AccessibleKeyShortcutProvider, SetAccessibleKeyShortcutAction } from '../key-shortcut/accessible-key-shortcut';
 import { MoveElementAction, MoveViewportAction } from '../move-zoom/move-handler';
 
@@ -40,7 +40,8 @@ export class MovementKeyTool implements Tool {
     @inject(SelectionService) selectionService: SelectionService;
     @inject(TYPES.ISnapper) @optional() readonly snapper?: ISnapper;
     @inject(TYPES.IActionDispatcher) readonly actionDispatcher: GLSPActionDispatcher;
-    @optional() @inject(TYPES.Grid) protected grid: Grid;
+    @inject(TYPES.Grid) @optional() protected grid: Grid;
+    @inject(TYPES.IChangeBoundsManager) readonly changeBoundsManager: ChangeBoundsManager;
 
     get id(): string {
         return MovementKeyTool.ID;
@@ -86,7 +87,7 @@ export class MoveKeyListener extends KeyListener implements AccessibleKeyShortcu
 
     override keyDown(_element: GModelElement, event: KeyboardEvent): Action[] {
         const selectedElementIds = this.tool.selectionService.getSelectedElementIDs();
-        const snap = useSnap(event);
+        const snap = this.tool.changeBoundsManager.usePositionSnap(event);
         const offsetX = snap ? this.grid.x : 1;
         const offsetY = snap ? this.grid.y : 1;
 
@@ -115,18 +116,22 @@ export class MoveKeyListener extends KeyListener implements AccessibleKeyShortcu
     }
 
     protected matchesMoveUpKeystroke(event: KeyboardEvent): boolean {
-        return matchesKeystroke(event, 'ArrowUp') || matchesKeystroke(event, 'ArrowUp', unsnapModifier());
+        const unsnap = this.tool.changeBoundsManager.unsnapModifier();
+        return matchesKeystroke(event, 'ArrowUp') || (!!unsnap && matchesKeystroke(event, 'ArrowUp', unsnap));
     }
 
     protected matchesMoveDownKeystroke(event: KeyboardEvent): boolean {
-        return matchesKeystroke(event, 'ArrowDown') || matchesKeystroke(event, 'ArrowDown', unsnapModifier());
+        const unsnap = this.tool.changeBoundsManager.unsnapModifier();
+        return matchesKeystroke(event, 'ArrowDown') || (!!unsnap && matchesKeystroke(event, 'ArrowDown', unsnap));
     }
 
     protected matchesMoveRightKeystroke(event: KeyboardEvent): boolean {
-        return matchesKeystroke(event, 'ArrowRight') || matchesKeystroke(event, 'ArrowRight', unsnapModifier());
+        const unsnap = this.tool.changeBoundsManager.unsnapModifier();
+        return matchesKeystroke(event, 'ArrowRight') || (!!unsnap && matchesKeystroke(event, 'ArrowRight', unsnap));
     }
 
     protected matchesMoveLeftKeystroke(event: KeyboardEvent): boolean {
-        return matchesKeystroke(event, 'ArrowLeft') || matchesKeystroke(event, 'ArrowLeft', unsnapModifier());
+        const unsnap = this.tool.changeBoundsManager.unsnapModifier();
+        return matchesKeystroke(event, 'ArrowLeft') || (!!unsnap && matchesKeystroke(event, 'ArrowLeft', unsnap));
     }
 }

packages/client/src/features/bounds/bounds-module.ts

@@ -52,5 +52,7 @@ export const boundsModule = new FeatureModule((bind, _unbind, isBound, _rebind)
     configureLayout(context, HBoxLayouter.KIND, HBoxLayouterExt);
     configureLayout(context, FreeFormLayouter.KIND, FreeFormLayouter);
 
+    // backwards compatibility
+    // eslint-disable-next-line deprecation/deprecation
     bind(PositionSnapper).toSelf();
 });

packages/client/src/features/bounds/freeform-layout.ts

@@ -13,19 +13,20 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import { injectable } from 'inversify';
 import {
     AbstractLayout,
     AbstractLayoutOptions,
     Bounds,
     BoundsData,
     Dimension,
     GChildElement,
+    GParentElement,
     LayoutContainer,
     Point,
-    GParentElement,
     StatefulLayouter
 } from '@eclipse-glsp/sprotty';
+import { injectable } from 'inversify';
+import { LayoutAware } from './layout-data';
 
 /**
  * Layouts children of a container with explicit X/Y positions
@@ -44,9 +45,11 @@ export class FreeFormLayouter extends AbstractLayout<AbstractLayoutOptions> {
 
         const maxWidth = childrenSize.width > 0 ? childrenSize.width + options.paddingLeft + options.paddingRight : 0;
         const maxHeight = childrenSize.height > 0 ? childrenSize.height + options.paddingTop + options.paddingBottom : 0;
-        if (maxWidth > 0 && maxHeight > 0) {
+        if (childrenSize.width > 0 && childrenSize.height > 0) {
             const offset = this.layoutChildren(container, layouter, options, maxWidth, maxHeight);
-            boundsData.bounds = this.getFinalContainerBounds(container, offset, options, maxWidth, maxHeight);
+            const computed = this.getComputedContainerDimensions(options, childrenSize.width, childrenSize.height);
+            LayoutAware.setComputedDimensions(boundsData, computed);
+            boundsData.bounds = this.getFinalContainerBounds(container, offset, options, computed.width, computed.height);
             boundsData.boundsChanged = true;
         } else {
             boundsData.bounds = { x: boundsData.bounds!.x, y: boundsData.bounds!.y, width: 0, height: 0 };
@@ -96,6 +99,13 @@ export class FreeFormLayouter extends AbstractLayout<AbstractLayoutOptions> {
         return currentOffset;
     }
 
+    protected getComputedContainerDimensions(options: AbstractLayoutOptions, maxWidth: number, maxHeight: number): Dimension {
+        return {
+            width: maxWidth + options.paddingLeft + options.paddingRight,
+            height: maxHeight + options.paddingTop + options.paddingBottom
+        };
+    }
+
     protected override getFinalContainerBounds(
         container: GParentElement & LayoutContainer,
         lastOffset: Point,
@@ -106,8 +116,8 @@ export class FreeFormLayouter extends AbstractLayout<AbstractLayoutOptions> {
         const result = {
             x: container.bounds.x,
             y: container.bounds.y,
-            width: Math.max(options.minWidth, maxWidth + options.paddingLeft + options.paddingRight),
-            height: Math.max(options.minHeight, maxHeight + options.paddingTop + options.paddingBottom)
+            width: Math.max(options.minWidth, maxWidth),
+            height: Math.max(options.minHeight, maxHeight)
         };
 
         return result;