brainhubeu / react-carousel

A pure extendable React carousel, powered by Brainhub (craftsmen who ❤️ JS)
https://brainhub.eu/
MIT License
1.07k stars 164 forks source link

"offset" prop does not work on "breakpoints" #597

Closed hauph closed 4 years ago

hauph commented 4 years ago

Describe the bug "offset" prop does not work on "breakpoints"

To Reproduce Steps to reproduce the behavior:

  1. Go to https://brainhubeu.github.io/react-carousel/docs/examples/responsive
  2. On LIVE JSX EDITOR, add offset: 10 inside 900 object
    900: {
      slidesPerPage: 2,
      arrows: false,
      offset: 10
    }
  3. Scale down the screen width below 900px, props slidesPerPage and arrows are applied, but offset is not

Expected behavior According to offset: 10, there should be padding or margin 10px between items when reaching screen width 900px

Environment Chrome, Window 10

piotr-s-brainhub commented 4 years ago

@hauph

Thanks for reporting this. Unfortunately, it seems we have many problems with the offset.

Anyway, I invite you to open a PR if you're able to fix this.

hauph commented 4 years ago

I have not checked your source code, but anyway I was able to make this work with the following trick:

export default class AppCont extends React.Component { 
    constructor(props: Props) {
        super(props)

        this.state = {
            slideVal: 0,
            width: null, // This 'width' state will be updated every time window is resized 
        }
    }

    // Function to update 'width' state
    updateDimensions = () => {
        this.setState({ width: window.innerWidth });
    };

    // Attach 'resize' event
    componentDidMount() {
        window.addEventListener('resize', this.updateDimensions);
    }

    // Detach 'resize' event
    componentWillUnmount() {
        window.removeEventListener('resize', this.updateDimensions);
    }

    onChangeCarousel(value) {
        this.setState({slideVal: value})

        // Rewind carousel when it reaches last element
        if (value == 2) {
            this.setState({ slideVal: 0});
        }
    }

    render() {
        const { slideVal, width } = this.state;

        // Add breakpoint's width, slidesPerPage and offset to an array to easily manipulate them
        const breakpointArr = [
            {width: 1024, slidesPerPage: 4, offset: 15},
            {width: 768, slidesPerPage: 3, offset: 10},
            {width: 480, slidesPerPage: 2, offset: 5}
        ] 

        let breakpoints = {};
        let offset = 20; // Default offset
        for (let i = 0; i <= breakpointArr.length - 1; i++) {
            // Create 'breakpoints' object
            breakpoints[breakpointArr[i].width] = {
                slidesPerPage: breakpointArr[i].slidesPerPage,
            }

            // Update 'offset' based on current width
            if (width != null && width <= breakpointArr[i].width) {
                offset = breakpointArr[i].offset;
            }
        }

        return <Carousel 
                    slidesPerPage={5} // Default 'slidesPerPage'
                    offset={offset}
                    arrows={true}
                    draggable={true}
                    animationSpeed={2000}
                    lazyLoad={true}
                    value={slideVal}
                    onChange={(e) => {this.onChangeCarousel(e)}}
                    breakpoints={breakpoints}
                >
                    <img src={imageOne} />
                    <img src={imageTwo} />
                    <img src={imageThree} />
                </Carousel>
    }
}

P/S: I am using version 1.19.16

RobertHebel commented 4 years ago

Thanks for posting this workaround. In the upcoming days, I will try to create a fix for that issue

mxdi9i7 commented 4 years ago

@RobertHebel Hello, I'm also having trouble with offset, has there been an update on this issue?

RobertHebel commented 4 years ago

@mxdi9i7 I will work on fixing this today

RobertHebel commented 4 years ago

@mxdi9i7 @hauph The issue should be fixed in v1.19.25. Does it work for you?

RobertHebel commented 4 years ago

@mxdi9i7 @hauph I'm closing this issue as I can't reproduce this bug anymore. If the problem will reoccur please write a message in this thread