search

codeforokc / openbudgetokc

Visualizations of OKC's budget data, and explanations about the budget process.
https://openbudgetokc.com
12 stars 42 forks source link

Issue #41: Budget Spending Per Citizen #137

Open brentlightsey opened 7 years ago

brentlightsey commented 7 years ago

This adds a visualization page to show spending per citizen. The layout is inspired by a New York Times data viz, mentioned in Issue #41 . budget-per-capita-thumb

The changes include adding the thumbnail to the visualizations page and updating the contributors page.

Link to fork: https://github.com/brentlightsey/openbudgetokc

brentlightsey commented 7 years ago

Looks like the Travis CI failed because the javascript file I've added isn't node-based. Probably should be. Let me know and I can resubmit if needed.

smurphy8 commented 7 years ago

Looks like it is breaking the tests. Please try and repair.

brentlightsey commented 7 years ago

Will do.

brentlightsey commented 7 years ago

OK, it took me multiple tries to get it right (my first PR), but the tests are passing now. There was an assumed javascript dependency that I wasn't checking for.

I was attempting to run npm test locally, but was not successful. Would definitely appreciate any suggestions you have about how to check the commit prior to doing a PR.

smurphy8 commented 7 years ago

@brentlightsey visually I think this looks awesome. But I did find a glitch when I looked at it in responsive mode. Looks like the text bleeds together a bit sometimes. responsive_bug

Also, how are you doing the division, that is what population figure is the data based on?

Sorry for the trouble with npm testing. Can you tell me more about the errors you were having running it?

I usually use yarn test and it works very well.

brentlightsey commented 7 years ago

Thanks for finding that bug @smurphy8. I had an external dependency on a javascript file that wasn't coming through, so nothing was happening on window resize. That should be fixed with the latest update to the branch. Let me know if it works for you.

I'm using the 2017 metro population of 1,358,452. But I totally agree that we should list that on the page somewhere (I think it's based off of a 2015 estimate done by the US Census Bureau, but it's not as accurate as the true census.) Should I add a note to the top of the page?