| [graphiti-dev] Diagrams in views - next steps |
|
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 |