From 2a82821ec5970691199e1ffc6a49ac31f311ab59 Mon Sep 17 00:00:00 2001 From: David Luzar <5153846+dwelle@users.noreply.github.com> Date: Wed, 24 Jun 2026 16:59:10 +0200 Subject: [PATCH 1/3] fix(editor): tweak freedraw settings and tablet UI/UX (#11551) * fix(editor): align constant freedraw stroke width more with generic shapes * reduce streamline for non-mouse * do not change `currentItemStrokeVariability` unintentionally * render penMode button under compact styles panel on tablets * show stroke variability as standalone button in compact actions menu * improve toolbar clicking UX with pen * change streamline defaults * change to `variable` stroke if toggling penMode for the first time --- packages/common/src/constants.ts | 2 +- packages/element/src/shape.ts | 8 +-- packages/excalidraw/actions/actionCanvas.tsx | 3 - .../excalidraw/actions/actionProperties.tsx | 20 +++++++ packages/excalidraw/components/Actions.tsx | 55 ++++++++++++++----- packages/excalidraw/components/App.tsx | 6 +- packages/excalidraw/components/LayerUI.scss | 18 ++++++ packages/excalidraw/components/LayerUI.tsx | 40 ++++++++++---- 8 files changed, 116 insertions(+), 36 deletions(-) diff --git a/packages/common/src/constants.ts b/packages/common/src/constants.ts index d13006f446..d4e4bb6548 100644 --- a/packages/common/src/constants.ts +++ b/packages/common/src/constants.ts @@ -552,4 +552,4 @@ export const MOBILE_ACTION_BUTTON_BG = { } as const; export const DEFAULT_STROKE_STREAMLINE = 0.5; -export const DEFAULT_STROKE_STREAMLINE_PRECISE = 0.3; +export const DEFAULT_STROKE_STREAMLINE_PRECISE = 0.2; diff --git a/packages/element/src/shape.ts b/packages/element/src/shape.ts index a0e5f42c69..337437a1c3 100644 --- a/packages/element/src/shape.ts +++ b/packages/element/src/shape.ts @@ -26,7 +26,6 @@ import { LINE_POLYGON_POINT_MERGE_DISTANCE, applyDarkModeFilter, DEFAULT_STROKE_STREAMLINE, - DEFAULT_STROKE_STREAMLINE_PRECISE, } from "@excalidraw/common"; import { RoughGenerator } from "roughjs/bin/generator"; @@ -1186,20 +1185,15 @@ const VARIABLE_WIDTH_FREEDRAW = { SIZE_FACTOR: 4.25, THINNING: 0.6, SMOOTHING: 0.5, - STREAMLINE: DEFAULT_STROKE_STREAMLINE, } as const; const CONSTANT_WIDTH_FREEDRAW = { /** Stroke size relative to `strokeWidth` for uniform (laser) strokes. */ SIZE_FACTOR: 1.4, - STREAMLINE: DEFAULT_STROKE_STREAMLINE_PRECISE, } as const; const getFreedrawStreamline = (element: ExcalidrawFreeDrawElement) => - element.strokeOptions?.streamline ?? - (element.strokeOptions?.variability === "constant" - ? CONSTANT_WIDTH_FREEDRAW.STREAMLINE - : VARIABLE_WIDTH_FREEDRAW.STREAMLINE); + element.strokeOptions?.streamline ?? DEFAULT_STROKE_STREAMLINE; /** * Pressure-sensitive (variable width) freedraw outline, rendered with diff --git a/packages/excalidraw/actions/actionCanvas.tsx b/packages/excalidraw/actions/actionCanvas.tsx index d1bce4aee6..9c8fea322f 100644 --- a/packages/excalidraw/actions/actionCanvas.tsx +++ b/packages/excalidraw/actions/actionCanvas.tsx @@ -112,9 +112,6 @@ export const actionClearCanvas = register({ theme: appState.theme, penMode: appState.penMode, penDetected: appState.penDetected, - currentItemStrokeVariability: appState.penDetected - ? "variable" - : "constant", exportBackground: appState.exportBackground, exportEmbedScene: appState.exportEmbedScene, gridSize: appState.gridSize, diff --git a/packages/excalidraw/actions/actionProperties.tsx b/packages/excalidraw/actions/actionProperties.tsx index 09e7bb7369..c8341dfdfa 100644 --- a/packages/excalidraw/actions/actionProperties.tsx +++ b/packages/excalidraw/actions/actionProperties.tsx @@ -87,6 +87,7 @@ import type { CaptureUpdateActionType } from "@excalidraw/element"; import { trackEvent } from "../analytics"; import { RadioSelection } from "../components/RadioSelection"; +import { ToolButton } from "../components/ToolButton"; import { ColorPicker } from "../components/ColorPicker/ColorPicker"; import { FontPicker } from "../components/FontPicker/FontPicker"; import { IconPicker } from "../components/IconPicker"; @@ -718,6 +719,25 @@ export const actionChangeFreedrawMode = register({ hasSelection ? null : appState.currentItemStrokeVariability, ) ?? appState.currentItemStrokeVariability; + // in the compact UI the pressure setting is rendered as a single button + // that cycles between the two variability modes on click + if (data?.cycle) { + const isVariable = strokeVariability === "variable"; + return ( + updateData(isVariable ? "constant" : "variable")} + /> + ); + } + return (
{t("labels.pressure")} diff --git a/packages/excalidraw/components/Actions.tsx b/packages/excalidraw/components/Actions.tsx index 8e3f86e6c4..20ae6c8fa9 100644 --- a/packages/excalidraw/components/Actions.tsx +++ b/packages/excalidraw/components/Actions.tsx @@ -395,11 +395,17 @@ const CombinedShapeProperties = ({ hasStrokeWidth(element.type), )) && renderAction("changeStrokeWidth")} - {(hasFreedrawMode(appState.activeTool.type) || - targetElements.some((element) => - hasFreedrawMode(element.type), - )) && - renderAction("changeFreedrawMode")} + { + /* in compact UI the freedraw pressure setting is rendered as a + standalone cycle button in the compact actions list; we render + it in the combined properties popup as well for clarity + */ + (hasFreedrawMode(appState.activeTool.type) || + targetElements.some((element) => + hasFreedrawMode(element.type), + )) && + renderAction("changeFreedrawMode") + } {(hasStrokeStyle(appState.activeTool.type) || targetElements.some((element) => hasStrokeStyle(element.type), @@ -832,6 +838,14 @@ export const CompactShapeActions = ({ )} + {/* Freedraw pressure: standalone button cycling the variability mode */} + {(hasFreedrawMode(appState.activeTool.type) || + targetElements.some((element) => hasFreedrawMode(element.type))) && ( +
+ {renderAction("changeFreedrawMode", { cycle: true })} +
+ )} + { + // Detect the pen here (pointerType is reliable on pointer-down) + // but DON'T enable pen mode yet: calling setState mid-gesture + // re-renders the controlled radio and, on iOS/iPadOS, aborts + // the ensuing click so the tool isn't selected on the first pen + // tap. Defer it until the tap's `change` has committed (below). if (!app.state.penDetected && pointerType === "pen") { - app.togglePenMode(true); + pendingPenDetectionRef.current = true; } if (value === "selection") { @@ -1170,16 +1194,21 @@ export const ShapesSwitcher = ({ } } }} - onChange={({ pointerType }) => { + onChange={() => { if (app.state.activeTool.type !== value) { trackEvent("toolbar", value, "ui"); } - if (value === "image") { - app.setActiveTool({ - type: value, - }); - } else { - app.setActiveTool({ type: value }); + app.setActiveTool({ type: value }); + + // Apply the pen detection captured on pointer-down now that the + // tool is selected. rAF keeps the resulting re-render out of the + // `change` event itself. We rely on the pointer-down detection + // rather than this handler's pointerType because the latter is + // unreliable on iOS (its backing ref is cleared before the + // delayed click fires). + if (pendingPenDetectionRef.current) { + pendingPenDetectionRef.current = false; + requestAnimationFrame(() => app.togglePenMode(true)); } }} /> diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 38b88a5806..d19019ccae 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -4308,7 +4308,9 @@ class App extends React.Component { return { penMode: force ?? !prevState.penMode, penDetected: true, - currentItemStrokeVariability: "variable", + currentItemStrokeVariability: !prevState.penDetected + ? "variable" + : prevState.currentItemStrokeVariability, }; }); }; @@ -9015,7 +9017,7 @@ class App extends React.Component { strokeOptions: { variability: strokeVariability, streamline: - strokeVariability === "constant" && event.pointerType !== "mouse" + event.pointerType !== "mouse" ? DEFAULT_STROKE_STREAMLINE_PRECISE : DEFAULT_STROKE_STREAMLINE, }, diff --git a/packages/excalidraw/components/LayerUI.scss b/packages/excalidraw/components/LayerUI.scss index 5c76a1ee28..ea1ae68391 100644 --- a/packages/excalidraw/components/LayerUI.scss +++ b/packages/excalidraw/components/LayerUI.scss @@ -120,6 +120,24 @@ } } + // on tablet, the pen mode button is rendered as a separate floating button + // below the compact actions menu (see LayerUI.tsx) + .App-menu_top__left > .ToolIcon__penMode { + justify-self: center; + + .ToolIcon__icon { + width: var(--lg-button-size); + height: var(--lg-button-size); + background-color: var(--island-bg-color); + box-shadow: var(--shadow-island); + } + + // no shadow while pen mode is active (the active fill is enough) + .ToolIcon_type_checkbox:checked + .ToolIcon__icon { + box-shadow: none; + } + } + .disable-view-mode { display: flex; justify-content: center; diff --git a/packages/excalidraw/components/LayerUI.tsx b/packages/excalidraw/components/LayerUI.tsx index bbf1f40f9c..0b61910007 100644 --- a/packages/excalidraw/components/LayerUI.tsx +++ b/packages/excalidraw/components/LayerUI.tsx @@ -235,8 +235,6 @@ const LayerUI = ({ ); const renderSelectedShapeActions = () => { - const isCompactMode = isCompactStylesPanel; - return (
- {isCompactMode ? ( + {isCompactStylesPanel ? ( {shouldRenderSelectedShapeActions && renderSelectedShapeActions()} + {/* in compact UI the pen mode button lives outside the toolbar, as + a separate floating button below the compact actions menu + (same as we render it on mobile); shown alongside the compact + actions island, i.e. when a drawing tool or elements are + selected */} + {isCompactStylesPanel && + !appState.viewModeEnabled && + shouldRenderSelectedShapeActions && ( + onPenModeToggle(null)} + title={t("toolBar.penMode")} + isMobile + penDetected={appState.penDetected} + /> + )} {!appState.viewModeEnabled && appState.openDialog?.name !== "elementLinkSelector" && ( @@ -343,13 +358,18 @@ const LayerUI = ({ /> {heading} - onPenModeToggle(null)} - title={t("toolBar.penMode")} - penDetected={appState.penDetected} - /> + {/* in compact UI the pen mode button is rendered + as a separate floating button below the compact + actions menu */} + {!isCompactStylesPanel && ( + onPenModeToggle(null)} + title={t("toolBar.penMode")} + penDetected={appState.penDetected} + /> + )} Date: Thu, 25 Jun 2026 23:36:02 +0530 Subject: [PATCH 2/3] fix(editor): prevent freeze from extremely large line elements (#11556) Co-authored-by: dwelle <5153846+dwelle@users.noreply.github.com> --- packages/excalidraw/data/restore.ts | 69 ++++++++++--------- .../excalidraw/tests/data/restore.test.ts | 47 +++++++++++++ 2 files changed, 83 insertions(+), 33 deletions(-) diff --git a/packages/excalidraw/data/restore.ts b/packages/excalidraw/data/restore.ts index db75c71099..c0b9e0b85f 100644 --- a/packages/excalidraw/data/restore.ts +++ b/packages/excalidraw/data/restore.ts @@ -101,7 +101,38 @@ type RestoredAppState = Omit< "offsetTop" | "offsetLeft" | "width" | "height" >; -const MAX_ARROW_PX = 75_000; +const MAX_LINEAR_PX = 75_000; + +// Last resort fix for extremely large linear elements (lines / arrows), which +// would otherwise freeze the editor while rendering — e.g. a dotted or dashed +// stroke spanning a huge distance generates an enormous dash array. +// https://github.com/excalidraw/excalidraw/issues/11497 +const handleOversizedLinearElements = ( + element: T, +): T => { + if (element.width <= MAX_LINEAR_PX && element.height <= MAX_LINEAR_PX) { + return element; + } + + const label = + element.type === "arrow" + ? `${isElbowArrow(element) ? "elbow" : "simple"} arrow` + : element.type; + + console.error( + `Removing extremely large ${label} ${element.id} (width: ${element.width}, height: ${element.height}, x: ${element.x}, y: ${element.y})`, + ); + + return { + ...element, + x: 0, + y: 0, + width: 100, + height: 100, + points: [pointFrom(0, 0), pointFrom(100, 100)], + isDeleted: true, + }; +}; const restoreLinearElementPoints = ( points: unknown, @@ -560,7 +591,7 @@ export const restoreElement = ( } as ExcalidrawLinearElement)); } - return restoreElementWithProperties(element, { + const restoredLine = restoreElementWithProperties(element, { type: "line", startBinding: null, endBinding: null, @@ -578,6 +609,8 @@ export const restoreElement = ( : {}), ...getSizeFromPoints(points), }); + + return handleOversizedLinearElements(restoredLine); case "arrow": { const startArrowhead = normalizeArrowhead(element.startArrowhead); const endArrowhead = @@ -644,37 +677,7 @@ export const restoreElement = ( ), }; - // Last resort fix for extremely large arrows - if ( - normalizedRestoredElement.width > MAX_ARROW_PX || - normalizedRestoredElement.height > MAX_ARROW_PX - ) { - console.error( - `Removing extremely large arrow ${ - normalizedRestoredElement.id - } (type: ${ - isElbowArrow(normalizedRestoredElement) ? "elbow" : "simple" - }, width: ${normalizedRestoredElement.width}, height: ${ - normalizedRestoredElement.height - }, x: ${normalizedRestoredElement.x}, y: ${ - normalizedRestoredElement.y - })`, - ); - return { - ...normalizedRestoredElement, - x: 0, - y: 0, - width: 100, - height: 100, - points: [ - pointFrom(0, 0), - pointFrom(100, 100), - ], - isDeleted: true, - }; - } - - return normalizedRestoredElement; + return handleOversizedLinearElements(normalizedRestoredElement); } // generic elements diff --git a/packages/excalidraw/tests/data/restore.test.ts b/packages/excalidraw/tests/data/restore.test.ts index df38fc1133..b33f756511 100644 --- a/packages/excalidraw/tests/data/restore.test.ts +++ b/packages/excalidraw/tests/data/restore.test.ts @@ -526,6 +526,53 @@ describe("restoreElements", () => { ]); }); + it("should mark extremely large linear elements as deleted to avoid freezing", () => { + const consoleError = vi + .spyOn(console, "error") + .mockImplementation(() => {}); + + // a degenerate line with astronomical coordinates (see #11497) + const hugeLine: any = API.createElement({ + type: "line", + x: 419048829414166, + y: 8484, + }); + hugeLine.points = [ + [0, 0], + [-302985021938436, 0], + [-838097658820234, 30], + ]; + + const hugeArrow: any = API.createElement({ type: "arrow" }); + hugeArrow.points = [ + [0, 0], + [900000, 0], + ]; + + const normalLine: any = API.createElement({ type: "line" }); + normalLine.points = [ + [0, 0], + [100, 200], + ]; + + const [restoredLine, restoredArrow, restoredNormal] = + restore.restoreElements([hugeLine, hugeArrow, normalLine], null); + + expect(restoredLine.isDeleted).toBe(true); + expect(restoredLine.width).toBe(100); + expect(restoredLine.height).toBe(100); + + expect(restoredArrow.isDeleted).toBe(true); + expect(restoredArrow.width).toBe(100); + expect(restoredArrow.height).toBe(100); + + expect(restoredNormal.isDeleted).toBe(false); + expect(restoredNormal.width).toBe(100); + expect(restoredNormal.height).toBe(200); + + consoleError.mockRestore(); + }); + it("when the number of points of a line is greater or equal 2", () => { const lineElement_0 = API.createElement({ type: "line", From e4c70cb6c6493c816da94ec23188321854296e64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rk=20Tolm=C3=A1cs?= Date: Fri, 26 Jun 2026 09:35:32 +0200 Subject: [PATCH 3/3] feat(editor): AnimationController for scrollToContent (#11553) --------- Signed-off-by: Mark Tolmacs Co-authored-by: dwelle <5153846+dwelle@users.noreply.github.com> --- packages/common/src/utils.ts | 129 ------------ packages/excalidraw/components/App.tsx | 192 +++++------------- packages/excalidraw/renderer/animation.ts | 5 + packages/excalidraw/scroll.ts | 147 ++++++++++++++ .../excalidraw/tests/fitToContent.test.tsx | 157 ++++++++++++-- 5 files changed, 341 insertions(+), 289 deletions(-) create mode 100644 packages/excalidraw/scroll.ts diff --git a/packages/common/src/utils.ts b/packages/common/src/utils.ts index e979323649..0b168fa946 100644 --- a/packages/common/src/utils.ts +++ b/packages/common/src/utils.ts @@ -204,135 +204,6 @@ export const easeOut = (k: number) => { return 1 - Math.pow(1 - k, 4); }; -const easeOutInterpolate = (from: number, to: number, progress: number) => { - return (to - from) * easeOut(progress) + from; -}; - -/** - * Animates values from `fromValues` to `toValues` using the requestAnimationFrame API. - * Executes the `onStep` callback on each step with the interpolated values. - * Returns a function that can be called to cancel the animation. - * - * @example - * // Example usage: - * const fromValues = { x: 0, y: 0 }; - * const toValues = { x: 100, y: 200 }; - * const onStep = ({x, y}) => { - * setState(x, y) - * }; - * const onCancel = () => { - * console.log("Animation canceled"); - * }; - * - * const cancelAnimation = easeToValuesRAF({ - * fromValues, - * toValues, - * onStep, - * onCancel, - * }); - * - * // To cancel the animation: - * cancelAnimation(); - */ -export const easeToValuesRAF = < - T extends Record, - K extends keyof T, ->({ - fromValues, - toValues, - onStep, - duration = 250, - interpolateValue, - onStart, - onEnd, - onCancel, -}: { - fromValues: T; - toValues: T; - /** - * Interpolate a single value. - * Return undefined to be handled by the default interpolator. - */ - interpolateValue?: ( - fromValue: number, - toValue: number, - /** no easing applied */ - progress: number, - key: K, - ) => number | undefined; - onStep: (values: T) => void; - duration?: number; - onStart?: () => void; - onEnd?: () => void; - onCancel?: () => void; -}) => { - let canceled = false; - let frameId = 0; - let startTime: number; - - function step(timestamp: number) { - if (canceled) { - return; - } - if (startTime === undefined) { - startTime = timestamp; - onStart?.(); - } - - const elapsed = Math.min(timestamp - startTime, duration); - const factor = easeOut(elapsed / duration); - - const newValues = {} as T; - - Object.keys(fromValues).forEach((key) => { - const _key = key as keyof T; - const result = ((toValues[_key] - fromValues[_key]) * factor + - fromValues[_key]) as T[keyof T]; - newValues[_key] = result; - }); - - onStep(newValues); - - if (elapsed < duration) { - const progress = elapsed / duration; - - const newValues = {} as T; - - Object.keys(fromValues).forEach((key) => { - const _key = key as K; - const startValue = fromValues[_key]; - const endValue = toValues[_key]; - - let result; - - result = interpolateValue - ? interpolateValue(startValue, endValue, progress, _key) - : easeOutInterpolate(startValue, endValue, progress); - - if (result == null) { - result = easeOutInterpolate(startValue, endValue, progress); - } - - newValues[_key] = result as T[K]; - }); - onStep(newValues); - - frameId = window.requestAnimationFrame(step); - } else { - onStep(toValues); - onEnd?.(); - } - } - - frameId = window.requestAnimationFrame(step); - - return () => { - onCancel?.(); - canceled = true; - window.cancelAnimationFrame(frameId); - }; -}; - // https://github.com/lodash/lodash/blob/es/chunk.js export const chunk = ( array: readonly T[], diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index d19019ccae..d119214dd4 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -77,11 +77,9 @@ import { updateObject, updateActiveTool, isTransparent, - easeToValuesRAF, muteFSAbortError, isTestEnv, isDevEnv, - easeOut, updateStable, addEventListener, normalizeEOL, @@ -203,7 +201,6 @@ import { cropElement, wrapText, isElementLink, - parseElementLinkFromURL, isMeasureTextSupported, normalizeText, measureText, @@ -264,6 +261,7 @@ import { getActiveTextElement, isEligibleFrameChildType, getBindingStrategyForDraggingBindingElementEndpoints, + parseElementLinkFromURL, } from "@excalidraw/element"; import type { GlobalPoint, LocalPoint, Radians } from "@excalidraw/math"; @@ -331,7 +329,7 @@ import { actionToggleCropEditor, } from "../actions"; import { actionWrapTextInContainer } from "../actions/actionBoundText"; -import { actionToggleHandTool, zoomToFit } from "../actions/actionCanvas"; +import { actionToggleHandTool } from "../actions/actionCanvas"; import { actionPaste } from "../actions/actionClipboard"; import { actionCopyElementLink } from "../actions/actionElementLink"; import { actionUnlockAllElements } from "../actions/actionElementLock"; @@ -413,6 +411,11 @@ import { isGridModeEnabled, } from "../snapping"; import { Renderer } from "../scene/Renderer"; +import { + type ScrollToContentOptions, + SCROLL_TO_CONTENT_ANIMATION_KEY, + scrollToElements, +} from "../scroll"; import { setEraserCursor, setCursor, @@ -425,16 +428,12 @@ import { withBatchedUpdates, withBatchedUpdatesThrottled } from "../reactUtils"; import { isPointHittingTextAutoResizeHandle } from "../textAutoResizeHandle"; import { textWysiwyg } from "../wysiwyg/textWysiwyg"; import { isOverScrollBars } from "../scene/scrollbars"; - import { isMaybeMermaidDefinition } from "../mermaid"; - import { LassoTrail } from "../lasso"; - import { EraserTrail } from "../eraser"; - import { getShortcutKey } from "../shortcut"; - import { tryParseSpreadsheet } from "../charts"; +import { AnimationController } from "../renderer/animation"; import ConvertElementTypePopup, { getConversionTypeFromElements, @@ -4341,148 +4340,60 @@ class App extends React.Component { }); }; - private cancelInProgressAnimation: (() => void) | null = null; - scrollToContent = ( - /** - * target to scroll to - * - * - string - id of element or group, or url containing elementLink - * - ExcalidrawElement | ExcalidrawElement[] - element(s) objects - */ - target: + target?: | string | ExcalidrawElement - | readonly ExcalidrawElement[] = this.scene.getNonDeletedElements(), - opts?: ( - | { - fitToContent?: boolean; - fitToViewport?: never; - viewportZoomFactor?: number; - animate?: boolean; - duration?: number; - } - | { - fitToContent?: never; - fitToViewport?: boolean; - /** when fitToViewport=true, how much screen should the content cover, - * between 0.1 (10%) and 1 (100%) - */ - viewportZoomFactor?: number; - animate?: boolean; - duration?: number; - } - ) & { - minZoom?: number; - maxZoom?: number; - canvasOffsets?: Offsets; - }, + | readonly NonDeletedExcalidrawElement[], + opts?: ScrollToContentOptions, ) => { + let elements: readonly NonDeleted[]; if (typeof target === "string") { - let id: string | null; - if (isElementLink(target)) { - id = parseElementLinkFromURL(target); - } else { - id = target; - } - if (id) { - const elements = this.scene.getElementsFromId(id); + const id = isElementLink(target) + ? parseElementLinkFromURL(target) + : target; + elements = id ? this.scene.getElementsFromId(id) : []; + } else if (Array.isArray(target)) { + elements = target; + } else if (target) { + elements = [target as NonDeleted]; + } else { + elements = this.scene.getNonDeletedElements(); + } - if (elements?.length) { - this.scrollToContent(elements, { - fitToContent: opts?.fitToContent ?? true, - animate: opts?.animate ?? true, - }); - } else if (isElementLink(target)) { - this.setState({ - toast: { - message: t("elementLink.notFound"), - duration: 3000, - closable: true, - }, - }); - } + if (!elements.length) { + if (typeof target === "string" && isElementLink(target)) { + this.setState({ + toast: { + message: t("elementLink.notFound"), + duration: 3000, + closable: true, + }, + }); } + return; } - this.cancelInProgressAnimation?.(); - - // convert provided target into ExcalidrawElement[] if necessary - const targetElements = Array.isArray(target) ? target : [target]; - - let zoom = this.state.zoom; - let scrollX = this.state.scrollX; - let scrollY = this.state.scrollY; - - if (opts?.fitToContent || opts?.fitToViewport) { - const { appState } = zoomToFit({ - canvasOffsets: opts.canvasOffsets, - targetElements, - appState: this.state, - fitToViewport: !!opts?.fitToViewport, - viewportZoomFactor: opts?.viewportZoomFactor, - minZoom: opts?.minZoom, - maxZoom: opts?.maxZoom, - }); - zoom = appState.zoom; - scrollX = appState.scrollX; - scrollY = appState.scrollY; - } else { - // compute only the viewport location, without any zoom adjustment - const scroll = calculateScrollCenter(targetElements, this.state); - scrollX = scroll.scrollX; - scrollY = scroll.scrollY; - } - - // when animating, we use RequestAnimationFrame to prevent the animation - // from slowing down other processes - if (opts?.animate) { - const origScrollX = this.state.scrollX; - const origScrollY = this.state.scrollY; - const origZoom = this.state.zoom.value; - - const cancel = easeToValuesRAF({ - fromValues: { - scrollX: origScrollX, - scrollY: origScrollY, - zoom: origZoom, - }, - toValues: { scrollX, scrollY, zoom: zoom.value }, - interpolateValue: (from, to, progress, key) => { - // for zoom, use different easing - if (key === "zoom") { - return from * Math.pow(to / from, easeOut(progress)); + // Navigating to an element by id or element-link defaults to zooming the + // element into view, animated — matching the historical element-link + // behavior — unless the caller opts out. + const resolvedOpts = + typeof target === "string" + ? { + ...opts, + fitToViewport: undefined, + fitToContent: opts?.fitToContent ?? true, + animate: opts?.animate ?? true, } - // handle using default - return undefined; - }, - onStep: ({ scrollX, scrollY, zoom }) => { - this.setState({ - scrollX, - scrollY, - zoom: { value: zoom }, - }); - }, - onStart: () => { - this.setState({ shouldCacheIgnoreZoom: true }); - }, - onEnd: () => { - this.setState({ shouldCacheIgnoreZoom: false }); - }, - onCancel: () => { - this.setState({ shouldCacheIgnoreZoom: false }); - }, - duration: opts?.duration ?? 500, - }); + : opts; - this.cancelInProgressAnimation = () => { - cancel(); - this.cancelInProgressAnimation = null; - }; - } else { - this.setState({ scrollX, scrollY, zoom }); - } + scrollToElements( + this.state, + elements, + this.setState.bind(this), + resolvedOpts, + ); }; private maybeUnfollowRemoteUser = () => { @@ -4495,7 +4406,8 @@ class App extends React.Component { private translateCanvas: React.Component["setState"] = ( state, ) => { - this.cancelInProgressAnimation?.(); + AnimationController.cancel(SCROLL_TO_CONTENT_ANIMATION_KEY); + this.setState({ shouldCacheIgnoreZoom: false }); this.maybeUnfollowRemoteUser(); this.setState(state); }; diff --git a/packages/excalidraw/renderer/animation.ts b/packages/excalidraw/renderer/animation.ts index d629117776..f6c7b04d68 100644 --- a/packages/excalidraw/renderer/animation.ts +++ b/packages/excalidraw/renderer/animation.ts @@ -123,4 +123,9 @@ export class AnimationController { AnimationController.animations.delete(key); AnimationController.cancelScheduledFrameIfIdle(); } + + static reset() { + AnimationController.animations.clear(); + AnimationController.cancelScheduledFrame(); + } } diff --git a/packages/excalidraw/scroll.ts b/packages/excalidraw/scroll.ts new file mode 100644 index 0000000000..d6d6789b03 --- /dev/null +++ b/packages/excalidraw/scroll.ts @@ -0,0 +1,147 @@ +import { easeOut } from "@excalidraw/common"; +import { clamp } from "@excalidraw/math"; + +import type { ExcalidrawElement } from "@excalidraw/element/types"; + +import { zoomToFit } from "./actions/actionCanvas"; +import { AnimationController } from "./renderer/animation"; +import { calculateScrollCenter } from "./scene/scroll"; + +import type { AppState, NormalizedZoomValue, Offsets } from "./types"; + +export const SCROLL_TO_CONTENT_ANIMATION_KEY = "animateScrollToContent"; + +/** default duration of the scroll/zoom animation, in milliseconds */ +const DEFAULT_ANIMATION_DURATION = 500; + +export type ScrollToContentOptions = ( + | { + fitToContent?: boolean; + fitToViewport?: never; + viewportZoomFactor?: number; + animate?: boolean; + duration?: number; + } + | { + fitToContent?: never; + fitToViewport?: boolean; + /** when fitToViewport=true, how much screen should the content cover, + * between 0.1 (10%) and 1 (100%) */ + viewportZoomFactor?: number; + animate?: boolean; + duration?: number; + } +) & { + minZoom?: number; + maxZoom?: number; + canvasOffsets?: Offsets; +}; + +type Viewport = Pick; + +/** + * Scrolls (and optionally zooms) the viewport so that the given target is in + * view, optionally animating the transition. + */ +export const scrollToElements = ( + state: AppState, + target: readonly ExcalidrawElement[], + onFrame: ( + state: Pick< + AppState, + "scrollX" | "scrollY" | "zoom" | "shouldCacheIgnoreZoom" + >, + ) => void, + opts?: ScrollToContentOptions, +) => { + AnimationController.cancel(SCROLL_TO_CONTENT_ANIMATION_KEY); + + const viewport = getTargetViewport(state, target, opts); + + if (opts?.animate) { + animateToViewport( + state, + viewport, + opts.duration ?? DEFAULT_ANIMATION_DURATION, + onFrame, + ); + } else { + // no animation: jump straight to the target. Re-enable zoom caching in + // case we just cancelled an in-flight animation that had suppressed it. + onFrame({ ...viewport, shouldCacheIgnoreZoom: false }); + } +}; + +/** Computes the viewport (scroll + zoom) that brings the target elements into + * view, based on the requested fit behavior. */ +const getTargetViewport = ( + state: AppState, + targetElements: readonly ExcalidrawElement[], + opts?: ScrollToContentOptions, +): Viewport => { + if (opts?.fitToContent || opts?.fitToViewport) { + const { appState } = zoomToFit({ + canvasOffsets: opts.canvasOffsets, + targetElements, + appState: state, + fitToViewport: !!opts.fitToViewport, + viewportZoomFactor: opts.viewportZoomFactor, + minZoom: opts.minZoom, + maxZoom: opts.maxZoom, + }); + + return { + scrollX: appState.scrollX, + scrollY: appState.scrollY, + zoom: appState.zoom, + }; + } + + // keep the current zoom, only recenter the viewport on the target + const { scrollX, scrollY } = calculateScrollCenter(targetElements, state); + + return { scrollX, scrollY, zoom: state.zoom }; +}; + +/** Eases the viewport from its current position to `target` over `duration`, + * driving the transition through the shared AnimationController so it doesn't + * slow down other processes. */ +const animateToViewport = ( + from: Pick, + target: Viewport, + duration: number, + onFrame: ( + state: Pick< + AppState, + "scrollX" | "scrollY" | "zoom" | "shouldCacheIgnoreZoom" + >, + ) => void, +) => { + AnimationController.start<{ elapsed: number }>( + SCROLL_TO_CONTENT_ANIMATION_KEY, + ({ deltaTime, state }) => { + const elapsed = (state?.elapsed ?? 0) + deltaTime; + const progress = Math.min(elapsed / duration, 1); + const factor = easeOut(clamp(progress, 0, 1)); + + onFrame({ + shouldCacheIgnoreZoom: progress < 1, // ignore zoom caching while animating + scrollX: from.scrollX + (target.scrollX - from.scrollX) * factor, + scrollY: from.scrollY + (target.scrollY - from.scrollY) * factor, + // zoom interpolates geometrically so the transition feels natural + zoom: { + value: (from.zoom.value * + Math.pow( + target.zoom.value / from.zoom.value, + factor, + )) as NormalizedZoomValue, + }, + }); + + // returning a falsy value signals the AnimationController to remove the + // animation; otherwise it would keep ticking (and calling onFrame) every + // frame forever after reaching the target + return progress < 1 ? { elapsed } : null; + }, + ); +}; diff --git a/packages/excalidraw/tests/fitToContent.test.tsx b/packages/excalidraw/tests/fitToContent.test.tsx index bfd16c4e14..f6f1e69bb1 100644 --- a/packages/excalidraw/tests/fitToContent.test.tsx +++ b/packages/excalidraw/tests/fitToContent.test.tsx @@ -1,20 +1,62 @@ import React from "react"; -import { vi } from "vitest"; import { Excalidraw } from "../index"; +import { AnimationController } from "../renderer/animation"; +import { SCROLL_TO_CONTENT_ANIMATION_KEY } from "../scroll"; import { API } from "./helpers/api"; import { act, render } from "./test-utils"; const { h } = window; -const waitForNextAnimationFrame = () => { +/** + * The scroll/zoom animation is driven by `AnimationController`. With render + * throttling enabled (see the `beforeEach` below) it schedules frames via + * `requestAnimationFrame`, advancing the easing based on elapsed wall-clock + * time. We use a very long animation `duration` (see `LONG_ANIMATION_DURATION`) + * so it can never complete while we sample it, and let a few frames pass + * between samples so the easing makes observable (but partial) progress. + */ +const LONG_ANIMATION_DURATION = 1_000_000; + +const waitForAnimationProgress = (frames = 4) => { return act( () => - new Promise((resolve) => { - requestAnimationFrame(() => { - requestAnimationFrame(resolve); - }); + new Promise((resolve) => { + let remaining = frames; + const step = () => { + if (--remaining <= 0) { + resolve(); + } else { + requestAnimationFrame(step); + } + }; + requestAnimationFrame(step); + }), + ); +}; + +/** + * Polls until the scroll/zoom animation has removed itself from the + * `AnimationController` (i.e. it ran to completion), or until `maxFrames` + * elapses as a safety net so a regression can't hang the suite. + */ +const waitForAnimationToStop = (maxFrames = 200) => { + return act( + () => + new Promise((resolve) => { + let remaining = maxFrames; + const check = () => { + if ( + !AnimationController.running(SCROLL_TO_CONTENT_ANIMATION_KEY) || + --remaining <= 0 + ) { + resolve(); + } else { + requestAnimationFrame(check); + } + }; + requestAnimationFrame(check); }), ); }; @@ -77,6 +119,34 @@ describe("fitToContent", () => { expect(h.state.zoom.value).toBeLessThanOrEqual(0.1); }); + it("should default to fitToContent when scrolling to an element by id", async () => { + await render(); + + h.state.width = 10; + h.state.height = 10; + + const rectElement = API.createElement({ + width: 50, + height: 100, + x: 50, + y: 100, + }); + + API.setElements([rectElement]); + + expect(h.state.zoom.value).toBe(1); + + act(() => { + // navigating by element id (a string target) should zoom-to-fit by + // default, even though no `fitToContent` option was passed + h.app.scrollToContent(rectElement.id, { animate: false }); + }); + + // element is 10x taller than the viewport, so fit-to-content should + // drop the zoom to <= 1/10 + expect(h.state.zoom.value).toBeLessThanOrEqual(0.1); + }); + it("should scroll the viewport to the selected element", async () => { await render(); @@ -109,11 +179,16 @@ describe("fitToContent", () => { describe("fitToContent animated", () => { beforeEach(() => { - vi.spyOn(window, "requestAnimationFrame"); + // pace the animation via requestAnimationFrame instead of a tight + // setTimeout(0) loop, which would otherwise starve the test's own timers + window.EXCALIDRAW_THROTTLE_RENDER = true; }); afterEach(() => { - vi.restoreAllMocks(); + window.EXCALIDRAW_THROTTLE_RENDER = undefined; + // stop any in-flight scroll/zoom animation so it doesn't keep ticking on + // the unmounted component and leak into the next test via the singleton + AnimationController.reset(); }); it("should ease scroll the viewport to the selected element", async () => { @@ -130,17 +205,18 @@ describe("fitToContent animated", () => { }); act(() => { - h.app.scrollToContent(rectElement, { animate: true }); + h.app.scrollToContent(rectElement, { + animate: true, + duration: LONG_ANIMATION_DURATION, + }); }); - expect(window.requestAnimationFrame).toHaveBeenCalled(); - - // Since this is an animation, we expect values to change through time. - // We'll verify that the scroll values change at 50ms and 100ms + // the animation hasn't progressed yet, so we're still at the origin expect(h.state.scrollX).toBe(0); expect(h.state.scrollY).toBe(0); - await waitForNextAnimationFrame(); + // Since this is an animation, we expect values to change through time. + await waitForAnimationProgress(); const prevScrollX = h.state.scrollX; const prevScrollY = h.state.scrollY; @@ -148,7 +224,7 @@ describe("fitToContent animated", () => { expect(h.state.scrollX).not.toBe(0); expect(h.state.scrollY).not.toBe(0); - await waitForNextAnimationFrame(); + await waitForAnimationProgress(); expect(h.state.scrollX).not.toBe(prevScrollX); expect(h.state.scrollY).not.toBe(prevScrollY); @@ -171,12 +247,14 @@ describe("fitToContent animated", () => { expect(h.state.scrollY).toBe(0); act(() => { - h.app.scrollToContent(rectElement, { animate: true, fitToContent: true }); + h.app.scrollToContent(rectElement, { + animate: true, + fitToContent: true, + duration: LONG_ANIMATION_DURATION, + }); }); - expect(window.requestAnimationFrame).toHaveBeenCalled(); - - await waitForNextAnimationFrame(); + await waitForAnimationProgress(); const prevScrollX = h.state.scrollX; const prevScrollY = h.state.scrollY; @@ -184,9 +262,48 @@ describe("fitToContent animated", () => { expect(h.state.scrollX).not.toBe(0); expect(h.state.scrollY).not.toBe(0); - await waitForNextAnimationFrame(); + await waitForAnimationProgress(); expect(h.state.scrollX).not.toBe(prevScrollX); expect(h.state.scrollY).not.toBe(prevScrollY); }); + + it("should stop ticking and settle on the target once complete", async () => { + await render(); + + h.state.width = 10; + h.state.height = 10; + + const rectElement = API.createElement({ + width: 100, + height: 100, + x: -100, + y: -100, + }); + + act(() => { + // a short duration so the animation completes within a few frames + h.app.scrollToContent(rectElement, { animate: true, duration: 10 }); + }); + + await waitForAnimationToStop(); + + // the animation must remove itself from the controller rather than keep + // ticking forever after reaching the target + expect(AnimationController.running(SCROLL_TO_CONTENT_ANIMATION_KEY)).toBe( + false, + ); + + // it should have settled on the target viewport (moved off the origin) + const settledScrollX = h.state.scrollX; + const settledScrollY = h.state.scrollY; + expect(settledScrollX).not.toBe(0); + expect(settledScrollY).not.toBe(0); + expect(h.state.shouldCacheIgnoreZoom).toBe(false); + + // further frames must not move the viewport (no perpetual re-rendering) + await waitForAnimationProgress(); + expect(h.state.scrollX).toBe(settledScrollX); + expect(h.state.scrollY).toBe(settledScrollY); + }); });