| Re: [graphiti-dev] Diagrams in views - next steps |
These are also valid points we need to have a look into.
How should we continue? Your second point seems to be related to what you are trying with using diagrams in popups. Would you like to work on that and I continue with the method cleanup (your first point)?
Michael
From: Felix Velasco [mailto:felix.velasco@xxxxxxxxx]
Sent: Mittwoch, 6. März 2013 10:55
To: Wenz, Michael
Cc: Discuss development topics on Graphiti (graphiti-dev@xxxxxxxxxxx); Patrick Paulin (patrick@xxxxxxxxxxxx)
Subject: Re: Diagrams in views - next steps
I agree with those points, and would like to add a couple more:
- Remove unused methods in DiagramEditor and DiagramComposite. There are some public methods in these classes that simply rely in DiagramBehavior's (I like the new name) equivalent, and should be called there. Currently there are 13 public methods in DiagramComposite that don't belong to an interface, and several that do but only call DiagramBehavior This would probably prompt us to move some methods from IDiagramContainer(UI) to DiagramBehavior.
- Review the relationship of the diagram with its containing part. What we are doing now is great for an editor, or a single-diagram view, but not for other configurations. We could add a method in DiagramBehavior called "hookWorkbenchPart", or something like that, that would trigger all the things we are doing with the Part right now. DiagramEditor would call it as soon as it created its DiagramBehavior, but DiagramComposite wouldn't. If somebody wanted a single-diagram view, it would have to call diagramComposite.getDiagramBehavior.hookWorkbenchPart() to hook the actions, for instance.
Regards,
Félix
2013/3/6 Wenz, Michael <michael.wenz@xxxxxxx>
Hi,
I just submitted my refactoring and Félix test submit. Thanks for the reviews! I had to use a force commit otherwise Gerrit wouldn’t have let me, but it’s in now…
I would see the following things as next steps (with hopefully smaller commits) and would like to start a discussion on these things:
- The separation between IDiagramContainer and IDiagramContainerUI is currently only a technical one. This should be also reflect semantics, I would propose the following:
o move the methods close, doSave, isAlive, updateDirtyState, getTitle from IDiagramContainerUI to IDiagramContainer
o add the methods refreshContent, getTitleToolTip implemented in the containers to IDiagramContainer
o After that all refresh handling, UI text getters will be part of the interface and will no longer be scattered
- The diagram type provider currently offers access to the container not directly to the support class, which causes that several commands (e.g. CreateConnectionCommand, ReconnectCommand) and the AbstractFeatureProvider need to trigger functionality via the container and not directly on the support
o We might say this is ok, but it is not conceptually clear
o Changing this would need to introduce an interface in the core plugin that we could use from the AbstractFeatureProvider (also part of the core plugin) and other spots to trigger functionality on the support. This interface would need methods like executeFeature and would look somewhat similar to the IDiagramContainer interface
- The DiagramSupport class could be renamed to DiagramBehavior. This would fit better to the other namings we use in the editor, like DefaultUpdateBehavior.
- Should we continue to use the DiagramEditorInput inside the support class? Simply using a URL would be sufficient and the editor input is usually connected to editors only. Using that inside a view or a popup appears strange.
- Other things?
Would do you thing?
Michael