Merge pull request #6021 from noamraph/sortable-hoc-rtl

Make SortableHOC work properly in RTL mode

src/lib/drag-utils.js

@@ -12,9 +12,10 @@
  * many rows, or a single row of items.
  * @param {{x: number, y: number}} position The xy coordinates to retreive the corresponding index of.
  * @param {Array.<DOMRect>} boxes The rects of the items, returned from `getBoundingClientRect`
+ * @param {bool} isRtl are the boxes in RTL order.
  * @return {?number} index of the corresponding box, or null if one could not be found.
  */
-const indexForPositionOnList = ({x, y}, boxes) => {
+const indexForPositionOnList = ({x, y}, boxes, isRtl) => {
     if (boxes.length === 0) return null;
     let index = null;
     const leftEdge = Math.min.apply(null, boxes.map(b => b.left));
@@ -25,16 +26,23 @@ const indexForPositionOnList = ({x, y}, boxes) => {
         const box = boxes[n];
         // Construct an "extended" box for each, extending out to infinity if
         // the box is along a boundary.
-        const minX = box.left === leftEdge ? -Infinity : box.left;
+        let minX = box.left === leftEdge ? -Infinity : box.left;
+        let maxX = box.right === rightEdge ? Infinity : box.right;
         const minY = box.top === topEdge ? -Infinity : box.top;
         const maxY = box.bottom === bottomEdge ? Infinity : box.bottom;
         // The last item in the wrapped list gets a right edge at infinity, even
-        // if it isn't the farthest right. Add this as an "or" condition for extension.
-        const maxX = (n === boxes.length - 1 || box.right === rightEdge) ?
-            Infinity : box.right;
+        // if it isn't the farthest right, in RTL mode. In LTR mode, it gets a
+        // left edge at infinity.
+        if (n === boxes.length - 1) {
+            if (isRtl) {
+                minX = -Infinity;
+            } else {
+                maxX = Infinity;
+            }
+        }
 
         // Check if the point is in the bounds.
-        if (x > minX && x <= maxX && y > minY && y <= maxY) {
+        if (x >= minX && x <= maxX && y >= minY && y <= maxY) {
             index = n;
             break; // No need to keep looking.
         }

src/lib/sortable-hoc.jsx

@@ -24,8 +24,8 @@ const SortableHOC = function (WrappedComponent) {
             if (newProps.dragInfo.dragging && !this.props.dragInfo.dragging) {
                 // Drag just started, snapshot the sorted bounding boxes for sortables.
                 this.boxes = this.sortableRefs.map(el => el && el.getBoundingClientRect());
-                this.boxes.sort((a, b) => { // Sort top-to-bottom, left-to-right.
-                    if (a.top === b.top) return a.left - b.left;
+                this.boxes.sort((a, b) => { // Sort top-to-bottom, left-to-right (in LTR) / right-to-left (in RTL).
+                    if (a.top === b.top) return (a.left - b.left) * (this.props.isRtl ? -1 : 1);
                     return a.top - b.top;
                 });
                 if (!this.ref) {
@@ -82,7 +82,7 @@ const SortableHOC = function (WrappedComponent) {
                         mouseOverIndex = 0;
                     } else {
                         mouseOverIndex = indexForPositionOnList(
-                            this.props.dragInfo.currentOffset, this.boxes);
+                            this.props.dragInfo.currentOffset, this.boxes, this.props.isRtl);
                     }
                 }
             }
@@ -125,11 +125,13 @@ const SortableHOC = function (WrappedComponent) {
             name: PropTypes.string.isRequired
         })),
         onClose: PropTypes.func,
-        onDrop: PropTypes.func
+        onDrop: PropTypes.func,
+        isRtl: PropTypes.bool
     };
 
     const mapStateToProps = state => ({
-        dragInfo: state.scratchGui.assetDrag
+        dragInfo: state.scratchGui.assetDrag,
+        isRtl: state.locales.isRtl
     });
 
     const mapDispatchToProps = () => ({});

test/unit/util/drag-utils.test.js

@@ -7,7 +7,7 @@ describe('indexForPositionOnList', () => {
         expect(indexForPositionOnList({x: 0, y: 0}, [])).toEqual(null);
     });
 
-    test('wrapped list with incomplete last row', () => {
+    test('wrapped list with incomplete last row LTR', () => {
         const boxes = [
             box(0, 100, 100, 0), // index: 0
             box(0, 200, 100, 100), // index: 1
@@ -17,25 +17,58 @@ describe('indexForPositionOnList', () => {
         ];
 
         // Inside the second box.
-        expect(indexForPositionOnList({x: 150, y: 50}, boxes)).toEqual(1);
+        expect(indexForPositionOnList({x: 150, y: 50}, boxes, false)).toEqual(1);
 
         // On the border edge of the first and second box. Given to the first box.
-        expect(indexForPositionOnList({x: 100, y: 50}, boxes)).toEqual(0);
+        expect(indexForPositionOnList({x: 100, y: 50}, boxes, false)).toEqual(0);
 
         // Off the top/left edge.
-        expect(indexForPositionOnList({x: -100, y: -100}, boxes)).toEqual(0);
+        expect(indexForPositionOnList({x: -100, y: -100}, boxes, false)).toEqual(0);
 
         // Off the left edge, in the second row.
-        expect(indexForPositionOnList({x: -100, y: 175}, boxes)).toEqual(3);
+        expect(indexForPositionOnList({x: -100, y: 175}, boxes, false)).toEqual(3);
 
         // Off the right edge, in the first row.
-        expect(indexForPositionOnList({x: 400, y: 75}, boxes)).toEqual(2);
+        expect(indexForPositionOnList({x: 400, y: 75}, boxes, false)).toEqual(2);
 
         // Off the top edge, middle of second item.
-        expect(indexForPositionOnList({x: 150, y: -75}, boxes)).toEqual(1);
+        expect(indexForPositionOnList({x: 150, y: -75}, boxes, false)).toEqual(1);
 
         // Within the right edge bounds, but on the second (incomplete) row.
         // This tests that wrapped lists with incomplete final rows work correctly.
-        expect(indexForPositionOnList({x: 375, y: 175}, boxes)).toEqual(4);
+        expect(indexForPositionOnList({x: 375, y: 175}, boxes, false)).toEqual(4);
     });
+
+    test('wrapped list with incomplete last row RTL', () => {
+        const boxes = [
+            box(0, 0, 100, -100), // index: 0
+            box(0, -100, 100, -200), // index: 1
+            box(0, -200, 100, -300), // index: 2
+            box(100, 0, 200, -100), // index: 3 (second row)
+            box(100, -100, 200, -200) // index: 4 (second row, left incomplete intentionally)
+        ];
+
+        // Inside the second box.
+        expect(indexForPositionOnList({x: -150, y: 50}, boxes, true)).toEqual(1);
+
+        // On the border edge of the first and second box. Given to the first box.
+        expect(indexForPositionOnList({x: -100, y: 50}, boxes, true)).toEqual(0);
+
+        // Off the top/right edge.
+        expect(indexForPositionOnList({x: 100, y: -100}, boxes, true)).toEqual(0);
+
+        // Off the right edge, in the second row.
+        expect(indexForPositionOnList({x: 100, y: 175}, boxes, true)).toEqual(3);
+
+        // Off the left edge, in the first row.
+        expect(indexForPositionOnList({x: -400, y: 75}, boxes, true)).toEqual(2);
+
+        // Off the top edge, middle of second item.
+        expect(indexForPositionOnList({x: -150, y: -75}, boxes, true)).toEqual(1);
+
+        // Within the left edge bounds, but on the second (incomplete) row.
+        // This tests that wrapped lists with incomplete final rows work correctly.
+        expect(indexForPositionOnList({x: -375, y: 175}, boxes, true)).toEqual(4);
+    });
+
 });