From d1072155640edbda6b047234018bd1446f1b202e Mon Sep 17 00:00:00 2001 From: David Luzar <5153846+dwelle@users.noreply.github.com> Date: Mon, 9 Sep 2024 19:57:22 +0200 Subject: [PATCH] fix: `select` instead of `focus` search input (#8483) --- packages/excalidraw/components/SearchMenu.tsx | 16 ++++------------ packages/excalidraw/components/TextField.tsx | 2 ++ packages/excalidraw/tests/helpers/api.ts | 7 ++++--- packages/excalidraw/tests/search.test.tsx | 12 +++++++----- 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/packages/excalidraw/components/SearchMenu.tsx b/packages/excalidraw/components/SearchMenu.tsx index f05660a52f..a7bf9b4473 100644 --- a/packages/excalidraw/components/SearchMenu.tsx +++ b/packages/excalidraw/components/SearchMenu.tsx @@ -10,8 +10,6 @@ import type { ExcalidrawTextElement } from "../element/types"; import { measureText } from "../element/textElement"; import { addEventListener, getFontString } from "../utils"; import { KEYS } from "../keys"; - -import "./SearchMenu.scss"; import clsx from "clsx"; import { atom, useAtom } from "jotai"; import { jotaiScope } from "../jotai"; @@ -21,6 +19,8 @@ import { randomInteger } from "../random"; import { CLASSES, EVENT } from "../constants"; import { useStable } from "../hooks/useStable"; +import "./SearchMenu.scss"; + const searchKeywordAtom = atom(""); export const searchItemInFocusAtom = atom(null); @@ -229,6 +229,7 @@ export const SearchMenu = () => { }); } searchInputRef.current?.focus(); + searchInputRef.current?.select(); } else { setAppState({ openSidebar: null, @@ -264,16 +265,6 @@ export const SearchMenu = () => { }); }, [setAppState, stableState, app]); - /** - * NOTE: - * - * for testing purposes, we're manually focusing instead of - * setting `selectOnRender` on - */ - useEffect(() => { - searchInputRef.current?.focus(); - }, []); - const matchCount = `${searchMatches.items.length} ${ searchMatches.items.length === 1 ? t("search.singleResult") @@ -292,6 +283,7 @@ export const SearchMenu = () => { onChange={(value) => { setKeyword(value); }} + selectOnRender /> diff --git a/packages/excalidraw/components/TextField.tsx b/packages/excalidraw/components/TextField.tsx index e1d3461112..c5bdc82609 100644 --- a/packages/excalidraw/components/TextField.tsx +++ b/packages/excalidraw/components/TextField.tsx @@ -51,6 +51,8 @@ export const TextField = forwardRef( useLayoutEffect(() => { if (selectOnRender) { + // focusing first is needed because vitest/jsdom + innerRef.current?.focus(); innerRef.current?.select(); } }, [selectOnRender]); diff --git a/packages/excalidraw/tests/helpers/api.ts b/packages/excalidraw/tests/helpers/api.ts index 409d5825b7..6c16e51909 100644 --- a/packages/excalidraw/tests/helpers/api.ts +++ b/packages/excalidraw/tests/helpers/api.ts @@ -76,11 +76,12 @@ export class API { }); }; - static updateElement = ( - ...[element, updates]: Parameters + // eslint-disable-next-line prettier/prettier + static updateElement = ( + ...args: Parameters> ) => { act(() => { - mutateElement(element, updates); + mutateElement(...args); }); }; diff --git a/packages/excalidraw/tests/search.test.tsx b/packages/excalidraw/tests/search.test.tsx index bed596c438..ae729b2101 100644 --- a/packages/excalidraw/tests/search.test.tsx +++ b/packages/excalidraw/tests/search.test.tsx @@ -1,6 +1,6 @@ import React from "react"; -import { render, waitFor } from "./test-utils"; -import { Excalidraw, mutateElement } from "../index"; +import { act, render, waitFor } from "./test-utils"; +import { Excalidraw } from "../index"; import { CLASSES, SEARCH_SIDEBAR } from "../constants"; import { Keyboard } from "./helpers/ui"; import { KEYS } from "../keys"; @@ -22,7 +22,7 @@ const querySearchInput = async () => { describe("search", () => { beforeEach(async () => { await render(); - h.setState({ + API.setAppState({ openSidebar: null, }); }); @@ -50,7 +50,9 @@ describe("search", () => { `.${CLASSES.SEARCH_MENU_INPUT_WRAPPER} input`, ); - searchInput?.blur(); + act(() => { + searchInput?.blur(); + }); expect(h.app.state.openSidebar).not.toBeNull(); expect(searchInput?.matches(":focus")).toBe(false); @@ -109,7 +111,7 @@ describe("search", () => { }), ]); - mutateElement(h.elements[0] as ExcalidrawTextElement, { + API.updateElement(h.elements[0] as ExcalidrawTextElement, { text: "t\ne\ns\nt \nt\ne\nx\nt \ns\np\nli\nt \ni\nn\nt\no\nm\nu\nlt\ni\np\nl\ne \nli\nn\ne\ns", originalText: "test text split into multiple lines", });