Expenses Chart Component using Vanilla JS and CSS

Community feedback

• 

  Arsh Goyal•940

  @arshGoyalDevPosted over 2 years ago

  You have done it great

  • It seems like your solution could work but you could use "%" instead of pixels.
  • And to fix some of the accessibility issues you should either remove all of the ids you added in amount div or you could add them like id="monday amount", id="tuesday amount" and more

  1

  ---

  

  Rushabh Javeri•20

  @rushabhherePosted over 2 years ago

  @arshWebDev Thanks for taking the time to provide feedback, I appreciate it. I have fixed the accessibility issues, however, I feel like your suggestion to use % is a bad idea. If there is a scenario where the expenditure in a day is more than $100, the height of the bar will shoot out of the container...

  1

  

  Arsh Goyal•940

  @arshGoyalDevPosted over 2 years ago

  @rushabhhere No, no that doesn't happen if we add height: 100% to the container

  0
• 

  Devmor•470

  @devmor-jPosted over 2 years ago

  Nice job 🎉 looks good and almost identical though I guess that cyan bar is supposed to be the max spending bar as shown in the design specs. Still not sure if this works but in your css file changing .day.today to sth like .day.max might be the fix (needs some js refactor as well):

  .day.max .bar {
    background-color: var(--clr-cyan);
  }
  

  then in your js file:

  // try to come up with a logic here:
  if (/* this bar is the max then add 'max' class to it */) {
      days[index].classList.add('max');
    }
  

  Mobile and zooming and all works. Fonts and sizings are also great. Will be a great solution. Keep it up bro 😉

  0

  ---

  

  Rushabh Javeri•20

  @rushabhherePosted over 2 years ago

  @devmor-j thanks for your review. The instructions were to have the current day's bar be cyan.

  0

  

  Devmor•470

  @devmor-jPosted over 2 years ago

  @rushabhhere Thanks, oh my apoligies then if that is like that.

  0