Skip to content

Commit 1b8ea93

Browse files
committed
Don't allow passing style/className/transform to <Draggable> or <DraggableCore>
1 parent b859042 commit 1b8ea93

File tree

5 files changed

+71
-22
lines changed

5 files changed

+71
-22
lines changed

lib/Draggable.es6

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import classNames from 'classnames';
44
import assign from 'object-assign';
55
import {createUIEvent, createTransform} from './utils/domFns';
66
import {canDragX, canDragY, getBoundPosition, snapToGrid} from './utils/positionFns';
7+
import {dontSetMe} from './utils/shims';
78
import DraggableCore from './DraggableCore';
89
import log from './utils/log';
910

@@ -140,7 +141,14 @@ export default class Draggable extends DraggableCore {
140141
* });
141142
* ```
142143
*/
143-
zIndex: PropTypes.number
144+
zIndex: PropTypes.number,
145+
146+
/**
147+
* These properties should be defined on the child, not here.
148+
*/
149+
className: dontSetMe,
150+
style: dontSetMe,
151+
transform: dontSetMe
144152
});
145153

146154
static defaultProps = assign({}, DraggableCore.defaultProps, {
@@ -265,9 +273,12 @@ export default class Draggable extends DraggableCore {
265273
// Reuse the child provided
266274
// This makes it flexible to use whatever element is wanted (div, ul, etc)
267275
return (
268-
<DraggableCore {...this.props} style={style} className={className} transform={svgTransform}
269-
onStart={this.onDragStart} onDrag={this.onDrag} onStop={this.onDragStop}>
270-
{React.Children.only(this.props.children)}
276+
<DraggableCore {...this.props} onStart={this.onDragStart} onDrag={this.onDrag} onStop={this.onDragStop}>
277+
{React.cloneElement(React.Children.only(this.props.children), {
278+
className: className,
279+
style: assign({}, this.props.children.props.style, style),
280+
transform: svgTransform
281+
})}
271282
</DraggableCore>
272283
);
273284
}

lib/DraggableCore.es6

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {default as React, PropTypes} from 'react';
22
import {matchesSelector, createCoreEvent, addEvent, removeEvent, addUserSelectStyles,
33
removeUserSelectStyles, styleHacks} from './utils/domFns';
44
import {getControlPosition} from './utils/positionFns';
5+
import {dontSetMe} from './utils/shims';
56
import log from './utils/log';
67

78
// Simple abstraction for dragging events names.
@@ -150,7 +151,14 @@ export default class DraggableCore extends React.Component {
150151
* A workaround option which can be passed if onMouseDown needs to be accessed,
151152
* since it'll always be blocked (due to that there's internal use of onMouseDown)
152153
*/
153-
onMouseDown: PropTypes.func
154+
onMouseDown: PropTypes.func,
155+
156+
/**
157+
* These properties should be defined on the child, not here.
158+
*/
159+
className: dontSetMe,
160+
style: dontSetMe,
161+
transform: dontSetMe
154162
};
155163

156164
static defaultProps = {
@@ -334,8 +342,7 @@ export default class DraggableCore extends React.Component {
334342
// Reuse the child provided
335343
// This makes it flexible to use whatever element is wanted (div, ul, etc)
336344
return React.cloneElement(React.Children.only(this.props.children), {
337-
style: styleHacks(this),
338-
transform: this.props.transform,
345+
style: styleHacks(this.props.children.props.style),
339346

340347
// Note: mouseMove handler is attached to document so it will still function
341348
// when the user drags quickly and leaves the bounds of the element.

lib/utils/domFns.es6

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,18 +114,14 @@ export function removeUserSelectStyles() {
114114
document.body.setAttribute('style', style.replace(userSelectStyle, ''));
115115
}
116116

117-
export function styleHacks(draggable) {
118-
// Create style object. We extend from existing styles so we don't
119-
// remove anything already set (like background, color, etc).
120-
let childStyle = draggable.props.children.props.style || {};
121-
117+
export function styleHacks(childStyle = {}) {
122118
// Workaround IE pointer events; see #51
123119
// https://github.com/mzabriskie/react-draggable/issues/51#issuecomment-103488278
124120
let touchHacks = {
125121
touchAction: 'none'
126122
};
127123

128-
return assign(touchHacks, childStyle, draggable.props.style);
124+
return assign(touchHacks, childStyle);
129125
}
130126

131127
// Create an event exposed by <DraggableCore>

lib/utils/shims.es6

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,9 @@ export function isNum(num) {
1616
export function int(a) {
1717
return parseInt(a, 10);
1818
}
19+
20+
export function dontSetMe(props, propName, componentName) {
21+
if (props[propName]) {
22+
throw new Error(`Invalid prop ${propName} passed to ${componentName} - do not set this, set it on the child.`);
23+
}
24+
}

specs/draggable.spec.jsx

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ var dashedBrowserPrefix = browserPrefix ? '-' + browserPrefix.toLowerCase() + '-
1111
describe('react-draggable', function () {
1212
var drag;
1313

14+
beforeEach(function() {
15+
spyOn(console, 'error');
16+
});
17+
1418
afterEach(function() {
1519
try {
1620
ReactDOM.findDOMNode(drag);
@@ -35,7 +39,7 @@ describe('react-draggable', function () {
3539
it('should pass style and className properly from child', function () {
3640
drag = (<Draggable><div className="foo" style={{color: 'black'}}/></Draggable>);
3741

38-
expect(renderToHTML(drag)).toEqual('<div class="foo" ' +
42+
expect(renderToHTML(drag)).toEqual('<div class="foo react-draggable" ' +
3943
'style="touch-action:none;color:black;transform:translate(0px,0px);' + dashedBrowserPrefix +
4044
'transform:translate(0px,0px);" data-reactid=".1"></div>');
4145
});
@@ -48,13 +52,14 @@ describe('react-draggable', function () {
4852
var output = renderer.getRenderOutput();
4953

5054
var expected = (
51-
<DraggableCore {...Draggable.defaultProps} handle=".foo" className="react-draggable"
52-
style={{
53-
'transform': 'translate(0px,0px)',
54-
[browserPrefix + 'Transform']: 'translate(0px,0px)'
55-
}}
56-
transform={null}>
57-
<div />
55+
<DraggableCore {...Draggable.defaultProps} handle=".foo">
56+
<div
57+
className="react-draggable"
58+
style={{
59+
'transform': 'translate(0px,0px)',
60+
[browserPrefix + 'Transform']: 'translate(0px,0px)'
61+
}}
62+
transform={null} />
5863
</DraggableCore>
5964
);
6065
// Not easy to actually test equality here. The functions are bound as static props so we can't test those easily.
@@ -80,7 +85,7 @@ describe('react-draggable', function () {
8085
onStart={handleStart}
8186
onDrag={handleDrag}
8287
onStop={handleStop}>
83-
<div>
88+
<div className="foo bar">
8489
<div className="handle"/>
8590
<div className="cancel"/>
8691
</div>
@@ -97,6 +102,30 @@ describe('react-draggable', function () {
97102
expect(drag.props.onStop).toEqual(handleStop);
98103
});
99104

105+
it('should throw when setting className', function () {
106+
drag = (<Draggable className="foo"><span /></Draggable>);
107+
108+
TestUtils.renderIntoDocument(drag);
109+
110+
expect(console.error).toHaveBeenCalledWith('Warning: Failed propType: Invalid prop className passed to Draggable - do not set this, set it on the child.');
111+
});
112+
113+
it('should throw when setting style', function () {
114+
drag = (<Draggable style={{color: 'red'}}><span /></Draggable>);
115+
116+
TestUtils.renderIntoDocument(drag);
117+
118+
expect(console.error).toHaveBeenCalledWith('Warning: Failed propType: Invalid prop style passed to Draggable - do not set this, set it on the child.');
119+
});
120+
121+
it('should throw when setting transform', function () {
122+
drag = (<Draggable transform="translate(100, 100)"><span /></Draggable>);
123+
124+
TestUtils.renderIntoDocument(drag);
125+
126+
expect(console.error).toHaveBeenCalledWith('Warning: Failed propType: Invalid prop transform passed to Draggable - do not set this, set it on the child.');
127+
});
128+
100129
it('should call onStart when dragging begins', function () {
101130
var called = false;
102131
drag = TestUtils.renderIntoDocument(

0 commit comments

Comments
 (0)