time-tracking-dashboard(html,css and JS)

Community feedback

• 

  P

  Remus D. Buhaianu•3,125

  @MiculinoPosted almost 3 years ago

  Congratulations on completing the challenge, Nishant!

  I had a look at your code and your live solution. Here are a few suggestions / observations from me that I hope will be useful to you:

  • The most evident layout issue is the responsive design of the project. It requires some polishing for the bigger screen sizes.

  • The "transition" property needs to be set on the element itself and not on the :hover state. That's why the effect isn't working as expected

  • You don't need an event listener to create the hover effect. You can create the effect using CSS

  • To use the data from the JSON file, you can use the Fetch API https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API

  • Instead of creating 6 manually (.work, .play, etc) , you can use the forEach method on the array you'll extract from the JSON file. Then all you have to do is use template literal to create the HTML structure for it. Here's an example you can refer to: https://stackoverflow.com/questions/55976205/template-literal-loop-in-javascript

  • Your .component element doesn't fully cover the element behind it. A little bit of the color behind it can be see around the edges, especially at the bottom

  • You don't need to use inline styling to set "cursor: "pointer". You can do that in your CSS file as well

  Hope my suggestions will prove to be useful.

  Good job on completing this challenge and looking forward to seeing more of your projects here on Frontend Mentor!

  Marked as helpful

  1

  ---

  

  Nishant Sawant•70

  @Nishant-afkPosted almost 3 years ago

  @Remus432 Thank you for taking time and seeing my code and solution. Acc to your suggestion i will do make changes, As you said in 3rd point, i am doing the hover effect in css but it stops working when i add an eventlistener to change its color "onclick". Also thanks for giving me info about JSON fetch_api and template literal

  0
• 

  Ahmad Alqaisi•940

  @alQaisiPosted almost 3 years ago

  Hi Nishant, Great work but I have some advices to you First don't use em/rem units for padding or margin and I suggest to decrease the container's padding value or you can give the container 90% width of the whole page and then give it margin right and left by auto so the container would centered in the page. The second thing you should use media query so you can change the layout or the display style when the size of the page breaks some point, as I see in your solution things are getting messy after the page document width become less than 1130px, so you can make a media query when width become less than that size and then you can modify the grid layout to make it more suitable or you could change the cards size. I did not understand your second point but you don't need to have an event listener to change the hover style, and for changing the active text color you can define a css class with the color you want and when you click that text you can toggle or add that class to it. last thing I noticed that you did not implement data fetching in the right way, you can use fetch function and you can read more about fetching data from json file by clicking here

  Marked as helpful

  1