search

Blazor-Diagrams / Blazor.Diagrams

A fully customizable and extensible all-purpose diagrams library for Blazor
https://blazor-diagrams.zhaytam.com
MIT License
993 stars 196 forks source link

Change ' ShouldDelete*' methods to invoke 'Func' only once. #231

Closed snehabihani closed 11 months ago

snehabihani commented 2 years ago

Before this change 'ShouldDelete*' would call passed 'Func' once for every selection. With this change, it calls 'Func' only once for multiple selections. Issue discussed : https://github.com/Blazor-Diagrams/Blazor.Diagrams/issues/164

snehabihani commented 2 years ago

Please review this once you get time.

zHaytam commented 2 years ago

Hello. sorry for the late reply. I'm just still thinking to see whether this approach is the best one, I have 2 worries right now:

  1. Is this efficient? Creating 3 arrays (looping 3 times in your case too) might be too much
  2. What if the user asks to delete 2 nodes, but you can only delete 1 of them. In your case, if you return false you wouldn't delete anything, is that the desired behavior?

What do you think?

Here's another thought: What about having one ShouldDeleteModels? (Worry 2 is still there though)

snehabihani commented 2 years ago

I have made changes for the same. Removed multiple for loops . Please let me know can we do this way?

snehabihani commented 2 years ago

Is this solution will work for you? Please let me know so we can discuss further

snehabihani commented 2 years ago

Hi It will be great if you suggest something on this..

zHaytam commented 2 years ago

Hi sorry, I'm still thinking whether this is the best solution that we can do.

snehabihani commented 2 years ago

Please give your thoughts. So will make changes as required.