From 16c13fb07960a59894887acbceadffec9ff1e56d Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Mon, 15 Jan 2018 18:12:27 +0000 Subject: [PATCH] Replace TagPanel react-dnd with react-beautiful-dnd This new library handles the simple case of an ordered vertical (or horizontal) list of items that can be reordered. It provides animations, handles positioning of items mid-drag and exposes a much simpler API to react-dnd (with a slight loss of potential function, but we don't need this flexibility here anyway). Apart from this, TagOrderStore had to be changed in a highly coupled way, but arguably for the better. Instead of being updated incrementally every time an item is dragged over another and having a separate "commit" action, the asyncronous action `moveTag` is used to reposition the tag in the list and both dispatch an optimistic update and carry out the request as before. (The MatrixActions.accountData is still used to indicate a successful reordering of tags). The view is updated instantly, in an animated way, and this is handled at the layer "above" React by the DND library. --- package.json | 1 + src/actions/TagOrderActions.js | 20 +++-- src/actions/actionCreators.js | 11 ++- src/components/structures/TagPanel.js | 42 ++++++++-- src/components/views/elements/DNDTagTile.js | 88 ++++++--------------- src/stores/TagOrderStore.js | 23 ++---- 6 files changed, 85 insertions(+), 100 deletions(-) diff --git a/package.json b/package.json index eb2cabf854..dbc27b5a08 100644 --- a/package.json +++ b/package.json @@ -77,6 +77,7 @@ "querystring": "^0.2.0", "react": "^15.4.0", "react-addons-css-transition-group": "15.3.2", + "react-beautiful-dnd": "^4.0.0", "react-dnd": "^2.1.4", "react-dnd-html5-backend": "^2.1.2", "react-dom": "^15.4.0", diff --git a/src/actions/TagOrderActions.js b/src/actions/TagOrderActions.js index 60946ea7f1..db9230e964 100644 --- a/src/actions/TagOrderActions.js +++ b/src/actions/TagOrderActions.js @@ -31,16 +31,22 @@ const TagOrderActions = {}; * indicating the status of the request. * @see asyncAction */ -TagOrderActions.commitTagOrdering = function(matrixClient) { - return asyncAction('TagOrderActions.commitTagOrdering', () => { - // Only commit tags if the state is ready, i.e. not null - const tags = TagOrderStore.getOrderedTags(); - if (!tags) { - return; - } +TagOrderActions.moveTag = function(matrixClient, tag, destinationIx) { + // Only commit tags if the state is ready, i.e. not null + let tags = TagOrderStore.getOrderedTags(); + if (!tags) { + return; + } + tags = tags.filter((t) => t !== tag); + tags = [...tags.slice(0, destinationIx), tag, ...tags.slice(destinationIx)]; + + return asyncAction('TagOrderActions.moveTag', () => { Analytics.trackEvent('TagOrderActions', 'commitTagOrdering'); return matrixClient.setAccountData('im.vector.web.tag_ordering', {tags}); + }, () => { + // For an optimistic update + return {tags}; }); }; diff --git a/src/actions/actionCreators.js b/src/actions/actionCreators.js index bddfbc7c63..0238eee8c0 100644 --- a/src/actions/actionCreators.js +++ b/src/actions/actionCreators.js @@ -22,6 +22,9 @@ limitations under the License. * suffix determining whether it is pending, successful or * a failure. * @param {function} fn a function that returns a Promise. + * @param {function?} pendingFn a function that returns an object to assign + * to the `request` key of the ${id}.pending + * payload. * @returns {function} an action thunk - a function that uses its single * argument as a dispatch function to dispatch the * following actions: @@ -29,9 +32,13 @@ limitations under the License. * `${id}.success` or * `${id}.failure`. */ -export function asyncAction(id, fn) { +export function asyncAction(id, fn, pendingFn) { return (dispatch) => { - dispatch({action: id + '.pending'}); + dispatch({ + action: id + '.pending', + request: + typeof pendingFn === 'function' ? pendingFn() : undefined, + }); fn().then((result) => { dispatch({action: id + '.success', result}); }).catch((err) => { diff --git a/src/components/structures/TagPanel.js b/src/components/structures/TagPanel.js index 531c247ea6..323528e1f1 100644 --- a/src/components/structures/TagPanel.js +++ b/src/components/structures/TagPanel.js @@ -25,6 +25,8 @@ import TagOrderActions from '../../actions/TagOrderActions'; import sdk from '../../index'; import dis from '../../dispatcher'; +import { DragDropContext, Droppable } from 'react-beautiful-dnd'; + const TagPanel = React.createClass({ displayName: 'TagPanel', @@ -69,7 +71,9 @@ const TagPanel = React.createClass({ dis.dispatch(GroupActions.fetchJoinedGroups(this.context.matrixClient)); }, - onClick() { + onClick(e) { + // Ignore clicks on children + if (e.target !== e.currentTarget) return; dis.dispatch({action: 'deselect_tags'}); }, @@ -78,8 +82,20 @@ const TagPanel = React.createClass({ dis.dispatch({action: 'view_create_group'}); }, - onTagTileEndDrag() { - dis.dispatch(TagOrderActions.commitTagOrdering(this.context.matrixClient)); + onTagTileEndDrag(result) { + // Dragged to an invalid destination, not onto a droppable + if (!result.destination) { + return; + } + + // Dispatch synchronously so that the TagPanel receives an + // optimistic update from TagOrderStore before the previous + // state is shown. + dis.dispatch(TagOrderActions.moveTag( + this.context.matrixClient, + result.draggableId, + result.destination.index, + ), true); }, render() { @@ -89,16 +105,26 @@ const TagPanel = React.createClass({ const tags = this.state.orderedTags.map((tag, index) => { return ; }); return
-
- { tags } -
+ + + { (provided, snapshot) => ( +
+ { tags } + { provided.placeholder } +
+ ) } + + diff --git a/src/components/views/elements/DNDTagTile.js b/src/components/views/elements/DNDTagTile.js index 539163d0dc..e17ea52976 100644 --- a/src/components/views/elements/DNDTagTile.js +++ b/src/components/views/elements/DNDTagTile.js @@ -15,71 +15,29 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { DragSource, DropTarget } from 'react-dnd'; - import TagTile from './TagTile'; -import dis from '../../../dispatcher'; -import { findDOMNode } from 'react-dom'; -const tagTileSource = { - canDrag: function(props, monitor) { - return true; - }, +import { Draggable } from 'react-beautiful-dnd'; - beginDrag: function(props) { - // Return the data describing the dragged item - return { - tag: props.tag, - }; - }, - - endDrag: function(props, monitor, component) { - const dropResult = monitor.getDropResult(); - if (!monitor.didDrop() || !dropResult) { - return; - } - props.onEndDrag(); - }, -}; - -const tagTileTarget = { - canDrop(props, monitor) { - return true; - }, - - hover(props, monitor, component) { - if (!monitor.canDrop()) return; - const draggedY = monitor.getClientOffset().y; - const {top, bottom} = findDOMNode(component).getBoundingClientRect(); - const targetY = (top + bottom) / 2; - dis.dispatch({ - action: 'order_tag', - tag: monitor.getItem().tag, - targetTag: props.tag, - // Note: we indicate that the tag should be after the target when - // it's being dragged over the top half of the target. - after: draggedY < targetY, - }); - }, - - drop(props) { - // Return the data to be returned by getDropResult - return { - tag: props.tag, - }; - }, -}; - -export default - DropTarget('TagTile', tagTileTarget, (connect, monitor) => ({ - connectDropTarget: connect.dropTarget(), - }))(DragSource('TagTile', tagTileSource, (connect, monitor) => ({ - connectDragSource: connect.dragSource(), - }))((props) => { - const { connectDropTarget, connectDragSource, ...otherProps } = props; - return connectDropTarget(connectDragSource( -
- -
, - )); - })); +export default function DNDTagTile(props) { + return
+ + { (provided, snapshot) => ( +
+
+ +
+ { provided.placeholder } +
+ ) } + +
; +} diff --git a/src/stores/TagOrderStore.js b/src/stores/TagOrderStore.js index 9c9427284e..27ce6cbb73 100644 --- a/src/stores/TagOrderStore.js +++ b/src/stores/TagOrderStore.js @@ -78,24 +78,11 @@ class TagOrderStore extends Store { this._updateOrderedTags(); break; } - // Puts payload.tag at payload.targetTag, placing the targetTag before or after the tag - case 'order_tag': { - if (!this._state.orderedTags || - !payload.tag || - !payload.targetTag || - payload.tag === payload.targetTag - ) return; - - const tags = this._state.orderedTags; - - let orderedTags = tags.filter((t) => t !== payload.tag); - const newIndex = orderedTags.indexOf(payload.targetTag) + (payload.after ? 1 : 0); - orderedTags = [ - ...orderedTags.slice(0, newIndex), - payload.tag, - ...orderedTags.slice(newIndex), - ]; - this._setState({orderedTags}); + case 'TagOrderActions.moveTag.pending': { + // Optimistic update of a moved tag + this._setState({ + orderedTags: payload.request.tags, + }); break; } case 'select_tag': {