Expenses chart component

Community feedback

• 

  Elaine•11,400

  @elaineleungPosted over 2 years ago

  Hi Paweł, well done on this challenge! I think you did an excellent job with your forEach methods, and even if it took a while to figure it all out, I'm sure these are skills that will take you far.

  I see you using a switch statement for applying the style to the current day bar; most of the code in the cases are actually fairly similar, so you can consider how to make it "DRY-er". Here's what I did instead in my code to condense all that into a ternary operation:

  const currentDay = new Date().getDay()
  
  data.forEach( (info, idx) => {
     ...
     idx !== 6   // this means if the data is not for Sunday, whose idx is 6
        ? idx + 1 === currentDay && barDiv.classList.add('highlight')
        : currentDay === 0 && barDiv.classList.add('highlight')
  }
  

  One final suggestion I have is to add a try... catch for your fetch method in async/await just to catch errors in case there are any.

  Keep leveling up on that JavaScript! 😊

  Marked as helpful

  0

  ---

  

  Paweł•1,030

  @Pawel-GnatPosted over 2 years ago

  @elaineleung Wow, thank you for priceless advice and kind words! :) I didn't thought I could use forEach to add my 'active' class. I'm not that experienced yet :) but should I add my classList.remove there? If I add 'active' class, then I should remove it later to toggle it in next day. Am I right?

  I know that try/catch function (I used it in my advice generator app) and I wanted to try another method here :)

  Thank you so much <3

  0

  

  Elaine•11,400

  @elaineleungPosted over 2 years ago

  @Pawel-Gnat Actually in this case, the highlighted color only goes on the day that is the current day. For example, today is Wednesday, so only the Wednesday bar would need the color. The Thursday bar would naturally get the color on Thursday if all the logic is written correctly, so that's no need at all to remove the class. Honestly I don't even think you need the classList.remove() in your switch statements. You can try removing and see whether there's a difference!

  Try/catch is mainly for error handling, so if you used it in your other challenge, that means you were also using async/await like in this one. If you're looking to try other methods, the other most common one would be just to use .then() and .catch() for promises and error handling.

  Marked as helpful

  0
• 

  Patrick Senchim•50

  @blayzkingPosted over 2 years ago

  Hi, I keep getting this error, 'Access to fetch has been blocked by CORS Policy.' Did you encounter this? And how did you work your way around it?

  0

  ---

  

  Paweł•1,030

  @Pawel-GnatPosted over 2 years ago

  @blayzking I'm sorry but I didnt' know that's even possible here.

  0

  

  Patrick Senchim•50

  @blayzkingPosted about 2 years ago

  @Pawel-Gnat All good. I solved it by running it through a live server

  0