Enhance Get Help feature with interactive feedback and improved UI/UX #7635

phungmanhcuong · 2024-11-12T05:02:47Z

Description:

This pull request introduces enhancements to the Get Help feature, focusing on improved interactivity, usability, and accessibility. Key updates include:

Interactive Feedback Options: Allows users to provide real-time feedback through like, dislike, and dismiss actions on specific feedback items, enhancing engagement and interaction.

UI Improvements: Refined layout and styling to present feedback in an organized, accessible manner.
These improvements aim to create a more responsive, intuitive interface, making it easier for users to navigate and utilize the Get Help feature effectively.

Context:

This aligns with the goals outlined in the Get Help Improvement proposal and supports enhanced student support and accessibility during assessments.

phungmanhcuong · 2024-11-14T23:29:32Z

client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx

-        question.id
-      ]?.feedbackFiles ?? [],
-  );
+  // TODO: consider removing feedbackFiles


@cysjonathan should I remove this one since its outdated now ?

remove which? feedbackFiles or the comment in line 352?
If a variable is not used in this file then yes you can remove it.

Just make sure your code handles feedback across multiple files.

cysjonathan

to align with rest of site: don't bold the [Get Help] window title, use normal font-weight
[Get Help] is broken when there are multiple questions (single page). window doesn't appear at the 2nd question.
What happened to "click-on-comment" to focus on the line in code-editor? Will we be dropping that previously implemented feature?
Why are first 2 prompts still "I am stuck" and "My code doesn't work"?
suggestions array should use translations to support internationalization.
Does your code handle feedback across multiple files? I see that you're flattening it.
When the case of no-feedback-generated occurs, the spinner just keeps spinning. Can you handle this properly?
Send button should be disabled until some actual text is in the input.
Positioning should be relative to the [Get Help] button. position seems different in the Single-page layout vs Tabbed layout.
Where's the part of the code where we will handle the updated responses from Codaveri?

cysjonathan · 2024-11-15T01:05:14Z

...undles/course/assessment/submission/pages/SubmissionEditIndex/components/QuestionContent.tsx

+  const liveFeedback = useAppSelector((state) =>
+    getFeedbackByQuestionId(state, questionId),
+  );
+  const isShowingPopup = !!liveFeedback?.isShowingPopup;


Typically convention is to use isXxxOpen, e.g. isDialogOpen, isEditOpen.

cysjonathan · 2024-11-15T01:06:38Z

...pp/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/GetHelpPage.tsx

+      </Box>
+      <Box className="get-help-page-box-full-width">
+        <TextField
+          className="get-help-page-textfield"


Unless there's a good justification, please use our existing styling (e.g. take the same one from the Programming qns generator) instead of creating new styles.

I have converted to use the TextField in common lib. In Programming qns generator, they are using the TableCell, a bit different then this case.

cysjonathan · 2024-11-15T01:08:44Z

client/app/bundles/course/assessment/submission/reducers/liveFeedback.js

-        }
+        updateFeedbackForQuestion(draft, questionId, {
+          isShowingPopup,
+          suggestedReplies: ['I am stuck', "My code doesn't work"],


Don't repeat the first two elements in the suggestions array, you should be only hardcoding strings like this once.

Actually, they are only showed when first time popup open. Then after that, they are randomly generated.

cysjonathan

Your 2nd, 3rd, 4th, 5th commits seem to all be part of the 1st commit.
e.g. commit 2 removes a depedency in your useEffect array introduced in commit 1,
commit 3 adds newline,
commit 4 adjusts whitespace-formatting only,
commit 5 fixes CSS values introduced in commit 1

Any reason why you separated them?

client/app/bundles/course/assessment/submission/components/answers/index.tsx

...pp/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/GetHelpPage.css

phungmanhcuong · 2024-11-17T13:37:19Z

to align with rest of site: don't bold the [Get Help] window title, use normal font-weight

[Get Help] is broken when there are multiple questions (single page). window doesn't appear at the 2nd question.

What happened to "click-on-comment" to focus on the line in code-editor? Will we be dropping that previously implemented feature?

Why are first 2 prompts still "I am stuck" and "My code doesn't work"?

suggestions array should use translations to support internationalization.

Does your code handle feedback across multiple files? I see that you're flattening it.

When the case of no-feedback-generated occurs, the spinner just keeps spinning. Can you handle this properly?

Send button should be disabled until some actual text is in the input.

Positioning should be relative to the [Get Help] button. position seems different in the Single-page layout vs Tabbed layout.

Where's the part of the code where we will handle the updated responses from Codaveri?

I have updated for 1-9, about 10, it should be stay in the PR about integration with new Codaveri API, I think

client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx

cysjonathan · 2024-11-18T06:32:59Z

...pp/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/GetHelpPage.css

@@ -0,0 +1,79 @@
+.get-help-page-paper {


https://github.com/Coursemology/coursemology2/blob/master/client/README.md#-only-use-tailwind-utilities-for-styling

cysjonathan · 2024-11-18T06:38:15Z

...pp/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/GetHelpPage.tsx

+      id: `course.assessment.submission.liveFeedback.${toCamelCase(suggestion)}`,
+      defaultMessage: suggestion,


Don't do this text to camelCase, use defineMessages instead for suggestions.

cysjonathan

Please remove all arbitrary values in the tailwind classes.
If you do need to use any, provide justification and clear with me first.

cysjonathan · 2024-11-21T10:00:44Z

client/app/bundles/course/assessment/submission/reducers/liveFeedback.js

+const getRandomSuggestions = () => {
+  const suggestionValues = Object.values(suggestionsTranslations);
+  const shuffled = suggestionValues.sort(() => 0.5 - Math.random());
+  // TODO: Temporarily change to show no suggestions
+  return shuffled.slice(0, 0);
+};


Don't implement your own shuffle, your algorithm is not truly random.

You can use our lodash library's shuffle.

cysjonathan · 2024-11-21T10:03:45Z

...pp/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/GetHelpPage.tsx

+  <div className="absolute top-[-4rem] flex justify-center mb-0 w-full flex-row h-[52%] overflow-x-auto">
+    {suggestions.map((suggestion, index) => (
+      <Button
+        key={index}


Don't use the index as the key, the translation object itself should have an id.

Actually, to make it extensible, suggestions can just take in an array of strings as you have done so (since the API would just give you an array of strings).
But don't use the index as key, can just use the suggestion itself as the key.

cysjonathan · 2024-11-21T10:27:17Z

...pp/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/GetHelpPage.tsx

+  suggestions: string[];
+  handleSendMessage: (message: string) => void;
+}> = ({ loading, suggestions, handleSendMessage }) => (
+  <div className="absolute top-[-4rem] flex justify-center mb-0 w-full flex-row h-[52%] overflow-x-auto">


Can use this "scrollbar-hidden absolute bottom-full flex p-3 gap-3 justify-around w-full overflow-x-auto", please refrain from using arbitrary values.

cysjonathan · 2024-11-21T10:27:26Z

...pp/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/GetHelpPage.tsx

+    {suggestions.map((suggestion, index) => (
+      <Button
+        key={index}
+        className="bg-white mx-1 text-[1.3rem] min-w-[160px]"


"bg-white whitespace-nowrap shrink-0"

...pp/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/GetHelpPage.tsx

cysjonathan · 2024-11-21T10:37:13Z

client/app/bundles/course/assessment/submission/reducers/liveFeedback.js

+            .sort((a, b) => a.linenum - b.linenum)
+            .map((line, index) => ({
+              text: `Line ${line.linenum}: ${line.feedback}`,
+              sender: 'Codaveri',
+              linenum: line.linenum,
+              timestamp:
+                index === feedbackFile.feedbackLines.length - 1
+                  ? new Date().toISOString()
+                  : null,
+              isBold: false,
+            })),


Ben also requested to collapse feedback which share the same line, please include that

cysjonathan · 2024-11-21T10:46:17Z

client/app/bundles/course/assessment/submission/reducers/liveFeedback.js

+const updateFeedbackForQuestion = (draft, questionId, newFeedbacks) => {
+  draft.feedbackByQuestion[questionId] = {
+    ...getOrDefault(draft.feedbackByQuestion, questionId, {}),
+    ...newFeedbacks,
+  };
+  saveStateToLocalStorage(draft);
+};


Your states are not being saved properly, please double-check.

e.g. leave the [Get Help] window in open state, navigate away, then navigate back, the window won't show up as open state.

cysjonathan · 2024-11-21T10:51:40Z

client/app/bundles/course/assessment/submission/reducers/liveFeedback.js

+              linenum: line.linenum,
+              timestamp:
+                index === feedbackFile.feedbackLines.length - 1
+                  ? new Date().toISOString()


Use CM's datetime formatters instead of just ISO string please, formatShortDateTime from moment.ts

cysjonathan · 2024-11-21T10:56:54Z

...pp/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/GetHelpPage.tsx

+      {loading && (
+        <ListItem className="justify-start py-0">
+          <Bubble>
+            <BeatLoader color="grey" size={8} />


Use our existing <LoadingEllipsis /> or write your own, following loading ellipsis example.

- Refactor feedback grouping logic by line number for better readability - Enhance suggestions display using translations for dynamic content - Replace react-spinners with a custom loading component (LoadingEllipsis) - Use moment for consistent timestamp formatting - Remove react-spinners dependency to streamline the package

- Change text property in LiveFeedbackMessage to always be an array for consistency - Simplify rendering logic in Message component - Add error message handling in liveFeedbackReducer for failure cases - Enhance UI interactions with feedback lines and error notifications

- Removed unused moment import and related date formatting functions. - Added bgColor property to LiveFeedbackMessage type for dynamic styling of messages. - Simplified text rendering logic in the Message component for better readability. - Enhanced reducer logic to include bgColor for different message types like errors and user messages. - Fixed handling of null linenum in line navigation to prevent unintended behavior. - Replaced redundant state management with direct use of conversation in GetHelpPage.

bivanalhar · 2024-11-26T05:53:54Z

client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx


    return (
      <div key={keyString} id={keyString} style={{ position: 'relative' }}>
-        <Box marginRight={shouldRenderDrawer ? '315px' : '0px'}>
+        <Box marginRight="0px">


change into <Box className="mr-0">. Remember that we're migrating to tailwind, which also means that all styling for any component should be done in tailwind unless it cannot be done (which is, as far as I know, never the case)

bivanalhar · 2024-11-26T06:00:44Z

client/yarn.lock

@@ -9371,11 +9371,6 @@ react-sortable-tree-patch-react-17@^2.9.0:
    react-lifecycles-compat "^3.0.4"
    react-virtualized "^9.21.2"

-react-spinners@^0.14.1:


the previous commit (still within this PR) adds this library, while this commit deletes it. If this library is not needed at all, you need to not even have it added in any commits in this PR

with that being said, any line that relies on this react-spinners from the previous commit needs to be removed at that commit, not here

bivanalhar · 2024-11-26T06:17:54Z

client/app/bundles/course/assessment/submission/reducers/liveFeedback.js

            sender: 'Student',
-            timestamp: new Date().toISOString(),
+            timestamp: moment(new Date()).format(SHORT_DATE_TIME_FORMAT),


the naming timestamp has an inference that it will be of type DateTime, but this one is actually of type string. It's not entirely wrong, but I suggest for this timestamp to be stored as new Date() then inside GetHelpPage.tsx, you then use this functionality towards message.timestamp

Also, we have the functionality formatShortDateTime which basically encapsulates what is being done here. I recommend you to search that function within our codebase, then apply it accordingly.

Do the same also for all the similar lines within this page (and also other page, if it exists)

bivanalhar · 2024-11-26T06:44:47Z

...pp/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/GetHelpPage.tsx

+    sender: string;
+    linenum: number | null;
+    isBold: boolean;
+    text: string | string[];


if you put text as a prop name, other dev will think it to be obviously string; there will be no way the prop name text will be regarded as an array (texts might be, though)

I suggest to just make it one type (string[] should be fitting well with the purpose here), change the prop name to texts or anything that suits what it serves to be, then change everything accordingly (for example, if there's a place where indeed text is singular, change it into [text] then handle the array accordingly)

bivanalhar · 2024-11-26T06:48:50Z

...pp/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/GetHelpPage.tsx

+  message: {
+    sender: string;
+    linenum: number | null;
+    isBold: boolean;


I saw this props being assigned but never being actually used anywhere. If this prop ended up not being used, just remove it (also actually our consensus is generally to avoid using bold unless necessary. Alternative is usually to explore the variant props in Typography)

bivanalhar · 2024-11-26T06:50:47Z

...pp/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/GetHelpPage.tsx

+            </Typography>
+          )}
+          {message.text.map((line, index) => (
+            <Typography key={index} className="text-[1.3rem]">


remember not to use index for our key. Using line suffices. Fix it accordingly, and also in all the places in which you put index as value for key

bivanalhar · 2024-11-26T06:53:15Z

...pp/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/GetHelpPage.tsx

+
+  return (
+    <ListItem
+      className={`py-0 ${message.sender === 'Codaveri' ? 'justify-start' : 'justify-end'}`}


I saw this string Codaveri being used in many places within your PR. Consider to assign a constant, for example, CODAVERI_SENDER or something like that, as Codaveri, then you use that constant instead.

So it will be like message.sender === CODAVERI_SENDER instead of hard-coded string

bivanalhar · 2024-11-26T08:34:30Z

client/app/bundles/course/assessment/submission/reducers/liveFeedback.js

@@ -142,9 +142,18 @@ const liveFeedbackReducer = function (state = initialState, action) {
    case actions.LIVE_FEEDBACK_FAILURE: {
      const { questionId } = action.payload;
      return produce(state, (draft) => {
+        const previousConversation = getConversation(draft, questionId);
+        const errorMessage = {
+          text: ['An error occurred while processing your request !!!'],


If you need this text, don't put it as hard-coded string over here. Use translations instead

phungmanhcuong marked this pull request as draft November 12, 2024 05:02

phungmanhcuong force-pushed the mcphung/get-help-enhancement branch 5 times, most recently from bacf5a6 to fee8f78 Compare November 14, 2024 16:45

phungmanhcuong commented Nov 14, 2024

View reviewed changes

phungmanhcuong marked this pull request as ready for review November 14, 2024 23:37

cysjonathan requested changes Nov 15, 2024

View reviewed changes

cysjonathan reviewed Nov 15, 2024

View reviewed changes

phungmanhcuong force-pushed the mcphung/get-help-enhancement branch 2 times, most recently from 8e3c5d5 to 20fc4fc Compare November 15, 2024 02:40

phungmanhcuong commented Nov 15, 2024

View reviewed changes

client/app/bundles/course/assessment/submission/components/answers/index.tsx Outdated Show resolved Hide resolved

phungmanhcuong commented Nov 15, 2024

View reviewed changes

...pp/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/GetHelpPage.css Outdated Show resolved Hide resolved

phungmanhcuong force-pushed the mcphung/get-help-enhancement branch 5 times, most recently from bf3b349 to 7898ff8 Compare November 17, 2024 13:35

phungmanhcuong force-pushed the mcphung/get-help-enhancement branch 5 times, most recently from 046f203 to 22eea36 Compare November 18, 2024 01:32

phungmanhcuong self-assigned this Nov 18, 2024

phungmanhcuong added the UI label Nov 18, 2024

phungmanhcuong requested a review from cysjonathan November 18, 2024 02:07

phungmanhcuong force-pushed the mcphung/get-help-enhancement branch from 22eea36 to d132eba Compare November 18, 2024 04:14

cysjonathan requested changes Nov 18, 2024

View reviewed changes

phungmanhcuong force-pushed the mcphung/get-help-enhancement branch 10 times, most recently from b160947 to 774d97f Compare November 21, 2024 05:04

feat(get-help): enhance live feedback for student assessments

454de03

phungmanhcuong force-pushed the mcphung/get-help-enhancement branch from 774d97f to 454de03 Compare November 21, 2024 07:33

cysjonathan requested changes Nov 21, 2024

View reviewed changes

phungmanhcuong force-pushed the mcphung/get-help-enhancement branch 2 times, most recently from e23a188 to 1b34a53 Compare November 26, 2024 05:40

phungmanhcuong force-pushed the mcphung/get-help-enhancement branch from 1b34a53 to 50514ba Compare November 26, 2024 05:44

bivanalhar requested changes Nov 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance Get Help feature with interactive feedback and improved UI/UX #7635

Enhance Get Help feature with interactive feedback and improved UI/UX #7635

phungmanhcuong commented Nov 12, 2024 •

edited

Loading

phungmanhcuong Nov 14, 2024

cysjonathan Nov 14, 2024

cysjonathan left a comment

cysjonathan Nov 15, 2024

cysjonathan Nov 15, 2024

phungmanhcuong Nov 17, 2024 •

edited

Loading

cysjonathan Nov 15, 2024

phungmanhcuong Nov 15, 2024

cysjonathan left a comment

phungmanhcuong commented Nov 17, 2024

cysjonathan Nov 18, 2024

cysjonathan Nov 18, 2024

cysjonathan left a comment

cysjonathan Nov 21, 2024

cysjonathan Nov 21, 2024

cysjonathan Nov 22, 2024

cysjonathan Nov 21, 2024

cysjonathan Nov 21, 2024

cysjonathan Nov 21, 2024

cysjonathan Nov 21, 2024

cysjonathan Nov 21, 2024

cysjonathan Nov 21, 2024

bivanalhar Nov 26, 2024

bivanalhar Nov 26, 2024

bivanalhar Nov 26, 2024

bivanalhar Nov 26, 2024

bivanalhar Nov 26, 2024

bivanalhar Nov 26, 2024

bivanalhar Nov 26, 2024

bivanalhar Nov 26, 2024

		id: `course.assessment.submission.liveFeedback.${toCamelCase(suggestion)}`,
		defaultMessage: suggestion,

Enhance Get Help feature with interactive feedback and improved UI/UX #7635

Are you sure you want to change the base?

Enhance Get Help feature with interactive feedback and improved UI/UX #7635

Conversation

phungmanhcuong commented Nov 12, 2024 • edited Loading

Description:

Context:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cysjonathan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phungmanhcuong Nov 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cysjonathan left a comment

Choose a reason for hiding this comment

phungmanhcuong commented Nov 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cysjonathan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phungmanhcuong commented Nov 12, 2024 •

edited

Loading

phungmanhcuong Nov 17, 2024 •

edited

Loading