Page MenuHomePhabricator

Use LazyLoadTransform in content-html
Closed, ResolvedPublic

Description

Consider using the LazyLoadTransform of the page library so that the initial transforms for lazy loading images (img -> placeholder span) are run server side.
For testing and convenience also consider adding JS to the DOM to trigger the reversal of the above transformation (placeholder span -> img) so that the images are visible again.

Event Timeline

bearND triaged this task as Normal priority.Jul 2 2018, 11:40 PM
bearND created this task.
Restricted Application removed a project: Patch-For-Review. · View Herald TranscriptJul 2 2018, 11:40 PM

@bearND, LazyLoadTransformer is for client integrations. LazyLoadTransform.queryLazyLoadableImages() and convertImagesToPlaceholders() should be all that's needed for the server-side transform. AFAIK, these functions already have no dependency on events and should run headless. Let me know if you have trouble with them.

Jhernandez added a subscriber: Jhernandez.

Put it on doing since it is a subtask of the task you are doing and you are assigned. If you want someone to pick it up and help with it, move it back to todo and unassign from it ✊

Change 445073 had a related patch set uploaded (by BearND; owner: BearND):
[mediawiki/services/mobileapps@master] WIP: content-html: add lazy loading of images

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

@Niedzielski Thank you. I think you're right. There is probably no need to split up things for this one in the page lib. What confused me first was that the class LazyLoadTransform also has the loadPlaceholder method. (Possibly this method could be moved to a client only class, but I'm ok with that now.)

Now the images are turned into the placeholder spans. What I'm struggling with is right now is how to correctly use the loadPlaceholder method to turn them back into images once the placeholder gets close to the viewport.

I'm using this in a script tag right now:

const transformer = new pagelib.LazyLoadTransformer(window, 2);
transformer.loadPlaceholders();

The above is derived from the Android app sections.js.

With my above patch (PS 1), I still don't see the images, just the placeholder spans. What am I missing?

bearND renamed this task from Split up LazyLoadTransform transform into client and server side code to Use LazyLoadTransform in content-html.Jul 11 2018, 5:47 AM
bearND updated the task description. (Show Details)

@bearND, in case you haven't seen them, I generally recommend using the demos in the pagelib itself where available for example integrations. Here's the lazy load transform and lazy load transform**er** demo.

The list of images pending download are initialized in the call to convertImagesToPlaceholders(). If you're supplying clients with this list in some other way, then you may want to alter the constructor to allow an array to be passed in. Otherwise, you'll still need to call transformer.convertImagesToPlaceholders() at the start (or per section if you're loading content dynamically). Let me know if you have any trouble

bearND added a comment.EditedJul 11 2018, 3:28 PM

@Niedzielski I've noticed the two demos but not sure why there are two demos for lazy loading of images, and not just one. Looks like LazyLoadTransform also runs the image widening and reimplements loadPlaceholders but probably a little bit differently.

If the client has to run the same functions (LazyLoadTransform.queryLazyLoadableImages() and LazyLoadTransform.convertImagesToPlaceholders()) again then I think it doesn't help the client much to run the lazy load transform on the server side. Should we just skip the whole lazy loading prep on the server side?

LazyLoadTransformer is the client glue that would be necessary for client integration. It could probably be better named. It's stored in the pagelib to stay platform independent and so that two implementations aren't maintained on Android and iOS. LazyLoadTransformer composes LazyLoadTransform and various events such as scrolling. Android and iOS use LazyLoadTransformer and don't directly interact with LazyLoadTransform.

The LazyLoadTransform demo showcases lazy loading and not much else. It's really useful for development and testing of lazy loading code specifically since images shouldn't load until clicked.

The LazyLoadTransformer demo shows the same lazy loading (since it wraps LazyLoadTransform) but also how other image transformations tie in and event handling like scrolling.

It would be excellent to perform the placeholder transform server side since this has to occur prior to rendering and requires the client to construct a full DOM in a separate Document otherwise which is resource intensive. The images converted to placeholders could appear in the JSON response and the LazyLoadTransformer constructor modified to accept a parameter, the initializing array. I think it would be a two line change in the pagelib.

bearND added a comment.EditedJul 12 2018, 3:51 AM

@Niedzielski Thank you. I got things working now. See my updated Gerrit patch (PS4), which depends on PR 151 in the page lib.

I've added an alternate method to convertImagesToPlaceholders which looks for elements with the placeholder class generated on the server side.

Change 445073 merged by jenkins-bot:
[mediawiki/services/mobileapps@master] content-html: add lazy loading of images

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