Visualization Feedback - Githubissues

Hey Vivian,

Nice work on the visualization. It's great to see multiple graphs, and it looks like your tooltip is working well.

I've broken out feedback into two pieces: feedback on the end product, and code review.

End Product

In the digital bar chart, it's not clear how the bars are sorted. For the most part it seems like they're sorted by sales, but not entirely. For example, Lady Gaga's Bad Romance sold 15 million copies, but is wedged in between two songs that sold 12 and 11.7 million songs. (This doesn't seem to be an issue for the physical bar chart.)
In your scatterplots, it looks like the size of the dot corresponds to the number of sales. It's a nice effect, but the visualization is essentially duplicating the sales data in two places: the size of the dot and the height of the dot in the chart. Typically when the sizes change, they reflect some third dimension that you're trying to visualize (maybe song duration?). Might be hard to do in your case, since I know you've scraped year and sales data only, but if you can think of a third dimension you'd like to visualize and that you can get the data for, it might be good to add in to the scatterplot data.

Code Review

In general, it's preferable to remove commented out code rather than leaving it in the repo. Since Git tracks all your changes, if that code exists in a previous commit, you can always get back to it by looking through your history. Some examples of commented-out code that you might want to remove:

https://github.com/v-lai/popular_music/blob/master/app.js#L1-L7 https://github.com/v-lai/popular_music/blob/master/app.js#L69-L75 https://github.com/v-lai/popular_music/blob/master/app.js#L214

Nice use of two callbacks inside of d3.csv to format your data before manipulating it!
On this line (https://github.com/v-lai/popular_music/blob/master/app.js#L57) you can remove the parentheses around d since your arrow function only takes one argument.
Small issue, but your indentation doesn't quite abide by the D3 conventions here: https://github.com/v-lai/popular_music/blob/master/app.js#L77-L98 - methods like append, which return a new selection, should be indented two spaces. Methods like style, which return the existing selection, should be indented four spaces.
For these lines (https://github.com/v-lai/popular_music/blob/master/app.js#L111-L113), you could do a small refactor if your intention is to have both graph types have the same dimensions and same margin. Right now there's a little bit of code duplication here. In general, if you see duplication between scatterPlot and barPlot, see if there are any quick refactors you can do to reduce the amount of code you've got.
One nice thing about template strings is that you can write multi-line strings in them. This lets you write slightly more readable html when you're creating your tooltip. E.g. on line https://github.com/v-lai/popular_music/blob/master/app.js#L177 you could do something like:

div.html(`
  <strong>Artist:</strong> <span style='color:red'>${d.artist}</span><br>
  <strong>Song:</strong> <span style='color:red'>${d.single}</span><br>
  <strong>Year:</strong> <span style='color:red'>${d.released}</span><br>
  <strong>Sales (mil):</strong> <span style='color:red'>${d.sales}</span><br>
`)

Also, I'd advocate for using something like a p tag rather than using spans and br tags. I try to avoid using br tags, since it mixes styling with markup; rules for how the text looks should be in CSS, not in HTML.

Again, nice work!

v-lai / popular_music

Visualization Feedback #1