feat: implement mentoring page#57
Conversation
| @@ -0,0 +1,23 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
[optional]: Let's try remove this import
|
|
||
| const MentoringTab = ({ intl }) => ( | ||
| <> | ||
| <h2 aria-level="1" className="h2 my-3"> |
There was a problem hiding this comment.
[nit]: I wonder if it is necessary to change aria-level="1" for the <h2> heading? By default, all headers have aria-level equal to the header level (<h2> - aria-level="2"), in this case you increase aria-level (<h2> - aria-level="1").
I looked, on other tabs (Progress, Dates) a similar aria attribute is added in case of using another tag, different from the heading (<div role="heading" aria-level="1" class="h2 my-3">Important dates</div>).
Let's change this a <h1> level heading and remove aria-level
| <h2 aria-level="1" className="h2 my-3"> | |
| <h1 className="my-3"> |
| }, [url]); | ||
|
|
||
| const sendMessage = (msg) => { | ||
| if (socket.current.readyState === WebSocket.OPEN) { |
There was a problem hiding this comment.
[optional]: Let's additionally check for the presence of socket.current
| if (socket.current.readyState === WebSocket.OPEN) { | |
| if (socket.current && socket.current.readyState === WebSocket.OPEN) { |
| const messages = defineMessages({ | ||
| title: { | ||
| id: 'learning.mentoring.title', | ||
| defaultMessage: 'Mentoring Assistant', |
There was a problem hiding this comment.
[optional]: Similar to Your progress (first letter is capital, rest are lowercase)
| defaultMessage: 'Mentoring Assistant', | |
| defaultMessage: 'Mentoring assistant', |
| title: { | ||
| id: 'learning.mentoring.title', | ||
| defaultMessage: 'Mentoring Assistant', | ||
| description: 'The title of mentoring tab (course timeline).', |
There was a problem hiding this comment.
| description: 'The title of mentoring tab (course timeline).', | |
| description: 'The title of mentoring tab (course outline).', |
| const { courseId } = useSelector(state => state.courseHome); | ||
|
|
||
| const { sendMessage } = useWebSocket({ | ||
| url: `ws://${OPENEDX_AI_SOCKET_DOMAIN}/ws/chatgpt/${courseId}/`, |
There was a problem hiding this comment.
[question]: Have you tried to test this functionality on different courses?
There was a problem hiding this comment.
It would also be useful to understand what limitations we have related to the course size + how the provision of course context works if it is empty.
| const data = JSON.parse(e.data); | ||
| onMessage(data); | ||
| } catch (err) { | ||
| onMessage({ text: e.data }); |
There was a problem hiding this comment.
[optional]: Should we display an error message in the catch block?
| gap={2} | ||
| ref={idx === chatMessages.length - 1 ? lastMessageRef : null} | ||
| > | ||
| {msg.sender === 'ai' && ( |
There was a problem hiding this comment.
[optional]: Let's move all senders to the SENDER_TYPES or SENDERS object
For example:
const SENDERS = {
STUDENT: 'student',
AI: 'ai',
...
}
| <Stack | ||
| key={idx} // eslint-disable-line react/no-array-index-key | ||
| direction="horizontal" | ||
| className={`w-100 fade-in ${msg.sender === 'student' ? 'justify-content-end' : 'justify-content-start'}`} |
There was a problem hiding this comment.
| className={`w-100 fade-in ${msg.sender === 'student' ? 'justify-content-end' : 'justify-content-start'}`} | |
| className={`w-100 fade-in ${msg.sender === SENDERS.STUDENT ? 'justify-content-end' : 'justify-content-start'}`} |
| import messages from './messages'; | ||
| import './MentoringTab.scss'; | ||
|
|
||
| const MentoringTab = ({ intl }) => ( |
There was a problem hiding this comment.
[optional]: Let's add a little support message about what AI Mentoring is
There was a problem hiding this comment.
As an additional support option, we can add a paragon product tour to quickly introduce the user to the mentoring application. What do you think?
2 participants
Description: implement mentoring page