Add Lazy Loading for Homepage Images and Check script #5998

shivamgupta2020 · 2024-05-28T23:49:20Z

Proposed Changes

added lazy load in homepage images

knative-prow · 2024-05-28T23:49:26Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: shivamgupta2020
Once this PR has been reviewed and has the lgtm label, please assign dsimansk for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

netlify · 2024-05-28T23:49:39Z

✅ Deploy Preview for knative ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`34b7d52`
🔍 Latest deploy log	https://app.netlify.com/sites/knative/deploys/6668bf5c7f3a05000946eb2f
😎 Deploy Preview	https://deploy-preview-5998--knative.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

aliok · 2024-05-29T09:05:18Z

@shivamgupta2020

The check script should do these:

Build the website
Parse HTML pages that are built
Check for img tags without lazy
Report the locations (file, line number) of the tags w/o lazy attribute in the script logs (stdout)
Fail (by exiting with a code that's not zero)

This script should be running in a Github action.

I would recommend this approach:

Write that script (can be a Python script)
Include it in the PR, with documentation and dependencies (requirements.txt that can be used by pip)
Get feedback, make sure it works
Then create a GitHub action that runs the script

shivamgupta2020 · 2024-05-29T10:14:55Z

Thanks, @aliok , for the suggestions. I’m currently working on the script part and will include all your suggestions in a new commit as soon as possible.

shivamgupta2020 · 2024-05-31T05:35:46Z

@shivamgupta2020

The check script should do these:

Build the website

Parse HTML pages that are built

Check for img tags without lazy

Report the locations (file, line number) of the tags w/o lazy attribute in the script logs (stdout)

Fail (by exiting with a code that's not zero)

This script should be running in a Github action.

I would recommend this approach:

Write that script (can be a Python script)

Include it in the PR, with documentation and dependencies (requirements.txt that can be used by pip)

Get feedback, make sure it works

Then create a GitHub action that runs the script

Should the script also run for v1.13, v1.12, and the development folder present in the "site" folder?

aliok · 2024-06-04T21:42:50Z

check_img_lazy.py

+import os
+import sys
+import subprocess
+from bs4 import BeautifulSoup


Is it a possibility to use standard Python libraries for parsing HTML files?

What's your opinion here @shivamgupta2020 ? We need to instruct people to install some dependencies if we go with BeautifulSoup. However, we might have some non-standard HTML content that might not be parseable with standard Python libraries.

Let's ask @knative/productivity-leads and @cardil specifically.

@aliok, I experimented with HTMLParser and IXML for parsing HTML files, but encountered numerous errors and incomplete parsing. Therefore, I switched to BeautifulSoup. I also think we should gather opinions from others on this approach.

cc @pierDipi @cardil

aliok · 2024-06-04T21:46:05Z

check_img_lazy.py

+def build_website():
+    """Build the website using the specified command."""
+    try:
+        subprocess.check_call(['./hack/docker/test.sh'])  
+    except subprocess.CalledProcessError as e:
+        print(f"Website build failed: {e}")
+        sys.exit(1)


Please leave this part out of the script.

This script will eventually run in a GitHub Actions workflow:

Build the website

Run the script

If there are images w/o loading=lazy, script should fail

If there are no images, script should exit successfully

Can you put the script in a GitHub actions workflow please?

Also, we need to document this change with instructions. In the relevant section here: https://github.com/knative/docs/blob/main/contribute-to-docs/README.md

Yeah, sure, I will remove the build function from the script.
I will look into the GitHub action workflow and add the script as soon as possible.

aliok · 2024-06-04T21:47:21Z

check_img_lazy.py

+    build_website()
+
+
+    base_dir = 'site'  


Please make this configurable. Script should receive the directories to check HTML files.

The function runs on the "site" folder because this folder is created after the build command. That's why I set base_dir = 'site.'
Why do we need to make it configurable?

Sounds good. Let's leave as is. I need to check this later.

shivamgupta2020 · 2024-06-11T21:25:28Z

I've created the workflow file as @aliok suggested. Currently, most of the images on the website are not lazy-loaded, resulting in many errors. Could you please check the script output on your local machine to verify it's working as expected? Also, please review the workflow code to ensure it's implemented correctly.
Thank you!

knative-prow bot requested review from nainaz and skonto May 28, 2024 23:49

knative-prow bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 28, 2024

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 1, 2024

knative-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 4, 2024

aliok reviewed Jun 4, 2024

View reviewed changes

knative-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 11, 2024

shivamgupta2020 added 2 commits June 12, 2024 01:50

added lazy-load in homepage images

5f5d2d6

added script

4da7b8e

shivamgupta2020 force-pushed the lazy-load-image-check branch from e4d6861 to 0b192e9 Compare June 11, 2024 20:22

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 11, 2024

added workflow file & removed build command from script

34b7d52

shivamgupta2020 force-pushed the lazy-load-image-check branch from 0b192e9 to 34b7d52 Compare June 11, 2024 21:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Lazy Loading for Homepage Images and Check script #5998

Add Lazy Loading for Homepage Images and Check script #5998

shivamgupta2020 commented May 28, 2024

knative-prow bot commented May 28, 2024

netlify bot commented May 28, 2024 •

edited

Loading

aliok commented May 29, 2024

shivamgupta2020 commented May 29, 2024

shivamgupta2020 commented May 31, 2024 •

edited

Loading

aliok Jun 4, 2024

shivamgupta2020 Jun 6, 2024

aliok Jun 6, 2024

aliok Jun 4, 2024

shivamgupta2020 Jun 5, 2024

aliok Jun 4, 2024

shivamgupta2020 Jun 5, 2024

aliok Jun 5, 2024

shivamgupta2020 commented Jun 11, 2024

Add Lazy Loading for Homepage Images and Check script #5998

Are you sure you want to change the base?

Add Lazy Loading for Homepage Images and Check script #5998

Conversation

shivamgupta2020 commented May 28, 2024

Proposed Changes

knative-prow bot commented May 28, 2024

netlify bot commented May 28, 2024 • edited Loading

✅ Deploy Preview for knative ready!

aliok commented May 29, 2024

shivamgupta2020 commented May 29, 2024

shivamgupta2020 commented May 31, 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

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

shivamgupta2020 commented Jun 11, 2024

netlify bot commented May 28, 2024 •

edited

Loading

shivamgupta2020 commented May 31, 2024 •

edited

Loading