From 0c26e980ee26845d197609210d56f03d411646f6 Mon Sep 17 00:00:00 2001 From: michaelroytman Date: Wed, 16 Aug 2023 20:32:11 -0400 Subject: [PATCH 1/3] fix: Xpert styling This commit includes a number of CSS fixes and changes. The fundamental issue is that the existing Xpert code used Tailwind classes, which we do not have access to in the learning MFE. Furthermore, the existing Xpert code did not make use of Bootstrap. This commit replaces Tailwind classes with Bootstrap classes, where equivalent Bootstrap classes exist. Tailwind classes that did not have an equivalent Bootstrap class are replaced by equivalent CSS. The CSS files were converted to SCSS files. Other changes include conditionally rendering the Sidebar component instead of relying on opacity and absolute positioning to hide and show the Sidebar, remove unncessary styling, and relying on flexbox for positioning where possible over absolute positioning. --- src/components/ChatBox/ChatBox.css | 38 ------ src/components/ChatBox/ChatBox.scss | 36 ++++++ src/components/ChatBox/index.jsx | 4 +- src/components/Message/Message.css | 25 ---- src/components/Message/Message.scss | 19 +++ src/components/Message/index.jsx | 10 +- src/components/Sidebar/Sidebar.css | 33 ------ src/components/Sidebar/Sidebar.scss | 81 +++++++++++++ src/components/Sidebar/index.jsx | 123 ++++++++++---------- src/components/ToggleXpertButton/index.css | 13 --- src/components/ToggleXpertButton/index.jsx | 10 +- src/components/ToggleXpertButton/index.scss | 22 ++++ 12 files changed, 235 insertions(+), 179 deletions(-) delete mode 100644 src/components/ChatBox/ChatBox.css create mode 100644 src/components/ChatBox/ChatBox.scss delete mode 100644 src/components/Message/Message.css create mode 100644 src/components/Message/Message.scss delete mode 100644 src/components/Sidebar/Sidebar.css create mode 100644 src/components/Sidebar/Sidebar.scss delete mode 100644 src/components/ToggleXpertButton/index.css create mode 100644 src/components/ToggleXpertButton/index.scss diff --git a/src/components/ChatBox/ChatBox.css b/src/components/ChatBox/ChatBox.css deleted file mode 100644 index 7776eb3..0000000 --- a/src/components/ChatBox/ChatBox.css +++ /dev/null @@ -1,38 +0,0 @@ - -.scroller { - overflow-y: scroll; - scrollbar-color: rebeccapurple green; - scrollbar-width: thin; - width: 100%; - opacity: 1; - margin-right: 0; -} - -.scroller::after { - content: ""; /* Add an empty content area after the chat messages */ - display: block; - height: 0; - clear: both; -} - -.loading { - text-align: left; - font-size: 13px; - padding-left: 10px; -} - -.loading: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; -} - -@keyframes ellipsis { - to { - width: 1.25em; - } -} diff --git a/src/components/ChatBox/ChatBox.scss b/src/components/ChatBox/ChatBox.scss new file mode 100644 index 0000000..aab9574 --- /dev/null +++ b/src/components/ChatBox/ChatBox.scss @@ -0,0 +1,36 @@ +.scroller { + overflow-y: scroll; + scrollbar-color: rebeccapurple green; + scrollbar-width: thin; + width: 100%; + opacity: 1; + margin-right: 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; + + &: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; + } +} + +@keyframes ellipsis { + to { + width: 1.25em; + } +} diff --git a/src/components/ChatBox/index.jsx b/src/components/ChatBox/index.jsx index a4baa46..5013883 100644 --- a/src/components/ChatBox/index.jsx +++ b/src/components/ChatBox/index.jsx @@ -1,7 +1,7 @@ import React, { useEffect, useState } from 'react'; import PropTypes from 'prop-types'; import Message from '../Message'; -import './ChatBox.css'; +import './ChatBox.scss'; // container for all of the messages const ChatBox = ({ messageList, chatboxContainerRef }) => { @@ -16,7 +16,7 @@ const ChatBox = ({ messageList, chatboxContainerRef }) => { }, [messageList]); return ( -
+
{messageList.map(({ role, content, timestamp }) => ( ))} diff --git a/src/components/Message/Message.css b/src/components/Message/Message.css deleted file mode 100644 index 76e9456..0000000 --- a/src/components/Message/Message.css +++ /dev/null @@ -1,25 +0,0 @@ -.msg { - width: fit-content; - max-width: 70%; - border-radius: 10px; - padding: 5px 10px; - margin: 5px 10px; - text-align: left; - font-size: 15px; -} - -.msg.user { - background:#EBEBEB; - margin-left: auto; -} - -.msg.assistant { - background:#0A3055; - color: white; -} - -.time { - font-size: 10px; - padding-left: 10px; - text-align: right; -} diff --git a/src/components/Message/Message.scss b/src/components/Message/Message.scss new file mode 100644 index 0000000..bc0b34f --- /dev/null +++ b/src/components/Message/Message.scss @@ -0,0 +1,19 @@ +.message { + width: fit-content; + max-width: 70%; + border-radius: 10px; + font-size: 15px; + + &.user { + background:#EBEBEB; + } + + &.assistant { + background:#0A3055; + color: white; + } +} + +.time { + font-size: 10px; +} diff --git a/src/components/Message/index.jsx b/src/components/Message/index.jsx index 4569fe4..8bd1ff5 100644 --- a/src/components/Message/index.jsx +++ b/src/components/Message/index.jsx @@ -1,15 +1,19 @@ import React from 'react'; -import './Message.css'; +import './Message.scss'; import PropTypes from 'prop-types'; const Message = ({ variant, message, timestamp }) => { if (!timestamp) { return null; } + return ( -
+
{message} -
{`${timestamp?.getHours()}:${timestamp?.getMinutes()}:${timestamp?.getSeconds()}`}
+
{`${timestamp?.getHours()}:${timestamp?.getMinutes()}:${timestamp?.getSeconds()}`}
); }; diff --git a/src/components/Sidebar/Sidebar.css b/src/components/Sidebar/Sidebar.css deleted file mode 100644 index 97f5b44..0000000 --- a/src/components/Sidebar/Sidebar.css +++ /dev/null @@ -1,33 +0,0 @@ -.sidebar { - height: 100vh; /* Set the height of the chat container */ - overflow-y: scroll; /* Enable vertical scrolling */ - scroll-behavior: smooth; /* Add smooth scrolling behavior */ -} - -.open-sidebar { - width: 30%; - opacity: 1; - margin-right: 0; - position: fixed; - right: 0; - top: 0; - bottom: 0; -} - -.closed-sidebar { - width: 0; - opacity: 0; - right: -30%; - top: 0; - bottom: 0; -} - -.separator { - z-index: 100; - width: 100%; - height: 10px; - padding: 10px; - background: -webkit-linear-gradient(270deg, rgba(0, 0, 0, 0.35), transparent); - background: linear-gradient(180deg, rgba(0, 0, 0, 0.35), transparent); - opacity: 0.2; -} diff --git a/src/components/Sidebar/Sidebar.scss b/src/components/Sidebar/Sidebar.scss new file mode 100644 index 0000000..492ca10 --- /dev/null +++ b/src/components/Sidebar/Sidebar.scss @@ -0,0 +1,81 @@ +.sidebar { + height: 100vh; + /* Set the height of the chat container */ + overflow-y: scroll; + /* Enable vertical scrolling */ + scroll-behavior: smooth; + /* Add smooth scrolling behavior */ + top: 0; + bottom: 0; + border-left-width: 1px; + border-color: #9CA3AF; + background-color: #F9FAFB; + + &.open { + width: 30%; + right: 0; + } + + h1 { + font-size: 1.25rem; + line-height: 1.75rem; + } + + button.close { + /* Override default opacity:0.5 for buttons */ + opacity: 1; + height: fit-content; + top: 0; + right: 0; + background: none; + + svg { + width: 1.5rem; + height: 1.5rem; + fill: #6B7280; + } + + svg:hover { + fill: #4B5563; + } + } + + button.clear { + /* Bootstrap doesn't have any 0.75*$spacer utility classes. */ + padding-left: 0.75rem; + padding-right: 0.75rem; + border-radius: 9999px; + border-style: solid; + background-color: #DBEAFE; + color: #1E3A8A; + } + + form { + input { + border-width: 1px; + border-color: #D1D5DB; + border-radius: 0.375rem; + border-style: solid; + + &:hover { + outline-style: none; + border-color: #3B82F6; + } + } + + button { + color: #1E3A8A; + background: none; + } + } +} + +.separator { + z-index: 100; + width: 100%; + height: 10px; + padding: 10px; + background: -webkit-linear-gradient(270deg, rgba(0, 0, 0, 0.35), transparent); + background: linear-gradient(180deg, rgba(0, 0, 0, 0.35), transparent); + opacity: 0.2; +} diff --git a/src/components/Sidebar/index.jsx b/src/components/Sidebar/index.jsx index c09ad73..60168e6 100644 --- a/src/components/Sidebar/index.jsx +++ b/src/components/Sidebar/index.jsx @@ -3,7 +3,7 @@ import { useDispatch, useSelector } from 'react-redux'; import { Send } from 'react-feather'; import PropTypes from 'prop-types'; import ChatBox from '../ChatBox'; -import './Sidebar.css'; +import './Sidebar.scss'; import { addChatMessage, clearMessages, @@ -23,30 +23,33 @@ const Sidebar = ({ // that a message is larger than the chat window height. useEffect(() => { const messageContainer = chatboxContainerRef.current; - const { scrollHeight, clientHeight } = messageContainer; - const maxScrollTop = scrollHeight - clientHeight; - const duration = 200; - if (maxScrollTop > 0) { - const startTime = Date.now(); - const endTime = startTime + duration; + if (messageContainer) { + const { scrollHeight, clientHeight } = messageContainer; + const maxScrollTop = scrollHeight - clientHeight; + const duration = 200; - const scroll = () => { - const currentTime = Date.now(); - const timeFraction = (currentTime - startTime) / duration; - const scrollTop = maxScrollTop * timeFraction; + if (maxScrollTop > 0) { + const startTime = Date.now(); + const endTime = startTime + duration; - messageContainer.scrollTo({ - top: scrollTop, - behavior: 'smooth', - }); + const scroll = () => { + const currentTime = Date.now(); + const timeFraction = (currentTime - startTime) / duration; + const scrollTop = maxScrollTop * timeFraction; - if (currentTime < endTime) { - requestAnimationFrame(scroll); - } - }; + messageContainer.scrollTo({ + top: scrollTop, + behavior: 'smooth', + }); + + if (currentTime < endTime) { + requestAnimationFrame(scroll); + } + }; - requestAnimationFrame(scroll); + requestAnimationFrame(scroll); + } } }, [messageList]); @@ -70,64 +73,64 @@ const Sidebar = ({ }; return ( -
-
+ isOpen && ( +
-
-

- Hi, I'm Xpert! +
+

+ Hi, I'm Xpert! +

-

+

Stuck on a concept? Need more clarification on a complicated topic? Let's chat!

-

- - -
- + - - -
- +
+ + +
+
+ +
-
- ); + )); }; Sidebar.propTypes = { diff --git a/src/components/ToggleXpertButton/index.css b/src/components/ToggleXpertButton/index.css deleted file mode 100644 index 5a6592e..0000000 --- a/src/components/ToggleXpertButton/index.css +++ /dev/null @@ -1,13 +0,0 @@ -.sidebar-closed { - right: 1%; -} - -.sidebar-open { - right: 31%; -} - -.open-negative-margin { - background-color: #0A3055; - border-radius: 10px; - margin-left: -80px; -} diff --git a/src/components/ToggleXpertButton/index.jsx b/src/components/ToggleXpertButton/index.jsx index 1b9db1a..6cf2c62 100644 --- a/src/components/ToggleXpertButton/index.jsx +++ b/src/components/ToggleXpertButton/index.jsx @@ -2,7 +2,7 @@ import { ChevronRight } from 'react-feather'; import PropTypes from 'prop-types'; import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import { ReactComponent as NewXeySvg } from '../../assets/new_xey.svg'; -import './index.css'; +import './index.scss'; const ToggleXpert = ({ isOpen, setIsOpen, courseId }) => { const handleClick = () => { @@ -18,7 +18,7 @@ const ToggleXpert = ({ isOpen, setIsOpen, courseId }) => { if (isOpen) { return ( diff --git a/src/components/ToggleXpertButton/index.scss b/src/components/ToggleXpertButton/index.scss new file mode 100644 index 0000000..ac85521 --- /dev/null +++ b/src/components/ToggleXpertButton/index.scss @@ -0,0 +1,22 @@ +.open-negative-margin { + background-color: #0A3055; + border-radius: 10px; + bottom: 4rem; +} + +button { + bottom: 1rem; + + &.open { + border-radius: 9999px; + border-style: solid; + border-color: rgb(229, 231, 235); + box-shadow: 0 1px 3px 0 rgba(0, 0, 0, 0.1), 0 1px 2px 0 rgba(0, 0, 0, 0.06); + right: 31%; + } + + &.closed { + background: none; + right: 1% + } +} From 57987df5b5b4deaf7059f4fce736e15b780343a7 Mon Sep 17 00:00:00 2001 From: michaelroytman Date: Fri, 18 Aug 2023 08:44:09 -0400 Subject: [PATCH 2/3] fix: Xpert bugs This commit fixes a number of bugs in the Xpert. These fixes include changing the propType for the chatboxContainerRef from a string to an object, storing serializable values for dates in Redux, and correctly dispatching the addChatMessage thunk action creator. --- src/components/ChatBox/index.jsx | 10 ++++------ src/components/Message/index.jsx | 6 ++++-- src/data/thunks.js | 9 +++++++-- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/components/ChatBox/index.jsx b/src/components/ChatBox/index.jsx index 5013883..ec77bca 100644 --- a/src/components/ChatBox/index.jsx +++ b/src/components/ChatBox/index.jsx @@ -28,12 +28,10 @@ const ChatBox = ({ messageList, chatboxContainerRef }) => { }; ChatBox.propTypes = { - messageList: PropTypes.arrayOf(PropTypes.shape({ - role: PropTypes.string.isRequired, - content: PropTypes.string.isRequired, - timestamp: PropTypes.instanceOf(Date).isRequired, - })).isRequired, - chatboxContainerRef: PropTypes.string.isRequired, + chatboxContainerRef: PropTypes.oneOfType([ + PropTypes.func, + PropTypes.shape({ current: PropTypes.instanceOf(Element) }), + ]).isRequired, }; export default ChatBox; diff --git a/src/components/Message/index.jsx b/src/components/Message/index.jsx index 8bd1ff5..9f5f776 100644 --- a/src/components/Message/index.jsx +++ b/src/components/Message/index.jsx @@ -7,13 +7,15 @@ const Message = ({ variant, message, timestamp }) => { return null; } + const parsedTimestamp = new Date(Date.parse(timestamp)); + return (
{message} -
{`${timestamp?.getHours()}:${timestamp?.getMinutes()}:${timestamp?.getSeconds()}`}
+
{`${parsedTimestamp?.getHours()}:${parsedTimestamp?.getMinutes()}:${parsedTimestamp?.getSeconds()}`}
); }; @@ -21,7 +23,7 @@ const Message = ({ variant, message, timestamp }) => { Message.propTypes = { variant: PropTypes.string.isRequired, message: PropTypes.string.isRequired, - timestamp: PropTypes.instanceOf(Date).isRequired, + timestamp: PropTypes.string.isRequired, }; export default Message; diff --git a/src/data/thunks.js b/src/data/thunks.js index 9b94e94..a0d28e5 100644 --- a/src/data/thunks.js +++ b/src/data/thunks.js @@ -11,10 +11,15 @@ import { export function addChatMessage(role, content) { return (dispatch, getState) => { const { messageList, conversationId } = getState().learningAssistant; + + // Redux recommends only serializable values in the store, so we'll stringify the timestap to store in Redux. + // When we need to operate on the Date object, we'll deserialize the string. + const timestamp = new Date(); + const message = { role, content, - timestamp: new Date(), + timestamp: timestamp.toString(), }; const updatedMessageList = [...messageList, message]; dispatch(setMessageList({ messageList: updatedMessageList })); @@ -33,7 +38,7 @@ export function getChatResponse(courseId) { const { messageList } = getState().learningAssistant; try { const message = await fetchChatResponse(courseId, messageList); - addChatMessage(message.role, message.content); + dispatch(addChatMessage(message.role, message.content)); } catch (error) { dispatch(setApiError()); } From c219134f50783535d6d57a5483521d862b845b77 Mon Sep 17 00:00:00 2001 From: michaelroytman Date: Fri, 18 Aug 2023 11:03:44 -0400 Subject: [PATCH 3/3] refactor: Xpert improvements This commit makes a number of improvements to the Xpert code. These improvements include moving the API call logic from Xpert widget to Sidebar component to make it easier to follow how an API call is made and to avoid side effects, moving the API loading state to the Redux store to have a cleaner data flow and to make the data available to the entire component tree, and correctly scrolling to the bottom of the ChatBox when the Sidebar is opened. --- src/components/ChatBox/index.jsx | 17 +++++------------ src/components/Sidebar/index.jsx | 6 +++++- src/data/slice.js | 5 +++++ src/data/thunks.js | 7 ++++++- src/widgets/Xpert.jsx | 19 +++++++------------ 5 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/components/ChatBox/index.jsx b/src/components/ChatBox/index.jsx index ec77bca..7aaeedd 100644 --- a/src/components/ChatBox/index.jsx +++ b/src/components/ChatBox/index.jsx @@ -1,26 +1,19 @@ -import React, { useEffect, useState } from 'react'; +import React from 'react'; +import { useSelector } from 'react-redux'; import PropTypes from 'prop-types'; import Message from '../Message'; import './ChatBox.scss'; // container for all of the messages -const ChatBox = ({ messageList, chatboxContainerRef }) => { - const [isResponseLoading, setResponseLoading] = useState(false); - - useEffect(() => { - if (messageList[messageList.length - 1]?.role === 'user') { - setResponseLoading(true); - } else { - setResponseLoading(false); - } - }, [messageList]); +const ChatBox = ({ chatboxContainerRef }) => { + const { messageList, apiIsLoading } = useSelector(state => state.learningAssistant); return (
{messageList.map(({ role, content, timestamp }) => ( ))} - {isResponseLoading && ( + {apiIsLoading && (
Xpert is thinking
)}
diff --git a/src/components/Sidebar/index.jsx b/src/components/Sidebar/index.jsx index 60168e6..0f43c80 100644 --- a/src/components/Sidebar/index.jsx +++ b/src/components/Sidebar/index.jsx @@ -7,11 +7,13 @@ import './Sidebar.scss'; import { addChatMessage, clearMessages, + getChatResponse, updateCurrentMessage, } from '../../data/thunks'; import { ReactComponent as NewXeySvg } from '../../assets/new_xey.svg'; const Sidebar = ({ + courseId, isOpen, setIsOpen, }) => { @@ -51,7 +53,7 @@ const Sidebar = ({ requestAnimationFrame(scroll); } } - }, [messageList]); + }, [messageList, isOpen]); const handleClick = () => { setIsOpen(false); @@ -65,6 +67,7 @@ const Sidebar = ({ event.preventDefault(); if (currentMessage) { dispatch(addChatMessage('user', currentMessage)); + dispatch(getChatResponse(courseId)); } }; @@ -134,6 +137,7 @@ const Sidebar = ({ }; Sidebar.propTypes = { + courseId: PropTypes.string.isRequired, isOpen: PropTypes.bool.isRequired, setIsOpen: PropTypes.func.isRequired, }; diff --git a/src/data/slice.js b/src/data/slice.js index 5f6b5bb..c4a23cd 100644 --- a/src/data/slice.js +++ b/src/data/slice.js @@ -8,6 +8,7 @@ export const learningAssistantSlice = createSlice({ currentMessage: '', messageList: [], apiError: false, + apiIsLoading: false, conversationId: uuidv4(), }, reducers: { @@ -28,6 +29,9 @@ export const learningAssistantSlice = createSlice({ setApiError: (state) => { state.apiError = true; }, + setApiIsLoading: (state, { payload }) => { + state.apiIsLoading = payload; + }, }, }); @@ -37,6 +41,7 @@ export const { setMessageList, resetMessages, setApiError, + setApiIsLoading, } = learningAssistantSlice.actions; export const { diff --git a/src/data/thunks.js b/src/data/thunks.js index a0d28e5..7b76034 100644 --- a/src/data/thunks.js +++ b/src/data/thunks.js @@ -3,9 +3,10 @@ import fetchChatResponse from './api'; import { setCurrentMessage, clearCurrentMessage, + resetMessages, setMessageList, setApiError, - resetMessages, + setApiIsLoading, } from './slice'; export function addChatMessage(role, content) { @@ -36,11 +37,15 @@ export function addChatMessage(role, content) { export function getChatResponse(courseId) { return async (dispatch, getState) => { const { messageList } = getState().learningAssistant; + + dispatch(setApiIsLoading(true)); try { const message = await fetchChatResponse(courseId, messageList); + dispatch(setApiIsLoading(false)); dispatch(addChatMessage(message.role, message.content)); } catch (error) { dispatch(setApiError()); + dispatch(setApiIsLoading(false)); } }; } diff --git a/src/widgets/Xpert.jsx b/src/widgets/Xpert.jsx index 84f4708..e49c0ce 100644 --- a/src/widgets/Xpert.jsx +++ b/src/widgets/Xpert.jsx @@ -1,25 +1,20 @@ import PropTypes from 'prop-types'; -import { useState, useEffect } from 'react'; -import { useDispatch, useSelector } from 'react-redux'; +import { useState } from 'react'; import ToggleXpert from '../components/ToggleXpertButton'; import Sidebar from '../components/Sidebar'; -import { getChatResponse } from '../data/thunks'; const Xpert = ({ courseId }) => { - const { messageList } = useSelector(state => state.learningAssistant); const [sidebarIsOpen, setSidebarIsOpen] = useState(false); - const dispatch = useDispatch(); - - useEffect(() => { - if (messageList[messageList.length - 1]?.role === 'user') { - dispatch(getChatResponse(courseId)); - } - }, [dispatch, courseId, messageList]); return (
- +