Expenses chart component

Community feedback

• 

  P

  UreaLaden•240

  @UreaLadenPosted almost 2 years ago

  Overall I think your solutions pretty good for a start and it is fairly easy to understand. The biggest issue is that this solution is not reusable. You're hardcoding a lot of strings and values so this can't be used in any other context. There's quite a bit of repeat code which should tell you that you may need to include some functions or do some refactoring. Your repo contains the data.json but you copied your own. Why not just read from the json file and use that instead? I love your use of css variables. One last note and this is something i'm constantly struggling with. All of your positioning and sizing may be pixel perfect but you'll want to start considering different viewport sizes. While it may look decent on your screen, it'll probably look strange on another. Great job!

  Marked as helpful

  1

  ---

  

  Tommy Miguel•300

  @CodingTimmyethPosted almost 2 years ago

  @UreaLaden Thank you, I appreciate the feedback! I understand that I have to refactor my JavaScript code and thin them out. I realized after submitting the project that I could fetch the JSON data using the fetch() method, so I will use that instead of pasting the data into my actual project. I will look into the viewport sizing. Do you have a youtube video or article where I can learn viewport sizing? Thanks again!

  0