Does not support Gatsby Image <Img /> Jsx Element

RamiMustaklem commented 4 years ago

Describe the bug The documentation says the Carousel item has to be a jsx element, and Gatsby Image component is a jsx element but errors out when putting inside the Carousel items.

To Reproduce Steps to reproduce the behavior:

Import the Carousel inside a Gatsby page or component
Import Gatsby Image and use it inside the Carousel
See error

Expected behavior Gatsby Image component is a Jsx element that should work inside the Carousel.

Screenshots error 1 error 2

Environment

Browser [Chrome]

piotr-s-brainhub commented 4 years ago

@RamiMustaklem

Do you use the latest react-carousel?

RamiMustaklem commented 4 years ago

@piotr-s-brainhub My installed version is "@brainhubeu/react-carousel": "^1.19.14",. I also took the master, same thing.

Edit: If you put a div, it doesn't render it, also if you put Gatsby <Img /> component it shows this error. It only takes a normal HTML <img> tag.

piotr-s-brainhub commented 4 years ago

@RamiMustaklem

in the docs, div works inside the carousel:

<Carousel
  slidesPerPage={2}
  arrows
  infinite
>
<div>foo-1</div>
<div>foo-2</div>
<div>foo-3</div>
</Carousel>

RamiMustaklem commented 4 years ago

@piotr-s-brainhub Yes, It does render a div with text inside of it, but if you wrap the Gatsby Image component inside a div inside the <Carousel></Carousel> component, it technically renders them, But, it doesn't give the .BrainhubCarousel__container and every element inside it a height. So the Carousel is collapsed to height of Zero.

piotr-s-brainhub commented 4 years ago

@RamiMustaklem

this also works:

<Carousel
  slidesPerPage={2}
  arrows
  infinite
>
  <div><img src={imageOne} /></div>
  <div><img src={imageTwo} /></div>
  <div><img src={imageThree} /></div>
</Carousel>

so could you provide an example code that doesn't work?

RamiMustaklem commented 4 years ago

@piotr-s-brainhub

Yeah I could, and I'm sorry but the title says Gatsby Image.

import Img from 'gatsby-image';

<Carousel
  slidesPerPage={2}
  arrows
  infinite
>
  <div><Img fluid={fluid} /></div>
  <div><Img fluid={fluid} /></div>
  <div><Img fluid={fluid} /></div>
</Carousel>

Have you tried it with Gatsby Image?

webbugt commented 4 years ago

works for me like this

import React from "react"
import { graphql } from "gatsby"
import Img from "gatsby-image/withIEPolyfill" // so objectFit works

import Layout from "../components/layout"
import Carousel, { Dots } from "@brainhubeu/react-carousel"
import "@brainhubeu/react-carousel/lib/style.css"

import styled from "styled-components"

const CarouselWrapper = styled.div`
  width: 100%;
  height: ${(props) => props.desktopHeight};

  & .gatsby-image-carousel-wrapper {

    // carousel container is height:auto, so children need to have set height
    height: ${(props) => props.desktopHeight};
    width: 100%; // 100% of width of the Carousel set container
    padding: 2%;

    // setting gatsby image to 100x100%
    .gatsby-image-wrapper {
      width: 100%;
      height: 100%;
    }
  }
`

CarouselWrapper.defaultProps = {
  desktopHeight: "600px",
}

const Page = ({ data }) => {
  const images = data.allFile.nodes
  return (
    <Layout>
      <CarouselWrapper desktopHeight="400px">
        <Carousel arrows infinite centered slidesPerPage={3}>
          {images.map((img, idx) => (
            <div key={idx} class="gatsby-image-carousel-wrapper">
              <Img fluid={img.childImageSharp.fluid} objectFit="contain" />
            </div>
          ))}
        </Carousel>
      </CarouselWrapper>
    </Layout>
  )
}

...

You don't have to use styled-components, you can use pure imported css or inline style

piotr-s-brainhub commented 4 years ago

@webbugt

So your example works for you?

webbugt commented 4 years ago

@piotr-s-brainhub Yes, I landed upon the exact same issue and error today, that's how I ended up here. After A bit of tinkering, the above approach has worked.

piotr-s-brainhub commented 4 years ago

Thanks @webbugt

@RamiMustaklem does this approach work for you?

RamiMustaklem commented 4 years ago

@piotr-s-brainhub nope, this is a hack. I'm not supposed to give it a height, I'd like my Image height. I'll have to specify a height for big screen, small, medium screens, this loses the point of using Gatsby Image.

I'm not using styled-components, and I do solve it with just css and going around and giving heights all over the place but that shouldn't be the proper solution.

webbugt commented 4 years ago

Agreed, it is hacky. I don't get why isn't the wrapper

element the height of the carousel? Its already getting width dynamically, why not set the height of the elemente to the height of the carousel as well?

On Tue, 7 Jul 2020, 17:41 Rami Mustaklem, notifications@github.com wrote:

@piotr-s-brainhub https://github.com/piotr-s-brainhub nope, this is a hack. I'm not supposed to give it a height, I'd like my Image height. I'll have to specify a height for big screen, small, medium screens, this loses the point of using Gatsby Image.

I'm not using styled-components, and I do solve it with just css and going around and giving heights all over the place but that shouldn't be the proper solution.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/brainhubeu/react-carousel/issues/585#issuecomment-654950368, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHB3H63UPXEG57EC4YUAQGDR2M62RANCNFSM4OQZPDDQ .

piotr-s-brainhub commented 4 years ago

@RamiMustaklem

Are you able to provide a sandbox?

I've created https://codesandbox.io/s/angry-wozniak-c0yd8?file=/src/App.js but I get: