Skip to content
This repository was archived by the owner on May 29, 2018. It is now read-only.

added exporting.csv.url option#12

Closed
raveren wants to merge 3 commits intohighcharts:masterfrom
raveren:master
Closed

added exporting.csv.url option#12
raveren wants to merge 3 commits intohighcharts:masterfrom
raveren:master

Conversation

@raveren
Copy link
Copy Markdown

@raveren raveren commented Apr 11, 2014

no more privacy concerns and

https://raw.github.com/highslide-software/highcharts.com/master/studies/csv-export/csv.php

says

/**
 * DISCLAIMER: Don't use www.highcharts.com/studies/csv-export/csv.php in 
 * production! This file may be removed at any time.
 */

Comment thread export-csv.js Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style question -- would it be better to explicitly pass false as the parameter, rather than relying on the fact that unpassed parameters default to undefined, which is evaluated as false?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really sure, since JS does not offer default parameter values by default, I usually go this way, but it's just a question of consistency in the whole codebase actually (which I admit didn't check).

@dsjoerg
Copy link
Copy Markdown

dsjoerg commented Apr 11, 2014

Sorry, that last comment was from me, not @enggine.

Also, do I correctly understand a second feature of this PR, aside from permitting the url to be set, is the ability to call getCSV() on a Chart and then directly get the csv data, which one could then do something else with in javascript?

@raveren
Copy link
Copy Markdown
Author

raveren commented Apr 11, 2014

Correct, but this was the functionality before my changes so I made effort to keep it that way.

@dsjoerg
Copy link
Copy Markdown

dsjoerg commented Apr 11, 2014

Ah @raveren I see now, thanks.

raveren added 2 commits April 14, 2014 15:27
* y-values for same x-values from different series are placed in same row
* x-values no longer duplicated as columns for each series
* X column title and downloaded file title is taken from user provided values when configuring the chart if available.
* Browser-side download is used if supported by browser bypassing server-side script
* Default title of chart is passed to backend along with csv string.

known issues: multiple x axes (no longer?) supported
@raveren
Copy link
Copy Markdown
Author

raveren commented Apr 14, 2014

Ok, that is not what I intended, I wanted a separate pull request, as the two latter commits introduce potentially breaking changes which I only tested in my own production setting.

It used to generate CSV with x-axes repeated as many times as there were series, which is illogical:

clipboard01

so I set out to fix it, please let me know what you think and whether I can improve anything not to your liking.

@infacq
Copy link
Copy Markdown

infacq commented Sep 21, 2014

1+

@A----
Copy link
Copy Markdown
Contributor

A---- commented Mar 18, 2015

This one can be closed since #40 has been merged.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants