fix(EditableTypography): should call onChange with empty string when …

…placeholder is used (#1932)
mondaycom · Feb 7, 2024 · b6ba0a5 · b6ba0a5
1 parent d46f93c
commit b6ba0a5
Show file tree

Hide file tree

Showing 2 changed files with 157 additions and 2 deletions.
diff --git a/src/components/EditableText/__tests__/editableText-tests.jest.tsx b/src/components/EditableText/__tests__/editableText-tests.jest.tsx
@@ -29,6 +29,29 @@ describe("EditableText", () => {
     expect(input).not.toBeInTheDocument();
   });
 
+  describe("rendered value", () => {
+    it("should render original value when value of an editable component is cleared", async () => {
+      const value = "Editable test";
+      render(<EditableText value={value} />);
+
+      const component = screen.getByRole("button");
+      fireEvent.click(component);
+
+      const input = screen.getByRole("input");
+      fireEvent.change(input, {
+        target: { value: "" }
+      });
+
+      expect(input).toHaveValue("");
+
+      await waitFor(() => {
+        fireEvent.keyDown(input, { key: "Enter" });
+      });
+
+      expect(within(screen.getByRole("button")).getByText(value)).toBeInTheDocument();
+    });
+  });
+
   describe("event handling", () => {
     describe("onClick", () => {
       const onClick = jest.fn();
@@ -50,13 +73,114 @@ describe("EditableText", () => {
         expect(onClick).toHaveBeenCalledTimes(1);
       });
     });
+
+    describe("onChange", () => {
+      const onChange = jest.fn();
+      it("should call onChange with new value when changed in an editable component", async () => {
+        const value = "Editable test";
+        const newValue = "New Editable test";
+        render(<EditableText value={value} onChange={onChange} />);
+
+        const component = screen.getByRole("button");
+        fireEvent.click(component);
+
+        const input = screen.getByRole("input");
+        fireEvent.change(input, {
+          target: { value: newValue }
+        });
+
+        expect(input).toHaveValue(newValue);
+
+        await waitFor(() => {
+          fireEvent.keyDown(input, { key: "Enter" });
+        });
+
+        expect(within(screen.getByRole("button")).getByText(newValue)).toBeInTheDocument();
+
+        expect(onChange).toHaveBeenCalledTimes(1);
+        expect(onChange).toHaveBeenCalledWith(newValue);
+      });
+
+      it("should not call onChange when value isn't changed in an editable component", async () => {
+        const value = "Editable test";
+        render(<EditableText value={value} onChange={onChange} />);
+
+        const component = screen.getByRole("button");
+        fireEvent.click(component);
+
+        const input = screen.getByRole("input");
+
+        expect(input).toHaveValue(value);
+
+        await waitFor(() => {
+          fireEvent.keyDown(input, { key: "Enter" });
+        });
+
+        expect(within(screen.getByRole("button")).getByText(value)).toBeInTheDocument();
+
+        expect(onChange).not.toBeCalled();
+      });
+
+      it("should not call onChange when value of an editable component is cleared", async () => {
+        const value = "Editable test";
+        render(<EditableText value={value} onChange={onChange} />);
+
+        const component = screen.getByRole("button");
+        fireEvent.click(component);
+
+        const input = screen.getByRole("input");
+        fireEvent.change(input, {
+          target: { value: "" }
+        });
+
+        expect(input).toHaveValue("");
+
+        await waitFor(() => {
+          fireEvent.keyDown(input, { key: "Enter" });
+        });
+
+        expect(within(screen.getByRole("button")).getByText(value)).toBeInTheDocument();
+        expect(onChange).not.toBeCalled();
+      });
+    });
+
+    describe("onEditModeChange", () => {
+      const onEditModeChange = jest.fn();
+      it("should call onEditModeChange with true when enter edit mode", async () => {
+        const value = "Editable test";
+        render(<EditableText value={value} onEditModeChange={onEditModeChange} />);
+
+        const component = screen.getByRole("button");
+        fireEvent.click(component);
+
+        expect(onEditModeChange).toHaveBeenCalledTimes(1);
+        expect(onEditModeChange).toHaveBeenCalledWith(true);
+      });
+
+      it("should call onEditModeChange with false when exit edit mode", async () => {
+        const value = "Editable test";
+        render(<EditableText value={value} onEditModeChange={onEditModeChange} />);
+
+        const component = screen.getByRole("button");
+        fireEvent.click(component);
+
+        const input = screen.getByRole("input");
+        await waitFor(() => {
+          fireEvent.keyDown(input, { key: "Enter" });
+        });
+
+        expect(onEditModeChange).toHaveBeenCalledTimes(2);
+        expect(onEditModeChange).toHaveBeenLastCalledWith(false);
+      });
+    });
   });
 
   describe("with placeholder", () => {
     it("should show a placeholder if provided and input is empty", async () => {
+      const onChange = jest.fn();
       const placeholderText = "Placeholder text";
       const value = "Editable test";
-      render(<EditableText value={value} placeholder={placeholderText} />);
+      render(<EditableText value={value} placeholder={placeholderText} onChange={onChange} />);
 
       const component = screen.getByRole("button");
       fireEvent.click(component);
@@ -74,6 +198,32 @@ describe("EditableText", () => {
       });
 
       expect(within(screen.getByRole("button")).getByText(placeholderText)).toBeInTheDocument();
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange).toHaveBeenCalledWith("");
+    });
+
+    it("should not show a placeholder if provided and input is not empty", async () => {
+      const placeholderText = "Placeholder text";
+      const value = "Editable test";
+      const newValue = "New Editable test";
+      render(<EditableText value={value} placeholder={placeholderText} />);
+
+      const component = screen.getByRole("button");
+      fireEvent.click(component);
+
+      const input = screen.getByRole("input");
+      fireEvent.change(input, {
+        target: { value: newValue }
+      });
+
+      expect(input).toHaveValue(newValue);
+      expect(screen.getByPlaceholderText(placeholderText)).toBeInTheDocument();
+
+      await waitFor(() => {
+        fireEvent.keyDown(input, { key: "Enter" });
+      });
+
+      expect(within(screen.getByRole("button")).getByText(newValue)).toBeInTheDocument();
     });
   });
 });
diff --git a/src/components/EditableTypography/EditableTypography.tsx b/src/components/EditableTypography/EditableTypography.tsx
@@ -94,11 +94,16 @@ const EditableTypography: VibeComponent<EditableTypographyProps, HTMLElement> =
     function handleInputValueChange() {
       handleEditModeChange(false);
 
+      if (value === inputValue) {
+        return;
+      }
+
       const shouldShowPlaceholderWhenEmpty = clearable && placeholder;
-      if ((!inputValue && !shouldShowPlaceholderWhenEmpty) || value === inputValue) {
+      if (!inputValue && !shouldShowPlaceholderWhenEmpty) {
         setInputValue(value);
         return;
       }
+      setInputValue(inputValue);
       onChange?.(inputValue);
     }