Skip to content
This repository has been archived by the owner on Jan 25, 2018. It is now read-only.

Improve drag and drop to work better inside incremental dom components #3

Open
mairatma opened this issue Nov 10, 2016 · 5 comments
Open

Comments

@mairatma
Copy link
Contributor

This drag and drop module works really well on its own, but it's hard to configure it to work well inside incremental dom components. That's because it was created before we decided to use incremental dom in the first place, and so its architecture wasn't ready for it. Some examples of current problems:

  • It changes dragged dom elements directly. This can cause many problems with incremental dom components, since when they're updated via incremental dom they may lose the changes done by metal-drag-drop, since those are coming via a different way.
  • Drag and drop holds element references, but component dom elements can change a lot during updates. This causes components to have to keep updating the drag and drop instance with new references.
  • Drag and drop needs to receive both drag sources and drop targets in the same place, but they may be rendered by different components.

I believe that we should rewrite a good part of metal-drag-drop, having it be a component so that it can do its changes on the right elements via incremental dom as well. A good example of an api that achieves that is react-dnd.

This will probably need to be a major version, since the api would change a lot.

@p2kmgcl
Copy link

p2kmgcl commented Sep 25, 2017

Hi [edited]people[/edited], I am using this repo right know, and found that I need to resolve some of these problems, so I will be working on this library for a couple of days 😄

@p2kmgcl
Copy link

p2kmgcl commented Sep 27, 2017

Well, I have being trying to plan a rewrite, specially for "Drag and drop needs to receive both drag sources and drop targets in the same place, but they may be rendered by different components.". I have had a look at react-dnd documentation, and these are the main differences with metal-drag-drop:

  • It does not define an instance of ReactDND for managing the whole process; instead of that, it wraps everything inside a DragDropContext, a component that manages the elements being dragged, but it's only connection with the DOM are the synthetic drag and drop events.
  • Then it has DragSource and DropTarget higher order components used for wrapping the sources and targets being dragged. This make the component not querySelector or reference dependant, as every instance of these components it's responsible of it's specific interactions.
  • Finally there is a DragLayer that is automatically rendered inside DragDropContext and listens to native drag and drop events. For our case I think that this component could be omitted and merged with DragDropContext, as we do not need to server-side render this behaviour (I think).

And, in order to not break everything, this is my proposal:

  • Update DragDrop component, and allow managing a list of sources and targets internally, completely separated from the current implementation so it does not break anything. This could be performed by inheriting from Component instead of State, and performing different behaviour if there are/aren't children.
  • Create DragSource and DropTarget components that somehow tell DragDrop when they have been dragged/dropped and perform all the drag process inside DragDrop.

This could be a template using this behaviour:

{template .render}
    {call DragDropContext}
        <section>
            <h1>Here you can drop squares</h1>
            {call DropTarget}
                {param type: 'square' /}
                <div class="my-drop-target"></div>
            {/call}
        </section>

        <section>
            <h1>Here you can drop circles</h1>
            {call DropTarget}
                {param type: 'circle' /}
                <div class="my-other-drop-target"></div>
            {/call}
        </section>

        {call .otherTemplateOrComponent}{/call}
    {/call}
{/template}

{template .otherTemplateOrComponent}
    {foreach $squareOrCircle in $squaresOrCircles}
        {call DragSource}
            <div class="{$squareOrCircle}"></div>
        {/call}
    {/foreach}
{/template}

With this implementation we can separate the drag process from the elements themselves, and the also can keep DragSources and DropTargets in different parts the DOM tree. There are some problems that I found at the moment of thinking about the API:

  • For creating the drag-drop process, react-dnd uses a react feature called context, that basically allows react elements to inject some properties into certain children. This is how the perform the communication between DragSources, DropTargets, and the DragDropContext. I am not sure if this feature could be emulated using events (@jbalsas idea, I think it could work), but we need to move to something near to this if we want to stop depending on querySelectors.
  • In this template there is no support for handles (a DragSource can be dragged moving just a specific part of it), so I would have to think about how to implement that.
  • DragDropContext needs to create copies of these elements for the dragging placeholders and avoid rerenders during the process.
  • How am I going to implement it? Well, this a future Pablo's problem 😅

So... tell me how it looks please :)

/cc @Robert-Frampton
/cc @jbalsas

@p2kmgcl
Copy link

p2kmgcl commented Oct 20, 2017

TLDR: please forget about this for the moment

I have spoken with @jbalsas and I think we should leave this batch of changes to the future: it would imply a big rewrite of the library, and with some small patches I could get a similar behaviour (or at least the behaviour I need to implement with metal 😄)

I am creating a small example of the interface I want to implement in order to show the issues I am experimenting.

@robframpton
Copy link
Contributor

Hey @p2kmgcl

Any update on the example you planned making? It would definitely be helpful.

Also, given that we don't want to do a full rewrite at this time, I'm not sure what patches are possible to address the issue of incremental dom interfering with drag elements. It might just come down to writing your components in such a way that the drag elements never get updated by incremental dom, but maybe your example will help clear things up.

@p2kmgcl
Copy link

p2kmgcl commented Nov 2, 2017

Hi @Robert-Frampton, I opened another issue which the specific behaviour that I need (and there's a code sample too): #7

@pavel-savinov is also working with metal-drag-drop and added another issue a some days ago: #6

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants