fix: improve canvas search scroll behavior further (#8491)

packages/excalidraw/actions/actionCanvas.tsx

@@ -24,7 +24,7 @@ import { CODES, KEYS } from "../keys";
 import { getNormalizedZoom } from "../scene";
 import { centerScrollOn } from "../scene/scroll";
 import { getStateForZoom } from "../scene/zoom";
-import type { AppState } from "../types";
+import type { AppState, Offsets } from "../types";
 import { getShortcutKey, updateActiveTool } from "../utils";
 import { register } from "./register";
 import { Tooltip } from "../components/Tooltip";
@@ -38,7 +38,7 @@ import { DEFAULT_CANVAS_BACKGROUND_PICKS } from "../colors";
 import type { SceneBounds } from "../element/bounds";
 import { setCursor } from "../cursor";
 import { StoreAction } from "../store";
-import { clamp } from "../../math";
+import { clamp, roundToStep } from "../../math";
 
 export const actionChangeViewBackgroundColor = register({
   name: "changeViewBackgroundColor",
@@ -259,89 +259,85 @@ const zoomValueToFitBoundsOnViewport = (
   const adjustedZoomValue =
     smallestZoomValue * clamp(viewportZoomFactor, 0.1, 1);
 
-  const zoomAdjustedToSteps =
-    Math.floor(adjustedZoomValue / ZOOM_STEP) * ZOOM_STEP;
-
-  return getNormalizedZoom(Math.min(zoomAdjustedToSteps, 1));
+  return Math.min(adjustedZoomValue, 1);
 };
 
 export const zoomToFitBounds = ({
   bounds,
   appState,
+  canvasOffsets,
   fitToViewport = false,
   viewportZoomFactor = 1,
+  minZoom = -Infinity,
+  maxZoom = Infinity,
 }: {
   bounds: SceneBounds;
+  canvasOffsets?: Offsets;
   appState: Readonly<AppState>;
   /** whether to fit content to viewport (beyond >100%) */
   fitToViewport: boolean;
   /** zoom content to cover X of the viewport, when fitToViewport=true */
   viewportZoomFactor?: number;
+  minZoom?: number;
+  maxZoom?: number;
 }) => {
+  viewportZoomFactor = clamp(viewportZoomFactor, MIN_ZOOM, MAX_ZOOM);
+
   const [x1, y1, x2, y2] = bounds;
   const centerX = (x1 + x2) / 2;
   const centerY = (y1 + y2) / 2;
 
-  let newZoomValue;
-  let scrollX;
-  let scrollY;
+  const canvasOffsetLeft = canvasOffsets?.left ?? 0;
+  const canvasOffsetTop = canvasOffsets?.top ?? 0;
+  const canvasOffsetRight = canvasOffsets?.right ?? 0;
+  const canvasOffsetBottom = canvasOffsets?.bottom ?? 0;
+
+  const effectiveCanvasWidth =
+    appState.width - canvasOffsetLeft - canvasOffsetRight;
+  const effectiveCanvasHeight =
+    appState.height - canvasOffsetTop - canvasOffsetBottom;
+
+  let adjustedZoomValue;
 
   if (fitToViewport) {
     const commonBoundsWidth = x2 - x1;
     const commonBoundsHeight = y2 - y1;
 
-    newZoomValue =
+    adjustedZoomValue =
       Math.min(
-        appState.width / commonBoundsWidth,
-        appState.height / commonBoundsHeight,
-      ) * clamp(viewportZoomFactor, 0.1, 1);
-
-    newZoomValue = getNormalizedZoom(newZoomValue);
-
-    let appStateWidth = appState.width;
-
-    if (appState.openSidebar) {
-      const sidebarDOMElem = document.querySelector(
-        ".sidebar",
-      ) as HTMLElement | null;
-      const sidebarWidth = sidebarDOMElem?.offsetWidth ?? 0;
-      const isRTL = document.documentElement.getAttribute("dir") === "rtl";
-
-      appStateWidth = !isRTL
-        ? appState.width - sidebarWidth
-        : appState.width + sidebarWidth;
-    }
-
-    scrollX = (appStateWidth / 2) * (1 / newZoomValue) - centerX;
-    scrollY = (appState.height / 2) * (1 / newZoomValue) - centerY;
+        effectiveCanvasWidth / commonBoundsWidth,
+        effectiveCanvasHeight / commonBoundsHeight,
+      ) * viewportZoomFactor;
   } else {
-    newZoomValue = zoomValueToFitBoundsOnViewport(
+    adjustedZoomValue = zoomValueToFitBoundsOnViewport(
       bounds,
       {
-        width: appState.width,
-        height: appState.height,
+        width: effectiveCanvasWidth,
+        height: effectiveCanvasHeight,
       },
       viewportZoomFactor,
     );
+  }
 
-    const centerScroll = centerScrollOn({
-      scenePoint: { x: centerX, y: centerY },
-      viewportDimensions: {
-        width: appState.width,
-        height: appState.height,
-      },
-      zoom: { value: newZoomValue },
-    });
+  const newZoomValue = getNormalizedZoom(
+    clamp(roundToStep(adjustedZoomValue, ZOOM_STEP, "floor"), minZoom, maxZoom),
+  );
 
-    scrollX = centerScroll.scrollX;
-    scrollY = centerScroll.scrollY;
-  }
+  const centerScroll = centerScrollOn({
+    scenePoint: { x: centerX, y: centerY },
+    viewportDimensions: {
+      width: appState.width,
+      height: appState.height,
+    },
+    offsets: canvasOffsets,
+    zoom: { value: newZoomValue },
+  });
 
   return {
     appState: {
       ...appState,
-      scrollX,
-      scrollY,
+      scrollX: centerScroll.scrollX,
+      scrollY: centerScroll.scrollY,
       zoom: { value: newZoomValue },
     },
     storeAction: StoreAction.NONE,
@@ -349,25 +345,34 @@ export const zoomToFitBounds = ({
 };
 
 export const zoomToFit = ({
+  canvasOffsets,
   targetElements,
   appState,
   fitToViewport,
   viewportZoomFactor,
+  minZoom,
+  maxZoom,
 }: {
+  canvasOffsets?: Offsets;
   targetElements: readonly ExcalidrawElement[];
   appState: Readonly<AppState>;
   /** whether to fit content to viewport (beyond >100%) */
   fitToViewport: boolean;
   /** zoom content to cover X of the viewport, when fitToViewport=true */
   viewportZoomFactor?: number;
+  minZoom?: number;
+  maxZoom?: number;
 }) => {
   const commonBounds = getCommonBounds(getNonDeletedElements(targetElements));
 
   return zoomToFitBounds({
+    canvasOffsets,
     bounds: commonBounds,
     appState,
     fitToViewport,
     viewportZoomFactor,
+    minZoom,
+    maxZoom,
   });
 };
 
@@ -388,6 +393,7 @@ export const actionZoomToFitSelectionInViewport = register({
         userToFollow: null,
       },
       fitToViewport: false,
+      canvasOffsets: app.getEditorUIOffsets(),
     });
   },
   // NOTE shift-2 should have been assigned actionZoomToFitSelection.
@@ -413,7 +419,7 @@ export const actionZoomToFitSelection = register({
         userToFollow: null,
       },
       fitToViewport: true,
-      viewportZoomFactor: 0.7,
+      canvasOffsets: app.getEditorUIOffsets(),
     });
   },
   // NOTE this action should use shift-2 per figma, alas
@@ -430,7 +436,7 @@ export const actionZoomToFit = register({
   icon: zoomAreaIcon,
   viewMode: true,
   trackEvent: { category: "canvas" },
-  perform: (elements, appState) =>
+  perform: (elements, appState, _, app) =>
     zoomToFit({
       targetElements: elements,
       appState: {
@@ -438,6 +444,7 @@ export const actionZoomToFit = register({
         userToFollow: null,
       },
       fitToViewport: false,
+      canvasOffsets: app.getEditorUIOffsets(),
     }),
   keyTest: (event) =>
     event.code === CODES.ONE &&

packages/excalidraw/components/App.tsx

@@ -259,6 +259,7 @@ import type {
   ElementsPendingErasure,
   GenerateDiagramToCode,
   NullableGridSize,
+  Offsets,
 } from "../types";
 import {
   debounce,
@@ -3232,6 +3233,7 @@ class App extends React.Component<AppProps, AppState> {
     if (opts.fitToContent) {
       this.scrollToContent(newElements, {
         fitToContent: true,
+        canvasOffsets: this.getEditorUIOffsets(),
       });
     }
   };
@@ -3544,7 +3546,7 @@ class App extends React.Component<AppProps, AppState> {
     target:
       | ExcalidrawElement
       | readonly ExcalidrawElement[] = this.scene.getNonDeletedElements(),
-    opts?:
+    opts?: (
       | {
           fitToContent?: boolean;
           fitToViewport?: never;
@@ -3561,7 +3563,12 @@ class App extends React.Component<AppProps, AppState> {
           viewportZoomFactor?: number;
           animate?: boolean;
           duration?: number;
-        },
+        }
+    ) & {
+      minZoom?: number;
+      maxZoom?: number;
+      canvasOffsets?: Offsets;
+    },
   ) => {
     this.cancelInProgressAnimation?.();
 
@@ -3574,10 +3581,13 @@ class App extends React.Component<AppProps, AppState> {
 
     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;
@@ -3805,40 +3815,42 @@ class App extends React.Component<AppProps, AppState> {
     },
   );
 
-  public getEditorUIOffsets = (): {
-    top: number;
-    right: number;
-    bottom: number;
-    left: number;
-  } => {
+  public getEditorUIOffsets = (): Offsets => {
     const toolbarBottom =
       this.excalidrawContainerRef?.current
         ?.querySelector(".App-toolbar")
         ?.getBoundingClientRect()?.bottom ?? 0;
-    const sidebarWidth = Math.max(
-      this.excalidrawContainerRef?.current
-        ?.querySelector(".default-sidebar")
-        ?.getBoundingClientRect()?.width ?? 0,
-    );
-    const propertiesPanelWidth = Math.max(
-      this.excalidrawContainerRef?.current
-        ?.querySelector(".App-menu__left")
-        ?.getBoundingClientRect()?.width ?? 0,
-      0,
-    );
+    const sidebarRect = this.excalidrawContainerRef?.current
+      ?.querySelector(".sidebar")
+      ?.getBoundingClientRect();
+    const propertiesPanelRect = this.excalidrawContainerRef?.current
+      ?.querySelector(".App-menu__left")
+      ?.getBoundingClientRect();
+
+    const PADDING = 16;
 
     return getLanguage().rtl
       ? {
-          top: toolbarBottom,
-          right: propertiesPanelWidth,
-          bottom: 0,
-          left: sidebarWidth,
+          top: toolbarBottom + PADDING,
+          right:
+            Math.max(
+              this.state.width -
+                (propertiesPanelRect?.left ?? this.state.width),
+              0,
+            ) + PADDING,
+          bottom: PADDING,
+          left: Math.max(sidebarRect?.right ?? 0, 0) + PADDING,
         }
       : {
-          top: toolbarBottom,
-          right: sidebarWidth,
-          bottom: 0,
-          left: propertiesPanelWidth,
+          top: toolbarBottom + PADDING,
+          right: Math.max(
+            this.state.width -
+              (sidebarRect?.left ?? this.state.width) +
+              PADDING,
+            0,
+          ),
+          bottom: PADDING,
+          left: Math.max(propertiesPanelRect?.right ?? 0, 0) + PADDING,
         };
   };
 
@@ -3923,7 +3935,7 @@ class App extends React.Component<AppProps, AppState> {
               animate: true,
               duration: 300,
               fitToContent: true,
-              viewportZoomFactor: 0.8,
+              canvasOffsets: this.getEditorUIOffsets(),
             });
           }
 
@@ -3979,6 +3991,7 @@ class App extends React.Component<AppProps, AppState> {
                 this.scrollToContent(nextNode, {
                   animate: true,
                   duration: 300,
+                  canvasOffsets: this.getEditorUIOffsets(),
                 });
               }
             }
@@ -4411,6 +4424,7 @@ class App extends React.Component<AppProps, AppState> {
             this.scrollToContent(firstNode, {
               animate: true,
               duration: 300,
+              canvasOffsets: this.getEditorUIOffsets(),
             });
           }
         }

packages/excalidraw/components/SearchMenu.tsx

@@ -20,6 +20,7 @@ import { CLASSES, EVENT } from "../constants";
 import { useStable } from "../hooks/useStable";
 
 import "./SearchMenu.scss";
+import { round } from "../../math";
 
 const searchQueryAtom = atom<string>("");
 export const searchItemInFocusAtom = atom<number | null>(null);
@@ -154,16 +155,23 @@ export const SearchMenu = () => {
       const match = searchMatches.items[focusIndex];
 
       if (match) {
+        const zoomValue = app.state.zoom.value;
+
         const matchAsElement = newTextElement({
           text: match.searchQuery,
           x: match.textElement.x + (match.matchedLines[0]?.offsetX ?? 0),
           y: match.textElement.y + (match.matchedLines[0]?.offsetY ?? 0),
           width: match.matchedLines[0]?.width,
           height: match.matchedLines[0]?.height,
+          fontSize: match.textElement.fontSize,
+          fontFamily: match.textElement.fontFamily,
         });
 
+        const FONT_SIZE_LEGIBILITY_THRESHOLD = 14;
+
+        const fontSize = match.textElement.fontSize;
         const isTextTiny =
-          match.textElement.fontSize * app.state.zoom.value < 12;
+          fontSize * zoomValue < FONT_SIZE_LEGIBILITY_THRESHOLD;
 
         if (
           !isElementCompletelyInViewport(
@@ -184,9 +192,17 @@ export const SearchMenu = () => {
         ) {
           let zoomOptions: Parameters<AppClassProperties["scrollToContent"]>[1];
 
-          if (isTextTiny && app.state.zoom.value >= 1) {
-            zoomOptions = { fitToViewport: true };
-          } else if (isTextTiny || app.state.zoom.value > 1) {
+          if (isTextTiny) {
+            if (fontSize >= FONT_SIZE_LEGIBILITY_THRESHOLD) {
+              zoomOptions = { fitToContent: true };
+            } else {
+              zoomOptions = {
+                fitToViewport: true,
+                // calculate zoom level to make the fontSize ~equal to FONT_SIZE_THRESHOLD, rounded to nearest 10%
+                maxZoom: round(FONT_SIZE_LEGIBILITY_THRESHOLD / fontSize, 1),
+              };
+            }
+          } else {
             zoomOptions = { fitToContent: true };
           }
 
@@ -194,6 +210,7 @@ export const SearchMenu = () => {
             animate: true,
             duration: 300,
             ...zoomOptions,
+            canvasOffsets: app.getEditorUIOffsets(),
           });
         }
       }

packages/excalidraw/element/newElement.ts

@@ -223,7 +223,6 @@ export const newTextElement = (
     verticalAlign?: VerticalAlign;
     containerId?: ExcalidrawTextContainer["id"] | null;
     lineHeight?: ExcalidrawTextElement["lineHeight"];
-    strokeWidth?: ExcalidrawTextElement["strokeWidth"];
     autoResize?: ExcalidrawTextElement["autoResize"];
   } & ElementConstructorOpts,
 ): NonDeleted<ExcalidrawTextElement> => {

packages/excalidraw/element/sizeHelpers.ts

@@ -2,7 +2,7 @@ import type { ElementsMap, ExcalidrawElement } from "./types";
 import { mutateElement } from "./mutateElement";
 import { isFreeDrawElement, isLinearElement } from "./typeChecks";
 import { SHIFT_LOCKING_ANGLE } from "../constants";
-import type { AppState, Zoom } from "../types";
+import type { AppState, Offsets, Zoom } from "../types";
 import { getCommonBounds, getElementBounds } from "./bounds";
 import { viewportCoordsToSceneCoords } from "../utils";
 
@@ -67,12 +67,7 @@ export const isElementCompletelyInViewport = (
     scrollY: number;
   },
   elementsMap: ElementsMap,
-  padding?: Partial<{
-    top: number;
-    right: number;
-    bottom: number;
-    left: number;
-  }>,
+  padding?: Offsets,
 ) => {
   const [x1, y1, x2, y2] = getCommonBounds(elements, elementsMap); // scene coordinates
   const topLeftSceneCoords = viewportCoordsToSceneCoords(

packages/excalidraw/scene/scroll.ts

@@ -1,4 +1,4 @@
-import type { AppState, PointerCoords, Zoom } from "../types";
+import type { AppState, Offsets, PointerCoords, Zoom } from "../types";
 import type { ExcalidrawElement } from "../element/types";
 import {
   getCommonBounds,
@@ -31,14 +31,28 @@ export const centerScrollOn = ({
   scenePoint,
   viewportDimensions,
   zoom,
+  offsets,
 }: {
   scenePoint: PointerCoords;
   viewportDimensions: { height: number; width: number };
   zoom: Zoom;
+  offsets?: Offsets;
 }) => {
+  let scrollX =
+    (viewportDimensions.width - (offsets?.right ?? 0)) / 2 / zoom.value -
+    scenePoint.x;
+
+  scrollX += (offsets?.left ?? 0) / 2 / zoom.value;
+
+  let scrollY =
+    (viewportDimensions.height - (offsets?.bottom ?? 0)) / 2 / zoom.value -
+    scenePoint.y;
+
+  scrollY += (offsets?.top ?? 0) / 2 / zoom.value;
+
   return {
-    scrollX: viewportDimensions.width / 2 / zoom.value - scenePoint.x,
-    scrollY: viewportDimensions.height / 2 / zoom.value - scenePoint.y,
+    scrollX,
+    scrollY,
   };
 };
 

packages/excalidraw/types.ts

@@ -851,3 +851,10 @@ export type GenerateDiagramToCode = (props: {
   frame: ExcalidrawMagicFrameElement;
   children: readonly ExcalidrawElement[];
 }) => MaybePromise<{ html: string }>;
+
+export type Offsets = Partial<{
+  top: number;
+  right: number;
+  bottom: number;
+  left: number;
+}>;

packages/math/utils.ts

@@ -1,14 +1,27 @@
 export const PRECISION = 10e-5;
 
-export function clamp(value: number, min: number, max: number) {
+export const clamp = (value: number, min: number, max: number) => {
   return Math.min(Math.max(value, min), max);
-}
+};
 
-export function round(value: number, precision: number) {
+export const round = (
+  value: number,
+  precision: number,
+  func: "round" | "floor" | "ceil" = "round",
+) => {
   const multiplier = Math.pow(10, precision);
 
-  return Math.round((value + Number.EPSILON) * multiplier) / multiplier;
-}
+  return Math[func]((value + Number.EPSILON) * multiplier) / multiplier;
+};
+
+export const roundToStep = (
+  value: number,
+  step: number,
+  func: "round" | "floor" | "ceil" = "round",
+): number => {
+  const factor = 1 / step;
+  return Math[func](value * factor) / factor;
+};
 
 export const average = (a: number, b: number) => (a + b) / 2;