flexbox, vanilla js

Community feedback

• 

  Rémi Martineau•355

  @MartineauRemiPosted about 2 years ago

  Hey ! Congratulations, you did a great job ! Here are some small improvments you could implement to make your JS file easier to read / udpate.

  1. You could define your functions in a separated file, and import them in the main.js file. That way, you can see better which part of the code is applied on the page.

  2. There are a lot of 'for loops' in your code, and sometimes, for loops inside of other for loops. This can create a really verbose code and it may be hard to read. You could use some functions from JS such as map (link to map documentation). For example, in your code (main.js - line 128) : You could replace this :

  for (let i = 0; i < data.comments.length; i++) {
      mainContent.innerHTML += commentObject(data.comments[i].user.username, data.comments[i].user.image.webp, data.comments[i].createdAt, data.comments[i].content, data.comments[i].score)
  
      for (let j = 0; j < data.comments[i].replies.length; j++) {
        let replySection = document.getElementsByClassName('replySection');
        replySection[i].innerHTML += replyObject(data.comments[i].replies[j].user.username, data.comments[i].replies[j].user.image.webp, data.comments[i].replies[j].createdAt, data.comments[i].replies[j].content, data.comments[i].replies[j].score, data.comments[i].replies[j].replyingTo)
      }
  
    }
  

  With this :

  data.comments.map((comment, index) => {
      mainContent.innerHTML += commentObject(comment.user.username, comment.user.image.webp, comment.createdAt, comment.content, comment.score);
      comment.replies.map((reply) => {
          let replySection = document.getElementsByClassName('replySection');
          replySection[index] += replyObject(reply.username, reply.user.image.webp, reply.createdAt, reply.content, reply.score, reply.replyingTo);
      });
  });
  

  Notice the parameters passed to commentObject() and replyObject(). It is overall, easier to understand in my opinion. (I used an arrow function inside map. If you're not familiar with this concept, here is the doc.)

  3. Instead of doing several for loops having the same limit, you could do one single for loop, and put all your instructions inside. For example, with your function toLike(), it can be something like:

  function toLike() {
      let plus = document.getElementsByClassName('plus');
      let moins = document.getElementsByClassName('minus');
      let screen = document.getElementsByClassName('screen');
  
      for(let i = 0; i < screen.length; i++) {
          plus[i].addEventListener('click', () => {
              screen[i].innerHTML = parseInt(screen[i].innerHTML) + 1;
              moins[i].disabled = false;
              plus[i].disabled = true;
            });
          
          moins[i].addEventListener('click', () => {
              if (plus[i].disabled) {
                  screen[i].innerHTML = parseInt(screen[i].innerHTML) - 1;
                  moins[i].disabled = true;
                  plus[i].disabled = false;
              }
          });
      }
  }
  

  Hope this is helpful, keep up the good work ! Happy coding :)

  Marked as helpful

  1