fix: support collaboration in bound text (#4573)

* fix: support collaboration in bounded text

* align implementation irrespective of collab/submit

* don't wrap when submitted

* fix

* tests: exit editor via ESCAPE instead to remove async hacks

* simplify and remove dead comment

* remove mutating coords in submit since its taken care in updateWysiwygStyle

Co-authored-by: dwelle <luzar.david@gmail.com>
This commit is contained in:
Aakansha Doshi 2022-01-17 17:35:35 +05:30 committed by GitHub
parent 0850ab0dd0
commit 24bf4cb5fb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 56 additions and 82 deletions

View File

@ -476,7 +476,7 @@ class App extends React.Component<AppProps, AppState> {
zenModeEnabled={zenModeEnabled} zenModeEnabled={zenModeEnabled}
toggleZenMode={this.toggleZenMode} toggleZenMode={this.toggleZenMode}
langCode={getLanguage().code} langCode={getLanguage().code}
isCollaborating={this.props.isCollaborating || false} isCollaborating={this.props.isCollaborating}
renderTopRightUI={renderTopRightUI} renderTopRightUI={renderTopRightUI}
renderCustomFooter={renderFooter} renderCustomFooter={renderFooter}
viewModeEnabled={viewModeEnabled} viewModeEnabled={viewModeEnabled}
@ -1912,20 +1912,15 @@ class App extends React.Component<AppProps, AppState> {
text: string, text: string,
originalText: string, originalText: string,
isDeleted: boolean, isDeleted: boolean,
isSubmit: boolean,
) => { ) => {
this.scene.replaceAllElements([ this.scene.replaceAllElements([
...this.scene.getElementsIncludingDeleted().map((_element) => { ...this.scene.getElementsIncludingDeleted().map((_element) => {
if (_element.id === element.id && isTextElement(_element)) { if (_element.id === element.id && isTextElement(_element)) {
return updateTextElement( return updateTextElement(_element, {
_element, text,
{ isDeleted,
text, originalText,
isDeleted, });
originalText,
},
isSubmit,
);
} }
return _element; return _element;
}), }),
@ -1950,14 +1945,14 @@ class App extends React.Component<AppProps, AppState> {
]; ];
}, },
onChange: withBatchedUpdates((text) => { onChange: withBatchedUpdates((text) => {
updateElement(text, text, false, false); updateElement(text, text, false);
if (isNonDeletedElement(element)) { if (isNonDeletedElement(element)) {
updateBoundElements(element); updateBoundElements(element);
} }
}), }),
onSubmit: withBatchedUpdates(({ text, viaKeyboard, originalText }) => { onSubmit: withBatchedUpdates(({ text, viaKeyboard, originalText }) => {
const isDeleted = !text.trim(); const isDeleted = !text.trim();
updateElement(text, originalText, isDeleted, true); updateElement(text, originalText, isDeleted);
// select the created text element only if submitting via keyboard // select the created text element only if submitting via keyboard
// (when submitting via click it should act as signal to deselect) // (when submitting via click it should act as signal to deselect)
if (!isDeleted && viaKeyboard) { if (!isDeleted && viaKeyboard) {
@ -1997,7 +1992,7 @@ class App extends React.Component<AppProps, AppState> {
// do an initial update to re-initialize element position since we were // 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) // modifying element's x/y for sake of editor (case: syncing to remote)
updateElement(element.text, element.originalText, false, false); updateElement(element.text, element.originalText, false);
} }
private deselectElements() { private deselectElements() {

View File

@ -21,7 +21,7 @@ import { AppState } from "../types";
import { getElementAbsoluteCoords } from "."; import { getElementAbsoluteCoords } from ".";
import { adjustXYWithRotation } from "../math"; import { adjustXYWithRotation } from "../math";
import { getResizedElementAbsoluteCoords } from "./bounds"; import { getResizedElementAbsoluteCoords } from "./bounds";
import { getContainerElement, measureText } from "./textElement"; import { getContainerElement, measureText, wrapText } from "./textElement";
import { isBoundToContainer } from "./typeChecks"; import { isBoundToContainer } from "./typeChecks";
import { BOUND_TEXT_PADDING } from "../constants"; import { BOUND_TEXT_PADDING } from "../constants";
@ -247,17 +247,17 @@ export const updateTextElement = (
text, text,
isDeleted, isDeleted,
originalText, originalText,
}: { text: string; isDeleted?: boolean; originalText: string }, }: {
text: string;
isSubmit: boolean, isDeleted?: boolean;
originalText: string;
},
): ExcalidrawTextElement => { ): ExcalidrawTextElement => {
const boundToContainer = isBoundToContainer(element); const container = getContainerElement(element);
if (container) {
// Don't update dimensions and text value for bounded text unless submitted text = wrapText(text, getFontString(element), container.width);
const dimensions = }
boundToContainer && !isSubmit const dimensions = getAdjustedDimensions(element, text);
? undefined
: getAdjustedDimensions(element, text);
return newElementWith(element, { return newElementWith(element, {
text, text,
originalText, originalText,

View File

@ -8,7 +8,11 @@ import {
import Scene from "../scene/Scene"; import Scene from "../scene/Scene";
import { isBoundToContainer, isTextElement } from "./typeChecks"; import { isBoundToContainer, isTextElement } from "./typeChecks";
import { CLASSES, BOUND_TEXT_PADDING } from "../constants"; import { CLASSES, BOUND_TEXT_PADDING } from "../constants";
import { ExcalidrawElement, ExcalidrawTextElement } from "./types"; import {
ExcalidrawElement,
ExcalidrawTextElement,
ExcalidrawLinearElement,
} from "./types";
import { AppState } from "../types"; import { AppState } from "../types";
import { mutateElement } from "./mutateElement"; import { mutateElement } from "./mutateElement";
import { import {
@ -100,7 +104,6 @@ export const textWysiwyg = ({
let originalContainerHeight: number; let originalContainerHeight: number;
let approxLineHeight = getApproxLineHeight(getFontString(element)); let approxLineHeight = getApproxLineHeight(getFontString(element));
const initialText = element.originalText;
const updateWysiwygStyle = () => { const updateWysiwygStyle = () => {
const updatedElement = Scene.getScene(element)?.getElement(id); const updatedElement = Scene.getScene(element)?.getElement(id);
if (updatedElement && isTextElement(updatedElement)) { if (updatedElement && isTextElement(updatedElement)) {
@ -222,6 +225,7 @@ export const textWysiwyg = ({
if (isTestEnv()) { if (isTestEnv()) {
editable.style.fontFamily = getFontFamilyString(updatedElement); editable.style.fontFamily = getFontFamilyString(updatedElement);
} }
mutateElement(updatedElement, { x: coordX, y: coordY });
} }
}; };
@ -442,61 +446,41 @@ export const textWysiwyg = ({
// it'd get stuck in an infinite loop of blur→onSubmit after we re-focus the // it'd get stuck in an infinite loop of blur→onSubmit after we re-focus the
// wysiwyg on update // wysiwyg on update
cleanup(); cleanup();
const updateElement = Scene.getScene(element)?.getElement(element.id); const updateElement = Scene.getScene(element)?.getElement(
element.id,
) as ExcalidrawTextElement;
if (!updateElement) { if (!updateElement) {
return; return;
} }
let wrappedText = ""; let text = editable.value;
if (isTextElement(updateElement) && updateElement?.containerId) { const container = getContainerElement(updateElement);
const container = getContainerElement(updateElement);
if (container) { if (container) {
wrappedText = wrapText( text = updateElement.text;
editable.value, if (editable.value) {
getFontString(updateElement), const boundTextElementId = getBoundTextElementId(container);
container.width, if (!boundTextElementId || boundTextElementId !== element.id) {
); mutateElement(container, {
boundElements: (container.boundElements || []).concat({
if (updateElement.containerId) { type: "text",
const editorHeight = Number(editable.style.height.slice(0, -2)); id: element.id,
if (editable.value) { }),
// Don't mutate if text is not updated });
if (initialText !== editable.value) {
mutateElement(updateElement, {
// vertically center align
y: container.y + container.height / 2 - editorHeight / 2,
height: editorHeight,
width: Number(editable.style.width.slice(0, -2)),
// preserve padding
x: container.x + BOUND_TEXT_PADDING,
angle: container.angle,
});
}
const boundTextElementId = getBoundTextElementId(container);
if (!boundTextElementId || boundTextElementId !== element.id) {
mutateElement(container, {
boundElements: (container.boundElements || []).concat({
type: "text",
id: element.id,
}),
});
}
} else {
mutateElement(container, {
boundElements: container.boundElements?.filter(
(ele) => ele.type !== "text",
),
});
}
} }
} else {
mutateElement(container, {
boundElements: container.boundElements?.filter(
(ele) =>
!isTextElement(
ele as ExcalidrawTextElement | ExcalidrawLinearElement,
),
),
});
} }
} else {
wrappedText = editable.value;
} }
onSubmit({ onSubmit({
text: normalizeText(wrappedText), text,
viaKeyboard: submittedViaKeyboard, viaKeyboard: submittedViaKeyboard,
originalText: editable.value, originalText: editable.value,
}); });

View File

@ -17,7 +17,7 @@ const Excalidraw = (props: ExcalidrawProps) => {
initialData, initialData,
excalidrawRef, excalidrawRef,
onCollabButtonClick, onCollabButtonClick,
isCollaborating, isCollaborating = false,
onPointerUpdate, onPointerUpdate,
renderTopRightUI, renderTopRightUI,
renderFooter, renderFooter,

View File

@ -158,11 +158,8 @@ describe("element binding", () => {
expect(editor).not.toBe(null); expect(editor).not.toBe(null);
// we defer binding blur event on wysiwyg, hence wait a bit
await new Promise((r) => setTimeout(r, 30));
fireEvent.change(editor, { target: { value: "" } }); fireEvent.change(editor, { target: { value: "" } });
editor.blur(); fireEvent.keyDown(editor, { key: KEYS.ESCAPE });
expect( expect(
document.querySelector(".excalidraw-textEditorContainer > textarea"), document.querySelector(".excalidraw-textEditorContainer > textarea"),
@ -202,11 +199,8 @@ describe("element binding", () => {
expect(editor).not.toBe(null); expect(editor).not.toBe(null);
// we defer binding blur event on wysiwyg, hence wait a bit
await new Promise((r) => setTimeout(r, 30));
fireEvent.change(editor, { target: { value: "asdasdasdasdas" } }); fireEvent.change(editor, { target: { value: "asdasdasdasdas" } });
editor.blur(); fireEvent.keyDown(editor, { key: KEYS.ESCAPE });
expect( expect(
document.querySelector(".excalidraw-textEditorContainer > textarea"), document.querySelector(".excalidraw-textEditorContainer > textarea"),

View File

@ -300,6 +300,7 @@ export type AppProps = ExcalidrawProps & {
}; };
detectScroll: boolean; detectScroll: boolean;
handleKeyboardGlobally: boolean; handleKeyboardGlobally: boolean;
isCollaborating: boolean;
}; };
/** A subset of App class properties that we need to use elsewhere /** A subset of App class properties that we need to use elsewhere