search

mks-greenfield / trendr

"don't create trends, follow them"
2 stars 5 forks source link

[feat] Add d3 display for trends in SF and Chicago that are retrieved… #28

Closed SimonDing87 closed 8 years ago

SimonDing87 commented 8 years ago

… from mongolab DB

polinabythebay commented 8 years ago

:+1: love the pie charts! :pineapple: :pineapple:

Just as an fyi you are getting a d3 pie error-- not sure if you want to address that or let it be for now.

screen shot 2015-12-29 at 12 32 39 am

Other than that, I think the only thing is putting your mongoose code into a separate file would be a good idea before merging to keep our concerns clean (definitely something to work on tomorrow, don't worry about doing it tonight).

We can take a look at this PR tomorrow and then I'm hoping we can deploy what we have by the end of tomorrow. :dancer:

kcvan commented 8 years ago

hihi! yeah we couldn't figure out how to refresh the charts on click, they keep stacking on top of each other and...doubling the numbers HAHA. pain.

SimonDing87 commented 8 years ago

hey Polina, Kevin and I worked some more on this. We separated the api call into its own file (cityTrends.js) and took it out of server.js. Hope all looks good!

polinabythebay commented 8 years ago

I'll take a look in a bit! :)

polinabythebay commented 8 years ago

Aside from rouge semi colon, looks great! When you mentioned the double data issue, is this what you were referring to? This is what happened when I selected San Francisco:

screen shot 2016-01-02 at 5 23 44 pm
kcvan commented 8 years ago

that's really weird! we make it do the div destroyed itself when calling it again. i was working on ours :x

SimonDing87 commented 8 years ago

I fixed that semi colon error and refactored a bit more. That's really strange that you're still getting dupe data. Did you click it multiple times for it to show like that, or was it first click? I'll have to look into it more.

and just to note we actually don't destroy the div -- we select the div and empty out all of its contents via $('#pieChart').empty()

SimonDing87 commented 8 years ago

Just a quick note for my last commit: it's pretty useful to run "grunt jshint" to catch some missing semicolons and stuff

Sorry this PR is turning out to be a huge beast lol

polinabythebay commented 8 years ago

I'll download the PR again and check to see what I did to make it look like that, not sure if I did something extra.

polinabythebay commented 8 years ago

Hey @SimonDing87 can you pull in upstream development changes?

SimonDing87 commented 8 years ago

oh ya sure. There are a few conflicts let me resolve them before i push

SimonDing87 commented 8 years ago

Okay! I thinkkkkk that did the trick. Please let me know if there are any more issues

polinabythebay commented 8 years ago

@SimonDing87 still merge conflicts :( lmk if i can help

SimonDing87 commented 8 years ago

Okay finally I think I figured out how to resolve the conflicts. I was trying to --rebase the entire time and eventually gave up and just did a normal pull lol. I resolved a few things in bower.json, index.html, and your stackedAreaChartController.js

Hope it all works out now

polinabythebay commented 8 years ago

sweet! merging now