isaacHagoel / svelte-dnd-action

An action based drag and drop container for Svelte
MIT License
1.77k stars 106 forks source link

Dropzone Accepting List of Types #380

Open ecoinomist opened 2 years ago

ecoinomist commented 2 years ago

Is there a way to define an array of type that can be dropped?

Use case: Dropzone: <Button> component that only accepts <Text> or <Icon> components: types = ['icon', 'text'] Draggable 1: <Icon> can be dropped inside <Button> Draggable 2: <Text> can be dropped inside <Button> Draggable 3: <Button> cannot be dropped inside another <Button>

Would be happy to submit a PR, if it does not likely involve complex logic because I'm new to this lib.

isaacHagoel commented 2 years ago

You can assign types to dndzones. If you want control at the item level you can tweak the drop from others disabled on other zones in your consider handler when drag starts. If you make a basic repl i can show u how to add this logic

On Sat, Jun 18, 2022, 03:53 Ecoinomist @.***> wrote:

Is there a way to define an array of type that can be dropped?

Use case: Dropzone: component that only accepts or components: types = ['icon', 'text'] Draggable 1: can be dropped inside Draggable 2: can be dropped inside Draggable 3: cannot be dropped inside another

Would be happy to submit a PR, if it does not likely involve complex logic because I'm new to this lib.

— Reply to this email directly, view it on GitHub https://github.com/isaacHagoel/svelte-dnd-action/issues/380, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE4OZC46WXR4U7NBCYFYCXLVPS3R7ANCNFSM5ZC7W7CQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

ecoinomist commented 2 years ago

Do you have an example of assigning types to dndzones? I did not see that in the documentation.

I need this config to be declarative, without writing adhoc consider handlers, because all nodes will be created dynamically via Flux/Redux like pattern; and all nodes can be recursively nested within any node by default (unless types array is defined).

The repl would be identical to the Crazy Nesting example, except for node 4 should only accept item, and reject dropping of node 1, node 2 and node 3 (i.e. accept types=['item']), while nodes from 1 to 3 can accept either item or other node (i.e. accept anything).

isaacHagoel commented 2 years ago

this example for dndzones with types appears in the readme

ecoinomist commented 2 years ago

I saw that example, it only has type (i.e. a single String).

What I'm proposing is an addition to the API of dndzones to have an additional types<String[]> prop that is a list of acceptable type strings. If types is an empty array, then make dndzone accept any type, if void 0/undefined/null, then fallback to a single type behavior like current API.

isaacHagoel commented 2 years ago

Ah, now I understand. This shouldn't be too complex to implement. There is a map from type to the zones in that type so it might just work as is with almost no changes (I hope). I suggest that the existing "type" prop will accept either a string (like it currently does) or an array of stings. If you'd like to make a PR that would be great

On Sat, Jun 18, 2022 at 9:45 PM Ecoinomist @.***> wrote:

I saw that example, it only has type (i.e. a single String).

What I'm proposing is an addition to the API of dndzones to have an additional types<String[]> prop that is a list of acceptable type strings. If types is an empty array, then make dndzone accept any type, if undefined/null, then fallback to a single type behavior like current API.

— Reply to this email directly, view it on GitHub https://github.com/isaacHagoel/svelte-dnd-action/issues/380#issuecomment-1159450091, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE4OZC766C46YKTJTODCPXTVPWZEDANCNFSM5ZC7W7CQ . You are receiving this because you commented.Message ID: @.***>

ecoinomist commented 2 years ago

Great, will look into it tonight and possibly submit PR.

I would keep the type as is, because you need that as a single string to identify the draggable node (acting as ID). Whereas types is only used for the dropzone node to validate acceptable type IDs (acting as validation).

In the current implementation, type act as both ID and validation. Hence it should not/cannot be an array to play the role of ID.

ecoinomist commented 2 years ago

perhaps, it may be more explicit to name it canDropTypes, instead of just types?

isaacHagoel commented 2 years ago

Ideally i want it to stay backwards compatible and keep the number of props under control (even if the original name is not ideal)

On Sat, Jun 18, 2022, 22:02 Ecoinomist @.***> wrote:

perhaps, it may be more explicit to name it canDropTypes, instead of just types?

— Reply to this email directly, view it on GitHub https://github.com/isaacHagoel/svelte-dnd-action/issues/380#issuecomment-1159452466, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE4OZCZGTUB2YXOCK3ZMQH3VPW3G7ANCNFSM5ZC7W7CQ . You are receiving this because you commented.Message ID: @.***>

ecoinomist commented 2 years ago

it will be backwards compatible, but additional prop is unavoidable for the desired use case.

In crazy nesting example, if I set:

<List type={['node', 'item']} ...>

recursively, then how do I know what type is being dragged to check if it's in the array of ['node', 'item']?

It can only be defined like this:

<List type={item.type} canDropTypes={['node', 'item']} ...>

Then internal lib logic becomes:

const canDrop = canDropTypes ? (canDropTypes.length === 0 || canDropTypes.includes(type)) : (currentType === type)

PS. exact use case example from react-dnd

ecoinomist commented 2 years ago

@isaacHagoel, I started writing the code, then realised this will be a major refactoring, because currently the dropTargets filtering logic is tightly coupled to the typeToDropZones.get(config.type) logic, which is used on almost all events, such as: watchDraggedElement, unWatchDraggedElement, handleDrop, handleDragStart, etc.

Here is a hypothetical filtering function:

// index.d.ts
export interface Options {
    items: Item[]; // the list of items that was used to generate the children of the given node
    type?: string; // the type of the dnd zone. children dragged from here can only be dropped in other zones of the same type, defaults to a base type
    accepts?: string[]; // accepted types of children. An empty array means any type of children can be dropped here
}
/**
 * Create a filter function to get a list of dropzone nodes that can accept dragged item from given originNode.
 * @example:
 *      const dropTargets = Array.from(typeToDropZones.get(config.type)) // => this does not give us all dnd-zones
 *          .filter(dropTargetsFilter(node, dzToConfig, config.type))
 *
 * @param {HTMLElement} originNode - the dropzone where dragged item originates from
 * @param {Map} dzToConfig - map of dropzone node elements to their respective config objects
 * @param {string} [type] - of the originNode dropzone
 * @returns {(function(HTMLElement): (boolean))}
 */
export function dropTargetsFilter (originNode, dzToConfig, type = dzToConfig.get(originNode).type) {
    return function (dz) {
        if (dz === originNode) return true // the target is the origin dropzone where drag started
        const {accepts, dropFromOthersDisabled} = dzToConfig.get(dz)
        return !dropFromOthersDisabled && (accepts ? (accepts.length === 0 || accepts.includes(type)) : true)
    }
}

In above code, what we actually need is something like this:

const dropTargets = allDropZones.filter(dropTargetsFilter(node, dzToConfig, config.type))

which means a lot of code will have to be refactored for all above mentioned events in keyboardAtion.js and pointerAction.js

Proposal: Encapsulate common events logic for keyboard vs pointer into separate functional utils, and abstract away the logic for computing drop targets.

I'll probably need more time to work out a clear architecture for my use cases, but it seems like a canDrop callback is mostly needed in the future for more granular control, like what react-dnd gives you.

Though I really like the efficiency, performant and intuitive direct DOM manipulation approach of this lib, because it allows intuitive animations that react-dnd cannot give you without major hassles.

Another idea is to merge mouse and touch events into one unified pointer events for better performance. Is there is any particular reason they are separate?

isaacHagoel commented 2 years ago

One second... I want to make sure we are on the same page. Let me try to repeat the problem to you and tell me if I got it right. We have three lists:

Items from A or B can be dragged into C. Items from C can sometimes be dragged into A or B (only a square into A, only a circle into B). If that's correct it means that the items need a type as well. Please confirm the above or correct me.

ecoinomist commented 2 years ago

Yes, you are correct. I believe we had different assumptions of what type and items mean.

Please confirm if I have a correct assumption of how it currently works:

What I actually need for my use case:

Back to your example:

C cannot be dragged into A or B, but Items from C can sometimes be dragged into A or B (only a square into A, only a circle into B). C or its Items can be dragged into another C.

isaacHagoel commented 2 years ago

So when an item from A is dragged into C, does it get "converted" to a triangle or lose its type? or does it keep being a "circle"?

On Sun, Jun 19, 2022 at 9:06 AM Ecoinomist @.***> wrote:

Yes, you are correct. I believe we had different assumptions of what type and items mean.

Please confirm if I have a correct assumption of how it currently works:

  • dnd-zone type applies to all children items being dragged from/to the container
  • a draggable item does not have an explicit type on its own.

What I actually need for my use case:

  • each node can be both a dnd-zone container and a draggable item (crazy nesting)
  • type is explicitly defined at item level, not implicitly though the dnd-zone that contains it.

Back to your example: A - type: "squares" B - type: "circles", accepts: ["circles"] C - type: "triangles", accepts: ["squares", "circles", "triangles"]

C cannot be dragged into A or B, but Items from C can sometimes be dragged into A or B (only a square into A, only a circle into B). C or its Items can be dragged into another C.

— Reply to this email directly, view it on GitHub https://github.com/isaacHagoel/svelte-dnd-action/issues/380#issuecomment-1159579353, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE4OZC2FJS55BZSAOHOQTQDVPZI67ANCNFSM5ZC7W7CQ . You are receiving this because you were mentioned.Message ID: @.***>

ecoinomist commented 2 years ago

an Item always keeps its type, no matter where it is.

isaacHagoel commented 2 years ago

That means that the items have to be marked as well. It is not enough to make the zones. Also, zone type can currently be changed on the fly and that can cause illegal states. As i said at the beginning, the behaviour your after can be achieved with a custom condier handler and a store. If you want i can make a repl to demo it when i have some spare time

On Sun, Jun 19, 2022, 19:23 Ecoinomist @.***> wrote:

and Item always keeps its type, no matter where it is.

— Reply to this email directly, view it on GitHub https://github.com/isaacHagoel/svelte-dnd-action/issues/380#issuecomment-1159666137, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE4OZC3ANTVUUF2RBR6GUP3VP3RH3ANCNFSM5ZC7W7CQ . You are receiving this because you were mentioned.Message ID: @.***>

ecoinomist commented 2 years ago

Thank you, but don't worry about making a repl, because my use case cannot be solved with a custom consider handler.

I'm creating a drag and drop UI builder interface, similar to WordPress Elementor plugin, that creates entire UI with just static JSON files, like this example (down the bottom you can download meta.json to see how it is created). Hence, the need for a declarative approach.

I should experiment by forking this lib first, and redesigning it with the missing APIs for multi targets dropzone. What I really like about your lib, is the fact that it's super simple to setup.

isaacHagoel commented 2 years ago

Up to you but in my opinion it would be easy to wrap the action provided by the lib with another action that encapsulates the consider handler and expose a declarative api

On Sun, Jun 19, 2022, 20:07 Ecoinomist @.***> wrote:

Thank you, but don't worry about making a repl, because my use case cannot be solved with a custom consider handler.

I'm creating a drag and drop UI builder interface, similar to WordPress Elementor plugin, that creates entire UI with just static JSON files, like this example https://eisgroup.github.io/ui-render/examples#all (down the bottom you can download meta.json to see how it is created). Hence, the need for a declarative approach.

I should experiment by forking this lib first, and redesigning it with the missing APIs for multi targets dropzone. What I really like about your lib, is the fact that it's super simple to setup.

— Reply to this email directly, view it on GitHub https://github.com/isaacHagoel/svelte-dnd-action/issues/380#issuecomment-1159677072, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE4OZC4CC7FW4DMT4ILOPTDVP3WPJANCNFSM5ZC7W7CQ . You are receiving this because you were mentioned.Message ID: @.***>

ecoinomist commented 2 years ago

Wrapping the consider handler could be a good idea, thank you for the suggestion.

My other concern is performance, because I hit React performance ceiling several times, from tables with lots of complex rows, to large redux stores with geolocation map, and 3D Babylon.js integration with too many event listeners (the CPU usage shoots up crazy).

With this use case, the app can simultaneously have multiple page editor instances (similar to multiple WP Elementor instances), each having hundreds of dropzone nodes. Hence repeatedly running a consider handler function on all of them, which easily adds up to thousands, is definitely going to be a bottle neck.

I'm currently looking at a slightly different approach, only attaching pointer events to the container dnd-zone (list), and surgically manipulate direct children (item) on move events. This should drastically reduce the number of event listener callbacks. An adhoc implementation of such strategy has proven to work well in the past with long infinite list of items.