Dashboard wallet worth chart #141
Conversation
|
What I don't like though is that our type checker finds many type errors in the package... Maybe I should look for alternatives. |
nop33
changed the title
|
I replaced the Ready for review! |
| const trimInitialZeroDataPoints = (data: DataPoint[]) => data.slice(data.findIndex((point) => point.worth !== 0)) | ||
|
|
||
| let dataPoints = computeChartDataPoints() | ||
| dataPoints = trimInitialZeroDataPoints(dataPoints) |
There was a problem hiding this comment.
Nit:
const dataPoints = trimInitialZeroDataPoints(computeChartDataPoints())Also, computeChartDataPoints is only used once and doesn't need to be a function?
There was a problem hiding this comment.
Both computeChartDataPoints and trimInitialZeroDataPoints are called only once. The reason I made them functions is to use the function name as a hint to the developer of what is going on there so that I don't have to add a comment. I think it improves code readability.
There was a problem hiding this comment.
Yes, I agree that declaring trimInitialZeroDataPoints makes it more readable. IMO chartDataPoints could be a variable, but I'm 💯% fine if we keep a function for this.
There was a problem hiding this comment.
Yes! Just be careful, you have a styled component hidden in between two utils functions!
| <Stop offset="100%" stopColor={theme.bg.back1} /> | ||
| </LinearGradient> | ||
| </Defs> | ||
| <VictoryArea |
There was a problem hiding this comment.
👍 I like how lightweight the API is.
Closes #70