From 4912a29e75567bc62cd0f868944e00a14df160ac Mon Sep 17 00:00:00 2001 From: David Luzar Date: Fri, 3 Apr 2020 14:16:14 +0200 Subject: [PATCH] sync intermediate text updates (#1174) * sync intermediate text updates * fix initial render text position * batch updates * tweak onChange subscription --- src/components/App.tsx | 248 +++++++++++++++++++++--------------- src/element/index.ts | 2 +- src/element/newElement.ts | 27 ++-- src/element/textWysiwyg.tsx | 11 +- 4 files changed, 168 insertions(+), 120 deletions(-) diff --git a/src/components/App.tsx b/src/components/App.tsx index 7a5c5e38..53d63216 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -46,11 +46,14 @@ import { SocketUpdateDataSource, exportCanvas, } from "../data"; -import { restore } from "../data/restore"; import { renderScene } from "../renderer"; import { AppState, GestureEvent, Gesture } from "../types"; -import { ExcalidrawElement, ExcalidrawLinearElement } from "../element/types"; +import { + ExcalidrawElement, + ExcalidrawLinearElement, + ExcalidrawTextElement, +} from "../element/types"; import { rotate, adjustXYWithRotation } from "../math"; import { @@ -474,7 +477,15 @@ export class App extends React.Component { ); }); const { atLeastOneVisibleElement, scrollBars } = renderScene( - globalSceneState.getAllElements(), + globalSceneState.getAllElements().filter((element) => { + // don't render text element that's being currently edited (it's + // rendered on remote only) + return ( + !this.state.editingElement || + this.state.editingElement.type !== "text" || + element.id !== this.state.editingElement.id + ); + }), this.state, this.state.selectionElement, window.devicePixelRatio, @@ -717,9 +728,7 @@ export class App extends React.Component { decryptedData: SocketUpdateDataSource["SCENE_INIT" | "SCENE_UPDATE"], ) => { const { elements: remoteElements } = decryptedData.payload; - const restoredState = restore(remoteElements || [], null, { - scrollToContent: true, - }); + // Perform reconciliation - in collaboration, if we encounter // elements with more staler versions than ours, ignore them // and keep ours. @@ -727,7 +736,7 @@ export class App extends React.Component { globalSceneState.getAllElements() == null || globalSceneState.getAllElements().length === 0 ) { - globalSceneState.replaceAllElements(restoredState.elements); + globalSceneState.replaceAllElements(remoteElements); } else { // create a map of ids so we don't have to iterate // over the array more than once. @@ -736,7 +745,7 @@ export class App extends React.Component { ); // Reconcile - const newElements = restoredState.elements + const newElements = remoteElements .reduce((elements, element) => { // if the remote element references one that's currently // edited on local, skip it (it'll be added in the next @@ -779,7 +788,7 @@ export class App extends React.Component { } return elements; - }, [] as Mutable) + }, [] as Mutable) // add local elements that weren't deleted or on remote .concat(...Object.values(localElementMap)); @@ -1082,6 +1091,96 @@ export class App extends React.Component { globalSceneState.replaceAllElements(elements); }; + private handleTextWysiwyg( + element: ExcalidrawTextElement, + { + x, + y, + isExistingElement = false, + }: { x: number; y: number; isExistingElement?: boolean }, + ) { + const resetSelection = () => { + this.setState({ + draggingElement: null, + editingElement: null, + }); + }; + + // deselect all other elements when inserting text + this.setState({ selectedElementIds: {} }); + + const deleteElement = () => { + globalSceneState.replaceAllElements([ + ...globalSceneState.getAllElements().map((_element) => { + if (_element.id === element.id) { + return newElementWith(_element, { isDeleted: true }); + } + return _element; + }), + ]); + }; + + const updateElement = (text: string) => { + globalSceneState.replaceAllElements([ + ...globalSceneState.getAllElements().map((_element) => { + if (_element.id === element.id) { + return newTextElement({ + ..._element, + x: element.x, + y: element.y, + text, + font: this.state.currentItemFont, + }); + } + return _element; + }), + ]); + }; + + textWysiwyg({ + x, + y, + initText: element.text, + strokeColor: element.strokeColor, + opacity: element.opacity, + font: element.font, + angle: element.angle, + zoom: this.state.zoom, + onChange: withBatchedUpdates((text) => { + if (text) { + updateElement(text); + } else { + deleteElement(); + } + }), + onSubmit: withBatchedUpdates((text) => { + updateElement(text); + this.setState((prevState) => ({ + selectedElementIds: { + ...prevState.selectedElementIds, + [element.id]: true, + }, + })); + if (this.state.elementLocked) { + setCursorForShape(this.state.elementType); + } + history.resumeRecording(); + resetSelection(); + }), + onCancel: withBatchedUpdates(() => { + deleteElement(); + if (isExistingElement) { + history.resumeRecording(); + } + resetSelection(); + }), + }); + + // do an initial update to re-initialize element position since we were + // modifying element's x/y for sake of editor (case: syncing to remote) + updateElement(element.text); + } + private startTextEditing = ({ x, y, @@ -1124,13 +1223,10 @@ export class App extends React.Component { let textX = clientX || x; let textY = clientY || y; - if (elementAtPosition && isTextElement(elementAtPosition)) { - globalSceneState.replaceAllElements( - globalSceneState - .getAllElements() - .filter((element) => element.id !== elementAtPosition.id), - ); + let isExistingTextElement = false; + if (elementAtPosition && isTextElement(elementAtPosition)) { + isExistingTextElement = true; const centerElementX = elementAtPosition.x + elementAtPosition.width / 2; const centerElementY = elementAtPosition.y + elementAtPosition.height / 2; @@ -1152,64 +1248,40 @@ export class App extends React.Component { x: centerElementX, y: centerElementY, }); - } else if (centerIfPossible) { - const snappedToCenterPosition = this.getTextWysiwygSnappedToCenterPosition( - x, - y, - this.state, - this.canvas, - window.devicePixelRatio, - ); + } else { + globalSceneState.replaceAllElements([ + ...globalSceneState.getAllElements(), + element, + ]); - if (snappedToCenterPosition) { - mutateElement(element, { - x: snappedToCenterPosition.elementCenterX, - y: snappedToCenterPosition.elementCenterY, - }); - textX = snappedToCenterPosition.wysiwygX; - textY = snappedToCenterPosition.wysiwygY; + if (centerIfPossible) { + const snappedToCenterPosition = this.getTextWysiwygSnappedToCenterPosition( + x, + y, + this.state, + this.canvas, + window.devicePixelRatio, + ); + + if (snappedToCenterPosition) { + mutateElement(element, { + x: snappedToCenterPosition.elementCenterX, + y: snappedToCenterPosition.elementCenterY, + }); + textX = snappedToCenterPosition.wysiwygX; + textY = snappedToCenterPosition.wysiwygY; + } } } - const resetSelection = () => { - this.setState({ - draggingElement: null, - editingElement: null, - }); - }; + this.setState({ + editingElement: element, + }); - // deselect all other elements when inserting text - this.setState({ selectedElementIds: {} }); - - textWysiwyg({ - initText: element.text, + this.handleTextWysiwyg(element, { x: textX, y: textY, - strokeColor: element.strokeColor, - font: element.font, - opacity: this.state.currentItemOpacity, - zoom: this.state.zoom, - angle: element.angle, - onSubmit: (text) => { - if (text) { - globalSceneState.replaceAllElements([ - ...globalSceneState.getAllElements(), - // we need to recreate the element to update dimensions & position - newTextElement({ ...element, text, font: element.font }), - ]); - } - this.setState((prevState) => ({ - selectedElementIds: { - ...prevState.selectedElementIds, - [element.id]: true, - }, - })); - history.resumeRecording(); - resetSelection(); - }, - onCancel: () => { - resetSelection(); - }, + isExistingElement: isExistingTextElement, }); }; @@ -1670,48 +1742,14 @@ export class App extends React.Component { font: this.state.currentItemFont, }); - const resetSelection = () => { - this.setState({ - draggingElement: null, - editingElement: null, - }); - }; + globalSceneState.replaceAllElements([ + ...globalSceneState.getAllElements(), + element, + ]); - textWysiwyg({ - initText: "", + this.handleTextWysiwyg(element, { x: snappedToCenterPosition?.wysiwygX ?? event.clientX, y: snappedToCenterPosition?.wysiwygY ?? event.clientY, - strokeColor: this.state.currentItemStrokeColor, - opacity: this.state.currentItemOpacity, - font: this.state.currentItemFont, - zoom: this.state.zoom, - angle: 0, - onSubmit: (text) => { - if (text) { - globalSceneState.replaceAllElements([ - ...globalSceneState.getAllElements(), - newTextElement({ - ...element, - text, - font: this.state.currentItemFont, - }), - ]); - } - this.setState((prevState) => ({ - selectedElementIds: { - ...prevState.selectedElementIds, - [element.id]: true, - }, - })); - if (this.state.elementLocked) { - setCursorForShape(this.state.elementType); - } - history.resumeRecording(); - resetSelection(); - }, - onCancel: () => { - resetSelection(); - }, }); resetCursor(); if (!this.state.elementLocked) { diff --git a/src/element/index.ts b/src/element/index.ts index 1c4e8886..f87490d6 100644 --- a/src/element/index.ts +++ b/src/element/index.ts @@ -37,7 +37,7 @@ export function getSyncableElements(elements: readonly ExcalidrawElement[]) { // There are places in Excalidraw where synthetic invisibly small elements are added and removed. // It's probably best to keep those local otherwise there might be a race condition that // gets the app into an invalid state. I've never seen it happen but I'm worried about it :) - return elements.filter((el) => !isInvisiblySmallElement(el)); + return elements.filter((el) => el.isDeleted || !isInvisiblySmallElement(el)); } export function getElementMap(elements: readonly ExcalidrawElement[]) { diff --git a/src/element/newElement.ts b/src/element/newElement.ts index 9c58ed4b..674b7a53 100644 --- a/src/element/newElement.ts +++ b/src/element/newElement.ts @@ -6,6 +6,7 @@ import { } from "../element/types"; import { measureText } from "../utils"; import { randomInteger, randomId } from "../random"; +import { newElementWith } from "./mutateElement"; type ElementConstructorOpts = { x: ExcalidrawGenericElement["x"]; @@ -75,17 +76,21 @@ export function newTextElement( ): ExcalidrawTextElement { const { text, font } = opts; const metrics = measureText(text, font); - const textElement = { - ..._newElementBase("text", opts), - text: text, - font: font, - // Center the text - x: opts.x - metrics.width / 2, - y: opts.y - metrics.height / 2, - width: metrics.width, - height: metrics.height, - baseline: metrics.baseline, - }; + const textElement = newElementWith( + { + ..._newElementBase("text", opts), + isDeleted: false, + text: text, + font: font, + // Center the text + x: opts.x - metrics.width / 2, + y: opts.y - metrics.height / 2, + width: metrics.width, + height: metrics.height, + baseline: metrics.baseline, + }, + {}, + ); return textElement; } diff --git a/src/element/textWysiwyg.tsx b/src/element/textWysiwyg.tsx index af328998..22e6c041 100644 --- a/src/element/textWysiwyg.tsx +++ b/src/element/textWysiwyg.tsx @@ -21,6 +21,7 @@ type TextWysiwygParams = { opacity: number; zoom: number; angle: number; + onChange?: (text: string) => void; onSubmit: (text: string) => void; onCancel: () => void; }; @@ -34,6 +35,7 @@ export function textWysiwyg({ opacity, zoom, angle, + onChange, onSubmit, onCancel, }: TextWysiwygParams) { @@ -96,6 +98,12 @@ export function textWysiwyg({ } }; + if (onChange) { + editable.oninput = () => { + onChange(trimText(editable.innerText)); + }; + } + editable.onkeydown = (ev) => { if (ev.key === KEYS.ESCAPE) { ev.preventDefault(); @@ -121,9 +129,6 @@ export function textWysiwyg({ } function cleanup() { - editable.onblur = null; - editable.onkeydown = null; - editable.onpaste = null; window.removeEventListener("wheel", stopEvent, true); document.body.removeChild(editable); }