fix: course update iframe

[CefBoud]

Intro

This PR addresses a bug that emerges in course updates displayed in the LmsHtmlFragment component. Currently, we rely on the body's scrollHeight to set the iframe 's height. This poses a problem in case the body node's height is inferior to the html node's height resulting in hidden content. (cf. example below)

This PR also deals with another issue regarding the Show More button and whether the message can be shortened. Presently, we compare the original update's length with a truncated version's length to determine if the message can be shortened. We rely on truncate-html module to truncate the message. This module parses the content before the truncating and can change its length even if there is no need for truncating.

const truncate = require("truncate-html")
let a = "<h1  >Test</h1>"  // notice the additional spaces
let b = truncate(a, 100, { byWords: true, keepWhitespaces: true })
console.log("a:", a, "    ", "a.length:", a.length, "    ", "b:", b, "    ", "b.length:", b.length)
// a: <h1  >Test</h1>      a.length: 15      b: <h1>Test</h1>      b.length: 13

In addition, truncate-html strips comments regardless of truncating and this results in a length change. That's why this PR introduces a "cleaning" step that aims to format the update into a canonical format before truncating it.

useMemo is also used to avoid recomputing the truncate at every render.

Reproducing insufficient height issue

1- head to studio Course Update section e.g. http://localhost:18010/course_info/course-v1:edX+DemoX+Demo_Course
2- Create a course update containing the following HTML:

<h3><img src="" width="117" height="117" align="left" /></h3>
<p></p>
<p></p>
<hr />
<p></p>
<p></p>
<h3><strong>Test Test Test Test!</strong></h3>
<p><strong>-Test Test Test Test Test Test Test Test, Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test <a href="#" target="_blank" rel="noopener">Test Test Test Test Test Test Test Test</a>. </strong></p>
<p><strong>-Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test  Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test.</strong></p>
<p>Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test--&gt; Home Page</p>
<p>-Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test Test</p>

3 - Head to the course http://localhost:2000/course/course-v1:edX+DemoX+Demo_Course/home and notice that the lower part of the update is not fully displayed after clicking on Show More

demo_course_update.mp4

Reproducing unnecessary "Show More"

1- head to studio Course Update section e.g. http://localhost:18010/course_info/course-v1:edX+DemoX+Demo_Course
2- Create a course update containing the following HTML:

<p > I am short</p>

3 - Head to the course http://localhost:2000/course/course-v1:edX+DemoX+Demo_Course/home
4 - The Show More button is displayed.

[openedx-webhooks]

Thanks for the pull request, @CefBoud! 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.

[itsjeyd]

Hi @CefBoud, thanks for these changes! I'm lining them up for a test run now.

[e0d]

@CefBoud there are some failed checks that require attention.

[CefBoud]

@e0d Thank you for the heads up. I just pushed a fix.

[codecov]

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (5604def) 88.04% compared to head (657b146) 88.08%.
Report is 4 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1204      +/-   ##
==========================================
+ Coverage   88.04%   88.08%   +0.03%     
==========================================
  Files         266      268       +2     
  Lines        4518     4565      +47     
  Branches     1142     1149       +7     
==========================================
+ Hits         3978     4021      +43     
- Misses        526      529       +3     
- Partials       14       15       +1     

Files	Coverage Δ
src/course-home/outline-tab/LmsHtmlFragment.jsx	76.47% <100.00%> (ø)
...course-home/outline-tab/widgets/WelcomeMessage.jsx	88.46% <100.00%> (+2.09%)	⬆️

... and 6 files with indirect coverage changes

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

[e0d]

@CefBoud thanks, all 🟢 @mphilbrick211 I've moved to ready for review.

[mphilbrick211]

@itsjeyd - flagging for you!

[itsjeyd]

@mphilbrick211 @e0d Thanks!

[CefBoud]

I closed the PR by mistake. I am sorry for the confusion.

[itsjeyd]

@CefBoud I'm lining this up for another test run.

Who is your OpenCraft reviewer for this PR?

[CefBoud]

@itsjeyd Ok, thanks.

Who is your OpenCraft reviewer for this PR?

@viadanna is.

[itsjeyd]

@CefBoud OK, let me know when you get approval from him so we can line this up for upstream engineering review :)

[CefBoud]

@itsjeyd It has been approved. 🙂

[itsjeyd]

@CefBoud Great!

@viadanna Could you please drop a formal 👍 here when you get a minute?

[viadanna]

Approved 👍

• I tested this on a sandbox
• I read through the code
• Includes tests

@itsjeyd @CefBoud

[itsjeyd]

@mattcarter This PR is ready for engineering review by Aurora.

[itsjeyd]

Hey @mattcarter, a friendly reminder to line this PR up for engineering review.

[itsjeyd]

@CefBoud Just to keep you in the loop, we're still working on getting this PR unblocked.

CC @mphilbrick211

[itsjeyd]

@BbrSofiane A similar question here: Would you be able to review this PR as a CC?

[itsjeyd]

Calling remaining CCs for frontend-app-learning:

@brian-smith-tcril
@arbrandes
@viktorrusakov

Would one of you have bandwidth for reviewing and merging this PR?

[openedx-webhooks]

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