diff --git a/packages/excalidraw/change.ts b/packages/excalidraw/change.ts index dc2964b23..1cbf39e89 100644 --- a/packages/excalidraw/change.ts +++ b/packages/excalidraw/change.ts @@ -17,13 +17,16 @@ import { hasBoundTextElement, isBindableElement, isBoundToContainer, + isImageElement, isTextElement, } from "./element/typeChecks"; import type { ExcalidrawElement, + ExcalidrawImageElement, ExcalidrawLinearElement, ExcalidrawTextElement, NonDeleted, + Ordered, OrderedExcalidrawElement, SceneElementsMap, } from "./element/types"; @@ -626,6 +629,18 @@ export class AppStateChange implements Change { ); break; + case "croppingElementId": { + const croppingElementId = nextAppState[key]; + const element = + croppingElementId && nextElements.get(croppingElementId); + + if (element && !element.isDeleted) { + visibleDifferenceFlag.value = true; + } else { + nextAppState[key] = null; + } + break; + } case "editingGroupId": const editingGroupId = nextAppState[key]; @@ -756,6 +771,7 @@ export class AppStateChange implements Change { selectedElementIds, editingLinearElementId, selectedLinearElementId, + croppingElementId, ...standaloneProps } = delta as ObservedAppState; @@ -779,7 +795,10 @@ export class AppStateChange implements Change { } } -type ElementPartial = Omit, "seed">; +type ElementPartial = Omit< + ElementUpdate>, + "seed" +>; /** * Elements change is a low level primitive to capture a change between two sets of elements. @@ -1216,6 +1235,18 @@ export class ElementsChange implements Change { }); } + if (isImageElement(element)) { + const _delta = delta as Delta>; + // we want to override `crop` only if modified so that we don't reset + // when undoing/redoing unrelated change + if (_delta.deleted.crop || _delta.inserted.crop) { + Object.assign(directlyApplicablePartial, { + // apply change verbatim + crop: _delta.inserted.crop ?? null, + }); + } + } + if (!flags.containsVisibleDifference) { // strip away fractional as even if it would be different, it doesn't have to result in visible change const { index, ...rest } = directlyApplicablePartial; diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 2a8e1b274..479a24464 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -5173,15 +5173,19 @@ class App extends React.Component { }; private startImageCropping = (image: ExcalidrawImageElement) => { + this.store.shouldCaptureIncrement(); this.setState({ croppingElementId: image.id, }); }; private finishImageCropping = () => { - this.setState({ - croppingElementId: null, - }); + if (this.state.croppingElementId) { + this.store.shouldCaptureIncrement(); + this.setState({ + croppingElementId: null, + }); + } }; private handleCanvasDoubleClick = ( diff --git a/packages/excalidraw/element/cropElement.ts b/packages/excalidraw/element/cropElement.ts index dd387086f..895188b2c 100644 --- a/packages/excalidraw/element/cropElement.ts +++ b/packages/excalidraw/element/cropElement.ts @@ -11,6 +11,7 @@ import { vectorScale, pointFromVector, clamp, + isCloseTo, } from "../../math"; import type { TransformHandleType } from "./transformHandles"; import type { @@ -64,12 +65,14 @@ export const cropElement = ( let nextWidth = element.width; let nextHeight = element.height; - const crop = element.crop ?? { + + let crop: ImageCrop | null = element.crop ?? { x: 0, y: 0, width: naturalWidth, height: naturalHeight, - naturalDimension: [naturalWidth, naturalHeight], + naturalWidth, + naturalHeight, }; const previousCropHeight = crop.height; @@ -138,6 +141,14 @@ export const cropElement = ( nextHeight, ); + // reset crop to null if we're back to orig size + if ( + isCloseTo(crop.width, crop.naturalWidth) && + isCloseTo(crop.height, crop.naturalHeight) + ) { + crop = null; + } + return { x: newOrigin[0], y: newOrigin[1], @@ -242,12 +253,12 @@ export const getUncroppedImageElement = ( topLeftVector, vectorScale( topEdgeNormalized, - (-cropX * width) / element.crop.naturalDimension[0], + (-cropX * width) / element.crop.naturalWidth, ), ), vectorScale( leftEdgeNormalized, - (-cropY * height) / element.crop.naturalDimension[1], + (-cropY * height) / element.crop.naturalHeight, ), ); @@ -282,9 +293,9 @@ export const getUncroppedImageElement = ( export const getUncroppedWidthAndHeight = (element: ExcalidrawImageElement) => { if (element.crop) { const width = - element.width / (element.crop.width / element.crop.naturalDimension[0]); + element.width / (element.crop.width / element.crop.naturalWidth); const height = - element.height / (element.crop.height / element.crop.naturalDimension[1]); + element.height / (element.crop.height / element.crop.naturalHeight); return { width, @@ -309,11 +320,11 @@ const adjustCropPosition = ( const flipY = scale[1] === -1; if (flipX) { - cropX = crop.naturalDimension[0] - Math.abs(cropX) - crop.width; + cropX = crop.naturalWidth - Math.abs(cropX) - crop.width; } if (flipY) { - cropY = crop.naturalDimension[1] - Math.abs(cropY) - crop.height; + cropY = crop.naturalHeight - Math.abs(cropY) - crop.height; } return { diff --git a/packages/excalidraw/element/types.ts b/packages/excalidraw/element/types.ts index 74cee8a30..c2cce533a 100644 --- a/packages/excalidraw/element/types.ts +++ b/packages/excalidraw/element/types.ts @@ -137,7 +137,8 @@ export type ImageCrop = { y: number; width: number; height: number; - naturalDimension: [number, number]; + naturalWidth: number; + naturalHeight: number; }; export type ExcalidrawImageElement = _ExcalidrawElementBase & diff --git a/packages/excalidraw/renderer/staticSvgScene.ts b/packages/excalidraw/renderer/staticSvgScene.ts index 7926e81bc..f4781c0ce 100644 --- a/packages/excalidraw/renderer/staticSvgScene.ts +++ b/packages/excalidraw/renderer/staticSvgScene.ts @@ -428,11 +428,9 @@ const renderElementToSvg = ( symbol.setAttribute( "viewBox", `${ - element.crop.x / - (element.crop.naturalDimension[0] / uncroppedWidth) + element.crop.x / (element.crop.naturalWidth / uncroppedWidth) } ${ - element.crop.y / - (element.crop.naturalDimension[1] / uncroppedHeight) + element.crop.y / (element.crop.naturalHeight / uncroppedHeight) } ${width} ${height}`, ); image.setAttribute("width", `${uncroppedWidth}`); diff --git a/packages/excalidraw/store.ts b/packages/excalidraw/store.ts index 8e934ccba..b3e2713c4 100644 --- a/packages/excalidraw/store.ts +++ b/packages/excalidraw/store.ts @@ -21,6 +21,7 @@ export const getObservedAppState = (appState: AppState): ObservedAppState => { selectedGroupIds: appState.selectedGroupIds, editingLinearElementId: appState.editingLinearElement?.elementId || null, selectedLinearElementId: appState.selectedLinearElement?.elementId || null, + croppingElementId: appState.croppingElementId, }; Reflect.defineProperty(observedAppState, hiddenObservedAppStateProp, { diff --git a/packages/excalidraw/tests/cropElement.test.tsx b/packages/excalidraw/tests/cropElement.test.tsx index 702913a8c..ed8fdcbbe 100644 --- a/packages/excalidraw/tests/cropElement.test.tsx +++ b/packages/excalidraw/tests/cropElement.test.tsx @@ -3,7 +3,7 @@ import ReactDOM from "react-dom"; import { vi } from "vitest"; import { Keyboard, Pointer, UI } from "./helpers/ui"; import type { ExcalidrawImageElement, ImageCrop } from "../element/types"; -import { GlobalTestState, render } from "./test-utils"; +import { act, GlobalTestState, render } from "./test-utils"; import { Excalidraw, exportToCanvas, exportToSvg } from ".."; import { API } from "./helpers/api"; import type { NormalizedZoomValue } from "../types"; @@ -60,15 +60,7 @@ const compareCrops = (cropA: ImageCrop, cropB: ImageCrop) => { const propA = cropA[key]; const propB = cropB[key]; - if (key === "naturalDimension") { - const [naturalWidthA, naturalHeightA] = propA as [number, number]; - const [naturalWidthB, naturalHeightB] = propB as [number, number]; - - expect(naturalWidthA).toBeCloseTo(naturalWidthB); - expect(naturalHeightA).toBeCloseTo(naturalHeightB); - } else { - expect(propA as number).toBeCloseTo(propB as number); - } + expect(propA as number).toBeCloseTo(propB as number); }); }; @@ -158,7 +150,9 @@ describe("Cropping and other features", async () => { ]); Keyboard.keyDown(KEYS.ESCAPE); const duplicatedImage = duplicateElement(null, new Map(), image, {}); - h.app.scene.insertElement(duplicatedImage); + act(() => { + h.app.scene.insertElement(duplicatedImage); + }); expect(duplicatedImage.width).toBe(image.width); expect(duplicatedImage.height).toBe(image.height); diff --git a/packages/excalidraw/tests/helpers/ui.ts b/packages/excalidraw/tests/helpers/ui.ts index 8f27dbd7d..172c4ccc4 100644 --- a/packages/excalidraw/tests/helpers/ui.ts +++ b/packages/excalidraw/tests/helpers/ui.ts @@ -590,7 +590,7 @@ export class UI { clientY + mouseMove[1], ); - mutateElement(element, mutations); + API.updateElement(element, mutations); } static rotate( diff --git a/packages/excalidraw/types.ts b/packages/excalidraw/types.ts index 5a1044e91..b843dfacb 100644 --- a/packages/excalidraw/types.ts +++ b/packages/excalidraw/types.ts @@ -224,6 +224,7 @@ export type ObservedElementsAppState = { editingLinearElementId: LinearElementEditor["elementId"] | null; // Right now it's coupled to `editingLinearElement`, ideally it should not be really needed as we already have selectedElementIds & editingLinearElementId selectedLinearElementId: LinearElementEditor["elementId"] | null; + croppingElementId: AppState["croppingElementId"]; }; export interface AppState { diff --git a/packages/math/utils.ts b/packages/math/utils.ts index bbdf61d8d..8807c275e 100644 --- a/packages/math/utils.ts +++ b/packages/math/utils.ts @@ -28,3 +28,6 @@ export const average = (a: number, b: number) => (a + b) / 2; export const isFiniteNumber = (value: any): value is number => { return typeof value === "number" && Number.isFinite(value); }; + +export const isCloseTo = (a: number, b: number, precision = PRECISION) => + Math.abs(a - b) < precision;