Skip to content

Commit

Permalink
[956] Improve the layout of border nodes
Browse files Browse the repository at this point in the history
This commit changes the way the child is added to its parent in convertDiagram.
A child is now added with the add method instead of parent.children=xxx. It
allows to properly set SParentElement.parent property.

Each child adds itself to its parent as soon as created to make sure that its
own children will be added in a graph containing a root element.

The border node enters its parent container with 8px. The ELK layout has been
updated to have a border node offset. The ELK option CoreOptions.PORT_BORDER_OFFSET
is set by default to 8 pixels entering the parent node.

Bug: #956
Signed-off-by: Laurent Fasani <laurent.fasani@obeo.fr>
  • Loading branch information
lfasani authored and sbegaudeau committed Mar 2, 2022
1 parent 4bcf663 commit 047fe87
Show file tree
Hide file tree
Showing 14 changed files with 505 additions and 53 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.adoc
Expand Up @@ -9,6 +9,7 @@
- [ADR-041] Add the ability to contribute additional services to the EMFQueryService
- [ADR-042] Use wildcard collections instead of List<Object> in Providers
- [ADR-043] Consider multiple objects as the input of a form
- [ADR-044] Use border node "snap to parent container" algorithm

=== Deprecation warning
- [core] The various DTOs related to the creation and renaming of both documents and representations will be removed from the project at some point. The only reason why we will keep them for the moment is that some of them are used to trigger some specific behavior in the `EditingContextEventProcessor`. The split of the representation metadata should help us remove those special use cases
Expand Down Expand Up @@ -52,6 +53,7 @@
- https://github.com/eclipse-sirius/sirius-components/issues/1054[#1054] [diagram] Add missing variables to compute the label of an edge
- https://github.com/eclipse-sirius/sirius-components/issues/1063[#1063] [explorer] It is now possible to expand or collapse items in the explorer without selecting them by clicking directly on the expand/collapse arrow icon
- https://github.com/eclipse-sirius/sirius-components/issues/1068[#1068] [form] Add support for displaying details on arbitrary element kinds
- https://github.com/eclipse-sirius/sirius-components/issues/956[#956] [diagram] Add the border node concept on front-end and implement the border node snap. The user can move the border node only on the side of its parent node. The border node enters its parent node with 8px. The ELK automatic layout is adapated to have the same behavior.

=== New features

Expand Down
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2019, 2021 Obeo.
* Copyright (c) 2019, 2022 Obeo.
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v2.0
* which accompanies this distribution, and is available at
Expand Down Expand Up @@ -67,6 +67,11 @@ public class LayoutConfiguratorRegistry {
*/
private static final int MIN_WIDTH_CONSTRAINT = 150;

/**
* The default value for port border offset.
*/
private static final double DEFAULT_PORT_BORDER_OFFSET = -8;

private final List<IDiagramLayoutConfiguratorProvider> customLayoutProviders;

public LayoutConfiguratorRegistry(List<IDiagramLayoutConfiguratorProvider> customLayoutProviders) {
Expand All @@ -90,7 +95,8 @@ public ISiriusWebLayoutConfigurator getDefaultLayoutConfigurator() {
.setProperty(CoreOptions.NODE_SIZE_OPTIONS, EnumSet.of(SizeOptions.ASYMMETRICAL))
.setProperty(CoreOptions.NODE_SIZE_MINIMUM, new KVector(LayoutOptionValues.MIN_WIDTH_CONSTRAINT, LayoutOptionValues.MIN_HEIGHT_CONSTRAINT))
.setProperty(CoreOptions.NODE_LABELS_PLACEMENT, NodeLabelPlacement.insideTopCenter())
.setProperty(CoreOptions.NODE_SIZE_MINIMUM, new KVector(MIN_WIDTH_CONSTRAINT, MIN_HEIGHT_CONSTRAINT));
.setProperty(CoreOptions.NODE_SIZE_MINIMUM, new KVector(MIN_WIDTH_CONSTRAINT, MIN_HEIGHT_CONSTRAINT))
.setProperty(CoreOptions.PORT_BORDER_OFFSET, DEFAULT_PORT_BORDER_OFFSET);

configurator.configureByType(NodeType.NODE_LIST)
.setProperty(CoreOptions.ALGORITHM, FixedLayouterOptions.ALGORITHM_ID)
Expand All @@ -103,7 +109,8 @@ public ISiriusWebLayoutConfigurator getDefaultLayoutConfigurator() {

configurator.configureByType(NodeType.NODE_IMAGE)
.setProperty(CoreOptions.NODE_SIZE_CONSTRAINTS, EnumSet.of(SizeConstraint.MINIMUM_SIZE, SizeConstraint.PORT_LABELS, SizeConstraint.PORTS))
.setProperty(CoreOptions.NODE_LABELS_PLACEMENT, NodeLabelPlacement.outsideTopCenter());
.setProperty(CoreOptions.NODE_LABELS_PLACEMENT, NodeLabelPlacement.outsideTopCenter())
.setProperty(CoreOptions.PORT_BORDER_OFFSET, DEFAULT_PORT_BORDER_OFFSET);

// This image type does not match any diagram item. We add it to define the image size as constraint for the node image parent.
configurator.configureByType(ELKDiagramConverter.DEFAULT_IMAGE_TYPE)
Expand Down
30 changes: 30 additions & 0 deletions doc/adrs/044_use_border_node_snap_to_container_algorithm.adoc
@@ -0,0 +1,30 @@
= ADR-44 - Use border node "snap to parent container" algorithm

== Context

On the front end, consider a diagram containing nodes with border nodes.

The user has the abilty to move the border nodes on a side of the parent rectangle node or onto another side.

Until now, the border node is considered as any node and consequently the user has the ability to move the border node inside the parent node and only inside.

== Decision

A "snap to parent rectangle" is added.
It is used by the front end code.

Now, when the user moves the border node, it is moved only on the border of the parent node, outside the parent node.

An 8px offset is used to have the border node entering the parent node.

=== Algorithm details

When the user is dragging the border node, to determine the side on which the border node will snap, the algorithm calculates the distance between the center of the dragged border node to each side segment of the parent node. The snap is done on the closest parent node side.

Then the offset is applied on the border node to have the border node entering the parent node

== Status

WIP

== Consequences
12 changes: 9 additions & 3 deletions frontend/src/diagram/DiagramWebSocketContainer.tsx
Expand Up @@ -106,7 +106,7 @@ import { Toolbar } from 'diagram/Toolbar';
import { atLeastOneCanInvokeEdgeTool, canInvokeTool } from 'diagram/toolServices';
import React, { useCallback, useContext, useEffect, useRef } from 'react';
import { SelectionDialogWebSocketContainer } from 'selection/SelectionDialogWebSocketContainer';
import { EditLabelAction, HoverFeedbackAction, SEdge, SGraph, SModelElement, SNode } from 'sprotty';
import { EditLabelAction, HoverFeedbackAction, SEdge, SGraph, SModelElement, SNode, SPort } from 'sprotty';
import { FitToScreenAction } from 'sprotty-protocol';
import { v4 as uuid } from 'uuid';
import { RepresentationComponentProps, Selection, SelectionEntry } from 'workbench/Workbench.types';
Expand Down Expand Up @@ -454,7 +454,9 @@ export const DiagramWebSocketContainer = ({
const deleteElements = useCallback(
(diagramElements: SModelElement[], deletionPolicy: GQLDeletionPolicy): void => {
const edgeIds = diagramElements.filter((diagramElement) => diagramElement instanceof SEdge).map((elt) => elt.id);
const nodeIds = diagramElements.filter((diagramElement) => diagramElement instanceof SNode).map((elt) => elt.id);
const nodeIds = diagramElements
.filter((diagramElement) => diagramElement instanceof SNode || diagramElement instanceof SPort)
.map((elt) => elt.id);

const input = {
id: uuid(),
Expand Down Expand Up @@ -568,7 +570,11 @@ export const DiagramWebSocketContainer = ({
if (selectedElement instanceof SGraph) {
const { id, label, kind } = selectedElement as any; // (as any) to be removed when the proper type will be available
newSelection.entries.push({ id, label, kind });
} else if (selectedElement instanceof SNode || selectedElement instanceof SEdge) {
} else if (
selectedElement instanceof SNode ||
selectedElement instanceof SPort ||
selectedElement instanceof SEdge
) {
const { id, kind, targetObjectId, targetObjectKind, targetObjectLabel } = selectedElement as any; // (as any) to be removed when the proper type will be available

const semanticSelectionEntry: SelectionEntry = {
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/diagram/palette/ContextualPalette.tsx
Expand Up @@ -21,7 +21,7 @@ import {
import { ContextualPaletteProps } from 'diagram/palette/ContextualPalette.types';
import { ToolSection } from 'diagram/palette/tool-section/ToolSection';
import { Tool } from 'diagram/palette/tool/Tool';
import { Node } from 'diagram/sprotty/Diagram.types';
import { BorderNode, Node } from 'diagram/sprotty/Diagram.types';
import { isContextualTool } from 'diagram/toolServices';
import { GQLSynchronizationPolicy } from 'index';
import React from 'react';
Expand Down Expand Up @@ -109,7 +109,7 @@ const isSynchronized = (diagramDescription: GQLDiagramDescription, element: SMod
elementWithTarget = elementWithTarget.parent;
}

if (element instanceof Node) {
if (element instanceof Node || element instanceof BorderNode) {
const descriptionId = element.descriptionId;
return (
findNodeDescription(diagramDescription.nodeDescriptions, [], descriptionId).synchronizationPolicy ===
Expand Down Expand Up @@ -245,7 +245,7 @@ export const ContextualPalette = ({
});

let renameEntry;
if (invokeLabelEdit) {
if (invokeLabelEdit && !(targetElement instanceof BorderNode)) {
renameEntry = (
<div className={classes.toolEntry}>
<Tool tool={editTool} thumbnail={true} onClick={() => invokeLabelEdit()} />
Expand Down
19 changes: 13 additions & 6 deletions frontend/src/diagram/sprotty/DependencyInjection.ts
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2019, 2021 Obeo.
* Copyright (c) 2019, 2022 Obeo.
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v2.0
* which accompanies this distribution, and is available at
Expand All @@ -10,7 +10,7 @@
* Contributors:
* Obeo - initial API and implementation
*******************************************************************************/
import { Node } from 'diagram/sprotty/Diagram.types';
import { BorderNode, Node } from 'diagram/sprotty/Diagram.types';
import { DiagramServer, HIDE_CONTEXTUAL_TOOLBAR_ACTION, SPROTTY_DELETE_ACTION } from 'diagram/sprotty/DiagramServer';
import { SetActiveConnectorToolsAction, SetActiveToolAction } from 'diagram/sprotty/DiagramServer.types';
import { edgeCreationFeedback } from 'diagram/sprotty/edgeCreationFeedback';
Expand Down Expand Up @@ -119,7 +119,9 @@ const siriusWebContainerModule = new ContainerModule((bind, unbind, isBound, reb
// @ts-ignore
configureModelElement(context, 'node:list:item', Node, ListItemView);
// @ts-ignore
configureView({ bind, isBound }, 'port:square', RectangleView);
configureModelElement(context, 'port:rectangle', BorderNode, RectangleView);
// @ts-ignore
configureModelElement(context, 'port:image', BorderNode, ImageView);
configureView({ bind, isBound }, 'edge:straight', EdgeView);
// @ts-ignore
configureModelElement(context, 'label:inside-center', SEditableLabel, LabelView);
Expand Down Expand Up @@ -229,9 +231,14 @@ export const createDependencyInjectionContainer = (containerId: string, getCurso
}
}

const findModelElementWithSemanticTarget = (element: SModelElement): SGraph | Node | SEdge | null => {
let graphicalElement: SGraph | Node | SEdge | null = null;
if (element instanceof SGraph || element instanceof Node || element instanceof SEdge) {
const findModelElementWithSemanticTarget = (element: SModelElement): SGraph | Node | BorderNode | SEdge | null => {
let graphicalElement: SGraph | Node | BorderNode | SEdge | null = null;
if (
element instanceof SGraph ||
element instanceof Node ||
element instanceof BorderNode ||
element instanceof SEdge
) {
graphicalElement = element;
} else if (element instanceof SLabel) {
graphicalElement = findModelElementWithSemanticTarget(element.parent);
Expand Down
12 changes: 10 additions & 2 deletions frontend/src/diagram/sprotty/Diagram.types.ts
Expand Up @@ -30,6 +30,16 @@ export class Node extends SNode implements WithEditableLabel {
targetObjectLabel: string;
}

export class BorderNode extends SPort implements WithEditableLabel {
editableLabel?: EditableLabel & Label;
descriptionId: string;
kind: string;
style: INodeStyle;
targetObjectId: string;
targetObjectKind: string;
targetObjectLabel: string;
}

export interface INodeStyle {}

export class ImageNodeStyle implements INodeStyle {
Expand Down Expand Up @@ -108,5 +118,3 @@ export class LabelStyle {
strikeThrough: boolean;
underline: boolean;
}

export class Port extends SPort {}
3 changes: 2 additions & 1 deletion frontend/src/diagram/sprotty/DiagramServer.tsx
Expand Up @@ -47,6 +47,7 @@ import {
SelectionResult,
SGraph,
SNode,
SPort,
ViewportResult,
} from 'sprotty';
import {
Expand Down Expand Up @@ -430,7 +431,7 @@ export class DiagramServer extends ModelSource {
};

let edgeStartPosition = { x: 0, y: 0 };
if (element instanceof SNode) {
if (element instanceof SNode || element instanceof SPort) {
edgeStartPosition = {
x: (lastPositionOnDiagram.x - scroll.x) * zoom,
y: (lastPositionOnDiagram.y - scroll.y) * zoom,
Expand Down
131 changes: 131 additions & 0 deletions frontend/src/diagram/sprotty/__tests__/borderNodes.test.ts
@@ -0,0 +1,131 @@
/*******************************************************************************
* Copyright (c) 2022 Obeo.
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Obeo - initial API and implementation
*******************************************************************************/
import { httpOrigin } from 'common/URL';
import 'reflect-metadata';
import { SNode } from 'sprotty';
import { Point } from 'sprotty-protocol/';
import { convertDiagram } from '../convertDiagram';
import { BorderNode } from '../Diagram.types';
import { SiriusDragAndDropMouseListener } from '../siriusDragAndDropMouseListener';
import { siriusWebDiagram } from './siriusWebDiagram';

describe('Border node positioning', () => {
it('snaps the border node', () => {
const sprottyDiagram = convertDiagram(siriusWebDiagram, httpOrigin, false);

const sprottyNode: SNode = <SNode>sprottyDiagram.children[1];

const borderNode: BorderNode = <BorderNode>sprottyNode.children.filter((value) => {
return value instanceof BorderNode;
})[0];

// border node position when its center is positionned at the parent upper left corner(origin))
const borderNodeOrigin: Point = { x: -borderNode.size.width / 2, y: -borderNode.size.height / 2 };
const parentWidth: number = sprottyNode.size.width;
const parentHeight: number = sprottyNode.size.height;

const siriusDragAndDropMouseListener: SiriusDragAndDropMouseListener = new SiriusDragAndDropMouseListener();
expect(
siriusDragAndDropMouseListener.snap({ x: borderNodeOrigin.x - 10, y: borderNodeOrigin.y - 5 }, borderNode, false)
).toStrictEqual({ x: -52, y: -20 });
expect(
siriusDragAndDropMouseListener.snap({ x: borderNodeOrigin.x - 10, y: borderNodeOrigin.y - 15 }, borderNode, false)
).toStrictEqual({ x: -30, y: -32 });
expect(
siriusDragAndDropMouseListener.snap({ x: borderNodeOrigin.x + 50, y: borderNodeOrigin.y - 25 }, borderNode, false)
).toStrictEqual({ x: 20, y: -32 });
expect(
siriusDragAndDropMouseListener.snap({ x: borderNodeOrigin.x + 50, y: borderNodeOrigin.y + 5 }, borderNode, false)
).toStrictEqual({ x: 20, y: -32 });
expect(
siriusDragAndDropMouseListener.snap(
{ x: borderNodeOrigin.x + parentWidth + 5, y: borderNodeOrigin.y - 10 },
borderNode,
false
)
).toStrictEqual({ x: 250, y: -32 });
expect(
siriusDragAndDropMouseListener.snap(
{ x: borderNodeOrigin.x + parentWidth + 15, y: borderNodeOrigin.y - 10 },
borderNode,
false
)
).toStrictEqual({ x: 272, y: -20 });
expect(
siriusDragAndDropMouseListener.snap(
{ x: borderNodeOrigin.x + parentWidth + 15, y: borderNodeOrigin.y + 50 },
borderNode,
false
)
).toStrictEqual({ x: 272, y: 30 });
expect(
siriusDragAndDropMouseListener.snap(
{ x: borderNodeOrigin.x + parentWidth - 5, y: borderNodeOrigin.y + 50 },
borderNode,
false
)
).toStrictEqual({ x: 272, y: 30 });
expect(
siriusDragAndDropMouseListener.snap(
{ x: borderNodeOrigin.x + parentWidth + 15, y: borderNodeOrigin.y + parentHeight + 10 },
borderNode,
false
)
).toStrictEqual({ x: 272, y: 160 });
expect(
siriusDragAndDropMouseListener.snap(
{ x: borderNodeOrigin.x + parentWidth + 10, y: borderNodeOrigin.y + parentHeight + 15 },
borderNode,
false
)
).toStrictEqual({ x: 250, y: 172 });
expect(
siriusDragAndDropMouseListener.snap(
{ x: borderNodeOrigin.x + parentWidth / 2, y: borderNodeOrigin.y + parentHeight - 5 },
borderNode,
false
)
).toStrictEqual({ x: 110, y: 172 });
expect(
siriusDragAndDropMouseListener.snap(
{ x: borderNodeOrigin.x + parentWidth / 2, y: borderNodeOrigin.y + parentHeight + 5 },
borderNode,
false
)
).toStrictEqual({ x: 110, y: 172 });

expect(
siriusDragAndDropMouseListener.snap({ x: borderNodeOrigin.x - 5, y: borderNodeOrigin.y + 10 }, borderNode, false)
).toStrictEqual({ x: -52, y: -10 });
expect(
siriusDragAndDropMouseListener.snap({ x: borderNodeOrigin.x - 10, y: borderNodeOrigin.y + 5 }, borderNode, false)
).toStrictEqual({ x: -52, y: -15 });
expect(
siriusDragAndDropMouseListener.snap(
{ x: borderNodeOrigin.x + 10, y: borderNodeOrigin.y + parentHeight / 2 },
borderNode,
false
)
).toStrictEqual({ x: -52, y: 70 });
expect(
siriusDragAndDropMouseListener.snap(
{ x: borderNodeOrigin.x - 10, y: borderNodeOrigin.y + parentHeight / 2 },
borderNode,
false
)
).toStrictEqual({ x: -52, y: 70 });
expect(
siriusDragAndDropMouseListener.snap({ x: borderNodeOrigin.x - 10, y: borderNodeOrigin.y - 5 }, borderNode, false)
).toStrictEqual({ x: -52, y: -20 });
});
});
8 changes: 4 additions & 4 deletions frontend/src/diagram/sprotty/__tests__/siriusWebDiagram.ts
Expand Up @@ -134,8 +134,8 @@ export const siriusWebDiagram: GQLDiagram = {
y: 64.34101572060084,
},
size: {
width: 286,
height: 181.6982421875,
width: 280,
height: 180,
},
borderNodes: [
{
Expand Down Expand Up @@ -181,8 +181,8 @@ export const siriusWebDiagram: GQLDiagram = {
y: 139.6982421875,
},
size: {
width: 30,
height: 30,
width: 60,
height: 40,
},
borderNodes: [],
childNodes: [],
Expand Down

0 comments on commit 047fe87

Please sign in to comment.