From 11f3153bc5c1cbfd4918262a83100d6195c373a9 Mon Sep 17 00:00:00 2001 From: Philipp Giese Date: Wed, 30 Nov 2016 09:49:46 +0100 Subject: [PATCH 1/6] clauderic/react-sortable-hoc#98 do not pass private props to wrapped component - removed number of instance properties - removed unused method getHelperDimensions from props and class --- src/SortableContainer/index.js | 94 +++++++++++++++++++--------------- src/SortableElement/index.js | 15 ++++-- src/SortableHandle/index.js | 8 +-- src/index.js | 5 ++ src/utils.js | 6 +++ 5 files changed, 79 insertions(+), 49 deletions(-) diff --git a/src/SortableContainer/index.js b/src/SortableContainer/index.js index 5c050260a..130c3a92a 100644 --- a/src/SortableContainer/index.js +++ b/src/SortableContainer/index.js @@ -1,11 +1,13 @@ import React, {Component, PropTypes} from 'react'; import ReactDOM from 'react-dom'; -import Manager from '../Manager'; -import {closest, events, vendorPrefix, limit, getElementMargin} from '../utils'; +import { omit } from 'lodash' import invariant from 'invariant'; +import Manager from '../Manager'; +import {closest, events, vendorPrefix, limit, getElementMargin, provideDisplayName} from '../utils'; + // Export Higher Order Sortable Container Component -export default function SortableContainer(WrappedComponent, config = {withRef: false}) { +export default function sortableContainer(WrappedComponent, config = {withRef: false}) { return class extends Component { constructor(props) { super(props); @@ -24,9 +26,7 @@ export default function SortableContainer(WrappedComponent, config = {withRef: f this.state = {}; } - static displayName = (WrappedComponent.displayName) ? `SortableList(${WrappedComponent.displayName})` : 'SortableList'; - - static WrappedComponent = WrappedComponent; + static displayName = provideDisplayName('sortableList', WrappedComponent); static defaultProps = { axis: 'y', @@ -43,11 +43,7 @@ export default function SortableContainer(WrappedComponent, config = {withRef: f } }, lockToContainerEdges: false, - lockOffset: '50%', - getHelperDimensions: ({node}) => ({ - width: node.offsetWidth, - height: node.offsetHeight - }) + lockOffset: '50%' }; static propTypes = { @@ -71,8 +67,7 @@ export default function SortableContainer(WrappedComponent, config = {withRef: f PropTypes.string, PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.number, PropTypes.string])) ]), - getContainer: PropTypes.func, - getHelperDimensions: PropTypes.func + getContainer: PropTypes.func }; static childContextTypes = { @@ -168,22 +163,17 @@ export default function SortableContainer(WrappedComponent, config = {withRef: f const active = this.manager.getActive(); if (active) { - const {axis, onSortStart, helperClass, hideSortableGhost, useWindowAsScrollContainer, getHelperDimensions} = this.props; + const {axis, onSortStart, helperClass, hideSortableGhost, useWindowAsScrollContainer} = this.props; let {node, collection} = active; const {index} = node.sortableInfo; const margin = getElementMargin(node); const containerBoundingRect = this.container.getBoundingClientRect(); - const dimensions = getHelperDimensions({index, node, collection}) this.node = node; - this.width = dimensions.width - this.height = dimensions.height this.margin = margin; this.width = node.offsetWidth; this.height = node.offsetHeight; - this.halfWidth = this.width / 2; - this.halfHeight = this.height / 2; this.marginOffset = { x: this.margin.left + this.margin.right, y: Math.max(this.margin.top, this.margin.bottom) @@ -219,12 +209,12 @@ export default function SortableContainer(WrappedComponent, config = {withRef: f this.minTranslate = {}; this.maxTranslate = {}; if (this.axis.x) { - this.minTranslate.x = ((useWindowAsScrollContainer) ? 0 : containerBoundingRect.left) - this.boundingClientRect.left - this.halfWidth; - this.maxTranslate.x = ((useWindowAsScrollContainer) ? this.contentWindow.innerWidth : containerBoundingRect.left + containerBoundingRect.width) - this.boundingClientRect.left - this.halfWidth; + this.minTranslate.x = ((useWindowAsScrollContainer) ? 0 : containerBoundingRect.left) - this.boundingClientRect.left - (this.width / 2); + this.maxTranslate.x = ((useWindowAsScrollContainer) ? this.contentWindow.innerWidth : containerBoundingRect.left + containerBoundingRect.width) - this.boundingClientRect.left - (this.width / 2); } if (this.axis.y) { - this.minTranslate.y = ((useWindowAsScrollContainer) ? 0 : containerBoundingRect.top) - this.boundingClientRect.top - this.halfHeight; - this.maxTranslate.y = ((useWindowAsScrollContainer) ? this.contentWindow.innerHeight : containerBoundingRect.top + containerBoundingRect.height) - this.boundingClientRect.top - this.halfHeight; + this.minTranslate.y = ((useWindowAsScrollContainer) ? 0 : containerBoundingRect.top) - this.boundingClientRect.top - (this.height / 2); + this.maxTranslate.y = ((useWindowAsScrollContainer) ? this.contentWindow.innerHeight : containerBoundingRect.top + containerBoundingRect.height) - this.boundingClientRect.top - (this.height / 2); } if (helperClass) { @@ -400,12 +390,12 @@ export default function SortableContainer(WrappedComponent, config = {withRef: f if (lockToContainerEdges) { const [minLockOffset, maxLockOffset] = this.getLockPixelOffsets(); const minOffset = { - x: this.halfWidth - minLockOffset.x, - y: this.halfHeight - minLockOffset.y + x: (this.width / 2) - minLockOffset.x, + y: (this.height / 2) - minLockOffset.y }; const maxOffset = { - x: this.halfWidth - maxLockOffset.x, - y: this.halfHeight - maxLockOffset.y + x: (this.width / 2) - maxLockOffset.x, + y: (this.height / 2) - maxLockOffset.y }; translate.x = limit( @@ -449,12 +439,10 @@ export default function SortableContainer(WrappedComponent, config = {withRef: f let {node, edgeOffset} = nodes[i]; const index = node.sortableInfo.index; const width = node.offsetWidth; - const halfWidth = width / 2; const height = node.offsetHeight; - const halfHeight = height / 2; const offset = { - width: (this.width > width) ? halfWidth : this.halfWidth, - height: (this.height > height) ? halfHeight : this.halfHeight + width: (this.width > width) ? (width / 2) : (this.width / 2), + height: (this.height > height) ? (height / 2) : (this.height / 2) }; let translate = { x: 0, @@ -580,21 +568,21 @@ export default function SortableContainer(WrappedComponent, config = {withRef: f y: 10 }; - if (translate.y >= this.maxTranslate.y - this.halfHeight) { + if (translate.y >= this.maxTranslate.y - (this.height / 2)) { direction.y = 1; // Scroll Down - speed.y = acceleration.y * Math.abs((this.maxTranslate.y - this.halfHeight - translate.y) / this.height); + speed.y = acceleration.y * Math.abs((this.maxTranslate.y - (this.height / 2) - translate.y) / this.height); } - else if (translate.x >= this.maxTranslate.x - this.halfWidth) { + else if (translate.x >= this.maxTranslate.x - (this.width / 2)) { direction.x = 1; // Scroll Right - speed.x = acceleration.x * Math.abs((this.maxTranslate.x - this.halfWidth - translate.x) / this.width); + speed.x = acceleration.x * Math.abs((this.maxTranslate.x - (this.width / 2) - translate.x) / this.width); } - else if (translate.y <= this.minTranslate.y + this.halfHeight) { + else if (translate.y <= this.minTranslate.y + (this.height / 2)) { direction.y = -1; // Scroll Up - speed.y = acceleration.y * Math.abs((translate.y - this.halfHeight - this.minTranslate.y) / this.height); + speed.y = acceleration.y * Math.abs((translate.y - (this.height / 2) - this.minTranslate.y) / this.height); } - else if (translate.x <= this.minTranslate.x + this.halfWidth) { + else if (translate.x <= this.minTranslate.x + (this.width / 2)) { direction.x = -1; // Scroll Left - speed.x = acceleration.x * Math.abs((translate.x - this.halfWidth - this.minTranslate.x) / this.width); + speed.x = acceleration.x * Math.abs((translate.x - (this.width / 2) - this.minTranslate.x) / this.width); } if (this.autoscrollInterval) { @@ -627,7 +615,33 @@ export default function SortableContainer(WrappedComponent, config = {withRef: f render() { const ref = (config.withRef) ? 'wrappedInstance' : null; - return ; + return ( + + ); } }; } diff --git a/src/SortableElement/index.js b/src/SortableElement/index.js index 1836806a9..cd82960d7 100644 --- a/src/SortableElement/index.js +++ b/src/SortableElement/index.js @@ -1,13 +1,14 @@ import React, {Component, PropTypes} from 'react'; import {findDOMNode} from 'react-dom'; +import { omit } from 'lodash' import invariant from 'invariant'; +import { provideDisplayName } from '../utils' + // Export Higher Order Sortable Element Component -export default function SortableElement (WrappedComponent, config = {withRef: false}) { +export default function sortableElement (WrappedComponent, config = {withRef: false}) { return class extends Component { - static displayName = (WrappedComponent.displayName) ? `SortableElement(${WrappedComponent.displayName})` : 'SortableElement'; - - static WrappedComponent = WrappedComponent; + static displayName = provideDisplayName('sortableElement', WrappedComponent); static contextTypes = { manager: PropTypes.object.isRequired @@ -75,8 +76,12 @@ export default function SortableElement (WrappedComponent, config = {withRef: fa render() { const ref = (config.withRef) ? 'wrappedInstance' : null; + return ( - + ); } }; diff --git a/src/SortableHandle/index.js b/src/SortableHandle/index.js index 75134f045..7621e47b5 100644 --- a/src/SortableHandle/index.js +++ b/src/SortableHandle/index.js @@ -2,12 +2,12 @@ import React, {Component} from 'react'; import {findDOMNode} from 'react-dom'; import invariant from 'invariant'; +import { provideDisplayName } from '../utils' + // Export Higher Order Sortable Element Component -export default function SortableHandle (WrappedComponent, config = {withRef: false}) { +export default function sortableHandle (WrappedComponent, config = {withRef: false}) { return class extends Component { - static displayName = (WrappedComponent.displayName) ? `SortableHandle(${WrappedComponent.displayName})` : 'SortableHandle'; - - static WrappedComponent = WrappedComponent; + static displayName = provideDisplayName('sortableHandle', WrappedComponent) componentDidMount() { let node = findDOMNode(this); diff --git a/src/index.js b/src/index.js index 22364a600..7d47c9c13 100644 --- a/src/index.js +++ b/src/index.js @@ -1,4 +1,9 @@ export SortableContainer from './SortableContainer'; export SortableElement from './SortableElement'; export SortableHandle from './SortableHandle'; + +export sortableContainer from './SortableContainer'; +export sortableElement from './SortableElement'; +export sortableHandle from './SortableHandle'; + export {arrayMove} from './utils'; diff --git a/src/utils.js b/src/utils.js index 5f04a5074..0dadeca91 100644 --- a/src/utils.js +++ b/src/utils.js @@ -67,3 +67,9 @@ export function getElementMargin(element) { left: getCSSPixelValue(style.marginLeft) }; } + +export function provideDisplayName(prefix, Component) { + const componentName = Component.displayName || Component.name + + return componentName ? `${prefix}(${componentName})` : prefix; +} From 1b67c87db5771c4b4ff3116a96357b09da6f38c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Claud=C3=A9ric=20Demers?= Date: Thu, 1 Dec 2016 11:56:12 -0500 Subject: [PATCH 2/6] nit: cherry-pick lodash methods for smaller bundle --- src/SortableContainer/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SortableContainer/index.js b/src/SortableContainer/index.js index 130c3a92a..753974ad8 100644 --- a/src/SortableContainer/index.js +++ b/src/SortableContainer/index.js @@ -1,6 +1,6 @@ import React, {Component, PropTypes} from 'react'; import ReactDOM from 'react-dom'; -import { omit } from 'lodash' +import omit from 'lodash/omit' import invariant from 'invariant'; import Manager from '../Manager'; From 0ebddc2555a75bf7d7ea734362c2af6fd91d423e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Claud=C3=A9ric=20Demers?= Date: Mon, 5 Dec 2016 10:07:12 -0500 Subject: [PATCH 3/6] fix: restore getHelperDimensions + remove duplicate declaration --- src/SortableContainer/index.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/SortableContainer/index.js b/src/SortableContainer/index.js index 753974ad8..93b174250 100644 --- a/src/SortableContainer/index.js +++ b/src/SortableContainer/index.js @@ -43,7 +43,11 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f } }, lockToContainerEdges: false, - lockOffset: '50%' + lockOffset: '50%', + getHelperDimensions: ({node}) => ({ + - width: node.offsetWidth, + - height: node.offsetHeight + - }) }; static propTypes = { @@ -67,7 +71,8 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f PropTypes.string, PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.number, PropTypes.string])) ]), - getContainer: PropTypes.func + getContainer: PropTypes.func, + getHelperDimensions: PropTypes.func }; static childContextTypes = { @@ -163,17 +168,18 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f const active = this.manager.getActive(); if (active) { - const {axis, onSortStart, helperClass, hideSortableGhost, useWindowAsScrollContainer} = this.props; + const {axis, getHelperDimensions, helperClass, hideSortableGhost, onSortStart, useWindowAsScrollContainer} = this.props; let {node, collection} = active; const {index} = node.sortableInfo; const margin = getElementMargin(node); const containerBoundingRect = this.container.getBoundingClientRect(); + const dimensions = getHelperDimensions({index, node, collection}); this.node = node; this.margin = margin; - this.width = node.offsetWidth; - this.height = node.offsetHeight; + this.width = dimensions.width; + - this.height = dimensions.height; this.marginOffset = { x: this.margin.left + this.margin.right, y: Math.max(this.margin.top, this.margin.bottom) From e235a1614355b3cba27452c5d4764b1ca9331bcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Claud=C3=A9ric=20Demers?= Date: Mon, 5 Dec 2016 10:10:19 -0500 Subject: [PATCH 4/6] fix: linting error due to copy paste --- src/SortableContainer/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SortableContainer/index.js b/src/SortableContainer/index.js index 93b174250..8f5c82ae4 100644 --- a/src/SortableContainer/index.js +++ b/src/SortableContainer/index.js @@ -45,9 +45,9 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f lockToContainerEdges: false, lockOffset: '50%', getHelperDimensions: ({node}) => ({ - - width: node.offsetWidth, - - height: node.offsetHeight - - }) + width: node.offsetWidth, + height: node.offsetHeight + }) }; static propTypes = { From b928de54024449b3388f6c59884d0d3d30688d44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Claud=C3=A9ric=20Demers?= Date: Mon, 5 Dec 2016 10:22:11 -0500 Subject: [PATCH 5/6] =?UTF-8?q?fix:=20copy=20pasting=20is=20dangerous=20?= =?UTF-8?q?=F0=9F=98=82?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/SortableContainer/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SortableContainer/index.js b/src/SortableContainer/index.js index 8f5c82ae4..f9c72ca6b 100644 --- a/src/SortableContainer/index.js +++ b/src/SortableContainer/index.js @@ -179,7 +179,7 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f this.node = node; this.margin = margin; this.width = dimensions.width; - - this.height = dimensions.height; + this.height = dimensions.height; this.marginOffset = { x: this.margin.left + this.margin.right, y: Math.max(this.margin.top, this.margin.bottom) From bb8d406774cff7be58f13cd63594f2ca7b92b27a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Claud=C3=A9ric=20Demers?= Date: Mon, 5 Dec 2016 10:26:57 -0500 Subject: [PATCH 6/6] nit: mixed spaces and tabs --- src/SortableContainer/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SortableContainer/index.js b/src/SortableContainer/index.js index f9c72ca6b..d864e71c0 100644 --- a/src/SortableContainer/index.js +++ b/src/SortableContainer/index.js @@ -179,7 +179,7 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f this.node = node; this.margin = margin; this.width = dimensions.width; - this.height = dimensions.height; + this.height = dimensions.height; this.marginOffset = { x: this.margin.left + this.margin.right, y: Math.max(this.margin.top, this.margin.bottom)