onInput issues

[pavlexander]

I would like to report a couple of issues in function onInput in following file: https://github.com/miracleonyenma/custom-vue-range-slider/blob/a7fe59c8d9eff250d55e6c30debeeaa15b4d60e3/src/components/CustomMinMaxSlider.vue#L72

1. the first improvement I would suggest is to introduce separate functions for min and max slider. there is no point in checking the name of the slider. You only have 2 of them, so you could just split the function into 2. This is merely a performance concern and not a very substantial one.

const onInputMin = ({ target }) => { //
}

const onInputMax = ({ target }) => { //
}

2. Secondly, why do you set the value that is equal to a value of another slider? target.value > sliderMaxValue.value ? target.value = sliderMaxValue.value : sliderMinValue.value = parseFloat(target.value);

per my understanding, it should not be a default behavior anyways even if it should be possible to do. The value of the opposing slider should be x +- 1. For min slider the value should be max slider -1, for max slider the minimum value should be min slier value +1.

    target.value >= sliderMaxValue.value  // see >=
      ? target.value = sliderMaxValue.value - 1 // see - 1 (for the other slider we set the +1
      : sliderMinValue.value = parseFloat(target.value);

3. lastly I would like to address the issue that the code does not make sense. The value should be set ALWAYS, regardless of the new value. There should be no "Else" statement. You can try to move the slider extremely fast, physically. then at some point you will notice that the range will not be set correctly. that is because of the else condition that you've set.

ideally it should be:

if (target.value >= sliderMaxValue.value){
target.value = sliderMaxValue.value - 1
}
sliderMinValue.value = parseFloat(target.value); // always set the value! no ELSE!

4. Lastly I also got rid of the parseFloat, not sure if this should be reported as a "problem", but I could not think of a use-case for this. The value is always returned as the integer. isn't it so? So at what point do we need to have a floating point conversion? This is the second performance concern. It should not influence the logic in any way.

Regardless if you agree with all suggested edits and proposals or not, here is the version of the code that I am using for my needs:

const onInputMin = ({ target }) => {
  if (target.value >= sliderMaxValue.value) {
    target.value = sliderMaxValue.value - 1
  }

  sliderMinValue.value = target.value
}

const onInputMax = ({ target }) => {
  if (target.value <= sliderMinValue.value) {
    target.value = sliderMinValue.value + 1
  }

  sliderMaxValue.value = target.value
}

Thank you very much for your solution! it works much faster than hand-made divs that act like range sliders. Soon as I added 2000 of range sliders your solution works just almost as good as 1 range slider. Whereas the custom made slider is laggy as hell and wants to obliterate my PC every second :)