Doesn't fall back if image fails to load

[designbyadrian]

I can briefly see my imgWrapperComponent being rendered, but if an image src URL doesn't return an image, imgWrapperComponent is removed and never shown again.

There needs to be a fallback function.

[Merri]

Thanks! I need to think more how imgWrapperComponent works; have been a bit unfortunate with having too many other tasks pushing my attention to other things at job, so I never had the time to actually implement it and see the possible issues. After some thinking I guess the major issue right now is that immediately the imgWrapperComponent is taken away before images even start loading, when it might make more sense to remove it only after it's image has fully loaded.

[oyeanuj]

@Merri I'm a little confused about how imgWrapperComponent works and how best to think about possible usecases? Is it possible to add details to that section of the README?

[oyeanuj]

@Merri Looking more into the code, it seems like that imgWrapperComponent is trying to meld two different functionalities - that of ensuring that child images are lazy loaded, and rendering a placeholder until they are loaded.

Is that a correct interpretation?

If yes, would it make it easier to reason to have two different props -

1. lazyLoadChildImages: a boolean that would lazy load the images nested as children.
2. placeholderComponent: this would display a placeholder component until the image has loaded.

(Separating it out would also possibly allow users who have their own placeholder mechanism to do that, and for more cases than just images. Currently, from the example, it wasn't intuitive either that the imgWrapperComponent can be used when not dealing with nested images).

[Merri]

A thought I've had is to separate this logic into a new component entirely as it seems making this feature actually work properly requires adding a lot of new stuff, thus makes it hard to understand what the component has to do in each case. It is a bit dangerous to create super components that can take anything you throw at it :)

[oyeanuj]

@Merri I'm curious what that component would look like - would the behavior change? And do you have any rough idea of the timescale we are talking about here :)

[oyeanuj]

@Merri Another point of feedback about imgWrapperComponent. In the README, you have a note on the performance -

For best performance it is recommended to use imgWrapperComponent and have multiple images inside a single Lazy container as this means only the container's position in viewport is checked for, not the position of every single image component.

This presupposes that one would have the same placeholder for every image. But increasingly, with progressive loading of images, one customizes the placeholder with either the dominant color or a low quality image. If this component was instead a function, being passed a prop for the image it was wrapping, then that would allow custom placeholders per image.

[Merri]

For now I'm prototyping splitting imgWrapperComponent behavior: I split it to imgFallback, imgPlaceholder and imgWrapper.

• imgFallback will be used if image fails to load.
• imgPlaceholder will be rendered before image has loaded or failed to load (this is what imgWrapperComponent mostly represented before)
• imgWrapper will be rendered around image once image has been successfully loaded.

I'm also changing callback function behavior: Lazy component onLoad will be called only after all images have been loaded (when using imgPlaceholder) and there is a new onViewport which works as onLoad worked before, which is a more logical naming.

I'm also splitting stuff to multiple files and consider removing verge dependency and removing prop-types from dist if there is now a working CDN copy of it available.

[Merri]

I changed quite a bit of things on another branch: https://github.com/Merri/react-lazy/tree/proto-img

The placeholder idea is untested atm, and I'm far too tired to work more on it right now (I didn't plan to be up this late/early, but sometimes life happens). Other changes are working: verge dependency removed, prop-types no longer part of UMD module, code split to multiple files and so on.

[Merri]

@oyeanuj Appears I wrote working code, there were no real changes required as I extended the demo to include wrapper/placeholder/fallback :)

[oyeanuj]

@Merri Cool, just trying it out now. Two questions -

1. Is there a way to specify a placeholder image per image in a group?
2. What usecases do you foresee for imgWrapper?

[Merri]

In the demo I used wrapper as the same as placeholder, just having the same container element. I don't see a use case for placeholder per image as most often I imagine the placeholder to be either empty or a loading indicator. Technically the only difference between placeholder and wrapper behavior is that there is an extra span wrapping the image in placeholder, which removes the image from the normal render flow (position absolute trickery) while wrapper is guaranteed to get the direct image element with an image ready to display.

Fallback instead receives all the original image props minus onError and onLoad.

[oyeanuj]

@Merri Maybe I can put forth my use-case for 'placeholder-per-image' - Like you suggested in the README, I put lazyload on the parent of group of images that are loaded together (say, like in a news feed). Now two of the most common progressive loading techniques for images are showing the dominant color of the image as the placeholder background (like Google Images or Pinterest), or show a low quality version of the image blurred (sorta like Medium). In both those cases, the placeholder depends on certain information about the original image - background, or image source.

As I think more about it, I could probably use cushion to ensure that the image component is loaded (with the background before the user ever sees the image area.

Do you see another way to achieve it with react-lazy?

Also, based on your comment above, it seems wrapper is more reliable due to the extra span. So, in that case, would you recommend using placeholder in any case? Maybe the root of the confusion is that they are two different features performing kinda the same tasks?

[Merri]

For color you can use pure CSS: just apply color style to element around the image and then use background-color: currentColor; and you get the color inherited into to the placeholder element. You don't need to pass any props to get the color :)

For the image I guess I could add feature to pass src property to both placeholder and wrapper. The reason I've abstracted them into two is that I don't consider the component as a placeholder once image has been loaded, instead I see it just as a wrapper. Or maybe there is a better name to describe an element that is wrapped around another element.

[oyeanuj]

Fair enough. To your last point, theoretically, one could have the wrapper in their markup anyway and not need to use one via react-lazy?

[Merri]

You won't know whether the image has loaded or not unless you add your own state management (via custom onError and onLoad per each image, for example).

Of course whether this is the best way to go isn't set in stone.

[Merri]

I now separated the logic to Lazy and LazyGroup, and renamed LazyImg as LazyChild as it has now become a generic container. Also updated readme in the branch. It probably still needs some refinement for better clarity.

[Merri]

I released v0.4.0 which deprecates imgWrapperComponent. Open a new issue if you find problems with LazyGroup.