Layout Shift Occurring When Using loader in Img component.

sabeerbikba commented 1 month ago

When using the loader prop in the Img component, I encountered a significant layout shift. This issue arises because the loader element is removed and replaced with the actual image once it's loaded, leading to a shift in the layout. This is noticeable when the network speed is throttled (between 800kbps to 500kbps).

To reproduce:

Use the Img component with a loader prop.
Simulate a slow network in the browser's Network tab (set between 800kbps to 500kbps).
Notice the layout shift when the image replaces the loader.

I attempted to mitigate this issue by wrapping the loader with a fixed container to maintain the layout size.

Code :

 <div style={{
   backgroundImage: `url(${bgImage})`,
   backgroundSize: 'cover',
   width: '350px',
   boxShadow: `1px 1px 5px ${boxShadowColor}`,
    aspectRatio: `${width} / ${height}`,
}}>
     <Img
         src={imageSources}
          // decode={true} // also tried 
          crossorigin="anonymous"
          style={{
             backgroundImage: `url(${bgImage})`,
             backgroundSize: 'cover',
             aspectRatio: `${width} / ${height}`,
          }}
          // loader={
          //    <div
          //       className='image-item'
          //       style={{
          //          // object-contain
          //          backgroundImage: `url(${bgImage})`,
          //          backgroundSize: 'cover',
          //          boxShadow: `1px 1px 5px ${boxShadowColor}`,
          //          width: '350px',
          //          aspectRatio: `${width} / ${height}`,
          //       }}
          //    />
          // }
        unloader={<img src={FALLBACK_IMG} />}
        onLoad={handleImageLoad}
        alt={alt}
     />
 </div>

Expected behavior: The layout should not shift when the image replaces the loader, and both elements should maintain the same dimensions.

Let me know if additional details or further steps are needed to investigate this issue.

mbrevda commented 1 month ago

Assuming the loader and image have identical dimensions, how would you propose swapping them such as to avoid triggering a layout shift?

sabeerbikba commented 1 month ago

Thanks for your response! I believe I found a solution to prevent the layout shift by wrapping both the loader and the actual image in a parent <div> that preserves the dimensions based on the loader's styles. This way, we can avoid any unexpected layout changes when the image replaces the loader.

My Approach:

Maintaining Layout Consistency: I wrapped both the loader and the image inside a parent <div>. By extracting the loader's width and height (if present), the parent container maintains the layout, even when the loader is replaced by the image.

Here's an example of how I handled this:
```
const Img = ({ loader = null }) => {
  const isUsingLoader = loader != null;
  const loaderWidth = isUsingLoader ? loader?.props?.style?.width : '';
  const loaderHeight = isUsingLoader ? loader?.props?.style?.height : '';

  return (
     <div style={{ width: loaderWidth, height: loaderHeight }}>
        {isUsingLoader && loader}
        <img src="" alt="" />
     </div>
  );
};
```
This ensures that the layout stays fixed based on the loader's dimensions, which helps in avoiding the layout shift.

Caveats:

This method works when users explicitly define width and height styles for the loader. However, if users style the loader using class names or IDs, the layout shift issue may still arise.

Improved Handling of Other Styles: To address this, I extended the approach by capturing any class names, IDs, and styles applied to the loader, and applying them to the parent <div>. This ensures that all styles are retained while preventing unwanted layout changes. For instance:

const Img = ({ loader = null }) => {
  const isUsingLoader = loader != null;
  const loaderClassNames = isUsingLoader ? loader?.props?.className : '';
  const loaderIds = isUsingLoader ? loader?.props.id : '';
  const loaderStyles = isUsingLoader ? loader?.props?.style : {};

  return (
     <div className={loaderClassNames} id={loaderIds} style={loaderStyles}>
        {isUsingLoader && loader}
        <img src="" alt="" />
     </div>
  );
};

Potential Issues:

Applying all styles from the loader directly to the parent might lead to visual effects, such as border duplication if both the loader and parent have borders.

Solution for Styling Conflicts:

To address these potential styling conflicts, I selectively applied styles to the parent container, ignoring properties like border, margin, and padding, which could affect the appearance of the parent container. For example:

   const Img = ({ loader = null }) => {
      const isUsingLoader = loader != null;
      const loaderClassNames = isUsingLoader ? loader?.props?.className : '';
      const loaderIds = isUsingLoader ? loader?.props.id : '';
      const loaderStyles = isUsingLoader ? loader?.props?.style : {};

      const parentStyles = {
         ...loaderStyles,
         border: '0',
         position: 'static',
         margin: '0',
         padding: '0',
         // Other styles that shouldn't affect the parent
      };

      return (
         <div className={loaderClassNames} id={loaderIds} style={parentStyles}>
            {isUsingLoader && loader}
            <img src="" alt="" />
         </div>
      );
   };

Conclusion:

By wrapping the loader and image inside a parent container and carefully applying styles, this solution ensures that the layout remains consistent and prevents shifts when replacing the loader with the actual image. This approach works across various styling methods (inline styles, class names, and IDs), and I’ve handled edge cases such as duplicate border effects by filtering specific styles.

Important: This approach works when the user provides a loader. If no loader is used, the user must explicitly define the height and width for the image via props to avoid layout shifts.

Let me know your thoughts!

mbrevda commented 1 month ago

a solution to prevent the layout shift by wrapping both the loader and the actual image in a parent
that preserves the dimensions based on the loader's styles

I am afraid that a sane way of tackling this is beyond the scope of this library, especially seeing how width/height are optional. Open to other suggestions!

mbrevda / react-image