Add variableHeight argument to CarouselProvider that disables the ratio

[reckter]

What:

Add a variableHeight attribute to CarouselProvider.
If set to true, the Slider will not adhere to the ratio being set with naturalSlideHeight and naturalSlideWidth.
The Slider will have the height of the biggest Slide.

Why:

So you can use non scaling context (like text on the slide).
Without these changes, the Slide would be either too small or too big (and just clip the Slide in half).

This somewhat addresses the issues in the following issues. This does not however change the hide of the Slider depending on the current shown slide, which might be another cool addition.

• Variable sized content #23
• Is there a way to let CarouselProvider naturalSlideHeight to be auto? #225
• Variable Height Support Needed #176
• slides containing text are overflowing and appear cut because of the way naturalSlideWidth and naturalSlideHeight are setup #154

How:

We store variableHeight in the store in CarouselProvider.
Each Slide then sets styles depending on that.
The styles Reset the position, height and padding-bottom css attributes, which allows the Slider to be as heigh as the biggest Slide, therefore dynamicaly scalling with the content.

Checklist:

• Documentation added/updated
• Typescript definitions updated
• Tests added and passing
• Ready to be merged

As this is my first change in the library, it might be wise to do a bit further testing, as I might have missed some egde cases.
But it feels very good, and all test pass.

Workaround
This can be archived "from the outside" by using these css rules:

.slider li {
    position: unset !important;
    height: unset !important;
    padding-bottom: unset !important;
}
.slider li .carousel__inner-slide {
    position: unset;
}

[mrbinky3000]

Generally, if it can be handled via CSS, then I'm against adding new props to the components. But this seems to be a recurring theme with the height so I relent.

I have some requests though...

Can you add an example to our list of live demos? I want to download the branch and see the demo so I can see how this affects a vertical carousel?

If one slide is 1000px in height, does that mean all slides are 1000px in height or are there "gaps" where the background will bleed through. Have you looked at flex-box instead. It will make all slides as tall as the tallest slide.

Is there an option for "variable width" on vertical carousels?

I might want to change the name to useIntrinsicDimension or intrinsicAxisSize or something more descriptive because "variable height" to me sounds like responsive height, and the carousel is currently responsive as is. Might be confusing to others. I'm getting the Axis concept from flex-box.

[reckter]

@mrbinky3000
I will add a Demo for sure.

Currently they do not flex to all become the same height.
Instead there is a gap beneath all smaller slides, that fills the missing height.
I am unsure what the right solution here would be regarding stretching all of the slides to be the same height, both (stretching and not-stretching) seem to make sense at first thought.

I haven't looked into the width case, so currently there is no such option, it shouldn't be too hard to add though.

Open for renaming, haven't put too much thought into that.
doNotEnforceRatio might be good candidate as well, as this is essentially what this does.

[reckter]

Looking at the vertical orientation:

The width of the whole slider currently scales to 100% of the container, in both vertical and horizontal arrangement. Based on that width, all the other sizes are calculated.
But to have the same behavior in vertical as in horizontal mode (with the new flag enabled), the height should be the size set from outside and all the rest depending on it, so the width then can become dependent on the widest slide.

With that being said:
I don't know if it makes sense to also add this feature, as it seems more edge-casy than in horizontal mode.
But if left out, the flag should either reflect that (being useIntrinsicHeight) or an error should be added to allert the user, that this is not supported behavior.

[mrbinky3000]

Can you add the error for using this new flag in vertical mode?

Also can we change the name of the option to "isIntrinsicHeight". We use "is[blank]" for booleans. Also the word "use" implies a hook.

Make those changes and I will approve.

[reckter]

Let me know if the Error message is ok the way it is, or if you want to have something added :)

[mrbinky3000]

It's all good, thanks!

[janoist1]

Thanks for this!
When is it going to be released?

[larafale]

when release ? :) thanks for this package

[tim-steele]

@allcontributors please add @reckter for code

[allcontributors]

@tim-steele

I've put up a pull request to add @reckter! 🎉

[bcarroll22]

🎉 This PR is included in version 1.26.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀