As in T135476#2318155, I'm re-tagging with sprint 74 in acknowledgment of @Nirzar's comment (in T135476#2318155) and to accommodate for more time to analyze design, while we wait for T133565: Images do not load until scroll event completes (not during scroll) on iOS to be built and settle in.

@Krinkle I'm having difficultly groking the description of this task.
Could you take a pass at explaining it better and separating the concerns captured in T135476 ?

In T135430#2299679, @Jdlrobson wrote:

I assume that this complexity is the reason the implementation also adds the image to the page layout after it has completely finished loading.

Can you elaborate what you mean by this statement?

I meant that I assume the intent to have animations is what led to the choice to load the image in a detached state and thus only insert it only once the image finished loading. Doing so understandably makes it considerably easier to have a fade-in animation. However, this comes at the cost of:

1. Having no visual progress while loading.
2. Having no image at all unless the loading successfully completes. If loading is disrupted, the user will see no image. In theory this could be worked around by inserting the image from an "error" handler, but this may not happen in the case of a connection that appears merely slow but is really absent ("LieFi"). It also feels like a futile attempt at trying to be better than the browser at rendering an image in a performant way that balances speed, user experience and power usage.
3. Having no image render during scroll (T133565). Due to the visibility depending on a CSS animation, which in turn is triggered from JavaScript adding a class, which in turn depends on a next animation frame, which is triggered from an image "load" event handler in JavaScript - no visual progress can happen while scrolling because image load JavaScript events (and other event loop stuff) tends to be at lower priority while scrolling. A scroll event handler can and will fire, but not an image load event. Those will be deferred.

This last point is the main one here. If the image is inserted into the DOM directly it will render naturally - even while scrolling.

This does mean however that there is no animation.

In the opening post I recommended removing the animation for that reason.

The other option I mentioned, if you want an animation, requires it to be a CSS transition or animation that is triggered independently (it must not depend on a JavaScript starting the animation manually via a timer or other callback) . For example, you can apply the animation to a plain img selector in CSS (without waiting for the loaded class). And perhaps delay the animation in CSS by 1 second. This means it will fade in after 1 second (counting from when the image load starts, as triggered by lazyload scroll handlers inserting the image in the DOM). If the image is loaded by then, it'll fade in naturally (even if the user is still scrolling). If the image is not loaded, the empty or partial image canvas will appear and it'll render progressively from there, just like it would without lazy loading. The animation has no added value in that case.

Thankyou. Much clearer now about what you are trying to achieve. Once T133565 is merged I can demo your suggestions to @Nirzar and see what he thinks

I talked to Nirzar. These were the conclusions:

• Spinner can be removed
• Animation should stay. It's important but it should not depend on the loaded class that is added via JavaScript
• Remove the border (and radius) and make the background color #f5f5f5 the one we use for base colors. which is much lighter

I talked to Nirzar. These were the conclusions:

+1

Change 289014 abandoned by Krinkle:
Lazy-load: Allow progressive image loading

https://gerrit.wikimedia.org/r/289014

Change 293305 had a related patch set uploaded (by Jhernandez):
Make lazy loading images UX less intrusive

https://gerrit.wikimedia.org/r/293305

293305 deals with:

• Spinner should be removed
• Remove the border (and radius) and make the background color @colorGrayLightest

Gif showing the loading now. Personally seems faster than with the spinner (even if it is the same loading time), which is cool.

Regarding

Animation should stay. It's important but it should not depend on the loaded class that is added via JavaScript. We should schedule the animation via CSS at the same point that we start loading the image itself. In other words: Insert the image element into the DOM and start the animation (for example: a 0.3 second fade-in animation with a 0.5s delay). Which means after 0.5s the image canvas will fade in whether completed or not. If not completed, the rest of the image will load progressively to the user directly. All which can happen concurrently with scroll events.

If we set a delay, the animation will trigger when image may be already visible, making a flash from image to white, then animate to image. Not good. If we set the image's opacity to 0 during that delay, cached images that load in 0 will appear white during the delay when they shouldn't necessarily.

• Because so, I'm not going to implement a forced delay on the animation (unless someone has any objections with good reasoning).

Regarding triggering the animation before the image loads, I want to clarify myself in written form with @Nirzar that that is the wanted behavior:

• Currently
  • Lazy images look like a light gray square before loading (see gif in previous comment)
  • When image is close to viewport, it'll start loading, but the square will still be the same (previously there was the spinner)
  • When the image is fully loaded, the gray rectangle cross-fades to the image quickly (0.3s)
• With the proposed change of inserting the image immediately while it loads:
  • The animation will trigger instantly the moment the image starts loading (instead of when fully loaded)
    • If the image is cached animation will show up almost instantly, and the gray rectangle will cross-fade to the image
    • If the image is not cached (most frequent case), then the rectangle will start crossfading to nothing (white background) and the image will start downloading. If during those 0.3s there's some image data coming in, the browser will render part of the image in between the fade.
    • If the image takes longer to respond than 0.3s, then the rectangle will have cross-faded to white, and there will be an empty white space there.

It may be hard to visualize these ^ interactions, I can work on showing them if necessary. Can you confirm the change @Nirzar?

Personally all animated image loading I've seen doesn't show the image until loaded to show a full consistent animation. But there's also a good point on making lazy loading images more invisible by simulating browser behavior (the progressive loading). But then if we want it to be transparent, the placeholder with the background doesn't make sense since the browsers don't do that by default.

Thoughts?

I'd appreciate review on the current patch for now, but don't move to signoff if merged, move back to doing or -1 please.

Personally all animated image loading I've seen doesn't show the image until loaded to show a full consistent animation. But there's also a good point on making lazy loading images more invisible by simulating browser behavior (the progressive loading). But then if we want it to be transparent, the placeholder with the background doesn't make sense since the browsers don't do that by default.

I agree, if we're trying to mimick the default loading behavior, we should get rid of the background and the animation.

Moving back to -1 as there is more work to be done and as suggested by @Jhernandez above.

Change 293305 merged by jenkins-bot:
Make lazy loading images UX less intrusive

https://gerrit.wikimedia.org/r/293305

I'd appreciate review on the current patch for now, but don't move to signoff if merged, move back to doing or -1 please.

Mibad.

I think it would be good with an example with and without the animation so @Nirzar can check, so we can agree of what it will look like.

I personally think 300 ms fading time can make it feel slow. When I use the Hovercards I get a TTFB 150-200 ms most of the time and I feel that.

@Peter I've collected some rationale on transitions and animations over at T77949.

@Peter I'm going to make a WIP patch that removes the animations and favors image insertion pre-loading (browser-like progressive image) and will record a few gifs to show and clarify understanding for everyone :D

Change 293708 had a related patch set uploaded (by Jhernandez):
Remove outdated comment

https://gerrit.wikimedia.org/r/293708

Change 293709 had a related patch set uploaded (by Jhernandez):
WIP: Insert lazy images in the DOM immediately

https://gerrit.wikimedia.org/r/293709

Mkay, here are two gifs I captured before and after 293709 (which makes the change specified on this task). Note that they take +10s to show the effect, so wait after you open them.

a) Currently: Delayed insertion (after image has loaded)

b) Proposed in this task: Immediate insertion (before image has loaded)

Gifs have been recorded on a Good 2G chrome dev-tools profile to make the progressive image effect obvious (otherwise what's the point to even discuss).

Notes:

• On a) the placeholder stays longer, but the image appears all at once with the cross-fade
• On b) the placeholder get's swapped earlier, fading to white (barely noticeable, since from f5f5f5 to ffffff in 300ms is really subtle), while the image gets inserted and starts loading. Then the browser progressively renders the image as it get's the data.

Hopefully this will clear what I'm asking about. This is a change in UX and just wanted to clear with Design that this is what we're going for.

If so, I'd recommend considering removing the gray placeholder, and fully imitating the browser (empty space while image is waiting to be loaded).

Change 293708 merged by jenkins-bot:
Remove outdated comment

https://gerrit.wikimedia.org/r/293708

If so, I'd recommend considering removing the gray placeholder, and fully imitating the browser (empty space while image is waiting to be loaded).

+1

@Jhernandez

Last where I left the conversation i remember talking about this issue - while I'm scrolling through the article and images being loaded only when you stop loading due to the fade in animation. If that's still the case then version B will create half loaded images while you are scrolling the article. It is also more jarring than the whole picture showing up.

We started from placeholders with spinners in them and ending up with browser style image loading. the only difference is, images will load lazily. It is not good experience but something we could live with.

The placeholer background is a good to have since as the name suggests, it holds the place otherwise you will see empty white spaces for no reason.

I'm guessing there is another task to show the blurry version of the image and even for that we will *have to* do the fade in. I carefully saw medium's experience and they fade in the image on touchmove events even without touchend happening. I think solving that problem is future proof if we go for blurry placeholders in the future.

The placeholer background is a good to have since as the name suggests, it holds the place otherwise you will see empty white spaces for no reason.

Without the spinner the user won't know whether something is still loading inside the placeholder (Users who block ads may think of the placeholder as a place where ad is supposed to appear - I know we don't serve ads). Coupled with showing the image once it's ready, this is a bad UI decision imo. The users are already familiar with empty spaces as browsers have been operating that way, so I'd say remove the background and let the browser load the image partially.

It's worth noting the original issue (dating back to Jon Katz's original bug report T133565) was around scrolling so it would be only fair to redo these gifs with a page on a 2g connection on an ios tablet.

You can see how medium did it here:
https://jmperezperez.com/medium-image-progressive-loading-placeholder/
(feel free to play around with a test article https://medium.freecodecamp.com/salary-negotiation-how-not-to-set-a-bunch-of-money-on-fire-605aabbaf84b#.exmxqyt5v)

The key thing they do differently to use is use a css transition and when I tried that out it was much smoother. I haven't had much time to research but I'm guessing the CSS keyframe animation is blocked on a scroll but a CSS transition is not since it can be triggered as soon as it's in the DOM. Was there a reason we used keyframes?

FWIW medium.com on a 2G connection:

In T135430#2378825, @Jdlrobson wrote:

Was there a reason we used keyframes?

See 103614e. Now that we're not using setTimeout, we can revert to using CSS transitions if we want.

we can revert to using CSS transitions if we want.

can we please try and figure this out. i would like to get the smooth transition in and not go back to 90s browser style image load. as i said before, in future when we do blurry placeholder all of this will pay off :D

Change 293709 abandoned by Jhernandez:
WIP: Insert lazy images in the DOM immediately

Reason:
Not the wanted behavior for now. See https://phabricator.wikimedia.org/T135430#2381123

https://gerrit.wikimedia.org/r/293709

Per design's comments (T135430#2381123), I've abandoned the progressive image loading in favor of the current version given the UX concerns.

I've extracted what I've been able to gather about the images not animating in while scrolling into a followup task better specified and self contained (T137874)

If needed so, discussion about if images should load browser-like or app-like will have to happen to reach clear consensus and common understanding.

There are valid concerns and there are heuristics we can apply to mix the two modes if needed be. For example, if an image is taking more than 3s (or other amount) to load, in order to not show an empty placeholder with no indication of loading or progress, we could switch to progressive loading and that image wouldn't have the full consistent animation, but would progressively show loading thus avoiding the empty placeholder for a long time without signs of activity (while keeping other images that loaded faster with the better UX). Of course magic-numbers are always troublesome, but there's things we can do.

I'm going to consider this task worked on to the fullest of it's extent and move it to signoff.

If there are more concerns about behavior I'd suggest having a meeting of Web-Team-Backlog, Performance-Team and Design to discuss and reach clear common consensus of the work to be done.

In T135430#2378887, @phuedx wrote:

In T135430#2378825, @Jdlrobson wrote:

Was there a reason we used keyframes?

See 103614e. Now that we're not using setTimeout, we can revert to using CSS transitions if we want.

Ignore the second sentence. Of course we might have to revert to using setTimeout to reliably trigger the CSS transition :/

This is working on the browsers below on the beta cluster in beta mode. But it seems to be pushing the content around upon lazy load, making things perceptibly jumpy:

Contrast this with the existing lazy loading on the prod cluster in beta mode, which doesn't have the jumpiness.

We should get rid of the jumpiness. I assumed incorrectly that implicit in the Description was the notion that we would continue to have the non-reflowing behavior of the existing lazy loaded images. So, trying to make that more explicit, this is what I think in the short run we need to do until we reconvene on T137874: Lazy loaded images animation doesn't trigger until scrolling stops in mobile browsers -

• Spinner should be removed
• Make the background color @colorGrayLightest
• (not sure if this already the case?) The placeholder should be the same size as the image
• The border, if any, should not outline the placeholder
• There should be no additional user perceived reflowing introduced by these changes

@Jhernandez, would this be too much trouble for this task? If I understand correctly it's probably reinstating and maybe modifying some of the lines deleted from images.less. My naive read of the the tip of MASTER is that our latest enhancements aren't made any less performant by reinstating some of those lines.

Browsers tested:

• iPhone 5c iOS 9.3 Safari
• Nexus 7 Android 5 Chrome
• LG Nexus 4 Android 5 Chrome
• HTC Trophy 7 Windows 7.5 IE
• Opera Mini (via iPhone 5c iOS 9.3 with transcoder enabled)

Thanks for the through testing. Investigating...

Change 294689 had a related patch set uploaded (by Jhernandez):
Fix reflow from lazy loaded images improvements

https://gerrit.wikimedia.org/r/294689

I paired with @Jhernandez on 294689: Fix reflow from lazy loaded images improvements so I'm uneasy about merging it.

Change 294689 merged by jenkins-bot:
Fix reflow from lazy loaded images improvements

https://gerrit.wikimedia.org/r/294689

Agreed, I didn't see the reflow on iOS Safari or Android Chrome.