-
-
Notifications
You must be signed in to change notification settings - Fork 773
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
perf(image): replace js images lazy loading by html loading
attribute
#5816
base: edge
Are you sure you want to change the base?
perf(image): replace js images lazy loading by html loading
attribute
#5816
Conversation
5b058a5
to
542f683
Compare
], | ||
return preg_replace( | ||
'/<((?:img|iframe)[^>]+?)([^>]*)>/i', | ||
'<$1 loading="lazy"$2>', |
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.
We need to check upstream that we filter out existing loading
attributes (that might already be the case), to make sure we do not end up with two identical attributes, which would be invalid
@@ -15,7 +15,6 @@ echo htmlspecialchars(json_encode(array( | |||
'auto_mark_scroll' => !!$mark['scroll'], | |||
'auto_load_more' => !!FreshRSS_Context::$user_conf->auto_load_more, | |||
'auto_actualize_feeds' => !!Minz_Session::param('actualize_feeds', false), | |||
'does_lazyload' => !!FreshRSS_Context::$user_conf->lazyload , |
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.
This removes the option of not having lazy loading, which is not good
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.
Thanks for the PR!
The use-case of not using lazy-loading must be preserved (descriptions of the need can be found in past issues, for instance on slow connections, to make sure all images are loaded before starting to read)
Closes #2371
Changes proposed in this pull request:
loading="lazy"
html attributeloading="lazy"
on all rss content image if lazy-loading configuration is true (even if the "display_post is enabled)How to test the feature manually:
loading="lazy"
attribute is setPull request checklist:
Additional information can be found in the documentation.