camunda / camunda-modeler-plugins

Plugins for the Camunda Modeler.
167 stars 67 forks source link

bpmn-js-plugin-embedded-comments conflict with documentation element #20

Closed FO-e-X closed 4 years ago

FO-e-X commented 4 years ago

Hi,

there is a strange behavior in combination with embedded comments and the documentation element. As I can see, the comments are stored in bpmn:documentation-elements with the attribute textFormat="text/x-comments". OK, its fine for me. But now I found a difference between first adding comments and then add my own documentation and first adding my own documentation and then add comments.

scenario 1, first comments: An element with textFormat="text/x-comments" is added and shown as documentation in properties panel. When I add my own documentation the attribute textFormat is lost and all comments are part of the normal documentation element. But, now it get really strange, its removed in the XML but the plugin seems to use the known comments only. If I now add a new comment, all other comments are gone, only the new one(s) are shown as comments. The former comments are now plain documentation too. I think this isn't a correct behavior.

scenario 2, first documentation: An element without textFormat appears in the XML. When adding comment(s) now, another element with textFormat is added after the first plain documentation element. The plugin knows to use only the comments. The properties panel documentation element only shows my own documentation. And even the property info plugin knows, that only the first plain documentation element is to use. This sounds really like the desired behavior. Now try to delete the plain comment... The modeller removes all documentation elements for the BPMN-element, including the comments. But some caches do the magic, the plugin continues showing the comments. Unfortunately the cached comments do not survive closing and reopening the BPMN-model.

My suggestion: When the first comment is added, check for other documentation elements and add the one with textFormat at the end. If no other documentation is availabe, add an empty plain documentation element first. But at the other side, the modeler itself need a fix. When deleting the plain documentation, its not a good idea to delete all documentation elements. Maybe an empty element must remain if other documentation elements are available at the BPMN-element.

Sorry for this long text. But I hope it can help to understand the problem better :)

Regards Frank

nikku commented 4 years ago

Please link to the resources you're quoting and add screenshots that make your case. It is otherwise hard to follow your line of thought.

Even better, create a small screencast that records a broken interaction.

FO-e-X commented 4 years ago

Sorry, I missed, that the issue isn't placed at the folder I opened it, but it placed at the project root... I named the plugin already, here is the link https://github.com/camunda/camunda-modeler-plugins/tree/master/bpmn-js-plugin-embedded-comments

I add some screenshots, because a screencast is a bit difficult here. Also I builded a little dummy process. The scenarios I will divide...

FO-e-X commented 4 years ago

scenario 1, first comments adding some comments... enter_comments_first

element documentation enter_comments_first_documentation

XML (btw: modeler 3.6.0 and plugin embedded comments master from last week -> saving comments is only possilbe when modifying the model... ah, sorry, I forgot: no btw ;) ) `

:Dies ist ein Kommentar; ;:Es gehen auch mehrere Kommentare...; ;:noch eine Zeile; ;:noch eine Zeile mehr SequenceFlow_0av8imw SequenceFlow_0p2xcqe ` OK. Now adding some description (where to add else?) ![enter_comments_first_documentation_add](https://user-images.githubusercontent.com/36297434/73929997-1aea8600-48d6-11ea-81af-843f11e5840e.PNG) XML (now the comments are destroyed and used as description): ` :Dies ist ein Kommentar; ;:Es gehen auch mehrere Kommentare...; ;:noch eine Zeile; ;:noch eine Zeile mehr Meine eigene Dokumentation` The comments are still cached and shown as comments, until the next reload. Then they are gone...
FO-e-X commented 4 years ago

scenario 2, first documentation adding some documentation enter_documentation_first enter_documentation_first_no_comment

Now adding some comments enter_documentation_first_add_comment enter_documentation_first_add_comment_descr

XML (again, comment changes are only saved after editing something others in the bpmn) `

Meine eigene Dokumentation :Ein Kommentar; ;:gern auch mehrzeilig SequenceFlow_0av8imw SequenceFlow_0p2xcqe ` Looks great, comments and documentation properly separated 👍 But now I delete the documentation! ![enter_documentation_first_clear_docu](https://user-images.githubusercontent.com/36297434/73957942-6cf7cf80-4907-11ea-8011-f0c03998bef1.PNG) XML ` SequenceFlow_0av8imw SequenceFlow_0p2xcqe ` Where are the comments gone? Why are all documentation elements removed from the task?
FO-e-X commented 4 years ago

I hope it's now more clear what the strange behaviors are. Both ways run in into other mistakes, both are a combination of unwanted cross-editing from Camunda Modeler an embedded-comments-plugin. I think, the plugin itself can't fix this problem without some changes in the Modeler. The Modeler should respect special documentation elements. Maybe a fix in the Modeler only will fix all these problems. Only show simple documentation entries and ignore special documentation elements as well as leaving them untouched. It's a bit like scenario 2, but without deleting all documentation elements when removing the simple documentation :)

Regards, Frank

nikku commented 4 years ago

Thanks for providing additional details.

From what I can see this is an issue with the fact that the Camunda Modeler drops the textFormat attribute, while it

a) Should ignore the documentation, unknown to her entirely or b) keep the textFormat attribute

I've opened https://github.com/camunda/camunda-modeler/issues/1682 to track this as a bug. Please cross read it to see if it captures your issue.

Closing this issue, as it is really a modeler bug that needs to be fixed.