search

theia-ide / sprotty

A next-gen web-based graphics framework
Apache License 2.0
138 stars 23 forks source link

Barriers name #87

Closed marcdumais-work closed 7 years ago

marcdumais-work commented 7 years ago

This pull request contains my patch to name the barriers and Miro's scope fix to make it work correctly.

JanKoehnlein commented 7 years ago

Two comments: 1) BarrierNodeSchema should also define the name property 2) Cosmetic: The unique name validation looks a bit strange a) It will produce multiple errors on the same element if more than two elements share the same name b) Instead of containsKey(key)and later get(key) with a null guard, we could use get and the null guard in the first place.

JanKoehnlein commented 7 years ago

Feel free to merge anyway.

marcdumais-work commented 7 years ago

Thanks for the review @JanKoehnlein . I have added a patch to address comment 1. About comment 2, I kind of like being shown all the places where there is a name clash problem. However I have changed the error message . Let me know what you think