feat: Show toast when exporting course tags #995

ChrisChV · 2024-05-15T22:18:26Z

Description

Show In progress toast when exporting course tags

Supporting information

Github issue: Display an indicator when exporting tags modular-learning#215
Internal ticket: FAL-3733

Technical Details

The Export Tags button is on the StudioHeader (frontend-component-header). This component only receives as parameters the text of the button and the link to where that button should redirect.

There is no way to send an onClick function or know when the user clicked the button or add some flag to know if the export is in progress, this is necessary to display the toast. The component logic is closed in the link that is passed as a parameter.

The workaroun used on this PR is to use an internal MFE route instead the edx-platform API. The detail is that there should be no redirection. To achieve this I have used the url fragment. The useEffect of this code is to verify the fragment and execute the export code we want with the respective toasts.

Testing instructions

Enable contentstore.new_studio_mfe.use_new_course_outline_page.
Go to a course outline.
Click on Tools > Export Tags.
Check that the toast appears.
Check that when the file is ready, the toast is gone.

openedx-webhooks · 2024-05-15T22:18:31Z

Thanks for the pull request, @ChrisChV! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

codecov · 2024-05-15T23:30:59Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.31%. Comparing base (7c539f3) to head (1d37c84).
Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #995      +/-   ##
==========================================
+ Coverage   92.29%   92.31%   +0.01%     
==========================================
  Files         708      708              
  Lines       12505    12527      +22     
  Branches     2686     2689       +3     
==========================================
+ Hits        11542    11564      +22     
  Misses        926      926              
  Partials       37       37

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pomegranited

👍 Thank you for this PR @ChrisChV ! It solves the issue, and the code looks great.

I left a few comments, but they're just suggestions.

I tested this on my tutor dev stack using the PR test instructions.
I read through the code
I checked for accessibility issues by using my keyboard to navigate.
Includes code documentation
User-facing strings are extracted for translation

pomegranited · 2024-05-15T23:52:33Z

src/course-outline/CourseOutline.jsx

+    if (location.hash === '#export') {
+      setToastMessage(intl.formatMessage(messages.exportTagsToastMessage));
+      getTagsExportFile(courseId, courseName).finally(() => {
+        setToastMessage(null);


Is there any way to make the Toast appear for some minimum amount of time? When the export is quick, it flickers and disappears so fast I can't read it.

This wasn't mentioned as part of the issue, so no worries if paragon's Toast doesn't support this.

Do we need to show a failure message if the export fails?

@pomegranited @bradenmacdonald By default the toast has a delay of 5 seconds before closing. I left the default value and I added success/error toast messages when the export is done; to avoid to close a toast quickly

Commit: 449251b

pomegranited · 2024-05-15T23:57:03Z

src/course-outline/data/api.js

+  // Gets exported tags and builds the blob to download CSV file.
+  // This can be done with this code:
+  // `window.location.href = exportTags(contentId);`
+  // but it is done in this way to known when the operation ends to close the toast.


nit:

Suggested change

// but it is done in this way to known when the operation ends to close the toast.

// but it is done in this way so we know when the operation ends to close the toast.

Updated: 79368d7

pomegranited · 2024-05-16T00:02:42Z

src/course-outline/data/api.js

+  a.click();
+
+  window.URL.revokeObjectURL(url);
+}


There's a weird side effect of this change, where after I select "Export Tags", the #export stays in the URL, and so the "Export Tags" menu item stays selected, so I can't re-select it and export again unless I navigate somewhere else first.

I don't think this is important -- people are unlikely to want to export tags multiple times -- it's just annoying, so not a blocker for this PR.

export_tags.mp4

Fixed: 6ccf705

xitij2000

I'm not sure I understand why the url is being used the way it is but other than that it is working.

xitij2000 · 2024-05-17T04:11:20Z

src/course-outline/CourseOutline.jsx

+  const [toastMessage, setToastMessage] = useState(/** @type{null|string} */ (null));
+
+  useEffect(() => {
+    if (location.hash === '#export-tags') {
+      setToastMessage(intl.formatMessage(messages.exportTagsCreatingToastMessage));
+      getTagsExportFile(courseId, courseName).then(() => {
+        setToastMessage(intl.formatMessage(messages.exportTagsSuccessToastMessage));
+      }).catch(() => {
+        setToastMessage(intl.formatMessage(messages.exportTagsErrorToastMessage));
+      });
+
+      // Delete `#export-tags` from location
+      window.location.href = '#';
+    }
+  }, [location]);


I'm not sure why this URL change is being used as an indicator of this state.

I think whatever triggers the tag exporting should set the toastMessage or some flag that an export is in progress.

The Export Tags button is on the StudioHeader (frontend-component-header). This component only receives as parameters the text of the button and the link to where that button should redirect.

There is no way to send an onClick function or know when the user clicked the button or add some flag to know if the export is in progress, this is necessary to display the toast. The component logic is closed in the link that is passed as a parameter.

My workaroun is to use an internal MFE route instead the edx-platform API. The detail is that there should be no redirection. To achieve this I have used the url fragment. The useEffect of this code is to verify the fragment and execute the export code we want with the respective toasts.

Sounds good!

I think another mechanism could be to use postMessage, but I guess if all you can do is inject a link this is a good solutions.

xitij2000 · 2024-05-17T04:54:46Z

src/course-outline/messages.js

+  },
+  exportTagsSuccessToastMessage: {
+    id: 'course-authoring.course-outline.export-tags.toast.success.message',
+    defaultMessage: 'File created successfully',


I'm assuming this message will change later, but it's too unspecific. This is specifically a message about tags being exported from a particular course and that being successful, so the message can be more tailored tot hat.

Sure 👍 Updated: 1d37c84

openedx-webhooks · 2024-05-24T10:25:01Z

@ChrisChV 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

feat: Show toast when exporting course tags

f829572

ChrisChV requested a review from a team as a code owner May 15, 2024 22:18

ChrisChV marked this pull request as draft May 15, 2024 22:18

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label May 15, 2024

test: For export tags in course outline

096d5bc

ChrisChV marked this pull request as ready for review May 15, 2024 23:30

pomegranited approved these changes May 16, 2024

View reviewed changes

ChrisChV added 4 commits May 16, 2024 11:18

style: Nits on export tags toast

79368d7

fix: Issue with multiple exports

6ccf705

feat: Add new success/error toast messages when exporting tags

449251b

fix: Fix test coverage

bc63bfc

ChrisChV force-pushed the chris/FAL-3733-toast-on-export branch from b7e98a0 to bc63bfc Compare May 16, 2024 18:23

xitij2000 reviewed May 17, 2024

View reviewed changes

style: Export tags messages updated

1d37c84

xitij2000 approved these changes May 24, 2024

View reviewed changes

xitij2000 merged commit c3df0b0 into openedx:master May 24, 2024
6 checks passed

xitij2000 deleted the chris/FAL-3733-toast-on-export branch May 24, 2024 10:25

ChrisChV mentioned this pull request May 27, 2024

[Cherry pick] feat: Show toast when exporting course tags #1051

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Show toast when exporting course tags #995

feat: Show toast when exporting course tags #995

ChrisChV commented May 15, 2024 •

edited

Loading

openedx-webhooks commented May 15, 2024 •

edited

Loading

codecov bot commented May 15, 2024 •

edited

Loading

pomegranited left a comment

pomegranited May 15, 2024

pomegranited May 15, 2024

ChrisChV May 16, 2024

pomegranited May 15, 2024

ChrisChV May 16, 2024

pomegranited May 16, 2024

ChrisChV May 16, 2024

xitij2000 left a comment

xitij2000 May 17, 2024

ChrisChV May 17, 2024 •

edited

Loading

xitij2000 May 24, 2024

xitij2000 May 17, 2024

ChrisChV May 17, 2024

openedx-webhooks commented May 24, 2024

	// but it is done in this way to known when the operation ends to close the toast.
	// but it is done in this way so we know when the operation ends to close the toast.

feat: Show toast when exporting course tags #995

feat: Show toast when exporting course tags #995

Conversation

ChrisChV commented May 15, 2024 • edited Loading

Description

Supporting information

Technical Details

Testing instructions

openedx-webhooks commented May 15, 2024 • edited Loading

codecov bot commented May 15, 2024 • edited Loading

Codecov Report

pomegranited 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

xitij2000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrisChV May 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

Choose a reason for hiding this comment

openedx-webhooks commented May 24, 2024

ChrisChV commented May 15, 2024 •

edited

Loading

openedx-webhooks commented May 15, 2024 •

edited

Loading

codecov bot commented May 15, 2024 •

edited

Loading

ChrisChV May 17, 2024 •

edited

Loading