Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Options object doesn't work after migration #79

Closed
damian-balas opened this issue Jul 22, 2020 · 50 comments
Closed

Options object doesn't work after migration #79

damian-balas opened this issue Jul 22, 2020 · 50 comments
Labels
not a bug This issue is not a bug

Comments

@damian-balas
Copy link

damian-balas commented Jul 22, 2020

Options object doesn't work after migration from embla-carousel-react to embla-carousel/react

my config:

const [EmblaCarouselReact, embla] = useEmblaCarousel({
    align: 'start',
    slidesToScroll: 1,
    loop: true,
});

Loop doesn't work.
If emblaCarousel loads for the first time, after dragging I get "maximum call stack size exceeded" error.

image

It doesn't work on embla-carousel-react v2.0.3 either.

I tried to migrate from version: ^1.3.3

@davidjerleke davidjerleke added the investigating Issue is being looked into label Jul 23, 2020
@davidjerleke
Copy link
Owner

davidjerleke commented Jul 23, 2020

Hi Damian (@damian-balas),

Thank you for your question. I'm not sure why you get this error because I can't reproduce it. I just created a CodeSandbox with the same options and it's working just fine.

You haven't provided any of your code so please take a moment to reproduce the error by forking the CodeSandbox above. Alternatively, you can provide your code and the contents of your package.json file so I can debug this further.

Kindly,
David

@damian-balas
Copy link
Author

Ok, I'll try my best :)

@davidjerleke davidjerleke added awaiting response Issue is awaiting feedback and removed investigating Issue is being looked into labels Jul 23, 2020
@damian-balas
Copy link
Author

damian-balas commented Jul 23, 2020

@davidcetinkaya Here is some additional info:

package.json dependencies:

 "dependencies": {
    "@reduxjs/toolkit": "^1.3.6",
    "@testing-library/jest-dom": "^4.2.4",
    "@testing-library/react": "^9.3.2",
    "@testing-library/user-event": "^7.1.2",
    "axios": "0.19.2",
    "date-fns": "^2.14.0",
    "embla-carousel": "^3.0.21",
    "framer-motion": "^1.11.1",
    "i18next": "^19.4.5",
    "i18next-browser-languagedetector": "^4.2.0",
    "i18next-http-backend": "^1.0.13",
    "i18next-localstorage-cache": "^1.1.1",
    "lodash": "^4.17.15",
    "prop-types": "^15.7.2",
    "react": "^16.13.1",
    "react-dom": "^16.13.1",
    "react-i18next": "^11.5.0",
    "react-redux": "^7.2.0",
    "react-router-dom": "^5.2.0",
    "react-scripts": "3.4.1",
    "redux": "^4.0.5",
    "styled-components": "^5.1.0"
  },

EmblaCarouselWrapperComponent:

const TestimonialsSection = styled.section`
  position: relative;
  background-image: linear-gradient(45deg, #53b5be, #3882a8);
`;

EmblaCarouselComponent:

import React, { useEffect } from 'react';
import { useEmblaCarousel } from "embla-carousel/react";
import styled from 'styled-components';
import TestimonialsSlide from 'components/atoms/TestimonialsSlide';
import { maxWidthStyle } from 'constant';

const Testimonials = [
  {
    name: 'John Doe',
    role: 'Student',
    text:
      'Lorem ipsum dolor sit amet consectetur adipisicing elit. Dolorem ea at nulla, fugiat nostrum esse. Lorem ipsum dolor sit amet consectetur adipisicing elit. Fugiat nostrum esse sit amet.',
  },
  {
    name: 'John Travis',
    role: 'Student',
    text:
      'Dolorum nesciunt praesentium nihil doloribus dolorem ea at nulla, fugiat nostrum esse.',
  },
  {
    name: 'Jonas Schmidt',
    role: 'Student',
    text:
      'Lorem ipsum dolor sit amet consectetur adipisicing elit. Dolorum nesciunt praesentium nihil doloribus dolorem ea at nulla, fugiat nostrum esse.',
  },
  {
    name: 'Alice Adams',
    role: 'Student',
    text:
      'Lorem ipsum dolor sit amet consectetur. Dolorum nesciunt praesentium nihil dolorem ea at nulla, fugiat nostrum esse.',
  },
];

const TestimonialsCarousel = () => {
  const [EmblaCarouselReact, embla] = useEmblaCarousel({
    align: 'start',
    slidesToScroll: 1,
    loop: true,
  });

  const scrollNext = () => {
    embla.scrollNext();
  };

  const scrollPrev = () => {
    embla.scrollPrev();
  };

  return (
    <Container>
      <CarouselButton type="button" onClick={scrollPrev}>
        <svg
          xmlns="http://www.w3.org/2000/svg"
          width="25"
          height="16"
          viewBox="0 0 25 16"
        >
          <g
            fill="none"
            fillRule="evenodd"
            stroke="#1d386b"
            strokeLinecap="round"
            strokeLinejoin="round"
            strokeWidth="2"
          >
            <path d="M24 8H1M8 15L1 8l7-7" />
          </g>
        </svg>
      </CarouselButton>
      <CarouselButton type="button" onClick={scrollNext}>
        <svg
          xmlns="http://www.w3.org/2000/svg"
          width="25"
          height="16"
          viewBox="0 0 25 16"
          transform="scale(-1,1)"
        >
          <g
            fill="none"
            fillRule="evenodd"
            stroke="#FFF"
            strokeLinecap="round"
            strokeLinejoin="round"
            strokeWidth="2"
          >
            <path d="M24 8H1M8 15L1 8l7-7" />
          </g>
        </svg>
      </CarouselButton>
      <EmblaCarouselReact>
        <div style={{ display: 'flex' }}>
          {Testimonials.map(({ name, text, role }, index) => (
            <TestimonialsSlide
              key={name}
              name={name}
              text={text}
              role={role}
              index={index}
            />
          ))}
        </div>
      </EmblaCarouselReact>
    </Container>
  );
};

const Container = styled.div`
  transform: translateY(-3rem);
  ${maxWidthStyle};
`;

const CarouselButton = styled.button`
  margin: 0;
  margin-left: 0.8rem;
  height: 3rem;
  width: 3rem;
  border: none;
  background: none;
  font-size: 1.8rem;
  line-height: 2.5rem;
  display: block;
  color: #ffffff;
  text-align: center;
`;

export default TestimonialsCarousel;

And the slide for Carousel:

import React from 'react';
import styled from 'styled-components';
import { breakPoints } from 'constant';

const QUOTE_COLORS = ['#53B5BE', '#47aabc', '#3d9db7', '#398fb0', '#3882a8'];

const TestimonialsSlide = ({ name, text, role, index }) => {
  const t = 'test';

  return (
    <Wrapper>
      <SlideContainer>
        <QuoteIconWrapper>
          <svg
            xmlns="http://www.w3.org/2000/svg"
            xmlnsXlink="http://www.w3.org/1999/xlink"
            version="1.1"
            width="35px"
            height="35px"
            viewBox="0 0 124 124"
            xmlSpace="preserve"
          >
            <g>
              <path
                fill={QUOTE_COLORS[index % QUOTE_COLORS.length]}
                d="M49.8,29.032c3.1-1.3,4.4-5,3-8l-4.9-10.3c-1.4-2.899-4.8-4.2-7.8-2.899c-8.5,3.6-15.8,8.3-21.6,14   C11.4,28.532,6.6,36.232,4,44.732c-2.6,8.601-4,20.3-4,35.2v30.7c0,3.3,2.7,6,6,6h39.3c3.3,0,6-2.7,6-6v-39.3c0-3.301-2.7-6-6-6   H26.5c0.2-10.101,2.6-18.2,7-24.301C37.1,36.133,42.5,32.133,49.8,29.032z"
              />
              <path
                fill={QUOTE_COLORS[index % QUOTE_COLORS.length]}
                d="M120.4,29.032c3.1-1.3,4.399-5,3-8l-4.9-10.199c-1.4-2.9-4.8-4.2-7.8-2.9c-8.4,3.6-15.601,8.3-21.5,13.9   c-7.101,6.8-12,14.5-14.601,23c-2.6,8.399-3.899,20.1-3.899,35.1v30.7c0,3.3,2.7,6,6,6H116c3.3,0,6-2.7,6-6v-39.3   c0-3.301-2.7-6-6-6H97.1c0.2-10.101,2.601-18.2,7-24.301C107.7,36.133,113.1,32.133,120.4,29.032z"
              />
            </g>
          </svg>
        </QuoteIconWrapper>
        <TestimonialText>{text}</TestimonialText>
        <TestimonialAuthor>{name}</TestimonialAuthor>
        {role && <TestimonialRole>{role}</TestimonialRole>}
      </SlideContainer>
    </Wrapper>
  );
};

const QuoteIconWrapper = styled.div`
  opacity: 0.7;
`;

const Wrapper = styled.div`
  position: relative;
  width: 100%;
  flex: 0 0 100%;
  @media (min-width: ${breakPoints.beforeTablet}) {
    flex: 0 0 50%;
  }

  @media (min-width: 68em) {
    flex: 0 0 33.33%;
  }
`;

const SlideContainer = styled.div`
  position: relative;
  background-color: #ffffff;
  margin: 1.5rem;
  padding: 1rem;
  border-radius: 15px;
  box-shadow: 0 16px 24px -16px rgba(59, 142, 184, 0.25);
`;

const TestimonialText = styled.p`
  color: ${({ theme }) => theme.txtColor};
  font-size: 1rem;
  line-height: 1.8;
  font-style: italic;

  ::after {
    content: '';
    height: 3px;
    width: 70%;
    border-radius: 3px;
    display: block;
    margin: 1rem auto;
    background-color: #f0f0f0;
  }
`;

const TestimonialAuthor = styled.h4`
  font-size: 1.2rem;
  line-height: 1.2rem;
  color: ${({ theme }) => theme.secondary};
  text-align: center;
`;

const TestimonialRole = styled.p`
  font-size: 1rem;
  color: ${({ theme }) => theme.txtColor};
  text-align: center;
  margin: 0;
`;

export default TestimonialsSlide;

I'm using framer-motion and react-router for routing with animation.

I hope this helps.

Thanks :)

@damian-balas
Copy link
Author

I have on route "/" my first emblaCarousel and on /some-route my second emblaCarousel

If I am on /some-route and refresh the page, then I get this maximum stack size exceeded error

If I am on "/" the emblaCarousel doesn't loop. If I go from "/" to "/some-route" I don't get maximum stack size exceeded error and the emblaCarousel works fine. (on "/" it's still broken)

@damian-balas
Copy link
Author

Just found out something.

If I change

const Wrapper = styled.div`
  position: relative;
  width: 100%;
  flex: 0 0 100%;
  @media (min-width: ${breakPoints.beforeTablet}) {
    flex: 0 0 50%;
  }

  @media (min-width: 68em) {
    flex: 0 0 33.33%;
  }
`; 

to:

const Wrapper = styled.div`
  position: relative;
  flex: 0 0 100%;
`; 

It works fine. I can also do flex: 0 0 50%. But I CAN'T go below 50%.

" maximum stack size exceeded error " still not fixed :/ I don't know what could be the problem there.

@davidjerleke
Copy link
Owner

davidjerleke commented Jul 23, 2020

Hi again Damian (@damian-balas),

Thanks for the additional information. I will take a look at the provided details and get back to you when I’ve done so.

In the meantime, please take a look at this issue that one of my users submitted some time ago. Seems like framer-motion page transitions can cause some weird and unexpected behavior. You can try the solution provided in that issue. Give it a go and let me know if it helps.

Best,
David

@damian-balas
Copy link
Author

I've already implemented exitBeforeEnter prop. 😞

@davidjerleke davidjerleke added investigating Issue is being looked into and removed awaiting response Issue is awaiting feedback labels Jul 23, 2020
@damian-balas
Copy link
Author

damian-balas commented Jul 23, 2020

MORE INFO

I found out that when you have only 4 slides and try to display 3 at the same time (width: 33.33% / flex: 0 0 33.33%) EmblaCarousel breaks. If I add more slides EmblaCarousel starts working again. I guess there are some issues with loop functionality when there are only few slides.

@davidjerleke
Copy link
Owner

davidjerleke commented Jul 23, 2020

Thanks for digging further and narrowing it down @damian-balas!

I think I know why this is happening now. I’m guessing that only the React version of Embla suffers from this. When slides aren’t enough to loop Embla will pick that up and automatically fall back to loop false by re-initializing itself.
React seems to “react” when this happens and re-renders Embla, causing and infinite call stack.

I will look into this and get back to you when I’ve done that.

Kindly,
David

@damian-balas
Copy link
Author

Awesome! I really want to use this package, because it's the smoothest one :)

@davidjerleke
Copy link
Owner

davidjerleke commented Jul 23, 2020

@damian-balas seems like my guess wasn't correct.

The fallback to loop: false when slides aren't enough to make the loop effect works for the react version as well. If I use 4 slides with flex: 0 0 33.33% like you mentioned here it still works as expected. Please take a look at this CodeSandbox.

I think you should try to toggle components on and off to see if you can find what's causing this. If it suddenly starts working when you've deactivated a component you can narrow it down. I would start with framer-motion and its page transitions. Because without access to your repo it's hard to debug further when I don't have the whole picture.

Let me know if you find anything.

Kindly,
David

@damian-balas
Copy link
Author

damian-balas commented Jul 23, 2020

@davidcetinkaya the CodeSandbox example doesn't work for me.

image

Even the prev button is disabled

image

@davidjerleke
Copy link
Owner

@damian-balas what do you mean by doesn't work? Can you drag the carousel?

@damian-balas
Copy link
Author

damian-balas commented Jul 23, 2020

I can drag it, but it doesn't loop. As you can see in the first screenshot - there are blank rectangles. I can't drag it further.

My OS - Ubuntu 18.04
Browser - Google Chrome 83.0.4103.116

@davidjerleke
Copy link
Owner

I see. That's because Embla realizes that the slides are not enough for the loop effect to work. So it changes loop: true to loop: false for you automatically.

@damian-balas
Copy link
Author

Then why does it work on your machine? 😕

@davidjerleke
Copy link
Owner

How do you mean 😕? The CodeSandbox is working fine?

@damian-balas
Copy link
Author

No, the CodeSandbox doesn't work. You said:

If I use 4 slides with flex: 0 0 33.33% like you mentioned here it still works as expected.

If I understood it correctly, it works for you, right?

@damian-balas
Copy link
Author

UPDATE

I have removed framer-motion. I still get Maximum stack size exceeded error 😕

@davidjerleke
Copy link
Owner

I can drag it, but it doesn't loop. As you can see in the first screenshot - there are blank rectangles. I can't drag it further.
I found out that when you have only 4 slides and try to display 3 at the same time (width: 33.33% / flex: 0 0 33.33%) EmblaCarousel breaks. If I add more slides EmblaCarousel starts working again. I guess there are some issues with loop functionality when there are only few slides.

This is expected behavior when you don't provide enough slides with enough width for the loop effect to work. There has to be enough slides to cover the gap needed when flipping slide positions, otherwise the carousel would flicker when looping.

@damian-balas
Copy link
Author

damian-balas commented Jul 23, 2020

Ok, so 1 "bug" is solved. I'll just make it 50% instead of 33.33%. It would be nice if you could give a note about this in the docs. So anyone having this issue will know what's going on.

I still have no idea what could cause the maximum stack size exceeded error.

Could this possibly help you out?

image

@davidjerleke
Copy link
Owner

It would be nice if you could give a note about this in the docs.

It's actually stated in the docs 🙂. Take a look here:

option-loop

Unfortunately, I can't see anything helpful from the screenshot 😕.

@davidjerleke
Copy link
Owner

@damian-balas does the previous and next buttons work for you?

@damian-balas
Copy link
Author

You're right, my bad! I didn't see it :)

It would be nice if you could give a note about this in the docs.

It's actually stated in the docs . Take a look here:

option-loop

Unfortunately, I can't see anything helpful from the screenshot .

@damian-balas
Copy link
Author

@damian-balas does the previous and next buttons work for you?

No 😕

@davidjerleke
Copy link
Owner

davidjerleke commented Jul 23, 2020

@damian-balas so the previous and next buttons also trigger the maximum call stack issue?

@damian-balas
Copy link
Author

No, they don't do anything.

@damian-balas
Copy link
Author

Is there a way to programmatically rerender/reinitialize the carousel?

@davidjerleke
Copy link
Owner

davidjerleke commented Jul 23, 2020

Is there a way to programmatically rerender/reinitialize the carousel?

Yes, like so:

embla.reInit({})

@damian-balas
Copy link
Author

Ok, I'll try something and be back in a minute or so :)

@damian-balas
Copy link
Author

damian-balas commented Jul 23, 2020

  useEffect(()=>{
    if(embla) {
      setTimeout(()=>{
        console.log('reinit');
        embla.reInit({
          loop: false,
          align: "start",
          slidesToScroll: 1,
        });
      },3000)
    }
  },[embla])

If I wait then for 3 seconds, I don't get this error.

Maybe there is a problem like the counting failed because Embla didn't see any "slides" and it's stuck then?

@davidjerleke
Copy link
Owner

Do you get the error if you do it like this?

useEffect(() => {
  if (embla) {
    embla.reInit({
      loop: false,
      align: 'start',
      slidesToScroll: 1,
    })
  }
}, [embla])

Maybe there is a problem like the counting failed because Embla didn't see any "slides" and it's stuck then?

This could be the case. Maybe there's no children there when you initialize embla for the first time.

@damian-balas
Copy link
Author

If I do it like this, the carousel works fine, but it's a nasty fix IMO 😕

There should be children, because my 'test code' looks like this:

      <EmblaCarouselReact>
        <div style={{ display: "flex" }}>
          <div style={{minWidth: '100%', minHeight: '300px', position: 'relative', backgroundColor: '#1ff'}}>1254436313</div>
          <div style={{minWidth: '100%', minHeight: '300px', position: 'relative', backgroundColor: '#f2f'}}>12132132133213123</div>
          <div style={{minWidth: '100%', minHeight: '300px', position: 'relative', backgroundColor: '#ff3'}}>1213213213</div>
          <div style={{minWidth: '100%', minHeight: '300px', position: 'relative', backgroundColor: '#125'}}>1213123</div>
        </div>
      </EmblaCarouselReact>

@damian-balas

This comment has been minimized.

@damian-balas
Copy link
Author

SOLUTION

I found the problem!
It's because of i18n, which uses react suspense. :) I think I can fix it on my own now!

@davidjerleke
Copy link
Owner

Nice digging @damian-balas 😃! Let me know how it goes. I hope that this will solve it so you can enjoy the power of Embla Carousel.

@damian-balas
Copy link
Author

Ok, it works!
Thanks for your time!
I appreciate it.

You can put this information in the docs - "Might not work with react suspense".

@davidjerleke davidjerleke added not a bug This issue is not a bug and removed investigating Issue is being looked into labels Jul 24, 2020
@davidjerleke
Copy link
Owner

davidjerleke commented Jul 25, 2020

No worries. I’m glad you got it sorted @damian-balas 🙂.

You can put this information in the docs - "Might not work with react suspense".

I think a better approach would be to try to reproduce the issue in a CodeSandbox that uses React suspense, so I can understand why this is happening and try to fix this. That would be a nice addition.

Thank you for reporting this.

Kindly
David

@davidjerleke
Copy link
Owner

davidjerleke commented Jul 25, 2020

@damian-balas, I just realized something. If you want the loop effect to work with 4 slides when each slide is 1/3 wide, you have to use the following css:

.slide {
  flex: 0 0 33.33333%;
}

/* ...instead of */

.slide {
  flex: 0 0 33.33%;
}

This is because the carousel viewport is 100%, and 33.33% x 3 slides = 99.99%, which is why Embla falls back to loop false. So if you use 33.33333% x 3 slides = 100% because the browser will round it to 100%. Alternatively, you can use the css calc function to divide 100% / 3.

Let me know if it helps!

Best,
David

@damian-balas
Copy link
Author

damian-balas commented Jul 26, 2020

@damian-balas, I just realized something. If you want the loop effect to work with 4 slides when each slide is 1/3 wide, you have to use the following css:

.slide {
  flex: 0 0 33.33333%;
}

/* ...instead of */

.slide {
  flex: 0 0 33.33%;
}

This is because the carousel viewport is 100%, and 33.33% x 3 slides = 99.99%, which is why Embla falls back to loop false. So if you use 33.33333% x 3 slides = 100% because the browser will round it to 100%. Alternatively, you can use the css calc function to divide 100% / 3.

Let me know if it helps!

Best,
David

Wow thanks! :) I had to do 33.35% to make it work.

@davidjerleke
Copy link
Owner

Hello again Damian (@damian-balas),

I've recently released a patch fix with Embla version 3.0.23. If you don't mind you can try if react suspense still trigger the error you initially mentioned in this issue.

Kindly,
David

@azeroth1993
Copy link

Hello,

First of all, thank you very much for this amazing, slick carousel. Great work 🙂

I've recently encountered a very similar problem, so i thought it would be good to report it here.
I use Embla with react and here is the problem:

Imagine there are more than one instances of Embla in one page and they are inside a wrapper element, like a <div></div>.
The parent div has a css "display: none" at the page load. it's like a 'more info' box.
And there's a button to open the info box.
The button changes the css from "display:none" to "display:block", so that the box shows up.

Now after opening the box, if you start dragging or interacting with the carousel, The "maximum call stack size exceeded" Error appears.

It seems that if the Embla instance is not "visible/rendered" or at least is hidden with "display:none" at the page initial load, the error shows up.

The solution i use until now is to hide the carousel by setting the wrapper elements height and max-height to "0" instead.

It's not a big problem, just thought i'd mention it here if anybody has the same problem.

Good luck and regards

@davidjerleke
Copy link
Owner

Hi @azeroth1993,

Thank you. What version of Embla Carousel are you using?

Beat,
David

@azeroth1993
Copy link

azeroth1993 commented May 12, 2021 via email

@davidjerleke
Copy link
Owner

davidjerleke commented May 12, 2021

Hi @azeroth1993,

The issue you describe got some attention in #151 and a fix was released with v4.2.0. But I would recommend you to update to the latest version.

Let me know how it goes.

Best,
David

@davidjerleke
Copy link
Owner

@azeroth1993 have you had the change to read my response yet?

@azeroth1993
Copy link

azeroth1993 commented May 14, 2021 via email

@davidjerleke
Copy link
Owner

davidjerleke commented May 14, 2021

@azeroth1993 no worries, I was just curious.

Please note that you still have to run re-init when removing display: none. This is because it's impossible for Embla to pick up the correct container and slide dimensions when any parent has display: none, because these dimensions doesn't exist until you display the element.

Best,
David

@azeroth1993
Copy link

azeroth1993 commented May 14, 2021 via email

@davidjerleke
Copy link
Owner

davidjerleke commented May 14, 2021

Nice to see a rtl implementation of Embla Carousel 👍. Thank you for sharing the link.

Try updating to the latest version and run embla.reInit() when removing display: none and the issue should go away. Let me know how it goes.

Best,
David

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not a bug This issue is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants