Skip to content

Commit

Permalink
211: Revise TypeHints and server side feedback for creation actions
Browse files Browse the repository at this point in the history
- Add a new parameter to EdgeTypeHint, 'dynamic', indicating that new
edges need to check with the server before allowing creation
- Add a new Request/Response to implement this check
- Update the EdgeCreationTool to check with the server when trying to
create a new edge configured as Dynamic
  • Loading branch information
CamilleLetavernier committed Sep 11, 2023
1 parent 256d64d commit b98c98b
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 16 deletions.
11 changes: 10 additions & 1 deletion packages/client/src/features/hints/type-hints-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,22 @@
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
import { FeatureModule, SetTypeHintsAction, TYPES, bindAsService, configureActionHandler, configureCommand } from '~glsp-sprotty';
import {
CheckEdgeTargetResultAction,
FeatureModule,
SetTypeHintsAction,
TYPES,
bindAsService,
configureActionHandler,
configureCommand
} from '~glsp-sprotty';
import { ApplyTypeHintsCommand, TypeHintProvider } from './type-hints';

export const typeHintsModule = new FeatureModule((bind, unbind, isBound) => {
const context = { bind, unbind, isBound };
bindAsService(context, TYPES.ITypeHintProvider, TypeHintProvider);
bind(TYPES.IDiagramStartup).toService(TypeHintProvider);
configureActionHandler(context, SetTypeHintsAction.KIND, TypeHintProvider);
configureActionHandler(context, CheckEdgeTargetResultAction.KIND, TypeHintProvider);
configureCommand(context, ApplyTypeHintsCommand);
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,22 @@ import { inject, injectable } from 'inversify';
import {
Action,
AnchorComputerRegistry,
CheckEdgeTargetResultAction,
CreateEdgeOperation,
IActionDispatcher,
RequestCheckEdgeTargetAction,
SEdge,
SModelElement,
TYPES,
TriggerEdgeCreationAction,
findParentByFeature,
isConnectable,
isCtrlOrCmd
} from '~glsp-sprotty';
import { DragAwareMouseListener } from '../../../base/drag-aware-mouse-listener';

import { CursorCSS, cursorFeedbackAction } from '../../../base/feedback/css-feedback';
import { EnableDefaultToolsAction } from '../../../base/tool-manager/tool';
import { ITypeHintProvider } from '../../hints/type-hints';
import { BaseCreationTool } from '../base-tools';
import { DrawFeedbackEdgeAction, RemoveFeedbackEdgeAction } from './dangling-edge-feedback';
import { FeedbackEdgeEndMovingMouseListener } from './edge-creation-tool-feedback';
Expand All @@ -42,6 +46,8 @@ export class EdgeCreationTool extends BaseCreationTool<TriggerEdgeCreationAction

@inject(AnchorComputerRegistry) protected anchorRegistry: AnchorComputerRegistry;

@inject(TYPES.ITypeHintProvider) protected typeHintProvider: ITypeHintProvider;

protected isTriggerAction = TriggerEdgeCreationAction.is;

get id(): string {
Expand All @@ -52,7 +58,9 @@ export class EdgeCreationTool extends BaseCreationTool<TriggerEdgeCreationAction
const mouseMovingFeedback = new FeedbackEdgeEndMovingMouseListener(this.anchorRegistry, this.feedbackDispatcher);
this.toDisposeOnDisable.push(
mouseMovingFeedback,
this.mouseTool.registerListener(new EdgeCreationToolMouseListener(this.triggerAction, this)),
this.mouseTool.registerListener(
new EdgeCreationToolMouseListener(this.triggerAction, this.actionDispatcher, this.typeHintProvider, this)
),
this.mouseTool.registerListener(mouseMovingFeedback),
this.registerFeedback([cursorFeedbackAction(CursorCSS.OPERATION_NOT_ALLOWED)], this, [
RemoveFeedbackEdgeAction.create(),
Expand All @@ -70,7 +78,12 @@ export class EdgeCreationToolMouseListener extends DragAwareMouseListener {
protected allowedTarget = false;
protected proxyEdge: SEdge;

constructor(protected triggerAction: TriggerEdgeCreationAction, protected tool: EdgeCreationTool) {
constructor(
protected triggerAction: TriggerEdgeCreationAction,
protected actionDispatcher: IActionDispatcher,
protected typeHintProvider: ITypeHintProvider,
protected tool: EdgeCreationTool
) {
super();
this.proxyEdge = new SEdge();
this.proxyEdge.type = triggerAction.elementTypeId;
Expand Down Expand Up @@ -128,33 +141,74 @@ export class EdgeCreationToolMouseListener extends DragAwareMouseListener {
return this.target !== undefined;
}

override mouseOver(target: SModelElement, event: MouseEvent): Action[] {
override mouseOver(target: SModelElement, event: MouseEvent): (Action | Promise<Action>)[] {
const newCurrentTarget = findParentByFeature(target, isConnectable);
if (newCurrentTarget !== this.currentTarget) {
this.currentTarget = newCurrentTarget;
if (this.currentTarget) {
if (!this.isSourceSelected()) {
this.allowedTarget = this.isAllowedSource(newCurrentTarget);
} else if (!this.isTargetSelected()) {
this.allowedTarget = this.isAllowedTarget(newCurrentTarget);
}
if (this.allowedTarget) {
const action = !this.isSourceSelected()
? cursorFeedbackAction(CursorCSS.EDGE_CREATION_SOURCE)
: cursorFeedbackAction(CursorCSS.EDGE_CREATION_TARGET);
return [action];
// Temporarily mark the target as invalid while we check for a proper result,
// so a fast-clicking user doesn't get a chance to create the edge in the meantime.
this.allowedTarget = false;
const actions = this.isAllowedTarget(newCurrentTarget).then(allowedCheck => {
// Make sure we didn't change the target element while
// checking for valid target.
if (allowedCheck.targetElement === this.currentTarget) {
this.allowedTarget = allowedCheck.allowed;
return this.updateEdgeFeedback();
}
// FIXME Is there a proper no-op action? We can't return <undefined> or [] here
// because of the method signature.
return <Action>{
kind: 'no-op'
};
});
return [actions];
}
return [this.updateEdgeFeedback()];
}
return [cursorFeedbackAction(CursorCSS.OPERATION_NOT_ALLOWED)];
return [this.updateEdgeFeedback()];
}
return [];
}

protected updateEdgeFeedback(): Action {
if (this.allowedTarget) {
const action = !this.isSourceSelected()
? cursorFeedbackAction(CursorCSS.EDGE_CREATION_SOURCE)
: cursorFeedbackAction(CursorCSS.EDGE_CREATION_TARGET);
return action;
}
return cursorFeedbackAction(CursorCSS.OPERATION_NOT_ALLOWED);
}

protected isAllowedSource(element: SModelElement | undefined): boolean {
return element !== undefined && isConnectable(element) && element.canConnect(this.proxyEdge, 'source');
}

protected isAllowedTarget(element: SModelElement | undefined): boolean {
return element !== undefined && isConnectable(element) && element.canConnect(this.proxyEdge, 'target');
protected async isAllowedTarget(element: SModelElement | undefined): Promise<AllowedTargetCheck> {
let allowed = element !== undefined && isConnectable(element) && element.canConnect(this.proxyEdge, 'target');
if (this.source && element && allowed && this.isDynamic(this.proxyEdge.type)) {
const response = await this.actionDispatcher.request<CheckEdgeTargetResultAction>(
RequestCheckEdgeTargetAction.create(this.source, element, this.proxyEdge.type)
);
allowed = response.isValid;
}
return {
targetElement: element,
allowed
};
}

protected isDynamic(edgeTypeId: string): boolean {
const typeHint = this.typeHintProvider.getEdgeTypeHint(edgeTypeId);
return typeHint?.dynamic === true;
}
}

interface AllowedTargetCheck {
targetElement: SModelElement | undefined;
allowed: boolean;
}
79 changes: 78 additions & 1 deletion packages/protocol/src/action-protocol/element-type-hints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { SModelElement } from 'sprotty-protocol';
import { hasArrayProp } from '../utils/type-util';
import { Action, RequestAction, ResponseAction } from './base-protocol';

Expand Down Expand Up @@ -73,9 +74,18 @@ export interface EdgeTypeHint extends TypeHint {
readonly sourceElementTypeIds: string[];

/**
* Allowed targe element types for this edge type
* Allowed target element types for this edge type
*/
readonly targetElementTypeIds: string[];

/**
* Indicates whether this type hint is dynamic or not. Dynamic edge type hints
* require an additional runtime check before creating an edge, when checking
* source and target element types is not sufficient.
*
* @see {@link RequestCheckEdgeTargetAction}
*/
readonly dynamic?: boolean;
}

/**
Expand Down Expand Up @@ -133,3 +143,70 @@ export namespace SetTypeHintsAction {
};
}
}

/**
* Response Action for a {@link RequestCheckEdgeTargetAction}. It returns
* a boolean indicating whether the requested element is a valid target
* for the edge being created.
*/
export interface CheckEdgeTargetResultAction extends ResponseAction {
kind: typeof CheckEdgeTargetResultAction.KIND;

/**
* true if the selected element is a valid target for this edge,
* false otherwise.
*/
isValid: boolean;
}

export namespace CheckEdgeTargetResultAction {
export const KIND = 'checkEdgeTargetResult';
}

/**
* Send a Request to the server to check if an element is a valid target
* when creating a new Edge.
*/
export interface RequestCheckEdgeTargetAction extends RequestAction<CheckEdgeTargetResultAction> {
kind: typeof RequestCheckEdgeTargetAction.KIND;

/**
* The element type of the edge being created.
*/
edgeTypeId: string;

/**
* The ID of the edge source element.
*/
sourceElementId: string;

/**
* The ID of the edge target element to check.
*/
targetElementId: string;
}

export namespace RequestCheckEdgeTargetAction {
export const KIND = 'requestCheckEdgeTarget';

export function create(
sourceElement: SModelElement | string,
targetElement: SModelElement | string,
edgeTypeId: string
): RequestCheckEdgeTargetAction {
return {
kind: KIND,
edgeTypeId,
sourceElementId: getElementId(sourceElement),
targetElementId: getElementId(targetElement),
requestId: ''
};
}
}

function getElementId(element: SModelElement | string): string {
if (typeof element === 'string') {
return element;
}
return element.id;
}

0 comments on commit b98c98b

Please sign in to comment.