Skip to content

Commit

Permalink
PR Feedback
Browse files Browse the repository at this point in the history
- Fix issue with wrong coordinates on project bars during move/resize
- Fix move during resize for constrained, symmetric resize
- Ensure symmetric resize does not trigger deselect of element

- Remove !important from CSS rules
- Allow undefined unsnap modifier for adopters
- Use isSizeable for debug decoration to avoid wrong edge decoration
- Remove non-generic getAbsoluteEdgeBounds
- Better unify the helper functions for CSS classes
- Fix deprecation warnings
- Minor cleanup

Contributed on behalf of Axon Ivy AG
  • Loading branch information
martin-fleck-at committed May 16, 2024
1 parent 0b1c053 commit 7c15c61
Show file tree
Hide file tree
Showing 25 changed files with 316 additions and 198 deletions.
13 changes: 11 additions & 2 deletions packages/client/css/change-bounds.css
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,19 @@
}

.sprotty-resize-handle.resize-not-allowed {
fill: var(--glsp-error-foreground) !important;
fill: var(--glsp-error-foreground);
}

.sprotty g .resize-not-allowed > .sprotty-node {
stroke: var(--glsp-error-foreground) !important;
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;
}
6 changes: 3 additions & 3 deletions packages/client/css/helper-lines.css
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@

.helper-line {
pointer-events: none;
stroke: red;
stroke-width: 1px;
stroke: #f7086c;
stroke-width: 0.5;
opacity: 1;
}

Expand All @@ -28,6 +28,6 @@
stroke-linejoin: miter;
stroke-linecap: round;
stroke: darkblue;
stroke-width: 1px;
stroke-width: 0.5;
stroke-dasharray: 2;
}
9 changes: 6 additions & 3 deletions packages/client/src/base/feedback/feeback-emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ 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 {
Expand Down Expand Up @@ -116,20 +116,22 @@ export class MoveKeyListener extends KeyListener implements AccessibleKeyShortcu
}

protected matchesMoveUpKeystroke(event: KeyboardEvent): boolean {
return matchesKeystroke(event, 'ArrowUp') || matchesKeystroke(event, 'ArrowUp', this.tool.changeBoundsManager.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', this.tool.changeBoundsManager.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', this.tool.changeBoundsManager.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', this.tool.changeBoundsManager.unsnapModifier());
const unsnap = this.tool.changeBoundsManager.unsnapModifier();
return matchesKeystroke(event, 'ArrowLeft') || (!!unsnap && matchesKeystroke(event, 'ArrowLeft', unsnap));
}
}
2 changes: 2 additions & 0 deletions packages/client/src/features/bounds/bounds-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
2 changes: 1 addition & 1 deletion packages/client/src/features/bounds/layout-data.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/********************************************************************************
* Copyright (c) 2024 EclipseSource and others.
* Copyright (c) 2024 Axon Ivy AG 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
/* eslint-disable import/no-deprecated */
/* eslint-disable deprecation/deprecation */

import { GModelElement } from '@eclipse-glsp/sprotty';
import { expect } from 'chai';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
/* eslint-disable @typescript-eslint/no-shadow */
/* eslint-disable import/no-deprecated */
/* eslint-disable deprecation/deprecation */

import { GModelElement, ISnapper, Point, Writable } from '@eclipse-glsp/sprotty';
import { calculateDeltaBetweenPoints } from '../../utils/gmodel-util';
import { isMouseEvent } from '../../utils/html-utils';
Expand Down
2 changes: 1 addition & 1 deletion packages/client/src/features/change-bounds/tracker.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/********************************************************************************
* Copyright (c) 2024 EclipseSource and others.
* Copyright (c) 2024 Axon Ivy AG 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
Expand Down
8 changes: 4 additions & 4 deletions packages/client/src/features/debug/debug-bounds-decorator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
********************************************************************************/
/* eslint-disable max-len */

import { Bounds, GModelElement, IVNodePostprocessor, Point, TYPES, isBoundsAware, setClass, svg } from '@eclipse-glsp/sprotty';
import { Bounds, GModelElement, IVNodePostprocessor, Point, TYPES, isSizeable, setClass, svg } from '@eclipse-glsp/sprotty';
import { inject, injectable, optional } from 'inversify';
import { VNode } from 'snabbdom';
import { GGraph } from '../../model';
Expand All @@ -33,8 +33,8 @@ export class DebugBoundsDecorator implements IVNodePostprocessor {
if (!this.debugManager?.isDebugEnabled) {
return vnode;
}
if (isBoundsAware(element)) {
this.decorateBoundsAware(vnode, element);
if (isSizeable(element)) {
this.decorateSizeable(vnode, element);
}
if (element instanceof GGraph) {
this.decorateGraph(vnode, element);
Expand Down Expand Up @@ -67,7 +67,7 @@ export class DebugBoundsDecorator implements IVNodePostprocessor {
);
}

protected decorateBoundsAware(vnode: VNode, element: BoundsAwareModelElement): void {
protected decorateSizeable(vnode: VNode, element: BoundsAwareModelElement): void {
setClass(vnode, 'debug-bounds', true);
vnode.children?.push(this.renderTopLeftCorner(element));
vnode.children?.push(this.renderTopRightCorner(element));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,15 @@ import { Action, Dimension, GModelElement, MoveAction, Point, isBoundsAware, isM
import { DragAwareMouseListener } from '../../base/drag-aware-mouse-listener';
import { CSS_HIDDEN, ModifyCSSFeedbackAction } from '../../base/feedback/css-feedback';
import { FeedbackEmitter } from '../../base/feedback/feeback-emitter';
import { getAbsolutePosition } from '../../utils';
import { ChangeBoundsManager, ChangeBoundsTracker, FeedbackAwareTool, MoveFinishedEventAction, TrackedElementMove } from '../tools';
import { MoveableElement, getAbsolutePosition } from '../../utils';
import {
CSS_RESIZE_MODE,
ChangeBoundsManager,
ChangeBoundsTracker,
FeedbackAwareTool,
MoveFinishedEventAction,
TrackedElementMove
} from '../tools';

export interface PositioningTool extends FeedbackAwareTool {
readonly changeBoundsManager: ChangeBoundsManager;
Expand All @@ -39,42 +46,56 @@ export class MouseTrackingElementPositionListener extends DragAwareMouseListener
this.moveGhostFeedback = this.tool.createFeedbackEmitter();
}

protected getTrackedElement(target: GModelElement, event: MouseEvent): MoveableElement | undefined {
const element = target.root.index.getById(this.elementId);
return !element || !isMoveable(element) ? undefined : element;
}

override mouseMove(target: GModelElement, event: MouseEvent): Action[] {
super.mouseMove(target, event);
const element = target.root.index.getById(this.elementId);
if (!element || !isMoveable(element)) {
const element = this.getTrackedElement(target, event);
if (!element) {
return [];
}
if (!this.tracker.isTracking()) {
this.tracker.startTracking();
const mousePosition = getAbsolutePosition(target, event);
element.position =
this.cursorPosition === 'middle' && isBoundsAware(element)
? Point.subtract(mousePosition, Dimension.center(element.bounds))
: mousePosition;
this.initialize(element, target, event);
}
// we only validate if we use the position already as a target, otherwise we need to re-evaluate after moving to the center anyway
const move = this.tracker.moveElements([element], { snap: event, restrict: event, validate: true });
const elementMove = move.elementMoves[0];
if (!elementMove) {
return [];
}
this.addMoveFeeback(elementMove);
// since we are moving a ghost element that is feedback-only and will be removed anyway,
// we just send a MoveFinishedEventAction instead of reseting the position with a MoveAction and the finished flag set to true.
this.moveGhostFeedback
.add(
MoveAction.create([{ elementId: this.elementId, toPosition: elementMove.toPosition }], { animate: false }),
MoveFinishedEventAction.create()
)
.submit();
this.moveGhostFeedback.add(
MoveAction.create([{ elementId: this.elementId, toPosition: elementMove.toPosition }], { animate: false }),
MoveFinishedEventAction.create()
);
this.addMoveFeeback(elementMove);
this.moveGhostFeedback.submit();
this.tracker.updateTrackingPosition(elementMove.moveVector);
return [];
}

protected initialize(element: MoveableElement, target: GModelElement, event: MouseEvent): void {
this.tracker.startTracking();
element.position = this.initializeElementPosition(element, target, event);
}

protected initializeElementPosition(element: MoveableElement, target: GModelElement, event: MouseEvent): Point {
const mousePosition = getAbsolutePosition(target, event);
return this.cursorPosition === 'middle' && isBoundsAware(element)
? Point.subtract(mousePosition, Dimension.center(element.bounds))
: mousePosition;
}

protected addMoveFeeback(move: TrackedElementMove): void {
this.tool.changeBoundsManager.addRestrictionFeedback(this.moveGhostFeedback, move);
this.moveGhostFeedback.add(ModifyCSSFeedbackAction.create({ elements: [move.element.id], remove: [CSS_HIDDEN] }));
this.moveGhostFeedback.add(
ModifyCSSFeedbackAction.create({ add: [CSS_RESIZE_MODE] }),
ModifyCSSFeedbackAction.create({ remove: [CSS_RESIZE_MODE] })
);
}

override dispose(): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
Point,
TYPES,
Viewport,
equalUpTo,
findParentByFeature,
isBoundsAware,
isDecoration,
Expand Down Expand Up @@ -293,7 +294,7 @@ export class DrawHelperLinesFeedbackCommand extends FeedbackCommand {
}

protected isMatch(leftCoordinate: number, rightCoordinate: number, epsilon: number): boolean {
return Math.abs(leftCoordinate - rightCoordinate) <= epsilon;
return equalUpTo(leftCoordinate, rightCoordinate, epsilon);
}

protected log(message: string, ...params: any[]): void {
Expand Down
16 changes: 15 additions & 1 deletion packages/client/src/features/select/select-mouse-listener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,23 @@
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
import { Action, BringToFrontAction, GModelElement, SelectAction, SelectMouseListener } from '@eclipse-glsp/sprotty';
import { Action, BringToFrontAction, GModelElement, SelectAction, SelectMouseListener, TYPES } from '@eclipse-glsp/sprotty';
import { inject, injectable, optional } from 'inversify';
import { Ranked } from '../../base/ranked';
import { SelectableElement } from '../../utils';
import { SResizeHandle } from '../change-bounds';
import { ChangeBoundsManager } from '../tools';

/**
* Ranked select mouse listener that is executed before default mouse listeners when using the RankedMouseTool.
* This ensures that default mouse listeners are working on a model that has selection changes already applied.
*/
@injectable()
export class RankedSelectMouseListener extends SelectMouseListener implements Ranked {
rank: number = Ranked.DEFAULT_RANK - 100; /* we want to be executed before all default mouse listeners */

@inject(TYPES.IChangeBoundsManager) @optional() readonly changeBoundsManager?: ChangeBoundsManager;

protected override handleSelectTarget(
selectableTarget: SelectableElement,
deselectedElements: GModelElement[],
Expand Down Expand Up @@ -52,4 +58,12 @@ export class RankedSelectMouseListener extends SelectMouseListener implements Ra
result.push(SelectAction.create({ selectedElementsIDs: [], deselectedElementsIDs: deselectedElements.map(e => e.id) }));
return result;
}

protected override handleButton(target: GModelElement, event: MouseEvent): (Action | Promise<Action>)[] | undefined {
if (target instanceof SResizeHandle && this.changeBoundsManager?.useSymmetricResize(event)) {
// avoid de-selecting elements when resizing with a modifier key
return [];
}
return super.handleButton(target, event);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/********************************************************************************
* Copyright (c) 2024 EclipseSource and others.
* Copyright (c) 2024 Axon Ivy AG 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
Expand Down Expand Up @@ -40,6 +40,8 @@ import {
import { IHelperLineManager } from '../../helper-lines';
import { ChangeBoundsTracker, TrackedElementMove } from './change-bounds-tracker';

export const CSS_RESIZE_MODE = 'resize-mode';

@injectable()
export class ChangeBoundsManager {
constructor(
Expand All @@ -49,7 +51,7 @@ export class ChangeBoundsManager {
@optional() @inject(TYPES.IHelperLineManager) readonly helperLineManager?: IHelperLineManager
) {}

unsnapModifier(): KeyboardModifier {
unsnapModifier(): KeyboardModifier | undefined {
return 'shift';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
import { Action, Disposable, ElementMove, GModelElement, GModelRoot, MoveAction, Point, findParentByFeature } from '@eclipse-glsp/sprotty';
import { Action, ElementMove, GModelElement, GModelRoot, MoveAction, Point, findParentByFeature } from '@eclipse-glsp/sprotty';

import { DebouncedFunc, debounce } from 'lodash';
import { ChangeBoundsTracker, TrackedMove } from '.';
Expand All @@ -32,7 +32,7 @@ import { MoveFinishedEventAction, MoveInitializedEventAction } from './change-bo
* the visual feedback but also the basis for sending the change to the server
* (see also `tools/MoveTool`).
*/
export class FeedbackMoveMouseListener extends DragAwareMouseListener implements Disposable {
export class FeedbackMoveMouseListener extends DragAwareMouseListener {
protected rootElement?: GModelRoot;
protected tracker: ChangeBoundsTracker;
protected elementId2startPos = new Map<string, Point>();
Expand Down
Loading

0 comments on commit 7c15c61

Please sign in to comment.