-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cleanup the single bar component #39
Conversation
✅ Deploy Preview for state-decarbonize ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Hey @derekeder, was just taking a look at this out of curiosity, one more peculiarity of the
// helpers
const getPct = (value, total) => { return Math.round((value/total)*100) }
const getLabel = (
entry,
total,
field,
label
) => {
return `${label}: ${getPct(entry[field], total)}%`
}
function SingleBarChart ({emissions_data}) {
// sum all emissions fields except year
const totalEmissions = Object.entries(emissions_data[0])
.filter((key) => key !== 'year')
.reduce((acc, curr) => acc + curr[1], 0);
// ... some lines later
<Bar dataKey="dumps_farms_industrial_other" stackId="a" fill="#98886c">
<LabelList
valueAccessor={entry => getLabel(
entry,
totalEmissions,
'dumps_farms_industrial_other',
'Dumps, Farms, Industrial & Other'
)}
position="insideTopLeft"
/>
</Bar>
} |
@nofurtherinformation nice! one question for the bar in your screenshot, why do the totals add up to less than 100%? (20 + 7 + 23 + 28 = 78%) |
🤦♀️ My bad on that one, the filter statement wasn’t destructuring the keys properly, so wasn’t removing the year data. Will push a commit here once I’m back home
…Sent from my iPhone
On Mar 23, 2022, at 5:03 PM, Derek Eder ***@***.***> wrote:
@nofurtherinformation nice! one question for the bar in your screenshot, why do the totals add up to less than 100%? (20 + 7 + 23 + 28 = 78%)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|
@derekeder - this look solid but I don't think it actually closes #17, since that also includes a drawdown graph showing emissions going down to 0. I think this does close Issue #29. Also could we move the axis closer to the actual data? It's really hard to read in my mind since it's so far away. However, if the specific Gigatonne values don't matter (which I am open to, since it's basically for showing percentages), we could just drop the axis and the label. |
@nofurtherinformation nice - this looks correct now. Thanks for the assist @vkoves I agree the YAxis is a bit far away. It doesn't give us much value so I removed it. Also updated the issue description so it closes #18, which was mistakenly closed from the previous PR #36. This is ready for another look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work! There's a few unused variables and imports GitHub is pointing out, but I don't think that's a blocker - I'd rather get this merged so I can integrate it on the state details page and start figuring out how to highlight individual sections as green.
@vkoves cool. I removed those references. Once it builds I'll bring this in. |
Overview
Small PR to clean up the code from #36 and place it in the proper place on the home page
Closes #18
Demo
Testing Instructions