From ee2cd9a6922bac4edacd7ed9fbd048060b742c7b Mon Sep 17 00:00:00 2001 From: Amin Mirzaee Date: Mon, 6 Nov 2017 05:32:59 -0500 Subject: [PATCH 1/8] enable animationed return for helper div --- src/SortableContainer/index.js | 108 ++++++++++++++++++++++++--------- 1 file changed, 80 insertions(+), 28 deletions(-) diff --git a/src/SortableContainer/index.js b/src/SortableContainer/index.js index f84741ed8..ead580c23 100644 --- a/src/SortableContainer/index.js +++ b/src/SortableContainer/index.js @@ -144,6 +144,13 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f } } } + componentWillUpdate(nextProps, nextState) { + if (this.cleanupTimeout) { + clearTimeout(this.cleanupTimeout); + this.cleanupTimeout = null; + this.performCleanup(); + } + } handleStart = e => { const {distance, shouldCancelStart} = this.props; @@ -177,10 +184,10 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f this.manager.active = {index, collection}; /* - * Fixes a bug in Firefox where the :active state of anchor tags - * prevent subsequent 'mousemove' events from being fired - * (see https://github.com/clauderic/react-sortable-hoc/issues/118) - */ + * Fixes a bug in Firefox where the :active state of anchor tags + * prevent subsequent 'mousemove' events from being fired + * (see https://github.com/clauderic/react-sortable-hoc/issues/118) + */ if (e.target.tagName.toLowerCase() === 'a') { e.preventDefault(); } @@ -380,7 +387,7 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f }; handleSortEnd = e => { - const {hideSortableGhost, onSortEnd} = this.props; + const {transitionDuration, onSortEnd} = this.props; const {collection} = this.manager.active; // Remove the event listeners if the node is still in the DOM @@ -394,34 +401,39 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f this.listenerNode.removeEventListener(eventName, this.handleSortEnd)); } - // Remove the helper from the DOM - this.helper.parentNode.removeChild(this.helper); + this.updatePosition(e); // Make sure we have the right position for the helper - if (hideSortableGhost && this.sortableGhost) { - this.sortableGhost.style.visibility = ''; - this.sortableGhost.style.opacity = ''; + // For now, transition the helper to a position over the ghost. + if (transitionDuration) { + this.helper.style[`${vendorPrefix}TransitionDuration`] = `${transitionDuration}ms`; + + const helperStart = this.helper.getBoundingClientRect(); + const helperDestination = this.sortableGhost.getBoundingClientRect(); + this.helper.style[`${vendorPrefix}Transform`] = `translate3d(${ + helperDestination.left - (helperStart.left - this.translate.x) + }px,${ + helperDestination.top - (helperStart.top - this.translate.y) + }px,0)`; } - const nodes = this.manager.refs[collection]; - for (let i = 0, len = nodes.length; i < len; i++) { - const node = nodes[i]; - const el = node.node; + // Remove helper after a transition back to place from the DOM + setTimeout((helper => { + if (this.props.hideSortableGhost && this.sortableGhost) { + this.sortableGhost.style[`${vendorPrefix}Transform`] = ''; + this.sortableGhost.style[`${vendorPrefix}TransitionDuration`] = ''; + this.sortableGhost.style.visibility = ''; + this.sortableGhost.style.opacity = ''; + } + helper.parentNode.removeChild(helper); + }).bind(this, this.helper), transitionDuration); - // Clear the cached offsetTop / offsetLeft value - node.edgeOffset = null; - - // Remove the transforms / transitions - el.style[`${vendorPrefix}Transform`] = ''; - el.style[`${vendorPrefix}TransitionDuration`] = ''; - } + // This function might be pre-empted if the parent calls a re-render quickly. + this.cleanupTimeout = setTimeout(this.performCleanup, transitionDuration); // Stop autoscroll clearInterval(this.autoscrollInterval); this.autoscrollInterval = null; - // Update state - this.manager.active = null; - this.setState({ sorting: false, sortingIndex: null, @@ -440,7 +452,24 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f this._touched = false; }; + performCleanup = () => { + if (!this.manager.active) return; + if (this.cleanupTimeout) this.cleanupTimeout = null; + + // Remove styles as part of this cleanup (will be called before rerendering) + this.manager.refs[this.manager.active.collection].forEach((node) =>{ + node.edgeOffset = null; + + if (node.node === this.sortableGhost) + return; // For the ghost node, we'll do it when we remove the helper + node.node.style[`${vendorPrefix}Transform`] = ''; + node.node.style[`${vendorPrefix}TransitionDuration`] = ''; + }); + + // Update manager state + this.manager.active = null; + }; getEdgeOffset(node, offset = {top: 0, left: 0}) { // Get the actual offsetTop / offsetLeft value, no matter how deep the node is nested if (node) { @@ -596,7 +625,17 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f top: (window.pageYOffset - this.initialWindowScroll.top), left: (window.pageXOffset - this.initialWindowScroll.left), }; + + const ghostOffset = nodes[this.index].edgeOffset || + (nodes[this.index].edgeOffset = this.getEdgeOffset(nodes[this.index].node)); + + const ghostTranslate = { + x: 0, + y: 0, + }; + const prevIndex = this.newIndex; + this.newIndex = null; for (let i = 0, len = nodes.length; i < len; i++) { @@ -604,6 +643,7 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f const index = node.sortableInfo.index; const width = node.offsetWidth; const height = node.offsetHeight; + const margin = getElementMargin(node); const offset = { width: this.width > width ? width / 2 : this.width / 2, height: this.height > height ? height / 2 : this.height / 2, @@ -634,10 +674,10 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f if (index === this.index) { if (hideSortableGhost) { /* - * With windowing libraries such as `react-virtualized`, the sortableGhost - * node may change while scrolling down and then back up (or vice-versa), - * so we need to update the reference to the new node just to be safe. - */ + * With windowing libraries such as `react-virtualized`, the sortableGhost + * node may change while scrolling down and then back up (or vice-versa), + * so we need to update the reference to the new node just to be safe. + */ this.sortableGhost = node; node.style.visibility = 'hidden'; node.style.opacity = 0; @@ -677,6 +717,8 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f } if (this.newIndex === null) { this.newIndex = index; + ghostTranslate.x = edgeOffset.left - ghostOffset.left + this.margin.left - margin.left; + ghostTranslate.y = edgeOffset.top - ghostOffset.top + this.margin.left - margin.left; } } else if ( index > this.index && @@ -700,6 +742,8 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f translate.y = prevNode.edgeOffset.top - edgeOffset.top; } this.newIndex = index; + ghostTranslate.x = edgeOffset.left - ghostOffset.left; + ghostTranslate.y = edgeOffset.top - ghostOffset.top; } } else { if ( @@ -708,6 +752,7 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f ) { translate.x = -(this.width + this.marginOffset.x); this.newIndex = index; + ghostTranslate.x = edgeOffset.left - ghostOffset.left + node.offsetWidth + translate.x; } else if ( index < this.index && (sortingOffset.left + windowScrollDelta.left) <= edgeOffset.left + offset.width @@ -715,6 +760,7 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f translate.x = this.width + this.marginOffset.x; if (this.newIndex == null) { this.newIndex = index; + ghostTranslate.x = edgeOffset.left - ghostOffset.left; } } } @@ -725,6 +771,7 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f ) { translate.y = -(this.height + this.marginOffset.y); this.newIndex = index; + ghostTranslate.y = edgeOffset.top - ghostOffset.top + node.offsetHeight + translate.y; } else if ( index < this.index && (sortingOffset.top + windowScrollDelta.top) <= edgeOffset.top + offset.height @@ -732,6 +779,7 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f translate.y = this.height + this.marginOffset.y; if (this.newIndex == null) { this.newIndex = index; + ghostTranslate.y = edgeOffset.top - ghostOffset.top; } } } @@ -742,6 +790,10 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f this.newIndex = this.index; } + if (this.sortableGhost) { + this.sortableGhost.style[`${vendorPrefix}Transform`] = + `translate3d(${ghostTranslate.x}px,${ghostTranslate.y}px,0)`; + } if (onSortOver && this.newIndex !== prevIndex) { onSortOver({ newIndex: this.newIndex, From ee155f1e0185fdf0d28a2b49c98ce7b46545bfd4 Mon Sep 17 00:00:00 2001 From: Amin Mirzaee Date: Mon, 6 Nov 2017 20:35:07 -0500 Subject: [PATCH 2/8] fix some things with margins --- src/.stories/Storybook.scss | 11 +++++++++-- src/.stories/index.js | 26 ++++++++++++++++++++++++++ src/SortableContainer/index.js | 4 ++-- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/.stories/Storybook.scss b/src/.stories/Storybook.scss index 84d8eb9ed..3decb2b6a 100644 --- a/src/.stories/Storybook.scss +++ b/src/.stories/Storybook.scss @@ -72,11 +72,18 @@ flex-shrink: 0; align-items: center; justify-content: center; - width: 200px; + width: 100px; border-right: 1px solid #EFEFEF; border-bottom: 0; } - +.verticalMargins { + margin-top: 5px; + margin-bottom: 10px; +} +.horizontalMargins { + margin-left: 5px; + margin-right: 15px; +} // Grid .grid { display: block; diff --git a/src/.stories/index.js b/src/.stories/index.js index 11a2f0783..c8c19b842 100644 --- a/src/.stories/index.js +++ b/src/.stories/index.js @@ -482,6 +482,32 @@ storiesOf('Customization', module) ); }) + .add('Horizontal with Margins', () => { + return ( +
+ +
+ ); + }) + .add('Vertical with Margins', () => { + return ( +
+ +
+ ); + }) .add('Transition duration', () => { return (
diff --git a/src/SortableContainer/index.js b/src/SortableContainer/index.js index ead580c23..598f17986 100644 --- a/src/SortableContainer/index.js +++ b/src/SortableContainer/index.js @@ -752,7 +752,7 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f ) { translate.x = -(this.width + this.marginOffset.x); this.newIndex = index; - ghostTranslate.x = edgeOffset.left - ghostOffset.left + node.offsetWidth + translate.x; + ghostTranslate.x = edgeOffset.left - ghostOffset.left + node.offsetWidth - this.width; } else if ( index < this.index && (sortingOffset.left + windowScrollDelta.left) <= edgeOffset.left + offset.width @@ -771,7 +771,7 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f ) { translate.y = -(this.height + this.marginOffset.y); this.newIndex = index; - ghostTranslate.y = edgeOffset.top - ghostOffset.top + node.offsetHeight + translate.y; + ghostTranslate.y = edgeOffset.top - ghostOffset.top + node.offsetHeight - this.height; } else if ( index < this.index && (sortingOffset.top + windowScrollDelta.top) <= edgeOffset.top + offset.height From 2f1cc2561d8fb16327cec834a2039e32f0889548 Mon Sep 17 00:00:00 2001 From: Amin Mirzaee Date: Fri, 25 May 2018 21:35:28 -0400 Subject: [PATCH 3/8] fix issue when hiding sortable ghost --- 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 598f17986..1bb0549d7 100644 --- a/src/SortableContainer/index.js +++ b/src/SortableContainer/index.js @@ -315,8 +315,8 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f this.helper.style.boxSizing = 'border-box'; this.helper.style.pointerEvents = 'none'; + this.sortableGhost = node; if (hideSortableGhost) { - this.sortableGhost = node; node.style.visibility = 'hidden'; node.style.opacity = 0; } From 0a34e79bffd2535163985cdad75d652f0fb99734 Mon Sep 17 00:00:00 2001 From: Amin Mirzaee Date: Fri, 25 May 2018 22:00:47 -0400 Subject: [PATCH 4/8] another issue with not hiding sortable ghost --- src/SortableContainer/index.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/SortableContainer/index.js b/src/SortableContainer/index.js index 1bb0549d7..669587166 100644 --- a/src/SortableContainer/index.js +++ b/src/SortableContainer/index.js @@ -418,11 +418,13 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f // Remove helper after a transition back to place from the DOM setTimeout((helper => { - if (this.props.hideSortableGhost && this.sortableGhost) { + if (this.sortableGhost) { this.sortableGhost.style[`${vendorPrefix}Transform`] = ''; this.sortableGhost.style[`${vendorPrefix}TransitionDuration`] = ''; - this.sortableGhost.style.visibility = ''; - this.sortableGhost.style.opacity = ''; + if (this.props.hideSortableGhost) { + this.sortableGhost.style.visibility = ''; + this.sortableGhost.style.opacity = ''; + } } helper.parentNode.removeChild(helper); }).bind(this, this.helper), transitionDuration); From baf43420a162ff889cfdae85fe72bc2cec41b12f Mon Sep 17 00:00:00 2001 From: Amin Mirzaee Date: Fri, 25 May 2018 22:17:54 -0400 Subject: [PATCH 5/8] make ghost stay in place while helper animates back over it --- src/SortableContainer/index.js | 54 +++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/src/SortableContainer/index.js b/src/SortableContainer/index.js index 669587166..e43e6d5c7 100644 --- a/src/SortableContainer/index.js +++ b/src/SortableContainer/index.js @@ -403,6 +403,18 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f this.updatePosition(e); // Make sure we have the right position for the helper + // This function might be pre-empted if the parent calls a re-render quickly. + this.cleanupTimeout = setTimeout(this.performCleanup, transitionDuration); + + // Remove helper after a transition back to place from the DOM + setTimeout((helper => { + if (this.props.hideSortableGhost) { + this.sortableGhost.style.visibility = ''; + this.sortableGhost.style.opacity = ''; + } + helper.parentNode.removeChild(helper); + }).bind(this, this.helper), transitionDuration*2); + // For now, transition the helper to a position over the ghost. if (transitionDuration) { this.helper.style[`${vendorPrefix}TransitionDuration`] = `${transitionDuration}ms`; @@ -416,22 +428,6 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f }px,0)`; } - // Remove helper after a transition back to place from the DOM - setTimeout((helper => { - if (this.sortableGhost) { - this.sortableGhost.style[`${vendorPrefix}Transform`] = ''; - this.sortableGhost.style[`${vendorPrefix}TransitionDuration`] = ''; - if (this.props.hideSortableGhost) { - this.sortableGhost.style.visibility = ''; - this.sortableGhost.style.opacity = ''; - } - } - helper.parentNode.removeChild(helper); - }).bind(this, this.helper), transitionDuration); - - // This function might be pre-empted if the parent calls a re-render quickly. - this.cleanupTimeout = setTimeout(this.performCleanup, transitionDuration); - // Stop autoscroll clearInterval(this.autoscrollInterval); this.autoscrollInterval = null; @@ -452,6 +448,12 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f ); } + if (this.sortableGhost) { // remove transform off of the ghost AFTER onSortEnd is called. + this.sortableGhost.style[`${vendorPrefix}Transform`] = ''; + this.sortableGhost.style[`${vendorPrefix}TransitionDuration`] = ''; + } + + this._touched = false; }; performCleanup = () => { @@ -462,9 +464,11 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f this.manager.refs[this.manager.active.collection].forEach((node) =>{ node.edgeOffset = null; - if (node.node === this.sortableGhost) + node.node.style.zIndex = ''; + if (node.node === this.sortableGhost) { + console.log(node.node) return; // For the ghost node, we'll do it when we remove the helper - + } node.node.style[`${vendorPrefix}Transform`] = ''; node.node.style[`${vendorPrefix}TransitionDuration`] = ''; }); @@ -672,8 +676,16 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f nextNode.edgeOffset = this.getEdgeOffset(nextNode.node); } + if (transitionDuration) { + node.style[ + `${vendorPrefix}TransitionDuration` + ] = `${transitionDuration}ms`; + } + + node.style.zIndex = '1'; // If the node is the one we're currently animating, skip it if (index === this.index) { + node.style.zIndex = '0'; if (hideSortableGhost) { /* * With windowing libraries such as `react-virtualized`, the sortableGhost @@ -687,12 +699,6 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f continue; } - if (transitionDuration) { - node.style[ - `${vendorPrefix}TransitionDuration` - ] = `${transitionDuration}ms`; - } - if (this.axis.x) { if (this.axis.y) { // Calculations for a grid setup From 26cea2410fb306b027713900f9107be3df676315 Mon Sep 17 00:00:00 2001 From: Amin Mirzaee Date: Fri, 25 May 2018 22:32:59 -0400 Subject: [PATCH 6/8] fix issue with helper returning to the ghost position before it's fully updated --- src/SortableContainer/index.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/SortableContainer/index.js b/src/SortableContainer/index.js index 1a6090391..0ca465913 100644 --- a/src/SortableContainer/index.js +++ b/src/SortableContainer/index.js @@ -407,7 +407,7 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f this.listenerNode.removeEventListener(eventName, this.handleSortEnd)); } - this.updatePosition(e); // Make sure we have the right position for the helper + this.updatePosition(event); // Make sure we have the right position for the helper // This function might be pre-empted if the parent calls a re-render quickly. this.cleanupTimeout = setTimeout(this.performCleanup, transitionDuration); @@ -423,6 +423,10 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f // For now, transition the helper to a position over the ghost. if (transitionDuration) { + if (this.sortableGhost) { + // remove transition off of the ghost BEFORE repositioning helper + this.sortableGhost.style[`${vendorPrefix}TransitionDuration`] = ''; + } this.helper.style[`${vendorPrefix}TransitionDuration`] = `${transitionDuration}ms`; const helperStart = this.helper.getBoundingClientRect(); @@ -454,9 +458,9 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f ); } - if (this.sortableGhost) { // remove transform off of the ghost AFTER onSortEnd is called. + if (this.sortableGhost) { + // remove transform off of the ghost AFTER onSortEnd is called. this.sortableGhost.style[`${vendorPrefix}Transform`] = ''; - this.sortableGhost.style[`${vendorPrefix}TransitionDuration`] = ''; } From eed59143934bd6d698cb4064b7d5efb6f6e0f208 Mon Sep 17 00:00:00 2001 From: Amin Mirzaee Date: Mon, 6 Nov 2017 00:02:28 -0500 Subject: [PATCH 7/8] Improve performance drastically in for Edge & MSIE. --- src/SortableContainer/index.js | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/src/SortableContainer/index.js b/src/SortableContainer/index.js index f666cce2f..85bd87320 100644 --- a/src/SortableContainer/index.js +++ b/src/SortableContainer/index.js @@ -372,13 +372,33 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f } }; + _handleSortMove = event => { + this.animateNodes(); + this.autoscroll(); + + if (window.requestAnimationFrame) + this.sortMoveAF = null; + else setTimeout(() =>{ + this.sortMoveAF = null; + }, 1000/60); // aim for 60 fps + }; + handleSortMove = event => { const {onSortMove} = this.props; event.preventDefault(); // Prevent scrolling on mobile + if (this.sortMoveAF) { + return; + } + this.updatePosition(event); - this.animateNodes(); - this.autoscroll(); + + if (window.requestAnimationFrame) { + this.sortMoveAF = window.requestAnimationFrame(this._handleSortMove); + } else { + this.sortMoveAF = true; + this._handleSortMove(); // call inner function now if no animation frame + } if (onSortMove) { onSortMove(event); @@ -389,6 +409,12 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f const {hideSortableGhost, onSortEnd} = this.props; const {collection} = this.manager.active; + // Remove the move handler if there's a frame that hasn't run yet. + if (window.cancelAnimationFrame && this.sortMoveAF){ + window.cancelAnimationFrame(this.sortMoveAF); + this.sortMoveAF = null; + } + // Remove the event listeners if the node is still in the DOM if (this.listenerNode) { events.move.forEach(eventName => @@ -748,7 +774,7 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f config.withRef, 'To access the wrapped instance, you need to pass in {withRef: true} as the second argument of the SortableContainer() call' ); - + return this.refs.wrappedInstance; } From a7a5bb9dc45f3ee217eb5a68cd6bd448c6695866 Mon Sep 17 00:00:00 2001 From: Amin Mirzaee Date: Fri, 25 May 2018 22:42:19 -0400 Subject: [PATCH 8/8] remove console.log --- src/SortableContainer/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SortableContainer/index.js b/src/SortableContainer/index.js index 0ca465913..fd4909f57 100644 --- a/src/SortableContainer/index.js +++ b/src/SortableContainer/index.js @@ -476,7 +476,6 @@ export default function sortableContainer(WrappedComponent, config = {withRef: f node.node.style.zIndex = ''; if (node.node === this.sortableGhost) { - console.log(node.node) return; // For the ghost node, we'll do it when we remove the helper } node.node.style[`${vendorPrefix}Transform`] = '';