Multi-step-form | HTML | SCSS | vanilla JS

Community feedback

• 

  Vicktor•900

  @Victor-NyagudiPosted almost 2 years ago

  Hi, Lucas.

  Good attempt on this one.

  I noticed you repeated this snippet of code in your JavaScript where you check if a value is an empty string and then change its display property.

  if (nameInput.value == "") {
     nameInput.style.borderColor = "hsl(354, 84%, 57%)";
     errorMessages[0].style.display = "block";
  }
  else {
     nameInput.style.borderColor = "hsl(229, 24%, 87%)";
     errorMessages[0].style.display = "none";
  }
  

  This works now, but it's fragile and can break easily.

  Imagine you had to change something in this code snippet e.g. use backgroundColor instead of borderColor, you'd have to go through every if statement and make the change manually.

  You could encapsulate it in a method and call the method multiple times and supply different arguments. This way, if you need to change something, you can do so in one place.

  Here's an example of the code in a method that's called multiple times.

  function changeBorderColor(input, hsl) {
     if(input.value == ""){
          input.style.borderColor = hsl;
          errorMessages[0].style.display = "block";
      }
      else{
          input.style.borderColor = hsl;
          errorMessages[0].style.display = "none";
      }
  }
  
  changeBorderColor(nameInput, "hsl(229, 24%, 87%)");
  changeBorderColor(mailInput, "hsl(229, 24%, 87%)");
  changeBorderColor(numberInput, "hsl(354, 84%, 57%)");
  

  Which of these looks cleaner and easier to maintain to you?

  I'll let you decide.

  I hope this helps.

  All the best with future solutions.

  1

  ---

  

  Lucas Cerri•1,140

  @llKryptonixllPosted almost 2 years ago

  @Victor-Nyagudi thanks for the help, I will use this way, it looks much cleaner

  1