epochjs / epoch

A general purpose, real-time visualization library.
http://epochjs.github.io/epoch
MIT License
4.97k stars 278 forks source link

Needs Updated Dependencies #145

Closed mattdodge closed 9 years ago

mattdodge commented 9 years ago

The homepage of this package lists only d3 as a dependency. However, every example seems to use jQuery. I see that Issue #62 seems to be complete, but I am unable to get it completely working with only d3. It looks as though an adapter (jQuery, MooTools, zepto) is needed.

One example:

new Epoch.Time.Line({el: '#graph', data: []})

If I try to declare a real-time line chart (without including width and height), I see failures due to this line: https://github.com/fastly/epoch/blob/master/src/core/chart.coffee#L41 This is because d3 selectors do not have a width function like jQuery does. Doing this prevents the issue, but I would like to be able to use the auto width/height functionality.

new Epoch.Time.Line({el: '#graph', data: [], width: 300, height: 300})

This may just be a documentation issue, or it may be a bug, depending on the goal of the library. If the goal is to only have d3 as a dependency, there are some spots where the code needs fixing. If the goal is to have one of the provided adapters be a dependency, this probably needs to be reflected in the docs somehow

rsandor commented 9 years ago

Looks like you might be right, I am unsure how the unit tests missed this but they will need to be updated. @mattdodge - Would you mind perhaps digging in, fixing the tests, and the core implementation? It would be a great help if you could open a PR that addresses the issues you've found.

rsandor commented 9 years ago

No idea if this is still an issue. If anyone is seeing the same issue, please report it here.

rsandor commented 9 years ago

I am unable to reproduce this. Works fine without the width and height options. I will see if this gets reported again, but closing for now.

mattdodge commented 9 years ago

It's been a while since I've looked at this, but pulling down the latest does seem to prevent any JS errors from popping up. I see there are width and height functions added to the d3.selection prototype, but those functions also appear to have been there when the issue was filed? I'm not seeing any errors either when running off of commit 91558a1 though, which seems to be the commit I would have been using when filing the original issue.

I haven't been using the library, so don't know how much I can help. But for what it's worth, I am still unable to get a chart to appear using the d3 syntax. The examples on the docs are still all jQuery dependent.

When running this:

new Epoch.Time.Line({el: '#graph', data: data});

I just see an empty chart like so:

screenshot 2015-10-05 11 35 14

But the documented jQuery syntax:

$('#graph').epoch({type:'time.line', data: data});

makes the chart render properly (or at least I see something):

screenshot 2015-10-05 11 36 00

I'm fine with this being closed since it's been so long. But figured you might want to be aware of what I was seeing when using only d3 (no jQuery). Let me know if I can help — I can set up a fiddle or plunkr if needed.

rsandor commented 9 years ago

Thanks is for following up on this, I was able to get a d3 only example working last night. I'll reopen this and link to a gist for you to try when I get home tonight. Hopefully we can figure out what is going on here :) On Mon, Oct 5, 2015 at 11:38 AM Matt Dodge notifications@github.com wrote:

It's been a while since I've looked at this, but pulling down the latest does seem to prevent any JS errors from popping up. I see there are width and height functions added to the d3.selection prototype, but those functions also appear to have been there when the issue was filed? I'm not seeing any errors either when running off of commit 91558a1 https://github.com/epochjs/epoch/commit/91558a11a2d8da94b8cbbd14ba346bd26667f2ec though, which seems to be the commit I would have been using when filing the original issue.

I haven't been using the library, so don't know how much I can help. But for what it's worth, I am still unable to get a chart to appear using the d3 syntax. The examples on the docs are still all jQuery dependent.

When running this:

new Epoch.Time.Line({el: '#graph', data: data});

I just see an empty chart like so: [image: screenshot 2015-10-05 11 35 14] https://cloud.githubusercontent.com/assets/797259/10289799/320c65fa-6b55-11e5-90d7-b4e731e34133.png

But the documented jQuery syntax:

$('#graph').epoch({type:'time.line', data: data});

makes the chart render properly (or at least I see something): [image: screenshot 2015-10-05 11 36 00] https://cloud.githubusercontent.com/assets/797259/10289820/5587d302-6b55-11e5-99aa-ce6c69e79bab.png

I'm fine with this being closed since it's been so long. But figured you might want to be aware of what I was seeing when using only d3 (no jQuery). Let me know if I can help — I can set up a fiddle or plunkr if needed.

— Reply to this email directly or view it on GitHub https://github.com/epochjs/epoch/issues/145#issuecomment-145625967.

mattdodge commented 9 years ago

Awesome, sounds good!

rsandor commented 9 years ago

Also sorry it took so long to address the issue, the repository was locked in a private org for quite some time. On Mon, Oct 5, 2015 at 12:26 PM Matt Dodge notifications@github.com wrote:

Awesome, sounds good!

— Reply to this email directly or view it on GitHub https://github.com/epochjs/epoch/issues/145#issuecomment-145639922.

rsandor commented 9 years ago

@mattdodge try this example:

<html>
<head>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/d3/3.5.6/d3.min.js"></script>
  <!-- <script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/2.1.4/jquery.min.js"></script> -->
  <script src="dist/js/epoch.js"></script>
  <link rel="stylesheet" type="text/css" href="dist/css/epoch.css">
</head>
<body>

  <div class="epoch" id="event-bar-chart" style="width: 500px; height: 150px"></div>

  <script>
  var chart = new Epoch.Time.Line({
    el: '#event-bar-chart',
    axes: ['right', 'bottom'],
    tickFormats: {
      right: function (d) {
        return '$' + (d / 100000).toFixed(3);
      }
    },
    data: [
      // First series
      {
        label: "Series 1",
        values: [{time: 1370044800, y: 1000000}, {time: 1370044801, y: 1000000}]
      },
      // The second series
      {
        label: "Series 2",
        values: [{time: 1370044800, y: 780000}, {time: 1370044801, y: 98000}]
      }
    ]
  });
  chart.draw();
  </script>
</body>
</html>

This seems to render just fine for me.

rsandor commented 9 years ago

Oh I think I just spotted the issue, you have to call .draw explicitly for some reason right now when you use the classes directly. I am not sure why that it is though.

mattdodge commented 9 years ago

Ah, yep, that did it! chart.draw() to the rescue.

rsandor commented 9 years ago

We should probably have both methods work the same way, can you do me a favor and open a new ticket for this issue? On Tue, Oct 6, 2015 at 11:43 AM Matt Dodge notifications@github.com wrote:

Ah, yep, that did it! chart.draw() to the rescue.

— Reply to this email directly or view it on GitHub https://github.com/epochjs/epoch/issues/145#issuecomment-145960247.

mattdodge commented 9 years ago

No problem, threw it up over at #195

rsandor commented 9 years ago

Thanks man! If you have time to look into it I would appreciate it (super busy with work this week). First place to look would be the src/adapters/jquery.coffee to see what is going on there. Probably would have to update the base chart in src/common/charts.coffee to add the automatic draw call. On Tue, Oct 6, 2015 at 2:11 PM Matt Dodge notifications@github.com wrote:

No problem, threw it up over at #195 https://github.com/epochjs/epoch/issues/195

— Reply to this email directly or view it on GitHub https://github.com/epochjs/epoch/issues/145#issuecomment-146002352.