Passing in elementsMap in updatePoints, refactor to ElementsMap types

This commit is contained in:
Marcel Mraz 2025-04-15 14:02:50 +02:00
parent 94c773a990
commit 567c9f51e4
No known key found for this signature in database
GPG key ID: 4EBD6E62DC830CD2
11 changed files with 86 additions and 68 deletions

View file

@ -177,6 +177,8 @@ const bindOrUnbindLinearElementEdge = (
unboundFromElementIds: Set<ExcalidrawBindableElement["id"]>,
scene: Scene,
): void => {
const elementsMap = scene.getNonDeletedElementsMap();
// "keep" is for method chaining convenience, a "no-op", so just bail out
if (bindableElement === "keep") {
return;
@ -211,7 +213,7 @@ const bindOrUnbindLinearElementEdge = (
linearElement,
bindableElement,
startOrEnd,
scene.getNonDeletedElementsMap(),
elementsMap,
(...args) => scene.mutate(...args),
);
boundToElementIds.add(bindableElement.id);
@ -221,7 +223,7 @@ const bindOrUnbindLinearElementEdge = (
linearElement,
bindableElement,
startOrEnd,
scene.getNonDeletedElementsMap(),
elementsMap,
(...args) => scene.mutate(...args),
);
boundToElementIds.add(bindableElement.id);
@ -434,12 +436,14 @@ export const maybeBindLinearElement = (
pointerCoords: { x: number; y: number },
scene: Scene,
): void => {
const elementsMap = scene.getNonDeletedElementsMap();
if (appState.startBoundElement != null) {
bindLinearElement(
linearElement,
appState.startBoundElement,
"start",
scene.getNonDeletedElementsMap(),
elementsMap,
(...args) => scene.mutate(...args),
);
}
@ -447,7 +451,7 @@ export const maybeBindLinearElement = (
const hoveredElement = getHoveredElementForBinding(
pointerCoords,
scene.getNonDeletedElements(),
scene.getNonDeletedElementsMap(),
elementsMap,
appState.zoom,
isElbowArrow(linearElement),
isElbowArrow(linearElement),
@ -465,7 +469,7 @@ export const maybeBindLinearElement = (
linearElement,
hoveredElement,
"end",
scene.getNonDeletedElementsMap(),
elementsMap,
(...args) => scene.mutate(...args),
);
}
@ -496,7 +500,7 @@ export const bindLinearElement = (
linearElement: NonDeleted<ExcalidrawLinearElement>,
hoveredElement: ExcalidrawBindableElement,
startOrEnd: "start" | "end",
elementsMap: Map<string, ExcalidrawElement>,
elementsMap: ElementsMap,
mutator: (
element: ExcalidrawElement,
updates: ElementUpdate<ExcalidrawElement>,
@ -753,7 +757,7 @@ const calculateFocusAndGap = (
// in explicitly.
export const updateBoundElements = (
changedElement: NonDeletedExcalidrawElement,
elementsMap: Map<string, ExcalidrawElement>,
elementsMap: ElementsMap,
options?: {
simultaneouslyUpdated?: readonly ExcalidrawElement[];
newSize?: { width: number; height: number };
@ -856,19 +860,14 @@ export const updateBoundElements = (
}> => update !== null,
);
LinearElementEditor.movePoints(
element,
updates,
{
...(changedElement.id === element.startBinding?.elementId
? { startBinding: bindings.startBinding }
: {}),
...(changedElement.id === element.endBinding?.elementId
? { endBinding: bindings.endBinding }
: {}),
},
elementsMap as NonDeletedSceneElementsMap,
);
LinearElementEditor.movePoints(element, elementsMap, updates, {
...(changedElement.id === element.startBinding?.elementId
? { startBinding: bindings.startBinding }
: {}),
...(changedElement.id === element.endBinding?.elementId
? { endBinding: bindings.endBinding }
: {}),
});
const boundText = getBoundTextElement(element, elementsMap);
if (boundText && !boundText.isDeleted) {
@ -1501,7 +1500,7 @@ export const fixDuplicatedBindingsAfterDuplication = (
const fixReversedBindingsForBindables = (
original: ExcalidrawBindableElement,
duplicate: ExcalidrawBindableElement,
originalElements: Map<string, ExcalidrawElement>,
originalElements: ElementsMap,
elementsWithClones: ExcalidrawElement[],
oldIdToDuplicatedId: Map<ExcalidrawElement["id"], ExcalidrawElement["id"]>,
) => {
@ -1589,7 +1588,7 @@ const fixReversedBindingsForBindables = (
const fixReversedBindingsForArrows = (
original: ExcalidrawArrowElement,
duplicate: ExcalidrawArrowElement,
originalElements: Map<string, ExcalidrawElement>,
originalElements: ElementsMap,
bindingProp: "startBinding" | "endBinding",
oldIdToDuplicatedId: Map<ExcalidrawElement["id"], ExcalidrawElement["id"]>,
elementsWithClones: ExcalidrawElement[],
@ -1650,7 +1649,7 @@ const fixReversedBindingsForArrows = (
};
export const fixReversedBindings = (
originalElements: Map<string, ExcalidrawElement>,
originalElements: ElementsMap,
elementsWithClones: ExcalidrawElement[],
oldIdToDuplicatedId: Map<ExcalidrawElement["id"], ExcalidrawElement["id"]>,
) => {

View file

@ -238,11 +238,11 @@ const getOffsets = (
const addNewNode = (
element: ExcalidrawFlowchartNodeElement,
elementsMap: ElementsMap,
appState: AppState,
direction: LinkDirection,
scene: Scene,
) => {
const elementsMap = scene.getNonDeletedElementsMap();
const successors = getSuccessors(element, elementsMap, direction);
const predeccessors = getPredecessors(element, elementsMap, direction);
@ -277,7 +277,6 @@ const addNewNode = (
const bindingArrow = createBindingArrow(
element,
nextNode,
elementsMap,
direction,
appState,
scene,
@ -291,7 +290,6 @@ const addNewNode = (
export const addNewNodes = (
startNode: ExcalidrawFlowchartNodeElement,
elementsMap: ElementsMap,
appState: AppState,
direction: LinkDirection,
scene: Scene,
@ -357,7 +355,6 @@ export const addNewNodes = (
const bindingArrow = createBindingArrow(
startNode,
nextNode,
elementsMap,
direction,
appState,
scene,
@ -373,7 +370,6 @@ export const addNewNodes = (
const createBindingArrow = (
startBindingElement: ExcalidrawFlowchartNodeElement,
endBindingElement: ExcalidrawFlowchartNodeElement,
elementsMap: ElementsMap,
direction: LinkDirection,
appState: AppState,
scene: Scene,
@ -447,18 +443,20 @@ const createBindingArrow = (
elbowed: true,
});
const elementsMap = scene.getNonDeletedElementsMap();
bindLinearElement(
bindingArrow,
startBindingElement,
"start",
scene.getNonDeletedElementsMap(),
elementsMap,
(...args) => scene.mutate(...args),
);
bindLinearElement(
bindingArrow,
endBindingElement,
"end",
scene.getNonDeletedElementsMap(),
elementsMap,
(...args) => scene.mutate(...args),
);
@ -476,7 +474,7 @@ const createBindingArrow = (
bindingArrow as OrderedExcalidrawElement,
);
LinearElementEditor.movePoints(bindingArrow, [
LinearElementEditor.movePoints(bindingArrow, elementsMap, [
{
index: 1,
point: bindingArrow.points[1],
@ -641,15 +639,14 @@ export class FlowChartCreator {
createNodes(
startNode: ExcalidrawFlowchartNodeElement,
elementsMap: ElementsMap,
appState: AppState,
direction: LinkDirection,
scene: Scene,
) {
const elementsMap = scene.getNonDeletedElementsMap();
if (direction !== this.direction) {
const { nextNode, bindingArrow } = addNewNode(
startNode,
elementsMap,
appState,
direction,
scene,
@ -663,7 +660,6 @@ export class FlowChartCreator {
this.numberOfNodes += 1;
const newNodes = addNewNodes(
startNode,
elementsMap,
appState,
direction,
scene,

View file

@ -7,6 +7,7 @@ import { getBoundTextElement } from "./textElement";
import { hasBoundTextElement } from "./typeChecks";
import type {
ElementsMap,
ExcalidrawElement,
FractionalIndex,
OrderedExcalidrawElement,
@ -152,7 +153,7 @@ export const orderByFractionalIndex = (
*/
export const syncMovedIndices = (
elements: readonly ExcalidrawElement[],
movedElements: Map<string, ExcalidrawElement>,
movedElements: ElementsMap,
): OrderedExcalidrawElement[] => {
try {
const indicesGroups = getMovedIndicesGroups(elements, movedElements);
@ -210,7 +211,7 @@ export const syncInvalidIndices = (
*/
const getMovedIndicesGroups = (
elements: readonly ExcalidrawElement[],
movedElements: Map<string, ExcalidrawElement>,
movedElements: ElementsMap,
) => {
const indicesGroups: number[][] = [];

View file

@ -307,7 +307,7 @@ export class LinearElementEditor {
event[KEYS.CTRL_OR_CMD] ? null : app.getEffectiveGridSize(),
);
LinearElementEditor.movePoints(element, [
LinearElementEditor.movePoints(element, elementsMap, [
{
index: selectedIndex,
point: pointFrom(
@ -331,6 +331,7 @@ export class LinearElementEditor {
LinearElementEditor.movePoints(
element,
elementsMap,
selectedPointsIndices.map((pointIndex) => {
const newPointPosition: LocalPoint =
pointIndex === lastClickedPoint
@ -451,7 +452,7 @@ export class LinearElementEditor {
selectedPoint === element.points.length - 1
) {
if (isPathALoop(element.points, appState.zoom.value)) {
LinearElementEditor.movePoints(element, [
LinearElementEditor.movePoints(element, elementsMap, [
{
index: selectedPoint,
point:
@ -948,7 +949,9 @@ export class LinearElementEditor {
if (!event.altKey) {
if (lastPoint === lastUncommittedPoint) {
LinearElementEditor.deletePoints(element, [points.length - 1]);
LinearElementEditor.deletePoints(element, elementsMap, [
points.length - 1,
]);
}
return {
...appState.editingLinearElement,
@ -986,14 +989,16 @@ export class LinearElementEditor {
}
if (lastPoint === lastUncommittedPoint) {
LinearElementEditor.movePoints(element, [
LinearElementEditor.movePoints(element, elementsMap, [
{
index: element.points.length - 1,
point: newPoint,
},
]);
} else {
LinearElementEditor.addPoints(element, [{ point: newPoint }]);
LinearElementEditor.addPoints(element, elementsMap, [
{ point: newPoint },
]);
}
return {
...appState.editingLinearElement,
@ -1226,7 +1231,7 @@ export class LinearElementEditor {
// potentially expanding the bounding box
if (pointAddedToEnd) {
const lastPoint = element.points[element.points.length - 1];
LinearElementEditor.movePoints(element, [
LinearElementEditor.movePoints(element, elementsMap, [
{
index: element.points.length - 1,
point: pointFrom(lastPoint[0] + 30, lastPoint[1] + 30),
@ -1245,6 +1250,7 @@ export class LinearElementEditor {
static deletePoints(
element: NonDeleted<ExcalidrawLinearElement>,
elementsMap: ElementsMap,
pointIndices: readonly number[],
) {
let offsetX = 0;
@ -1275,28 +1281,41 @@ export class LinearElementEditor {
return acc;
}, []);
LinearElementEditor._updatePoints(element, nextPoints, offsetX, offsetY);
LinearElementEditor._updatePoints(
element,
elementsMap,
nextPoints,
offsetX,
offsetY,
);
}
static addPoints(
element: NonDeleted<ExcalidrawLinearElement>,
elementsMap: ElementsMap,
targetPoints: { point: LocalPoint }[],
) {
const offsetX = 0;
const offsetY = 0;
const nextPoints = [...element.points, ...targetPoints.map((x) => x.point)];
LinearElementEditor._updatePoints(element, nextPoints, offsetX, offsetY);
LinearElementEditor._updatePoints(
element,
elementsMap,
nextPoints,
offsetX,
offsetY,
);
}
static movePoints(
element: NonDeleted<ExcalidrawLinearElement>,
elementsMap: ElementsMap,
targetPoints: { index: number; point: LocalPoint; isDragging?: boolean }[],
otherUpdates?: {
startBinding?: PointBinding | null;
endBinding?: PointBinding | null;
},
sceneElementsMap?: NonDeletedSceneElementsMap,
) {
const { points } = element;
@ -1330,6 +1349,7 @@ export class LinearElementEditor {
LinearElementEditor._updatePoints(
element,
elementsMap,
nextPoints,
offsetX,
offsetY,
@ -1340,7 +1360,6 @@ export class LinearElementEditor {
dragging || targetPoint.isDragging === true,
false,
),
sceneElementsMap,
},
);
}
@ -1443,6 +1462,7 @@ export class LinearElementEditor {
private static _updatePoints(
element: NonDeleted<ExcalidrawLinearElement>,
elementsMap: ElementsMap,
nextPoints: readonly LocalPoint[],
offsetX: number,
offsetY: number,
@ -1479,8 +1499,7 @@ export class LinearElementEditor {
updates.points = Array.from(nextPoints);
// TODO_SCENE: fix
mutateElementWith(element, options?.sceneElementsMap!, updates, {
mutateElementWith(element, elementsMap, updates, {
isDragging: options?.isDragging,
});
} else {

View file

@ -14,6 +14,7 @@ import { deepCopyElement } from "@excalidraw/element/duplicate";
import { API } from "@excalidraw/excalidraw/tests/helpers/api";
import type {
ElementsMap,
ExcalidrawElement,
FractionalIndex,
} from "@excalidraw/element/types";
@ -749,7 +750,7 @@ function testInvalidIndicesSync(args: {
function prepareArguments(
elementsLike: { id: string; index?: string }[],
movedElementsIds?: string[],
): [ExcalidrawElement[], Map<string, ExcalidrawElement> | undefined] {
): [ExcalidrawElement[], ElementsMap | undefined] {
const elements = elementsLike.map((x) =>
API.createElement({ id: x.id, index: x.index as FractionalIndex }),
);
@ -764,7 +765,7 @@ function prepareArguments(
function test(
name: string,
elements: ExcalidrawElement[],
movedElements: Map<string, ExcalidrawElement> | undefined,
movedElements: ElementsMap | undefined,
expectUnchangedElements: Map<string, { id: string }>,
expectValidInput?: boolean,
) {

View file

@ -257,7 +257,11 @@ export const actionDeleteSelected = register({
: endBindingElement,
};
LinearElementEditor.deletePoints(element, selectedPointsIndices);
LinearElementEditor.deletePoints(
element,
elementsMap,
selectedPointsIndices,
);
return {
elements,

View file

@ -58,6 +58,7 @@ import type { LocalPoint } from "@excalidraw/math";
import type {
Arrowhead,
ElementsMap,
ExcalidrawBindableElement,
ExcalidrawElement,
ExcalidrawLinearElement,
@ -774,7 +775,7 @@ type ChangeFontFamilyData = Partial<
>
> & {
/** cache of selected & editing elements populated on opened popup */
cachedElements?: Map<string, ExcalidrawElement>;
cachedElements?: ElementsMap;
/** flag to reset all elements to their cached versions */
resetAll?: true;
/** flag to reset all containers to their cached versions */
@ -983,7 +984,7 @@ export const actionChangeFontFamily = register({
return result;
},
PanelComponent: ({ elements, appState, app, updateData }) => {
const cachedElementsRef = useRef<Map<string, ExcalidrawElement>>(new Map());
const cachedElementsRef = useRef<ElementsMap>(new Map());
const prevSelectedFontFamilyRef = useRef<number | null>(null);
// relying on state batching as multiple `FontPicker` handlers could be called in rapid succession and we want to combine them
const [batchedData, setBatchedData] = useState<ChangeFontFamilyData>({});
@ -992,7 +993,7 @@ export const actionChangeFontFamily = register({
const selectedFontFamily = useMemo(() => {
const getFontFamily = (
elementsArray: readonly ExcalidrawElement[],
elementsMap: Map<string, ExcalidrawElement>,
elementsMap: ElementsMap,
) =>
getFormValue(
elementsArray,
@ -1668,7 +1669,7 @@ export const actionChangeArrowType = register({
newElement,
startHoveredElement,
"start",
app.scene.getNonDeletedElementsMap(),
elementsMap,
(...args) => app.scene.mutate(...args),
);
endHoveredElement &&
@ -1676,7 +1677,7 @@ export const actionChangeArrowType = register({
newElement,
endHoveredElement,
"end",
app.scene.getNonDeletedElementsMap(),
elementsMap,
(...args) => app.scene.mutate(...args),
);
@ -1736,7 +1737,7 @@ export const actionChangeArrowType = register({
newElement,
startElement,
"start",
app.scene.getNonDeletedElementsMap(),
elementsMap,
(...args) => app.scene.mutate(...args),
);
}
@ -1750,7 +1751,7 @@ export const actionChangeArrowType = register({
newElement,
endElement,
"end",
app.scene.getNonDeletedElementsMap(),
elementsMap,
(...args) => app.scene.mutate(...args),
);
}

View file

@ -4186,7 +4186,6 @@ class App extends React.Component<AppProps, AppState> {
) {
this.flowChartCreator.createNodes(
selectedElements[0],
this.scene.getNonDeletedElementsMap(),
this.state,
getLinkDirectionFromKey(event.key),
this.scene,

View file

@ -216,13 +216,12 @@ const StatsDragInput = <
y: number;
} | null = null;
let originalElementsMap: Map<string, ExcalidrawElement> | null =
app.scene
.getNonDeletedElements()
.reduce((acc: ElementsMap, element) => {
acc.set(element.id, deepCopyElement(element));
return acc;
}, new Map());
let originalElementsMap: ElementsMap | null = app.scene
.getNonDeletedElements()
.reduce((acc: ElementsMap, element) => {
acc.set(element.id, deepCopyElement(element));
return acc;
}, new Map());
let originalElements: readonly E[] | null = elements.map(
(element) => originalElementsMap!.get(element.id) as E,

View file

@ -418,7 +418,6 @@ class Scene {
};
// TODO_SCENE: should be accessed as app.scene through the API
// TODO_SCENE: in theory actions (and most of the App handlers) should not needs this as they are ending with "replaceAllElements" anyway
// TODO_SCENE: inform mutation false is the new default, meaning all mutateElement with nothing should likely use scene instead
// TODO_SCENE: think one more time about moving the scene inside element (probably we will end up with it either way)
// Mutate an element with passed updates and trigger the component to update. Make sure you

View file

@ -1384,7 +1384,7 @@ describe("Test Linear Elements", () => {
const [origStartX, origStartY] = [line.x, line.y];
act(() => {
LinearElementEditor.movePoints(line, [
LinearElementEditor.movePoints(line, arrayToMap(h.elements), [
{
index: 0,
point: pointFrom(line.points[0][0] + 10, line.points[0][1] + 10),