Closed danielkrizian closed 10 years ago
Canvas Annotations Plugin was created not as a part of core Dygraph plugins, but as a separate pluggable module in extra, that's why we add it in the separate SCRIPT tag (https://github.com/danielkrizian/dygraphs/blob/canvas-annotations/tests/canvas-annotation-synchronize-series.html#L15) and define it in Dygraph constructor configuration (https://github.com/danielkrizian/dygraphs/blob/canvas-annotations/tests/canvas-annotation-synchronize-series.html#L571), that's why it would be missing in dygraph-combined.js
Probably, it makes sense to move Canvas Annotations Plugin to core plugins. What do you think?
Moving Canvas Annotations to core plugins would be nice, but we have to take into account possible objection by danvk, who might insist that it stays in extras.
In any case, the goal is to be able to download & source the latest dev version with new extras from a compact minified single file saved locally for speed, like http://dygraphs.com/download.html. Right now my application has to source via http://rawgit.com/danielkrizian/dygraphs/master/dygraph-dev.js , which takes time.
dygraph-combined.js
from http://dygraphs.com/download.html seems to include extras
folders too (as cursory search in http://dygraphs.com/1.0.1/dygraph-combined.js seems to reveal)
Does this work for you? (see instructions http://dygraphs.com/download.html)
git clone https://github.com/danvk/dygraphs.git
./generate-combined.sh
Ultimately, if we manage to compile right contents into dygraph-combined.js
, this should work alone (benefits of a single interface):
<script type="text/javascript" src="localdowloadpath/dygraph-combined.js"></script>
Since we're running our own fork, I suppose there's no strict need for danvk's approval. We send pull requests upstream and we have a sane repository structure to easily merge upstream changes. So, let's just do what's valuable for our use cases and implement the necessary features no matter if it would be accepted upstream or not. This is GitHub and Open Source which give us the tools and the philosophy. If our changes would be accepted upstream or commented or even improved -- that's good, if not -- that's the features we need and are going to continue using. That's it. That's my opinion.
I've investigated generate-combined.sh
script -- it doesn't include extras
into the minified output: https://github.com/danielkrizian/dygraphs/blob/master/generate-combined.sh#L8:L26
We could either change generate-combined.sh
script to include extras
into the output, or move Canvas Annotations to core plugins, but in this case we would still need to separately include SCRIPT tag with extras/shapes.js
.
I agree, was just trying to anticipate upstream philosophy, hoping that after we are finished with our enhancements, they would hopefully all be accepted upstream, thus no need to maintain our branch in sync with upstream. Best scenario.
Strange, I searched for keyword 'TRIANGLE' in http://dygraphs.com/dygraph-combined.js and it was there, so I assumed extras/shapes.js
code was included as well. 'TRIANGLE' seems to be only part of 'extras/shapes.js' : https://github.com/danvk/dygraphs/search?q=TRIANGLE
Maybe generate-combined.ph
parses shapes.js
indirectly?
Anyway, I would vote for test running generate-combine.sh
to have fresh dygraph-combined.js
. It is already written as to compile all the plugins anyway, see https://github.com/danielkrizian/dygraphs/blob/master/generate-combined.sh#L18
As I mentioned, I would prefer to have just a single SCRIPT tag in the final HTML, i.e.
<script type="text/javascript" src="localdowloadpath/dygraph-combined.js"></script>
as it streamlines dependencies into one centralized place. My application has to generate these tags dynamically, so the least number of volatile parts the better.
Probably, the combined version on site was done other way than it's described on the download page. If I run generate-combine.sh
on danvk/dygraphs
master branch there are no shapes
in the output:
$ git clone https://github.com/danvk/dygraphs.git && cd dygraphs
$ grep -q TRIANGLE dygraph-combined.js && echo 'Found' || echo 'Not found'
Not found
I've just filed issue https://code.google.com/p/dygraphs/issues/detail?id=516
As soon as we consider Canvas Annotations Plugin as done, I'm going to merge the plugin into the master
branch and update generate-combine.sh
script file to have all the necessary plugins and also extras/shapes.js
concatenated into the dygraph-combined.js
.
Perfect, I think the Canvas Annotations Plugin is done now, no? The main test is working and if the edge cases we can't think of at the moment may surface any bugs over time, I assume it is alright to fix them in the master
. Let's proceed with dygraph-combined.js
production and the other two outstanding items.
Yes, I'm on it!
By the way, what OS do you use?
Windows 7, amateur here :)
Oh... So, we have at least three variants here:
dygraph-combined.js
for you (every time I push to master
for example. It could be done with git hook) and host it somewhereLazy amateur here :)
Could you #1
on major (not all) changes to master
and host it somewhere from GitHub (perhaps github pages) ?
(eventually sometime in the long-term, I figured I need to get my hands dirty with Cygwin
. Linux in next life, guess it's too late :)
Many thanks!
Here updated versions are deployed: http://danielkrizian.github.io/dygraphs/
Thanks Petr, I understand that this automatically runs on each commit! Brilliant
Hi Petr,
can you please post a step-by-step guide for javascript beginners like me how to produce a minified version of the most recent code that we have (incl. canvas annotations and crosshair branches) This one is outdated: https://github.com/danielkrizian/dygraphs/blob/canvas-annotations/dygraph-combined.js
Help much appreciated! Daniel