blazor-dragdrop
blazor-dragdrop copied to clipboard
Dropzone should not modify source and destination item list by default
Hi there,
the current implementation of the drop handling is strongly opinionated.
This makes is really difficult if not impossible to use the package in scenarios where you don't want to modify the source and destination item list by default. The reason for this would be the requirement to execute some logic after drop and only if the logic succeeds the operation is considered a success. Modifying source and destination right away makes no sense here as the logic can fail and modifications cannot easily be undone right now.
To avoid breaking existing implementations I suggest we add another event OnItemDropping
. This event receives a parameter SkipDefaultHandling
which can be set to true to skip the copy / swap / instant replace handling. The component can now skip the default handling, raise OnItemDrop
, the component consumer can execute logic in this event and when the logic succeeds its up to the consumer to modify the lists accordingly.
@Postlagerkarte What do you say?
Great! I would also be open to make the new custom handling the default and force users to set a flag to enable the old "auto" handling. Would make sense if we want to drop support for the old handling in a future version. Do you prefer that or should we keep the current as default and make the new way opt-in as suggested above?
I was primarily searching for a way to extend the component but I'm also keen on adding a parameter that sets the default behavior to avoid increasing the complexity even further.
My suggestion is to add a parameter named DropAction
of type enum whereas the enum describes the current actions possible plus an no-op action and / or callback action. This would also improve the readability of the code as currently the if else blocks are hard to grasp.
I guess I should create a PR and discuss things there when needed, right?
Yes, go ahead!
Is there a reason why OnDropItemOnSpacing
and OnDrop
is a synchronous method?
I ask this because I pretty much discarded the enum idea already as it just refactores the existing if else blocks into new if else blocks. Currently I'm trying the initial idea with a EventCallback but with a synchronous method I cannot await the EventCallback.
Also could you please leave the issue open until it is resolved / implemented? I'm tracking open issues instead of open PRs.
No special reason for those method being synchronous. What about dropping the current implementation completely? I am not opposed to breaking changes if it is necessary and reduces complexity. As you pointed out the idea that the component handles the list manipulation is sub optimal anyways. I assume that migrating should be very easy for users. Don't you think so?
What about dropping the current implementation completely? I am not opposed to breaking changes if it is necessary and reduces complexity. As you pointed out the idea that the component handles the list manipulation is sub optimal anyways.
Yes, I understand your point but dropping the current default behavior (move item from a to b) would break all existing implementations. I think new consumers of this component will expect the component to at least implement a drag & drop action by default to get started right away.
I have started a PR regarding the EventCallback approach. To me its a good solution as it enables custom actions, it defaults to move action to avoid breaking implementations as much as possible and code change is also minimal and I think the implementation of the action classes are as transparent as it gets.
Lets discuss the PR in review comments.