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

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

Open
1 task done
hmartiins opened this issue Jul 21, 2023 · 12 comments
Open
1 task done
Labels
feature request New feature or request

Comments

@hmartiins
Copy link

hmartiins commented Jul 21, 2023

Feature request is related to

  • embla-carousel (core package)

Embla Carousel version

  • embla-carousel-react: 8.0.0-rc10,

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

  • When deactivating the carousel on the second slide, it should not go back to the first slide, but should be deactivated on the slide that I stopped

Additional context

- Video showing the behavior

@hmartiins hmartiins added the bug Something isn't working label Jul 21, 2023
@davidjerleke
Copy link
Owner

davidjerleke commented Jul 22, 2023

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

@davidjerleke davidjerleke changed the title Changing the carousel asset setting resets the slide to first Changing the carousel active setting resets the slide to first Jul 23, 2023
@hmartiins
Copy link
Author

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 davidjerleke added feature request New feature or request and removed bug Something isn't working labels Jul 24, 2023
@davidjerleke
Copy link
Owner

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
Copy link
Author

Awesome! 🤩

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

@davidjerleke
Copy link
Owner

@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
Copy link
Author

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

@Lemaro86
Copy link

Lemaro86 commented Sep 3, 2023

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
Copy link
Owner

davidjerleke commented Sep 3, 2023

@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
Copy link

Lemaro86 commented Sep 3, 2023

@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
Copy link
Contributor

silllli commented Sep 20, 2023

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
Copy link

omakise commented Jan 24, 2024

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
Copy link
Owner

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

@davidjerleke davidjerleke changed the title Changing the carousel active setting resets the slide to first [Feat]: Changing the carousel active setting resets the slide to first Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants