Skip to content

Commit

Permalink
[2759] Update PR according to the last review comments
Browse files Browse the repository at this point in the history 
Bug: #2759
Signed-off-by: Florian Barbin <[email protected]>
  • Loading branch information
@florianbarbin
florianbarbin committed Jul 8, 2024
1 parent 18cf4ec commit f9ec7fb
Show file tree
Hide file tree
Showing 43 changed files with 251 additions and 320 deletions.
15 changes: 0 additions & 15 deletions CHANGELOG.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,22 +52,7 @@ More existing APIs will be migrated to this new common pattern.
}
```

* The `SingleClickOnDiagramElementTool#dialogDescriptionId` has been replaced by a `dialog: Dialog` field to provide the information about the dialog that the frontend needs to retrieve in the extension point.
```
type SingleClickOnDiagramElementTool implements Tool {
id: ID!
label: String!
iconURL: [String!]!
appliesToDiagramRoot: Boolean!
dialog: Dialog
targetDescriptions: [DiagramElementDescription!]!
}
```
* The `String` attribute `org.eclipse.sirius.components.collaborative.diagrams.dto.InvokeSingleClickOnDiagramElementToolInput#selectedObjectId` has been replaced by the attribute `variables` of type `List<ToolVariable>`
* The `String` attribute `org.eclipse.sirius.components.collaborative.diagrams.dto.SingleClickOnDiagramElementTool#selectionDescriptionId` has been replaced by the attribute `dialog` of type `Dialog`
* The `String` attribute `org.eclipse.sirius.components.diagrams.tools.SingleClickOnDiagramElementTool` has been replaced by the attribute `dialog` of type `Dialog`
* The `Palette` component in `sirius-components-diagrams` has a new property `diagramElementTargetObjectId` that represents the id of the diagram element target object.
* The `DiagramPalette` component in `sirius-components-diagrams` has a new property `diagramTargetObjectId: string` that represents the diagram target object.

=== Dependency update

Expand Down
2 changes: 1 addition & 1 deletion ...lipse/sirius/components/compatibility/services/diagrams/CompatibilityPaletteProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ private ITool convertTool(org.eclipse.sirius.components.diagrams.tools.ITool too
if (tool instanceof org.eclipse.sirius.components.diagrams.tools.SingleClickOnDiagramElementTool singleClickOnDiagramElementTool) {
convertedTool = new SingleClickOnDiagramElementTool(singleClickOnDiagramElementTool.getId(), singleClickOnDiagramElementTool.getLabel(),
singleClickOnDiagramElementTool.getIconURL(), singleClickOnDiagramElementTool.getTargetDescriptions(),
singleClickOnDiagramElementTool.getDialog(), singleClickOnDiagramElementTool.isAppliesToDiagramRoot());
singleClickOnDiagramElementTool.getDialogDescriptionId(), singleClickOnDiagramElementTool.isAppliesToDiagramRoot());
}
if (tool instanceof org.eclipse.sirius.components.diagrams.tools.SingleClickOnTwoDiagramElementsTool singleClickOnTwoDiagramElementsTool) {
List<SingleClickOnTwoDiagramElementsCandidate> candidates = new ArrayList<>();
Expand Down
20 changes: 10 additions & 10 deletions ...main/java/org/eclipse/sirius/components/compatibility/services/diagrams/ToolProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import org.eclipse.sirius.components.diagrams.description.EdgeDescription;
import org.eclipse.sirius.components.diagrams.description.IDiagramElementDescription;
import org.eclipse.sirius.components.diagrams.description.NodeDescription;
import org.eclipse.sirius.components.diagrams.tools.Dialog;
import org.eclipse.sirius.components.diagrams.tools.ITool;
import org.eclipse.sirius.components.diagrams.tools.Palette;
import org.eclipse.sirius.components.diagrams.tools.SingleClickOnDiagramElementTool;
Expand All @@ -46,7 +45,6 @@
import org.eclipse.sirius.components.representations.IStatus;
import org.eclipse.sirius.components.representations.Success;
import org.eclipse.sirius.components.representations.VariableManager;
import org.eclipse.sirius.components.selection.description.SelectionDescription;
import org.eclipse.sirius.diagram.description.AbstractNodeMapping;
import org.eclipse.sirius.diagram.description.AdditionalLayer;
import org.eclipse.sirius.diagram.description.ContainerMapping;
Expand Down Expand Up @@ -230,16 +228,17 @@ private SingleClickOnDiagramElementTool convertNodeCreationDescription(Map<Strin
List<String> imagePath = this.toolImageProvider.getIcon(nodeCreationTool);
List<IDiagramElementDescription> targetDescriptions = this.getParentNodeDescriptions(nodeCreationTool.getNodeMappings(), id2NodeDescriptions);
var selectModelElementVariableOpt = new SelectModelElementVariableProvider().getSelectModelElementVariable(nodeCreationTool.getVariable());
Dialog dialog = selectModelElementVariableOpt
.map(selectModelElementVariable -> new Dialog(this.identifierProvider.getIdentifier(selectModelElementVariable), SelectionDescription.TYPE))
.orElse(null);
String dialogDescriptionId = null;
if (selectModelElementVariableOpt.isPresent()) {
dialogDescriptionId = this.identifierProvider.getIdentifier(selectModelElementVariableOpt.get());
}
return SingleClickOnDiagramElementTool.newSingleClickOnDiagramElementTool(id)
.label(label)
.iconURL(imagePath)
.handler(this.createNodeCreationHandler(interpreter, nodeCreationTool))
.targetDescriptions(targetDescriptions)
.appliesToDiagramRoot(this.atLeastOneRootMapping(nodeCreationTool.getNodeMappings()))
.dialog(dialog)
.dialogDescriptionId(dialogDescriptionId)
.build();
}

Expand All @@ -250,16 +249,17 @@ private SingleClickOnDiagramElementTool convertContainerCreationDescription(Map<
List<String> imagePath = this.toolImageProvider.getIcon(containerCreationDescription);
List<IDiagramElementDescription> targetDescriptions = this.getParentNodeDescriptions(containerCreationDescription.getContainerMappings(), id2NodeDescriptions);
var selectModelElementVariableOpt = new SelectModelElementVariableProvider().getSelectModelElementVariable(containerCreationDescription.getVariable());
Dialog dialog = selectModelElementVariableOpt
.map(selectModelElementVariable -> new Dialog(this.identifierProvider.getIdentifier(selectModelElementVariable), SelectionDescription.TYPE))
.orElse(null);
String dialogDescriptionId = null;
if (selectModelElementVariableOpt.isPresent()) {
dialogDescriptionId = this.identifierProvider.getIdentifier(selectModelElementVariableOpt.get());
}
return SingleClickOnDiagramElementTool.newSingleClickOnDiagramElementTool(id)
.label(label)
.iconURL(imagePath)
.handler(this.createContainerCreationHandler(interpreter, containerCreationDescription))
.targetDescriptions(targetDescriptions)
.appliesToDiagramRoot(this.atLeastOneRootMapping(containerCreationDescription.getContainerMappings()))
.dialog(dialog)
.dialogDescriptionId(dialogDescriptionId)
.build();
}

Expand Down
11 changes: 5 additions & 6 deletions ...eclipse/sirius/components/collaborative/diagrams/dto/SingleClickOnDiagramElementTool.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@
import java.util.Objects;

import org.eclipse.sirius.components.diagrams.description.IDiagramElementDescription;
import org.eclipse.sirius.components.diagrams.tools.Dialog;

/**
* A tool triggered by a single click in the palette on a diagram element.
*
* @author mcharfadi
*/
public record SingleClickOnDiagramElementTool(String id, String label, List<String> iconURL, List<IDiagramElementDescription> targetDescriptions, Dialog dialog,
public record SingleClickOnDiagramElementTool(String id, String label, List<String> iconURL, List<IDiagramElementDescription> targetDescriptions, String dialogDescriptionId,
boolean appliesToDiagramRoot) implements ITool {

public SingleClickOnDiagramElementTool {
Expand Down Expand Up @@ -54,7 +53,7 @@ public static final class Builder {

private List<IDiagramElementDescription> targetDescriptions;

private Dialog dialog;
private String dialogDescriptionId;

private boolean appliesToDiagramRoot;

Expand Down Expand Up @@ -82,13 +81,13 @@ public Builder appliesToDiagramRoot(boolean appliesToDiagramRoot) {
return this;
}

public Builder dialog(Dialog dialog) {
this.dialog = dialog;
public Builder dialogDescriptionId(String dialogDescriptionId) {
this.dialogDescriptionId = dialogDescriptionId;
return this;
}

public SingleClickOnDiagramElementTool build() {
return new SingleClickOnDiagramElementTool(this.id, this.label, this.iconURL, this.targetDescriptions, this.dialog, this.appliesToDiagramRoot);
return new SingleClickOnDiagramElementTool(this.id, this.label, this.iconURL, this.targetDescriptions, this.dialogDescriptionId, this.appliesToDiagramRoot);
}
}

Expand Down
6 changes: 3 additions & 3 deletions ...ts/collaborative/diagrams/handlers/InvokeSingleClickOnDiagramElementToolEventHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,13 @@ private IStatus executeTool(IEditingContext editingContext, IDiagramContext diag

if (self.isPresent()) {
VariableManager variableManager = this.populateVariableManager(editingContext, diagramContext, node, edge, self);
var dialog = tool.getDialog();
if (dialog != null && !variables.isEmpty()) {
var dialogDescriptionId = tool.getDialogDescriptionId();
if (dialogDescriptionId != null && !variables.isEmpty()) {
this.handleDialogVariables(editingContext, variableManager, variables);
}

//We do not apply the tool if a dialog is defined but no variables have been provided
if (dialog == null || !variables.isEmpty()) {
if (dialogDescriptionId == null || !variables.isEmpty()) {
result = tool.getHandler().apply(variableManager);
Position newPosition = Position.at(startingPositionX, startingPositionY);
diagramContext.getDiagramEvents().add(new SinglePositionEvent(diagramElementId, newPosition));
Expand Down
25 changes: 10 additions & 15 deletions ...ckend/sirius-components-collaborative-diagrams/src/main/resources/schema/diagram.graphqls
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,15 +291,10 @@ type SingleClickOnDiagramElementTool implements Tool {
label: String!
iconURL: [String!]!
appliesToDiagramRoot: Boolean!
dialog: Dialog
dialogDescriptionId: String
targetDescriptions: [DiagramElementDescription!]!
}

type Dialog {
dialogDescriptionId: String!
dialogDescriptionType: String!
}

type SingleClickOnTwoDiagramElementsTool implements Tool {
id: ID!
label: String!
Expand Down Expand Up @@ -371,12 +366,8 @@ extend type Mutation {
arrangeAll(input: ArrangeAllInput!): ArrangeAllPayload!
deleteFromDiagram(input: DeleteFromDiagramInput!): DeleteFromDiagramPayload!
editLabel(input: EditLabelInput!): EditLabelPayload!
invokeSingleClickOnDiagramElementTool(
input: InvokeSingleClickOnDiagramElementToolInput!
): InvokeSingleClickOnDiagramElementToolPayload!
invokeSingleClickOnTwoDiagramElementsTool(
input: InvokeSingleClickOnTwoDiagramElementsToolInput!
): InvokeSingleClickOnTwoDiagramElementsToolPayload!
invokeSingleClickOnDiagramElementTool(input: InvokeSingleClickOnDiagramElementToolInput!): InvokeSingleClickOnDiagramElementToolPayload!
invokeSingleClickOnTwoDiagramElementsTool(input: InvokeSingleClickOnTwoDiagramElementsToolInput!): InvokeSingleClickOnTwoDiagramElementsToolPayload!
updateNodeBounds(input: UpdateNodeBoundsInput!): UpdateNodeBoundsPayload!
updateNodePosition(input: UpdateNodePositionInput!): UpdateNodePositionPayload!
dropOnDiagram(input: DropOnDiagramInput!): DropOnDiagramPayload!
Expand Down Expand Up @@ -460,7 +451,7 @@ input InvokeSingleClickOnTwoDiagramElementsToolInput {
}

union InvokeSingleClickOnTwoDiagramElementsToolPayload =
ErrorPayload
ErrorPayload
| InvokeSingleClickOnTwoDiagramElementsToolSuccessPayload

type InvokeSingleClickOnTwoDiagramElementsToolSuccessPayload {
Expand Down Expand Up @@ -492,7 +483,9 @@ enum ToolVariableType {
OBJECT_ID_ARRAY
}

union InvokeSingleClickOnDiagramElementToolPayload = ErrorPayload | InvokeSingleClickOnDiagramElementToolSuccessPayload
union InvokeSingleClickOnDiagramElementToolPayload =
ErrorPayload
| InvokeSingleClickOnDiagramElementToolSuccessPayload

type InvokeSingleClickOnDiagramElementToolSuccessPayload {
id: ID!
Expand Down Expand Up @@ -527,7 +520,9 @@ input UpdateNodePositionInput {
newPositionY: Float!
}

union UpdateNodePositionPayload = ErrorPayload | UpdateNodePositionSuccessPayload
union UpdateNodePositionPayload =
ErrorPayload
| UpdateNodePositionSuccessPayload

type UpdateNodePositionSuccessPayload {
id: ID!
Expand Down
10 changes: 3 additions & 7 deletions ...llaborative/diagrams/handlers/InvokeSingleClickOnDiagramElementToolEventHandlerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
import org.eclipse.sirius.components.diagrams.description.InsideLabelDescription;
import org.eclipse.sirius.components.diagrams.description.LabelStyleDescription;
import org.eclipse.sirius.components.diagrams.description.NodeDescription;
import org.eclipse.sirius.components.diagrams.tools.Dialog;
import org.eclipse.sirius.components.diagrams.tools.ITool;
import org.eclipse.sirius.components.diagrams.tools.SingleClickOnDiagramElementTool;
import org.eclipse.sirius.components.representations.IStatus;
Expand All @@ -81,8 +80,6 @@
*/
public class InvokeSingleClickOnDiagramElementToolEventHandlerTests {

private static final String SELECTION_DIALOG_TYPE = "SELECTION_DIALOG_TYPE";

private static final String NAME_VARIABLE_VALUE = "nameVariableValue";

private static final String NAME_VARIABLE = "name";
Expand Down Expand Up @@ -244,7 +241,7 @@ public Optional<Node> findNodeById(Diagram diagram, String nodeId) {
return new Success(ChangeKind.SEMANTIC_CHANGE, Map.of());
};

var tool = this.createTool(TOOL_ID, false, List.of(nodeDescription), new Dialog(DIALOG_DESCRIPTION_ID, SELECTION_DIALOG_TYPE), toolHandler);
var tool = this.createTool(TOOL_ID, false, List.of(nodeDescription), DIALOG_DESCRIPTION_ID, toolHandler);

var toolService = new IToolService.NoOp() {
@Override
Expand Down Expand Up @@ -444,13 +441,12 @@ private SingleClickOnDiagramElementTool createTool(String toolId, boolean applie
return this.createTool(toolId, appliesToDiagramRoot, diagramElementsDescriptions, null, variableManager -> new Success(ChangeKind.SEMANTIC_CHANGE, Map.of()));
}

private SingleClickOnDiagramElementTool createTool(String toolId, boolean appliesToDiagramRoot, List<IDiagramElementDescription> diagramElementsDescriptions, Dialog dialog,
Function<VariableManager, IStatus> handler) {
private SingleClickOnDiagramElementTool createTool(String toolId, boolean appliesToDiagramRoot, List<IDiagramElementDescription> diagramElementsDescriptions, String dialogDescriptionId, Function<VariableManager, IStatus> handler) {
return SingleClickOnDiagramElementTool.newSingleClickOnDiagramElementTool(toolId)
.label(TOOL_LABEL)
.iconURL(List.of(TOOL_IMAGE_URL))
.targetDescriptions(diagramElementsDescriptions)
.dialog(dialog)
.dialogDescriptionId(dialogDescriptionId)
.handler(handler)
.appliesToDiagramRoot(appliesToDiagramRoot)
.build();
Expand Down
4 changes: 1 addition & 3 deletions ...rc/main/java/org/eclipse/sirius/components/diagrams/tests/graphql/PaletteQueryRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ query getPalette($editingContextId: ID!, $representationId: ID!, $diagramElement
id
}
appliesToDiagramRoot
dialog {
dialogDescriptionId
}
dialogDescriptionId
}
}
""";
Expand Down
28 changes: 0 additions & 28 deletions ...omponents-diagrams/src/main/java/org/eclipse/sirius/components/diagrams/tools/Dialog.java
View file

This file was deleted.

14 changes: 7 additions & 7 deletions ...in/java/org/eclipse/sirius/components/diagrams/tools/SingleClickOnDiagramElementTool.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public final class SingleClickOnDiagramElementTool implements ITool {

private boolean appliesToDiagramRoot;

private Dialog dialog;
private String dialogDescriptionId;

private SingleClickOnDiagramElementTool() {
// Prevent instantiation
Expand Down Expand Up @@ -78,8 +78,8 @@ public String getLabel() {
return this.label;
}

public Dialog getDialog() {
return this.dialog;
public String getDialogDescriptionId() {
return this.dialogDescriptionId;
}

@Override
Expand Down Expand Up @@ -113,7 +113,7 @@ public static final class Builder {

private boolean appliesToDiagramRoot;

private Dialog dialog;
private String dialogDescriptionId;

private Builder(String id) {
this.id = Objects.requireNonNull(id);
Expand Down Expand Up @@ -144,8 +144,8 @@ public Builder handler(Function<VariableManager, IStatus> handler) {
return this;
}

public Builder dialog(Dialog dialog) {
this.dialog = dialog;
public Builder dialogDescriptionId(String dialogDescriptionId) {
this.dialogDescriptionId = dialogDescriptionId;
return this;
}

Expand All @@ -157,7 +157,7 @@ public SingleClickOnDiagramElementTool build() {
tool.handler = Objects.requireNonNull(this.handler);
tool.targetDescriptions = Objects.requireNonNull(this.targetDescriptions);
tool.appliesToDiagramRoot = this.appliesToDiagramRoot;
tool.dialog = this.dialog;
tool.dialogDescriptionId = this.dialogDescriptionId;
return tool;
}
}
Expand Down
Loading

0 comments on commit f9ec7fb

Please sign in to comment.