Time tracking dashboard with grid and json

Community feedback

• 

  Cats-n-coffee•670

  @Cats-n-coffeePosted about 2 years ago

  Hi Marcin!

  Nice work on this challenge! Your desktop view looks very good!

  Feedback to answer your question(s) about your Js (please note: your solution seems to be working fine, so below is my advice/opinion - it's always a little biased) :

  • you could place your function definitions outside your timeTrack function. What you did is called closure I believe, it's not wrong at all, but it's not needed here (read on it if you're not familiar, it's fun and very helpful in certain situations). You can define your functions outside and call them inside your timeTrack. Going forward, isolating functionality will help you make your code more modular/reusable.
  • your timeTrack function could probably be inside an event listener such as DOMContentLoaded or load, I'll let you read on that.
  • I wonder if you'll be able to use CSS for your 'active' tab instead of adding one class and removing 2 classes? You can style certain states with CSS - like hover - for certain tags that have those states (so you might need to change your HTML tag for this to work - not sure). If you're able to do that, you can make dailyData your event handler. (not tested - just a thought)
  • your dailyData, weekData, ... functions do a lot of work. You should be able to loop through this JSON data without having to do data[0] and repeat your code in each of those data functions to only change one key (those functions are the exact same except for the time key). Look into Js objects and the looping methods associated (Object.keys, Object.entries, ...). There's a lot of good stuff on MDN, and of course google search. Working with data (and objects) is fun and challenging, good job!
  • you can use a linter to help with things like keeping your code in a more logical order. I.e: you have you JSON file in a const but it's in the middle of querySelectors. A linter will probably tell you to move it up or down (if configured correctly). And linters do many more things, they do help most people write cleaner code by following conventions, best practices, ... .

  These are the couple points I observed could be improved outside of your question(using FF):

  • your mobile view isn't responsive all the way to 375px. Around 400px there is no padding around the cards and the cards themselves are being crunched (I get a horizontal scroll bar). The FEM attribution lingers half way on the middle card.
  • on the mobile view, it seems that each card could use a little less padding or bigger font size (not sure of the specs) - if you're doing this with the screenshots (as opposed the design files) that's a good challenge!
  • your HTML file is easy to read and the CSS classes are easy to follow (my opinion), I think you could replace some of those div by other HTML tags if you look at the MDN docs. It's also missing a heading but the report above already says that.

  Hope this helps!

  Marked as helpful

  0

  ---

  

  Marcin•270

  @marcinsuskiPosted about 2 years ago

  Hi @Cats-n-coffee!

  Thank You very much. I've been learning front-end for the last 3-4 weeks and I still haven't heard about many things You just pointed out. Thanks for all the tips - I will definitely read on all of them.

  As for the mobile view, I did look at it on mobile but after Your remarks did I try all the possible views and I tracked all that You mentioned. Thanks for the detailed check!

  as for closure, it's true and I know I did that, though somehow I wasn't able to take it all outside timeTrack that fetched json. Outsdie fetch those functions seemed to not work at all. I will have to read on how to solve this as weel. I don't like how it is structured at the moment.

  I'm really grateful for the time You spent on my solution :)

  0

  

  Cats-n-coffee•670

  @Cats-n-coffeePosted about 2 years ago

  @marcinsuski My pleasure! Those functions are synchronous and your timeTrack function is asynchronous, but those data functions need to use the data constant. If you keep them inside like you have, you're accessing your data constant that's already been awaited. Once you take that logic outside and pass data as a parameter to any of those functions, data might be undefined because those functions are still synchronous and event handlers can be a little tricky when it come to passing arguments (checkout this post if you're leaning towards this https://stackoverflow.com/questions/10000083/javascript-event-handler-with-parameters). So you might need to async/await more than your timeTrack function. I would place the entire request outside in its own function as well, and you could store the result in a variable that can be accessed by your data functions. Whichever makes the most sense to you!

  1