diff --git a/packages/excalidraw/change.ts b/packages/excalidraw/change.ts index 8774cfdb9..884235e80 100644 --- a/packages/excalidraw/change.ts +++ b/packages/excalidraw/change.ts @@ -1477,19 +1477,28 @@ export class ElementsChange implements Change { return elements; } - const previous = Array.from(elements.values()); - const reordered = orderByFractionalIndex([...previous]); + const unordered = Array.from(elements.values()); + const ordered = orderByFractionalIndex([...unordered]); + const moved = Delta.getRightDifferences(unordered, ordered, true).reduce( + (acc, arrayIndex) => { + const candidate = unordered[Number(arrayIndex)]; + if (candidate && changed.has(candidate.id)) { + acc.set(candidate.id, candidate); + } - if ( - !flags.containsVisibleDifference && - Delta.isRightDifferent(previous, reordered, true) - ) { + return acc; + }, + new Map(), + ); + + if (!flags.containsVisibleDifference && moved.size) { // we found a difference in order! flags.containsVisibleDifference = true; } - // let's synchronize all invalid indices of moved elements - return arrayToMap(syncMovedIndices(reordered, changed)) as typeof elements; + // synchronize all elements that were actually moved + // could fallback to synchronizing all invalid indices + return arrayToMap(syncMovedIndices(ordered, moved)) as typeof elements; } /** diff --git a/packages/excalidraw/fractionalIndex.ts b/packages/excalidraw/fractionalIndex.ts index 618500439..d66a23f59 100644 --- a/packages/excalidraw/fractionalIndex.ts +++ b/packages/excalidraw/fractionalIndex.ts @@ -133,27 +133,11 @@ const getMovedIndicesGroups = ( let i = 0; while (i < elements.length) { - if ( - movedElements.has(elements[i].id) && - !isValidFractionalIndex( - elements[i]?.index, - elements[i - 1]?.index, - elements[i + 1]?.index, - ) - ) { + if (movedElements.has(elements[i].id)) { const indicesGroup = [i - 1, i]; // push the lower bound index as the first item while (++i < elements.length) { - if ( - !( - movedElements.has(elements[i].id) && - !isValidFractionalIndex( - elements[i]?.index, - elements[i - 1]?.index, - elements[i + 1]?.index, - ) - ) - ) { + if (!movedElements.has(elements[i].id)) { break; } diff --git a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap index d6f4114ed..89e937151 100644 --- a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap @@ -17320,7 +17320,7 @@ exports[`history > singleplayer undo/redo > should support changes in elements' "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 6, + "version": 7, "width": 10, "x": 10, "y": 0, @@ -17352,7 +17352,7 @@ exports[`history > singleplayer undo/redo > should support changes in elements' "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 9, + "version": 10, "width": 10, "x": 40, "y": 40, @@ -17604,7 +17604,7 @@ History { "index": "a2", }, "inserted": { - "index": "a0", + "index": "Zz", }, }, "id41" => Delta { @@ -17612,7 +17612,7 @@ History { "index": "a3", }, "inserted": { - "index": "a0V", + "index": "a0", }, }, }, diff --git a/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap index 0855a70d8..611d7c559 100644 --- a/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap @@ -336,7 +336,7 @@ History { "groupIds": [ "id5", ], - "index": "a1V", + "index": "a2", }, "inserted": { "groupIds": [], @@ -348,9 +348,11 @@ History { "groupIds": [ "id5", ], + "index": "a3", }, "inserted": { "groupIds": [], + "index": "a2", }, }, }, @@ -719,7 +721,7 @@ History { "groupIds": [ "id4", ], - "index": "a1V", + "index": "a2", }, "inserted": { "groupIds": [], @@ -731,9 +733,11 @@ History { "groupIds": [ "id4", ], + "index": "a3", }, "inserted": { "groupIds": [], + "index": "a2", }, }, }, @@ -1856,7 +1860,7 @@ History { "groupIds": [ "id5", ], - "index": "a1V", + "index": "a2", }, "inserted": { "groupIds": [], @@ -1868,9 +1872,11 @@ History { "groupIds": [ "id5", ], + "index": "a3", }, "inserted": { "groupIds": [], + "index": "a2", }, }, }, @@ -12810,7 +12816,7 @@ History { "id5", "id3", ], - "index": "a1V", + "index": "a2", }, "inserted": { "groupIds": [ @@ -12825,11 +12831,13 @@ History { "id5", "id3", ], + "index": "a3", }, "inserted": { "groupIds": [ "id3", ], + "index": "a2", }, }, }, diff --git a/packages/excalidraw/tests/fractionalIndex.test.ts b/packages/excalidraw/tests/fractionalIndex.test.ts index 2875fd0fb..60cf4f5a5 100644 --- a/packages/excalidraw/tests/fractionalIndex.test.ts +++ b/packages/excalidraw/tests/fractionalIndex.test.ts @@ -77,97 +77,6 @@ describe("sync invalid indices with array order", () => { }); }); - describe("should NOT sync index when it is already valid", () => { - testMovedIndicesSync({ - elements: [ - { id: "A", index: "a2" }, - { id: "B", index: "a4" }, - ], - movedElements: ["A"], - expect: { - validInput: true, - unchangedElements: ["A", "B"], - }, - }); - - testMovedIndicesSync({ - elements: [ - { id: "A", index: "a2" }, - { id: "B", index: "a4" }, - ], - movedElements: ["B"], - expect: { - validInput: true, - unchangedElements: ["A", "B"], - }, - }); - }); - - describe("should NOT sync indices when they are already valid", () => { - { - testMovedIndicesSync({ - elements: [ - { id: "A", index: "a1" }, - { id: "B", index: "a0" }, - { id: "C", index: "a2" }, - ], - movedElements: ["B", "C"], - expect: { - // this should not sync 'C' - unchangedElements: ["A", "C"], - }, - }); - - testMovedIndicesSync({ - elements: [ - { id: "A", index: "a1" }, - { id: "B", index: "a0" }, - { id: "C", index: "a2" }, - ], - movedElements: ["A", "B"], - expect: { - // but this should sync 'A' as it's invalid! - unchangedElements: ["C"], - }, - }); - } - - testMovedIndicesSync({ - elements: [ - { id: "A", index: "a0" }, - { id: "B", index: "a2" }, - { id: "C", index: "a1" }, - { id: "D", index: "a1" }, - { id: "E", index: "a2" }, - ], - movedElements: ["B", "D", "E"], - expect: { - // should not sync 'E' - unchangedElements: ["A", "C", "E"], - }, - }); - - testMovedIndicesSync({ - elements: [ - { id: "A" }, - { id: "B" }, - { id: "C", index: "a0" }, - { id: "D", index: "a2" }, - { id: "E" }, - { id: "F", index: "a3" }, - { id: "G" }, - { id: "H", index: "a1" }, - { id: "I", index: "a2" }, - { id: "J" }, - ], - movedElements: ["A", "B", "D", "E", "F", "G", "J"], - expect: { - // should not sync 'D' and 'F' - unchangedElements: ["C", "D", "F"], - }, - }); - }); - describe("should sync when fractional index is not defined", () => { testMovedIndicesSync({ elements: [{ id: "A" }], @@ -384,6 +293,122 @@ describe("sync invalid indices with array order", () => { }); }); + describe("should sync all moved elements regardless of their validity", () => { + testMovedIndicesSync({ + elements: [ + { id: "A", index: "a2" }, + { id: "B", index: "a4" }, + ], + movedElements: ["A"], + expect: { + validInput: true, + unchangedElements: ["B"], + }, + }); + + testMovedIndicesSync({ + elements: [ + { id: "A", index: "a2" }, + { id: "B", index: "a4" }, + ], + movedElements: ["B"], + expect: { + validInput: true, + unchangedElements: ["A"], + }, + }); + + testMovedIndicesSync({ + elements: [ + { id: "C", index: "a2" }, + { id: "D", index: "a3" }, + { id: "A", index: "a0" }, + { id: "B", index: "a1" }, + ], + movedElements: ["C", "D"], + expect: { + unchangedElements: ["A", "B"], + }, + }); + + testMovedIndicesSync({ + elements: [ + { id: "A", index: "a1" }, + { id: "B", index: "a2" }, + { id: "D", index: "a4" }, + { id: "C", index: "a3" }, + { id: "F", index: "a6" }, + { id: "E", index: "a5" }, + { id: "H", index: "a8" }, + { id: "G", index: "a7" }, + { id: "I", index: "a9" }, + ], + movedElements: ["D", "F", "H"], + expect: { + unchangedElements: ["A", "B", "C", "E", "G", "I"], + }, + }); + + { + testMovedIndicesSync({ + elements: [ + { id: "A", index: "a1" }, + { id: "B", index: "a0" }, + { id: "C", index: "a2" }, + ], + movedElements: ["B", "C"], + expect: { + unchangedElements: ["A"], + }, + }); + + testMovedIndicesSync({ + elements: [ + { id: "A", index: "a1" }, + { id: "B", index: "a0" }, + { id: "C", index: "a2" }, + ], + movedElements: ["A", "B"], + expect: { + unchangedElements: ["C"], + }, + }); + } + + testMovedIndicesSync({ + elements: [ + { id: "A", index: "a0" }, + { id: "B", index: "a2" }, + { id: "C", index: "a1" }, + { id: "D", index: "a1" }, + { id: "E", index: "a2" }, + ], + movedElements: ["B", "D", "E"], + expect: { + unchangedElements: ["A", "C"], + }, + }); + + testMovedIndicesSync({ + elements: [ + { id: "A" }, + { id: "B" }, + { id: "C", index: "a0" }, + { id: "D", index: "a2" }, + { id: "E" }, + { id: "F", index: "a3" }, + { id: "G" }, + { id: "H", index: "a1" }, + { id: "I", index: "a2" }, + { id: "J" }, + ], + movedElements: ["A", "B", "D", "E", "F", "G", "J"], + expect: { + unchangedElements: ["C", "H", "I"], + }, + }); + }); + describe("should generate fractions for explicitly moved elements", () => { describe("should generate a fraction between 'A' and 'C'", () => { testMovedIndicesSync({