Add SysML diagram frame item #2867
Conversation
github-actions
bot
added
python
| @@ -20,6 +20,7 @@ def test_delete_selected_items(create, diagram, event_manager): | |||
| assert not diagram.ownedPresentation | |||
|
|
|||
|
|
|||
| # TODO: Do we still want this feature? | |||
There was a problem hiding this comment.
@amolenaar @danyeaw @sz332
This test fails. I did not want to just remove it. I am not sure whether we want to have a feature where we delete diagram's element from the model when a diagram gets deleted.
If you have "Remove unused elements" feature enabled, it performs this cleanup anyways (I tested it after my change); but now we have an option to not delete it when the feature is disabled - which would be expected behavior in my opinion.
I would like to have a discussion on whether I should delete the test because I have removed the feature.
| if version_lower_than(gaphor_version, (2, 22, 0)): | ||
| ensure_sysml_diagrams_have_a_frame(elements) |
There was a problem hiding this comment.
I am not sure whether it is taking place at the correct step in model loading. The way I implemented it; it has to happen before we start loading elements and items.
Another option is to do it afterwards, but replacing an item is easier to be done by simply changing the type string rather than to rewire everything to a new item.
There was a problem hiding this comment.
I would do it as early as possible.
What would happen if we leave out this upgrade? Is it required to do this upgrade or existing models just remain as is?
There was a problem hiding this comment.
Since it is not possible to add diagram frame item explicitly later, existing SysML diagrams need to get the item at minimal. I think I'll go with just adding frame item on SysML diagrams to surround all the elements there and leave the other refactoring for the users. Seems to be the safest option. Then you can choose when to start utilizing the frame for parameters/ports. I suppose we could leave a guide or add blog entry about this as well on how to utilize SysML frame.
Another alternative is to perform no upgrade, but in order to get diagram frame, you need to create brand new diagram.
| if len(element_items) == 1: | ||
| replace_diagram_element_item_with_frame(element_items[0]) | ||
| else: | ||
| add_frame(diagram) | ||
| else: | ||
| add_frame(diagram) |
There was a problem hiding this comment.
Are these acceptable defaults?
It ensures that there is always a frame which surrounds all the existing diagram items. If there is a single item which presents the same element as the diagram itself; we replace it with frame instead.
The latter may not be the safest thing to do, especially if we have a bdd representing a block; and we have the same block on the diagram which has the associations with other blocks; it may either: a) look like we form the associations with a diagram; b) the associations visualization may be broken. I suppose the option (a). But it is not desirable substitution in this case, so I am also considering just adding frame and surrounding all the items and call it a day and leave the cleanup for the modeler.
I tested this with my models; and on all of them; it was as I expected, but I can be considered rather strict in modeling and I do not use the block to define the block's internals as it would be kinda like defining systems engineering as "Systems Engineering is something that Systems Engineers do" where Systems Engineer is defined as "Someone who does Systems Engineering".
Again, I am interested in having a discussions.
There was a problem hiding this comment.
I've decided to play it save an simply add a diagram frame item on all the sysml diagrams without any fancy magic. I leave it up to the modeler to decide whether they want to continue modeling the old ways; or they like to leverage the frame's ability to hold ports/parameters. I wont enforce this. 🙂
marek-piirikivi
force-pushed
the
diagram_frame_item
branch
from
b8ac897
to
a6df403
Compare
|
I actually have a question. Do we want to have some constraints for the diagram, so that elements can be added only inside the border? Or if they added outside, then the border auto-resizes itself in order to contain the elements? Also, how should the border resize if it gets smaller than the area occupied by the existing elements? Shall we enable resize for those situations? |
| if isinstance(self.subject, Activity): | ||
| self.update_activity_parameters() | ||
|
|
||
| def update_activity_parameters(self): |
There was a problem hiding this comment.
Now it's only activity diagrams. I guess you have an idea of how to extend this to other diagram types?
There was a problem hiding this comment.
I may actually replace this feature in favor of having the ability to drag existing activity parameters onto the frame. Firstly, it would be consistent with ports where dragging is the only way. Secondly, gives more freedom to choose what parameters you want to show on this particular diagram. Thirdly, it is possible to delete parameter items, but you can never get them back - so if you make a mistake, you need to take a lot of steps to get it back (if ctrl+z is not an option because you discovered the need later).
So, this would mean that we do not generate possible frame items automatically which would simplify maintaining this as well.
What do you think?
| if version_lower_than(gaphor_version, (2, 22, 0)): | ||
| ensure_sysml_diagrams_have_a_frame(elements) |
There was a problem hiding this comment.
I would do it as early as possible.
What would happen if we leave out this upgrade? Is it required to do this upgrade or existing models just remain as is?
@sz332 Definitely would be a fine addition. I would prefer if the frame would surround its internal element's presentations then. It is fine to add metadata and comments and other drawings outside of the frame. 🙂 I also know how to do that, so the question is whether we want to do it or not. And whether we want to do it now; or take some time to think it through and add this later. Currently you have to drag the diagram smaller/larger. There is also an option to use a tool similar to alignment where you can explicitly select when you want to automatically fit the frame. |
marek-piirikivi
force-pushed
the
diagram_frame_item
branch
from
a6df403
to
3a11558
Compare
* Hides diagram header for SysML diagrams * Automatically adds DiagramFrameItem onto SysML diagram representing the diagram's element * Allow connecting diagram element's ports on the frame when the element is block * Automatically attach activity parameters on the frame * Make frame undeletable * Prevent deleting diagram's element when diagram item which subject is the element is deleted * Replace diagram element's single representation with a frame item on load; otherwise just add a frame onto the diagram surrounding all the items
marek-piirikivi
force-pushed
the
diagram_frame_item
branch
from
d6e4a16
to
9f951a0
Compare
|
Converted to draft. I want to do bit more work on this and wait to get droppable parameter nodes merged in first (for example). |
It makes it more explicit what element the diagram represents; what are its interfaces, if any. Overall, the diagrams would look much clearer, conforms better with SysML specification, and allows to be future compatible and control better the way each diagram looks like and "behaves".
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Currently, the diagram frame is attached to the diagram page. In order to get the ports/parameters of the diagram's element; you need to drag the element from the model browser onto the diagram and get the ports/parameters from there.
Issue Number: #2213 (partially)
What is the new behavior?
See top description.
Does this PR introduce a breaking change?
Kinda. It modifies existing SysML diagrams to include diagram frame; and replaces diagram's element representation (only if one is found) with a frame item. So, existing diagrams are expected to change.
Other information
This is prerequisite to complete parametric modelling. I did not want to do this without the proper frame.
It would support potential future features such as preventing adding activities onto activity diagram, instead, when dropping activity, we can automatically replace this with CallBehaviorAction; or when the activity was dropped on the Action, we can convert the Action into CallBehaviorAction → I see this as a great user experience improvement.
gaphor-2023-11-25_09.02.56.mp4