Time-tracking-dashboard

Community feedback

• 

  P

  Aaron Smith•280

  @medic-codePosted 4 months ago

  General feedback

  Generally a good piece of work, nice responsiveness between mobile and desktop views, something i forgot about when completing my solution. There's a few minor UI aspects that could've been corrected. The HTML and CSS are relatively good. The JS i think there's some room for improvement in terms of conciseness, but looks like solid in terms of output on the screen.

  Specific feedback

  UI

  1. On clicking on daily,weekly, monthly i see you haven't highlighted the hrs part of the cards,

  HTML

  1. Some of the class names are a bit non descriptive, perhaps think about using a methodlogy like BEM
  2. Add in the hr's text to the cards as per the design (i can see why you left it out, makes the JS trickier)

  CSS

  1. I noticed that the dashboard isn't placed centrally within the page, you probably don't need to use grid for this on the main/body elements. Grid is better used for aligning complicated parts in 2 dimensions. Not to say i'm not so sure why place-items is not currently centering for you though, looks like there may be a problem with main element inheriting the place-items grid.

  2. Noticed you've got some padding on the body element that might be contributing to it not centering properly

  3. Couple of places you're using hex values instead of hsl, just a very minor point.

  JS

  1. Consider using fetch and live-server to serve your html in VSCode, you've dumped the data into the js file.

  2. The approach of removing and adding classes is probably a little redundant for a simple change in textContent for some elements. 2.1 Consider an approach to change the textContent of the time and last week time elements for each card instead of recreating large amount of the card html.

  Hope some of that is useful!

  Marked as helpful

  0