davidjerleke / embla-carousel

A lightweight carousel library with fluid motion and great swipe precision.
https://www.embla-carousel.com
MIT License
5.4k stars 166 forks source link

[Feat]: Changing the carousel active setting resets the slide to first #540

Open hmartiins opened 11 months ago

hmartiins commented 11 months ago

Feature request is related to

Embla Carousel version

Describe the feature request

I have a state, which starts as true, controlling the active option of my carousel.

When scrolling my carousel to a slide other than the initial slide and changing the value of this state to false, my carousel goes back to the first slide

When I change my state value to true again, it returns my carousel to the slide it was before the first state change occurred

CodeSandbox

Steps to reproduce

  1. Create a carousel and pass a boolean state that controls the active option
  2. Add a button that when clicked changes this state to false
  3. Scroll the carousel to the second slide
  4. Click on the button created to update the state of the carousel and note that the carousel that was on the second slide returned to the first slide

Expected behavior

Additional context

- Video showing the behavior

davidjerleke commented 11 months ago

Hi @hmartiins,

Thanks for your bug report. I wouldn’t call this a bug though because it isn’t obvious to me that it should work the way you describe. I think you could argue for both sides.

The active option is there to activate/deactivate the carousel. It destroys the carousel with the exception of letting it watch the active option in case the user activates it again.

You can preserve the state yourself like this:

// preserve location before changing the active option to false
const preservedLocation = emblaApi.internalEngine().location.get()

And apply that value to the container after you’ve deactivated the carousel with translateX(‘${preservedLocation}px’).

Best, David

hmartiins commented 11 months ago

Hi @davidjerleke!

I understand what you mean, after creating this issue I took a look at the library code and understood what you meant.

However, in my understanding, when changing the state of the active option, it should not reset the carousel to the first slide. But really, this isn't a bug according to the code.

It would be cool and very useful if there was an option in the embla api that we could change the value of active without the carousel going back to the first slide.

davidjerleke commented 11 months ago

Hi @hmartiins,

Thank you for your input. Many devs use this option to disable the carousel entirely on desktops and activate the carousel on smaller screens, for example mobile phones. In these cases, removing any styles applied by Embla is necessary. This is how it works today and in practice this means resetting to the first slide.

So if we’re to introduce something it should be an additional feature and not replace the current behavior.

For example, one way would be to change the active option to accept string values:

type ActiveOptionType = boolean | 'freeze';

true // Carousel is active
false // Carousel is basically destroyed but listening for active option to change
'freeze' // Preserve last scroll location

Is this close to what you meant?

Best, David

hmartiins commented 11 months ago

Awesome! 🤩

Yes, it was exactly what I had in mind, and I believe it solves the case I mentioned.

davidjerleke commented 11 months ago

@hmartiins,

Thank you for your swift response. I want to be clear so you have the right expectations:

I have a lot to do before the stable v8 release so new features will not be prioritized until v8 is released. This means that you have to use the workaround for now if you still want to use this library.

The specification for this feature is not a breaking change so it can easily be introduced after the major v8 release.

In the meantime, if you can think of a better term than freeze, please let me know.

Best, David

hmartiins commented 11 months ago

@davidjerleke I couldn't think of any better terms, I think freeze suits it well.

Lemaro86 commented 10 months ago

I have trouble with this bug too. Can't add to slide list new slides in start of array. My carousel go to 1 slide and it looks like a bug. Maybe @hmartiins you can create patch and pr with your decide how to prevent moving to slide 1? Thank you for highlighting this point.

davidjerleke commented 10 months ago

@Lemaro86 thanks for chiming in.

First of all, this isn’t a bug. It’s a feature request.

Secondly, when reading your description of the problem you’re most likely doing something wrong and it doesn’t seem to be a bug or related to this.

Lastly, what do you mean with this:

Maybe @davidjerleke you can create patch and pr with your decide how to prevent moving to slide 1?

?

Lemaro86 commented 10 months ago

@Lemaro86 thanks for chiming in.

First of all, this isn’t a bug. It’s a feature request.

Secondly, when reading your description of the problem you’re most likely doing something wrong and it doesn’t seem to be a bug or related to this.

Lastly, what do you mean with this:

Maybe @davidjerleke you can create patch and pr with your decide how to prevent moving to slide 1?

?

Thank you for so fast response. I am sorry. I mean hmartiins. I'll edit my message. If it feature in what version you wanna to add it in roadmap?

silllli commented 9 months ago

Hi @hmartiins,

Thank you for your input. Many devs use this option to disable the carousel entirely on desktops and activate the carousel on smaller screens, for example mobile phones. In these cases, removing any styles applied by Embla is necessary. This is how it works today and in practice this means resetting to the first slide.

So if we’re to introduce something it should be an additional feature and not replace the current behavior.

For example, one way would be to change the active option to accept string values:

type ActiveOptionType = boolean | 'freeze';

true // Carousel is active
false // Carousel is basically destroyed but listening for active option to change
'freeze' // Preserve last scroll location

Is this close to what you meant?

Best, David

I would love to have this feature too. I keep forgetting that setting active to false completely destroys the carousel—in my mind the words ‘active’ and ‘inactive’ are more of a temporary nature. But yes, the wording is debatable. 🤠

Another suggestion would be to introduce an additional option, something like listening, which determines whether the carousel will (listen and) react to any clicking, dragging or swiping.

Edit: Oh, I just noticed that the draggable option was renamed to watchDrag and is exactly doing that. Works perfectly in my case (which already had the draggable option in use but wasn’t updated yet).

omakise commented 5 months ago

I'm having a similar problem. When I set the parent element to display:none; the value of translate3d resets to 0px! I think we should keep the current location.

davidjerleke commented 5 months ago

@omakise you can preserve the index before applying display: none and restore it when the carousel gets visible with the startIndex option.