-
Notifications
You must be signed in to change notification settings - Fork 45
Image Lazy Loading & Enhanced Loading Experience #1230
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
Conversation
- Add an X button to Loading View to allow exiting out of V spinner view early - First attempt to add lazy loading to images - Attempt to add timeouts and in case of failure set "Dead Image" badge similar to Dead Tweets
…. This can be done by the browser as iOS15.4+ now accepts the loading="lazy" attribute. Tweets continue to use intersection method. If an image fails to download during a lazyload, replace with Dead Image ghost badge.
|
|
||
| // Create new image element with native browser loading | ||
| const successImg = document.createElement('img'); | ||
| successImg.setAttribute('alt', ''); |
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.
Why clear the alt here, is it visible with a long-press or similar, and if it is user-visible is it worth preserving the original by stuffing it into a data-orig-alt attribute?
(I’m reading this on my phone and can’t see much context but I’m assuming this is the success case for lazy loading)
| let config = { | ||
| root: document.body.posts, | ||
| rootMargin: `${topMarginOffset}px 0px`, | ||
| threshold: 0.000001, |
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 worth a comment here for intent, e.g. // load when first row of pixels visible ?
(This is my first time looking at IntersectionObserver so I had to check MDN and guess at the config's intent)
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.
Oops, originally meant to add this to the newly added config, did not realize this is old/moved code, not going to ask you to guess at the original author's intent. So never mind unless you know for sure.
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.
If it's questionable and eyebrow raising, I probably was the original author. If not today, then years ago...
This particular snippet is not in the latest commit tho. There's similar setups to this used for the playback of the ghost lottie and also for "lazy loading" Tweets. These are better defined with descriptive names and comments in the latest commit.
e.g.
// Set up lazy-loading IntersectionObserver for tweet embeds
// Tweets are loaded before entering the viewport based on LAZY_LOAD_LOOKAHEAD_DISTANCE
// Disconnect previous observer if it exists (prevents memory leak on re-render)
if (Awful.tweetLazyLoadObserver) {
Awful.tweetLazyLoadObserver.disconnect();
}
const lazyLoadConfig = {
root: null,
rootMargin: `${LAZY_LOAD_LOOKAHEAD_DISTANCE} 0px`,
threshold: 0.000001,
};
IntersectionObserver is meant to detect when html elements have crossed into the viewport. My homespun lazy load solution was to use this with an offset of 600px so that we fire the embedTweetNow javascript command for tweets a post or two in advance of them scrolling into view.
For the ghost, it's to reduce resources playing the lottie animation when the thing isn't anywhere near within view.
|
I don't feel qualified to fully bless a PR but I like the feature and wanted to give you some eyeballs; LGTM. |
|
I’m clearly the bottleneck here so please feel free to merge any pr’s that make sense. Ideally someone does some kind of review first :) |
|
Thanks for the eyeballs! I have pushed some changes and added some comments. Always happy to clarify or change/redo anything. |
syncsynchalt
left a comment
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.
Users are going to be stoked to get this (though really they won't notice it, just that paging no longer "hangs" as much).
I say if it's working in your testing, ![]()
|
I'll do a little more testing and then merge this one in Thanks again |
… were (race condition between twitter.js widget being loaded and embed processing taking place). Added the retry option to Dead Tweet badge, similar to Dead Image badge
…eout in RenderView.js). Added optional show/hide configuration option for the newly introduced LoadingView status message and exit button. This will now display for loading threads but not for loading while previewing new posts or private messages.
Since Imgur and maybe even postimages are dying/dead, many smaller image hosts are being used by SA posters.
These new hosts can cause timeouts/long page load times for images, resulting in the V throbber lottie progress view to be displayed for many minutes sometimes.
This branch reduces the number of images a new page downloads to ten. This does not count Avatars or forums attachments, but rather all other third party images in post contents.
Images 11+ are lazy loaded using the browser's native loading="lazy" attribute which was supported by iOS Safari starting in 15.4.
(Previous commits on this branch used IntersectionObservers with a 600px offset so that the images were download and embedded before the user scrolls to the post. That is no longer the case for images, but it IS the approach applied to embedding Tweets.)
In addition to this, the loadingview has been enhanced to include an exit button and progress messages, on a 3 second delay. For most page loads, this will remain unseen but on longer load times the two new elements will fade into the view. This allows the user to exit out of the loading view early and begin reading the page.
Attempts to load the initial batch of images now have timeouts and retry logic.
The "Dead Tweet" badge has been adapted to now also apply to images.
Dead Image badges have a manual Retry button that users can use if the image fails to load.
These features make the loadingview exit button and progress indicator even less likely to appear, but am leaving them in as they still provide an escape hatch if needed.
A good page for testing is Page 2 of the Comics thread in PYF.
Below is the LLM generated take on the changes (expect emojis):
Lazy Loading and Enhanced Loading UX
Overview
Implements lazy loading for post content images to improve perceived performance and page load times, along with enhanced loading view feedback including progress tracking, timeout detection, and user control.
Features
1. 🚀 Lazy Loading for Images
What: Posts now load the first 10 images immediately, then defer loading of subsequent images using the browser's native lazy loading.
Why: Improves initial page load time and reduces unnecessary bandwidth usage. Users see content faster while images below the fold load on-demand.
Implementation:
loading="lazy"attribute on post content images 11+Files:
loading="lazy"to images 11+2. 💀 Dead Image Badges
What: Failed images are replaced with animated ghost badges (similar to dead tweets) that show the filename and offer retry functionality. Works for both immediately-loaded images (first 10) and lazy-loaded images (11+).
Why: Provides clear visual feedback when images fail to load (404s, broken images, etc.) and gives users the ability to retry without refreshing the entire page.
Features:
Implementation:
.dead-embed()mixin for reuse across tweets/imagesFiles:
3. 📊 Image Download Progress Tracking
What: Loading view displays real-time progress of image downloads ("Downloading images: 5/10").
Why: Users understand what's happening during page load and have visibility into download progress.
Implementation:
imageLoadTrackersends progress updates to Swift sideFiles:
4.⏏️ Enhanced Loading View
What: Loading view improvements for better UX:
Why: Users stuck on slow loads can escape without force-quitting the app. Status messages provide transparency.
Features:
Implementation:
onDismisscallback for exit button tapupdateStatus()virtual method for subclass customizationFiles:
Technical Details
Constants & Configuration
All timeout values, distances, and delays are defined as named constants with detailed documentation:
IMMEDIATELY_LOADED_IMAGE_COUNT = 10(synced between Swift/JS)statusElementsVisibilityDelay = 3.0secondsMemory Management
{ once: true }for single-fire event listenersRace Condition Prevention
handledflags prevent duplicate event processingembedTweetsInProgressflag prevents concurrent setupBrowser Compatibility
loading="lazy"attribute (iOS 15.4+ support)Performance Impact
Positive:
Considerations:
Net Result: Significant improvement in perceived performance, especially for image-heavy threads.
Testing Recommendations
Image Lazy Loading
Timeout & Error Handling
Progress Tracking
Loading View
Edge Cases
Files Changed
7 files changed (+1131, -177 lines)