thednp / bootstrap.native

Bootstrap components build with Typescript
http://thednp.github.io/bootstrap.native/
MIT License
1.7k stars 177 forks source link

Bootstrap 4 Native Carousel Not initalized with slide.length <2 #412

Closed Chrisyss closed 3 years ago

Chrisyss commented 3 years ago

This has to be removed to initialize the Carousel even with just 1 carousel-item. This is important, because if you use a carousel in a modal then the modal does not get launched.

To fix this uncomment line 642 in bootstrap-native-4.min.js // invalidate when not enough items // if (slides.length < 2) { return; }

results in no early exit.

I believe this should not be intended behavior.

thednp commented 3 years ago

Yes, that filter was put into place to solve some animation issue with previous versions.

However it would still make little sense not to have it and I see no connection with Modal. The only case where the filter would be better off, is to init the carousel empty and populate dynamically and re-init on every markup mutation. Its just better to re-init on every mutation.

But maybe I don't get your issue? Perhaps a demo fiddle would shed more light.

Chrisyss commented 3 years ago

I appreciate the quick response! The problem is having no carousel with only 1 slide item. For consistency if you have an automated website where elements get populated on the fly ( such as HUGO SSG) The codepen or fiddle demo would consume a bit too much time right now. Extra code to initialize it manually would consume more unnecessary bytes.

You were right about the animation. the fix was to limit the animation to only slides.length>=2

So now you can have a carousel with 1 slide with no extra code

I do hope this is clear. Fantastic work on the Native version of bootstrap 4.. It brought down a lot of kilobytes for our users and also Jquery free. You are doing a great deal for industries.

roughly line 582 in the minified version

    //slides.length>=2 change
    if(slides.length>=2){
    if (getElementTransitionDuration(slides[next]) && element.classList.contains('slide')) {
      slides[next].classList.add(("carousel-item-" + orientation));
      reflow(slides[next]);
      slides[next].classList.add(("carousel-item-" + (vars.direction)));
      slides[activeItem].classList.add(("carousel-item-" + (vars.direction)));

      emulateTransitionEnd(slides[next], transitionEndHandler);
    } else {
      slides[next].classList.add('active');
      reflow(slides[next]);
      slides[activeItem].classList.remove('active');
      setTimeout(function () {
        vars.isSliding = false;
        // check for element, might have been disposed
        if (ops.interval && element && !element.classList.contains('paused')) {
          self.cycle();
        }
        dispatchCustomEvent.call(element, slidCustomEvent);
      }, 100);
    }
  }
thednp commented 3 years ago

@Chrisyss I don't see how this change will benefit the community at large, this filter is meant to address inconsistent animations as well as the fact that you don't need to allocate memory where it doesn't make sense.

I remember now my reasoning behind some of my implementations, so I have a workaround that would make use of the BSN.initCallback whenever you add new items to your DOM or specific container. I think it's the best way to address your specific issue, with/without MutationObserver and any other callbacks your scripts need.

With all that said however, I'm not going to rule out the possibility to implement it when more people would actually need it and ask for it.

thednp commented 3 years ago

@Chrisyss we good to close this?

thednp commented 3 years ago

Closed due to no activity.