Browse Source

fix(frontend): improve the <Input /> component (#1195)

* create a better chat input component

* remove the useInputComposition hook
sp.wack 1 year ago
parent
commit
ca1d53c161

+ 131 - 0
frontend/src/components/ChatInput.test.tsx

@@ -0,0 +1,131 @@
+import React from "react";
+import userEvent from "@testing-library/user-event";
+import { act, render } from "@testing-library/react";
+import ChatInput from "./ChatInput";
+
+const tMock = vi.fn((key: string) => key);
+
+vi.mock("react-i18next", () => ({
+  useTranslation: () => ({ t: tMock }),
+}));
+
+describe("ChatInput", () => {
+  const onSendMessage = vi.fn();
+
+  afterEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("should render a textarea", () => {
+    const { getByRole } = render(<ChatInput onSendMessage={onSendMessage} />);
+    const textarea = getByRole("textbox");
+    expect(textarea).toBeInTheDocument();
+  });
+
+  it("should be able to be set as disabled", () => {
+    const { getByRole } = render(
+      <ChatInput disabled onSendMessage={onSendMessage} />,
+    );
+    const textarea = getByRole("textbox");
+    expect(textarea).toBeDisabled();
+  });
+
+  // Note that this test only checks that the placeholder is rendered, not the actual value
+  it("should render with a placeholder", () => {
+    tMock.mockReturnValue("value-returned-by-t");
+
+    const { getByPlaceholderText } = render(
+      <ChatInput onSendMessage={onSendMessage} />,
+    );
+    const textarea = getByPlaceholderText("value-returned-by-t");
+    expect(textarea).toBeInTheDocument();
+  });
+
+  it("should render a send button", () => {
+    const { getByRole } = render(<ChatInput onSendMessage={onSendMessage} />);
+    const button = getByRole("button");
+    expect(button).toBeInTheDocument();
+  });
+
+  it("should call sendChatMessage with the input when the send button is clicked", () => {
+    const { getByRole } = render(<ChatInput onSendMessage={onSendMessage} />);
+    const textarea = getByRole("textbox");
+    const button = getByRole("button");
+
+    act(() => {
+      userEvent.type(textarea, "Hello, world!");
+      userEvent.click(button);
+    });
+
+    expect(onSendMessage).toHaveBeenCalledWith("Hello, world!");
+  });
+
+  it("should be able to send a message when the enter key is pressed", () => {
+    const { getByRole } = render(<ChatInput onSendMessage={onSendMessage} />);
+    const textarea = getByRole("textbox");
+
+    act(() => {
+      userEvent.type(textarea, "Hello, world!{enter}");
+    });
+
+    expect(onSendMessage).toHaveBeenCalledWith("Hello, world!");
+  });
+
+  it("should NOT send a message when shift + enter is pressed", () => {
+    const { getByRole } = render(<ChatInput onSendMessage={onSendMessage} />);
+    const textarea = getByRole("textbox");
+
+    act(() => {
+      userEvent.type(textarea, "Hello, world!{shift}{enter}");
+    });
+
+    expect(onSendMessage).not.toHaveBeenCalled();
+  });
+
+  it("should NOT send an empty message", () => {
+    const { getByRole } = render(<ChatInput onSendMessage={onSendMessage} />);
+    const textarea = getByRole("textbox");
+    const button = getByRole("button");
+
+    act(() => {
+      userEvent.type(textarea, " {enter}"); // Only whitespace
+    });
+
+    expect(onSendMessage).not.toHaveBeenCalled();
+
+    act(() => {
+      userEvent.click(button);
+    });
+
+    expect(onSendMessage).not.toHaveBeenCalled();
+  });
+
+  it("should clear the input message after sending a message", () => {
+    const { getByRole } = render(<ChatInput onSendMessage={onSendMessage} />);
+    const textarea = getByRole("textbox");
+    const button = getByRole("button");
+
+    act(() => {
+      userEvent.type(textarea, "Hello, world!");
+    });
+
+    expect(textarea).toHaveValue("Hello, world!");
+
+    act(() => {
+      userEvent.click(button);
+    });
+
+    expect(textarea).toHaveValue("");
+
+    act(() => {
+      userEvent.type(textarea, "Hello, world!{enter}");
+    });
+
+    expect(textarea).toHaveValue(""); // no new line
+  });
+
+  // this is already implemented but need to figure out how to test it
+  it.todo(
+    "should NOT send a message when the enter key is pressed while composing",
+  );
+});

+ 73 - 0
frontend/src/components/ChatInput.tsx

@@ -0,0 +1,73 @@
+import { Textarea } from "@nextui-org/react";
+import React from "react";
+import { useTranslation } from "react-i18next";
+import { twMerge } from "tailwind-merge";
+import { VscSend } from "react-icons/vsc";
+import { I18nKey } from "../i18n/declaration";
+
+interface ChatInputProps {
+  disabled?: boolean;
+  onSendMessage: (message: string) => void;
+}
+
+function ChatInput({ disabled, onSendMessage }: ChatInputProps) {
+  const { t } = useTranslation();
+
+  const [message, setMessage] = React.useState("");
+  // This is true when the user is typing in an IME (e.g., Chinese, Japanese)
+  const [isComposing, setIsComposing] = React.useState(false);
+
+  const handleSendChatMessage = () => {
+    if (message.trim()) {
+      onSendMessage(message);
+      setMessage("");
+    }
+  };
+
+  const onKeyPress = (event: React.KeyboardEvent<HTMLInputElement>) => {
+    if (event.key === "Enter" && !event.shiftKey && !isComposing) {
+      event.preventDefault(); // prevent a new line
+      handleSendChatMessage();
+    }
+  };
+
+  return (
+    <div className="w-full relative text-base">
+      <Textarea
+        value={message}
+        onChange={(e) => setMessage(e.target.value)}
+        disabled={disabled}
+        onKeyDown={onKeyPress}
+        onCompositionStart={() => setIsComposing(true)}
+        onCompositionEnd={() => setIsComposing(false)}
+        placeholder={t(I18nKey.CHAT_INTERFACE$INPUT_PLACEHOLDER)}
+        className="pt-2 pb-4 px-4"
+        classNames={{
+          inputWrapper: "bg-neutral-700",
+          input: "pr-16 py-2",
+        }}
+        maxRows={10}
+        minRows={1}
+        variant="bordered"
+      />
+
+      <button
+        type="button"
+        onClick={handleSendChatMessage}
+        className={twMerge(
+          "bg-transparent border-none rounded py-2.5 px-5 hover:opacity-80 cursor-pointer select-none absolute right-5 bottom-6",
+          disabled && "cursor-not-allowed opacity-80",
+        )}
+        aria-label="Send message"
+      >
+        <VscSend />
+      </button>
+    </div>
+  );
+}
+
+ChatInput.defaultProps = {
+  disabled: false,
+};
+
+export default ChatInput;

+ 5 - 2
frontend/src/components/ChatInterface.tsx

@@ -5,13 +5,14 @@ import { useSelector } from "react-redux";
 import { useTypingEffect } from "../hooks/useTypingEffect";
 import {
   addAssistantMessageToChat,
+  sendChatMessage,
   setTypingActive,
   takeOneAndType,
 } from "../services/chatService";
 import { Message } from "../state/chatSlice";
 import { RootState } from "../store";
 import AgentStatusBar from "./AgentStatusBar";
-import Input from "./Input";
+import ChatInput from "./ChatInput";
 import AgentControlBar from "./AgentControlBar";
 
 interface IChatBubbleProps {
@@ -114,6 +115,8 @@ function MessageList(): JSX.Element {
 }
 
 function ChatInterface(): JSX.Element {
+  const { initialized } = useSelector((state: RootState) => state.task);
+
   return (
     <div className="flex flex-col h-full p-0 bg-neutral-800">
       <div className="flex items-center gap-2 border-b border-neutral-600 text-sm px-4 py-2">
@@ -123,7 +126,7 @@ function ChatInterface(): JSX.Element {
       <MessageList />
       <AgentStatusBar />
       <AgentControlBar />
-      <Input />
+      <ChatInput disabled={!initialized} onSendMessage={sendChatMessage} />
     </div>
   );
 }

+ 0 - 78
frontend/src/components/Input.tsx

@@ -1,78 +0,0 @@
-import { Textarea } from "@nextui-org/react";
-import React, { ChangeEvent, KeyboardEvent, useState } from "react";
-import { useTranslation } from "react-i18next";
-import { VscSend } from "react-icons/vsc";
-import { useSelector } from "react-redux";
-import { twMerge } from "tailwind-merge";
-import useInputComposition from "../hooks/useInputComposition";
-import { I18nKey } from "../i18n/declaration";
-import { sendChatMessage } from "../services/chatService";
-import { RootState } from "../store";
-
-function Input() {
-  const { t } = useTranslation();
-  const { initialized } = useSelector((state: RootState) => state.task);
-  const [inputMessage, setInputMessage] = useState("");
-
-  const handleSendMessage = () => {
-    if (inputMessage.trim() !== "") {
-      sendChatMessage(inputMessage);
-      setInputMessage("");
-    }
-  };
-
-  const { onCompositionEnd, onCompositionStart, isComposing } =
-    useInputComposition();
-
-  const handleChangeInputMessage = (e: ChangeEvent<HTMLInputElement>) => {
-    if (e.target.value !== "\n") {
-      setInputMessage(e.target.value);
-    }
-  };
-
-  const handleSendMessageOnEnter = (e: KeyboardEvent<HTMLInputElement>) => {
-    if (e.key === "Enter" && !e.shiftKey) {
-      // Prevent "Enter" from sending during IME input (e.g., Chinese, Japanese)
-      if (isComposing) {
-        return;
-      }
-      e.preventDefault();
-      handleSendMessage();
-    }
-  };
-
-  return (
-    <div className="w-full relative text-base">
-      <Textarea
-        className="pt-2 pb-4 px-4"
-        classNames={{
-          inputWrapper: "bg-neutral-700",
-          input: "pr-16 py-2",
-        }}
-        value={inputMessage}
-        maxRows={10}
-        minRows={1}
-        variant="bordered"
-        onChange={handleChangeInputMessage}
-        onKeyDown={handleSendMessageOnEnter}
-        onCompositionStart={onCompositionStart}
-        onCompositionEnd={onCompositionEnd}
-        placeholder={t(I18nKey.CHAT_INTERFACE$INPUT_PLACEHOLDER)}
-      />
-      <button
-        type="button"
-        className={twMerge(
-          "bg-transparent border-none rounded py-2.5 px-5 hover:opacity-80 cursor-pointer select-none absolute right-5 bottom-6",
-          !initialized && "cursor-not-allowed opacity-80",
-        )}
-        onClick={handleSendMessage}
-        disabled={!initialized}
-        aria-label="Send message"
-      >
-        <VscSend />
-      </button>
-    </div>
-  );
-}
-
-export default Input;

+ 0 - 35
frontend/src/hooks/useInputComposition.test.ts

@@ -1,35 +0,0 @@
-import { act, renderHook } from "@testing-library/react";
-import useInputComposition from "./useInputComposition";
-
-describe("useInputComposition", () => {
-  it("should return isComposing as false by default", () => {
-    const { result } = renderHook(() => useInputComposition());
-    expect(result.current.isComposing).toBe(false);
-  });
-
-  it("should set isComposing to true when onCompositionStart is called", () => {
-    const { result } = renderHook(() => useInputComposition());
-
-    act(() => {
-      result.current.onCompositionStart();
-    });
-
-    expect(result.current.isComposing).toBe(true);
-  });
-
-  it("should set isComposing to false when onCompositionEnd is called", () => {
-    const { result } = renderHook(() => useInputComposition());
-
-    act(() => {
-      result.current.onCompositionStart();
-    });
-
-    expect(result.current.isComposing).toBe(true);
-
-    act(() => {
-      result.current.onCompositionEnd();
-    });
-
-    expect(result.current.isComposing).toBe(false);
-  });
-});

+ 0 - 19
frontend/src/hooks/useInputComposition.ts

@@ -1,19 +0,0 @@
-import { useState } from "react";
-
-const useInputComposition = () => {
-  const [isComposing, setIsComposing] = useState(false);
-  const handleCompositionStart = () => {
-    setIsComposing(true);
-  };
-  const handleCompositionEnd = () => {
-    setIsComposing(false);
-  };
-
-  return {
-    isComposing,
-    onCompositionStart: handleCompositionStart,
-    onCompositionEnd: handleCompositionEnd,
-  };
-};
-
-export default useInputComposition;