search

freegroup / draw2d

Create Visio like drawings, diagrams or workflows with JavaScript and HTML5
https://freegroup.github.io/draw2d/#/examples
MIT License
738 stars 227 forks source link

Contribution of fixes and additions to main project #186

Closed rudolphi closed 3 years ago

rudolphi commented 3 years ago

Fixed Bugs #183, #182, #181, #180 Added shapes for flow chemistry

freegroup commented 3 years ago

fixing 4 bugs with different scopes in one pull request is not a good idea.

remove the "eval" break a lot of legacy application. I have tested your solution already a year before and this do not work in all setups with different package loader.

adding new objects can be solved just by include an independent lib instead of increase the lib size of draw2d for all users. I prefere to split the icons of the different packages into different libs/moduls and load them if required. Makes the core smaller.

face the call of removeChildren in the load operation breaks everything. What about shapes they do not rely on the serialization? What happens with the objects references?

Sorry I must reject this pull request.

rudolphi commented 3 years ago

Ok, thanks for your explanations, I understand. If I see a perspective to get my points solved "the right way", I am willing to help and contribute. Splitting the PRs is no problem, that is just a matter of organization.

With regard to the "eval vs recursive window[..] approach": under which circumstances does my solution not work? Is it a matter of the browser used, or the features used, the integration, anything else? Could we at least have a configuration option, letting the developer choose which way to go?

For adding new shapes as modules, could you make a quick example, maybe with the set of electronic components? I will then mimic your best practice. I believe that such extensions would bear a lot of potential.

With regard to the serialization: I would find great if whatever scheme the user constructs could be serialized and restored; shapes (assuming the class will be present when deserializing) incl. ports, children and connections, but no code or event handlers. I believe this is complex, but feasible. What would be the alternative, how could people use draw2d if this does not work?

Please do not see my actions as hostile; I wanted to solve problems, and not just write bug reports while leaving all the work to you. I tested multiple similar libraries before choosing yours, and I think "the glass is much closer to being full than to being empty".