time tracker

Community feedback

• 

  P

  Remus D. Buhaianu•3,125

  @MiculinoPosted almost 3 years ago

  Hey @efs0-cod3

  Good job on completing the challenge. It looks pretty good and the main feature works as expected. A few suggestions based on your final solution:

  • .cont__info doesn't fully cover the background color of the parent element around the edges. You can see a little bit of the color

  • Responsive design needs some adjustments. You should implement the media query sooner and also make sure everything is vertically and horizontally centered

  • Try to avoid fixed px based width and heights values. It's a bad practice when trying to build responsive layouts. Build your layout around the available content and try to use relative units such as rem, em, vh / vw, %

  • You already have all the data inside the data.json file, you don't need to write it again in your JS file

  • Avoid using innerHTML property. It's not a safe option as it can lead to security risks. The appendChild() method combined with DOM Nodes is a much better approach to this issue

  • Use const instead of let when declaring a function expression

  Hope this helps. Keep up the good work!

  Marked as helpful

  1

  ---

  

  Erick F. Felix•265

  @efs0-cod3Posted almost 3 years ago

  @Remus432 Thanks for your time and your point will do the fix. Thank you again!

  1

  

  P

  Remus D. Buhaianu•3,125

  @MiculinoPosted almost 3 years ago

  @efs0-cod3 happy to help! ;)

  Let me know how it goes!

  1

  

  Erick F. Felix•265

  @efs0-cod3Posted almost 3 years ago

  @Remus432 hey! I could improve a lot of elements, but I couldn't make it with <appendChild()>. I'll be glad if you point me in the right direction. Again, thanks for your feedback. Hope all well on your end!

  0