Properly handle events fired by children of handle or cancel selectors.

Fixes #88
react-grid-layout · May 20, 2016 · a6b4789 · a6b4789
1 parent 028a391
commit a6b4789
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 14 deletions.
diff --git a/example/index.html b/example/index.html
@@ -146,7 +146,7 @@ <h1>React Draggable</h1>
           </Draggable>
           <Draggable handle="strong" {...dragHandlers}>
             <div className="box no-cursor">
-              <strong className="cursor">Drag here</strong>
+              <strong className="cursor"><div>Drag here</div></strong>
               <div>You must click my handle to drag me</div>
             </div>
           </Draggable>

diff --git a/lib/DraggableCore.es6 b/lib/DraggableCore.es6
@@ -1,6 +1,7 @@
 // @flow
 import React, {PropTypes} from 'react';
-import {matchesSelector, addEvent, removeEvent, addUserSelectStyles, getTouchIdentifier,
+import ReactDOM from 'react-dom';
+import {matchesSelectorAndParentsTo, addEvent, removeEvent, addUserSelectStyles, getTouchIdentifier,
         removeUserSelectStyles, styleHacks} from './utils/domFns';
 import {createCoreData, getControlPosition, snapToGrid} from './utils/positionFns';
 import {dontSetMe} from './utils/shims';
@@ -187,8 +188,8 @@ export default class DraggableCore extends React.Component {
     // Short circuit if handle or cancel prop was provided and selector doesn't match.
     if (this.props.disabled ||
       (!(e.target instanceof Node)) ||
-      (this.props.handle && !matchesSelector(e.target, this.props.handle)) ||
-      (this.props.cancel && matchesSelector(e.target, this.props.cancel))) {
+      (this.props.handle && !matchesSelectorAndParentsTo(e.target, this.props.handle, ReactDOM.findDOMNode(this))) ||
+      (this.props.cancel && matchesSelectorAndParentsTo(e.target, this.props.cancel, ReactDOM.findDOMNode(this)))) {
       return;
     }
 

diff --git a/lib/utils/domFns.es6 b/lib/utils/domFns.es6
@@ -23,6 +23,18 @@ export function matchesSelector(el: Node, selector: string): boolean {
   return el[matchesSelectorFunc].call(el, selector);
 }
 
+// Works up the tree to the draggable itself attempting to match selector.
+export function matchesSelectorAndParentsTo(el: Node, selector: string, baseNode: Node): boolean {
+  let node = el;
+  do {
+    if (matchesSelector(node, selector)) return true;
+    if (node === baseNode) return false;
+    node = node.parentNode;
+  } while (node);
+
+  return false;
+}
+
 export function addEvent(el: ?Node, event: string, handler: Function): void {
   if (!el) { return; }
   if (el.attachEvent) {

diff --git a/specs/draggable.spec.jsx b/specs/draggable.spec.jsx
@@ -322,6 +322,20 @@ describe('react-draggable', function () {
   });
 
   describe('interaction', function () {
+
+    function mouseDownOn(drag, selector, shouldDrag) {
+      resetDragging(drag);
+      const node = ReactDOM.findDOMNode(drag).querySelector(selector);
+      if (!node) throw new Error(`Selector not found: ${selector}`);
+      TestUtils.Simulate.mouseDown(node);
+      expect(drag.state.dragging).toEqual(shouldDrag);
+    }
+
+    function resetDragging(drag) {
+      TestUtils.Simulate.mouseUp(ReactDOM.findDOMNode(drag));
+      expect(drag.state.dragging).toEqual(false);
+    }
+
     it('should initialize dragging onmousedown', function () {
       drag = TestUtils.renderIntoDocument(<Draggable><div/></Draggable>);
 
@@ -339,11 +353,27 @@ describe('react-draggable', function () {
         </Draggable>
       );
 
-      TestUtils.Simulate.mouseDown(ReactDOM.findDOMNode(drag).querySelector('.content'));
-      expect(drag.state.dragging).toEqual(false);
+      mouseDownOn(drag, '.content', false);
+      mouseDownOn(drag, '.handle', true);
+    });
 
-      TestUtils.Simulate.mouseDown(ReactDOM.findDOMNode(drag).querySelector('.handle'));
-      expect(drag.state.dragging).toEqual(true);
+    it('should only initialize dragging onmousedown of handle, even if children fire event', function () {
+      drag = TestUtils.renderIntoDocument(
+        <Draggable handle=".handle">
+          <div>
+            <div className="handle">
+              <div><span><div className="deep">Handle</div></span></div>
+            </div>
+            <div className="content">Lorem ipsum...</div>
+          </div>
+        </Draggable>
+      );
+
+      mouseDownOn(drag, '.content', false);
+      mouseDownOn(drag, '.deep', true);
+      mouseDownOn(drag, '.handle > div', true);
+      mouseDownOn(drag, '.handle span', true);
+      mouseDownOn(drag, '.handle', true);
     });
 
     it('should not initialize dragging onmousedown of cancel', function () {
@@ -356,11 +386,27 @@ describe('react-draggable', function () {
         </Draggable>
       );
 
-      TestUtils.Simulate.mouseDown(ReactDOM.findDOMNode(drag).querySelector('.cancel'));
-      expect(drag.state.dragging).toEqual(false);
+      mouseDownOn(drag, '.cancel', false);
+      mouseDownOn(drag, '.content', true);
+    });
 
-      TestUtils.Simulate.mouseDown(ReactDOM.findDOMNode(drag).querySelector('.content'));
-      expect(drag.state.dragging).toEqual(true);
+    it('should not initialize dragging onmousedown of handle, even if children fire event', function () {
+      drag = TestUtils.renderIntoDocument(
+        <Draggable cancel=".cancel">
+          <div>
+            <div className="cancel">
+              <div><span><div className="deep">Cancel</div></span></div>
+            </div>
+            <div className="content">Lorem ipsum...</div>
+          </div>
+        </Draggable>
+      );
+
+      mouseDownOn(drag, '.content', true);
+      mouseDownOn(drag, '.deep', false);
+      mouseDownOn(drag, '.cancel > div', false);
+      mouseDownOn(drag, '.cancel span', false);
+      mouseDownOn(drag, '.cancel', false);
     });
 
     it('should discontinue dragging onmouseup', function () {
@@ -369,8 +415,7 @@ describe('react-draggable', function () {
       TestUtils.Simulate.mouseDown(ReactDOM.findDOMNode(drag));
       expect(drag.state.dragging).toEqual(true);
 
-      TestUtils.Simulate.mouseUp(ReactDOM.findDOMNode(drag));
-      expect(drag.state.dragging).toEqual(false);
+      resetDragging(drag);
     });
 
     it('should modulate position on scroll', function (done) {