From b98c98b9d2d87489852709501c2fccfb3a9807d5 Mon Sep 17 00:00:00 2001 From: Camille Letavernier Date: Fri, 8 Sep 2023 12:47:47 +0200 Subject: [PATCH] 211: Revise TypeHints and server side feedback for creation actions - 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 --- .../src/features/hints/type-hints-module.ts | 11 ++- .../tools/edge-creation/edge-creation-tool.ts | 82 +++++++++++++++---- .../src/action-protocol/element-type-hints.ts | 79 +++++++++++++++++- 3 files changed, 156 insertions(+), 16 deletions(-) diff --git a/packages/client/src/features/hints/type-hints-module.ts b/packages/client/src/features/hints/type-hints-module.ts index 0ba11f9d..f07a079b 100644 --- a/packages/client/src/features/hints/type-hints-module.ts +++ b/packages/client/src/features/hints/type-hints-module.ts @@ -13,7 +13,15 @@ * * 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) => { @@ -21,5 +29,6 @@ export const typeHintsModule = new FeatureModule((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); }); diff --git a/packages/client/src/features/tools/edge-creation/edge-creation-tool.ts b/packages/client/src/features/tools/edge-creation/edge-creation-tool.ts index 4aca5b6b..947684ca 100644 --- a/packages/client/src/features/tools/edge-creation/edge-creation-tool.ts +++ b/packages/client/src/features/tools/edge-creation/edge-creation-tool.ts @@ -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'; @@ -42,6 +46,8 @@ export class EdgeCreationTool extends BaseCreationTool)[] { const newCurrentTarget = findParentByFeature(target, isConnectable); if (newCurrentTarget !== this.currentTarget) { this.currentTarget = newCurrentTarget; @@ -136,25 +149,66 @@ export class EdgeCreationToolMouseListener extends DragAwareMouseListener { 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 or [] here + // because of the method signature. + return { + 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 { + 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( + 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; +} diff --git a/packages/protocol/src/action-protocol/element-type-hints.ts b/packages/protocol/src/action-protocol/element-type-hints.ts index a0bbebe6..108160e9 100644 --- a/packages/protocol/src/action-protocol/element-type-hints.ts +++ b/packages/protocol/src/action-protocol/element-type-hints.ts @@ -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'; @@ -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; } /** @@ -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 { + 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; +}