Add support for diagram markers #217
Conversation
|
|
||
| static readonly KIND = 'setMarkers'; | ||
|
|
||
| markers: Marker[]; |
There was a problem hiding this comment.
Why do we need this field?
| */ | ||
| execute(context: CommandExecutionContext): CommandResult { | ||
| this.markers = this.action.markers; | ||
| for (let marker of this.markers) { |
There was a problem hiding this comment.
Should be for (const marker ..., because the value is never reassigned.
| return issueMarker; | ||
| } | ||
|
|
||
| private createSIssue(marker: Marker): SIssue { |
There was a problem hiding this comment.
Doesn't access any member variables so can be defined as function
|
|
||
| undo(context: CommandExecutionContext): CommandResult { | ||
| this.markers = this.action.markers; | ||
| for (let marker of this.markers) { |
There was a problem hiding this comment.
Should be for (const marker
| for (let marker of this.markers) { | ||
| const modelElement: SModelElement | undefined = context.root.index.getById(marker.elementId); | ||
| if (modelElement instanceof SParentElement) { | ||
| for (let child of modelElement.children) { |
There was a problem hiding this comment.
Should be for (const child
| getMarkers(): Marker[] { | ||
| return this.markers; | ||
| } | ||
|
|
There was a problem hiding this comment.
Please remove blank line
| return this.execute(context); | ||
| } | ||
|
|
||
| getMarkers(): Marker[] { |
There was a problem hiding this comment.
Why do we need that getter? this.markers is public anyways
| markers.addAll(validator.validate(modelState, modelElement)); | ||
| } | ||
|
|
||
| // TODO get element ID of root element on client side and send it (see |
There was a problem hiding this comment.
Seems like the TODOs are outdated.
| * Defines kinds of model markers | ||
| * | ||
| */ | ||
| public class MarkerKind { |
There was a problem hiding this comment.
@tanjaem Thanks a lot! Works great and I only found a few minor issues/nitpicks.
Maybe the markers should be hoverable so that we can provide additional information (e.g. description) via Popup. But this can be handled in a follow-up PR
|
Thanks for your feedback! Should all be incorporated now. |
From #216 :
Closes #30
This is an initial contribution for offering model validation support that shows validation info/warning/error messages as diagram decorators. I opened issues #213 and #214 for further improving this initial model validation support. I am happy to work on these issues if you think the suggested improvements make sense.