feat: Update to scroll behavior and UX tweaks (#77)

* feat: Update to scroll behavior and UX tweaks * chore: Added tests for ChatBox and ScrollToTop * chore: Updated Xpert tests * chore: Swapped some hardcoded colors with their respective scss variable
edx · Jan 2, 2025 · 375847b · 375847b
1 parent e7b1829
commit 375847b
Show file tree

Hide file tree

Showing 11 changed files with 243 additions and 130 deletions.
diff --git a/src/components/ChatBox/ChatBox.scss b/src/components/ChatBox/ChatBox.scss
@@ -1,30 +1,53 @@
-.scroller {
-  overflow-y: scroll;
-  scrollbar-width: thin;
-  width: 100%;
-  opacity: 1;
-  margin-right: 0;
+.xpert-chat-scroller {
+  position: relative;
+  flex: 1;
+
+  .messages-list {
+    overflow-y: scroll;
+    scrollbar-width: thin;
+    position: absolute;
+    inset: 0;
+    padding: 1rem 0;
+  }
 
   &:after {
     content: ""; /* Add an empty content area after the chat messages */
     display: block;
     height: 0;
     clear: both;
   }
-}
 
-.loading {
-  font-size: 13px;
-  padding-left: 10px;
+  .loading {
+    font-size: 13px;
+    padding-left: 10px;
 
-  &:after {
-    overflow: hidden;
-    display: inline-block;
-    vertical-align: bottom;
-    -webkit-animation: ellipsis steps(4,end) 900ms infinite;
-    animation: ellipsis steps(4,end) 900ms infinite;
-    content: "\2026"; /* ascii code for the ellipsis character */
-    width: 0px;
+    &:after {
+      overflow: hidden;
+      display: inline-block;
+      vertical-align: bottom;
+      -webkit-animation: ellipsis steps(4,end) 900ms infinite;
+      animation: ellipsis steps(4,end) 900ms infinite;
+      content: "\2026"; /* ascii code for the ellipsis character */
+      width: 0px;
+    }
+  }
+
+  .separator {
+    position: absolute;
+    z-index: 100;
+    height: 5px;
+    padding: 5px;
+    opacity: 0.3;
+
+    &--top {
+      inset: 0 0 auto 0;
+      background: linear-gradient(180deg, rgba(0, 0, 0, 0.35), transparent);
+    }
+
+    &--bottom {
+      inset: auto 0 0 0;
+      background: linear-gradient(0, rgba(0, 0, 0, 0.35), transparent);
+    }
   }
 }
 

diff --git a/src/components/ChatBox/index.jsx b/src/components/ChatBox/index.jsx
@@ -1,9 +1,9 @@
-import React from 'react';
+import React, { useEffect, useRef } from 'react';
 import { useSelector } from 'react-redux';
-import PropTypes from 'prop-types';
 import Message from '../Message';
 import './ChatBox.scss';
 import MessageDivider from '../MessageDivider';
+import { scrollToBottom } from '../../utils/scroll';
 
 function isToday(date) {
   const today = new Date();
@@ -15,33 +15,43 @@ function isToday(date) {
 }
 
 // container for all of the messages
-const ChatBox = ({ chatboxContainerRef }) => {
+const ChatBox = () => {
+  const firstRender = useRef(true);
+  const messageContainerRef = useRef();
+
   const { messageList, apiIsLoading } = useSelector(state => state.learningAssistant);
   const messagesBeforeToday = messageList.filter((m) => (!isToday(new Date(m.timestamp))));
   const messagesToday = messageList.filter((m) => (isToday(new Date(m.timestamp))));
 
-  // message divider should not display if no messages or if all messages sent today.
+  useEffect(() => {
+    if (firstRender.current) {
+      scrollToBottom(messageContainerRef);
+      firstRender.current = false;
+      return;
+    }
+
+    scrollToBottom(messageContainerRef, true);
+  }, [messageList.length]);
+
   return (
-    <div ref={chatboxContainerRef} className="flex-grow-1 scroller d-flex flex-column pb-4">
-      {messagesBeforeToday.map(({ role, content, timestamp }) => (
-        <Message key={timestamp} variant={role} message={content} />
-      ))}
-      {(messageList.length !== 0 && messagesBeforeToday.length !== 0) && (<MessageDivider text="Today" />)}
-      {messagesToday.map(({ role, content, timestamp }) => (
-        <Message key={timestamp} variant={role} message={content} />
-      ))}
-      {apiIsLoading && (
+    <div className="xpert-chat-scroller">
+      <div className="messages-list d-flex flex-column" ref={messageContainerRef} data-testid="messages-container">
+        {messagesBeforeToday.map(({ role, content, timestamp }) => (
+          <Message key={timestamp} variant={role} message={content} />
+        ))}
+        {/* Message divider should not display if no messages or if all messages sent today. */}
+        {(messageList.length !== 0 && messagesBeforeToday.length !== 0) && (<MessageDivider text="Today" />)}
+        {messagesToday.map(({ role, content, timestamp }) => (
+          <Message key={timestamp} variant={role} message={content} />
+        ))}
+        {apiIsLoading && (
         <div className="loading">Xpert is thinking</div>
-      )}
+        )}
+      </div>
+      <span className="separator separator--top" />
+      <span className="separator separator--bottom" />
     </div>
   );
 };
 
-ChatBox.propTypes = {
-  chatboxContainerRef: PropTypes.oneOfType([
-    PropTypes.func,
-    PropTypes.shape({ current: PropTypes.instanceOf(Element) }),
-  ]).isRequired,
-};
-
 export default ChatBox;
diff --git a/src/components/ChatBox/index.test.jsx b/src/components/ChatBox/index.test.jsx
@@ -1,8 +1,8 @@
 import React from 'react';
 import { screen, act } from '@testing-library/react';
-
 import { render as renderComponent } from '../../utils/utils.test';
-import { initialState } from '../../data/slice';
+import { initialState, setMessageList } from '../../data/slice';
+import { scrollToBottom } from '../../utils/scroll';
 
 import ChatBox from '.';
 
@@ -16,6 +16,8 @@ const defaultProps = {
   chatboxContainerRef: jest.fn(),
 };
 
+jest.mock('../../utils/scroll');
+
 const render = async (props = {}, sliceState = {}) => {
   const componentProps = {
     ...defaultProps,
@@ -30,27 +32,34 @@ const render = async (props = {}, sliceState = {}) => {
       },
     },
   };
-  return act(async () => renderComponent(
-    <ChatBox {...componentProps} />,
-    initState,
-  ));
+
+  let handlers;
+  await act(async () => {
+    handlers = renderComponent(
+      <ChatBox {...componentProps} />,
+      initState,
+    );
+  });
+
+  return { ...handlers, initState, componentProps };
 };
 
 describe('<ChatBox />', () => {
-  beforeEach(() => {
+  afterEach(() => {
     jest.resetAllMocks();
   });
-  it('message divider does not appear when no messages', () => {
+
+  it('message divider does not appear when no messages', async () => {
     const messageList = [];
     const sliceState = {
       messageList,
     };
-    render(undefined, sliceState);
+    await render(undefined, sliceState);
 
     expect(screen.queryByText('Today')).not.toBeInTheDocument();
   });
 
-  it('message divider does not appear when all messages from today', () => {
+  it('message divider does not appear when all messages from today', async () => {
     const date = new Date();
     const messageList = [
       { role: 'user', content: 'hi', timestamp: date - 60 },
@@ -59,14 +68,14 @@ describe('<ChatBox />', () => {
     const sliceState = {
       messageList,
     };
-    render(undefined, sliceState);
+    await render(undefined, sliceState);
 
     expect(screen.queryByText('hi')).toBeInTheDocument();
     expect(screen.queryByText('hello')).toBeInTheDocument();
     expect(screen.queryByText('Today')).not.toBeInTheDocument();
   });
 
-  it('message divider shows when all messages from before today', () => {
+  it('message divider shows when all messages from before today', async () => {
     const date = new Date();
     const messageList = [
       { role: 'user', content: 'hi', timestamp: date.setDate(date.getDate() - 1) },
@@ -75,14 +84,14 @@ describe('<ChatBox />', () => {
     const sliceState = {
       messageList,
     };
-    render(undefined, sliceState);
+    await render(undefined, sliceState);
 
     expect(screen.queryByText('hi')).toBeInTheDocument();
     expect(screen.queryByText('hello')).toBeInTheDocument();
     expect(screen.queryByText('Today')).toBeInTheDocument();
   });
 
-  it('correctly divides old and new messages', () => {
+  it('correctly divides old and new messages', async () => {
     const today = new Date();
     const messageList = [
       { role: 'user', content: 'Today yesterday', timestamp: today.setDate(today.getDate() - 1) },
@@ -91,12 +100,61 @@ describe('<ChatBox />', () => {
     const sliceState = {
       messageList,
     };
-    render(undefined, sliceState);
+    await render(undefined, sliceState);
 
     const results = screen.getAllByText('Today', { exact: false });
     expect(results.length).toBe(3);
     expect(results[0]).toHaveTextContent('Today yesterday');
     expect(results[1]).toHaveTextContent('Today');
     expect(results[2]).toHaveTextContent('Today today');
   });
+
+  it('scrolls to the last comment immediately when rendered', async () => {
+    const date = new Date();
+    const messageList = [
+      { role: 'user', content: 'hi', timestamp: date - 60 },
+      { role: 'user', content: 'hello', timestamp: date },
+    ];
+    const sliceState = {
+      messageList,
+    };
+
+    await act(() => render(undefined, sliceState));
+
+    const messagesContainer = screen.getByTestId('messages-container');
+
+    expect(scrollToBottom).toHaveBeenCalledWith({ current: messagesContainer });
+  });
+
+  it('scrolls to the last comment smoothly when adding messages', async () => {
+    const date = new Date();
+    const messageList = [
+      { role: 'user', content: 'hi', timestamp: date - 60 },
+      { role: 'user', content: 'hello', timestamp: date - 30 },
+    ];
+    const sliceState = {
+      messageList,
+    };
+
+    let store;
+
+    await act(async () => {
+      ({ store } = await render(undefined, sliceState));
+    });
+
+    const messagesContainer = screen.getByTestId('messages-container');
+
+    expect(scrollToBottom).toHaveBeenCalledWith({ current: messagesContainer });
+
+    act(() => {
+      store.dispatch(setMessageList({
+        messageList: [
+          ...messageList,
+          { role: 'user', content: 'New message', timestamp: +date },
+        ],
+      }));
+    });
+
+    expect(scrollToBottom).toHaveBeenCalledWith({ current: messagesContainer }, true);
+  });
 });
diff --git a/src/components/Message/index.jsx b/src/components/Message/index.jsx
@@ -5,7 +5,7 @@ import PropTypes from 'prop-types';
 
 const Message = ({ variant, message }) => (
   <div
-    className={`message ${variant} ${variant === 'user' ? 'align-self-end' : ''} text-left my-1 mx-2 py-2 px-3`}
+    className={`message ${variant} ${variant === 'user' ? 'align-self-end' : ''} text-left my-1 mx-4 py-2 px-3`}
     data-hj-suppress
   >
     <ReactMarkdown>

diff --git a/src/components/MessageForm/MessageForm.scss b/src/components/MessageForm/MessageForm.scss
@@ -1,11 +1,22 @@
 .message-form {
+  padding: 0.75rem 1.5rem;
+
   .send-message-input {
     .pgn__form-control-floating-label {
-      color: #ADADAD;
+      color: $gray-300;
     }
 
     input {
       border-radius: 1rem;
+      border: 1px solid $gray-200;
     }
   }
+
+  .pgn__form-control-decorator-group {
+    margin-inline-end: 0;
+  }
+
+  button {
+    color: $gray-400;
+  }
 }
diff --git a/src/components/MessageForm/index.jsx b/src/components/MessageForm/index.jsx
@@ -61,18 +61,16 @@ const MessageForm = ({ courseId, shouldAutofocus, unitId }) => {
 
   return (
     <Form className="message-form w-100" onSubmit={handleSubmitMessage} data-testid="message-form">
-      <Form.Group>
-        <Form.Control
-          data-hj-suppress
-          disabled={apiIsLoading}
-          floatingLabel="Write a message"
-          onChange={handleUpdateCurrentMessage}
-          trailingElement={getSubmitButton()}
-          value={currentMessage}
-          ref={inputRef}
-          className="send-message-input"
-        />
-      </Form.Group>
+      <Form.Control
+        data-hj-suppress
+        disabled={apiIsLoading}
+        floatingLabel="Write a message"
+        onChange={handleUpdateCurrentMessage}
+        trailingElement={getSubmitButton()}
+        value={currentMessage}
+        ref={inputRef}
+        className="send-message-input"
+      />
     </Form>
   );
 };