search

mitodl / ocw-hugo-themes

A Hugo theme for building OCW websites
BSD 3-Clause "New" or "Revised" License
5 stars 4 forks source link

Optimize imports using global flag variable to ensure videojs and nanogallery are loaded once only #1191

Closed ibrahimjaved12 closed 1 year ago

ibrahimjaved12 commented 1 year ago

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/1749

Description (What does it do?)

When we replaced window.onload with window.addEventListener to allow multiple listeners for load, we faced a new problem that a single kind of imports (eg. videojs imports) are called multiple times if there are multiple videos in a page. Not only that was not optimized, but this also caused some problems with the typescript functionality of download button popup (opening and closing) on videojs (online) videos.

This PR fixes this problem by using a global flag variable to keep track of videojs/nanogallery2 dynamic imports and to ensure they execute only once.

How can this be tested?

  1. Make sure the videos play + the video download button / related resources tab etc works as expected.

The functionality of the download button is as: Click on video's download button, a popup should appear. Click on it again, it should close. Or, click outside the area, and it should close. Or click on another video's download button within the same page, the first video's popup should close and the other one's should open. (If it has Download Video or Download Transcript present)

The edge cases that caused problems:

  1. If there were multiple dynamic imports present in a single page using window.onload only the last one would execute, hence we switched to using AddEventHandler. The original approach caused issues in the course where we had both image gallery+video present in a single page. (16.885j-fall-2005 --> related resources)
  2. With the AddEventHandler, we had problem in multiple videos in a single page, because multiple import calls were made. (18.02sc-fall-2010 -- 57th session) Here we had problem we download button popup not working as expected, which is the reason for this PR.

And then of course we will want to handle these and every case for offline theme as well. The steps to test this would be same as this: https://github.com/mitodl/ocw-hugo-themes/pull/1185 but a more thorough test with regards to videojs/nanogallery2 functionalities such as combinations of such different edge cases.

Now as I understand, for offline theme, we do not have the video download button working as part of functionality because it's offline theme. And if we have other video tabs, like related resources, or "view video page", we would show the download icon on the right but it still shouldn't work. And if we don't have those additional tabs, then we wouldn't have the tab with download button icon at all. Please confirm this @ChristopherChudzicki.

github-actions[bot] commented 1 year ago

Netlify Deployments:
www: https://ocw-hugo-themes-www-pr-1191--ocw-next.netlify.app/
Course v2: https://ocw-hugo-themes-course-v2-pr-1191--ocw-next.netlify.app/