fix zindex to account for group boundaries (#2065)

src/actions/actionDuplicateSelection.tsx

@@ -10,7 +10,11 @@ import { t } from "../i18n";
 import { getShortcutKey } from "../utils";
 import { LinearElementEditor } from "../element/linearElementEditor";
 import { mutateElement } from "../element/mutateElement";
-import { selectGroupsForSelectedElements } from "../groups";
+import {
+  selectGroupsForSelectedElements,
+  getSelectedGroupForElement,
+  getElementsInGroup,
+} from "../groups";
 import { AppState } from "../types";
 import { fixBindingsAfterDuplication } from "../element/binding";
 import { ActionResult } from "./types";
@@ -82,9 +86,8 @@ const duplicateElements = (
   const newElements: ExcalidrawElement[] = [];
   const oldElements: ExcalidrawElement[] = [];
   const oldIdToDuplicatedId = new Map();
-  const finalElements = elements.reduce(
+
-    (acc: Array<ExcalidrawElement>, element: ExcalidrawElement) => {
+  const duplicateAndOffsetElement = (element: ExcalidrawElement) => {
-      if (appState.selectedElementIds[element.id]) {
     const newElement = duplicateElement(
       appState.editingGroupId,
       groupIdMap,
@@ -97,13 +100,39 @@ const duplicateElements = (
     oldIdToDuplicatedId.set(element.id, newElement.id);
     oldElements.push(element);
     newElements.push(newElement);
-        return acc.concat([element, newElement]);
+    return newElement;
-      }
+  };
-      return acc.concat(element);
+
-    },
+  const finalElements: ExcalidrawElement[] = [];
-    [],
+
+  let i = 0;
+  while (i < elements.length) {
+    const element = elements[i];
+    if (appState.selectedElementIds[element.id]) {
+      if (element.groupIds.length) {
+        const groupId = getSelectedGroupForElement(appState, element);
+        // if group selected, duplicate it atomically
+        if (groupId) {
+          const groupElements = getElementsInGroup(elements, groupId);
+          finalElements.push(
+            ...groupElements,
+            ...groupElements.map((element) =>
+              duplicateAndOffsetElement(element),
+            ),
           );
+          i = i + groupElements.length;
+          continue;
+        }
+      }
+      finalElements.push(element, duplicateAndOffsetElement(element));
+    } else {
+      finalElements.push(element);
+    }
+    i++;
+  }
+
   fixBindingsAfterDuplication(finalElements, oldElements, oldIdToDuplicatedId);
+
   return {
     elements: finalElements,
     appState: selectGroupsForSelectedElements(

src/actions/actionZindex.tsx

@@ -15,69 +15,12 @@ import {
   SendToBackIcon,
   BringForwardIcon,
 } from "../components/icons";
-import { ExcalidrawElement } from "../element/types";
-import { AppState } from "../types";
-
-const getElementIndices = (
-  direction: "left" | "right",
-  elements: readonly ExcalidrawElement[],
-  appState: AppState,
-) => {
-  const selectedIndices: number[] = [];
-  let deletedIndicesCache: number[] = [];
-
-  const cb = (element: ExcalidrawElement, index: number) => {
-    if (element.isDeleted) {
-      // we want to build an array of deleted elements that are preceeding
-      //  a selected element so that we move them together
-      deletedIndicesCache.push(index);
-    } else {
-      if (appState.selectedElementIds[element.id]) {
-        selectedIndices.push(...deletedIndicesCache, index);
-      }
-      // always empty cache of deleted elements after either pushing a group
-      //  of selected/deleted elements, of after encountering non-deleted elem
-      deletedIndicesCache = [];
-    }
-  };
-
-  // sending back → select contiguous deleted elements that are to the left of
-  //  selected element(s)
-  if (direction === "left") {
-    let i = -1;
-    const len = elements.length;
-    while (++i < len) {
-      cb(elements[i], i);
-    }
-    // moving to front → loop from right to left so that we don't need to
-    //  backtrack when gathering deleted elements
-  } else {
-    let i = elements.length;
-    while (--i > -1) {
-      cb(elements[i], i);
-    }
-  }
-  // sort in case we were gathering indexes from right to left
-  return selectedIndices.sort();
-};
-
-const moveElements = (
-  func: typeof moveOneLeft,
-  elements: readonly ExcalidrawElement[],
-  appState: AppState,
-) => {
-  const _elements = elements.slice();
-  const direction =
-    func === moveOneLeft || func === moveAllLeft ? "left" : "right";
-  const indices = getElementIndices(direction, _elements, appState);
-  return func(_elements, indices);
-};
 
 export const actionSendBackward = register({
   name: "sendBackward",
   perform: (elements, appState) => {
     return {
-      elements: moveElements(moveOneLeft, elements, appState),
+      elements: moveOneLeft(elements, appState),
       appState,
       commitToHistory: true,
     };
@@ -102,7 +45,7 @@ export const actionBringForward = register({
   name: "bringForward",
   perform: (elements, appState) => {
     return {
-      elements: moveElements(moveOneRight, elements, appState),
+      elements: moveOneRight(elements, appState),
       appState,
       commitToHistory: true,
     };
@@ -127,7 +70,7 @@ export const actionSendToBack = register({
   name: "sendToBack",
   perform: (elements, appState) => {
     return {
-      elements: moveElements(moveAllLeft, elements, appState),
+      elements: moveAllLeft(elements, appState),
       appState,
       commitToHistory: true,
     };
@@ -160,7 +103,7 @@ export const actionBringToFront = register({
   name: "bringToFront",
   perform: (elements, appState) => {
     return {
-      elements: moveElements(moveAllRight, elements, appState),
+      elements: moveAllRight(elements, appState),
       appState,
       commitToHistory: true,
     };

src/element/newElement.ts

@@ -284,7 +284,7 @@ export const duplicateElement = <TElement extends Mutable<ExcalidrawElement>>(
   overrides?: Partial<TElement>,
 ): TElement => {
   let copy: TElement = deepCopyElement(element);
-  copy.id = randomId();
+  copy.id = process.env.NODE_ENV === "test" ? `${copy.id}_copy` : randomId();
   copy.seed = randomInteger();
   copy.groupIds = getNewGroupIdsForDuplication(
     copy.groupIds,

src/groups.ts

@@ -45,10 +45,17 @@ export function isSelectedViaGroup(
   appState: AppState,
   element: ExcalidrawElement,
 ) {
-  return !!element.groupIds
+  return getSelectedGroupForElement(appState, element) != null;
+}
+
+export const getSelectedGroupForElement = (
+  appState: AppState,
+  element: ExcalidrawElement,
+) => {
+  return element.groupIds
     .filter((groupId) => groupId !== appState.editingGroupId)
     .find((groupId) => appState.selectedGroupIds[groupId]);
-}
+};
 
 export function getSelectedGroupIds(appState: AppState): GroupId[] {
   return Object.entries(appState.selectedGroupIds)

src/tests/__snapshots__/move.test.tsx.snap

@@ -8,7 +8,7 @@ Object {
   "fillStyle": "hachure",
   "groupIds": Array [],
   "height": 50,
-  "id": "id2",
+  "id": "id0_copy",
   "isDeleted": false,
   "opacity": 100,
   "roughness": 1,

src/tests/__snapshots__/regressionTests.test.tsx.snap

@@ -2366,7 +2366,7 @@ Object {
   "fillStyle": "hachure",
   "groupIds": Array [],
   "height": 10,
-  "id": "id2",
+  "id": "id0_copy",
   "isDeleted": false,
   "opacity": 100,
   "roughness": 1,
@@ -2480,7 +2480,7 @@ Object {
           "fillStyle": "hachure",
           "groupIds": Array [],
           "height": 10,
-          "id": "id2",
+          "id": "id0_copy",
           "isDeleted": false,
           "opacity": 100,
           "roughness": 1,
@@ -11227,10 +11227,10 @@ Object {
   "boundElementIds": null,
   "fillStyle": "hachure",
   "groupIds": Array [
-    "id7",
+    "id6",
   ],
   "height": 10,
-  "id": "id6",
+  "id": "id0_copy",
   "isDeleted": false,
   "opacity": 100,
   "roughness": 1,
@@ -11255,10 +11255,10 @@ Object {
   "boundElementIds": null,
   "fillStyle": "hachure",
   "groupIds": Array [
-    "id7",
+    "id6",
   ],
   "height": 10,
-  "id": "id8",
+  "id": "id1_copy",
   "isDeleted": false,
   "opacity": 100,
   "roughness": 1,
@@ -11283,10 +11283,10 @@ Object {
   "boundElementIds": null,
   "fillStyle": "hachure",
   "groupIds": Array [
-    "id7",
+    "id6",
   ],
   "height": 10,
-  "id": "id9",
+  "id": "id2_copy",
   "isDeleted": false,
   "opacity": 100,
   "roughness": 1,
@@ -11692,10 +11692,10 @@ Object {
           "boundElementIds": null,
           "fillStyle": "hachure",
           "groupIds": Array [
-            "id7",
+            "id6",
           ],
           "height": 10,
-          "id": "id6",
+          "id": "id0_copy",
           "isDeleted": false,
           "opacity": 100,
           "roughness": 1,
@@ -11717,10 +11717,10 @@ Object {
           "boundElementIds": null,
           "fillStyle": "hachure",
           "groupIds": Array [
-            "id7",
+            "id6",
           ],
           "height": 10,
-          "id": "id8",
+          "id": "id1_copy",
           "isDeleted": false,
           "opacity": 100,
           "roughness": 1,
@@ -11742,10 +11742,10 @@ Object {
           "boundElementIds": null,
           "fillStyle": "hachure",
           "groupIds": Array [
-            "id7",
+            "id6",
           ],
           "height": 10,
-          "id": "id9",
+          "id": "id2_copy",
           "isDeleted": false,
           "opacity": 100,
           "roughness": 1,
@@ -22477,7 +22477,7 @@ Object {
   "scrollY": 0,
   "scrolledOutside": false,
   "selectedElementIds": Object {
-    "id1": true,
+    "id0_copy": true,
   },
   "selectedGroupIds": Object {},
   "selectionElement": null,
@@ -22528,7 +22528,7 @@ Object {
   "fillStyle": "hachure",
   "groupIds": Array [],
   "height": 20,
-  "id": "id1",
+  "id": "id0_copy",
   "isDeleted": false,
   "opacity": 100,
   "roughness": 1,
@@ -22603,7 +22603,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {
-          "id1": true,
+          "id0_copy": true,
         },
         "viewBackgroundColor": "#ffffff",
       },
@@ -22638,7 +22638,7 @@ Object {
           "fillStyle": "hachure",
           "groupIds": Array [],
           "height": 20,
-          "id": "id1",
+          "id": "id0_copy",
           "isDeleted": false,
           "opacity": 100,
           "roughness": 1,

src/utils.ts

@@ -254,3 +254,39 @@ export const muteFSAbortError = (error?: Error) => {
   }
   throw error;
 };
+
+export const findIndex = <T>(
+  array: readonly T[],
+  cb: (element: T, index: number, array: readonly T[]) => boolean,
+  fromIndex: number = 0,
+) => {
+  if (fromIndex < 0) {
+    fromIndex = array.length + fromIndex;
+  }
+  fromIndex = Math.min(array.length, Math.max(fromIndex, 0));
+  let i = fromIndex - 1;
+  while (++i < array.length) {
+    if (cb(array[i], i, array)) {
+      return i;
+    }
+  }
+  return -1;
+};
+
+export const findLastIndex = <T>(
+  array: readonly T[],
+  cb: (element: T, index: number, array: readonly T[]) => boolean,
+  fromIndex: number = array.length - 1,
+) => {
+  if (fromIndex < 0) {
+    fromIndex = array.length + fromIndex;
+  }
+  fromIndex = Math.min(array.length - 1, Math.max(fromIndex, 0));
+  let i = fromIndex + 1;
+  while (--i > -1) {
+    if (cb(array[i], i, array)) {
+      return i;
+    }
+  }
+  return -1;
+};

src/zindex.test.ts

@@ -1,101 +0,0 @@
-import { moveOneLeft, moveOneRight, moveAllLeft, moveAllRight } from "./zindex";
-
-const expectMove = <T>(
-  fn: (elements: T[], indicesToMove: number[]) => void,
-  elems: T[],
-  indices: number[],
-  equal: T[],
-) => {
-  fn(elems, indices);
-  expect(elems).toEqual(equal);
-};
-
-it("should moveOneLeft", () => {
-  expectMove(moveOneLeft, ["a", "b", "c", "d"], [1, 2], ["b", "c", "a", "d"]);
-  expectMove(moveOneLeft, ["a", "b", "c", "d"], [0], ["a", "b", "c", "d"]);
-  expectMove(
-    moveOneLeft,
-    ["a", "b", "c", "d"],
-    [0, 1, 2, 3],
-    ["a", "b", "c", "d"],
-  );
-  expectMove(moveOneLeft, ["a", "b", "c", "d"], [1, 3], ["b", "a", "d", "c"]);
-});
-
-it("should moveOneRight", () => {
-  expectMove(moveOneRight, ["a", "b", "c", "d"], [1, 2], ["a", "d", "b", "c"]);
-  expectMove(moveOneRight, ["a", "b", "c", "d"], [3], ["a", "b", "c", "d"]);
-  expectMove(
-    moveOneRight,
-    ["a", "b", "c", "d"],
-    [0, 1, 2, 3],
-    ["a", "b", "c", "d"],
-  );
-  expectMove(moveOneRight, ["a", "b", "c", "d"], [0, 2], ["b", "a", "d", "c"]);
-});
-
-it("should moveAllLeft", () => {
-  expectMove(
-    moveAllLeft,
-    ["a", "b", "c", "d", "e", "f", "g"],
-    [2, 5],
-    ["c", "f", "a", "b", "d", "e", "g"],
-  );
-  expectMove(
-    moveAllLeft,
-    ["a", "b", "c", "d", "e", "f", "g"],
-    [5],
-    ["f", "a", "b", "c", "d", "e", "g"],
-  );
-  expectMove(
-    moveAllLeft,
-    ["a", "b", "c", "d", "e", "f", "g"],
-    [0, 1, 2, 3, 4, 5, 6],
-    ["a", "b", "c", "d", "e", "f", "g"],
-  );
-  expectMove(
-    moveAllLeft,
-    ["a", "b", "c", "d", "e", "f", "g"],
-    [0, 1, 2],
-    ["a", "b", "c", "d", "e", "f", "g"],
-  );
-  expectMove(
-    moveAllLeft,
-    ["a", "b", "c", "d", "e", "f", "g"],
-    [4, 5, 6],
-    ["e", "f", "g", "a", "b", "c", "d"],
-  );
-});
-
-it("should moveAllRight", () => {
-  expectMove(
-    moveAllRight,
-    ["a", "b", "c", "d", "e", "f", "g"],
-    [2, 5],
-    ["a", "b", "d", "e", "g", "c", "f"],
-  );
-  expectMove(
-    moveAllRight,
-    ["a", "b", "c", "d", "e", "f", "g"],
-    [5],
-    ["a", "b", "c", "d", "e", "g", "f"],
-  );
-  expectMove(
-    moveAllRight,
-    ["a", "b", "c", "d", "e", "f", "g"],
-    [0, 1, 2, 3, 4, 5, 6],
-    ["a", "b", "c", "d", "e", "f", "g"],
-  );
-  expectMove(
-    moveAllRight,
-    ["a", "b", "c", "d", "e", "f", "g"],
-    [0, 1, 2],
-    ["d", "e", "f", "g", "a", "b", "c"],
-  );
-  expectMove(
-    moveAllRight,
-    ["a", "b", "c", "d", "e", "f", "g"],
-    [4, 5, 6],
-    ["a", "b", "c", "d", "e", "f", "g"],
-  );
-});

src/zindex.ts

@@ -1,202 +1,278 @@
-const swap = <T>(elements: T[], indexA: number, indexB: number) => {
+import { AppState } from "./types";
-  const element = elements[indexA];
+import { ExcalidrawElement } from "./element/types";
-  elements[indexA] = elements[indexB];
+import { getElementsInGroup } from "./groups";
-  elements[indexB] = element;
+import { findLastIndex, findIndex } from "./utils";
-};
 
-export const moveOneLeft = <T>(elements: T[], indicesToMove: number[]) => {
+/**
-  indicesToMove.sort((a: number, b: number) => a - b);
+ * Returns indices of elements to move based on selected elements.
-  let isSorted = true;
+ * Includes contiguous deleted elements that are between two selected elements,
-  // We go from left to right to avoid overriding the wrong elements
+ *  e.g.: [0 (selected), 1 (deleted), 2 (deleted), 3 (selected)]
-  indicesToMove.forEach((index, i) => {
+ */
-    // We don't want to bubble the first elements that are sorted as they are
+const getIndicesToMove = (
-    // already in their correct position
+  elements: readonly ExcalidrawElement[],
-    isSorted = isSorted && index === i;
+  appState: AppState,
-    if (isSorted) {
+) => {
-      return;
+  let selectedIndices: number[] = [];
+  let deletedIndices: number[] = [];
+  let includeDeletedIndex = null;
+  let i = -1;
+  while (++i < elements.length) {
+    if (appState.selectedElementIds[elements[i].id]) {
+      if (deletedIndices.length) {
+        selectedIndices = selectedIndices.concat(deletedIndices);
+        deletedIndices = [];
       }
-    swap(elements, index - 1, index);
+      selectedIndices.push(i);
-  });
+      includeDeletedIndex = i + 1;
-
+    } else if (elements[i].isDeleted && includeDeletedIndex === i) {
-  return elements;
+      includeDeletedIndex = i + 1;
+      deletedIndices.push(i);
+    } else {
+      deletedIndices = [];
+    }
+  }
+  return selectedIndices;
 };
 
-export const moveOneRight = <T>(elements: T[], indicesToMove: number[]) => {
+const toContiguousGroups = (array: number[]) => {
-  const reversedIndicesToMove = indicesToMove.sort(
+  let cursor = 0;
-    (a: number, b: number) => b - a,
+  return array.reduce((acc, value, index) => {
+    if (index > 0 && array[index - 1] !== value - 1) {
+      cursor = ++cursor;
+    }
+    (acc[cursor] || (acc[cursor] = [])).push(value);
+    return acc;
+  }, [] as number[][]);
+};
+
+/**
+ * Returns next candidate index that's available to be moved to. Currently that
+ *  is a non-deleted element, and not inside a group (unless we're editing it).
+ */
+const getTargetIndex = (
+  appState: AppState,
+  elements: ExcalidrawElement[],
+  boundaryIndex: number,
+  direction: "left" | "right",
+) => {
+  const sourceElement = elements[boundaryIndex];
+
+  const indexFilter = (element: ExcalidrawElement) => {
+    if (element.isDeleted) {
+      return false;
+    }
+    // if we're editing group, find closest sibling irrespective of whether
+    //  there's a different-group element between them (for legacy reasons)
+    if (appState.editingGroupId) {
+      return element.groupIds.includes(appState.editingGroupId);
+    }
+    return true;
+  };
+
+  const candidateIndex =
+    direction === "left"
+      ? findLastIndex(elements, indexFilter, Math.max(0, boundaryIndex - 1))
+      : findIndex(elements, indexFilter, boundaryIndex + 1);
+
+  const nextElement = elements[candidateIndex];
+
+  if (!nextElement) {
+    return -1;
+  }
+
+  if (appState.editingGroupId) {
+    if (
+      // candidate element is a sibling in current editing group → return
+      sourceElement?.groupIds.join("") === nextElement?.groupIds.join("")
+    ) {
+      return candidateIndex;
+    } else if (!nextElement?.groupIds.includes(appState.editingGroupId)) {
+      // candidate element is outside current editing group → prevent
+      return -1;
+    }
+  }
+
+  if (!nextElement.groupIds.length) {
+    return candidateIndex;
+  }
+
+  const siblingGroupId = appState.editingGroupId
+    ? nextElement.groupIds[
+        nextElement.groupIds.indexOf(appState.editingGroupId) - 1
+      ]
+    : nextElement.groupIds[nextElement.groupIds.length - 1];
+
+  const elementsInSiblingGroup = getElementsInGroup(elements, siblingGroupId);
+
+  if (elementsInSiblingGroup.length) {
+    // assumes getElementsInGroup() returned elements are sorted
+    //  by zIndex (ascending)
+    return direction === "left"
+      ? elements.indexOf(elementsInSiblingGroup[0])
+      : elements.indexOf(
+          elementsInSiblingGroup[elementsInSiblingGroup.length - 1],
         );
-  let isSorted = true;
-
-  // We go from right to left to avoid overriding the wrong elements
-  reversedIndicesToMove.forEach((index, i) => {
-    // We don't want to bubble the first elements that are sorted as they are
-    // already in their correct position
-    isSorted = isSorted && index === elements.length - i - 1;
-    if (isSorted) {
-      return;
   }
-    swap(elements, index + 1, index);
+
-  });
+  return candidateIndex;
-  return elements;
 };
 
-// Let's go through an example
+const shiftElements = (
-//        |        |
+  appState: AppState,
-// [a, b, c, d, e, f, g]
+  elements: ExcalidrawElement[],
-// -->
+  direction: "left" | "right",
-// [c, f, a, b, d, e, g]
+) => {
-//
+  const indicesToMove = getIndicesToMove(elements, appState);
-// We are going to override all the elements we want to move, so we keep them in an array
+  let groupedIndices = toContiguousGroups(indicesToMove);
-// that we will restore at the end.
-// [c, f]
-//
-// From now on, we'll never read those values from the array anymore
-//        |1       |0
-// [a, b, _, d, e, _, g]
-//
-// The idea is that we want to shift all the elements between the marker 0 and 1
-// by one slot to the right.
-//
-//        |1       |0
-// [a, b, _, d, e, _, g]
-//          -> ->
-//
-// which gives us
-//
-//        |1       |0
-// [a, b, _, _, d, e, g]
-//
-// Now, we need to move all the elements from marker 1 to the beginning by two (not one)
-// slots to the right, which gives us
-//
-//        |1       |0
-// [a, b, _, _, d, e, g]
-//  ---|--^  ^
-//     ------|
-//
-// which gives us
-//
-//        |1       |0
-// [_, _, a, b, d, e, g]
-//
-// At this point, we can fill back the leftmost elements with the array we saved at
-// the beginning
-//
-//        |1       |0
-// [c, f, a, b, d, e, g]
-//
-// And we are done!
-export const moveAllLeft = <T>(elements: T[], indicesToMove: number[]) => {
-  indicesToMove.sort((a: number, b: number) => a - b);
 
-  // Copy the elements to move
+  if (direction === "right") {
-  const leftMostElements = indicesToMove.map((index) => elements[index]);
+    groupedIndices = groupedIndices.reverse();
-
-  const reversedIndicesToMove = indicesToMove
-    // We go from right to left to avoid overriding elements.
-    .reverse()
-    // We add 0 for the final marker
-    .concat([0]);
-
-  reversedIndicesToMove.forEach((index, i) => {
-    // We skip the first one as it is not paired with anything else
-    if (i === 0) {
-      return;
   }
 
-    // We go from the next marker to the right (i - 1) to the current one (index)
+  groupedIndices.forEach((indices, i) => {
-    for (let pos = reversedIndicesToMove[i - 1] - 1; pos >= index; --pos) {
+    const leadingIndex = indices[0];
-      // We move by 1 the first time, 2 the second... So we can use the index i in the array
+    const trailingIndex = indices[indices.length - 1];
-      elements[pos + i] = elements[pos];
+    const boundaryIndex = direction === "left" ? leadingIndex : trailingIndex;
-    }
-  });
 
-  // The final step
+    const targetIndex = getTargetIndex(
-  leftMostElements.forEach((element, i) => {
+      appState,
-    elements[i] = element;
+      elements,
-  });
+      boundaryIndex,
-
+      direction,
-  return elements;
-};
-
-// Let's go through an example
-//        |        |
-// [a, b, c, d, e, f, g]
-// -->
-// [a, b, d, e, g, c, f]
-//
-// We are going to override all the elements we want to move, so we keep them in an array
-// that we will restore at the end.
-// [c, f]
-//
-// From now on, we'll never read those values from the array anymore
-//        |0       |1
-// [a, b, _, d, e, _, g]
-//
-// The idea is that we want to shift all the elements between the marker 0 and 1
-// by one slot to the left.
-//
-//        |0       |1
-// [a, b, _, d, e, _, g]
-//          <- <-
-//
-// which gives us
-//
-//        |0       |1
-// [a, b, d, e, _, _, g]
-//
-// Now, we need to move all the elements from marker 1 to the end by two (not one)
-// slots to the left, which gives us
-//
-//        |0       |1
-// [a, b, d, e, _, _, g]
-//              ^------
-//
-// which gives us
-//
-//        |0       |1
-// [a, b, d, e, g, _, _]
-//
-// At this point, we can fill back the rightmost elements with the array we saved at
-// the beginning
-//
-//        |0       |1
-// [a, b, d, e, g, c, f]
-//
-// And we are done!
-export const moveAllRight = <T>(elements: T[], indicesToMove: number[]) => {
-  const reversedIndicesToMove = indicesToMove.sort(
-    (a: number, b: number) => b - a,
     );
 
-  // Copy the elements to move
+    if (targetIndex === -1 || boundaryIndex === targetIndex) {
-  const rightMostElements = reversedIndicesToMove.map(
-    (index) => elements[index],
-  );
-
-  indicesToMove = reversedIndicesToMove
-    // We go from left to right to avoid overriding elements.
-    .reverse()
-    // We last element index for the final marker
-    .concat([elements.length]);
-
-  indicesToMove.forEach((index, i) => {
-    // We skip the first one as it is not paired with anything else
-    if (i === 0) {
       return;
     }
 
-    // We go from the next marker to the left (i - 1) to the current one (index)
+    const leadingElements =
-    for (let pos = indicesToMove[i - 1] + 1; pos < index; ++pos) {
+      direction === "left"
-      // We move by 1 the first time, 2 the second... So we can use the index i in the array
+        ? elements.slice(0, targetIndex)
-      elements[pos - i] = elements[pos];
+        : elements.slice(0, leadingIndex);
-    }
+    const targetElements = elements.slice(leadingIndex, trailingIndex + 1);
-  });
+    const displacedElements =
+      direction === "left"
+        ? elements.slice(targetIndex, leadingIndex)
+        : elements.slice(trailingIndex + 1, targetIndex + 1);
+    const trailingElements =
+      direction === "left"
+        ? elements.slice(trailingIndex + 1)
+        : elements.slice(targetIndex + 1);
 
-  // The final step
+    elements =
-  rightMostElements.forEach((element, i) => {
+      direction === "left"
-    elements[elements.length - i - 1] = element;
+        ? [
+            ...leadingElements,
+            ...targetElements,
+            ...displacedElements,
+            ...trailingElements,
+          ]
+        : [
+            ...leadingElements,
+            ...displacedElements,
+            ...targetElements,
+            ...trailingElements,
+          ];
   });
 
   return elements;
 };
+
+const shiftElementsToEnd = (
+  elements: readonly ExcalidrawElement[],
+  appState: AppState,
+  direction: "left" | "right",
+) => {
+  const indicesToMove = getIndicesToMove(elements, appState);
+  const targetElements: ExcalidrawElement[] = [];
+  const displacedElements: ExcalidrawElement[] = [];
+
+  let leadingIndex, trailingIndex;
+  if (direction === "left") {
+    if (appState.editingGroupId) {
+      const groupElements = getElementsInGroup(
+        elements,
+        appState.editingGroupId,
+      );
+      if (!groupElements.length) {
+        return elements;
+      }
+      leadingIndex = elements.indexOf(groupElements[0]);
+    } else {
+      leadingIndex = 0;
+    }
+
+    trailingIndex = indicesToMove[indicesToMove.length - 1];
+  } else {
+    if (appState.editingGroupId) {
+      const groupElements = getElementsInGroup(
+        elements,
+        appState.editingGroupId,
+      );
+      if (!groupElements.length) {
+        return elements;
+      }
+      trailingIndex = elements.indexOf(groupElements[groupElements.length - 1]);
+    } else {
+      trailingIndex = elements.length - 1;
+    }
+
+    leadingIndex = indicesToMove[0];
+  }
+
+  for (let index = leadingIndex; index < trailingIndex + 1; index++) {
+    if (indicesToMove.includes(index)) {
+      targetElements.push(elements[index]);
+    } else {
+      displacedElements.push(elements[index]);
+    }
+  }
+
+  const leadingElements = elements.slice(0, leadingIndex);
+  const trailingElements = elements.slice(trailingIndex + 1);
+
+  return direction === "left"
+    ? [
+        ...leadingElements,
+        ...targetElements,
+        ...displacedElements,
+        ...trailingElements,
+      ]
+    : [
+        ...leadingElements,
+        ...displacedElements,
+        ...targetElements,
+        ...trailingElements,
+      ];
+};
+
+// public API
+// -----------------------------------------------------------------------------
+
+export const moveOneLeft = (
+  elements: readonly ExcalidrawElement[],
+  appState: AppState,
+) => {
+  return shiftElements(appState, elements.slice(), "left");
+};
+
+export const moveOneRight = (
+  elements: readonly ExcalidrawElement[],
+  appState: AppState,
+) => {
+  return shiftElements(appState, elements.slice(), "right");
+};
+
+export const moveAllLeft = (
+  elements: readonly ExcalidrawElement[],
+  appState: AppState,
+) => {
+  return shiftElementsToEnd(elements, appState, "left");
+};
+
+export const moveAllRight = (
+  elements: readonly ExcalidrawElement[],
+  appState: AppState,
+) => {
+  return shiftElementsToEnd(elements, appState, "right");
+};