Make react-draggable agnostic of the HTML structure position

[cosminnicula]

This is related to #56

Let's have a look at the ReactBootstrap and Bootstrap Modals side by side. I have updated the repository to include a Bootstrap example also https://github.com/cosminnicula/ReactBootstrapModalDraggable

1.Bootstrap -> this works

<Draggable handle=".modal-header">
    <div className="modal">
        <div className="modal-dialog">
            <div className="modal-content">
                <div className="modal-header">
                ...
                </div>
            </div>
        </div>
</Draggable>

2.ReactBootstrap -> this does not work

<Draggable handle=".modal-header">
    <Modal>
        ...
    </Modal>
</Draggable>

Under the hood, <Modal /> generates the same HTML mark-up as 1.

Questions:
a) Since 2. is equivalent to 1. in terms of HTML mark-up, why <Dragglable /> would not work with 2. ? (not asking technical details, but rather as a client of the library)
b) If <Draggable /> wants to cover more use cases, it has to be more flexible in terms where in the HTML structure needs to be included. E.g. if ReactBootstrap, or any other component decides not to expose a renderer function or a custom component, then <Draggable /> cannot be used. A solution would be for <Draggable /> to be agnostic of the HTML structure position and rely only on the handle property. Is this make sense?

Would ❤️ to see these two questions addressed!

[STRML]

The difference is that they are not equivalent in terms of how they are actually translated into markup.

As I explained earlier, the does not directly translate to:

<div className="modal">...</div>

See the source - there is a backdrop and more.

This is likely not possible to without defining some sort of className filter and having react-draggable actually traverse its child's tree in order to determine an injection point. If you would like to tackle that, please be my guest.

[cosminnicula]

Have a look at the React Bootstrap Modal code here http://bit.ly/1dgkDmn. You can see at line 68 that rendering the backdrop is optional, so we can leave this behind.

Regarding the "className filter", isn't this what the handle property should do: traverse its child's tree and determine an injection point based on the selector specified? If so, then there must be another reason why this is not working. And that's why I'm addressing this here. I haven't got the time to look at a possible solution, but it would be great if there would be a solve for this.

What I'm trying to point out here is that react-draggable would probably be used more often in composition with another React component like React Bootstrap Modal or others (example 2), than with static content (example 1).

[jmuerle]

I agree with cosminnicula. Fixing this relatively small issue would have a big impact on the usability of this component. As of right now I find myself making wrapper html elements for my React components if I want them to be draggable, which bloats my jsx and obfuscates the true purpose of my React components.

<Draggable>
  <div id="wrapper">
    <MyReactComponent />
  </div>
</Draggable>

The above is my workaround, and is pretty clearly suboptimal.

[STRML]

It's not a small or simple issue and there isn't a simple way to fix it because we don't know what the top level of a child component will export. I welcome any contributions.

[jmuerle]

Is it possible to use https://facebook.github.io/react/docs/top-level-api.html#react.finddomnode on componentDidMount to point the Draggable component to the top-level dom node? Currently it seems that you want a solution that can access the top-level DOM element before it's rendered. Whether that is possible I don't know, but I don't think it's absolutely necessary.

Have you thought about and/or explored options that allow the DOM to be rendered first? If so, why have those avenues been ruled out?

[STRML]

Accessing the top-level DOM node is not the problem. In the case of the bootstrap modal element, the top-level element is a wrapper that contains the modal itself and the backdrop. <Draggable> has no way to know this. Remember that in React, we must define our children declaratively, and we can only do so via props. If the component we're wrapping does not expose the requisite functionality via props, the best we can do is wrap it in a <div> and apply styles to that.

Selecting the draggable target with a CSS selector is not going to be easy without direct DOM munging, which is not how React is meant to be used and goes against the spirit of this component. At that point, it's no better than simply using something like jQuery-UI or any of the dozens of other draggable wrappers around the internet.

[jmuerle]

Maybe I'm confused because there are actually two issues at play here:

1. What you are saying, if I'm not mistaken, is that the actual Draggable item might be inside another React component at an arbitrary depth, and the Draggable library has no way of knowing this without traversing the DOM with a css selector. This makes sense, and I understand why you might be opposed to supporting this.
2. The issue I thought this thread was about was the fact that the Draggable component does not work at all if a React Component is the direct child of it. For instance,

<Draggable>
  <MyReactComponent />
</Draggable>

will not work for me--the component does not fire events and has no semblance of being draggable. But

<Draggable>
  <div id='wrapper>
    <MyReactComponent />
  </div>
</Draggable>

does work, because Draggable is able to attach itself to the div wrapper. At the least, I would expect the first code snippet to make the top-level DOM element draggable, but that does not happen.

Sorry if I'm being dense, and you are in fact addressing both issues, but I'm not sure if you are at the moment.

[STRML]

It is exactly that - both issues are the same, in that <Draggable> has little to no knowledge of its child. What is directly below the <Draggable> needs to be an element that we can manipulate the style of. If your component doesn't accept a style prop (and <Draggable> has no way of knowing if it does), then the style will be lost and your element won't be draggable. Rather than always insert a <span> (which may not be correct, especially if your component accepts a style prop, we rely on the user to know what is best.

[jmuerle]

Hmmm, I see now why implementing it the way it is now is the most flexible approach. I still am not quite satisfied with the current implementation, however, as I find it confusing that the Draggable component does not work around a non-DOM React component. Could it be detected whether the child element is a React DOM element, and then a wrapper element used in the case it isn't? If the user wishes to create their own wrapper, they could still do so, and the component would work no matter its children.

Do you think that this is a problem worthy of solving?

P.S. Whether this issue actually results in a fix or not, though, this behavior should be documented in the README so that future users of the library don't go through this headache.

[STRML]

There is no way I know of to detect if the child will actually respect
the style prop or not.

If you think the README needs updating, please write documentation that
you think would help you and submit it as a PR.

On 7/28/15 5:31 PM, jmuerle wrote:

Hmmm, I see now why implementing it the way it is now is the most
flexible approach. I still am not quite satisfied with the current
implementation, however, as I find it confusing that the Draggable
component does not work around a non-DOM React component. Could it be
detected whether the child element is a React DOM element, and then a
wrapper element used in the case it isn't? If the user wishes to
create their own wrapper, they could still do so, and the component
would work no matter its children.

Do you think that this is a problem worthy of solving?

P.S. Whether this issue actually results in a fix or not, though, this
behavior should be documented in the README so that future users of
the library don't go through this headache.

—
Reply to this email directly or view it on GitHub
#57 (comment).