diff --git a/.codesandbox/Dockerfile b/.codesandbox/Dockerfile index fd5b38d1e8..ce8c857650 100644 --- a/.codesandbox/Dockerfile +++ b/.codesandbox/Dockerfile @@ -1,4 +1,4 @@ -FROM node:18-bullseye +FROM node:24-bullseye # Vite wants to open the browser using `open`, so we # need to install those utils. diff --git a/.github/workflows/autorelease-excalidraw.yml b/.github/workflows/autorelease-excalidraw.yml index c365647ee8..289189fd83 100644 --- a/.github/workflows/autorelease-excalidraw.yml +++ b/.github/workflows/autorelease-excalidraw.yml @@ -9,11 +9,11 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 2 - name: Setup Node.js - uses: actions/setup-node@v2 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 20.x - name: Set up publish access diff --git a/.github/workflows/build-docker.yml b/.github/workflows/build-docker.yml index f5f9b45bbe..3e2dc3d3c5 100644 --- a/.github/workflows/build-docker.yml +++ b/.github/workflows/build-docker.yml @@ -9,5 +9,5 @@ jobs: build-docker: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - run: docker build -t excalidraw . diff --git a/.github/workflows/cancel.yml b/.github/workflows/cancel.yml index e1ef216651..a572df9bed 100644 --- a/.github/workflows/cancel.yml +++ b/.github/workflows/cancel.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 3 steps: - - uses: styfle/cancel-workflow-action@0.6.0 + - uses: styfle/cancel-workflow-action@ce177499ccf9fd2aded3b0426c97e5434c2e8a73 # 0.6.0 with: workflow_id: 400555, 400556, 905313, 1451724, 1710116, 3185001, 3438604 access_token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index cc73980d10..22ded0d079 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -7,10 +7,10 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Setup Node.js - uses: actions/setup-node@v2 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 20.x diff --git a/.github/workflows/locales-coverage.yml b/.github/workflows/locales-coverage.yml index d69f1b1449..9a5a93adac 100644 --- a/.github/workflows/locales-coverage.yml +++ b/.github/workflows/locales-coverage.yml @@ -10,12 +10,12 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: token: ${{ secrets.PUSH_TRANSLATIONS_COVERAGE_PAT }} - name: Setup Node.js - uses: actions/setup-node@v2 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 20.x @@ -40,7 +40,7 @@ jobs: echo ::set-output name=body::$body - name: Update description with coverage - uses: kt3k/update-pr-description@v1.0.1 + uses: kt3k/update-pr-description@1b35a6dcd84d81aa0bc1889610efdcde7f37b0c0 # v1.0.1 with: pr_body: ${{ steps.getCommentBody.outputs.body }} pr_title: "chore: Update translations from Crowdin" diff --git a/.github/workflows/publish-docker.yml b/.github/workflows/publish-docker.yml index 68eee27755..3019e9b097 100644 --- a/.github/workflows/publish-docker.yml +++ b/.github/workflows/publish-docker.yml @@ -11,18 +11,18 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Login to DockerHub - uses: docker/login-action@v2 + uses: docker/login-action@465a07811f14bebb1938fbed4728c6a1ff8901fc # v2 with: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_PASSWORD }} - name: Set up QEMU - uses: docker/setup-qemu-action@v3 + uses: docker/setup-qemu-action@c7c53464625b32c7a7e944ae62b3e17d2b600130 # v3 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3 + uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3 - name: Build and push - uses: docker/build-push-action@v5 + uses: docker/build-push-action@ca052bb54ab0790a636c9b5f226502c73d547a25 # v5 with: context: . push: true diff --git a/.github/workflows/semantic-pr-title.yml b/.github/workflows/semantic-pr-title.yml index 34a6413fe2..0dc2c879f4 100644 --- a/.github/workflows/semantic-pr-title.yml +++ b/.github/workflows/semantic-pr-title.yml @@ -6,11 +6,97 @@ on: - opened - edited - synchronize + - labeled + - unlabeled jobs: semantic: runs-on: ubuntu-latest + permissions: + pull-requests: read steps: - - uses: amannn/action-semantic-pull-request@v5 + - uses: amannn/action-semantic-pull-request@e32d7e603df1aa1ba07e981f2a23455dee596825 # v5 + with: + requireScope: true + scopes: | + app + editor + packages/excalidraw + packages/utils + docker + repo + ignoreLabels: | + skip-semantic-title env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + label-scope: + needs: semantic + if: github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + permissions: + issues: write + pull-requests: write + steps: + - name: Label scoped PR + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_TITLE: ${{ github.event.pull_request.title }} + REPOSITORY: ${{ github.repository }} + run: | + set -euo pipefail + + scope_labels=(s-app s-editor s-package) + + readarray -t desired_labels < <( + node <<'NODE' + const title = process.env.PR_TITLE; + const match = title.match(/^[a-z]+(?:\(([^)]+)\))?!?:/i); + const scopes = match?.[1]?.split(",").map((scope) => scope.trim()) ?? []; + const labels = new Set(); + + for (const scope of scopes) { + if (scope === "app") { + labels.add("s-app"); + } else if (scope === "editor") { + labels.add("s-editor"); + } else if (scope.startsWith("packages/")) { + labels.add("s-package"); + } + } + + process.stdout.write([...labels].join("\n")); + NODE + ) + + should_apply_label() { + local label="$1" + + for desired_label in "${desired_labels[@]}"; do + if [[ "$desired_label" == "$label" ]]; then + return 0 + fi + done + + return 1 + } + + for label in "${scope_labels[@]}"; do + if ! should_apply_label "$label"; then + gh api \ + --method DELETE \ + "repos/${REPOSITORY}/issues/${PR_NUMBER}/labels/${label}" \ + --silent 2>/dev/null || true + fi + done + + for label in "${desired_labels[@]}"; do + if ! gh api \ + --method POST \ + "repos/${REPOSITORY}/issues/${PR_NUMBER}/labels" \ + --field "labels[]=${label}" \ + --silent; then + echo "::warning::Could not apply ${label}. The workflow token likely does not have issues:write permission for this PR." + fi + done diff --git a/.github/workflows/sentry-production.yml b/.github/workflows/sentry-production.yml index 4434873fd3..c8270a0163 100644 --- a/.github/workflows/sentry-production.yml +++ b/.github/workflows/sentry-production.yml @@ -9,9 +9,9 @@ jobs: sentry: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Setup Node.js - uses: actions/setup-node@v2 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 20.x - name: Install and build diff --git a/.github/workflows/size-limit.yml b/.github/workflows/size-limit.yml index ded07f91fb..4c80695f1f 100644 --- a/.github/workflows/size-limit.yml +++ b/.github/workflows/size-limit.yml @@ -10,9 +10,9 @@ jobs: CI_JOB_NUMBER: 1 steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Setup Node.js - uses: actions/setup-node@v3 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 20.x - name: Install in packages/excalidraw @@ -20,7 +20,7 @@ jobs: working-directory: packages/excalidraw env: CI: true - - uses: andresz1/size-limit-action@v1 + - uses: andresz1/size-limit-action@e7493a72a44b113341c0cf6186ab49c17c4b65c1 # v1 with: github_token: ${{ secrets.GITHUB_TOKEN }} build_script: build:esm diff --git a/.github/workflows/test-coverage-pr.yml b/.github/workflows/test-coverage-pr.yml index 227991a7bc..ffd75a7c84 100644 --- a/.github/workflows/test-coverage-pr.yml +++ b/.github/workflows/test-coverage-pr.yml @@ -10,9 +10,9 @@ jobs: pull-requests: write steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: "Install Node" - uses: actions/setup-node@v2 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: "20.x" - name: "Install Deps" @@ -21,6 +21,6 @@ jobs: run: yarn test:coverage - name: "Report Coverage" if: always() # Also generate the report if tests are failing - uses: davelosert/vitest-coverage-report-action@v2 + uses: davelosert/vitest-coverage-report-action@2500dafcee7dd64f85ab689c0b83798a8359770e # v2 with: github-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8bebd6c1ee..78f5e9a7d2 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -8,9 +8,9 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Setup Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 20.x - name: Install and test diff --git a/Dockerfile b/Dockerfile index c08385d654..a941c99980 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM --platform=${BUILDPLATFORM} node:18 AS build +FROM --platform=${BUILDPLATFORM} node:24@sha256:8530f76a96d88820d288761f022e318970dda93d01536919fbc16076b7983e63 AS build WORKDIR /opt/node_app @@ -7,13 +7,13 @@ COPY . . # do not ignore optional dependencies: # Error: Cannot find module @rollup/rollup-linux-x64-gnu RUN --mount=type=cache,target=/root/.cache/yarn \ - npm_config_target_arch=${TARGETARCH} yarn --network-timeout 600000 + npm_config_target_arch=${TARGETARCH} yarn --frozen-lockfile --network-timeout 600000 ARG NODE_ENV=production RUN npm_config_target_arch=${TARGETARCH} yarn build:app:docker -FROM --platform=${TARGETPLATFORM} nginx:1.27-alpine +FROM nginx:stable-alpine-slim@sha256:2c605dbeab79a6b2a63340474fe58119d0ef95bdc4b1f41df0aa689659b3d13b COPY --from=build /opt/node_app/excalidraw-app/build /usr/share/nginx/html diff --git a/README.md b/README.md index 74fc2c084f..48b9d1de9f 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ PRs welcome! - Chat on Discord + Chat on Discord Ask DeepWiki diff --git a/dev-docs/docusaurus.config.js b/dev-docs/docusaurus.config.js index 4e8d75800a..d2b34843ee 100644 --- a/dev-docs/docusaurus.config.js +++ b/dev-docs/docusaurus.config.js @@ -97,8 +97,8 @@ const config = { href: "https://discord.gg/UexuTaE", }, { - label: "Twitter", - href: "https://twitter.com/excalidraw", + label: "𝕏", + href: "https://x.com/excalidraw", }, { label: "Linkedin", diff --git a/excalidraw-app/data/LocalData.ts b/excalidraw-app/data/LocalData.ts index 634fdc4f1c..d09d8aa88e 100644 --- a/excalidraw-app/data/LocalData.ts +++ b/excalidraw-app/data/LocalData.ts @@ -26,7 +26,6 @@ import { get, } from "idb-keyval"; -import { appJotaiStore, atom } from "excalidraw-app/app-jotai"; import { getNonDeletedElements } from "@excalidraw/element"; import type { LibraryPersistedData } from "@excalidraw/excalidraw/data/library"; @@ -39,6 +38,7 @@ import type { } from "@excalidraw/excalidraw/types"; import type { MaybePromise } from "@excalidraw/common/utility-types"; +import { appJotaiStore, atom } from "../app-jotai"; import { SAVE_TO_LOCAL_STORAGE_TIMEOUT, STORAGE_KEYS } from "../app_constants"; import { FileManager } from "./FileManager"; diff --git a/excalidraw-app/vite.config.mts b/excalidraw-app/vite.config.mts index fa8d63d956..dfb213ef3a 100644 --- a/excalidraw-app/vite.config.mts +++ b/excalidraw-app/vite.config.mts @@ -75,6 +75,13 @@ export default defineConfig(({ mode }) => { find: /^@excalidraw\/utils\/(.*?)/, replacement: path.resolve(__dirname, "../packages/utils/src/$1"), }, + { + find: /^@excalidraw\/fractional-indexing$/, + replacement: path.resolve( + __dirname, + "../packages/fractional-indexing/src/index.ts", + ), + }, ], }, build: { diff --git a/package.json b/package.json index 65ff221a49..df5c4e8a79 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,8 @@ "build:element": "yarn --cwd ./packages/element build:esm", "build:excalidraw": "yarn --cwd ./packages/excalidraw build:esm", "build:math": "yarn --cwd ./packages/math build:esm", - "build:packages": "yarn build:common && yarn build:math && yarn build:element && yarn build:excalidraw", + "build:fractional-indexing": "yarn --cwd ./packages/fractional-indexing build:esm", + "build:packages": "yarn build:common && yarn build:fractional-indexing && yarn build:math && yarn build:element && yarn build:excalidraw", "build:version": "yarn --cwd ./excalidraw-app build:version", "build": "yarn --cwd ./excalidraw-app build", "build:preview": "yarn --cwd ./excalidraw-app build:preview", diff --git a/packages/common/src/constants.ts b/packages/common/src/constants.ts index 4ff50335ef..0e94df5af6 100644 --- a/packages/common/src/constants.ts +++ b/packages/common/src/constants.ts @@ -337,9 +337,10 @@ export const MAX_DECIMALS_FOR_SVG_EXPORT = 2; export const EXPORT_SCALES = [1, 2, 3]; export const DEFAULT_EXPORT_PADDING = 10; // px -export const DEFAULT_MAX_IMAGE_WIDTH_OR_HEIGHT = 1440; - -export const MAX_ALLOWED_FILE_BYTES = 4 * 1024 * 1024; +export const DEFAULT_IMAGE_OPTIONS: AppProps["imageOptions"] = { + maxWidthOrHeight: 1440, + maxFileSizeBytes: 4 * 1024 * 1024, +}; export const SVG_NS = "http://www.w3.org/2000/svg"; export const SVG_DOCUMENT_PREAMBLE = ` diff --git a/packages/element/package.json b/packages/element/package.json index 7dff00a750..408f2c9d10 100644 --- a/packages/element/package.json +++ b/packages/element/package.json @@ -64,6 +64,7 @@ }, "dependencies": { "@excalidraw/common": "0.18.0", - "@excalidraw/math": "0.18.0" + "@excalidraw/math": "0.18.0", + "@excalidraw/fractional-indexing": "3.3.0" } } diff --git a/packages/element/src/Scene.ts b/packages/element/src/Scene.ts index 4ba663ceba..35b0cf4b95 100644 --- a/packages/element/src/Scene.ts +++ b/packages/element/src/Scene.ts @@ -338,29 +338,20 @@ export class Scene { this.callbacks.clear(); } - insertElementAtIndex(element: ExcalidrawElement, index: number) { - if (!Number.isFinite(index) || index < 0) { - throw new Error( - "insertElementAtIndex can only be called with index >= 0", - ); - } - - const nextElements = [ - ...this.elements.slice(0, index), - element, - ...this.elements.slice(index), - ]; - - syncMovedIndices(nextElements, arrayToMap([element])); - - this.replaceAllElements(nextElements); - } - - insertElementsAtIndex(elements: ExcalidrawElement[], index: number) { + /** low-level - generally use app.insertNewElements() */ + insertElementsAtIndex( + elements: ExcalidrawElement[], + /** null indicates end of the array */ + index: number | null, + ) { if (!elements.length) { return; } + if (index === null) { + index = this.elements.length; + } + if (!Number.isFinite(index) || index < 0) { throw new Error( "insertElementAtIndex can only be called with index >= 0", @@ -378,24 +369,9 @@ export class Scene { this.replaceAllElements(nextElements); } + /** low-level - generally use app.insertNewElement() */ insertElement = (element: ExcalidrawElement) => { - const index = element.frameId - ? this.getElementIndex(element.frameId) - : this.elements.length; - - this.insertElementAtIndex(element, index); - }; - - insertElements = (elements: ExcalidrawElement[]) => { - if (!elements.length) { - return; - } - - const index = elements[0]?.frameId - ? this.getElementIndex(elements[0].frameId) - : this.elements.length; - - this.insertElementsAtIndex(elements, index); + this.insertElementsAtIndex([element], null); }; getElementIndex(elementId: string) { diff --git a/packages/element/src/binding.ts b/packages/element/src/binding.ts index 5447b6784e..672be84245 100644 --- a/packages/element/src/binding.ts +++ b/packages/element/src/binding.ts @@ -735,12 +735,11 @@ const getBindingStrategyForDraggingBindingElementEndpoints_simple = ( }); // Handle outside-outside binding to the same element - if (otherBinding && otherBinding.elementId === hit?.id) { - invariant( - !opts?.newArrow || appState.selectedLinearElement?.initialState.origin, - "appState.selectedLinearElement.initialState.origin must be defined for new arrows", - ); - + if ( + otherBinding && + otherBinding.elementId === hit?.id && + (!opts?.newArrow || appState.selectedLinearElement?.initialState.origin) + ) { return { start: { mode: "inside", @@ -1983,9 +1982,9 @@ export const calculateFixedPointForElbowArrowBinding = ( return { fixedPoint: normalizeFixedPoint([ (nonRotatedSnappedGlobalPoint[0] - hoveredElement.x) / - hoveredElement.width, + Math.max(hoveredElement.width, PRECISION), (nonRotatedSnappedGlobalPoint[1] - hoveredElement.y) / - hoveredElement.height, + Math.max(hoveredElement.height, PRECISION), ]), }; }; @@ -2016,9 +2015,11 @@ export const calculateFixedPointForNonElbowArrowBinding = ( // Calculate the ratio relative to the element's bounds const fixedPointX = - (nonRotatedPoint[0] - hoveredElement.x) / hoveredElement.width; + (nonRotatedPoint[0] - hoveredElement.x) / + Math.max(hoveredElement.width, PRECISION); const fixedPointY = - (nonRotatedPoint[1] - hoveredElement.y) / hoveredElement.height; + (nonRotatedPoint[1] - hoveredElement.y) / + Math.max(hoveredElement.height, PRECISION); return { fixedPoint: normalizeFixedPoint([fixedPointX, fixedPointY]), diff --git a/packages/element/src/bounds.ts b/packages/element/src/bounds.ts index a73a7d28bb..a072b81a90 100644 --- a/packages/element/src/bounds.ts +++ b/packages/element/src/bounds.ts @@ -680,8 +680,9 @@ export const getMinMaxXYFromCurvePathOps = ( return [minX, minY, maxX, maxY]; }; -export const getBoundsFromPoints = ( - points: ExcalidrawFreeDrawElement["points"], +export const getBoundsFromPoints =

( + points: readonly P[], + padding: number = 0, ): Bounds => { let minX = Infinity; let minY = Infinity; @@ -695,7 +696,7 @@ export const getBoundsFromPoints = ( maxY = Math.max(maxY, y); } - return [minX, minY, maxX, maxY]; + return [minX - padding, minY - padding, maxX + padding, maxY + padding]; }; const getFreeDrawElementAbsoluteCoords = ( @@ -1261,6 +1262,17 @@ export const pointInsideBounds =

( ): boolean => p[0] > bounds[0] && p[0] < bounds[2] && p[1] > bounds[1] && p[1] < bounds[3]; +// TODO make pointInsideBounds inclusive and remove this function once we +// test nothing is breaking +export const pointInsideBoundsInclusive =

( + p: P, + bounds: Bounds, +): boolean => + p[0] >= bounds[0] && + p[0] <= bounds[2] && + p[1] >= bounds[1] && + p[1] <= bounds[3]; + export const doBoundsIntersect = ( bounds1: Bounds | null, bounds2: Bounds | null, @@ -1275,13 +1287,21 @@ export const doBoundsIntersect = ( return minX1 < maxX2 && maxX1 > minX2 && minY1 < maxY2 && maxY1 > minY2; }; +export const boundsContainBounds = (outerBounds: Bounds, innerBounds: Bounds) => + [ + pointFrom(innerBounds[0], innerBounds[1]), + pointFrom(innerBounds[0], innerBounds[3]), + pointFrom(innerBounds[2], innerBounds[1]), + pointFrom(innerBounds[2], innerBounds[3]), + ].every((point) => pointInsideBoundsInclusive(point, outerBounds)); + export const elementCenterPoint = ( element: ExcalidrawElement, elementsMap: ElementsMap, xOffset: number = 0, yOffset: number = 0, ) => { - if (isLinearElement(element)) { + if (isLinearElement(element) || isFreeDrawElement(element)) { const [x1, y1, x2, y2] = getElementAbsoluteCoords(element, elementsMap); const [x, y] = pointFrom((x1 + x2) / 2, (y1 + y2) / 2); diff --git a/packages/element/src/collision.ts b/packages/element/src/collision.ts index b17d563dbe..6999512b44 100644 --- a/packages/element/src/collision.ts +++ b/packages/element/src/collision.ts @@ -61,6 +61,8 @@ import { distanceToElement } from "./distance"; import { getBindingGap } from "./binding"; +import { hasBackground } from "./comparisons"; + import type { ElementsMap, ExcalidrawArrowElement, @@ -83,7 +85,7 @@ export const shouldTestInside = (element: ExcalidrawElement) => { } const isDraggableFromInside = - !isTransparent(element.backgroundColor) || + (hasBackground(element.type) && !isTransparent(element.backgroundColor)) || hasBoundTextElement(element) || isIframeLikeElement(element) || isTextElement(element); @@ -154,14 +156,11 @@ export const hitElementItself = ({ // Hit test against the extended, rotated bounding box of the element first const bounds = getElementBounds(element, elementsMap, true); - const hitBounds = isPointWithinBounds( - pointFrom(bounds[0] - threshold, bounds[1] - threshold), - pointRotateRads( - point, - getCenterForBounds(bounds), - -element.angle as Radians, - ), - pointFrom(bounds[2] + threshold, bounds[3] + threshold), + const hitBounds = isPointInRotatedBounds( + point, + bounds, + element.angle, + threshold, ); // PERF: Bail out early if the point is not even in the @@ -192,18 +191,32 @@ export const hitElementItself = ({ return result; }; +const isPointInRotatedBounds = ( + point: GlobalPoint, + bounds: Bounds, + angle: Radians, + tolerance = 0, +) => { + const adjustedPoint = + angle === 0 + ? point + : pointRotateRads(point, getCenterForBounds(bounds), -angle as Radians); + + return isPointWithinBounds( + pointFrom(bounds[0] - tolerance, bounds[1] - tolerance), + adjustedPoint, + pointFrom(bounds[2] + tolerance, bounds[3] + tolerance), + ); +}; + export const hitElementBoundingBox = ( point: GlobalPoint, element: ExcalidrawElement, elementsMap: ElementsMap, tolerance = 0, ) => { - let [x1, y1, x2, y2] = getElementBounds(element, elementsMap); - x1 -= tolerance; - y1 -= tolerance; - x2 += tolerance; - y2 += tolerance; - return isPointWithinBounds(pointFrom(x1, y1), point, pointFrom(x2, y2)); + const bounds = getElementBounds(element, elementsMap, true); + return isPointInRotatedBounds(point, bounds, element.angle, tolerance); }; export const hitElementBoundingBoxOnly = ( @@ -313,7 +326,10 @@ export const getAllHoveredElementAtPoint = ( ) { candidateElements.push(element); - if (!isTransparent(element.backgroundColor)) { + if ( + hasBackground(element.type) && + !isTransparent(element.backgroundColor) + ) { break; } } @@ -573,7 +589,9 @@ const intersectLinearOrFreeDrawWithLineSegment = ( continue; } - const hits = curveIntersectLineSegment(c, segment); + const hits = curveIntersectLineSegment(c, segment, { + iterLimit: 10, + }); if (hits.length > 0) { intersections.push(...hits); diff --git a/packages/element/src/duplicate.ts b/packages/element/src/duplicate.ts index c2cee4c089..24135c0879 100644 --- a/packages/element/src/duplicate.ts +++ b/packages/element/src/duplicate.ts @@ -111,6 +111,9 @@ export const duplicateElements = ( * user interaction. */ type: "everything"; + // TODO remove/review this once we add frame children order migration + // and invariant checks + preserveFrameChildrenOrder?: boolean; } | { /** @@ -170,6 +173,8 @@ export const duplicateElements = ( opts.type === "in-place" ? opts.idsOfElementsToDuplicate : new Map(elements.map((el) => [el.id, el])); + const preserveFrameChildrenOrder = + opts.type === "everything" && opts.preserveFrameChildrenOrder; // For sanity if (opts.type === "in-place") { @@ -250,6 +255,9 @@ export const duplicateElements = ( elementsWithDuplicates.splice(index + 1, 0, ...castArray(elements)); }; + // main + // --------------------------------------------------------------------------- + const frameIdsToDuplicate = new Set( elements .filter( @@ -274,7 +282,7 @@ export const duplicateElements = ( if (groupId) { const groupElements = getElementsInGroup(elements, groupId).flatMap( (element) => - isFrameLikeElement(element) + isFrameLikeElement(element) && !preserveFrameChildrenOrder ? [...getFrameChildren(elements, element.id), element] : [element], ); @@ -290,13 +298,25 @@ export const duplicateElements = ( // frame duplication // ------------------------------------------------------------------------- - if (element.frameId && frameIdsToDuplicate.has(element.frameId)) { + if ( + !preserveFrameChildrenOrder && + element.frameId && + frameIdsToDuplicate.has(element.frameId) + ) { continue; } if (isFrameLikeElement(element)) { const frameId = element.id; + if (preserveFrameChildrenOrder) { + insertBeforeOrAfterIndex( + findLastIndex(elementsWithDuplicates, (el) => el.id === frameId), + copyElements(element), + ); + continue; + } + const frameChildren = getFrameChildren(elements, frameId); const targetIndex = findLastIndex(elementsWithDuplicates, (el) => { diff --git a/packages/element/src/elbowArrow.ts b/packages/element/src/elbowArrow.ts index 9543b4182f..024b846b12 100644 --- a/packages/element/src/elbowArrow.ts +++ b/packages/element/src/elbowArrow.ts @@ -2124,8 +2124,8 @@ const normalizeArrowElementUpdate = ( offsetY < -MAX_POS || offsetY > MAX_POS || offsetX + points[points.length - 1][0] < -MAX_POS || - offsetY + points[points.length - 1][0] > MAX_POS || - offsetX + points[points.length - 1][1] < -MAX_POS || + offsetX + points[points.length - 1][0] > MAX_POS || + offsetY + points[points.length - 1][1] < -MAX_POS || offsetY + points[points.length - 1][1] > MAX_POS ) { console.error( diff --git a/packages/element/src/fractionalIndex.ts b/packages/element/src/fractionalIndex.ts index 44ca523c80..90a2e7c217 100644 --- a/packages/element/src/fractionalIndex.ts +++ b/packages/element/src/fractionalIndex.ts @@ -1,7 +1,10 @@ -import { generateNKeysBetween } from "fractional-indexing"; - import { arrayToMap } from "@excalidraw/common"; +import { + validateOrderKey, + generateNKeysBetween, +} from "@excalidraw/fractional-indexing"; + import { mutateElement, newElementWith } from "./mutateElement"; import { getBoundTextElement } from "./textElement"; import { hasBoundTextElement } from "./typeChecks"; @@ -382,6 +385,13 @@ const isValidFractionalIndex = ( return false; } + try { + // Format validation + validateOrderKey(index); + } catch { + return false; + } + if (predecessor && successor) { return predecessor < index && index < successor; } diff --git a/packages/element/src/frame.ts b/packages/element/src/frame.ts index 3c82099546..3d1449a072 100644 --- a/packages/element/src/frame.ts +++ b/packages/element/src/frame.ts @@ -1,7 +1,6 @@ import { arrayToMap } from "@excalidraw/common"; import { isPointWithinBounds, pointFrom } from "@excalidraw/math"; import { doLineSegmentsIntersect } from "@excalidraw/utils/bbox"; -import { elementsOverlappingBBox } from "@excalidraw/utils/withinBounds"; import type { AppClassProperties, @@ -18,9 +17,13 @@ import { getElementLineSegments, getCommonBounds, getElementAbsoluteCoords, + doBoundsIntersect, + getElementBounds, + boundsContainBounds, } from "./bounds"; import { mutateElement } from "./mutateElement"; import { getBoundTextElement, getContainerElement } from "./textElement"; +import { syncMovedIndices } from "./fractionalIndex"; import { isFrameElement, isFrameLikeElement, @@ -100,8 +103,9 @@ export const isElementContainingFrame = ( frame: ExcalidrawFrameLikeElement, elementsMap: ElementsMap, ) => { - return getElementsWithinSelection([frame], element, elementsMap).some( - (e) => e.id === frame.id, + return boundsContainBounds( + getElementBounds(element, elementsMap), + getElementBounds(frame, elementsMap), ); }; @@ -488,10 +492,44 @@ export const filterElementsEligibleAsFrameChildren = ( return eligibleElements; }; +export const getCommonFrameId = (elements: readonly ExcalidrawElement[]) => { + let commonFrameId: ExcalidrawElement["frameId"] | undefined; + + for (const element of elements) { + if (isFrameLikeElement(element) || !element.frameId) { + return null; + } + + if (commonFrameId === undefined) { + commonFrameId = element.frameId; + } else if (commonFrameId !== element.frameId) { + return null; + } + } + + return commonFrameId ?? null; +}; + +export const getFrameChildrenInsertionIndex = ( + elements: readonly ExcalidrawElement[], + frameId: ExcalidrawFrameLikeElement["id"], +): number | null => { + for (let index = elements.length - 1; index >= 0; index--) { + const element = elements[index]; + + if (element.id === frameId) { + return index; + } else if (element.frameId === frameId) { + return index + 1; + } + } + + return null; +}; + /** - * Retains (or repairs for target frame) the ordering invriant where children - * elements come right before the parent frame: - * [el, el, child, child, frame, el] + * Adds elements and their bound elements to frame. Reorders added elements to + * be just below frame, or just above its highest child (whichever is higher). * * @returns mutated allElements (same data structure) */ @@ -499,19 +537,11 @@ export const addElementsToFrame = ( allElements: T, elementsToAdd: NonDeletedExcalidrawElement[], frame: ExcalidrawFrameLikeElement, - appState: AppState, ): T => { const elementsMap = arrayToMap(allElements); - const currTargetFrameChildrenMap = new Map(); - for (const element of allElements.values()) { - if (element.frameId === frame.id) { - currTargetFrameChildrenMap.set(element.id, true); - } - } + const commonFrameId = getCommonFrameId(elementsToAdd); - const suppliedElementsToAddSet = new Set(elementsToAdd.map((el) => el.id)); - - const finalElementsToAdd: ExcalidrawElement[] = []; + const finalElementsToAdd = new Set(); const otherFrames = new Set(); @@ -522,7 +552,8 @@ export const addElementsToFrame = ( } // - add bound text elements if not already in the array - // - filter out elements that are already in the frame + // - keep elements already in the frame so mixed selections can be reordered + // together for (const element of omitGroupsContainingFrameLikes( allElements, elementsToAdd, @@ -535,38 +566,68 @@ export const addElementsToFrame = ( continue; } - // if the element is already in another frame (which is also in elementsToAdd), - // it means that frame and children are selected at the same time - // => keep original frame membership, do not add to the target frame - if ( - element.frameId && - appState.selectedElementIds[element.id] && - appState.selectedElementIds[element.frameId] - ) { + if (element.frameId && element.frameId !== frame.id) { continue; } - if (!currTargetFrameChildrenMap.has(element.id)) { - finalElementsToAdd.push(element); - } + finalElementsToAdd.add(element); const boundTextElement = getBoundTextElement(element, elementsMap); - if ( - boundTextElement && - !suppliedElementsToAddSet.has(boundTextElement.id) && - !currTargetFrameChildrenMap.has(boundTextElement.id) - ) { - finalElementsToAdd.push(boundTextElement); + if (boundTextElement && !finalElementsToAdd.has(boundTextElement)) { + finalElementsToAdd.add(boundTextElement); } } for (const element of finalElementsToAdd) { - mutateElement(element, elementsMap, { - frameId: frame.id, - }); + // we don't always need to update the element if it's already in the frame, + // but we still need to accumulate in finalElementsToAdd so we potentially + // reorder them if added together + if (element.frameId !== frame.id) { + mutateElement(element, elementsMap, { + frameId: frame.id, + }); + } } - return allElements; + // (re)order elements to be just below the frame, + // or just above the highest child if that is higher + // (latter case is denormalized order until we migrate) + // --------------------------------------------------------------------------- + + if ( + !finalElementsToAdd.size || + // if all elements to add already belong to the frame, then we don't want to + // reorder (case: we're dragging element children within the frame) + commonFrameId === frame.id + ) { + return allElements; + } + + const otherElements = Array.from(allElements.values()).filter( + (element) => !finalElementsToAdd.has(element), + ); + const insertionIndex = getFrameChildrenInsertionIndex( + otherElements, + frame.id, + ); + + if (insertionIndex === null) { + return allElements; + } + + const reorderedElements = [ + ...otherElements.slice(0, insertionIndex), + ...finalElementsToAdd, + ...otherElements.slice(insertionIndex), + ]; + + syncMovedIndices(reorderedElements, arrayToMap([...finalElementsToAdd])); + + return ( + Array.isArray(allElements) + ? reorderedElements + : new Map(reorderedElements.map((element) => [element.id, element])) + ) as T; }; export const removeElementsFromFrame = ( @@ -620,13 +681,11 @@ export const replaceAllElementsInFrame = ( allElements: readonly T[], nextElementsInFrame: ExcalidrawElement[], frame: ExcalidrawFrameLikeElement, - app: AppClassProperties, ): T[] => { return addElementsToFrame( removeAllElementsFromFrame(allElements, frame), nextElementsInFrame, frame, - app.state, ).slice(); }; @@ -920,16 +979,17 @@ export const getFrameLikeTitle = (element: ExcalidrawFrameLikeElement) => { export const getElementsOverlappingFrame = ( elements: readonly ExcalidrawElement[], frame: ExcalidrawFrameLikeElement, + elementsMap: ElementsMap, ) => { - return ( - elementsOverlappingBBox({ - elements, - bounds: frame, - type: "overlap", - }) - // removes elements who are overlapping, but are in a different frame, + return elements.filter( + (el) => + // exclude elements which are overlapping, but are in a different frame, // and thus invisible in target frame - .filter((el) => !el.frameId || el.frameId === frame.id) + (!el.frameId || el.frameId === frame.id) && + doBoundsIntersect( + getElementBounds(el, elementsMap), + getElementBounds(frame, elementsMap), + ), ); }; diff --git a/packages/element/src/linearElementEditor.ts b/packages/element/src/linearElementEditor.ts index 8d237b5b40..9e17fc1ded 100644 --- a/packages/element/src/linearElementEditor.ts +++ b/packages/element/src/linearElementEditor.ts @@ -488,7 +488,7 @@ export class LinearElementEditor { selectedPointsIndices, )}) points(0..${ element.points.length - 1 - }) lastClickedPoint(${lastClickedPoint})`, + }) lastClickedPoint(${lastClickedPoint}) isElbowArrow: ${elbowed}`, ); // Fall back to the actual last point as a last resort. @@ -2146,13 +2146,13 @@ const pointDraggingUpdates = ( } => { const naiveDraggingPoints = new Map( selectedPointsIndices.map((pointIndex) => { + // NOTE: Avoid stale point index issue potentially caused by elbow + // arrows unpredictably changing the number of points during dragging + const point = element.points[pointIndex] ?? element.points.at(-1); return [ pointIndex, { - point: pointFrom( - element.points[pointIndex][0] + deltaX, - element.points[pointIndex][1] + deltaY, - ), + point: pointFrom(point[0] + deltaX, point[1] + deltaY), isDragging: true, }, ]; diff --git a/packages/element/src/selection.ts b/packages/element/src/selection.ts index ea7fdb1b77..a65d1d3ce3 100644 --- a/packages/element/src/selection.ts +++ b/packages/element/src/selection.ts @@ -1,15 +1,32 @@ -import { arrayToMap, isShallowEqual } from "@excalidraw/common"; +import { arrayToMap, isShallowEqual, type Bounds } from "@excalidraw/common"; +import { + lineSegment, + pointFrom, + pointRotateRads, + type GlobalPoint, +} from "@excalidraw/math"; import type { AppState, + BoxSelectionMode, InteractiveCanvasAppState, } from "@excalidraw/excalidraw/types"; -import { getElementAbsoluteCoords, getElementBounds } from "./bounds"; +import { + boundsContainBounds, + doBoundsIntersect, + elementCenterPoint, + getElementAbsoluteCoords, + getElementBounds, + pointInsideBounds, +} from "./bounds"; +import { intersectElementWithLineSegment } from "./collision"; import { isElementInViewport } from "./sizeHelpers"; import { + isArrowElement, isBoundToContainer, isFrameLikeElement, + isFreeDrawElement, isLinearElement, isTextElement, } from "./typeChecks"; @@ -21,15 +38,33 @@ import { import { LinearElementEditor } from "./linearElementEditor"; import { selectGroupsForSelectedElements } from "./groups"; +import { getBoundTextElement } from "./textElement"; import type { ElementsMap, ElementsMapOrArray, ExcalidrawElement, + ExcalidrawFrameLikeElement, NonDeleted, NonDeletedExcalidrawElement, } from "./types"; +const shouldIgnoreElementFromSelection = ( + element: NonDeletedExcalidrawElement, +) => element.locked || isBoundToContainer(element); + +const excludeElementsFromFrames = ( + selectedElements: readonly T[], + framesInSelection: Set, +) => { + return selectedElements.filter((element) => { + if (element.frameId && framesInSelection.has(element.frameId)) { + return false; + } + return true; + }); +}; + /** * Frames and their containing elements are not to be selected at the same time. * Given an array of selected elements, if there are frames and their containing elements @@ -49,68 +84,286 @@ export const excludeElementsInFramesFromSelection = < } }); - return selectedElements.filter((element) => { - if (element.frameId && framesInSelection.has(element.frameId)) { - return false; - } - return true; - }); + return excludeElementsFromFrames(selectedElements, framesInSelection); }; export const getElementsWithinSelection = ( elements: readonly NonDeletedExcalidrawElement[], selection: NonDeletedExcalidrawElement, elementsMap: ElementsMap, + // TODO remove (this flag is effectively unused AFAIK) excludeElementsInFrames: boolean = true, -) => { - const [selectionX1, selectionY1, selectionX2, selectionY2] = + boxSelectionMode: BoxSelectionMode = "contain", +): NonDeletedExcalidrawElement[] => { + const [selectionStartX, selectionStartY, selectionEndX, selectionEndY] = getElementAbsoluteCoords(selection, elementsMap); + const selectionX1 = Math.min(selectionStartX, selectionEndX); + const selectionY1 = Math.min(selectionStartY, selectionEndY); + const selectionX2 = Math.max(selectionStartX, selectionEndX); + const selectionY2 = Math.max(selectionStartY, selectionEndY); + const selectionBounds = [ + selectionX1, + selectionY1, + selectionX2, + selectionY2, + ] as Bounds; + const selectionEdges = [ + lineSegment( + pointFrom(selectionX1, selectionY1), + pointFrom(selectionX2, selectionY1), + ), + lineSegment( + pointFrom(selectionX2, selectionY1), + pointFrom(selectionX2, selectionY2), + ), + lineSegment( + pointFrom(selectionX2, selectionY2), + pointFrom(selectionX1, selectionY2), + ), + lineSegment( + pointFrom(selectionX1, selectionY2), + pointFrom(selectionX1, selectionY1), + ), + ]; - let elementsInSelection = elements.filter((element) => { - let [elementX1, elementY1, elementX2, elementY2] = getElementBounds( - element, - elementsMap, - ); + const framesInSelection = excludeElementsInFrames + ? new Set() + : null; + const groups: Record = {}; + const elementsInSelection: Set = new Set(); - const containingFrame = getContainingFrame(element, elementsMap); - if (containingFrame) { - const [fx1, fy1, fx2, fy2] = getElementBounds( - containingFrame, + for (const element of elements) { + if (shouldIgnoreElementFromSelection(element)) { + continue; + } + + // Track only selectable top-level group members, so ignored elements such + // as bound text and locked elements don't affect group selection. + const groupId = element.groupIds.at(-1); + if (groupId) { + if (!groups[groupId]) { + groups[groupId] = []; + } + groups[groupId].push(element); + } + + const strokeWidth = element.strokeWidth; + let labelAABB: Bounds | null = null; + let elementAABB = getElementBounds(element, elementsMap); + + elementAABB = [ + elementAABB[0] - strokeWidth / 2, + elementAABB[1] - strokeWidth / 2, + elementAABB[2] + strokeWidth / 2, + elementAABB[3] + strokeWidth / 2, + ] as Bounds; + + // Whether the element bounds should include the bound text element bounds + const boundTextElement = + isArrowElement(element) && getBoundTextElement(element, elementsMap); + if (boundTextElement) { + const { x, y } = LinearElementEditor.getBoundTextElementPosition( + element, + boundTextElement, elementsMap, ); - - elementX1 = Math.max(fx1, elementX1); - elementY1 = Math.max(fy1, elementY1); - elementX2 = Math.min(fx2, elementX2); - elementY2 = Math.min(fy2, elementY2); + labelAABB = [ + x, + y, + x + boundTextElement.width, + y + boundTextElement.height, + ] as Bounds; } - return ( - element.locked === false && - element.type !== "selection" && - !isBoundToContainer(element) && - selectionX1 <= elementX1 && - selectionY1 <= elementY1 && - selectionX2 >= elementX2 && - selectionY2 >= elementY2 - ); - }); + // Clip element bounds by its containing frame (if any), since only the + // visible (frame-clipped) portion of the element is relevant for selection. + const associatedFrame = getContainingFrame(element, elementsMap); + if ( + associatedFrame && + elementOverlapsWithFrame(element, associatedFrame, elementsMap) + ) { + const frameAABB = getElementBounds(associatedFrame, elementsMap); + elementAABB = [ + Math.max(elementAABB[0], frameAABB[0]), + Math.max(elementAABB[1], frameAABB[1]), + Math.min(elementAABB[2], frameAABB[2]), + Math.min(elementAABB[3], frameAABB[3]), + ] as Bounds; - elementsInSelection = excludeElementsInFrames - ? excludeElementsInFramesFromSelection(elementsInSelection) - : elementsInSelection; - - elementsInSelection = elementsInSelection.filter((element) => { - const containingFrame = getContainingFrame(element, elementsMap); - - if (containingFrame) { - return elementOverlapsWithFrame(element, containingFrame, elementsMap); + labelAABB = labelAABB + ? ([ + Math.max(labelAABB[0], frameAABB[0]), + Math.max(labelAABB[1], frameAABB[1]), + Math.min(labelAABB[2], frameAABB[2]), + Math.min(labelAABB[3], frameAABB[3]), + ] as Bounds) + : null; } - return true; - }); + const commonAABB = labelAABB + ? ([ + Math.min(labelAABB[0], elementAABB[0]), + Math.min(labelAABB[1], elementAABB[1]), + Math.max(labelAABB[2], elementAABB[2]), + Math.max(labelAABB[3], elementAABB[3]), + ] as Bounds) + : elementAABB; - return elementsInSelection; + // ============== Evaluation ============== + + // 1. If the selection box WRAPs the element's AABB, then add it to the + // selection and move on, regardless of the selection mode. + // + // PERF: This trick only works with axis-aligned box selection and the + // current convex element shapes! + if (boundsContainBounds(selectionBounds, commonAABB)) { + if (framesInSelection && isFrameLikeElement(element)) { + framesInSelection.add(element.id); + } + elementsInSelection.add(element); + continue; + } + + // 2. Handle the case where the label is overlapped by the selection box + if ( + boxSelectionMode === "overlap" && + labelAABB && + doBoundsIntersect(selectionBounds, labelAABB) + ) { + elementsInSelection.add(element); + continue; + } + + // 3. Handle the case where the selection is not wrapping the element, but + // it does intersect the element's outline (non-AABB). + if ( + boxSelectionMode === "overlap" && + doBoundsIntersect(selectionBounds, elementAABB) + ) { + let hasIntersection = false; + + // Preliminary check potential intersection imprecision + if (isLinearElement(element) || isFreeDrawElement(element)) { + const center = elementCenterPoint(element, elementsMap); + hasIntersection = element.points.some((point) => { + const rotatedPoint = pointRotateRads( + pointFrom(element.x + point[0], element.y + point[1]), + center, + element.angle, + ); + + return pointInsideBounds(rotatedPoint, selectionBounds); + }); + } else { + const nonRotatedElementBounds = getElementBounds( + element, + elementsMap, + true, + ); + const center = elementCenterPoint(element, elementsMap); + hasIntersection = [ + pointRotateRads( + pointFrom( + (nonRotatedElementBounds[0] + nonRotatedElementBounds[2]) / 2, + nonRotatedElementBounds[1], + ), + center, + element.angle, + ), + pointRotateRads( + pointFrom( + nonRotatedElementBounds[2], + (nonRotatedElementBounds[1] + nonRotatedElementBounds[3]) / 2, + ), + center, + element.angle, + ), + pointRotateRads( + pointFrom( + (nonRotatedElementBounds[0] + nonRotatedElementBounds[2]) / 2, + nonRotatedElementBounds[3], + ), + center, + element.angle, + ), + pointRotateRads( + pointFrom( + nonRotatedElementBounds[0], + (nonRotatedElementBounds[1] + nonRotatedElementBounds[3]) / 2, + ), + center, + element.angle, + ), + ].some((point) => { + return pointInsideBounds( + pointRotateRads(point, center, element.angle), + selectionBounds, + ); + }); + } + + if (!hasIntersection) { + hasIntersection = selectionEdges.some( + (selectionEdge) => + intersectElementWithLineSegment( + element, + elementsMap, + selectionEdge, + strokeWidth / 2, + true, // Stop at first hit for better performance + ).length > 0, + ); + } + + if (hasIntersection) { + if (framesInSelection && isFrameLikeElement(element)) { + framesInSelection.add(element.id); + } + + elementsInSelection.add(element); + continue; + } + } + + // 4. We don't need to handle when the selection is inside the element + // as it is separately handled in App. + } + + if (framesInSelection) { + elementsInSelection.forEach((element) => { + if (element.frameId && framesInSelection.has(element.frameId)) { + elementsInSelection.delete(element); + } + }); + } + + if (boxSelectionMode === "overlap") { + Array.from(elementsInSelection).forEach((element) => { + const groupId = element.groupIds.at(-1); + const group = groupId ? groups[groupId] : null; + + group?.forEach((groupElement) => elementsInSelection.add(groupElement)); + }); + } else if (boxSelectionMode === "contain") { + elementsInSelection.forEach((element) => { + // note: currently we only support top-level group handling since + // we don't support box selecting while editing the group/subgroup + // see https://github.com/excalidraw/excalidraw/pull/11234#issuecomment-4387654451 + const groupId = element.groupIds.at(-1); + + const group = groupId ? groups[groupId] : null; + + if ( + group && + !group.every((groupElement) => elementsInSelection.has(groupElement)) + ) { + elementsInSelection.delete(element); + } + }); + } + + // to maintain original order elements (namely for group selection) + return elements.filter((element) => elementsInSelection.has(element)); }; export const getVisibleAndNonSelectedElements = ( diff --git a/packages/element/src/sortElements.ts b/packages/element/src/sortElements.ts index c98ff9d523..0f9e8da0f1 100644 --- a/packages/element/src/sortElements.ts +++ b/packages/element/src/sortElements.ts @@ -1,59 +1,56 @@ -import { arrayToMapWithIndex } from "@excalidraw/common"; +import { arrayToMap } from "@excalidraw/common"; import type { ExcalidrawElement } from "./types"; -const normalizeGroupElementOrder = (elements: readonly ExcalidrawElement[]) => { - const origElements: ExcalidrawElement[] = elements.slice(); - const sortedElements = new Set(); - - const orderInnerGroups = ( - elements: readonly ExcalidrawElement[], - ): ExcalidrawElement[] => { - const firstGroupSig = elements[0]?.groupIds?.join(""); - const aGroup: ExcalidrawElement[] = [elements[0]]; - const bGroup: ExcalidrawElement[] = []; - for (const element of elements.slice(1)) { - if (element.groupIds?.join("") === firstGroupSig) { - aGroup.push(element); - } else { - bGroup.push(element); - } - } - return bGroup.length ? [...aGroup, ...orderInnerGroups(bGroup)] : aGroup; +const defragmentGroups = (elements: readonly ExcalidrawElement[]) => { + const groupIdAtLevel = (element: ExcalidrawElement, level: number) => { + return element.groupIds[element.groupIds.length - level - 1]; }; - const groupHandledElements = new Map(); + const orderLevel = ( + levelElements: readonly ExcalidrawElement[], + level: number, + ): ExcalidrawElement[] => { + const buckets = new Map(); + // Slots preserve first-occurrence order: a groupId reserves its slot + // the first time one of its members is seen; loose elements occupy + // their own slot. Groups are then expanded (and recursed into) in place. + const slots: (ExcalidrawElement | string)[] = []; - origElements.forEach((element, idx) => { - if (groupHandledElements.has(element.id)) { - return; - } - if (element.groupIds?.length) { - const topGroup = element.groupIds[element.groupIds.length - 1]; - const groupElements = origElements.slice(idx).filter((element) => { - const ret = element?.groupIds?.some((id) => id === topGroup); - if (ret) { - groupHandledElements.set(element!.id, true); - } - return ret; - }); - - for (const elem of orderInnerGroups(groupElements)) { - sortedElements.add(elem); + for (const element of levelElements) { + const groupId = groupIdAtLevel(element, level); + if (groupId === undefined) { + slots.push(element); + continue; } - } else { - sortedElements.add(element); + let bucket = buckets.get(groupId); + if (!bucket) { + bucket = []; + buckets.set(groupId, bucket); + slots.push(groupId); + } + bucket.push(element); } - }); + + return slots.flatMap((slot) => + typeof slot === "string" + ? orderLevel(buckets.get(slot)!, level + 1) + : [slot], + ); + }; + + // `groupIds` is stored innermost-first, so the outermost group is the + // last entry. We recurse from level 0 (outermost) inward. + const sortedElements = orderLevel(elements, 0); // if there's a bug which resulted in losing some of the elements, return // original instead as that's better than losing data - if (sortedElements.size !== elements.length) { - console.error("normalizeGroupElementOrder: lost some elements... bailing!"); + if (sortedElements.length !== elements.length) { + console.error("defragmentGroups: lost some elements... bailing!"); return elements; } - return [...sortedElements]; + return sortedElements; }; /** @@ -68,39 +65,40 @@ const normalizeGroupElementOrder = (elements: readonly ExcalidrawElement[]) => { const normalizeBoundElementsOrder = ( elements: readonly ExcalidrawElement[], ) => { - const elementsMap = arrayToMapWithIndex(elements); + const elementsMap = arrayToMap(elements); - const origElements: (ExcalidrawElement | null)[] = elements.slice(); const sortedElements = new Set(); - origElements.forEach((element, idx) => { - if (!element) { - return; + for (const element of elements) { + if (sortedElements.has(element)) { + continue; } + if (element.boundElements?.length) { sortedElements.add(element); - origElements[idx] = null; - element.boundElements.forEach((boundElement) => { + for (const boundElement of element.boundElements) { const child = elementsMap.get(boundElement.id); if (child && boundElement.type === "text") { - sortedElements.add(child[0]); - origElements[child[1]] = null; + sortedElements.add(child); } - }); - } else if (element.type === "text" && element.containerId) { - const parent = elementsMap.get(element.containerId); - if (!parent?.[0].boundElements?.find((x) => x.id === element.id)) { - sortedElements.add(element); - origElements[idx] = null; - - // if element has a container and container lists it, skip this element - // as it'll be taken care of by the container } - } else { - sortedElements.add(element); - origElements[idx] = null; + continue; } - }); + + // if element has a container and container lists it, skip this element + // as it'll be taken care of by the container + if ( + element.type === "text" && + element.containerId && + elementsMap + .get(element.containerId) + ?.boundElements?.some((el) => el.id === element.id) + ) { + continue; + } + + sortedElements.add(element); + } // if there's a bug which resulted in losing some of the elements, return // original instead as that's better than losing data @@ -117,5 +115,5 @@ const normalizeBoundElementsOrder = ( export const normalizeElementOrder = ( elements: readonly ExcalidrawElement[], ) => { - return normalizeBoundElementsOrder(normalizeGroupElementOrder(elements)); + return normalizeBoundElementsOrder(defragmentGroups(elements)); }; diff --git a/packages/element/src/typeChecks.ts b/packages/element/src/typeChecks.ts index b609cc3f8a..3a8f5e36ef 100644 --- a/packages/element/src/typeChecks.ts +++ b/packages/element/src/typeChecks.ts @@ -392,3 +392,23 @@ export const canBecomePolygon = ( (points.length === 3 && !pointsEqual(points[0], points[points.length - 1])) ); }; + +export const isEligibleFrameChildType = (type: ElementOrToolType) => { + switch (type) { + case "rectangle": + case "diamond": + case "ellipse": + case "arrow": + case "line": + case "freedraw": + case "text": + case "image": + case "frame": + case "embeddable": { + return true; + } + default: { + return false; + } + } +}; diff --git a/packages/element/tests/collision.test.tsx b/packages/element/tests/collision.test.tsx index 4061a16cb6..a44f1f7bb0 100644 --- a/packages/element/tests/collision.test.tsx +++ b/packages/element/tests/collision.test.tsx @@ -1,4 +1,4 @@ -import { arrayToMap } from "@excalidraw/common"; +import { arrayToMap, reseed } from "@excalidraw/common"; import { type GlobalPoint, type LocalPoint, pointFrom } from "@excalidraw/math"; import { Excalidraw } from "@excalidraw/excalidraw"; import { API } from "@excalidraw/excalidraw/tests/helpers/api"; @@ -12,6 +12,7 @@ import { hitElementItself } from "../src/collision"; describe("check rotated elements can be hit:", () => { beforeEach(async () => { localStorage.clear(); + reseed(7); await render(); }); @@ -56,6 +57,7 @@ describe("hitElementItself cache", () => { }); localStorage.clear(); + reseed(7); await render(); }); diff --git a/packages/element/tests/fractionalIndex.test.ts b/packages/element/tests/fractionalIndex.test.ts index 1cc3ca5af3..2834a831e1 100644 --- a/packages/element/tests/fractionalIndex.test.ts +++ b/packages/element/tests/fractionalIndex.test.ts @@ -1,9 +1,8 @@ /* eslint-disable no-lone-blocks */ -import { generateKeyBetween } from "fractional-indexing"; - import { arrayToMap } from "@excalidraw/common"; import { + InvalidFractionalIndexError, syncInvalidIndices, syncMovedIndices, validateFractionalIndices, @@ -13,13 +12,34 @@ import { deepCopyElement } from "@excalidraw/element"; import { API } from "@excalidraw/excalidraw/tests/helpers/api"; +import { + generateKeyBetween, + validateOrderKey, +} from "@excalidraw/fractional-indexing"; + import type { ElementsMap, ExcalidrawElement, FractionalIndex, } from "@excalidraw/element/types"; -import { InvalidFractionalIndexError } from "../src/fractionalIndex"; +describe("fractional index format validation", () => { + it("should reject malformed base62 order keys", () => { + expect(() => validateOrderKey("a!")).toThrow(); + expect(() => validateOrderKey("a_")).toThrow(); + expect(() => validateOrderKey("a1!")).toThrow(); + expect(() => validateOrderKey("a1_")).toThrow(); + expect(() => validateOrderKey("zd0032")).toThrow(); + }); + + it("should accept valid base62 order keys", () => { + expect(() => validateOrderKey("Zz")).not.toThrow(); + expect(() => validateOrderKey("a0")).not.toThrow(); + expect(() => validateOrderKey("a1")).not.toThrow(); + expect(() => validateOrderKey("a1V")).not.toThrow(); + expect(() => validateOrderKey("z".padEnd(28, "z"))).not.toThrow(); + }); +}); describe("sync invalid indices with array order", () => { describe("should NOT sync empty array", () => { @@ -104,6 +124,46 @@ describe("sync invalid indices with array order", () => { }); }); + describe("should sync when fractional index is malformed", () => { + // "zd0032" has head "z" which requires length 28 per getIntegerLength, + // but the string is far too short, so validateOrderKey throws for it + testInvalidIndicesSync({ + elements: [{ id: "A", index: "zd0032" }], + expect: { + unchangedElements: [], + }, + }); + + testInvalidIndicesSync({ + elements: [ + { id: "A", index: "a1" }, + { id: "B", index: "zd0032" }, + { id: "C", index: "a3" }, + ], + expect: { + unchangedElements: ["A", "C"], + }, + }); + + testInvalidIndicesSync({ + elements: [{ id: "A", index: "a!" }], + expect: { + unchangedElements: [], + }, + }); + + testInvalidIndicesSync({ + elements: [ + { id: "A", index: "a1" }, + { id: "B", index: "a!" }, + { id: "C", index: "a2" }, + ], + expect: { + unchangedElements: ["A", "C"], + }, + }); + }); + describe("should sync when fractional indices are duplicated", () => { testInvalidIndicesSync({ elements: [ diff --git a/packages/element/tests/frame.test.tsx b/packages/element/tests/frame.test.tsx index 47f2160ac3..b419d00f02 100644 --- a/packages/element/tests/frame.test.tsx +++ b/packages/element/tests/frame.test.tsx @@ -2,15 +2,24 @@ import { convertToExcalidrawElements, Excalidraw, } from "@excalidraw/excalidraw"; +import { arrayToMap } from "@excalidraw/common"; import { API } from "@excalidraw/excalidraw/tests/helpers/api"; -import { Keyboard, Pointer } from "@excalidraw/excalidraw/tests/helpers/ui"; +import { Keyboard, Pointer, UI } from "@excalidraw/excalidraw/tests/helpers/ui"; +import { getTextEditor } from "@excalidraw/excalidraw/tests/queries/dom"; import { getCloneByOrigId, render, } from "@excalidraw/excalidraw/tests/test-utils"; -import type { ExcalidrawElement } from "../src/types"; +import { getSelectedElements } from "@excalidraw/excalidraw/scene"; + +import { elementOverlapsWithFrame } from "../src/frame"; + +import type { + ExcalidrawElement, + ExcalidrawFrameLikeElement, +} from "../src/types"; const { h } = window; const mouse = new Pointer("mouse"); @@ -125,6 +134,250 @@ describe("adding elements to frames", () => { }); }); + it("should treat an element fully containing a frame as overlapping the frame", () => { + const containingRect = API.createElement({ + type: "rectangle", + x: -50, + y: -50, + width: 250, + height: 250, + }); + + API.setElements([containingRect, frame]); + + expect( + elementOverlapsWithFrame( + containingRect, + frame as ExcalidrawFrameLikeElement, + arrayToMap(h.elements), + ), + ).toBe(true); + }); + + it("should not add a newly created element to a frame behind a non-frame element", () => { + const cover = API.createElement({ + id: "cover", + type: "rectangle", + x: 10, + y: 10, + width: 80, + height: 80, + backgroundColor: "#ffc9c9", + }); + + API.setElements([frame, cover]); + + UI.clickTool("rectangle"); + mouse.downAt(20, 20); + mouse.moveTo(40, 40); + mouse.upAt(40, 40); + + const createdElement = h.elements.find( + (element) => element.id !== frame.id && element.id !== cover.id, + ); + + expect(createdElement?.frameId).toBe(null); + expect(h.elements.map((element) => element.id)).toEqual([ + frame.id, + cover.id, + createdElement?.id, + ]); + }); + + it("should add a newly created element to a frame over a non-frame element", () => { + const cover = API.createElement({ + id: "cover", + type: "rectangle", + x: 10, + y: 10, + width: 80, + height: 80, + backgroundColor: "#ffc9c9", + }); + + API.setElements([cover, frame]); + + UI.clickTool("rectangle"); + mouse.downAt(20, 20); + mouse.moveTo(40, 40); + mouse.upAt(40, 40); + + const createdElement = h.elements.find( + (element) => element.id !== frame.id && element.id !== cover.id, + ); + + expect(createdElement?.frameId).toBe(frame.id); + }); + + it("should highlight the target frame while creating a new element", () => { + API.setElements([frame]); + + UI.clickTool("rectangle"); + mouse.downAt(20, 20); + mouse.moveTo(40, 40); + + expect(h.state.frameToHighlight?.id).toBe(frame.id); + + mouse.upAt(40, 40); + + expect(h.state.frameToHighlight).toBe(null); + }); + + it("should highlight the target frame while hovering with a creation tool", () => { + API.setElements([frame]); + + UI.clickTool("rectangle"); + mouse.moveTo(20, 20); + + expect(h.state.frameToHighlight?.id).toBe(frame.id); + + mouse.moveTo(200, 200); + + expect(h.state.frameToHighlight).toBe(null); + }); + + it("should not add grid-snapped text outside the frame to the clicked frame", async () => { + const offsetFrame = API.createElement({ + id: "offsetFrame", + type: "frame", + x: 10, + y: 0, + width: 150, + height: 150, + }); + + API.setElements([offsetFrame]); + API.setAppState({ + gridModeEnabled: true, + }); + + UI.clickTool("text"); + mouse.clickAt(12, 0); + + await getTextEditor(); + + const createdText = h.elements.find( + (element) => element.id !== offsetFrame.id, + ); + + expect(createdText?.x).toBe(0); + expect(createdText?.y).toBe(0); + expect(createdText?.frameId).toBe(null); + }); + + it("should add a newly created element to a frame behind another frame", () => { + const lockedFrame = API.createElement({ + id: "lockedFrame", + type: "frame", + x: 10, + y: 10, + width: 80, + height: 80, + locked: true, + }); + + API.setElements([frame, lockedFrame]); + + UI.clickTool("rectangle"); + mouse.downAt(20, 20); + mouse.moveTo(40, 40); + mouse.upAt(40, 40); + + const createdElement = h.elements.find( + (element) => element.id !== frame.id && element.id !== lockedFrame.id, + ); + + expect(createdElement?.frameId).toBe(frame.id); + }); + + it("should insert a newly created frame child just below its frame", () => { + const frameChildUnderCursor = API.createElement({ + id: "frameChildUnderCursor", + type: "rectangle", + x: 10, + y: 10, + width: 80, + height: 80, + backgroundColor: "#ffc9c9", + frameId: frame.id, + }); + const otherFrameChild = API.createElement({ + id: "otherFrameChild", + type: "rectangle", + x: 100, + y: 20, + width: 20, + height: 20, + frameId: frame.id, + }); + + API.setElements([frameChildUnderCursor, otherFrameChild, frame]); + + UI.clickTool("rectangle"); + mouse.downAt(20, 20); + mouse.moveTo(40, 40); + mouse.upAt(40, 40); + + const createdElement = h.elements.find( + (element) => + element.id !== frame.id && + element.id !== frameChildUnderCursor.id && + element.id !== otherFrameChild.id, + ); + + expect(createdElement?.frameId).toBe(frame.id); + expect(h.elements.map((element) => element.id)).toEqual([ + frameChildUnderCursor.id, + otherFrameChild.id, + createdElement?.id, + frame.id, + ]); + }); + + it("should insert a newly created frame child above the highest frame child", () => { + const frameChildUnderCursor = API.createElement({ + id: "frameChildUnderCursor", + type: "rectangle", + x: 10, + y: 10, + width: 80, + height: 80, + backgroundColor: "#ffc9c9", + frameId: frame.id, + }); + const otherFrameChild = API.createElement({ + id: "otherFrameChild", + type: "rectangle", + x: 100, + y: 20, + width: 20, + height: 20, + frameId: frame.id, + }); + + API.setElements([frame, frameChildUnderCursor, otherFrameChild]); + + UI.clickTool("rectangle"); + mouse.downAt(20, 20); + mouse.moveTo(40, 40); + mouse.upAt(40, 40); + + const createdElement = h.elements.find( + (element) => + element.id !== frame.id && + element.id !== frameChildUnderCursor.id && + element.id !== otherFrameChild.id, + ); + + expect(createdElement?.frameId).toBe(frame.id); + expect(h.elements.map((element) => element.id)).toEqual([ + frame.id, + frameChildUnderCursor.id, + otherFrameChild.id, + createdElement?.id, + ]); + }); + const commonTestCases = async ( func: typeof resizeFrameOverElement | typeof dragElementIntoFrame, ) => { @@ -415,6 +668,345 @@ describe("adding elements to frames", () => { describe("dragging elements into the frame", async () => { await commonTestCases(dragElementIntoFrame); + it("should add a dragged element fully containing the frame", () => { + const containingRect = API.createElement({ + type: "rectangle", + x: 220, + y: 20, + width: 300, + height: 300, + }); + + API.setElements([frame, containingRect]); + + dragElementIntoFrame(frame, containingRect); + + expect(API.getElement(containingRect).frameId).toBe(frame.id); + }); + + it("should drag an element into a frame", () => { + API.setElements([rect2, frame]); + + dragElementIntoFrame(frame, rect2); + + expect(rect2.frameId).toBe(frame.id); + }); + + it("should layer a dragged element above the highest frame child", () => { + const frameChild = API.createElement({ + id: "frameChild", + type: "rectangle", + x: 10, + y: 10, + width: 20, + height: 20, + frameId: frame.id, + }); + + API.setElements([frame, frameChild, rect2]); + + dragElementIntoFrame(frame, rect2); + + expect(rect2.frameId).toBe(frame.id); + expect(h.elements.map((element) => element.id)).toEqual([ + frame.id, + frameChild.id, + rect2.id, + ]); + expect(rect2.index! > frameChild.index!).toBe(true); + expect(rect2.index! > frame.index!).toBe(true); + }); + + it("should preview a dragged element above the highest frame child before pointerup", () => { + const frameChild = API.createElement({ + id: "frameChild", + type: "rectangle", + x: 10, + y: 10, + width: 20, + height: 20, + frameId: frame.id, + }); + + API.setElements([rect2, frame, frameChild]); + API.setSelectedElements([rect2]); + API.updateElement(rect2, { + x: 10, + y: 10, + }); + + const getRenderableElementIds = ( + selectedElementsAreBeingDragged: boolean, + ) => { + return h.app.renderer + .getRenderableElements({ + zoom: h.state.zoom, + offsetLeft: 0, + offsetTop: 0, + scrollX: 0, + scrollY: 0, + height: 1000, + width: 1000, + editingTextElement: h.state.editingTextElement, + newElement: h.state.newElement, + selectedElements: getSelectedElements(h.elements, h.state), + selectedElementsAreBeingDragged, + frameToHighlight: frame as ExcalidrawFrameLikeElement, + }) + .visibleElements.map((element) => element.id); + }; + + expect(h.elements.map((element) => element.id)).toEqual([ + rect2.id, + frame.id, + frameChild.id, + ]); + expect(getRenderableElementIds(false)).toEqual([ + rect2.id, + frame.id, + frameChild.id, + ]); + expect(getRenderableElementIds(true)).toEqual([ + frame.id, + frameChild.id, + rect2.id, + ]); + expect(h.elements.map((element) => element.id)).toEqual([ + rect2.id, + frame.id, + frameChild.id, + ]); + expect(rect2.frameId).toBe(null); + }); + + it("should not preview reorder dragged elements already in the highlighted frame", () => { + const frameChild = API.createElement({ + id: "frameChild", + type: "rectangle", + x: 10, + y: 10, + width: 20, + height: 20, + frameId: frame.id, + }); + const otherFrameChild = API.createElement({ + id: "otherFrameChild", + type: "rectangle", + x: 40, + y: 10, + width: 20, + height: 20, + frameId: frame.id, + }); + + API.setElements([frameChild, frame, otherFrameChild]); + API.setSelectedElements([frameChild]); + + const renderableElementIds = h.app.renderer + .getRenderableElements({ + zoom: h.state.zoom, + offsetLeft: 0, + offsetTop: 0, + scrollX: 0, + scrollY: 0, + height: 1000, + width: 1000, + editingTextElement: h.state.editingTextElement, + newElement: h.state.newElement, + selectedElements: getSelectedElements(h.elements, h.state), + selectedElementsAreBeingDragged: true, + frameToHighlight: frame as ExcalidrawFrameLikeElement, + }) + .visibleElements.map((element) => element.id); + + expect(renderableElementIds).toEqual([ + frameChild.id, + frame.id, + otherFrameChild.id, + ]); + }); + + it("should put a dragged mixed selection above the highest frame child", () => { + const frameChild = API.createElement({ + id: "frameChild", + type: "rectangle", + x: 50, + y: 10, + width: 20, + height: 20, + frameId: frame.id, + boundElements: [{ id: "boundText", type: "text" }], + }); + const boundText = API.createElement({ + id: "boundText", + type: "text", + x: 50, + y: 10, + width: 20, + height: 20, + containerId: frameChild.id, + frameId: frame.id, + }); + const otherFrameChild = API.createElement({ + id: "otherFrameChild", + type: "rectangle", + x: 80, + y: 10, + width: 20, + height: 20, + frameId: frame.id, + }); + const nonFrameElement = API.createElement({ + id: "nonFrameElement", + type: "rectangle", + x: 155, + y: 10, + width: 20, + height: 20, + }); + + API.setElements([ + frame, + frameChild, + boundText, + otherFrameChild, + nonFrameElement, + ]); + API.setSelectedElements([frameChild, nonFrameElement]); + + mouse.downAt( + nonFrameElement.x + nonFrameElement.width / 2, + nonFrameElement.y + nonFrameElement.height / 2, + ); + mouse.moveTo(frame.x + frame.width - 5, nonFrameElement.y + 10); + mouse.up(); + + expect(frameChild.frameId).toBe(frame.id); + expect(boundText.frameId).toBe(frame.id); + expect(nonFrameElement.frameId).toBe(frame.id); + expect(h.elements.map((element) => element.id)).toEqual([ + frame.id, + otherFrameChild.id, + frameChild.id, + boundText.id, + nonFrameElement.id, + ]); + }); + + it("should not reorder dragged elements already in the highlighted frame", () => { + const frameChild = API.createElement({ + id: "frameChild", + type: "rectangle", + x: 50, + y: 10, + width: 20, + height: 20, + frameId: frame.id, + }); + const otherFrameChild = API.createElement({ + id: "otherFrameChild", + type: "rectangle", + x: 80, + y: 10, + width: 20, + height: 20, + frameId: frame.id, + }); + + API.setElements([frame, frameChild, otherFrameChild]); + API.setSelectedElements([frameChild]); + + mouse.downAt( + frameChild.x + frameChild.width / 2, + frameChild.y + frameChild.height / 2, + ); + mouse.moveTo(frameChild.x + frameChild.width / 2 + 5, frameChild.y + 10); + mouse.up(); + + expect(frameChild.frameId).toBe(frame.id); + expect(h.elements.map((element) => element.id)).toEqual([ + frame.id, + frameChild.id, + otherFrameChild.id, + ]); + }); + + it("should not drag an element into a frame behind a non-frame element", () => { + const cover = API.createElement({ + id: "cover", + type: "rectangle", + x: 10, + y: 10, + width: 80, + height: 80, + backgroundColor: "#ffc9c9", + }); + API.setElements([frame, cover, rect2]); + + mouse.clickAt(rect2.x, rect2.y); + mouse.downAt(rect2.x + rect2.width / 2, rect2.y + rect2.height / 2); + mouse.moveTo(20, 20); + mouse.upAt(20, 20); + + expect(rect2.frameId).toBe(null); + }); + + it("should drag an element into a frame over a non-frame element", () => { + const cover = API.createElement({ + id: "cover", + type: "rectangle", + x: 10, + y: 10, + width: 80, + height: 80, + backgroundColor: "#ffc9c9", + }); + API.setElements([cover, rect2, frame]); + + mouse.clickAt(rect2.x, rect2.y); + mouse.downAt(rect2.x + rect2.width / 2, rect2.y + rect2.height / 2); + mouse.moveTo(20, 20); + mouse.upAt(20, 20); + + expect(rect2.frameId).toBe(frame.id); + }); + + it("should keep dragging a frame child over a non-frame element above its frame", () => { + const cover = API.createElement({ + id: "cover", + type: "rectangle", + x: 10, + y: 10, + width: 80, + height: 80, + backgroundColor: "#ffc9c9", + }); + const frameChild = API.createElement({ + id: "frameChild", + type: "rectangle", + x: 100, + y: 20, + width: 20, + height: 20, + frameId: frame.id, + }); + + API.setElements([frameChild, frame, cover]); + API.setSelectedElements([frameChild]); + + mouse.downAt( + frameChild.x + frameChild.width / 2, + frameChild.y + frameChild.height / 2, + ); + mouse.moveTo(20, 20); + + expect(h.state.frameToHighlight?.id).toBe(frame.id); + + mouse.upAt(20, 20); + + expect(frameChild.frameId).toBe(frame.id); + }); + it.skip("should drag element inside, duplicate it and keep it in frame", () => { API.setElements([frame, rect2]); diff --git a/packages/element/tests/sortElements.test.ts b/packages/element/tests/sortElements.test.ts index 0928b84f29..0554d38e35 100644 --- a/packages/element/tests/sortElements.test.ts +++ b/packages/element/tests/sortElements.test.ts @@ -326,19 +326,59 @@ describe("normalizeElementsOrder", () => { ]), [ "BA_rect1", + "CBA_rect3", + "CBA_rect7", "BA_rect5", "BA_rect6", "A_rect2", "A_rect5", - "CBA_rect3", - "CBA_rect7", "rect4", "X_rect8", - "X_rect11", "YX_rect10", + "X_rect11", "rect9", ], ); + assertOrder( + normalizeElementOrder([ + API.createElement({ + id: "A_rect1", + type: "rectangle", + groupIds: ["A"], + }), + API.createElement({ + id: "CBA_rect2", + type: "rectangle", + groupIds: ["C", "B", "A"], + }), + API.createElement({ + id: "A_rect3", + type: "rectangle", + groupIds: ["A"], + }), + ]), + ["A_rect1", "CBA_rect2", "A_rect3"], + ); + assertOrder( + normalizeElementOrder([ + API.createElement({ + id: "abcT_rect1", + type: "rectangle", + groupIds: ["ab", "c", "T"], + }), + API.createElement({ + id: "abcT_rect2", + type: "rectangle", + groupIds: ["a", "bc", "T"], + }), + API.createElement({ + id: "abcT_rect3", + type: "rectangle", + groupIds: ["ab", "c", "T"], + }), + ]), + ["abcT_rect1", "abcT_rect3", "abcT_rect2"], + ); }); // TODO diff --git a/packages/excalidraw/actions/actionDeselect.ts b/packages/excalidraw/actions/actionDeselect.ts new file mode 100644 index 0000000000..c9a92c7138 --- /dev/null +++ b/packages/excalidraw/actions/actionDeselect.ts @@ -0,0 +1,147 @@ +import { + getElementsInGroup, + isSomeElementSelected, + makeNextSelectedElementIds, + selectGroupsForSelectedElements, +} from "@excalidraw/element"; +import { CaptureUpdateAction } from "@excalidraw/element"; +import { KEYS, isWritableElement, updateActiveTool } from "@excalidraw/common"; + +import type { GroupId } from "@excalidraw/element/types"; + +import { register } from "./register"; + +import type { AppClassProperties, AppState } from "../types"; + +const getNextActiveTool = ( + appState: Readonly, + app: AppClassProperties, +) => { + if (appState.activeTool.type === "eraser") { + return updateActiveTool(appState, { + ...(appState.activeTool.lastActiveTool || { + type: app.state.preferredSelectionTool.type, + }), + lastActiveToolBeforeEraser: null, + }); + } + + return updateActiveTool(appState, { + type: app.state.preferredSelectionTool.type, + }); +}; + +const getParentEditingGroupId = ( + appState: Readonly, + app: AppClassProperties, + selectedElementIds: AppState["selectedElementIds"], +): GroupId | null => { + if (!appState.editingGroupId) { + return null; + } + + const nonDeletedElements = app.scene.getNonDeletedElements(); + const selectedElements = app.scene.getSelectedElements({ + selectedElementIds, + elements: nonDeletedElements, + }); + const candidateElements = selectedElements.length + ? selectedElements + : getElementsInGroup(nonDeletedElements, appState.editingGroupId); + + for (const element of candidateElements) { + const editingGroupIndex = element.groupIds.indexOf(appState.editingGroupId); + if (editingGroupIndex !== -1 && element.groupIds[editingGroupIndex + 1]) { + return element.groupIds[editingGroupIndex + 1] as GroupId; + } + } + + return null; +}; + +export const actionDeselect = register({ + name: "deselect", + label: "", + trackEvent: false, + perform: (_elements, appState, _, app) => { + const activeTool = getNextActiveTool(appState, app); + + if (appState.editingGroupId) { + const nonDeletedElements = app.scene.getNonDeletedElements(); + const selectedElementIds = + Object.keys(appState.selectedElementIds).length > 0 + ? appState.selectedElementIds + : getElementsInGroup( + nonDeletedElements, + appState.editingGroupId, + ).reduce((acc, element) => { + acc[element.id] = true; + return acc; + }, {} as Record); + + return { + appState: { + ...appState, + ...selectGroupsForSelectedElements( + { + editingGroupId: getParentEditingGroupId( + appState, + app, + selectedElementIds, + ), + selectedElementIds, + }, + nonDeletedElements, + appState, + app, + ), + activeEmbeddable: null, + activeTool, + selectedLinearElement: null, + selectionElement: null, + showHyperlinkPopup: false, + suggestedBinding: null, + frameToHighlight: null, + }, + captureUpdate: CaptureUpdateAction.IMMEDIATELY, + }; + } + + return { + appState: { + ...appState, + activeEmbeddable: null, + activeTool, + editingGroupId: null, + selectedElementIds: makeNextSelectedElementIds({}, appState), + selectedGroupIds: {}, + selectedLinearElement: null, + selectionElement: null, + showHyperlinkPopup: false, + suggestedBinding: null, + frameToHighlight: null, + }, + captureUpdate: CaptureUpdateAction.IMMEDIATELY, + }; + }, + keyTest: (event, appState, _, app) => { + if (event.key !== KEYS.ESCAPE) { + return false; + } + + if (isWritableElement(event.target)) { + return false; + } + + return ( + !appState.newElement && + appState.multiElement === null && + !appState.selectedLinearElement?.isEditing && + (appState.activeEmbeddable !== null || + appState.activeTool.type !== app.state.preferredSelectionTool.type || + !!appState.editingGroupId || + !!appState.selectedLinearElement || + isSomeElementSelected(app.scene.getNonDeletedElements(), appState)) + ); + }, +}); diff --git a/packages/excalidraw/actions/actionFinalize.tsx b/packages/excalidraw/actions/actionFinalize.tsx index 9e529621a7..f2e5db5613 100644 --- a/packages/excalidraw/actions/actionFinalize.tsx +++ b/packages/excalidraw/actions/actionFinalize.tsx @@ -329,8 +329,8 @@ export const actionFinalize = register({ selectionElement: null, multiElement: null, editingTextElement: null, - startBoundElement: null, suggestedBinding: null, + frameToHighlight: null, selectedElementIds: element && !appState.activeTool.locked && @@ -348,9 +348,7 @@ export const actionFinalize = register({ }; }, keyTest: (event, appState) => - (event.key === KEYS.ESCAPE && - (appState.selectedLinearElement?.isEditing || - (!appState.newElement && appState.multiElement === null))) || + (event.key === KEYS.ESCAPE && appState.selectedLinearElement?.isEditing) || ((event.key === KEYS.ESCAPE || event.key === KEYS.ENTER) && appState.multiElement !== null), PanelComponent: ({ appState, updateData, data }) => ( diff --git a/packages/excalidraw/actions/actionFrame.ts b/packages/excalidraw/actions/actionFrame.ts index 3a1b3635d3..81a43e243e 100644 --- a/packages/excalidraw/actions/actionFrame.ts +++ b/packages/excalidraw/actions/actionFrame.ts @@ -205,7 +205,6 @@ export const actionWrapSelectionInFrame = register({ [...app.scene.getElementsIncludingDeleted(), frame], selectedElements, frame, - appState, ); return { diff --git a/packages/excalidraw/actions/actionGroup.tsx b/packages/excalidraw/actions/actionGroup.tsx index c72216b761..c9bb8cf21d 100644 --- a/packages/excalidraw/actions/actionGroup.tsx +++ b/packages/excalidraw/actions/actionGroup.tsx @@ -277,7 +277,6 @@ export const actionUngroup = register({ elementsMap, ), frame, - app, ); } }); diff --git a/packages/excalidraw/actions/actionHistory.tsx b/packages/excalidraw/actions/actionHistory.tsx index 1b530adeaf..5db66670b7 100644 --- a/packages/excalidraw/actions/actionHistory.tsx +++ b/packages/excalidraw/actions/actionHistory.tsx @@ -1,5 +1,4 @@ import { - isWindows, KEYS, matchKey, arrayToMap, @@ -114,7 +113,7 @@ export const createRedoAction: ActionCreator = (history) => ({ ), keyTest: (event) => (event[KEYS.CTRL_OR_CMD] && event.shiftKey && matchKey(event, KEYS.Z)) || - (isWindows && event.ctrlKey && !event.shiftKey && matchKey(event, KEYS.Y)), + (event[KEYS.CTRL_OR_CMD] && !event.shiftKey && matchKey(event, KEYS.Y)), PanelComponent: ({ appState, updateData, data, app }) => { const { isRedoStackEmpty } = useEmitter( history.onHistoryChangedEmitter, diff --git a/packages/excalidraw/actions/index.ts b/packages/excalidraw/actions/index.ts index cc9ca1789c..d630c6ecaa 100644 --- a/packages/excalidraw/actions/index.ts +++ b/packages/excalidraw/actions/index.ts @@ -34,6 +34,7 @@ export { export { actionSetEmbeddableAsActiveTool } from "./actionEmbeddable"; export { actionFinalize } from "./actionFinalize"; +export { actionDeselect } from "./actionDeselect"; export { actionChangeProjectName, diff --git a/packages/excalidraw/actions/types.ts b/packages/excalidraw/actions/types.ts index ae80e4107c..02c67d34f3 100644 --- a/packages/excalidraw/actions/types.ts +++ b/packages/excalidraw/actions/types.ts @@ -114,6 +114,7 @@ export type ActionName = | "distributeVertically" | "flipHorizontal" | "flipVertical" + | "deselect" | "viewMode" | "exportWithDarkMode" | "toggleTheme" diff --git a/packages/excalidraw/animated-trail.ts b/packages/excalidraw/animatedTrail.ts similarity index 83% rename from packages/excalidraw/animated-trail.ts rename to packages/excalidraw/animatedTrail.ts index 2cf5540e08..e98e6e50ee 100644 --- a/packages/excalidraw/animated-trail.ts +++ b/packages/excalidraw/animatedTrail.ts @@ -1,5 +1,4 @@ import { LaserPointer } from "@excalidraw/laser-pointer"; - import { SVG_NS, getSvgPathFromStroke, @@ -8,7 +7,8 @@ import { import type { LaserPointerOptions } from "@excalidraw/laser-pointer"; -import type { AnimationFrameHandler } from "./animation-frame-handler"; +import { AnimationController } from "./renderer/animation"; + import type App from "./components/App"; import type { AppState } from "./types"; @@ -34,15 +34,16 @@ export class AnimatedTrail implements Trail { private container?: SVGSVGElement; private trailElement: SVGPathElement; private trailAnimation?: SVGAnimateElement; + private key: string; + + private static counter = 0; constructor( - private animationFrameHandler: AnimationFrameHandler, protected app: App, private options: Partial & Partial, ) { - this.animationFrameHandler.register(this, this.onFrame.bind(this)); - + this.key = `animated-trail-${AnimatedTrail.counter++}`; this.trailElement = document.createElementNS(SVG_NS, "path"); if (this.options.animateTrail) { this.trailAnimation = document.createElementNS(SVG_NS, "animate"); @@ -73,6 +74,15 @@ export class AnimatedTrail implements Trail { return false; } + private cleanup() { + this.pastTrails = []; + this.currentTrail = undefined; + + if (this.trailElement.parentNode === this.container) { + this.container?.removeChild(this.trailElement); + } + } + start(container?: SVGSVGElement) { if (container) { this.container = container; @@ -82,15 +92,23 @@ export class AnimatedTrail implements Trail { this.container.appendChild(this.trailElement); } - this.animationFrameHandler.start(this); + if (!AnimationController.running(this.key)) { + AnimationController.start(this.key, () => { + const needsNext = this.onFrame(); + if (needsNext) { + return { keep: true }; + } + + this.cleanup(); + + return null; + }); + } } stop() { - this.animationFrameHandler.stop(this); - - if (this.trailElement.parentNode === this.container) { - this.container?.removeChild(this.trailElement); - } + AnimationController.cancel(this.key); + this.cleanup(); } startPath(x: number, y: number) { @@ -145,21 +163,25 @@ export class AnimatedTrail implements Trail { if (this.currentTrail) { const currentPath = this.drawTrail(this.currentTrail, this.app.state); - paths.push(currentPath); } - this.pastTrails = this.pastTrails.filter((trail) => { - return trail.getStrokeOutline().length !== 0; - }); + this.pastTrails = this.pastTrails.filter( + (t) => + t.getStrokeOutline(t.options.size / this.app.state.zoom.value) + .length !== 0, + ); if (paths.length === 0) { - this.stop(); + // Clean up the SVG path if there are no trails to render + this.trailElement.setAttribute("d", ""); + + return false; } const svgPaths = paths.join(" ").trim(); - this.trailElement.setAttribute("d", svgPaths); + if (this.trailAnimation) { this.trailElement.setAttribute( "fill", @@ -175,6 +197,8 @@ export class AnimatedTrail implements Trail { (this.options.fill ?? (() => "black"))(this), ); } + + return true; } private drawTrail(trail: LaserPointer, state: AppState): string { diff --git a/packages/excalidraw/animation-frame-handler.ts b/packages/excalidraw/animation-frame-handler.ts deleted file mode 100644 index b1a9844669..0000000000 --- a/packages/excalidraw/animation-frame-handler.ts +++ /dev/null @@ -1,79 +0,0 @@ -export type AnimationCallback = (timestamp: number) => void | boolean; - -export type AnimationTarget = { - callback: AnimationCallback; - stopped: boolean; -}; - -export class AnimationFrameHandler { - private targets = new WeakMap(); - private rafIds = new WeakMap(); - - register(key: object, callback: AnimationCallback) { - this.targets.set(key, { callback, stopped: true }); - } - - start(key: object) { - const target = this.targets.get(key); - - if (!target) { - return; - } - - if (this.rafIds.has(key)) { - return; - } - - this.targets.set(key, { ...target, stopped: false }); - this.scheduleFrame(key); - } - - stop(key: object) { - const target = this.targets.get(key); - if (target && !target.stopped) { - this.targets.set(key, { ...target, stopped: true }); - } - - this.cancelFrame(key); - } - - private constructFrame(key: object): FrameRequestCallback { - return (timestamp: number) => { - const target = this.targets.get(key); - - if (!target) { - return; - } - - const shouldAbort = this.onFrame(target, timestamp); - - if (!target.stopped && !shouldAbort) { - this.scheduleFrame(key); - } else { - this.cancelFrame(key); - } - }; - } - - private scheduleFrame(key: object) { - const rafId = requestAnimationFrame(this.constructFrame(key)); - - this.rafIds.set(key, rafId); - } - - private cancelFrame(key: object) { - if (this.rafIds.has(key)) { - const rafId = this.rafIds.get(key)!; - - cancelAnimationFrame(rafId); - } - - this.rafIds.delete(key); - } - - private onFrame(target: AnimationTarget, timestamp: number): boolean { - const shouldAbort = target.callback(timestamp); - - return shouldAbort ?? false; - } -} diff --git a/packages/excalidraw/appState.ts b/packages/excalidraw/appState.ts index e51865b2ea..a64eed8ca7 100644 --- a/packages/excalidraw/appState.ts +++ b/packages/excalidraw/appState.ts @@ -99,7 +99,6 @@ export const getDefaultAppState = (): Omit< open: false, panels: STATS_PANELS.generalStats | STATS_PANELS.elementProperties, }, - startBoundElement: null, suggestedBinding: null, frameRendering: { enabled: true, clip: true, name: true, outline: true }, frameToHighlight: null, @@ -128,6 +127,7 @@ export const getDefaultAppState = (): Omit< lockedMultiSelections: {}, activeLockedId: null, bindMode: "orbit", + boxSelectionMode: "contain", }; }; @@ -193,6 +193,7 @@ const APP_STATE_STORAGE_CONF = (< gridModeEnabled: { browser: true, export: true, server: true }, height: { browser: false, export: false, server: false }, isBindingEnabled: { browser: true, export: false, server: false }, + boxSelectionMode: { browser: true, export: false, server: false }, bindingPreference: { browser: true, export: false, server: false }, isMidpointSnappingEnabled: { browser: true, export: false, server: false }, defaultSidebarDockedPreference: { @@ -229,7 +230,6 @@ const APP_STATE_STORAGE_CONF = (< selectionElement: { browser: false, export: false, server: false }, shouldCacheIgnoreZoom: { browser: true, export: false, server: false }, stats: { browser: true, export: false, server: false }, - startBoundElement: { browser: false, export: false, server: false }, suggestedBinding: { browser: false, export: false, server: false }, frameRendering: { browser: false, export: false, server: false }, frameToHighlight: { browser: false, export: false, server: false }, diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 24056ebcca..292d6bd132 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -27,7 +27,7 @@ import { KEYS, APP_NAME, CURSOR_TYPE, - DEFAULT_MAX_IMAGE_WIDTH_OR_HEIGHT, + DEFAULT_TRANSFORM_HANDLE_SPACING, DEFAULT_VERTICAL_ALIGN, DRAGGING_THRESHOLD, ELEMENT_SHIFT_TRANSLATE_AMOUNT, @@ -37,7 +37,6 @@ import { IMAGE_MIME_TYPES, IMAGE_RENDER_TIMEOUT, LINE_CONFIRM_THRESHOLD, - MAX_ALLOWED_FILE_BYTES, MIME_TYPES, MQ_RIGHT_SIDEBAR_MIN_WIDTH, POINTER_BUTTON, @@ -175,7 +174,9 @@ import { isValidTextContainer, redrawTextBoundingBox, hasBoundingBox, + getCommonFrameId, getFrameChildren, + getFrameChildrenInsertionIndex, isCursorInFrame, addElementsToFrame, replaceAllElementsInFrame, @@ -258,6 +259,7 @@ import { maybeHandleArrowPointlikeDrag, getUncroppedWidthAndHeight, getActiveTextElement, + isEligibleFrameChildType, } from "@excalidraw/element"; import type { GlobalPoint, LocalPoint, Radians } from "@excalidraw/math"; @@ -341,7 +343,6 @@ import { ActionManager } from "../actions/manager"; import { actions } from "../actions/register"; import { getShortcutFromShortcutName } from "../actions/shortcuts"; import { trackEvent } from "../analytics"; -import { AnimationFrameHandler } from "../animation-frame-handler"; import { getDefaultAppState, isEraserActive, @@ -415,7 +416,7 @@ import { setCursorForShape, } from "../cursor"; import { ElementCanvasButtons } from "../components/ElementCanvasButtons"; -import { LaserTrails } from "../laser-trails"; +import { LaserTrails } from "../laserTrails"; import { withBatchedUpdates, withBatchedUpdatesThrottled } from "../reactUtils"; import { isPointHittingTextAutoResizeHandle } from "../textAutoResizeHandle"; import { textWysiwyg } from "../wysiwyg/textWysiwyg"; @@ -602,6 +603,8 @@ const YOUTUBE_VIDEO_STATES = new Map< ValueOf >(); +const MAX_EMBEDDABLE_VIEWPORT_SCALE = 4; + let IS_PLAIN_PASTE = false; let IS_PLAIN_PASTE_TIMER = 0; let PLAIN_PASTE_TOAST_SHOWN = false; @@ -699,11 +702,9 @@ class App extends React.Component { previousPointerMoveCoords: { x: number; y: number } | null = null; lastViewportPosition = { x: 0, y: 0 }; - animationFrameHandler = new AnimationFrameHandler(); - - laserTrails = new LaserTrails(this.animationFrameHandler, this); - eraserTrail = new EraserTrail(this.animationFrameHandler, this); - lassoTrail = new LassoTrail(this.animationFrameHandler, this); + laserTrails = new LaserTrails(this); + eraserTrail = new EraserTrail(this); + lassoTrail = new LassoTrail(this); onChangeEmitter = new Emitter< [ @@ -1734,6 +1735,18 @@ class App extends React.Component { this.state.activeEmbeddable?.element === el && this.state.activeEmbeddable?.state === "hover"; + // scale video embeds based on zoom (capped) so that smaller embeds + // on canvas when zoomed are still of legible quality + // (note: for some embed types like gdrive, the quality is poor when + // scaling mid playback and works only when you initially start the + // playback at the higher zoom level) + const shouldScaleEmbeddableViewport = src?.type === "video"; + const embeddableViewportScale = clamp( + shouldScaleEmbeddableViewport ? scale : 1, + 0.75, + MAX_EMBEDDABLE_VIEWPORT_SCALE, + ); + return (

{ padding: `${el.strokeWidth}px`, }} > - {(isEmbeddableElement(el) - ? this.props.renderEmbeddable?.(el, this.state) - : null) ?? ( -