-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[3647] Add support for diagram style with background customization #3648
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,7 @@ | |
import org.eclipse.sirius.components.core.api.IRepresentationDescriptionSearchService; | ||
import org.eclipse.sirius.components.diagrams.CollapsingState; | ||
import org.eclipse.sirius.components.diagrams.Diagram; | ||
import org.eclipse.sirius.components.diagrams.DiagramStyle; | ||
import org.eclipse.sirius.components.diagrams.FreeFormLayoutStrategy; | ||
import org.eclipse.sirius.components.diagrams.InsideLabel; | ||
import org.eclipse.sirius.components.diagrams.InsideLabelLocation; | ||
|
@@ -171,6 +172,7 @@ public void testGetConnectorTools() { | |
.edgeDescriptions(new ArrayList<>()) | ||
.palettes(List.of(palette)) | ||
.dropHandler(variableManager -> new Failure("")) | ||
.styleProvider(variableManager -> DiagramStyle.newDiagramStyle().build()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason to have some of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, in fact, I started by adding a value at instantiation and then adding a default value. |
||
.build(); | ||
|
||
Node sourceNode = this.getNode(SOURCE_NODE_ID, SOURCE_NODE_TARGET_ID); | ||
|
@@ -237,6 +239,7 @@ private Diagram getDiagram(String id, List<Node> nodes) { | |
.size(Size.UNDEFINED) | ||
.nodes(nodes) | ||
.edges(List.of()) | ||
.style(DiagramStyle.newDiagramStyle().build()) | ||
.build(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This must be new since the last time I reviewed this PR because this cannot work. I could accept that in the services of Papaya diagrams because it would work in this case but imagine that I have 1 node in my diagram which has 10 child nodes and the view deletion request is for one of its child node then this code does not work.
Imagine now that I have 3 nodes in my diagram and a tool triggers the creation of 3 view deletion request then this code would not work either. Finding out that a diagram is empty in all use case is more complex than what you algorithm suggest. Please move both the implementation of this service and its definition out of IDiagramServices for now and into something in the Java services of Papaya (since I suspect that you are only using it there).
You could also improve this algorithm to make it work all the time but it will require some testing of corner cases. Consumers of Sirius Web such as SysON would be quite interested by such service.