Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance Improvements: Load YouTube JavaScript when needed on Home and Product pages #2154

Closed
1 task
briangrossman opened this issue Mar 18, 2021 · 10 comments
Assignees

Comments

@briangrossman
Copy link
Contributor

If you look at the Lighthouse reports for the Home and Product pages, the JavaScript for YouTube is listed first under Remove unused JavaScript (see image below). Ideally, the YouTube would only load if it is needed (i.e if the user plays the video). If possible, please change the behavior to only load when needed.

Designs and Mockups

Screen Shot 2021-03-18 at 5 45 23 PM

Acceptance Criteria:

  • YouTube JavaScript only loads if needed
@briangrossman
Copy link
Contributor Author

Related to #2068 and #2066

@arslanashraf7 arslanashraf7 self-assigned this Apr 2, 2021
@arslanashraf7
Copy link
Contributor

@briangrossman, I looked around this and it looks like we are currently using wagtail's embeds that give us the `embed/iframes for youtube videos and there are some thoughts I would want to share:

YouTube would only load if it is needed (i.e if the user plays the video)

I think this might not be possible since the player loads the thumbnail and other configs from the script and waiting for clicking the play button might need some kind of placeholder that would take the place of youtube embed which will then be replaced with a youtube embed when the user clicks a play button in the placeholder.

I couldn't exactly produce the same lighthouse results for youtube as the attached screenshots but,

  1. If you could try the lighthouse on a page without a youtube video does the light still show a youtube script issue?
  2. Does youtube produce the same result on all the pages with youtube video?

There might be a possibility to just defer the youtube script but we should look into the above points before diving in deep.

@briangrossman
Copy link
Contributor Author

@arslanashraf7
Thanks for digging into this one. It appears as though the YouTube code is only loading on pages that actually have YouTube videos (which is good). The catalog page doesn't load the JavaScript. Note that you can view the dashboard to get this information. Let me know if you want help navigating it. Here's an example report that shows the YouTube JavaScript as being the first issue on the list.

At a high level, we have been using the Lighthouse reporting as a benchmark our sites. We made a number of changes recently that we hoped would have a significant impact... but they did not. We should try to identify a couple focused changes that we think will have a measurable impact. The reports were pointing to this particular code being the most significant issue on a couple pages... it would be good to know if you think it's reasonable to try to defer this code.

Take a look at this page about Web Vitals which gives an overview of how the scoring works.

Thanks!

@arslanashraf7
Copy link
Contributor

@briangrossman, I took a deeper look at this, and below are some insights that might be useful:

  1. As Lighthouse is mentioning the Youtube script as unused but it's not completely unused except for some of the code in that script and since that script is 3rd party we can't edit it.
  2. As a workaround to point 1, I've created a draft PR(Performance Optimization: Defer youtube iframe's script loading #2179) as an option to defer the youtube scrips till the page is loaded and we have received DOMContentLoaded event. This might increase the performance to some extent if not much.
  3. I've noticed that our program page reports are running on Architecture and Systems Engineering and this program let alone has a background image of around 3.4MB which we should look into reducing. Comparing the results of different program pages with similar content in Lighthouse will give you a difference of around 4-6 points in performance from this program.

It might be a good idea to reduce the cover image size of this program.

@arslanashraf7
Copy link
Contributor

The associated PR was tested with dummy embeds since an issue was faced where embed tag of wagtail was returning empty in the local environment and there is an issue related to this in (wagtail/wagtail#6656). We might wait to test the PR once we have worked through #2155.

@briangrossman
Copy link
Contributor Author

@arslanashraf7
Thanks for pointing out the large images. I sent a list of large images to the folks on the marketing team who are responsible for that part of the site. Hopefully they'll cut down the file size.

Why does this rely on #2155, BTW?

I'm happy to test when we're ready. Thanks!

@arslanashraf7
Copy link
Contributor

Why does this rely on #2155, BTW?

@briangrossman, While working on this issue the wagtail embed tags that generate an embed/iframe that gets rendered on the page were not getting generated. The embed tag was returning empty for any youtube URLs and here is the specific line where we do that:

{% embed page.video_url as youtube_video %}

Due to the above issue, I was only able to test the #2179 by adding dummy iframes instead of embed tag. However, There has been a similar issue created on wagtail which I mentioned in a previous comment where it was said to have been resolved in wagtail==2.11 but we are currently on wagtail==2.9.3. Now, this wagtail upgrade is going to be a part of #2155 wherein the process of upgrading the Pillow we'll have to upgrade wagtail anyway.

I'm happy to test when we're ready. Thanks!

I would take another look at the PR to see if there could be a possibility of getting able to test the PR and I'll keep you posted!

@briangrossman
Copy link
Contributor Author

@arslanashraf7 - Can this be unblocked now that #2155 has been taken care of?

@arslanashraf7
Copy link
Contributor

arslanashraf7 commented May 14, 2021

@briangrossman Correct, The draft PR for this issue is already up at #2179 and since the wagtail upgrade is stable now, after the rebase we should be able to test it locally to check that the wagtail is now generating proper youtube embeds.
I'll update the associated PR for this issue in the coming week.

@arslanashraf7
Copy link
Contributor

Closing, Since the associated PR has been merged and deployed on Prod.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants