Dylan/excalidraw
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix various issues, fix tests

Browse Source
This commit is contained in:
Marcel Mraz 2025-05-24 23:27:05 +02:00
parent 57c0eca619
commit 0428c07a63
No known key found for this signature in database
GPG Key ID: 4EBD6E62DC830CD2
12 changed files with 2565 additions and 3175 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
excalidraw-app/tests/collab.test.tsx
View File

@ -205,6 +205,7 @@ describe("collaboration", () => {
// with explicit undo (as addition) we expect our item to be restored from the snapshot!
await waitFor(() => {
expect(API.getUndoStack().length).toBe(1);
expect(API.getRedoStack().length).toBe(1);
expect(API.getSnapshot()).toEqual([
expect.objectContaining(rect1Props),
expect.objectContaining({ ...rect2Props, isDeleted: false }),
@ -236,7 +237,7 @@ describe("collaboration", () => {
// with explicit redo (as removal) we again restore the element from the snapshot!
await waitFor(() => {
expect(API.getUndoStack().length).toBe(2);
expect(API.getUndoStack().length).toBe(1);
expect(API.getRedoStack().length).toBe(0);
expect(API.getSnapshot()).toEqual([
expect.objectContaining(rect1Props),
@ -248,13 +249,11 @@ describe("collaboration", () => {
]);
});
act(() => h.app.actionManager.executeAction(undoAction));
// simulate local update
API.updateScene({
elements: syncInvalidIndices([
h.elements[0],
newElementWith(h.elements[1], { x: 100 }),
newElementWith(h.elements[1], { x: 100, isDeleted: false }),
]),
captureUpdate: CaptureUpdateAction.IMMEDIATELY,
});
@ -271,55 +270,5 @@ describe("collaboration", () => {
expect.objectContaining({ ...rect2Props, isDeleted: false, x: 100 }),
]);
});
act(() => h.app.actionManager.executeAction(undoAction));
// we expect to iterate the stack to the first visible change
await waitFor(() => {
expect(API.getUndoStack().length).toBe(1);
expect(API.getRedoStack().length).toBe(1);
expect(API.getSnapshot()).toEqual([
expect.objectContaining(rect1Props),
expect.objectContaining({ ...rect2Props, isDeleted: false, x: 0 }),
]);
expect(h.elements).toEqual([
expect.objectContaining(rect1Props),
expect.objectContaining({ ...rect2Props, isDeleted: false, x: 0 }),
]);
});
// simulate force deleting the element remotely
API.updateScene({
elements: syncInvalidIndices([rect1]),
captureUpdate: CaptureUpdateAction.NEVER,
});
// snapshot was correctly updated and marked the element as deleted
await waitFor(() => {
expect(API.getUndoStack().length).toBe(1);
expect(API.getRedoStack().length).toBe(1);
expect(API.getSnapshot()).toEqual([
expect.objectContaining(rect1Props),
expect.objectContaining({ ...rect2Props, isDeleted: true, x: 0 }),
]);
expect(h.elements).toEqual([expect.objectContaining(rect1Props)]);
});
act(() => h.app.actionManager.executeAction(redoAction));
// with explicit redo (as update) we again restored the element from the snapshot!
await waitFor(() => {
expect(API.getUndoStack().length).toBe(2);
expect(API.getRedoStack().length).toBe(0);
expect(API.getSnapshot()).toEqual([
expect.objectContaining({ id: "A", isDeleted: false }),
expect.objectContaining({ id: "B", isDeleted: true, x: 100 }),
]);
expect(h.history.isRedoStackEmpty).toBeTruthy();
expect(h.elements).toEqual([
expect.objectContaining({ id: "A", isDeleted: false }),
expect.objectContaining({ id: "B", isDeleted: true, x: 100 }),
]);
});
});
});

148
packages/element/src/delta.ts
View File

@ -7,12 +7,9 @@ import {
isTestEnv,
} from "@excalidraw/common";
import type { LocalPoint } from "@excalidraw/math/types";
import type {
ExcalidrawElement,
ExcalidrawFreeDrawElement,
ExcalidrawImageElement,
ExcalidrawLinearElement,
ExcalidrawTextElement,
NonDeleted,
@ -266,12 +263,14 @@ export class Delta<T> {
arrayToObject(deletedArray, groupBy),
arrayToObject(insertedArray, groupBy),
),
(x) => x,
);
const insertedDifferences = arrayToObject(
Delta.getRightDifferences(
arrayToObject(deletedArray, groupBy),
arrayToObject(insertedArray, groupBy),
),
(x) => x,
);
if (
@ -871,13 +870,6 @@ export class AppStateDelta implements DeltaContainer<AppState> {
type ElementPartial<TElement extends ExcalidrawElement = ExcalidrawElement> =
Omit<Partial<Ordered<TElement>>, "id" | "updated" | "seed">;
type ElementPartialWithPoints = Omit<
ElementPartial<ExcalidrawFreeDrawElement | ExcalidrawLinearElement>,
"points"
> & {
points: { [key: string]: LocalPoint };
};
export type ApplyToOptions = {
excludedProperties: Set<keyof ElementPartial>;
};
@ -1108,6 +1100,70 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
);
}
/**
* Update delta/s based on the existing elements.
*
* @param elements current elements
* @param modifierOptions defines which of the delta (`deleted` or `inserted`) will be updated
* @returns new instance with modified delta/s
*/
public applyLatestChanges(
elements: SceneElementsMap,
modifierOptions: "deleted" | "inserted",
): ElementsDelta {
const modifier =
(element: OrderedExcalidrawElement) => (partial: ElementPartial) => {
const latestPartial: { [key: string]: unknown } = {};
for (const key of Object.keys(partial) as Array<keyof typeof partial>) {
// do not update following props:
// - `boundElements`, as it is a reference value which is postprocessed to contain only deleted/inserted keys
switch (key) {
case "boundElements":
latestPartial[key] = partial[key];
break;
default:
latestPartial[key] = element[key];
}
}
return latestPartial;
};
const applyLatestChangesInternal = (
deltas: Record<string, Delta<ElementPartial>>,
) => {
const modifiedDeltas: Record<string, Delta<ElementPartial>> = {};
for (const [id, delta] of Object.entries(deltas)) {
const existingElement = elements.get(id);
if (existingElement) {
const modifiedDelta = Delta.create(
delta.deleted,
delta.inserted,
modifier(existingElement),
modifierOptions,
);
modifiedDeltas[id] = modifiedDelta;
} else {
modifiedDeltas[id] = delta;
}
}
return modifiedDeltas;
};
const added = applyLatestChangesInternal(this.added);
const removed = applyLatestChangesInternal(this.removed);
const updated = applyLatestChangesInternal(this.updated);
return ElementsDelta.create(added, removed, updated, {
shouldRedistribute: true, // redistribute the deltas as `isDeleted` could have been updated
});
}
public applyTo(
elements: SceneElementsMap,
snapshot: StoreSnapshot["elements"] = StoreSnapshot.empty().elements,
@ -1266,14 +1322,13 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
) {
const directlyApplicablePartial: Mutable<ElementPartial> = {};
// some properties are not directly applicable, such as:
// - boundElements which contains only diff)
// - version & versionNonce, if we don't want to return to previous versions
for (const key of Object.keys(delta.inserted) as Array<
keyof typeof delta.inserted
>) {
if (
key === "boundElements" ||
(key as keyof ExcalidrawFreeDrawElement | ExcalidrawLinearElement) ===
"points"
) {
if (key === "boundElements") {
continue;
}
@ -1301,32 +1356,6 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
});
}
const deletedPoints = (delta.deleted as ElementPartialWithPoints).points;
const insertedPoints = (delta.inserted as ElementPartialWithPoints).points;
if (insertedPoints && deletedPoints) {
const mergedPoints = Delta.mergeObjects(
arrayToObject(
(element as ExcalidrawFreeDrawElement | ExcalidrawLinearElement)
.points,
),
insertedPoints,
deletedPoints,
);
const sortedPoints = Object.entries(mergedPoints)
.sort((aKey, bKey) => {
const a = Number(aKey);
const b = Number(bKey);
return a - b;
})
.map(([_, value]) => value);
Object.assign(directlyApplicablePartial, {
points: sortedPoints,
});
}
if (!flags.containsVisibleDifference) {
// strip away fractional index, as even if it would be different, it doesn't have to result in visible change
const { index, ...rest } = directlyApplicablePartial;
@ -1634,18 +1663,31 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
try {
Delta.diffArrays(deleted, inserted, "boundElements", (x) => x.id);
// points depend on the order, so we diff them as objects and group by index
// creates `ElementPartialWithPoints`
Delta.diffObjects(
deleted as ElementPartial<
ExcalidrawFreeDrawElement | ExcalidrawLinearElement
>,
inserted as ElementPartial<
ExcalidrawFreeDrawElement | ExcalidrawLinearElement
>,
"points",
(prevValue) => prevValue!,
);
// don't diff the points as:
// - we can't ensure the multiplayer order consistency without fractional index on each point
// - we prefer to not merge the points, as it might just lead to unexpected / incosistent results
const deletedPoints =
(
deleted as ElementPartial<
ExcalidrawFreeDrawElement | ExcalidrawLinearElement
>
).points ?? [];
const insertedPoints =
(
inserted as ElementPartial<
ExcalidrawFreeDrawElement | ExcalidrawLinearElement
>
).points ?? [];
if (
!Delta.isLeftDifferent(deletedPoints, insertedPoints) &&
!Delta.isRightDifferent(deletedPoints, insertedPoints)
) {
// delete the points from delta if there is no difference, otherwise leave them as they were captured due to consistency
Reflect.deleteProperty(deleted, "points");
Reflect.deleteProperty(inserted, "points");
}
} catch (e) {
// if postprocessing fails, it does not make sense to bubble up, but let's make sure we know about it
console.error(`Couldn't postprocess elements delta.`);

45
packages/element/src/fractionalIndex.ts
View File

@ -2,7 +2,9 @@ import { generateNKeysBetween } from "fractional-indexing";
import { arrayToMap } from "@excalidraw/common";
import { mutateElement } from "./mutateElement";
import type { Mutable } from "@excalidraw/common/utility-types";
import { mutateElement, newElementWith } from "./mutateElement";
import { getBoundTextElement } from "./textElement";
import { hasBoundTextElement } from "./typeChecks";
@ -161,9 +163,15 @@ export const syncMovedIndices = (
// try generatating indices, throws on invalid movedElements
const elementsUpdates = generateIndices(elements, indicesGroups);
const elementsCandidates = elements.map((x) =>
elementsUpdates.has(x) ? { ...x, ...elementsUpdates.get(x) } : x,
);
const elementsCandidates = elements.map((x) => {
const elementUpdates = elementsUpdates.get(x);
if (elementUpdates) {
return { ...x, index: elementUpdates.index };
}
return x;
});
// ensure next indices are valid before mutation, throws on invalid ones
validateFractionalIndices(
@ -177,8 +185,8 @@ export const syncMovedIndices = (
);
// split mutation so we don't end up in an incosistent state
for (const [element, update] of elementsUpdates) {
mutateElement(element, elementsMap, update);
for (const [element, { index }] of elementsUpdates) {
mutateElement(element, elementsMap, { index });
}
} catch (e) {
// fallback to default sync
@ -189,19 +197,33 @@ export const syncMovedIndices = (
};
/**
* Synchronizes all invalid fractional indices with the array order by mutating passed elements.
* Synchronizes all invalid fractional indices with the array order by mutating passed elements array.
*
* When `shouldCreateNewInstances` is true, it creates new instances of the elements, instead of mutating the existing ones.
*
* WARN: in edge cases it could modify the elements which were not moved, as it's impossible to guess the actually moved elements from the elements array itself.
*/
export const syncInvalidIndices = (
elements: readonly ExcalidrawElement[],
{
shouldCreateNewInstances = false,
}: {
shouldCreateNewInstances?: boolean;
} = {},
): OrderedExcalidrawElement[] => {
const elementsMap = arrayToMap(elements);
const indicesGroups = getInvalidIndicesGroups(elements);
const elementsUpdates = generateIndices(elements, indicesGroups);
for (const [element, update] of elementsUpdates) {
mutateElement(element, elementsMap, update);
for (const [element, { index, arrayIndex }] of elementsUpdates) {
if (shouldCreateNewInstances) {
const updatedElement = newElementWith(element, { index });
// mutate the element in the array with the new updated instance
(elements as Mutable<typeof elements>)[arrayIndex] = updatedElement;
} else {
const elementsMap = arrayToMap(elements);
mutateElement(element, elementsMap, { index });
}
}
return elements as OrderedExcalidrawElement[];
@ -380,7 +402,7 @@ const generateIndices = (
) => {
const elementsUpdates = new Map<
ExcalidrawElement,
{ index: FractionalIndex }
{ index: FractionalIndex; arrayIndex: number }
>();
for (const indices of indicesGroups) {
@ -398,6 +420,7 @@ const generateIndices = (
elementsUpdates.set(element, {
index: fractionalIndices[i],
arrayIndex: indices[i],
});
}
}

27
packages/element/src/store.ts
View File

@ -533,9 +533,7 @@ export class StoreDelta {
id,
elements: { added, removed, updated },
}: DTO<StoreDelta>) {
const elements = ElementsDelta.create(added, removed, updated, {
shouldRedistribute: false,
});
const elements = ElementsDelta.create(added, removed, updated);
return new this(id, elements, AppStateDelta.empty());
}
@ -676,11 +674,10 @@ export class StoreSnapshot {
nextElements.set(id, changedElement);
}
const nextAppState = Object.assign(
{},
this.appState,
change.appState,
) as ObservedAppState;
const nextAppState = getObservedAppState({
...this.appState,
...change.appState,
});
return StoreSnapshot.create(nextElements, nextAppState, {
// by default we assume that change is different from what we have in the snapshot
@ -933,18 +930,26 @@ const getDefaultObservedAppState = (): ObservedAppState => {
};
};
export const getObservedAppState = (appState: AppState): ObservedAppState => {
export const getObservedAppState = (
appState: AppState | ObservedAppState,
): ObservedAppState => {
const observedAppState = {
name: appState.name,
editingGroupId: appState.editingGroupId,
viewBackgroundColor: appState.viewBackgroundColor,
selectedElementIds: appState.selectedElementIds,
selectedGroupIds: appState.selectedGroupIds,
editingLinearElementId: appState.editingLinearElement?.elementId || null,
selectedLinearElementId: appState.selectedLinearElement?.elementId || null,
croppingElementId: appState.croppingElementId,
activeLockedId: appState.activeLockedId,
lockedMultiSelections: appState.lockedMultiSelections,
editingLinearElementId:
(appState as AppState).editingLinearElement?.elementId ?? // prefer app state, as it's likely newer
(appState as ObservedAppState).editingLinearElementId ?? // fallback to observed app state, as it's likely older coming from a previous snapshot
null,
selectedLinearElementId:
(appState as AppState).selectedLinearElement?.elementId ??
(appState as ObservedAppState).selectedLinearElementId ??
null,
};
Reflect.defineProperty(observedAppState, hiddenObservedAppStateProp, {

2
packages/element/tests/duplicate.test.tsx
View File

@ -505,8 +505,6 @@ describe("group-related duplication", () => {
mouse.up(frame.x + frame.width + 50, frame.y + frame.height + 50);
});
// console.log(h.elements);
assertElements(h.elements, [
{ id: frame.id },
{ id: rectangle1.id, frameId: frame.id },

24
packages/excalidraw/components/App.tsx
View File

@ -104,7 +104,11 @@ import {
Emitter,
} from "@excalidraw/common";
import { getCommonBounds, getElementAbsoluteCoords } from "@excalidraw/element";
import {
getCommonBounds,
getElementAbsoluteCoords,
getObservedAppState,
} from "@excalidraw/element";
import {
bindOrUnbindLinearElement,
@ -3993,22 +3997,30 @@ class App extends React.Component<AppProps, AppState> {
}) => {
const { elements, appState, collaborators, captureUpdate } = sceneData;
const nextElements = elements ? syncInvalidIndices(elements) : undefined;
const nextElements = elements
? syncInvalidIndices(elements, {
// we have to create new instances here, otherwise scheduled micro action below won't be able to
// detect the fractional index change and won't update the store snapshot
shouldCreateNewInstances: true,
})
: undefined;
if (captureUpdate) {
const nextElementsMap = elements
? (arrayToMap(nextElements ?? []) as SceneElementsMap)
: undefined;
const nextAppState = appState
? // new instance, with partial appstate applied to previously captured one, including hidden prop inside `prevCommittedAppState`
Object.assign({}, this.store.snapshot.appState, appState)
const nextObservedAppState = appState
? getObservedAppState({
...this.store.snapshot.appState,
...appState,
})
: undefined;
this.store.scheduleMicroAction({
action: captureUpdate,
elements: nextElementsMap,
appState: nextAppState,
appState: nextObservedAppState,
});
}

10
packages/excalidraw/history.ts
View File

@ -13,7 +13,7 @@ import type { SceneElementsMap } from "@excalidraw/element/types";
import type { AppState } from "./types";
class HistoryDelta extends StoreDelta {
export class HistoryDelta extends StoreDelta {
/**
* Apply the delta to the passed elements and appState, does not modify the snapshot.
*/
@ -27,7 +27,9 @@ class HistoryDelta extends StoreDelta {
// used to fallback into local snapshot in case we couldn't apply the delta
// due to a missing elements in the scene (force deleted)
snapshot.elements,
// we don't want to apply the version and versionNonce properties for history
// we don't want to apply the `version` and `versionNonce` properties for history
// as we always need to end up with a new version due to collaboration,
// approaching each undo / redo as a new user operation
{
excludedProperties: new Set(["version", "versionNonce"]),
},
@ -221,6 +223,10 @@ export class History {
}
private static push(stack: HistoryDelta[], entry: HistoryDelta) {
if (entry.isEmpty()) {
return;
}
const inversedEntry = HistoryDelta.inverse(entry);
return stack.push(inversedEntry);
}

64
packages/excalidraw/tests/__snapshots__/contextmenu.test.tsx.snap
View File

@ -1277,6 +1277,7 @@ exports[`contextMenu element > selecting 'Add to library' in context menu adds e
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": -10,
"y": 0,
@ -1525,6 +1526,7 @@ exports[`contextMenu element > selecting 'Bring forward' in context menu brings
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": -10,
"y": 0,
@ -1579,6 +1581,7 @@ exports[`contextMenu element > selecting 'Bring forward' in context menu brings
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": 20,
"y": 30,
@ -1614,9 +1617,11 @@ exports[`contextMenu element > selecting 'Bring forward' in context menu brings
"id0": {
"deleted": {
"index": "a2",
"version": 4,
},
"inserted": {
"index": "a0",
"version": 3,
},
},
},
@ -1858,6 +1863,7 @@ exports[`contextMenu element > selecting 'Bring to front' in context menu brings
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": -10,
"y": 0,
@ -1912,6 +1918,7 @@ exports[`contextMenu element > selecting 'Bring to front' in context menu brings
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": 20,
"y": 30,
@ -1947,9 +1954,11 @@ exports[`contextMenu element > selecting 'Bring to front' in context menu brings
"id0": {
"deleted": {
"index": "a2",
"version": 4,
},
"inserted": {
"index": "a0",
"version": 3,
},
},
},
@ -2159,6 +2168,7 @@ exports[`contextMenu element > selecting 'Copy styles' in context menu copies st
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": -10,
"y": 0,
@ -2371,6 +2381,7 @@ exports[`contextMenu element > selecting 'Delete' in context menu deletes elemen
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": -10,
"y": 0,
@ -2402,9 +2413,11 @@ exports[`contextMenu element > selecting 'Delete' in context menu deletes elemen
"id0": {
"deleted": {
"isDeleted": true,
"version": 4,
},
"inserted": {
"isDeleted": false,
"version": 3,
},
},
},
@ -2648,6 +2661,7 @@ exports[`contextMenu element > selecting 'Duplicate' in context menu duplicates
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": -10,
"y": 0,
@ -2702,6 +2716,7 @@ exports[`contextMenu element > selecting 'Duplicate' in context menu duplicates
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 5,
"width": 20,
"x": 0,
"y": 10,
@ -2959,6 +2974,7 @@ exports[`contextMenu element > selecting 'Group selection' in context menu group
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": -10,
"y": 0,
@ -3013,6 +3029,7 @@ exports[`contextMenu element > selecting 'Group selection' in context menu group
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": 20,
"y": 30,
@ -3068,9 +3085,11 @@ exports[`contextMenu element > selecting 'Group selection' in context menu group
"groupIds": [
"id9",
],
"version": 4,
},
"inserted": {
"groupIds": [],
"version": 3,
},
},
"id3": {
@ -3078,9 +3097,11 @@ exports[`contextMenu element > selecting 'Group selection' in context menu group
"groupIds": [
"id9",
],
"version": 4,
},
"inserted": {
"groupIds": [],
"version": 3,
},
},
},
@ -3324,6 +3345,7 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": -10,
"y": 0,
@ -3378,6 +3400,7 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": 20,
"y": 30,
@ -3405,9 +3428,11 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
"id3": {
"deleted": {
"strokeColor": "#e03131",
"version": 4,
},
"inserted": {
"strokeColor": "#1e1e1e",
"version": 3,
},
},
},
@ -3428,9 +3453,11 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
"id3": {
"deleted": {
"backgroundColor": "#a5d8ff",
"version": 5,
},
"inserted": {
"backgroundColor": "transparent",
"version": 4,
},
},
},
@ -3451,9 +3478,11 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
"id3": {
"deleted": {
"fillStyle": "cross-hatch",
"version": 6,
},
"inserted": {
"fillStyle": "solid",
"version": 5,
},
},
},
@ -3474,9 +3503,11 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
"id3": {
"deleted": {
"strokeStyle": "dotted",
"version": 7,
},
"inserted": {
"strokeStyle": "solid",
"version": 6,
},
},
},
@ -3497,9 +3528,11 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
"id3": {
"deleted": {
"roughness": 2,
"version": 8,
},
"inserted": {
"roughness": 1,
"version": 7,
},
},
},
@ -3520,9 +3553,11 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
"id3": {
"deleted": {
"opacity": 60,
"version": 9,
},
"inserted": {
"opacity": 100,
"version": 8,
},
},
},
@ -3556,6 +3591,7 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
"roughness": 2,
"strokeColor": "#e03131",
"strokeStyle": "dotted",
"version": 4,
},
"inserted": {
"backgroundColor": "transparent",
@ -3564,6 +3600,7 @@ exports[`contextMenu element > selecting 'Paste styles' in context menu pastes s
"roughness": 1,
"strokeColor": "#1e1e1e",
"strokeStyle": "solid",
"version": 3,
},
},
},
@ -3805,6 +3842,7 @@ exports[`contextMenu element > selecting 'Send backward' in context menu sends e
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": -10,
"y": 0,
@ -3859,6 +3897,7 @@ exports[`contextMenu element > selecting 'Send backward' in context menu sends e
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": 20,
"y": 30,
@ -3886,9 +3925,11 @@ exports[`contextMenu element > selecting 'Send backward' in context menu sends e
"id3": {
"deleted": {
"index": "Zz",
"version": 4,
},
"inserted": {
"index": "a1",
"version": 3,
},
},
},
@ -4130,6 +4171,7 @@ exports[`contextMenu element > selecting 'Send to back' in context menu sends el
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": -10,
"y": 0,
@ -4184,6 +4226,7 @@ exports[`contextMenu element > selecting 'Send to back' in context menu sends el
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": 20,
"y": 30,
@ -4211,9 +4254,11 @@ exports[`contextMenu element > selecting 'Send to back' in context menu sends el
"id3": {
"deleted": {
"index": "Zz",
"version": 4,
},
"inserted": {
"index": "a1",
"version": 3,
},
},
},
@ -4458,6 +4503,7 @@ exports[`contextMenu element > selecting 'Ungroup selection' in context menu ung
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": -10,
"y": 0,
@ -4512,6 +4558,7 @@ exports[`contextMenu element > selecting 'Ungroup selection' in context menu ung
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": 20,
"y": 30,
@ -4567,9 +4614,11 @@ exports[`contextMenu element > selecting 'Ungroup selection' in context menu ung
"groupIds": [
"id9",
],
"version": 4,
},
"inserted": {
"groupIds": [],
"version": 3,
},
},
"id3": {
@ -4577,9 +4626,11 @@ exports[`contextMenu element > selecting 'Ungroup selection' in context menu ung
"groupIds": [
"id9",
],
"version": 4,
},
"inserted": {
"groupIds": [],
"version": 3,
},
},
},
@ -4606,21 +4657,25 @@ exports[`contextMenu element > selecting 'Ungroup selection' in context menu ung
"id0": {
"deleted": {
"groupIds": [],
"version": 5,
},
"inserted": {
"groupIds": [
"id9",
],
"version": 4,
},
},
"id3": {
"deleted": {
"groupIds": [],
"version": 5,
},
"inserted": {
"groupIds": [
"id9",
],
"version": 4,
},
},
},
@ -5739,6 +5794,7 @@ exports[`contextMenu element > shows 'Group selection' in context menu for multi
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 10,
"x": -10,
"y": 0,
@ -5793,6 +5849,7 @@ exports[`contextMenu element > shows 'Group selection' in context menu for multi
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 10,
"x": 12,
"y": 0,
@ -6966,6 +7023,7 @@ exports[`contextMenu element > shows 'Ungroup selection' in context menu for gro
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 10,
"x": -10,
"y": 0,
@ -7020,6 +7078,7 @@ exports[`contextMenu element > shows 'Ungroup selection' in context menu for gro
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 10,
"x": 12,
"y": 0,
@ -7097,9 +7156,11 @@ exports[`contextMenu element > shows 'Ungroup selection' in context menu for gro
"groupIds": [
"id12",
],
"version": 4,
},
"inserted": {
"groupIds": [],
"version": 3,
},
},
"id3": {
@ -7107,9 +7168,11 @@ exports[`contextMenu element > shows 'Ungroup selection' in context menu for gro
"groupIds": [
"id12",
],
"version": 4,
},
"inserted": {
"groupIds": [],
"version": 3,
},
},
},
@ -9926,6 +9989,7 @@ exports[`contextMenu element > shows context menu for element > [end of test] un
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"version": 3,
"width": 20,
"x": -10,
"y": 0,

4744
packages/excalidraw/tests/__snapshots__/history.test.tsx.snap
View File

File diff suppressed because it is too large Load Diff

351
packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap
View File

File diff suppressed because it is too large Load Diff

245
packages/excalidraw/tests/history.test.tsx
View File

@ -25,7 +25,7 @@ import "@excalidraw/utils/test-utils";
import { ElementsDelta, AppStateDelta } from "@excalidraw/element";
import { CaptureUpdateAction, StoreDelta } from "@excalidraw/element";
import { CaptureUpdateAction } from "@excalidraw/element";
import type { LocalPoint, Radians } from "@excalidraw/math";
@ -34,6 +34,7 @@ import type {
ExcalidrawFrameElement,
ExcalidrawGenericElement,
ExcalidrawLinearElement,
ExcalidrawRectangleElement,
ExcalidrawTextElement,
FixedPointBinding,
FractionalIndex,
@ -53,6 +54,8 @@ import { getDefaultAppState } from "../appState";
import { Excalidraw } from "../index";
import * as StaticScene from "../renderer/staticScene";
import { HistoryDelta } from "../history";
import { API } from "./helpers/api";
import { Keyboard, Pointer, UI } from "./helpers/ui";
import {
@ -120,9 +123,19 @@ describe("history", () => {
API.setElements([rect]);
const corrupedEntry = StoreDelta.create(
const corrupedEntry = HistoryDelta.create(
ElementsDelta.empty(),
AppStateDelta.empty(),
// delta can't be empty, otherwise it won't be pushed into the undo stack
AppStateDelta.restore({
delta: {
inserted: {
selectedElementIds: {},
},
deleted: {
selectedElementIds: { [rect.id]: true },
},
},
}),
);
vi.spyOn(corrupedEntry.elements, "applyTo").mockImplementation(() => {
@ -389,7 +402,7 @@ describe("history", () => {
]);
});
it("should iterate through the history when selection changes do not produce visible change", async () => {
it("should not push into redo stack when selection changes dooes not produce a visible change", async () => {
await render(<Excalidraw handleKeyboardGlobally={true} />);
const rect = UI.createElement("rectangle", { x: 10 });
@ -420,18 +433,18 @@ describe("history", () => {
expect(API.getSelectedElements().length).toBe(0);
Keyboard.redo(); // acceptable empty redo
expect(API.getUndoStack().length).toBe(3);
expect(API.getUndoStack().length).toBe(2); // empty change, nothing goes into undo stack
expect(API.getRedoStack().length).toBe(0);
expect(API.getSelectedElements().length).toBe(0);
Keyboard.undo();
expect(API.getUndoStack().length).toBe(2);
expect(API.getUndoStack().length).toBe(1);
expect(API.getRedoStack().length).toBe(1);
assertSelectedElements(rect);
Keyboard.undo();
expect(API.getUndoStack().length).toBe(0); // now we iterated through the same undos!
expect(API.getRedoStack().length).toBe(3);
expect(API.getUndoStack().length).toBe(0);
expect(API.getRedoStack().length).toBe(2);
expect(API.getSelectedElements().length).toBe(0);
expect(h.elements).toEqual([
expect.objectContaining({ id: rect.id, isDeleted: true }),
@ -1313,6 +1326,10 @@ describe("history", () => {
captureUpdate: CaptureUpdateAction.IMMEDIATELY,
});
rect1 = h.elements[0] as ExcalidrawRectangleElement;
text = h.elements[1] as ExcalidrawTextElement;
rect2 = h.elements[2] as ExcalidrawRectangleElement;
// bind text1 to rect1
mouse.select([rect1, text]);
fireEvent.contextMenu(GlobalTestState.interactiveCanvas);
@ -2362,10 +2379,10 @@ describe("history", () => {
}),
]);
// We reached the bottom, again we iterate through invisible changes and reach the top
Keyboard.redo();
assertSelectedElements();
expect(API.getUndoStack().length).toBe(2);
// We reached the bottom, now there is only one non-empty history delta, which will be pushed to the undo stack
expect(API.getUndoStack().length).toBe(1);
expect(API.getRedoStack().length).toBe(0);
expect(h.elements).toEqual([
expect.objectContaining({
@ -2430,7 +2447,31 @@ describe("history", () => {
]);
Keyboard.redo();
expect(API.getUndoStack().length).toBe(5);
expect(API.getUndoStack().length).toBe(2);
expect(API.getRedoStack().length).toBe(3);
// visible change detected, so we don't iterate up again
expect(API.getSelectedElements()).toEqual([]);
expect(h.elements).toEqual([
expect.objectContaining({
id: rect1.id,
isDeleted: false,
}),
expect.objectContaining({
id: rect2.id,
isDeleted: true,
backgroundColor: transparent,
}),
expect.objectContaining({
id: rect3.id,
isDeleted: true,
x: 30,
y: 30,
}),
]);
Keyboard.redo();
// we iterate all the way up, as there are no visible changes
expect(API.getUndoStack().length).toBe(4);
expect(API.getRedoStack().length).toBe(0);
expect(API.getSelectedElements()).toEqual([]);
expect(h.elements).toEqual([
@ -2489,8 +2530,8 @@ describe("history", () => {
]);
Keyboard.redo();
expect(API.getUndoStack().length).toBe(3);
expect(API.getRedoStack().length).toBe(0);
expect(API.getUndoStack().length).toBe(2);
expect(API.getRedoStack().length).toBe(1);
// do not expect any selectedElementIds, as all relate to deleted elements
expect(API.getSelectedElements()).toEqual([]);
expect(h.elements).toEqual([
@ -2506,7 +2547,6 @@ describe("history", () => {
expect.objectContaining({ id: rect1.id, isDeleted: false }),
]);
// Simulate remote update
API.updateScene({
elements: [
h.elements[0],
@ -2523,14 +2563,13 @@ describe("history", () => {
Keyboard.redo();
expect(API.getUndoStack().length).toBe(2);
expect(API.getRedoStack().length).toBe(1);
expect(API.getSelectedElements()).toEqual([
expect.objectContaining({ id: rect2.id, isDeleted: false }),
]);
// redo entry was calculated again with the latest undo, which goes back to nothing being selected
expect(API.getSelectedElements()).toEqual([]);
Keyboard.redo();
expect(API.getUndoStack().length).toBe(3);
expect(API.getRedoStack().length).toBe(0);
// now we again expect these as selected, as they got restored remotely
// now we again expect these as selected, as they got restored remotely and this redo entry was calculated in the beginning
expect(API.getSelectedElements()).toEqual([
expect.objectContaining({ id: rect2.id }),
expect.objectContaining({ id: rect3.id }),
@ -2599,16 +2638,14 @@ describe("history", () => {
Keyboard.undo();
expect(API.getUndoStack().length).toBe(0);
expect(API.getRedoStack().length).toBe(2); // iterated two steps back!
expect(API.getRedoStack().length).toBe(1); // iterated two steps back and reduce two empty entries into one!
expect(h.state.selectedGroupIds).toEqual({});
Keyboard.redo();
expect(API.getUndoStack().length).toBe(2); // iterated two steps forward!
expect(API.getUndoStack().length).toBe(0); // no changes applied, so there is no new entry
expect(API.getRedoStack().length).toBe(0);
expect(h.state.selectedGroupIds).toEqual({});
Keyboard.undo();
// Simulate remote update
API.updateScene({
elements: [
@ -2621,22 +2658,6 @@ describe("history", () => {
],
captureUpdate: CaptureUpdateAction.NEVER,
});
Keyboard.redo();
expect(API.getUndoStack().length).toBe(1);
expect(API.getRedoStack().length).toBe(1);
expect(h.state.selectedGroupIds).toEqual({ A: true });
// Simulate remote update
API.updateScene({
elements: [h.elements[0], h.elements[1], rect3, rect4],
captureUpdate: CaptureUpdateAction.NEVER,
});
Keyboard.redo();
expect(API.getUndoStack().length).toBe(2);
expect(API.getRedoStack().length).toBe(0);
expect(h.state.selectedGroupIds).toEqual({ A: true, B: true });
});
it("should iterate through the history when editing group contains only remotely deleted elements", async () => {
@ -2681,33 +2702,7 @@ describe("history", () => {
Keyboard.undo();
expect(API.getUndoStack().length).toBe(0);
expect(API.getRedoStack().length).toBe(3);
expect(h.state.editingGroupId).toBeNull();
Keyboard.redo();
expect(API.getUndoStack().length).toBe(3);
expect(API.getRedoStack().length).toBe(0);
expect(h.state.editingGroupId).toBeNull();
// Simulate remote update
API.updateScene({
elements: [
newElementWith(h.elements[0], {
isDeleted: false,
}),
h.elements[1],
],
captureUpdate: CaptureUpdateAction.NEVER,
});
Keyboard.undo();
expect(API.getUndoStack().length).toBe(2);
expect(API.getRedoStack().length).toBe(1);
expect(h.state.editingGroupId).toBe("A");
Keyboard.redo();
expect(API.getUndoStack().length).toBe(3);
expect(API.getRedoStack().length).toBe(0);
expect(API.getRedoStack().length).toBe(0); // all changes relate to remotely deleted elements, there is no change and thus nothing to redo
expect(h.state.editingGroupId).toBeNull();
});
@ -2744,13 +2739,13 @@ describe("history", () => {
});
Keyboard.undo();
expect(API.getUndoStack().length).toBe(1);
expect(API.getRedoStack().length).toBe(3);
expect(API.getUndoStack().length).toBe(1); // iterated few entries back
expect(API.getRedoStack().length).toBe(1); // added just one non-empty entry into redo stack
expect(h.state.editingLinearElement).toBeNull();
expect(h.state.selectedLinearElement).toBeNull();
Keyboard.redo();
expect(API.getUndoStack().length).toBe(4);
expect(API.getUndoStack().length).toBe(1);
expect(API.getRedoStack().length).toBe(0);
expect(h.state.editingLinearElement).toBeNull();
expect(h.state.selectedLinearElement).toBeNull();
@ -2838,7 +2833,7 @@ describe("history", () => {
// We iterated two steps as there was no change in order!
expect(API.getUndoStack().length).toBe(0);
expect(API.getRedoStack().length).toBe(2);
expect(API.getSelectedElements().length).toBe(0);
assertSelectedElements([]);
expect(h.elements).toEqual([
expect.objectContaining({ id: rect1.id }), // a "Zx"
expect.objectContaining({ id: rect3.id }), // c "Zy"
@ -2846,7 +2841,7 @@ describe("history", () => {
]);
});
it("should iterate through the history when z-index changes do not produce visible change and we synced all indices", async () => {
it("should tolerate remote z-index changes with incorrect fractional indices", async () => {
const rect1 = API.createElement({ type: "rectangle", x: 10, y: 10 });
const rect2 = API.createElement({ type: "rectangle", x: 20, y: 20 });
const rect3 = API.createElement({ type: "rectangle", x: 30, y: 30 });
@ -2899,22 +2894,32 @@ describe("history", () => {
// Simulate remote update
API.updateScene({
elements: [
h.elements[1], // rect2
h.elements[0], // rect3
h.elements[2], // rect1
h.elements[1], // rect2
],
captureUpdate: CaptureUpdateAction.NEVER,
});
Keyboard.undo();
expect(API.getUndoStack().length).toBe(0);
expect(API.getRedoStack().length).toBe(2); // now we iterated two steps back!
assertSelectedElements([]);
expect(API.getUndoStack().length).toBe(1);
expect(API.getRedoStack().length).toBe(1);
assertSelectedElements([rect2]);
expect(h.elements).toEqual([
expect.objectContaining({ id: rect2.id }),
expect.objectContaining({ id: rect3.id }),
expect.objectContaining({ id: rect1.id }),
]);
Keyboard.redo();
expect(API.getUndoStack().length).toBe(2);
expect(API.getRedoStack().length).toBe(0);
assertSelectedElements([rect2]);
expect(h.elements).toEqual([
expect.objectContaining({ id: rect3.id }),
expect.objectContaining({ id: rect1.id }),
expect.objectContaining({ id: rect2.id }),
]);
});
it("should not let remote changes to interfere with in progress freedraw", async () => {
@ -3724,54 +3729,52 @@ describe("history", () => {
captureUpdate: CaptureUpdateAction.NEVER,
});
runTwice(() => {
Keyboard.redo();
expect(API.getUndoStack().length).toBe(1);
expect(API.getRedoStack().length).toBe(0);
expect(h.elements).toEqual([
expect.objectContaining({
id: container.id,
// previously bound text is preserved
// text bindings are not duplicated
boundElements: [{ id: remoteText.id, type: "text" }],
isDeleted: false,
}),
expect.objectContaining({
id: text.id,
// unbound
containerId: null,
isDeleted: false,
}),
expect.objectContaining({
id: remoteText.id,
// preserved existing binding!
containerId: container.id,
isDeleted: false,
}),
]);
Keyboard.redo();
expect(API.getUndoStack().length).toBe(1);
expect(API.getRedoStack().length).toBe(0);
expect(h.elements).toEqual([
expect.objectContaining({
id: container.id,
// previously bound text is preserved
// text bindings are not duplicated
boundElements: [{ id: remoteText.id, type: "text" }],
isDeleted: false,
}),
expect.objectContaining({
id: text.id,
// unbound
containerId: null,
isDeleted: false,
}),
expect.objectContaining({
id: remoteText.id,
// preserved existing binding!
containerId: container.id,
isDeleted: false,
}),
]);
Keyboard.undo();
expect(API.getUndoStack().length).toBe(0);
expect(API.getRedoStack().length).toBe(1);
expect(h.elements).toEqual([
expect.objectContaining({
id: container.id,
boundElements: [{ id: remoteText.id, type: "text" }],
isDeleted: false, // isDeleted got remotely updated to false
}),
expect.objectContaining({
id: text.id,
containerId: null,
isDeleted: false,
}),
expect.objectContaining({
id: remoteText.id,
// unbound
containerId: container.id,
isDeleted: false,
}),
]);
});
Keyboard.undo();
expect(API.getUndoStack().length).toBe(0);
expect(API.getRedoStack().length).toBe(1);
expect(h.elements).toEqual([
expect.objectContaining({
id: container.id,
boundElements: [{ id: remoteText.id, type: "text" }],
isDeleted: false, // isDeleted got remotely updated to false
}),
expect.objectContaining({
id: text.id,
containerId: null,
isDeleted: false,
}),
expect.objectContaining({
id: remoteText.id,
// unbound
containerId: container.id,
isDeleted: false,
}),
]);
});
it("should preserve latest remotely added binding and unbind previous one when the text is added through history", async () => {

23
packages/excalidraw/tests/test-utils.ts
View File

@ -435,12 +435,17 @@ export const assertElements = <T extends AllPossibleKeys<ExcalidrawElement>>(
expect(h.state.selectedElementIds).toEqual(selectedElementIds);
};
const stripSeed = (deltas: Record<string, { deleted: any; inserted: any }>) =>
const stripProps = (
deltas: Record<string, { deleted: any; inserted: any }>,
props: string[],
) =>
Object.entries(deltas).reduce((acc, curr) => {
const { inserted, deleted, ...rest } = curr[1];
delete inserted.seed;
delete deleted.seed;
for (const prop of props) {
delete inserted[prop];
delete deleted[prop];
}
acc[curr[0]] = {
inserted,
@ -457,9 +462,9 @@ export const checkpointHistory = (history: History, name: string) => {
...x,
elements: {
...x.elements,
added: stripSeed(x.elements.added),
removed: stripSeed(x.elements.removed),
updated: stripSeed(x.elements.updated),
added: stripProps(x.elements.added, ["seed", "versionNonce"]),
removed: stripProps(x.elements.removed, ["seed", "versionNonce"]),
updated: stripProps(x.elements.updated, ["seed", "versionNonce"]),
},
})),
).toMatchSnapshot(`[${name}] undo stack`);
@ -469,9 +474,9 @@ export const checkpointHistory = (history: History, name: string) => {
...x,
elements: {
...x.elements,
added: stripSeed(x.elements.added),
removed: stripSeed(x.elements.removed),
updated: stripSeed(x.elements.updated),
added: stripProps(x.elements.added, ["seed", "versionNonce"]),
removed: stripProps(x.elements.removed, ["seed", "versionNonce"]),
updated: stripProps(x.elements.updated, ["seed", "versionNonce"]),
},
})),
).toMatchSnapshot(`[${name}] redo stack`);