From fc601347cf57a55bbeed143dd66764ac770c2fbd Mon Sep 17 00:00:00 2001 From: Nishant <61119157+Nishant-l@users.noreply.github.com> Date: Wed, 12 Apr 2023 02:53:36 +0530 Subject: [PATCH 1/4] fix: fixing popover overflow on small screen (#6433) Co-authored-by: dwelle --- src/components/Popover.scss | 1 + src/components/Popover.tsx | 86 ++++++++++++++++++++++++++----------- 2 files changed, 61 insertions(+), 26 deletions(-) diff --git a/src/components/Popover.scss b/src/components/Popover.scss index 84d16e47f..9458b5026 100644 --- a/src/components/Popover.scss +++ b/src/components/Popover.scss @@ -3,5 +3,6 @@ position: absolute; z-index: 10; padding: 5px 0 5px; + outline: none; } } diff --git a/src/components/Popover.tsx b/src/components/Popover.tsx index 9a046599b..c1249f798 100644 --- a/src/components/Popover.tsx +++ b/src/components/Popover.tsx @@ -29,13 +29,15 @@ export const Popover = ({ }: Props) => { const popoverRef = useRef(null); - const container = popoverRef.current; - useEffect(() => { + const container = popoverRef.current; + if (!container) { return; } + container.focus(); + const handleKeyDown = (event: KeyboardEvent) => { if (event.key === KEYS.TAB) { const focusableElements = queryFocusableElements(container); @@ -44,15 +46,23 @@ export const Popover = ({ (element) => element === activeElement, ); - if (currentIndex === 0 && event.shiftKey) { - focusableElements[focusableElements.length - 1].focus(); + if (activeElement === container) { + if (event.shiftKey) { + focusableElements[focusableElements.length - 1]?.focus(); + } else { + focusableElements[0].focus(); + } + event.preventDefault(); + event.stopImmediatePropagation(); + } else if (currentIndex === 0 && event.shiftKey) { + focusableElements[focusableElements.length - 1]?.focus(); event.preventDefault(); event.stopImmediatePropagation(); } else if ( currentIndex === focusableElements.length - 1 && !event.shiftKey ) { - focusableElements[0].focus(); + focusableElements[0]?.focus(); event.preventDefault(); event.stopImmediatePropagation(); } @@ -62,35 +72,59 @@ export const Popover = ({ container.addEventListener("keydown", handleKeyDown); return () => container.removeEventListener("keydown", handleKeyDown); - }, [container]); + }, []); + + const lastInitializedPosRef = useRef<{ top: number; left: number } | null>( + null, + ); // ensure the popover doesn't overflow the viewport useLayoutEffect(() => { - if (fitInViewport && popoverRef.current) { - const element = popoverRef.current; - const { x, y, width, height } = element.getBoundingClientRect(); + if (fitInViewport && popoverRef.current && top != null && left != null) { + const container = popoverRef.current; + const { width, height } = container.getBoundingClientRect(); - //Position correctly when clicked on rightmost part or the bottom part of viewport - if (x + width - offsetLeft > viewportWidth) { - element.style.left = `${viewportWidth - width - 10}px`; - } - if (y + height - offsetTop > viewportHeight) { - element.style.top = `${viewportHeight - height}px`; + // hack for StrictMode so this effect only runs once for + // the same top/left position, otherwise + // we'd potentically reposition twice (once for viewport overflow) + // and once for top/left position afterwards + if ( + lastInitializedPosRef.current?.top === top && + lastInitializedPosRef.current?.left === left + ) { + return; } + lastInitializedPosRef.current = { top, left }; - //Resize to fit viewport on smaller screens - if (height >= viewportHeight) { - element.style.height = `${viewportHeight - 20}px`; - element.style.top = "10px"; - element.style.overflowY = "scroll"; - } if (width >= viewportWidth) { - element.style.width = `${viewportWidth}px`; - element.style.left = "0px"; - element.style.overflowX = "scroll"; + container.style.width = `${viewportWidth}px`; + container.style.left = "0px"; + container.style.overflowX = "scroll"; + } else if (left + width - offsetLeft > viewportWidth) { + container.style.left = `${viewportWidth - width - 10}px`; + } else { + container.style.left = `${left}px`; + } + + if (height >= viewportHeight) { + container.style.height = `${viewportHeight - 20}px`; + container.style.top = "10px"; + container.style.overflowY = "scroll"; + } else if (top + height - offsetTop > viewportHeight) { + container.style.top = `${viewportHeight - height}px`; + } else { + container.style.top = `${top}px`; } } - }, [fitInViewport, viewportWidth, viewportHeight, offsetLeft, offsetTop]); + }, [ + top, + left, + fitInViewport, + viewportWidth, + viewportHeight, + offsetLeft, + offsetTop, + ]); useEffect(() => { if (onCloseRequest) { @@ -105,7 +139,7 @@ export const Popover = ({ }, [onCloseRequest]); return ( -
+
{children}
); From 372743f59f03b2f368ea0bf704cb37c3dca60cc5 Mon Sep 17 00:00:00 2001 From: David Luzar Date: Wed, 12 Apr 2023 10:57:00 +0200 Subject: [PATCH 2/4] fix: autoredirect to plus in prod only (#6446) --- public/index.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/public/index.html b/public/index.html index 47a59624b..a8633fc4d 100644 --- a/public/index.html +++ b/public/index.html @@ -79,6 +79,7 @@ + <% if (process.env.NODE_ENV === "production") { %> + <% } %> From 13b27afe0f39cc43d80fa05232e985530d0f0600 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Thu, 13 Apr 2023 11:45:58 +0530 Subject: [PATCH 3/4] fix: update coords when text unbinded from its container (#6445) * fix: update coords when text unbinded from its container * Add specs --- src/actions/actionBoundText.tsx | 6 ++- src/element/textElement.ts | 8 +++- src/element/textWysiwyg.test.tsx | 39 +++++++++++++++++ src/tests/linearElementEditor.test.tsx | 59 +++++++++++++++++++++++++- 4 files changed, 109 insertions(+), 3 deletions(-) diff --git a/src/actions/actionBoundText.tsx b/src/actions/actionBoundText.tsx index 9f9fbfc0f..990f98d41 100644 --- a/src/actions/actionBoundText.tsx +++ b/src/actions/actionBoundText.tsx @@ -2,6 +2,7 @@ import { BOUND_TEXT_PADDING, ROUNDNESS, VERTICAL_ALIGN } from "../constants"; import { getNonDeletedElements, isTextElement, newElement } from "../element"; import { mutateElement } from "../element/mutateElement"; import { + computeBoundTextPosition, computeContainerDimensionForBoundText, getBoundTextElement, measureText, @@ -33,6 +34,7 @@ export const actionUnbindText = register({ trackEvent: { category: "element" }, predicate: (elements, appState) => { const selectedElements = getSelectedElements(elements, appState); + return selectedElements.some((element) => hasBoundTextElement(element)); }, perform: (elements, appState) => { @@ -52,13 +54,15 @@ export const actionUnbindText = register({ element.id, ); resetOriginalContainerCache(element.id); - + const { x, y } = computeBoundTextPosition(element, boundTextElement); mutateElement(boundTextElement as ExcalidrawTextElement, { containerId: null, width, height, baseline, text: boundTextElement.originalText, + x, + y, }); mutateElement(element, { boundElements: element.boundElements?.filter( diff --git a/src/element/textElement.ts b/src/element/textElement.ts index 26cf91a89..38da5df5a 100644 --- a/src/element/textElement.ts +++ b/src/element/textElement.ts @@ -245,10 +245,16 @@ export const handleBindTextResize = ( } }; -const computeBoundTextPosition = ( +export const computeBoundTextPosition = ( container: ExcalidrawElement, boundTextElement: ExcalidrawTextElementWithContainer, ) => { + if (isArrowElement(container)) { + return LinearElementEditor.getBoundTextElementPosition( + container, + boundTextElement, + ); + } const containerCoords = getContainerCoords(container); const maxContainerHeight = getMaxContainerHeight(container); const maxContainerWidth = getMaxContainerWidth(container); diff --git a/src/element/textWysiwyg.test.tsx b/src/element/textWysiwyg.test.tsx index cb852cd1c..c55a9befa 100644 --- a/src/element/textWysiwyg.test.tsx +++ b/src/element/textWysiwyg.test.tsx @@ -740,6 +740,45 @@ describe("textWysiwyg", () => { expect(rectangle.boundElements).toBe(null); }); + it("should bind text to container when triggered via context menu", async () => { + expect(h.elements.length).toBe(1); + expect(h.elements[0].id).toBe(rectangle.id); + + UI.clickTool("text"); + mouse.clickAt(20, 30); + const editor = document.querySelector( + ".excalidraw-textEditorContainer > textarea", + ) as HTMLTextAreaElement; + + fireEvent.change(editor, { + target: { + value: "Excalidraw is an opensource virtual collaborative whiteboard", + }, + }); + + editor.dispatchEvent(new Event("input")); + await new Promise((cb) => setTimeout(cb, 0)); + expect(h.elements.length).toBe(2); + expect(h.elements[1].type).toBe("text"); + + API.setSelectedElements([h.elements[0], h.elements[1]]); + fireEvent.contextMenu(GlobalTestState.canvas, { + button: 2, + clientX: 20, + clientY: 30, + }); + const contextMenu = document.querySelector(".context-menu"); + fireEvent.click( + queryByText(contextMenu as HTMLElement, "Bind text to the container")!, + ); + const text = h.elements[1] as ExcalidrawTextElementWithContainer; + expect(rectangle.boundElements).toStrictEqual([ + { id: h.elements[1].id, type: "text" }, + ]); + expect(text.containerId).toBe(rectangle.id); + expect(text.verticalAlign).toBe(VERTICAL_ALIGN.MIDDLE); + }); + it("should update font family correctly on undo/redo by selecting bounded text when font family was updated", async () => { expect(h.elements.length).toBe(1); diff --git a/src/tests/linearElementEditor.test.tsx b/src/tests/linearElementEditor.test.tsx index ac4d801bc..15fd105ec 100644 --- a/src/tests/linearElementEditor.test.tsx +++ b/src/tests/linearElementEditor.test.tsx @@ -23,7 +23,7 @@ import { getMaxContainerWidth, } from "../element/textElement"; import * as textElementUtils from "../element/textElement"; -import { ROUNDNESS } from "../constants"; +import { ROUNDNESS, VERTICAL_ALIGN } from "../constants"; const renderScene = jest.spyOn(Renderer, "renderScene"); @@ -1191,5 +1191,62 @@ describe("Test Linear Elements", () => { expect(queryByTestId(container, "align-horizontal-center")).toBeNull(); expect(queryByTestId(container, "align-right")).toBeNull(); }); + + it("should update label coords when a label binded via context menu is unbinded", async () => { + createTwoPointerLinearElement("arrow"); + const text = API.createElement({ + type: "text", + text: "Hello Excalidraw", + }); + expect(text.x).toBe(0); + expect(text.y).toBe(0); + + h.elements = [h.elements[0], text]; + + const container = h.elements[0]; + API.setSelectedElements([container, text]); + fireEvent.contextMenu(GlobalTestState.canvas, { + button: 2, + clientX: 20, + clientY: 30, + }); + let contextMenu = document.querySelector(".context-menu"); + + fireEvent.click( + queryByText(contextMenu as HTMLElement, "Bind text to the container")!, + ); + expect(container.boundElements).toStrictEqual([ + { id: h.elements[1].id, type: "text" }, + ]); + expect(text.containerId).toBe(container.id); + expect(text.verticalAlign).toBe(VERTICAL_ALIGN.MIDDLE); + + mouse.reset(); + mouse.clickAt( + container.x + container.width / 2, + container.y + container.height / 2, + ); + mouse.down(); + mouse.up(); + API.setSelectedElements([h.elements[0], h.elements[1]]); + + fireEvent.contextMenu(GlobalTestState.canvas, { + button: 2, + clientX: 20, + clientY: 30, + }); + contextMenu = document.querySelector(".context-menu"); + fireEvent.click(queryByText(contextMenu as HTMLElement, "Unbind text")!); + expect(container.boundElements).toEqual([]); + expect(text).toEqual( + expect.objectContaining({ + containerId: null, + width: 160, + height: 25, + x: -40, + y: 7.5, + }), + ); + }); }); }); From ca3be2c678dfc5fae50d005fdcfe3b8c84fc2544 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Thu, 13 Apr 2023 17:19:46 +0530 Subject: [PATCH 4/4] fix: exporting labelled arrows via export utils (#6443) * fix: exporting labelled arrows via export utils * add comments * lint * update changelog * fix lint * initialize scene in the utils so it can be availabe in the helper functions * fix library rendering * add comments --- src/components/LibraryUnit.tsx | 10 +++--- src/packages/excalidraw/CHANGELOG.md | 4 +++ src/packages/utils.ts | 48 ++++++++++++++++++++++++---- 3 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/components/LibraryUnit.tsx b/src/components/LibraryUnit.tsx index 6723ac800..749877cdd 100644 --- a/src/components/LibraryUnit.tsx +++ b/src/components/LibraryUnit.tsx @@ -2,7 +2,7 @@ import clsx from "clsx"; import oc from "open-color"; import { useEffect, useRef, useState } from "react"; import { useDevice } from "../components/App"; -import { exportToSvg } from "../scene/export"; +import { exportToSvg } from "../packages/utils"; import { LibraryItem } from "../types"; import "./LibraryUnit.scss"; import { CheckboxItem } from "./CheckboxItem"; @@ -36,14 +36,14 @@ export const LibraryUnit = ({ if (!elements) { return; } - const svg = await exportToSvg( + const svg = await exportToSvg({ elements, - { + appState: { exportBackground: false, viewBackgroundColor: oc.white, }, - null, - ); + files: null, + }); svg.querySelector(".style-fonts")?.remove(); node.innerHTML = svg.outerHTML; })(); diff --git a/src/packages/excalidraw/CHANGELOG.md b/src/packages/excalidraw/CHANGELOG.md index 628177a4a..c36883b8e 100644 --- a/src/packages/excalidraw/CHANGELOG.md +++ b/src/packages/excalidraw/CHANGELOG.md @@ -33,6 +33,10 @@ For more details refer to the [docs](https://docs.excalidraw.com) - The optional parameter `refreshDimensions` in [`restoreElements`](https://docs.excalidraw.com/docs/@excalidraw/excalidraw/api/utils/restore#restoreelements) has been removed and can be enabled via `opts` +### Fixes + +- Exporting labelled arrows via export utils [#6443](https://github.com/excalidraw/excalidraw/pull/6443) + ## 0.14.2 (2023-02-01) ### Features diff --git a/src/packages/utils.ts b/src/packages/utils.ts index d81995080..5161e0cab 100644 --- a/src/packages/utils.ts +++ b/src/packages/utils.ts @@ -5,7 +5,6 @@ import { import { getDefaultAppState } from "../appState"; import { AppState, BinaryFiles } from "../types"; import { ExcalidrawElement, NonDeleted } from "../element/types"; -import { getNonDeletedElements } from "../element"; import { restore } from "../data/restore"; import { MIME_TYPES } from "../constants"; import { encodePngMetadata } from "../data/image"; @@ -15,6 +14,7 @@ import { copyTextToSystemClipboard, copyToClipboard, } from "../clipboard"; +import Scene from "../scene/Scene"; export { MIME_TYPES }; @@ -44,9 +44,17 @@ export const exportToCanvas = ({ null, null, ); + // The helper methods getContainerElement and getBoundTextElement are + // dependent on Scene which will not be available + // when these pure utils are called outside Excalidraw or even if called + // from inside Excalidraw when Scene isn't available eg when using library items from store, as a result the element cannot be extracted + // hence initailizing a new scene with the elements + // so its always available to helper methods + const scene = new Scene(); + scene.replaceAllElements(restoredElements); const { exportBackground, viewBackgroundColor } = restoredAppState; return _exportToCanvas( - getNonDeletedElements(restoredElements), + scene.getNonDeletedElements(), { ...restoredAppState, offsetTop: 0, offsetLeft: 0, width: 0, height: 0 }, files || {}, { exportBackground, exportPadding, viewBackgroundColor }, @@ -114,8 +122,18 @@ export const exportToBlob = async ( }; } - const canvas = await exportToCanvas(opts); - + // The helper methods getContainerElement and getBoundTextElement are + // dependent on Scene which will not be available + // when these pure utils are called outside Excalidraw or even if called + // from inside Excalidraw when Scene isn't available eg when using library items from store, as a result the element cannot be extracted + // hence initailizing a new scene with the elements + // so its always available to helper methods + const scene = new Scene(); + scene.replaceAllElements(opts.elements); + const canvas = await exportToCanvas({ + ...opts, + elements: scene.getNonDeletedElements(), + }); quality = quality ? quality : /image\/jpe?g/.test(mimeType) ? 0.92 : 0.8; return new Promise((resolve, reject) => { @@ -132,7 +150,7 @@ export const exportToBlob = async ( blob = await encodePngMetadata({ blob, metadata: serializeAsJSON( - opts.elements, + scene.getNonDeletedElements(), opts.appState, opts.files || {}, "local", @@ -160,8 +178,16 @@ export const exportToSvg = async ({ null, null, ); + // The helper methods getContainerElement and getBoundTextElement are + // dependent on Scene which will not be available + // when these pure utils are called outside Excalidraw or even if called + // from inside Excalidraw when Scene isn't available eg when using library items from store, as a result the element cannot be extracted + // hence initailizing a new scene with the elements + // so its always available to helper methods + const scene = new Scene(); + scene.replaceAllElements(restoredElements); return _exportToSvg( - getNonDeletedElements(restoredElements), + scene.getNonDeletedElements(), { ...restoredAppState, exportPadding, @@ -177,6 +203,14 @@ export const exportToClipboard = async ( type: "png" | "svg" | "json"; }, ) => { + // The helper methods getContainerElement and getBoundTextElement are + // dependent on Scene which will not be available + // when these pure utils are called outside Excalidraw or even if called + // from inside Excalidraw when Scene isn't available eg when using library items from store, as a result the element cannot be extracted + // hence initailizing a new scene with the elements + // so its always available to helper methods + const scene = new Scene(); + scene.replaceAllElements(opts.elements); if (opts.type === "svg") { const svg = await exportToSvg(opts); await copyTextToSystemClipboard(svg.outerHTML); @@ -191,7 +225,7 @@ export const exportToClipboard = async ( ...getDefaultAppState(), ...opts.appState, }; - await copyToClipboard(opts.elements, appState, opts.files); + await copyToClipboard(scene.getNonDeletedElements(), appState, opts.files); } else { throw new Error("Invalid export type"); }