-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add Lazy Loading for Homepage Images and Check script #5998
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
name: Image LazyLoad Check | ||
|
||
on: | ||
push: | ||
branches: | ||
- main | ||
|
||
jobs: | ||
run-tests: | ||
runs-on: ubuntu-latest | ||
|
||
steps: | ||
Check failure on line 12 in .github/workflows/knative-lazyload-check.yaml GitHub Actions / style / suggester / github_actions
|
||
- name: Checkout repository | ||
uses: actions/checkout@v3 | ||
|
||
- name: Set up Python | ||
uses: actions/setup-python@v4 | ||
with: | ||
python-version: '3.x' | ||
|
||
- name: Set GITHUB_TOKEN environment variable | ||
run: echo "GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }}" >> $GITHUB_ENV | ||
|
||
- name: Run Build | ||
run: ./hack/docker/test.sh | ||
|
||
- name: Run Python script | ||
run: python check_img_lazy.py |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import os | ||
import sys | ||
import subprocess | ||
from bs4 import BeautifulSoup | ||
|
||
def check_img_tags(base_dir): | ||
"""Recursively check HTML files for <img> tags without the lazy loading attribute.""" | ||
issues_found = False | ||
|
||
for root, _, files in os.walk(base_dir): | ||
for file in files: | ||
if file.endswith('.html'): | ||
file_path = os.path.join(root, file) | ||
with open(file_path, 'r', encoding='utf-8') as f: | ||
content = f.read() | ||
soup = BeautifulSoup(content, 'html.parser') | ||
for img in soup.find_all('img'): | ||
if 'loading' not in img.attrs or img.attrs['loading'] != 'lazy': | ||
issues_found = True | ||
line_number = find_line_number(content, img) | ||
print(f"File: {file_path}, Line: {line_number}, Missing lazy loading: {img}") | ||
|
||
return issues_found | ||
|
||
def find_line_number(content, tag): | ||
"""Find the line number of the given tag in the content.""" | ||
content_lines = content.splitlines() | ||
|
||
|
||
tag_attrs = {k: ' '.join(v) if isinstance(v, list) else v for k, v in tag.attrs.items()} | ||
|
||
for i, line in enumerate(content_lines): | ||
|
||
if all(str(attr) in line for attr in tag_attrs.values()): | ||
return i + 1 | ||
|
||
return 'unknown' | ||
|
||
if __name__ == "__main__": | ||
|
||
base_dir = 'site' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please make this configurable. Script should receive the directories to check HTML files. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The function runs on the "site" folder because this folder is created after the build command. That's why I set base_dir = 'site.' There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. Let's leave as is. I need to check this later. |
||
issues_found = check_img_tags(base_dir) | ||
|
||
|
||
if issues_found: | ||
sys.exit(1) | ||
else: | ||
print("All img tags have lazy loading attribute.") | ||
sys.exit(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @pierDipi @cardil