Time Tracking Dashboard with Grid

Community feedback

• 

  Dylan Heslop•2,460

  @dylan-dot-cPosted 5 months ago

  Hey Julianna! This is an interesting solution and you did well, I like how you added the popup menu to the cards when you click them.

  Firstly for your question... normally fetch is like this

  fetch('./data.json') //gets file from storage
     .then(res => res.json()) //turns the file into a json object
     .then(data => {
          timeData = data; //set data to your global value timeData
          fetchTimeDaily(timeData)//use your object to get the daily data
      })
     .catch(e) .....
  
  

  so you can essentially call fetchTimeDaily(data) before doing timeData = data; this isn't really bad but maybe the function uses timeData as the global variable and not the timeData parameter and maybe that could cause problems.

  So the code you provided isn't correct as the data.json file needs to be fetched first then turned into json and then and only then can you use it in your code... hoope all of that is understandable.

  Here are a few ways to improve your code!

  • Layout: You have used grid and grid-template-columns: repeat(auto-fit, minmax(180px, 1fr)); to make it responsive which is good, but you might need to do a few more stuff to control the width and height of the grid. You also had position:absolute on the grid container but you could use better ways to center it.
  • Accessibility: You used buttons which is good but you need to use more semantic html tags like main, article, section and others instead of overusing divs in your code. Which can help screen-readers to understand your website better.
  • Fixed WIdths and Heights: Some places you have these and if you used them as %s it becomes hard to control the height of the containers.
  • Not using DRY code: Your JS code solves the problem but it's heavily repeated! You can break up your code into smaller functions and make use of arrays and dom Manipulation functions like querySelectorAll to loop through the arrays and only do it once and not 6 times. Code like this is hard to debug and change in the future. Same goes for the 3 buttons as that could be one function... let me give you a example

  <button value="daily" class="timeframe--button">Daily</button>
  <button value="weekly" class="timeframe--button">Weekly</button>
  <button value="monthly" class="timeframe--button">Monthly</button>
  

  const buttons = document.querySelectorAll('.timeframe--button');
  buttons.forEach( btn => {
     btn.addEventListener("click", e => {
         let data = btn.value;
         timeframe = data;
         updateData(timeframe);
     })
  })
  

  You should have an updateData function that is smart enough that it takes the current timeframe and updates the info using that data.

  This is alot but its important for being a better programmer.

  You could also check out my solution to see how I did it as I also used vanilla js for mine.

  Take care and have a great day!

  Marked as helpful

  0

  ---

  

  Julianna Messineo•290

  @mathematiCodePosted 5 months ago

  @dylan-dot-c Thank you! This is very helpful. I really appreciate the focus on writer cleaner code as that's a big focus for me.

  1