intro component with form using HTML, Sass, Bootstrap and JavaScript

Community feedback

• 

  Vicktor•900

  @Victor-NyagudiPosted about 2 years ago

  Good attempt on this one, Ricardo.

  The form works the way it's supposed to, and you made a good effort in following the design.

  I looked at your JavaScript, and there are some things you can improve on.

  First, you've repeated this snippet many times in your code.

  if(!lnInput){
          lnMessage.style.opacity = 1;
          lastInput.style.backgroundImage = "url('images/icon-error.svg')";
          lastInput.style.backgroundRepeat = "no-repeat";
          lastInput.style.backgroundPosition = "90% 50%";
      } else{
          lnMessage.style.opacity = 0;
          lastInput.style.backgroundImage = 'none';
      }
  

  This goes against the DRY principle - Don't Repeat Yourself, and can create bugs in your code.

  Imagine you had to change something like the background image URL. You would have to scan through your ENTIRE codebase and manually change the URL everywhere you used it, otherwise, the code will break.

  A better way is to encapsulate it in a method and then call that method where you need to, that way, you'll only need to make changes in one place, and it applies everywhere.

  Here's an example:

  function showErrorFeedback(input) {
     if(!input){
          lnMessage.style.opacity = 1;
          lastInput.style.backgroundImage = "url('images/icon-error.svg')";
          lastInput.style.backgroundRepeat = "no-repeat";
          lastInput.style.backgroundPosition = "90% 50%";
      } else{
          lnMessage.style.opacity = 0;
          lastInput.style.backgroundImage = 'none';
      }
  }
  
  form.addEventListener('submit', function(event){
      event.preventDefault();
      var trialForm = new FormData(form);
      var fnInput = trialForm.get('first-name-input');
      var lnInput = trialForm.get('last-name-input');
      var eInput = trialForm.get('email-input');
      var pInput = trialForm.get('password-input');
      
      showErrorFeedback(eInput);
      showErrorFeedback(fnInput);
      showErrorFeedback(pInput);
  
  // other method calls go here
  

  Which of these looks cleaner and easier to maintain? I'll let you decide.

  I'd also recommend keeping as much styling as you can in your .css file.

  Instead of having it in your JavaScript, you could instead use a boolean value that's false when the data supplied isn't correct and then apply a class to the element you want to appear.

  This way, the JS is only controlling whether or not a class is applied, and the class that's toggled is the one controlling all the CSS.

  Finally, you can remove the action attribute in the form if you're going to handle submit events yourself. This will also remove the error in the HTML validation report.

  Hope this helps.

  All the best with future solutions.

  Marked as helpful

  0

  ---

  

  Ricardo Fuentes Urbina•370

  @RicardoFuentes437Posted about 2 years ago

  @Victor-Nyagudi Thank you very much for the feedback, while i was writting that part of the code i was actually thinking that maybe there was a better way of doing it because it did seem messy to me but i just never got to improve it, so thank you very much this is really helpful :)

  1