Skip to content
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

Resize of task nodes no longer working #567

Closed
tortmayr opened this issue Mar 1, 2022 · 10 comments · Fixed by eclipse-glsp/glsp-client#174
Closed

Resize of task nodes no longer working #567

tortmayr opened this issue Mar 1, 2022 · 10 comments · Fixed by eclipse-glsp/glsp-client#174
Assignees
Labels
bug Something isn't working client
Milestone

Comments

@tortmayr
Copy link
Contributor

tortmayr commented Mar 1, 2022

With the improvement of the workflow example styling #492 we lost the ability to resize task nodes.
The problem is that the layout for these nodes has been switched from vbox to hbox. We don't have an improved
layouter for hbox yet (#443) and therefore are using the default sprotty hbox layouter which cannot handle the resize properly.

@tortmayr tortmayr added bug Something isn't working client labels Mar 1, 2022
@planger
Copy link
Member

planger commented Mar 1, 2022

Potentially related: #516

@tortmayr
Copy link
Contributor Author

tortmayr commented Mar 1, 2022

Not directly related. I already have a fix for #516 in place which re-enables the resize handles. When testing the fix I noticed the general issue with task nodes that is reproducible in all example integrations.

@rsoika
Copy link

rsoika commented Mar 1, 2022

I also recognized this issue. I think resizing is only lost in VBOX layout. In HBOX it seems to me that it works.

@tortmayr
Copy link
Contributor Author

tortmayr commented Mar 1, 2022

I also recognized this issue. I think resizing is only lost in VBOX layout. In HBOX it seems to me that it works.

I can't confirm that. In the workflow example it doesn't work for task nodes which have a hbox layout but it works as expected for category node which have a vbox layout.

@rsoika
Copy link

rsoika commented Mar 2, 2022

Sorry - yes my fault. I meant VBOX works and HBOX did not work.

@rsoika
Copy link

rsoika commented Mar 2, 2022

I want to give in some of my thoughts about this topic: I do not think it's worth to invest to much time in the idea to control the layout form the server side. If I look at my BPMN requirements (which I try to port currently to GLSP), at the end none of my elements will fit into the concept of VBOX/HBOX

image

I currently ended up to the FREEFORM layout and do the layouting in the client side.

I also wonder if you dilute the concept of macro layout and micro layout with placing to much layout information on the sever side. For me it seems to be fine if I can define on the server side an element just in its compartment layout.

  • An Event has a Circle and a Header.
  • The Circle in the event can have optional a symbol

or

  • A task has a Symbol, a Header and a Detail info

and not specifying how to align them and define paddings or aligns within the compartments.

This is for me more like HTML and CSS. As a JSF developer I learned that it's not my job to define the layout in my JSF/XHTML template/page. I just group sections, labels and input fields. All the design is part of CSS and not part of the server side JSF,

That's just a thought that comes to my mind when I think about layouting in GLSP. This is not a criticism of your great work.

@tortmayr
Copy link
Contributor Author

tortmayr commented Mar 2, 2022

@rsoika Thanks for your feedback. As always it's highly appreciated.
In general I think for this topic there is no "black and white" anwser.
It mostly depends on the basic architectural design one want to use to implement a GLSP based diagram language.
For instance following the main design paradigm of LSP one would try to keep the client as "dumb" (i.e. language agnostic) as possible and shift most of the language specific knowledge to the server. In this case ideally the client doesn't need to know or understand language specific constructs like task nodes, join nodes, fork nodes etc (or how to layout them). It just knows how to handle (i.e. render) basic model elements like "node", "label", "component" and the rest of the layout/styling information is provided by the server. Of course in contrast to LSP, real language-agnostic clients are not really feasible in graphical modelling, however, adopters might still be interested in keeping the client layer as thin as possible.
We have successfully implemented this approach in multiple customer projects and the possibility to control the layout from the server-side definitely came in handy there.

Of course for highly domain specific languages (like BPMN) might require a lot of additional custom views/elements anyways because the available default elements are not sufficient. In this case, keeping layout definition in the server responsibility might not provide any additional benefit. Additional client modifications like new views and model type definitions are required anyways so the overhead of moving the layout computation in the responsibility of the view is rather minimal.

Both use cases are valid and have its application fields. Like most features in GLSP using server-side layout options is opt-in and its completely up to the adopters which approach they want to use. We don't want to restrict this unnecessarily if there is no reason to. In addition, the layout options approach is already provided by sprotty as default so the overhead of adopting it for GLSP is rather minimal. So we rather want to keep the micro layout approach flexible and leave it up the the adopters to implement server-driven or client-driven micro layouting.

@tortmayr
Copy link
Contributor Author

tortmayr commented Mar 2, 2022

If I look at my BPMN requirements (which I try to port currently to GLSP), at the end none of my elements will fit into the concept of VBOX/HBOX
image

I slightly disagree with that statement. Apart from the "neu" node (it it an activity node? My BPMN knowledge is bit rusty 😉) all the other elements I see in this image could also be layouted with server-side layout options using vbox/hbox. Don't get me wrong I don't want to imply that this would be the easier or "more correct" approach and just wanted to clarify that it should be possible.

@rsoika
Copy link

rsoika commented Mar 2, 2022

Yes you are right the 'Neu' node is an Activity or Task.
I tried it several times to layout the Events and Gateway with VBOX but I failed always.
What would be your approach for the Event?

  • an invisible container in VBOX designs
  • with two compartments for the Circle and the label?
  • And than an additional compartment to the Circle with an Icon?

To be honest, I still have no Idea how the server side code should look like to get this working. In the moment my code looks like this:

   @Override
   public void setProperties(final EventNode node) {
      super.setProperties(node);
      node.setName(name);
      node.getCategory().add(eventType);
      node.setLayout(GConstants.Layout.FREEFORM);

      node.getChildren().add(BPMNBuilderHelper.createCompartmentHeader(node));
      node.getChildren().add(BPMNBuilderHelper.createPort(node, -5.0, -25.0, "_north"));
      node.getChildren().add(BPMNBuilderHelper.createPort(node, -25.0, -5.0, "_west"));
      node.getChildren().add(BPMNBuilderHelper.createPort(node, 15.0, -5.0, "_east"));
      node.getChildren().add(BPMNBuilderHelper.createPort(node, -5.0, 15.0, "_south"));
   }

The icon is computed by the client based on the property 'Category'.
The ugly part are the sever side port definitions which are defining the exact positions - I know this is very inconsequent but I did not found no other working solution:

image

Sorry - we are now totally offtopic

@planger planger modified the milestone: 1.0.0 Release Mar 9, 2022
@planger
Copy link
Member

planger commented Mar 9, 2022

#443

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working client
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants