Merge pull request #1705 from matrix-org/luke/tag-panel-beautiful-dnd

Replace TagPanel react-dnd with react-beautiful-dnd

.travis.yml

@@ -3,7 +3,10 @@ dist: trusty
 
 # we don't need sudo, so can run in a container, which makes startup much
 # quicker.
-sudo: false
+#
+# unfortunately we do temporarily require sudo as a workaround for
+# https://github.com/travis-ci/travis-ci/issues/8836
+sudo: required
 
 language: node_js
 node_js:

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",

src/actions/TagOrderActions.js

@@ -22,25 +22,32 @@ const TagOrderActions = {};
 
 /**
  * Creates an action thunk that will do an asynchronous request to
- * commit TagOrderStore.getOrderedTags() to account data and dispatch
- * actions to indicate the status of the request.
+ * move a tag in TagOrderStore to destinationIx.
  *
  * @param {MatrixClient} matrixClient the matrix client to set the
  *                                    account data on.
+ * @param {string} tag the tag to move.
+ * @param {number} destinationIx the new position of the tag.
  * @returns {function} an action thunk that will dispatch actions
  *                     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};
     });
 };
 

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) => {

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,31 @@ const TagPanel = React.createClass({
 
         const tags = this.state.orderedTags.map((tag, index) => {
             return <DNDTagTile
-                key={tag + '_' + index}
+                key={tag}
                 tag={tag}
+                index={index}
                 selected={this.state.selectedTags.includes(tag)}
-                onEndDrag={this.onTagTileEndDrag}
             />;
         });
-        return <div className="mx_TagPanel" onClick={this.onClick}>
-            <div className="mx_TagPanel_tagTileContainer">
-                { tags }
-            </div>
+        return <div className="mx_TagPanel">
+            <DragDropContext onDragEnd={this.onTagTileEndDrag}>
+                <Droppable droppableId="tag-panel-droppable">
+                    { (provided, snapshot) => (
+                        <div
+                            className="mx_TagPanel_tagTileContainer"
+                            ref={provided.innerRef}
+                            // react-beautiful-dnd has a bug that emits a click to the parent
+                            // of draggables upon dropping
+                            //   https://github.com/atlassian/react-beautiful-dnd/issues/273
+                            // so we use onMouseDown here as a workaround.
+                            onMouseDown={this.onClick}
+                        >
+                            { tags }
+                            { provided.placeholder }
+                        </div>
+                    ) }
+                </Droppable>
+            </DragDropContext>
             <AccessibleButton className="mx_TagPanel_createGroupButton" onClick={this.onCreateGroupClick}>
                 <TintableSvg src="img/icons-create-room.svg" width="25" height="25" />
             </AccessibleButton>

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(
-            <div>
-                <TagTile {...otherProps} />
-            </div>,
-        ));
-    }));
+export default function DNDTagTile(props) {
+    return <div>
+        <Draggable
+            key={props.tag}
+            draggableId={props.tag}
+            index={props.index}
+        >
+            { (provided, snapshot) => (
+                <div>
+                    <div
+                        ref={provided.innerRef}
+                        {...provided.draggableProps}
+                        {...provided.dragHandleProps}
+                    >
+                        <TagTile {...props} />
+                    </div>
+                    { provided.placeholder }
+                </div>
+            ) }
+        </Draggable>
+    </div>;
+}

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': {