search

keen / keen-js

https://keen.io/ JavaScript SDKs. Track users and visualise the results. Demo http://keen.github.io/keen-dataviz.js/
MIT License
578 stars 143 forks source link

v3.1.0-beta bug report #175

Closed dustinlarimer closed 10 years ago

dustinlarimer commented 10 years ago

Let's track bugs, breaks and cool ideas here.

ruleant commented 10 years ago

Testing the new beta release in my project : http://ruleant.github.io/buildtime-trend/buildtime-trend/keenjs-3.1-beta.html

This is just a list of things I've noticed that are different from using Keen JS 3.0.3, I haven't checked yet if they can be solved with a different config settings, using the new DataViz object or if it's a bug :

dustinlarimer commented 10 years ago

@ruleant

  1. width takes an integer, and "px" is appended in the metric view render method.. try dropping the "px" from your config
  2. see the checked item in my first post, I have a fix for that in the fix/beta-bugs branch.. this will occur in the metric view render method by default, and you will be able to set { prettyNumber: false } to disable this.
  3. can you elaborate on this?
  4. it's best to wrap the charts with a framework of some sort. we've begun a repo of dashboard examples and layouts that I think you might dig: https://github.com/keenlabs/dashboards
  5. the data transformer doesn't know how to interpret that data model.. if it was working before, I assure you it was a fluke :D. Kudos for the mergeSeries method, that's next on my plate! In the meantime, try to mimic a grouped interval response: https://github.com/keenlabs/keen-js/blob/master/docs/recipes.md#combine-two-line-charts
dustinlarimer commented 10 years ago

Heavy updates to Keen.Dataset will be rolling out shortly. Check it all out here: #178.

ruleant commented 10 years ago
  1. Solved (see above), thanks for noticing.
  2. OK, I'll have a look at it once prettynumber is part of a release. I noticed that suffix isn't added anymore either, did it get replaced by another way of formatting the number? (I vaguely remember something about it, but can't seem to find it in the docs)
  3. I mean a legend with a list of the different series and their correspondig colours. The piecharts still have the legend by default.
  4. Looks cool, it's on my todo-list already : https://github.com/ruleant/buildtime-trend/issues/15
  5. Too bad my solution is a lucky fluke, I was quite pleased that it worked and how the chart looks. I even wrote a blogpost about it. Speaking of which, I was wondering if there would be a way to use the functionality to change query parameters inside the .run() method, that way I can use one query and change the timeframe to get to each series. (reducing duplicate code)

I'm glad you like my mergeSeries method. I started with the grouped interval example but it didn't work or didn't have a result I liked (I don't remember), but I'll look into it again. Can the new dataviz/dataset components be used to merge the queries?

BTW : What is the weather going to be like in SF the next two weeks? I'll be in the Bay Area in one week time.

dustinlarimer commented 10 years ago

@ruleant new update has been pushed to the CDN.. same file, so throw a cache-buster param on the end of your script src.

2) this update is live now as well 3) the chartOptions config may be pushing the legend out of view. Try altering the width property on line 246:

  chartArea: { left: 75, width: 350 }

4) nice :) 5) It actually might work if the timeframe is the first property in each object.. each property becomes a column in the 2D array, the first one being the index column. Keen.Dataset can absolutely handle this.. it will take a little exploring to figure out, but I'll be working on an easier way to do exactly this w/out manual response munging. Check this example: http://jsfiddle.net/keen/cyqLwg7a/

ruleant commented 10 years ago

~~When using the latest file from CDN (with ?v1 as parameter), I get an error in Firebug :

TypeError: client.configure is undefined ...alization:function(){},client._config&&(client.configure.call(client,client._con... (line 1, column 16653)~~

Never mind : when loading it synchronously, it works without a problem.

3) yes that was the problem, I played a bit with the chart size to fully show the series-names in the legend. 5) the example looks nice. Just wondering if it will support non-timestamp data on the X-axis?

dustinlarimer commented 10 years ago

Good catch - the async script loader probably needs to be updated.

Re:5) yep, that should work just fine!

dustinlarimer commented 10 years ago

@ruleant #183 fixes that glitch, will deploy shortly - thanks for catching that!

dok commented 10 years ago

Handle failure is broken in refresh:

screen shot 2014-10-16 at 9 54 02 pm

screen shot 2014-10-16 at 9 52 45 pm

screen shot 2014-10-16 at 9 58 39 pm

Variable res is undefined here when the request fails.

dustinlarimer commented 10 years ago

Previously we've only used XHR for queries unless it wasn't available, so JSONP errors have always been a mess. I'll patch this for 3.1.0-beta.

dustinlarimer commented 10 years ago

@Dokko1230 #185 will resolve this

dustinlarimer commented 10 years ago

@Dokko1230 deployed, may need a cache-buster to see it immediately

dok commented 10 years ago

.draw will manipulate the config object. This is not ideal because a user will want to use the same type of options for multiple charts. We should copy the config object upon initialization.

https://github.com/keenlabs/keen-js/blob/master/src/visualization.js#L14-L19

dustinlarimer commented 10 years ago

:+1: want to include that with the script loading update later today?

dok commented 10 years ago

Yep! Should we do a deep copy or a shallow copy?

dok commented 10 years ago

186 allows shallow copy of the config object. Also, replacing google throw with log in rolled in with it.

dustinlarimer commented 10 years ago

@ruleant @Dokko1230 I think this is nearly ready to go. any lingering issues or suggestions to throw in before we push the big red launch button?