Lightbox images resize carousel isn't working properly in React.js

[shurns]

Hi everyone,

I’m creating a lightbox image gallery using React. On the lightbox, which pops and displays the image, I’m minimizing images so they could fit properly on the screen. I created an image resize function to fit a large image onto the screen, which I set the image width and height state as it’s value.

When I use console.log(this.imageResize(width, height).width), it’s correct whenever I click both the left and right arrows to navigate to the next/previous images. But when I pass the jsx image element’s width’s value as the image width’s state, it’s a different result as if the previous state looks to be a problem here.

Thanks

[shurns]

Here’s the code:

import React from 'react';
import PropTypes from 'prop-types';
import '../styles/components/Thumbnail_Gallery.scss';

class ThumbnailGallery extends React.Component {
  constructor(props) {
    super(props);

    this.state = {
      index: null,
      isOpen: false,
      imageLength: null,
      lightboxImages: props.lightboxImages,
      loopImages: false,
      lightboxImageDimensions: {
        width: null,
        height: null
      }
    };
  }

  componentWillMount() {
    document.addEventListener('click', this.handleGlobalClick);

    if (this.props.loopImages) {
      this.setState({
        loopImages: true
      });
    }
  }

  componentWillUnmount() {
    document.removeEventListener('click', this.handleGlobalClick);
  }
  
  openImage = ({ target, currentTarget: { dataset: { imageIndex } } }) => {
    const { isOpen, lightboxImages } = this.state;

    if (!isOpen) {
      this.setState({
        isOpen: true,
        index: parseInt(imageIndex),
        imageLength: lightboxImages.length
      });
    }
  };

  closeImage = () => {
    this.setState({
      isOpen: false,
      lightboxImageDimensions: {
        width: null,
        height: null
      }
    });
  };

  nextImage = () => {
    const { index, imageLength } = this.state;
    
    this.setState({
      index: (index + 1) % imageLength
    });
  };

  prevImage = () => {
    const { index, imageLength } = this.state;
    const lastImage = imageLength - 1;

    //Set state to last image to loop the images
    if (index < 1) {
      this.setState({
        index: lastImage
      });
    } else {
      this.setState({
        index: index - 1
      });
    }
  };

  isTargetClassName = (targetElement, list) => {
    let isTrue = false;

    list.forEach(item => {
      if (targetElement.className === item) {
        isTrue = true;
      }
    });

    return isTrue;
  };

  handleGlobalClick = ({ target }) => {
    const { isOpen, index } = this.state;
    const { isTargetClassName, closeImage } = this;
    const classNames = [
      'lightbox-img',
      'icon-keyboard_arrow_left',
      'icon-keyboard_arrow_right'
    ];

    /**
     * Close the displayed image if the click doesn't match
     * any of the target className elements
     */

    if (isOpen && !isTargetClassName(target, classNames)) {
      closeImage();
    }
  };

  onLeftArrowClick = () => {
    const { isOpen } = this.state;

    if (isOpen) {
      this.prevImage();
    }
  };

  onRightArrowClick = () => {
    const { isOpen } = this.state;

    if (isOpen) {
      this.nextImage();
    }
  };

  imageResize = ( imageWidth, imageHeight ) => {
    const w = window;
    let screenWidth = w.innerWidth / 1.2;
    let screenHeight = w.innerHeight / 1.2;
    let ratio = 0;

    if(imageWidth > screenWidth || imageHeight > screenHeight) {
      ratio = Math.min(screenWidth / imageWidth, screenHeight / imageHeight);
      return { width: Math.floor(imageWidth * ratio) }
    }
    return imageWidth;
  };

  onLightboxImageLoad = ({ target }) => {
    const adjustImage = this.imageResize(target.width, target.height);
    this.setState({
      lightboxImageDimensions: {
        width: adjustImage.width
      }
    });

    console.log(this.state.lightboxImageDimensions.width);
  };

  matchIndex = ( currentIndex, imageIndex ) => currentIndex === imageIndex;

  render() {
    const { lightboxImagesSrc, 
            lightboxImages, 
            thumbnailImagesSrc, 
            thumbnailImages } = this.props;

    const { isOpen, 
            index, 
            imageLength, 
            loopImages,
            lightboxImageDimensions: { width, height } } = this.state;

    const Arrow = ({ arrowClass, onArrowClick }) => (
      <i 
        className={arrowClass} 
        onClick={onArrowClick} 
      />
    )

    const LeftArrow = ({ onArrowLeftClick }) => (
      <Arrow arrowClass="icon-keyboard_arrow_left" 
        onArrowClick={onArrowLeftClick} 
      />
    )
    const RightArrow = ({ onArrowRightClick }) => (
      <Arrow 
        arrowClass="icon-keyboard_arrow_right" 
        onArrowClick={onArrowRightClick} 
      />
    )

    /**
     * Use conditional rendering with HOC to determine if the 
     * current index does not match the first and last index
     */
    
    const currentIndexMatchFirstIndex = Component => props => (
      !this.matchIndex(index, 0) && <Component {...props} />
    );

    const currentIndexMatchLastIndex = Component => props => (
      !this.matchIndex((index + 1), imageLength) && <Component {...props} />
    );
    
    const ExtendLeftArrow = !loopImages ? currentIndexMatchFirstIndex(LeftArrow) : LeftArrow;
    const ExtendRightArrow = !loopImages ? currentIndexMatchLastIndex(RightArrow) : RightArrow;

    return (
      <div className="thumbnail-container">
        <div className="grid">
          {thumbnailImages.map((list, index, array) => (
            <div
              className="cell"
              onClick={this.openImage}
              key={index}
              data-image-index={index}
            >
              <img
                src={`${thumbnailImagesSrc}${list}`}
                className="responsive-img image"
              />
            </div>
          ))}
        </div>
        {isOpen && (
        <div className="lightbox-container">
          <div className="lightbox-img-container">
          <img src={`${this.props.lightboxImagesSrc}${this.props.lightboxImages[index]}`}
            className="lightbox-img"
            onLoad={this.onLightboxImageLoad}
            width={width}
             />
          </div>
          <div className="navigation-container">
            <ExtendLeftArrow onArrowLeftClick={this.onLeftArrowClick}/>
            <ExtendRightArrow onArrowRightClick={this.onRightArrowClick}/>
          </div>
          <div className="lightbox-overlay" />
        </div>
        )}
      </div>
    );
  }
}

export default ThumbnailGallery;

[m3g4p0p]

From looking at your code it should actually set the state correctly; however state updates are happening ansynchronously… it does not alter the state right away, but just tells react to update when appropriate – see here for an in depth explanation. Hence, if you immediately log the state you’ll still get the old one; but you can pass a callback that gets called after the state got actually updated:

this.setState({
  lightboxImageDimensions: {
    width: adjustImage.width
  }
}, () => console.log(this.state.lightboxImageDimensions.width))

BTW, it’s very inefficient to declare your Arrow components anew each time inside the render function… better put these next to the Thumbnail component (or even in a dedicated module).

[shurns]

Hi m3g4p0p,

Thanks for helping me out. I’ve been trying to solve this issue for days. As you’ve said, the setState behaves asynchronously. Which could be why the width will return undefined after a couple of state changes. Unfortunately, the callback doesn’t seem to help solve the issue, either it seems. Maybe there need to be some kind of logic to reset the state in the callback after it’s update? Or would there be another way to solve this?

I’m going to refactor my code and also move the Arrow Components outside of the render function and into a separate file for better inefficiency. Thanks for pointing that out.

[m3g4p0p]

I found another issue with your code; in the imageResize() method, you sometimes return an object, sometimes a number primitive. Also, are you sure you want to take the target.width and target.height as a basis for your calculations? These would refer to the width and height attributes of the image, which are the attributes you are actually trying to set… what you want is probably the target.naturalWidth and target.naturalHeight properties, respectively. I put together a simplified version of your component with just the relevant bits that seems to be doing what you want:

import React, { Component } from 'react'

class ThumbnailGallery extends Component {
  constructor (props) {
    super(props)

    this.state = {
      isOpen: false,
      currentImage: '',
      imageDimensions: { width: 0 }
    }
  }

  openImage = ({ target }) => {
    const { images } = this.props

    this.setState({
      isOpen: true,
      currentImage: images[target.dataset.index]
    })
  }

  imageResize = ({
    naturalWidth: imageWidth,
    naturalWidth: imageHeight
  }) => {
    const w = window
    let screenWidth = w.innerWidth / 1.2
    let screenHeight = w.innerHeight / 1.2
    let ratio = 0

    if (imageWidth > screenWidth || imageHeight > screenHeight) {
      ratio = Math.min(screenWidth / imageWidth, screenHeight / imageHeight)

      return {
        width: Math.floor(imageWidth * ratio)
      }
    }

    return { width: imageWidth }
  }

  onImageLoad = ({ target }) => {
    const imageDimensions = this.imageResize(target)

    this.setState({ imageDimensions })
  }

  render () {
    const { thumbnails } = this.props
    const {
      isOpen,
      currentImage,
      imageDimensions: { width }
    } = this.state

    return (
      <div>
        {thumbnails.map((thumbnail, index) => (
          <img
            alt=''
            src={thumbnail}
            key={index}
            data-index={index}
            onClick={this.openImage}
          />
        ))}
        {isOpen && <img
          alt=''
          src={currentImage}
          width={width}
          onLoad={this.onImageLoad}
        />}
      </div>
    )
  }
}

class App extends Component {
  render() {
    return (
      <ThumbnailGallery thumbnails={[
        'http://lorempixel.com/400/200/sports/1',
        'http://lorempixel.com/400/200/sports/2',
        'http://lorempixel.com/400/200/sports/3'
      ]} images={[
        'http://lorempixel.com/800/400/sports/1',
        'http://lorempixel.com/800/400/sports/2',
        'http://lorempixel.com/800/400/sports/3'
      ]} />
    )
  }
}

But then again, why use JS for this in the first place… couldn’t you simply set a max-width: 80vw or something in the CSS? Otherwise you’d also have to listen to browser resize and orientationchange events, not to mention the additional overhead…

[shurns]

@m3g4p0p

I found another issue with your code; in the imageResize() method, you sometimes return an object, sometimes a number primitive.[/quote]

Apologies. That was my mistake. I never return an object and a different primitive type from a function. I was fiddling around with it, trying to discover on why it wasn’t behaving properly the way I wanted it to. My original function outputs an object with the same property.

[quote]But then again, why use JS for this in the first place… couldn’t you simply set a max-width: 80vw or something in the CSS? Otherwise you’d also have to listen to browser resize and orientationchange events, not to mention the additional overhead…

The images didn’t display properly in css at first, so I decided to write it in JavaScript instead. I knew if I got the imageSize() and state changes working properly, I would have to create a handler for a resize event. You’re definitely right about that.

I always preferred to implement something like this in css, but I never used vw/vh type values. I didn’t know much about them. I thought it was only used for font sizing based on the viewport’s width and height. I tried it out and it works perfectly! For days, I couldn’t figure this issue and now it’s solved. I should’ve post this question on the same day or the next day that I was having issues with this code. I shouldn’t have waited so long

Thanks for taking the time in writing your code. I’m going to look at it so I could improve my skills to be better.

Thanks for the other tips! May I ask, why is it appropriate to not create any functional stateless components in the render function? I’m assuming it has a lot to do with performance and other unexpected behaviors. Is there a article out there you can link me to? I always want to be a better React Developer and it’s been a while since anyone reviewed my code.

Edit: Now I see why it’s very important not to create any functional components in the render function. When I place the functional stateless components outside of the render function, it doesn’t cause any unnecessary re-render whenever the state changes.