Прошу помощи с кодом кратности input на js?

Question

[Денис]

Добрый день, прошу знающих людей дать комментарии и указать на ошибки, если таковые имеются. JS начал изучать недавно. Потребовалось реализовать поле input с возможностью увеличения/уменьшения через button, с возможностью учета кратности и максимального количества. Реализацию сделал, но в целях саморазвития прошу указать на возможные ошибки или возможность сокращения кода.

PS. Спасибо откликнувшимся за тыкание носом в ошибки. Что-то поправил, в чем-то разобрался. Код JS заменил на обновленный. Добавил полифилы для методов, для поддержки IE11, для этих же целей не стал использовать стрелочные функции.

<div class="wa-quantity-box">
        <button class="js-decrease">-</button>
        <input class="js-product-quantity" type="text" value="1" data-step="4" data-max-quantity="16">
        <button class="js-increase">+</button>
    </div>
    <div class="wa-quantity-box">
        <button class="js-decrease">-</button>
        <input class="js-product-quantity" type="text" value="1" data-step="" data-max-quantity="0">
        <button class="js-increase">+</button>
    </div>
    <div class="wa-quantity-box">
        <button class="js-decrease">-</button>
        <input class="js-product-quantity" type="text" value="1" data-min-quantity="0">
        <button class="js-increase">+</button>
    </div>

JS

(function() {

    // проверяем поддержку
    if (!Element.prototype.matches) {
      // Полифилл для .matches
      // определяем свойство
      Element.prototype.matches = Element.prototype.matchesSelector ||
        Element.prototype.webkitMatchesSelector ||
        Element.prototype.mozMatchesSelector ||
        Element.prototype.msMatchesSelector;
  
    }
    // Полифилл для .closest
    // проверяем поддержку
    if (!Element.prototype.closest) {
  
      // реализуем
      Element.prototype.closest = function(css) {
        var node = this;
  
        while (node) {
          if (node.matches(css)) return node;
          else node = node.parentElement;
        }
        return null;
      };
    }
  
})();

// Количество и кратность 
document.addEventListener('DOMContentLoaded', function(){

    //Обходим все инпуты и при наличии data-step присваиваем его инпуту
    let inputs = document.querySelectorAll('.wa-quantity-box .js-product-quantity');
    
    inputs.forEach(function(el) {
        increase = el.closest('.wa-quantity-box').querySelector('.js-increase');
        decrease = el.closest('.wa-quantity-box').querySelector('.js-decrease');
        el.value = parseInt(el.getAttribute('data-step')) || 1;

        // Убираем все лишнее при изменении input
        function Validate() {
            this.value = this.value.replace(/[^0-9]/g, '');
            const maxQuantity = parseInt(el.getAttribute('data-max-quantity')) || 0;
            this.value = maxQuantity > 0 && this.value >= maxQuantity ? maxQuantity : this.value;
            this.value = this.value <= 1 ? 1 : this.value;
        }
        
        // Проверяем кратность при изменении и добавляем до минимальной кратной в большую
        function Divisible(){
            if ( this.getAttribute('data-step')) {
                let value = parseInt(this.value);
                let round = parseInt(this.getAttribute('data-step'));
                let remainder = this.value % round;
                if  ( remainder != 0 ) {
                    this.value = value + round - remainder;
                } 
            }
        }
        
        // Обработчики событий кликов по кнопкам
        increase.addEventListener('click', function(event){
            event.preventDefault();
            const { value } = el;
            const maxQuantity = parseInt(el.getAttribute('data-max-quantity')) || 0;
            el.value = maxQuantity > 0 && parseInt(value) >= maxQuantity ? maxQuantity : parseInt(value) + (parseInt(el.getAttribute('data-step')) || 1);
        });

        decrease.addEventListener('click', function(event){
            event.preventDefault();
            const minPart = parseInt(el.getAttribute('data-step')) || 1;
            el.value = parseInt(el.value) - minPart < minPart ? minPart : parseInt(el.value) - minPart;
        });

        // Назначаем фунции обработчики на события
        el.addEventListener('input', Validate);
        el.addEventListener('keyup', Validate); 
        el.addEventListener('change', Divisible);  
    });

});

Answer 1

[Сергей delphinpro]

1. Жесткая привязка к DOM через выборку previous и next. Расположи кнопки по-другому и все ломается. Не универсально. Лучше дергать по классам или атрибутам.
2. Лишние проверки в обработчиках кликов. Все их можно вынести выше и в обработчике использовать уже считанные параметры счета.
3. Именование хромает. QIncrease => qIncrease. Так уж принято в javascript сообществе — использовать camelCase нотацию для переменных и CamelCase для функций/классов. Да и вообще приставка "Q"/"q" везде лишняя, визуальный мусор. Ну это мелочи.
4. Использование var. Без комментариев.
5. var part = parseInt("1"); Без комментариев.
6. Тоже мелочь, но. Вместо создания ненужного пустого массива можно использовать метод прототипа Array.prototype.forEach.call(). Или создать массив Array.from(qInputs).forEach().
7. this.hasAttribute('data-max-quantity') == true. Тоже оставлю без комментариев.

Ну и еще есть к чему придраться, лень писать.

Comments

[Денис]

Спасибо огромное, пройдусь по каждому пункту.

[Сергей delphinpro]

Денис, Как бы я сделал:

inputs = querySelectorAll('.wa-quantity-box')
forEach inputs {
  btnInc = 
  btnDec =
  input = 
  step = 
  min = 
  max = 

  function increase () {}  
  function decrease () {}  

  addEventListener...

}

Answer 2

[WbICHA]

[].forEach.call(qInputs,function(el){
Зачем? У нодлиста есть свой форыч.

function(){ ... }
К чему они тут, когда есть стрелочные?

var
Вары? Какой сейчас год?

if ( el.hasAttribute('data-step') == true && el.getAttribute('data-step') !== "" ) 
// =>
if(el.getAttribute('data-step'))

parseInt("1");
Серьёзно?

function (e) {
            e.preventDefault();
            if ( el.hasAttribute('data-step') == true && el.getAttribute('data-step') !== "" ) {
                var part = parseInt(el.getAttribute('data-step'));
            } else {
                var part = parseInt("1");
            }
            if ( el.hasAttribute('data-max-quantity') == true && el.value >= parseInt(el.getAttribute('data-max-quantity')) && parseInt(el.getAttribute('data-max-quantity')) != 0  ) {
                var count = parseInt(el.getAttribute('data-max-quantity'));
            } else {
                var count = parseInt(el.value) + part;
            }
            el.value = count;
        }
// =>
(event) => {
  event.preventDefault();
  
  const { value } = el;
  const maxQuantity = parseInt(el.getAttribute('data-max-quantity')) || 0;
  
  el.value = 
    maxQuantity > 0 && value >= maxQuantity 
  	? maxQuantity
  	: value + (parseInt(el.getAttribute('data-step')) || 1);
}

count = count < part ? part : count;
el.value = count;

А что мешало сразу в value записать?

if (this.value.match(/[^0-9]/g)) {
                this.value = this.value.replace(/[^0-9]/g, '');
            }

Для чего тут 2 раза проходить?

parseInt(this.getAttribute('data-step')) == ""  || this.value <= "1"

Серьёзно? Ты вообще в курсе что такое число и как оно выглядит?

В общем, да... Работоспособность не проверял, но тут итак проблем хватает.

Comments

[Денис]

Спасибо большое.