Time tracking dashboard

Community feedback

• 

  Grego•1,430

  @Grego14Posted 4 months ago

  👋 Hello! Congratulations on completing the challenge. 🎉

  After reading your code, here are some tips.

  HTML

  Use <button> instead of <p> elements for the options. Since you can't focus on these elements, and you should use buttons to implement that kind of interactivity.

  Use default width and height in images to avoid CLS🔗.

  • user_img and elipsis element.

  <img src=" ./images/image-jeremy.png" alt="Jeremy Robson" class="user_img" width="60" height="60">
  

  There's no need to use an alt attribute on decorative or semantic images.

  • img.icon

  Use a <h2> instead of a <p> in the cart__title element. And if you want to add the ellipse there as well, you can wrap them in a <div>.

  Using emmet -> div>h2+img

    <div>
      <h2></h2>
      <img/>
    </div>
  

  Replace the <h2> that wraps the time with a <div> or a <span>.

  JavaScript

  You don't need to excessively repeat class deletions to elements that don't have them, you could just get the old element that contains it, remove and add it to the new one.

  function updateOptionsCSS(selected) {
    const lastSelectedOption = document.querySelector(".option.selected_option")
    const selectedClass = "selected_option"
  
    lastSelectedOption.classList.remove(selectedClass)
  
    if(selected === 'daily'){
      daily.classList.add(selectedClass)
    }else if(selected === 'weekly'){
      weekly.classList.add(selectedClass)
    }else{
      monthly.classList.add(selectedClass)
    }
  }
  

  I would rename the function getPrefix to something like updatePrefix since you're not getting it but updating it.

  I recommend using data attributes in the options, Together with Event Delegation🔗 and thus avoid using an event listener for each option.

  <div id="options-container">
    <button type="button" data-time="daily">
    <button type="button" data-time="weekly">
    <button type="button" data-time="monthly">
  </div>
  

  const optionsContainer = document.getElementById('options-container')
  
  optionsContainer.addEventListener("click", (e) => {
    if(!e.target.getAttribute("data-time")) return
  
    updateData(e.target.getAttribute("data-time"))
  })
  

  Event delegation is a good way to make your code take up less memory by using only one function to handle events.

  Don't be afraid to use variables!

  for (let i = 0; i < json_data.length; i++) {
    const item = json_data[i]
    const title = item.title
    const timeframes = item.timeframes[selected]
  
    if (title === "Work") {
      workCurrent.textContent = `${timeframes.current}`;
      workPrevious.textContent = `${timeframes.previous}`;
    }else if (title === "Play") {
      playCurrent.textContent = `${timeframes.current}`;
      playPrevious.textContent = `${timeframes.previous}`;
    }else if (title === "Study") {
      studyCurrent.textContent = `${timeframes.current}`;
      studyPrevious.textContent = `${timeframes.previous}`;
    }else if (title === "Exercise") {
      exerciseCurrent.textContent = `${timeframes.current}`;
      exercisePrevious.textContent = `${timeframes.previous}`;
    }else if (title === "Social") {
      socialCurrent.textContent = `${timeframes.current}`;
      socialPrevious.textContent = `${timeframes.previous}`;
    }else if(title === "Self Care") {
      selfCareCurrent.textContent = `${timeframes.current}`;
      selfCarePrevious.textContent = `${timeframes.previous}`;
    }
  }
  

  I hope all these tips help you, keep coding! 😁

  Marked as helpful

  0