2
\$\begingroup\$

I posted similar question yesterday, And after receiving great feedback by Blindman67, i've decided to change the approach and came up with the following solution - Codepen

Please critique. The code will be used on home page, and main purpose is to have something simple/small size instead of bigger all in one libraries like owl and slick.

var listEl = document.querySelector('.home-grid.products-grid.products-grid--max-4');
var btnLeftEl = document.querySelector('#left-btn');
var btnRightEl = document.querySelector('#right-btn');
count = 0;
btnLeftEl.addEventListener("click", function(e) {
    count++;
    listEl.style.left = count * 286 + 'px';
    if (count > -2) {
        btnRightEl.style.display = 'block';
    }
    if (count >= 0) {
        btnLeftEl.style.display = 'none';
    }
});
btnRightEl.addEventListener("click", function(e) {
    count--;
    listEl.style.left = count * 286 + 'px';
    if (count < 0) {
        btnLeftEl.style.display = 'block';
    }
    if (count <= -2) {
        btnRightEl.style.display = 'none';
    }
});
#body {
  margin: 10px 40px;
}

.home-product-new-hldr {
  position: relative;
  width:1140px;
}  
  
.home-product-new {
  width:1140px;
  overflow:hidden;
}

.home-grid.products-grid.products-grid--max-4 {
  transition:-ms-transform 0.5s ease 0s, -webkit-transform 0.5s ease 0s, transform 0.5s ease 0s;
  position:relative;
  white-space:nowrap;
}

.item-container {
    display: inline-block;
    margin: 4px;
    vertical-align: top;
	width:274px;
}  

.slider-btn-hldr-left {
	left: -32px;
}

.slider-btn-hldr-right {
	right: -32px;
}

.slider-btn-hldr {
    bottom: 0;
    display: block;
    padding: 0;
	width: 24px;
    position: absolute;
    top: 40%;
    z-index: 1;
}

.slider-btn {
	background-color: transparent;
    border: 0 none;
    color: buttontext;
    cursor: pointer;
    display: block;
}

#left-btn {
	    display: none;
}

.slider-btn svg{
  width: 24px;
  height: 24px;
}
  
.products-grid .item {
	margin-left: 0;
	background: white; 
	box-shadow: 0 3px 6px rgba(0,0,0,0.16), 0 3px 6px rgba(0,0,0,0.23);
	float: left;
}
<div id="body">
<div class="home-product-new-hldr">
    <div class="slider-btn-hldr slider-btn-hldr-left">
        <button id="left-btn" class="slider-btn" style="display: none;">
        <svg viewBox="0 0 256 256">
            <polyline fill="none" stroke="black" stroke-width="16" points="184,16 72,128 184,240"></polyline>
        </svg>
    </button>
    </div>
    <div class="home-product-new">
        <div class="home-grid products-grid products-grid--max-4" style="left: 0px;">
            <div class="item-container">
                <div class="item">
					<img src="https://unsplash.it/274/400?image=7"/>
                </div>
            </div>
            <div class="item-container">
                <div class="item"> 
					<img src="https://unsplash.it/274/400?image=21"/>
				</div>
            </div>
            <div class="item-container">
                <div class="item">
					<img src="https://unsplash.it/274/400?image=14"/>
                </div>
            </div>
            <div class="item-container">
                <div class="item">
					<img src="https://unsplash.it/274/400?image=19"/>
                </div>
            </div>
            <div class="item-container">
                <div class="item first">
					<img src="https://unsplash.it/274/400?image=22"/>
                </div>
            </div>
            <div class="item-container">
                <div class="item">
					<img src="https://unsplash.it/274/400?image=23"/>
                </div>
            </div>
        </div>
    </div>
    <div class="slider-btn-hldr slider-btn-hldr-right">
        <button id="right-btn" class="slider-btn" style="display: block;">
        <svg viewBox="0 0 256 256">
            <polyline fill="none" stroke="black" stroke-width="16" points="72,16 184,128 72,240"></polyline>
        </svg>
    </button>
    </div>
</div>
<div>

\$\endgroup\$
2
\$\begingroup\$

Few concerns...

  • Everything is global. You risk messing with some other variable called count, as that's a pretty common var name. Put it in an IIFE.
  • count is not initialized anywhere.
  • The code requires there to be exactly the same number of slides. If you ever decide to add or remove an image you will have to edit the source code too.
  • Your two button handlers are basically the same exact code with the logic inverted. You can easily same yourself some maintenance time by combining them into a single function.

Here's a rewrite with my suggestions implemented.

(function(){

    var listEl = document.querySelector('.home-grid.products-grid.products-grid--max-4');
  var btnLeftEl = document.querySelector('#left-btn');
  var btnRightEl = document.querySelector('#right-btn');
  var count = 0;

  function slideImages(dir){
    var totalChildren = listEl.querySelectorAll(".item").length;
    dir === "left" ? ++count : --count;
    listEl.style.left = count * 286 + 'px';
    btnLeftEl.style.display = count < 0 ? "block" : "none";
    // Here, 4 is the number displayed at any given time
    btnRightEl.style.display = count > 4-totalChildren ? "block" : "none";
  }

  btnLeftEl.addEventListener("click", function(e) {
      slideImages("left");
  });
  btnRightEl.addEventListener("click", function(e) {
      slideImages("right");
  });


})();

I forked your codepen and added more forks for fork sake.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for?Browse other questions tagged or ask your own question.