Page MenuHomePhabricator

Make lazy loading images less intrusive
Closed, DuplicatePublic

Description

There are a couple of things we can do to make the lazy loading of images on mobile less intrusive. Meaning that for the user, she/he will not even know that the images are lazy loaded, only in the case of a really slow connection the user will see that, but that's the case even today.

I've been discussing this with @Krinkle and I think a couple of things can make the experience for the user more seamless.

Skip the spinner

You get the spinner every time (I think) and the camera on my phone couldn't catch it because the spinner is so fast on fast connection but check this desktop recording. Do you see the spinner?

spinner.gif (720×1 px, 940 KB)

I don't see how the spinner adds any value for the user. I think there should be a wait time before we use the spinner or rather don't use it at all (to mimic the normal browser behavior) and I think @Nirzar can help us here about best practice.

Do not highlight the image area before it arrives

The grey background and the border doesn't make any sense if we all agree that we want to make the lazy loading as seamless as possible. Then I think we need to make sure we reserve the space needed for making images fit, and do not highlight where it will be, so it looks the same as default browser behavior.

thatgreyarea.png (1×2 px, 1 MB)

Related Objects

Event Timeline

See also:

Two were merged into the current task. We can continue discussion here.

I urge we aim for simplicity with the goal to approach the non-lazyload experience as close as possible. We can add fancy things later once we know the basics are fine. The most important risks, and factors to gain confidence in, are:

  • Performance (no significant overhead in JavaScript processing and its forced style recalculation via used jQuery and mw.viewport methods).
  • UX (perceived performance shouldn't regress).
  • Bot crawling (fallbacks working properly).
  • Cross-browser compatibility of added HTML, JS and CSS. (noscript mode, Grade C, and Grade A)
  • Search engines (I'll elaborate on this in a separate task).
  • Permalinks to section headings (with regards to UX).
  • Firefox back-forth cache vs lazy-load logic.
  • Stability of isElementInViewport logic (does it return true for all edge cases?)
  • Stability of MobileFrontend loadImages logic (does it catch all ways in which the viewport can be initialised and/or changed through scrolling and otherwise etc.)

Proposed changes:

  • Remove border and border-radius. (Complicates layout, could be tried again later in beta.)
  • Insert image directly, don't wait until after loading is finished. (See T135430)
  • Match behaviour of non-lazyload:
    • Placeholder is a simple container in which the browser natively loads an image. The lazyload behaviour merely changes when the load starts. There is no need for extra design elements, styles or animations. (We've never had this in the regular version, either.)
    • Don't apply background-color and spinner by default. As enhancement, maybe apply background and spinner after 1 second of loading has passed (and cancel the timer when image is ready). The background-color could be given a simple CSS transition. - (See also Google RAIL guidelines)
    • This reduces cognitive overhead for the user and increases chances of the image flawlessly loading off-screen while the user scrolls to it. The placeholder should only introduce extra intermediary visual state in case the image fails to load quickly. The default experience should be the same as without lazyload. There's reserved space (just like the browser does normally) and the image loads into it. Nothing more, nothing less.
  • Unwrap image from placeholder once loading completes.

I moved text from this task into T133565 which is currently planned for next sprint. If you have any input on that please do so asap! (In general best to keep issues separately so we can discuss them one by one)

I think the remaining considerations are up to our design lead @Nirzar. Over to you!

I imagined the spinner experience to be different than what we have now.

I think we are missing a big issue here which is the threshold for "isInViewport?" we need to fix that value which will fix most of these issues mentioned here. The lazy loading starts pretty late and last second of something coming into the viewport, if we can change that and load images a tad bit earlier, we will not have the abruptness we are experiencing on fairly fast connections.

I am not against removing spinners here but as i said, if the image is still not loaded (even after we increase the threshold), we need some kind of indication.

the third thing about the treatment, we can remove the border (and radius) and make the background color #f5f5f5 the one we use for base colors. which is much lighter. I wouldn't remove it completely, though. it's difficult to grasp what to expect as a reader without bounding boxes.

I'm re-tagging with sprint 74 in acknowledgment of @Nirzar's comment 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.

I imagined the spinner experience to be different than what we have now.

I think we are missing a big issue here which is the threshold for "isInViewport?" we need to fix that value which will fix most of these issues mentioned here. The lazy loading starts pretty late and last second of something coming into the viewport, if we can change that and load images a tad bit earlier, we will not have the abruptness we are experiencing on fairly fast connections.

Even with that fixed there is an unavoidable situation where the image will sometimes not be present due to fast scrolling (rendering of page is blocked during scroll events).

I am not against removing spinners here but as i said, if the image is still not loaded (even after we increase the threshold), we need some kind of indication.

The spinners currently do not act as an indication until the scroll event stops at which point the image has usually loaded (loading of image is not blocked during a scroll event). I would thus agree that removing them is a good idea.

the third thing about the treatment, we can remove the border (and radius) and make the background color #f5f5f5 the one we use for base colors. which is much lighter. I wouldn't remove it completely, though. it's difficult to grasp what to expect as a reader without bounding boxes.

@Krinkle @Peter is this okay with you?

Change 289012 had a related patch set uploaded (by Krinkle):
Lazy-load: Remove animations

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

Change 289013 had a related patch set uploaded (by Krinkle):
Lazy-load: Remove the placeholder once the image has loaded

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

@Nirzar Could you comment on the matter of animations? Previously mentioned in T135476#2302505 and T133565#2310070.

Currently the animation is triggered after the image has finished loading. This has two kinds of issues:

  • The user doesn't see the image until it is completely loaded. Normally, when browsers render images, there is progressive loading. Which has the benefit of giving the user a partial image if loading gets interrupted or is slow - instead of waiting for the loading to time out and nothing being shown (current behaviour).
  • The browser is informed about the animation after loading has finished loading, via JavaScript. For performance reasons, the event handler of the image load completing is not prioritised by the browser when handling scroll events. As such, even when loading very slowly and on a fast connection, the image does not show up even when it finished loading until scrolling stops and our JavaScript starts the animation. The browser is perfectly capable of drawing to the screen while scrolling, but it has to be either native image loading, or CSS that the browser knew about ahead of time.

It sounds like you want the animation to be kept, however we'll need to decide how to do that with progressive loading.

I propose that we 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.

Normally, when browsers render images, there is progressive loading. Which has the benefit of giving the user a partial image if loading gets interrupted or is slow - instead of waiting for the loading to time out and nothing being shown (current behaviour).

Are you sure they work on mobile browser? if browsers have elegant (non abrupt) ways to progressively load the image we won't need the fadein. currently, from what i can see, images load abruptly without the fadein.

I just talked to @Jdlrobson about workaround the issue you have mentioned.

The browser is informed about the animation after loading has finished loading, via JavaScript.

If we have -webkit-transition in the default image class we can get around the JS entirely?

I propose that we 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.

that sounds good.

The conversation is scattered over these 2 bugs so I've merged them - let's have the conversation in one place.

Change 289012 abandoned by Krinkle:
Lazy-load: Remove animations

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

Change 289013 abandoned by Krinkle:
Lazy-load: Remove the placeholder once the image has loaded

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