Animated & Dynamically Rendered Expenses Chart with CSS GRID

Community feedback

• 

  Arseniy•170

  @ArseniyXPosted over 1 year ago

  Hello, looked around your solution, looks not bad, things to improve:

  1. use prettier and es-lint to make the code looks better.
  2. if you use animation library like framer-motion you shouldn't do setTimeout to animation its supposed to be build in with the library you just need to find the right way to do it.
  3. you not use full potential of react, you need to split big files to small components
  4. This function:

  const findHigh = () => {
        let max = 0, indx = 0;
        data.map((item)=>{
          return item.amount
        }).forEach((item, index)=>{
          if(item > max) {
              max = item 
              indx = index
          }
        })
        return indx       
      }
  

  can be called findHighIndex and you can achieve that with:

  const myArray = [5,6,7,2,8,0]
  const result = myArray.indexOf(Math.max(...myArray));
  console.log(result)
  

  Marked as helpful

  1

  ---

  

  P

  visualdennis•8,375

  @visualdennissPosted over 1 year ago

  @ArseniyX Hi,

  thanks for taking your time for the feedback and the suggestions. Much appreciated

  1. I have both installed and use to use but i had updated some stuff that got mess with them so i need to configure them sometime again, thnx for reminding.
  2. The setTimeOut is not used with any framer-motion animations, it is just added to the counter animation which is done in pure vanilla JS. It is not a real-animation i'd say, a pseudo one. setTimeOut is set to match framer-motion animation timelines. The ones using framer-motion are configured by its built-in props such as transition, delay and duration etc. already.
  3. I agree and disagree on that one. I think Reacts potential is more about reusability of the components, - so you can reuse them with props as the app gets more and more complex - rather than splitting code for the sake of splitting code. For a small project like this, i find it is not so necessary to split header and chart and chart container into its own components, as i don't want to end up with more files, as the repo has already have many. For small components like this, it actually helps sometimes read faster and control quicker if they are on the same file. I agree the helper functions could split into utils.js etc. But eventually that's a matter of preference. I usually do splitting if it gets bigger than this or more complex like i've done in Static Jobs Filter Challenge. Also even Reacts own examples on docs don't split everything into new files, sometimes even a file contains few small components.
  4. Oh, thats a useful one! I forgot about the built in max method, which is pretty concise. Thanks, i'll make use of that next time.

  0

  

  P

  visualdennis•8,375

  @visualdennissPosted over 1 year ago

  @ArseniyX btw here is a nice article i just came across about code splitting :) https://kentcdodds.com/blog/when-to-break-up-a-component-into-multiple-components

  0

  

  Arseniy•170

  @ArseniyXPosted over 1 year ago

  @visualdenniss I appreciate it, but years of days and nights of experience with react tells me more than such topics.

  1