Extra slick-active class applied when centerMode and slidesToShow even

[weberml2]

When the number of slides to show is even and in center mode, an extra slick-active class is applied to the the next offscreen slide.

A quick fix I did for my project is here: https://github.com/weberml2/slick/commit/1b29b453388afa9ea2275d71ab5909dac4dc3c82

[simeydotme]

Would you please create a jsfiddle to demonstrate this with the newest release? it would help us close this much faster :)

[weberml2]

This shows it succinctly: http://jsfiddle.net/5xnjxmqy/1/

Note how in odd numbered centered carousels, the lime background is applied to the hanging slide on the left.

After clicking "Calculate .slick-actives", I would expect the output to be:

#centerTwo: 2
#centerThree: 3
#centerFour: 4
#centerFive: 5

but instead it is:

#centerTwo: 3
#centerThree: 3
#centerFour: 5
#centerFive: 5

Applying this class correctly is important for my use case because it allows me to selectively hide content in the carousel depending on if it is on-screen, or on partially on-screen.

[anteksiler]

Hi @simeydotme, there are 84 pull requests at the moment, do you mind merging them? https://github.com/kenwheeler/slick/pulls?q=is%3Aopen+is%3Apr

[simeydotme]

@anteksiler ... u trollin' me, bro?

If you want to contribute, please find a more appropriate way to do it (such as joining Gitter chat, reddit or email) instead of writing on an unrelated issue. :)

[anteksiler]

@simeydotme Sorry about that, OP included a commit: https://github.com/weberml2/slick/commit/1b29b453388afa9ea2275d71ab5909dac4dc3c82

[simeydotme]

To elaborate for any spectators... Merging even 1 pr is a bunch of work, and potentially dangerous to the core (as we can see with even my own PRs; who as the highest contributor has a pretty good grasp of the core; which are occasionally buggy).

We have to make sure each and every one is:

1. Aligned with Slick's vision.
2. Aligned with Slick's code style.
3. Fixing a real issue
4. Not bloating the plugin unnecessarily
5. Up to date and able to merge
6. Test the PR across hundreds of option combinations (which doesnt always get done)
7. Not breaking existing functionality, or creating regressions

I would put money on 90% of those PRs failing to satisfy points: 1, 3, 4, 5, 6 and/or 7. -- but I can't guarantee it as I cannot possibly review 84 pull requests on my own in any reasonable time frame. -- The vast vast majority of the one's I've looked at are "fixing" some tiny little environment issue which works for them and their website, but would break every other scenario on every other website.

To be honest, I think there should be a way for @kenwheeler to auto-decline PRs now, only accepting any merges from verified issues and people who are willing to really test and contribute in a sensible fashion.

have you really looked at 84 PRs and can tell me truthfully you think that they should all be merged?

Anyway, rant over, but please drop by Gitter (contributing.md) if you want to discuss anything :) -- I enjoy supporting this project, but it is overwhelming, and I cannot possibly keep up with the amount of PRs and Issues that are generated... over 60% of which are not even "a thing" :(

Si.

[simeydotme]

A quick fix I did for my project is here: weberml2@1b29b45

^^ this is exactly what I'm talking about, no offense @weberml2 surely means well, but this is something I cannot entertain without investing a lot of my own time to review/integrate it...

[weberml2]

Yeah, don't merge my code. My code is a quick and dirty fix to make it work, definitely not up to standards of the project. This issue (specifically not a PR) is meant to bring the problem onto the radar of a more project-competent developer to investigate and fix in future release.

[simeydotme]

:bow: appreciated. -- and we are/will eventually get around to handling your issue. :)

[anteksiler]

Hi @simeydotme, all of the PRs from 2014 are either merged or denied, but remains Open for now. I would appreciate it if you could review them in your free time.

[jcandan]

I have the following work-around:

$(".slick__slider").on("afterChange", testingThings);
$(".slick__slider").on("breakpoint", testingThings);
$(".slick__slider").on("init", testingThings);

function testingThings (event, slick) {
  var slidesToShow = slick.slickGetOption("slidesToShow");

  if (slidesToShow % 2 == 0) {
    // Even number of slides in Slick Carousel is incorrectly offset.
    // Adjust active slides.
    slick["$slides"][slick.slickCurrentSlide()].previousElementSibling.classList.add("slick-active")
    slick["$slides"][slick.slickCurrentSlide()].nextElementSibling.classList.remove("slick-active")
  }
}