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 bef6eb376b..d37c8ffb7b 100644 --- a/packages/excalidraw/actions/actionCanvas.tsx +++ b/packages/excalidraw/actions/actionCanvas.tsx @@ -113,9 +113,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 128a5f2934..8e6fb53735 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -4315,7 +4315,9 @@ class App extends React.Component { return { penMode: force ?? !prevState.penMode, penDetected: true, - currentItemStrokeVariability: "variable", + currentItemStrokeVariability: !prevState.penDetected + ? "variable" + : prevState.currentItemStrokeVariability, }; }); }; @@ -4369,20 +4371,39 @@ class App extends React.Component { if (!elements.length) { if (typeof target === "string" && isElementLink(target)) { + this.setState({ + toast: { + message: t("elementLink.notFound"), + duration: 3000, + closable: true, + }, + }); } - this.setState({ - toast: { - message: t("elementLink.notFound"), - duration: 3000, - closable: true, - }, - }); return; } this.setScrollConstraints(null); - scrollToElements(this.state, elements, this.setState.bind(this), opts); + + // 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, + } + : opts; + + scrollToElements( + this.state, + elements, + this.setState.bind(this), + resolvedOpts, + ); }; private maybeUnfollowRemoteUser = () => { @@ -8965,7 +8986,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} + /> + )} ; -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/scroll.ts b/packages/excalidraw/scroll.ts index 30c49ae2b4..7f30a3e831 100644 --- a/packages/excalidraw/scroll.ts +++ b/packages/excalidraw/scroll.ts @@ -176,7 +176,12 @@ export const animateToConstraints = ( export const scrollToElements = ( state: AppState, target: readonly ExcalidrawElement[], - onFrame: (state: Pick) => void, + onFrame: ( + state: Pick< + AppState, + "scrollX" | "scrollY" | "zoom" | "shouldCacheIgnoreZoom" + >, + ) => void, opts?: ScrollToContentOptions, ) => { AnimationController.cancel(SCROLL_TO_CONTENT_ANIMATION_KEY); @@ -191,7 +196,9 @@ export const scrollToElements = ( onFrame, ); } else { - onFrame(viewport); + // 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 }); } }; @@ -265,7 +272,10 @@ const animateToViewport = ( }, }); - return { elapsed }; + // 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/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", diff --git a/packages/excalidraw/tests/fitToContent.test.tsx b/packages/excalidraw/tests/fitToContent.test.tsx index c80f46029d..f6f1e69bb1 100644 --- a/packages/excalidraw/tests/fitToContent.test.tsx +++ b/packages/excalidraw/tests/fitToContent.test.tsx @@ -2,6 +2,7 @@ import React from "react"; 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"; @@ -35,6 +36,31 @@ const waitForAnimationProgress = (frames = 4) => { ); }; +/** + * 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); + }), + ); +}; + describe("fitToContent", () => { it("should zoom to fit the selected element", async () => { await render(); @@ -93,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(); @@ -213,4 +267,43 @@ describe("fitToContent animated", () => { 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); + }); });