Skip to content

Commit ec67a44

Browse files
ericrosenbaumericrosenbaum
authored
Merge pull request scratchfoundation#6021 from noamraph/sortable-hoc-rtl
Make SortableHOC work properly in RTL mode
2 parents 6527e79 + 22cf318 commit ec67a44

File tree

3 files changed

+62
-19
lines changed

3 files changed

+62
-19
lines changed

‎src/lib/drag-utils.js

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@
1212
* many rows, or a single row of items.
1313
* @param {{x: number, y: number}} position The xy coordinates to retreive the corresponding index of.
1414
* @param {Array.<DOMRect>} boxes The rects of the items, returned from `getBoundingClientRect`
15+
* @param {bool} isRtl are the boxes in RTL order.
1516
* @return {?number} index of the corresponding box, or null if one could not be found.
1617
*/
17-
const indexForPositionOnList = ({x, y}, boxes) => {
18+
const indexForPositionOnList = ({x, y}, boxes, isRtl) => {
1819
if (boxes.length === 0) return null;
1920
let index = null;
2021
const leftEdge = Math.min.apply(null, boxes.map(b => b.left));
@@ -25,16 +26,23 @@ const indexForPositionOnList = ({x, y}, boxes) => {
2526
const box = boxes[n];
2627
// Construct an "extended" box for each, extending out to infinity if
2728
// the box is along a boundary.
28-
const minX = box.left === leftEdge ? -Infinity : box.left;
29+
let minX = box.left === leftEdge ? -Infinity : box.left;
30+
let maxX = box.right === rightEdge ? Infinity : box.right;
2931
const minY = box.top === topEdge ? -Infinity : box.top;
3032
const maxY = box.bottom === bottomEdge ? Infinity : box.bottom;
3133
// The last item in the wrapped list gets a right edge at infinity, even
32-
// if it isn't the farthest right. Add this as an "or" condition for extension.
33-
const maxX = (n === boxes.length - 1 || box.right === rightEdge) ?
34-
Infinity : box.right;
34+
// if it isn't the farthest right, in RTL mode. In LTR mode, it gets a
35+
// left edge at infinity.
36+
if (n === boxes.length - 1) {
37+
if (isRtl) {
38+
minX = -Infinity;
39+
} else {
40+
maxX = Infinity;
41+
}
42+
}
3543

3644
// Check if the point is in the bounds.
37-
if (x > minX && x <= maxX && y > minY && y <= maxY) {
45+
if (x >= minX && x <= maxX && y >= minY && y <= maxY) {
3846
index = n;
3947
break; // No need to keep looking.
4048
}

‎src/lib/sortable-hoc.jsx

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ const SortableHOC = function (WrappedComponent) {
2424
if (newProps.dragInfo.dragging && !this.props.dragInfo.dragging) {
2525
// Drag just started, snapshot the sorted bounding boxes for sortables.
2626
this.boxes = this.sortableRefs.map(el => el && el.getBoundingClientRect());
27-
this.boxes.sort((a, b) => { // Sort top-to-bottom, left-to-right.
28-
if (a.top === b.top) return a.left - b.left;
27+
this.boxes.sort((a, b) => { // Sort top-to-bottom, left-to-right (in LTR) / right-to-left (in RTL).
28+
if (a.top === b.top) return (a.left - b.left) * (this.props.isRtl ? -1 : 1);
2929
return a.top - b.top;
3030
});
3131
if (!this.ref) {
@@ -82,7 +82,7 @@ const SortableHOC = function (WrappedComponent) {
8282
mouseOverIndex = 0;
8383
} else {
8484
mouseOverIndex = indexForPositionOnList(
85-
this.props.dragInfo.currentOffset, this.boxes);
85+
this.props.dragInfo.currentOffset, this.boxes, this.props.isRtl);
8686
}
8787
}
8888
}
@@ -125,11 +125,13 @@ const SortableHOC = function (WrappedComponent) {
125125
name: PropTypes.string.isRequired
126126
})),
127127
onClose: PropTypes.func,
128-
onDrop: PropTypes.func
128+
onDrop: PropTypes.func,
129+
isRtl: PropTypes.bool
129130
};
130131

131132
const mapStateToProps = state => ({
132-
dragInfo: state.scratchGui.assetDrag
133+
dragInfo: state.scratchGui.assetDrag,
134+
isRtl: state.locales.isRtl
133135
});
134136

135137
const mapDispatchToProps = () => ({});

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

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ describe('indexForPositionOnList', () => {
77
expect(indexForPositionOnList({x: 0, y: 0}, [])).toEqual(null);
88
});
99

10-
test('wrapped list with incomplete last row', () => {
10+
test('wrapped list with incomplete last row LTR', () => {
1111
const boxes = [
1212
box(0, 100, 100, 0), // index: 0
1313
box(0, 200, 100, 100), // index: 1
@@ -17,25 +17,58 @@ describe('indexForPositionOnList', () => {
1717
];
1818

1919
// Inside the second box.
20-
expect(indexForPositionOnList({x: 150, y: 50}, boxes)).toEqual(1);
20+
expect(indexForPositionOnList({x: 150, y: 50}, boxes, false)).toEqual(1);
2121

2222
// On the border edge of the first and second box. Given to the first box.
23-
expect(indexForPositionOnList({x: 100, y: 50}, boxes)).toEqual(0);
23+
expect(indexForPositionOnList({x: 100, y: 50}, boxes, false)).toEqual(0);
2424

2525
// Off the top/left edge.
26-
expect(indexForPositionOnList({x: -100, y: -100}, boxes)).toEqual(0);
26+
expect(indexForPositionOnList({x: -100, y: -100}, boxes, false)).toEqual(0);
2727

2828
// Off the left edge, in the second row.
29-
expect(indexForPositionOnList({x: -100, y: 175}, boxes)).toEqual(3);
29+
expect(indexForPositionOnList({x: -100, y: 175}, boxes, false)).toEqual(3);
3030

3131
// Off the right edge, in the first row.
32-
expect(indexForPositionOnList({x: 400, y: 75}, boxes)).toEqual(2);
32+
expect(indexForPositionOnList({x: 400, y: 75}, boxes, false)).toEqual(2);
3333

3434
// Off the top edge, middle of second item.
35-
expect(indexForPositionOnList({x: 150, y: -75}, boxes)).toEqual(1);
35+
expect(indexForPositionOnList({x: 150, y: -75}, boxes, false)).toEqual(1);
3636

3737
// Within the right edge bounds, but on the second (incomplete) row.
3838
// This tests that wrapped lists with incomplete final rows work correctly.
39-
expect(indexForPositionOnList({x: 375, y: 175}, boxes)).toEqual(4);
39+
expect(indexForPositionOnList({x: 375, y: 175}, boxes, false)).toEqual(4);
4040
});
41+
42+
test('wrapped list with incomplete last row RTL', () => {
43+
const boxes = [
44+
box(0, 0, 100, -100), // index: 0
45+
box(0, -100, 100, -200), // index: 1
46+
box(0, -200, 100, -300), // index: 2
47+
box(100, 0, 200, -100), // index: 3 (second row)
48+
box(100, -100, 200, -200) // index: 4 (second row, left incomplete intentionally)
49+
];
50+
51+
// Inside the second box.
52+
expect(indexForPositionOnList({x: -150, y: 50}, boxes, true)).toEqual(1);
53+
54+
// On the border edge of the first and second box. Given to the first box.
55+
expect(indexForPositionOnList({x: -100, y: 50}, boxes, true)).toEqual(0);
56+
57+
// Off the top/right edge.
58+
expect(indexForPositionOnList({x: 100, y: -100}, boxes, true)).toEqual(0);
59+
60+
// Off the right edge, in the second row.
61+
expect(indexForPositionOnList({x: 100, y: 175}, boxes, true)).toEqual(3);
62+
63+
// Off the left edge, in the first row.
64+
expect(indexForPositionOnList({x: -400, y: 75}, boxes, true)).toEqual(2);
65+
66+
// Off the top edge, middle of second item.
67+
expect(indexForPositionOnList({x: -150, y: -75}, boxes, true)).toEqual(1);
68+
69+
// Within the left edge bounds, but on the second (incomplete) row.
70+
// This tests that wrapped lists with incomplete final rows work correctly.
71+
expect(indexForPositionOnList({x: -375, y: 175}, boxes, true)).toEqual(4);
72+
});
73+
4174
});

0 commit comments

Comments
 (0)