From 0258f374f14eaff403b38822c672b28a2554638e Mon Sep 17 00:00:00 2001
From: "D. Ror" <imnasnainaec@gmail.com>
Date: Tue, 9 Jul 2024 10:21:38 -0400
Subject: [PATCH] [CreateProject] Improve empty name handling (#3184)

---
 .../ProjectScreen/CreateProject.tsx           | 42 ++++++++++++-------
 .../tests/CreateProject.test.tsx              | 42 ++++++++++---------
 2 files changed, 51 insertions(+), 33 deletions(-)

diff --git a/src/components/ProjectScreen/CreateProject.tsx b/src/components/ProjectScreen/CreateProject.tsx
index 9e58d8817e..38daa16d3f 100644
--- a/src/components/ProjectScreen/CreateProject.tsx
+++ b/src/components/ProjectScreen/CreateProject.tsx
@@ -16,6 +16,7 @@ import {
   type FormEvent,
   Fragment,
   type ReactElement,
+  useEffect,
   useState,
 } from "react";
 import { Trans, useTranslation } from "react-i18next";
@@ -55,6 +56,14 @@ export default function CreateProject(): ReactElement {
   const [vernLangIsOther, setVernLangIsOther] = useState(false);
   const [vernLangOptions, setVernLangOptions] = useState<WritingSystem[]>([]);
 
+  useEffect(() => {
+    // Turn on the empty name error if the name is empty and another field isn't empty.
+    const empty =
+      !name.trim() &&
+      (!!languageData || (!!vernLang.bcp47 && vernLang.bcp47 !== undBcp47));
+    setError((prev) => (empty === prev.empty ? prev : { ...prev, empty }));
+  }, [languageData, name, vernLang.bcp47]);
+
   const { t } = useTranslation();
 
   const setVernBcp47 = (bcp47: string): void => {
@@ -98,9 +107,10 @@ export default function CreateProject(): ReactElement {
   const updateName = (
     e: ChangeEvent<HTMLTextAreaElement | HTMLInputElement | HTMLSelectElement>
   ): void => {
-    const name = e.target.value;
-    setName(name);
-    setError({ empty: !name, nameTaken: false });
+    setName(e.target.value);
+
+    // Turn off the nameTaken error when name is changed.
+    setError((prev) => (prev.nameTaken ? { ...prev, nameTaken: false } : prev));
   };
 
   const updateLanguageData = async (langData?: File): Promise<void> => {
@@ -159,15 +169,17 @@ export default function CreateProject(): ReactElement {
 
   const createProject = async (e: FormEvent<EventTarget>): Promise<void> => {
     e.preventDefault();
-    if (success) {
-      return;
-    }
     const trimmedName = name.trim();
-    if (!trimmedName) {
-      setError({ empty: true, nameTaken: false });
+    if (
+      success ||
+      !trimmedName ||
+      !vernLang.bcp47 ||
+      vernLang.bcp47 === undBcp47
+    ) {
+      // Backstop for cases when this function shouldn't have run in the first place.
       return;
     }
-    if (await projectDuplicateCheck(name)) {
+    if (await projectDuplicateCheck(trimmedName)) {
       setError({ empty: false, nameTaken: true });
       return;
     }
@@ -175,13 +187,13 @@ export default function CreateProject(): ReactElement {
     setLoading(true);
 
     if (languageData) {
-      await dispatch(asyncFinishProject(name, vernLang)).then(() =>
+      await dispatch(asyncFinishProject(trimmedName, vernLang)).then(() =>
         setSuccess(true)
       );
     } else {
-      await dispatch(asyncCreateProject(name, vernLang, [analysisLang])).then(
-        () => setSuccess(true)
-      );
+      await dispatch(
+        asyncCreateProject(trimmedName, vernLang, [analysisLang])
+      ).then(() => setSuccess(true));
     }
   };
 
@@ -294,7 +306,9 @@ export default function CreateProject(): ReactElement {
           >
             <LoadingDoneButton
               buttonProps={{ color: "primary", id: buttonIdSubmit }}
-              disabled={!vernLang.bcp47 || vernLang.bcp47 === undBcp47}
+              disabled={
+                !name.trim() || !vernLang.bcp47 || vernLang.bcp47 === undBcp47
+              }
               done={success}
               doneText={t("createProject.success")}
               loading={loading}
diff --git a/src/components/ProjectScreen/tests/CreateProject.test.tsx b/src/components/ProjectScreen/tests/CreateProject.test.tsx
index 85b48cdd0c..866998ba50 100644
--- a/src/components/ProjectScreen/tests/CreateProject.test.tsx
+++ b/src/components/ProjectScreen/tests/CreateProject.test.tsx
@@ -44,7 +44,8 @@ const mockSubmitEvent = (): Partial<FormEvent<HTMLFormElement>> => ({
 let projectMaster: ReactTestRenderer;
 let projectHandle: ReactTestInstance;
 
-beforeAll(async () => {
+beforeEach(async () => {
+  jest.resetAllMocks();
   await act(async () => {
     projectMaster = create(
       <Provider store={mockStore}>
@@ -55,27 +56,13 @@ beforeAll(async () => {
   projectHandle = projectMaster.root;
 });
 
-beforeEach(() => {
-  jest.resetAllMocks();
-});
-
 describe("CreateProject", () => {
-  it("errors on empty name", async () => {
-    const nameField = projectHandle.findByProps({ id: fieldIdName });
-    expect(nameField.props.error).toBeFalsy();
-
-    await act(async () => {
-      projectHandle
-        .findByProps({ id: formId })
-        .props.onSubmit(mockSubmitEvent());
-    });
-    expect(nameField.props.error).toBeTruthy();
-  });
-
   it("errors on taken name", async () => {
     const nameField = projectHandle.findByProps({ id: fieldIdName });
+    const langPickers = projectHandle.findAllByType(LanguagePicker);
     await act(async () => {
-      nameField.props.onChange(mockChangeEvent("non-empty-value"));
+      nameField.props.onChange(mockChangeEvent("non-empty-name"));
+      langPickers[0].props.setCode("non-empty-code");
     });
     expect(nameField.props.error).toBeFalsy();
 
@@ -88,9 +75,20 @@ describe("CreateProject", () => {
     expect(nameField.props.error).toBeTruthy();
   });
 
-  it("disables submit button when no vern lang bcp code", async () => {
+  it("disables submit button when empty name or empty vern lang bcp code", async () => {
+    const nameField = projectHandle.findByProps({ id: fieldIdName });
     const button = projectHandle.findByProps({ id: buttonIdSubmit });
+
+    // Start with empty name and vern language: button disabled.
+    expect(button.props.disabled).toBeTruthy();
+
+    // Add name but still no vern language: button still disabled.
+    await act(async () => {
+      nameField.props.onChange(mockChangeEvent("non-empty-value"));
+    });
     expect(button.props.disabled).toBeTruthy();
+
+    // Also add a vern language: button enabled.
     const langPickers = projectHandle.findAllByType(LanguagePicker);
     expect(langPickers).toHaveLength(2);
 
@@ -98,6 +96,12 @@ describe("CreateProject", () => {
       langPickers[0].props.setCode("non-empty");
     });
     expect(button.props.disabled).toBeFalsy();
+
+    // Change name to whitespace: button disabled again.
+    await act(async () => {
+      nameField.props.onChange(mockChangeEvent("   "));
+    });
+    expect(button.props.disabled).toBeTruthy();
   });
 
   it("disables analysis language pickers when file selected", async () => {