From 5a8dbe8030b60239ef1ea1bde4d901db8dadc082 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Thu, 11 Aug 2022 20:16:25 +0530 Subject: [PATCH] feat: show a mid point for linear elements (#5534) * feat: Add a mid point for linear elements * fix tests * show mid point only on hover * hacky fix :( * don't add mid points if present and only add outside editor * improve styling and always show phantom point instead of just on hover * fix tests * fix * only add polyfill for test * add hover state for phantom point * fix tests * fix * Add Array.at polyfill * reuse `centerPoint()` helper * reuse `distance2d` * use `Point` type Co-authored-by: dwelle --- src/components/App.tsx | 32 ++++- src/element/linearElementEditor.ts | 105 +++++++++++++++- src/excalidraw-app/index.tsx | 2 + src/packages/excalidraw/entry.js | 2 + src/packages/excalidraw/index.tsx | 1 - src/polyfill.ts | 26 ++++ src/renderer/renderScene.ts | 118 +++++++++++++----- src/setupTests.ts | 3 +- .../regressionTests.test.tsx.snap | 4 + .../__snapshots__/selection.test.tsx.snap | 16 ++- 10 files changed, 266 insertions(+), 43 deletions(-) create mode 100644 src/polyfill.ts diff --git a/src/components/App.tsx b/src/components/App.tsx index 9b34442e3..b280d079b 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -1,5 +1,6 @@ import React, { useContext } from "react"; import { flushSync } from "react-dom"; + import { RoughCanvas } from "roughjs/bin/canvas"; import rough from "roughjs/bin/rough"; import clsx from "clsx"; @@ -3030,6 +3031,7 @@ class App extends React.Component { } if (this.state.selectedLinearElement) { let hoverPointIndex = -1; + let midPointHovered = false; if ( isHittingElementNotConsideringBoundingBox(element, this.state, [ scenePointerX, @@ -3042,7 +3044,13 @@ class App extends React.Component { scenePointerX, scenePointerY, ); - if (hoverPointIndex >= 0) { + midPointHovered = LinearElementEditor.isHittingMidPoint( + linearElementEditor, + { x: scenePointerX, y: scenePointerY }, + this.state, + ); + + if (hoverPointIndex >= 0 || midPointHovered) { setCursor(this.canvas, CURSOR_TYPE.POINTER); } else { setCursor(this.canvas, CURSOR_TYPE.MOVE); @@ -3069,6 +3077,17 @@ class App extends React.Component { }, }); } + + if ( + this.state.selectedLinearElement.midPointHovered !== midPointHovered + ) { + this.setState({ + selectedLinearElement: { + ...this.state.selectedLinearElement, + midPointHovered, + }, + }); + } } else { setCursor(this.canvas, CURSOR_TYPE.AUTO); } @@ -3623,7 +3642,7 @@ class App extends React.Component { this.setState({ editingLinearElement: ret.linearElementEditor }); } } - if (ret.didAddPoint) { + if (ret.didAddPoint && !ret.isMidPoint) { return true; } } @@ -4112,6 +4131,7 @@ class App extends React.Component { // to ensure we don't create a 2-point arrow by mistake when // user clicks mouse in a way that it moves a tiny bit (thus // triggering pointermove) + if ( !pointerDownState.drag.hasOccurred && (this.state.activeTool.type === "arrow" || @@ -4128,7 +4148,6 @@ class App extends React.Component { return; } } - if (pointerDownState.resize.isResizing) { pointerDownState.lastCoords.x = pointerCoords.x; pointerDownState.lastCoords.y = pointerCoords.y; @@ -4339,8 +4358,10 @@ class App extends React.Component { } if (points.length === 1) { - mutateElement(draggingElement, { points: [...points, [dx, dy]] }); - } else if (points.length > 1) { + mutateElement(draggingElement, { + points: [...points, [dx, dy]], + }); + } else if (points.length === 2) { mutateElement(draggingElement, { points: [...points.slice(0, -1), [dx, dy]], }); @@ -6202,4 +6223,5 @@ if ( }, }); } + export default App; diff --git a/src/element/linearElementEditor.ts b/src/element/linearElementEditor.ts index 49200711f..9b7793a2b 100644 --- a/src/element/linearElementEditor.ts +++ b/src/element/linearElementEditor.ts @@ -11,6 +11,7 @@ import { isPathALoop, getGridPoint, rotatePoint, + centerPoint, } from "../math"; import { getElementAbsoluteCoords, getLockedLinearCursorAlignSize } from "."; import { getElementPointsCoords } from "./bounds"; @@ -51,6 +52,7 @@ export class LinearElementEditor { | "keep"; public readonly endBindingElement: ExcalidrawBindableElement | null | "keep"; public readonly hoverPointIndex: number; + public readonly midPointHovered: boolean; constructor(element: NonDeleted, scene: Scene) { this.elementId = element.id as string & { @@ -70,6 +72,7 @@ export class LinearElementEditor { lastClickedPoint: -1, }; this.hoverPointIndex = -1; + this.midPointHovered = false; } // --------------------------------------------------------------------------- @@ -157,7 +160,6 @@ export class LinearElementEditor { if (!linearElementEditor) { return false; } - const { selectedPointsIndices, elementId } = linearElementEditor; const element = LinearElementEditor.getElement(elementId); if (!element) { @@ -357,6 +359,59 @@ export class LinearElementEditor { }; } + static isHittingMidPoint = ( + linearElementEditor: LinearElementEditor, + scenePointer: { x: number; y: number }, + appState: AppState, + ) => { + if (appState.editingLinearElement) { + return false; + } + const { elementId } = linearElementEditor; + const element = LinearElementEditor.getElement(elementId); + if (!element) { + return false; + } + const clickedPointIndex = LinearElementEditor.getPointIndexUnderCursor( + element, + appState.zoom, + scenePointer.x, + scenePointer.y, + ); + if (clickedPointIndex >= 0) { + return false; + } + const points = LinearElementEditor.getPointsGlobalCoordinates(element); + if (points.length >= 3) { + return false; + } + + const midPoint = this.getMidPoint(linearElementEditor); + if (midPoint) { + const threshold = + LinearElementEditor.POINT_HANDLE_SIZE / appState.zoom.value; + const distance = distance2d( + midPoint[0], + midPoint[1], + scenePointer.x, + scenePointer.y, + ); + return distance <= threshold; + } + return false; + }; + + static getMidPoint(linearElementEditor: LinearElementEditor) { + const { elementId } = linearElementEditor; + const element = LinearElementEditor.getElement(elementId); + if (!element) { + return null; + } + const points = LinearElementEditor.getPointsGlobalCoordinates(element); + + return centerPoint(points[0], points.at(-1)!); + } + static handlePointerDown( event: React.PointerEvent, appState: AppState, @@ -367,11 +422,13 @@ export class LinearElementEditor { didAddPoint: boolean; hitElement: NonDeleted | null; linearElementEditor: LinearElementEditor | null; + isMidPoint: boolean; } { const ret: ReturnType = { didAddPoint: false, hitElement: null, linearElementEditor: null, + isMidPoint: false, }; if (!linearElementEditor) { @@ -384,6 +441,45 @@ export class LinearElementEditor { if (!element) { return ret; } + const hittingMidPoint = LinearElementEditor.isHittingMidPoint( + linearElementEditor, + scenePointer, + appState, + ); + if ( + LinearElementEditor.isHittingMidPoint( + linearElementEditor, + scenePointer, + appState, + ) + ) { + const midPoint = this.getMidPoint(linearElementEditor); + if (midPoint) { + mutateElement(element, { + points: [ + element.points[0], + LinearElementEditor.createPointAt( + element, + midPoint[0], + midPoint[1], + appState.gridSize, + ), + ...element.points.slice(1), + ], + }); + } + ret.didAddPoint = true; + ret.isMidPoint = true; + ret.linearElementEditor = { + ...linearElementEditor, + selectedPointsIndices: element.points[1], + pointerDownState: { + prevSelectedPointsIndices: linearElementEditor.selectedPointsIndices, + lastClickedPoint: -1, + }, + lastUncommittedPoint: null, + }; + } if (event.altKey && appState.editingLinearElement) { if (linearElementEditor.lastUncommittedPoint == null) { mutateElement(element, { @@ -397,6 +493,7 @@ export class LinearElementEditor { ), ], }); + ret.didAddPoint = true; } history.resumeRecording(); ret.linearElementEditor = { @@ -426,7 +523,7 @@ export class LinearElementEditor { // if we clicked on a point, set the element as hitElement otherwise // it would get deselected if the point is outside the hitbox area - if (clickedPointIndex > -1) { + if (clickedPointIndex >= 0 || hittingMidPoint) { ret.hitElement = element; } else { // You might be wandering why we are storing the binding elements on @@ -567,14 +664,14 @@ export class LinearElementEditor { /** scene coords */ static getPointsGlobalCoordinates( element: NonDeleted, - ) { + ): Point[] { const [x1, y1, x2, y2] = getElementAbsoluteCoords(element); const cx = (x1 + x2) / 2; const cy = (y1 + y2) / 2; return element.points.map((point) => { let { x, y } = element; [x, y] = rotate(x + point[0], y + point[1], cx, cy, element.angle); - return [x, y]; + return [x, y] as const; }); } diff --git a/src/excalidraw-app/index.tsx b/src/excalidraw-app/index.tsx index 321ee4d4e..3f059e6d8 100644 --- a/src/excalidraw-app/index.tsx +++ b/src/excalidraw-app/index.tsx @@ -1,3 +1,4 @@ +import polyfill from "../polyfill"; import LanguageDetector from "i18next-browser-languagedetector"; import { useCallback, useEffect, useRef, useState } from "react"; import { trackEvent } from "../analytics"; @@ -83,6 +84,7 @@ import { jotaiStore, useAtomWithInitialValue } from "../jotai"; import { reconcileElements } from "./collab/reconciliation"; import { parseLibraryTokensFromUrl, useHandleLibrary } from "../data/library"; +polyfill(); window.EXCALIDRAW_THROTTLE_RENDER = true; const isExcalidrawPlusSignedUser = document.cookie.includes( diff --git a/src/packages/excalidraw/entry.js b/src/packages/excalidraw/entry.js index 02c14fdb1..17253e740 100644 --- a/src/packages/excalidraw/entry.js +++ b/src/packages/excalidraw/entry.js @@ -1,5 +1,7 @@ import "./publicPath"; +import polyfill from "../../polyfill"; import "../../../public/fonts.css"; +polyfill(); export * from "./index"; diff --git a/src/packages/excalidraw/index.tsx b/src/packages/excalidraw/index.tsx index 7e0798f4d..c7b6f37ba 100644 --- a/src/packages/excalidraw/index.tsx +++ b/src/packages/excalidraw/index.tsx @@ -1,5 +1,4 @@ import React, { useEffect, forwardRef } from "react"; - import { InitializeApp } from "../../components/InitializeApp"; import App from "../../components/App"; diff --git a/src/polyfill.ts b/src/polyfill.ts new file mode 100644 index 000000000..cc4891b4d --- /dev/null +++ b/src/polyfill.ts @@ -0,0 +1,26 @@ +const polyfill = () => { + if (!Array.prototype.at) { + // Taken from https://github.com/tc39/proposal-relative-indexing-method#polyfill so that it works in tests + /* eslint-disable */ + Object.defineProperty(Array.prototype, "at", { + value: function (n: number) { + // ToInteger() abstract op + n = Math.trunc(n) || 0; + // Allow negative indexing from the end + if (n < 0) { + n += this.length; + } + // OOB access is guaranteed to return undefined + if (n < 0 || n >= this.length) { + return undefined; + } + // Otherwise, this is just normal property access + return this[n]; + }, + writable: true, + enumerable: false, + configurable: true, + }); + } +}; +export default polyfill; diff --git a/src/renderer/renderScene.ts b/src/renderer/renderScene.ts index 4eee17bcd..7db360022 100644 --- a/src/renderer/renderScene.ts +++ b/src/renderer/renderScene.ts @@ -2,7 +2,7 @@ import { RoughCanvas } from "roughjs/bin/canvas"; import { RoughSVG } from "roughjs/bin/svg"; import oc from "open-color"; -import { AppState, BinaryFiles, Zoom } from "../types"; +import { AppState, BinaryFiles, Point, Zoom } from "../types"; import { ExcalidrawElement, NonDeletedExcalidrawElement, @@ -157,34 +157,105 @@ const strokeGrid = ( context.restore(); }; +const renderSingleLinearPoint = ( + context: CanvasRenderingContext2D, + appState: AppState, + renderConfig: RenderConfig, + point: Point, + isSelected: boolean, + isPhantomPoint = false, +) => { + context.strokeStyle = "#5e5ad8"; + context.setLineDash([]); + context.fillStyle = "rgba(255, 255, 255, 0.9)"; + if (isSelected) { + context.fillStyle = "rgba(134, 131, 226, 0.9)"; + } else if (isPhantomPoint) { + context.fillStyle = "rgba(177, 151, 252, 0.7)"; + } + const { POINT_HANDLE_SIZE } = LinearElementEditor; + const radius = appState.editingLinearElement + ? POINT_HANDLE_SIZE + : POINT_HANDLE_SIZE / 2; + fillCircle( + context, + point[0], + point[1], + radius / renderConfig.zoom.value, + !isPhantomPoint, + ); +}; + const renderLinearPointHandles = ( context: CanvasRenderingContext2D, appState: AppState, renderConfig: RenderConfig, element: NonDeleted, ) => { + if (!appState.selectedLinearElement) { + return; + } context.save(); context.translate(renderConfig.scrollX, renderConfig.scrollY); context.lineWidth = 1 / renderConfig.zoom.value; - - LinearElementEditor.getPointsGlobalCoordinates(element).forEach( - (point, idx) => { - context.strokeStyle = "#5e5ad8"; - context.setLineDash([]); - context.fillStyle = - appState.editingLinearElement?.selectedPointsIndices?.includes(idx) - ? "rgba(134, 131, 226, 0.9)" - : "rgba(255, 255, 255, 0.9)"; - const { POINT_HANDLE_SIZE } = LinearElementEditor; - const radius = appState.editingLinearElement - ? POINT_HANDLE_SIZE - : POINT_HANDLE_SIZE / 2; - fillCircle(context, point[0], point[1], radius / renderConfig.zoom.value); - }, + const points = LinearElementEditor.getPointsGlobalCoordinates(element); + const centerPoint = LinearElementEditor.getMidPoint( + appState.selectedLinearElement, ); + if (!centerPoint) { + return; + } + points.forEach((point, idx) => { + const isSelected = + !!appState.editingLinearElement?.selectedPointsIndices?.includes(idx); + renderSingleLinearPoint(context, appState, renderConfig, point, isSelected); + }); + + if (!appState.editingLinearElement && points.length < 3) { + if (appState.selectedLinearElement.midPointHovered) { + const centerPoint = LinearElementEditor.getMidPoint( + appState.selectedLinearElement, + )!; + highlightPoint(centerPoint, context, appState, renderConfig); + + renderSingleLinearPoint( + context, + appState, + renderConfig, + centerPoint, + false, + ); + } else { + renderSingleLinearPoint( + context, + appState, + renderConfig, + centerPoint, + false, + true, + ); + } + } + context.restore(); }; +const highlightPoint = ( + point: Point, + context: CanvasRenderingContext2D, + appState: AppState, + renderConfig: RenderConfig, +) => { + context.fillStyle = "rgba(105, 101, 219, 0.4)"; + + fillCircle( + context, + point[0], + point[1], + LinearElementEditor.POINT_HANDLE_SIZE / renderConfig.zoom.value, + false, + ); +}; const renderLinearElementPointHighlight = ( context: CanvasRenderingContext2D, appState: AppState, @@ -202,23 +273,14 @@ const renderLinearElementPointHighlight = ( if (!element) { return; } - const [x, y] = LinearElementEditor.getPointAtIndexGlobalCoordinates( + const point = LinearElementEditor.getPointAtIndexGlobalCoordinates( element, hoverPointIndex, ); context.save(); context.translate(renderConfig.scrollX, renderConfig.scrollY); - context.fillStyle = "rgba(105, 101, 219, 0.4)"; - - fillCircle( - context, - x, - y, - LinearElementEditor.POINT_HANDLE_SIZE / renderConfig.zoom.value, - false, - ); - + highlightPoint(point, context, appState, renderConfig); context.restore(); }; @@ -345,7 +407,7 @@ export const _renderScene = ( if ( appState.selectedLinearElement && - appState.selectedLinearElement.hoverPointIndex !== -1 + appState.selectedLinearElement.hoverPointIndex >= 0 ) { renderLinearElementPointHighlight(context, appState, renderConfig); } diff --git a/src/setupTests.ts b/src/setupTests.ts index eb150a484..cb401838e 100644 --- a/src/setupTests.ts +++ b/src/setupTests.ts @@ -1,8 +1,9 @@ import "@testing-library/jest-dom"; import "jest-canvas-mock"; - import dotenv from "dotenv"; +import polyfill from "./polyfill"; +polyfill(); // jest doesn't know of .env.development so we need to init it ourselves dotenv.config({ path: require("path").resolve(__dirname, "../.env.development"), diff --git a/src/tests/__snapshots__/regressionTests.test.tsx.snap b/src/tests/__snapshots__/regressionTests.test.tsx.snap index 58cf28212..3f023df45 100644 --- a/src/tests/__snapshots__/regressionTests.test.tsx.snap +++ b/src/tests/__snapshots__/regressionTests.test.tsx.snap @@ -10982,6 +10982,7 @@ Object { "hoverPointIndex": -1, "isDragging": false, "lastUncommittedPoint": null, + "midPointHovered": false, "pointerDownState": Object { "lastClickedPoint": -1, "prevSelectedPointsIndices": null, @@ -11207,6 +11208,7 @@ Object { "hoverPointIndex": -1, "isDragging": false, "lastUncommittedPoint": null, + "midPointHovered": false, "pointerDownState": Object { "lastClickedPoint": -1, "prevSelectedPointsIndices": null, @@ -11659,6 +11661,7 @@ Object { "hoverPointIndex": -1, "isDragging": false, "lastUncommittedPoint": null, + "midPointHovered": false, "pointerDownState": Object { "lastClickedPoint": -1, "prevSelectedPointsIndices": null, @@ -12063,6 +12066,7 @@ Object { "hoverPointIndex": -1, "isDragging": false, "lastUncommittedPoint": null, + "midPointHovered": false, "pointerDownState": Object { "lastClickedPoint": -1, "prevSelectedPointsIndices": null, diff --git a/src/tests/__snapshots__/selection.test.tsx.snap b/src/tests/__snapshots__/selection.test.tsx.snap index d3f6ae527..1bf265d1b 100644 --- a/src/tests/__snapshots__/selection.test.tsx.snap +++ b/src/tests/__snapshots__/selection.test.tsx.snap @@ -21,6 +21,10 @@ Object { 0, 0, ], + Array [ + 15, + 25, + ], Array [ 30, 50, @@ -36,8 +40,8 @@ Object { "strokeWidth": 1, "type": "arrow", "updated": 1, - "version": 3, - "versionNonce": 449462985, + "version": 4, + "versionNonce": 453191, "width": 30, "x": 10, "y": 10, @@ -65,6 +69,10 @@ Object { 0, 0, ], + Array [ + 15, + 25, + ], Array [ 30, 50, @@ -80,8 +88,8 @@ Object { "strokeWidth": 1, "type": "line", "updated": 1, - "version": 3, - "versionNonce": 449462985, + "version": 4, + "versionNonce": 453191, "width": 30, "x": 10, "y": 10,