fix: do not strip invisible elements from array (#9844)

This commit is contained in:
David Luzar
2025-08-12 11:56:11 +02:00
committed by GitHub
parent 54c148f390
commit dda3affcb0
9 changed files with 104 additions and 46 deletions

View File

@@ -260,7 +260,7 @@ export const loadFromFirebase = async (
const storedScene = docSnap.data() as FirebaseStoredScene;
const elements = getSyncableElements(
restoreElements(await decryptElements(storedScene, roomKey), null, {
deleteEmptyTextElements: true,
deleteInvisibleElements: true,
}),
);

View File

@@ -261,13 +261,13 @@ export const loadScene = async (
{
repairBindings: true,
refreshDimensions: false,
deleteEmptyTextElements: true,
deleteInvisibleElements: true,
},
);
} else {
data = restore(localDataState || null, null, null, {
repairBindings: true,
deleteEmptyTextElements: true,
deleteInvisibleElements: true,
});
}

View File

@@ -5,7 +5,11 @@ import {
bindOrUnbindLinearElement,
isBindingEnabled,
} from "@excalidraw/element/binding";
import { isValidPolygon, LinearElementEditor } from "@excalidraw/element";
import {
isValidPolygon,
LinearElementEditor,
newElementWith,
} from "@excalidraw/element";
import {
isBindingElement,
@@ -78,7 +82,14 @@ export const actionFinalize = register({
let newElements = elements;
if (element && isInvisiblySmallElement(element)) {
// TODO: #7348 in theory this gets recorded by the store, so the invisible elements could be restored by the undo/redo, which might be not what we would want
newElements = newElements.filter((el) => el.id !== element!.id);
newElements = newElements.map((el) => {
if (el.id === element.id) {
return newElementWith(el, {
isDeleted: true,
});
}
return el;
});
}
return {
elements: newElements,
@@ -117,7 +128,12 @@ export const actionFinalize = register({
return {
elements:
element.points.length < 2 || isInvisiblySmallElement(element)
? elements.filter((el) => el.id !== element.id)
? elements.map((el) => {
if (el.id === element.id) {
return newElementWith(el, { isDeleted: true });
}
return el;
})
: undefined,
appState: {
...appState,
@@ -172,7 +188,12 @@ export const actionFinalize = register({
if (element && isInvisiblySmallElement(element)) {
// TODO: #7348 in theory this gets recorded by the store, so the invisible elements could be restored by the undo/redo, which might be not what we would want
newElements = newElements.filter((el) => el.id !== element!.id);
newElements = newElements.map((el) => {
if (el.id === element?.id) {
return newElementWith(el, { isDeleted: true });
}
return el;
});
}
if (isLinearElement(element) || isFreeDrawElement(element)) {

View File

@@ -2344,7 +2344,7 @@ class App extends React.Component<AppProps, AppState> {
}
const scene = restore(initialData, null, null, {
repairBindings: true,
deleteEmptyTextElements: true,
deleteInvisibleElements: true,
});
scene.appState = {
...scene.appState,
@@ -3204,7 +3204,7 @@ class App extends React.Component<AppProps, AppState> {
fitToContent?: boolean;
}) => {
const elements = restoreElements(opts.elements, null, {
deleteEmptyTextElements: true,
deleteInvisibleElements: true,
});
const [minX, minY, maxX, maxY] = getCommonBounds(elements);

View File

@@ -173,7 +173,7 @@ export const loadSceneOrLibraryFromBlob = async (
{
repairBindings: true,
refreshDimensions: false,
deleteEmptyTextElements: true,
deleteInvisibleElements: true,
},
),
};

View File

@@ -243,7 +243,7 @@ const restoreElementWithProperties = <
export const restoreElement = (
element: Exclude<ExcalidrawElement, ExcalidrawSelectionElement>,
opts?: { deleteEmptyTextElements?: boolean },
opts?: { deleteInvisibleElements?: boolean },
): typeof element | null => {
element = { ...element };
@@ -291,7 +291,7 @@ export const restoreElement = (
// if empty text, mark as deleted. We keep in array
// for data integrity purposes (collab etc.)
if (opts?.deleteEmptyTextElements && !text && !element.isDeleted) {
if (opts?.deleteInvisibleElements && !text && !element.isDeleted) {
// TODO: we should not do this since it breaks sync / versioning when we exchange / apply just deltas and restore the elements (deletion isn't recorded)
element = { ...element, originalText: text, isDeleted: true };
element = bumpVersion(element);
@@ -529,7 +529,7 @@ export const restoreElements = (
| {
refreshDimensions?: boolean;
repairBindings?: boolean;
deleteEmptyTextElements?: boolean;
deleteInvisibleElements?: boolean;
}
| undefined,
): OrderedExcalidrawElement[] => {
@@ -540,29 +540,38 @@ export const restoreElements = (
(elements || []).reduce((elements, element) => {
// filtering out selection, which is legacy, no longer kept in elements,
// and causing issues if retained
if (element.type !== "selection" && !isInvisiblySmallElement(element)) {
let migratedElement: ExcalidrawElement | null = restoreElement(
element,
{
deleteEmptyTextElements: opts?.deleteEmptyTextElements,
},
);
if (migratedElement) {
const localElement = localElementsMap?.get(element.id);
if (localElement && localElement.version > migratedElement.version) {
migratedElement = bumpVersion(
migratedElement,
localElement.version,
);
}
if (existingIds.has(migratedElement.id)) {
migratedElement = { ...migratedElement, id: randomId() };
}
existingIds.add(migratedElement.id);
elements.push(migratedElement);
}
if (element.type === "selection") {
return elements;
}
let migratedElement: ExcalidrawElement | null = restoreElement(element, {
deleteInvisibleElements: opts?.deleteInvisibleElements,
});
if (migratedElement) {
const localElement = localElementsMap?.get(element.id);
const shouldMarkAsDeleted =
opts?.deleteInvisibleElements && isInvisiblySmallElement(element);
if (
shouldMarkAsDeleted ||
(localElement && localElement.version > migratedElement.version)
) {
migratedElement = bumpVersion(migratedElement, localElement?.version);
}
if (shouldMarkAsDeleted) {
migratedElement = { ...migratedElement, isDeleted: true };
}
if (existingIds.has(migratedElement.id)) {
migratedElement = { ...migratedElement, id: randomId() };
}
existingIds.add(migratedElement.id);
elements.push(migratedElement);
}
return elements;
}, [] as ExcalidrawElement[]),
);
@@ -806,7 +815,7 @@ export const restore = (
elementsConfig?: {
refreshDimensions?: boolean;
repairBindings?: boolean;
deleteEmptyTextElements?: boolean;
deleteInvisibleElements?: boolean;
},
): RestoredDataState => {
return {

View File

@@ -60,7 +60,11 @@ describe("restoreElements", () => {
const rectElement = API.createElement({ type: "rectangle" });
mockSizeHelper.mockImplementation(() => true);
expect(restore.restoreElements([rectElement], null).length).toBe(0);
expect(
restore.restoreElements([rectElement], null, {
deleteInvisibleElements: true,
}),
).toEqual([expect.objectContaining({ isDeleted: true })]);
});
it("should restore text element correctly passing value for each attribute", () => {
@@ -85,7 +89,7 @@ describe("restoreElements", () => {
});
});
it("should not delete empty text element when deleteEmptyTextElements is not defined", () => {
it("should not delete empty text element when opts.deleteInvisibleElements is not defined", () => {
const textElement = API.createElement({
type: "text",
text: "",
@@ -115,7 +119,7 @@ describe("restoreElements", () => {
expect(textElement.isDeleted).toBe(false);
const restoredText = restore.restoreElements([textElement], null, {
deleteEmptyTextElements: true,
deleteInvisibleElements: true,
})[0] as ExcalidrawTextElement;
expect(restoredText.isDeleted).toBe(true);
expect(restoredText).toMatchSnapshot({
@@ -193,13 +197,16 @@ describe("restoreElements", () => {
y: 0,
});
const restoredElements = restore.restoreElements([arrowElement], null);
const restoredElements = restore.restoreElements([arrowElement], null, {
deleteInvisibleElements: true,
});
const restoredArrow = restoredElements[0] as
| ExcalidrawArrowElement
| undefined;
expect(restoredArrow).toBeUndefined();
expect(restoredArrow).not.toBeUndefined();
expect(restoredArrow?.isDeleted).toBe(true);
});
it("should keep 'imperceptibly' small freedraw/line elements", () => {
@@ -864,6 +871,7 @@ describe("repairing bindings", () => {
let restoredElements = restore.restoreElements(
[container, invisibleBoundElement, boundElement],
null,
{ deleteInvisibleElements: true },
);
expect(restoredElements).toEqual([
@@ -871,6 +879,11 @@ describe("repairing bindings", () => {
id: container.id,
boundElements: [obsoleteBinding, invisibleBinding, nonExistentBinding],
}),
expect.objectContaining({
id: invisibleBoundElement.id,
containerId: container.id,
isDeleted: true,
}),
expect.objectContaining({
id: boundElement.id,
containerId: container.id,
@@ -880,7 +893,7 @@ describe("repairing bindings", () => {
restoredElements = restore.restoreElements(
[container, invisibleBoundElement, boundElement],
null,
{ repairBindings: true },
{ repairBindings: true, deleteInvisibleElements: true },
);
expect(restoredElements).toEqual([
@@ -888,6 +901,11 @@ describe("repairing bindings", () => {
id: container.id,
boundElements: [],
}),
expect.objectContaining({
id: invisibleBoundElement.id,
containerId: container.id,
isDeleted: true,
}),
expect.objectContaining({
id: boundElement.id,
containerId: container.id,

View File

@@ -315,7 +315,12 @@ describe("Test dragCreate", () => {
);
expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`5`);
expect(h.state.selectionElement).toBeNull();
expect(h.elements.length).toEqual(0);
expect(h.elements).toEqual([
expect.objectContaining({
type: "arrow",
isDeleted: true,
}),
]);
});
it("line", async () => {
@@ -344,7 +349,12 @@ describe("Test dragCreate", () => {
);
expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`5`);
expect(h.state.selectionElement).toBeNull();
expect(h.elements.length).toEqual(0);
expect(h.elements).toEqual([
expect.objectContaining({
type: "line",
isDeleted: true,
}),
]);
});
});
});

View File

@@ -49,7 +49,7 @@ export const exportToCanvas = ({
{ elements, appState },
null,
null,
{ deleteEmptyTextElements: true },
{ deleteInvisibleElements: true },
);
const { exportBackground, viewBackgroundColor } = restoredAppState;
return _exportToCanvas(
@@ -180,7 +180,7 @@ export const exportToSvg = async ({
{ elements, appState },
null,
null,
{ deleteEmptyTextElements: true },
{ deleteInvisibleElements: true },
);
const exportAppState = {