Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure csv/xls mimetypes are present #26

Merged
merged 1 commit into from
Oct 25, 2018

Conversation

eleland
Copy link
Contributor

@eleland eleland commented Dec 4, 2015

I was having an issue where the filenames for CSV and XLS files were
being downloaded as the full text of the USE_TITLE_FOR_FILENAME
function. This commit ensures that the xls and csv mime types will be
included regardless of the specific text in the csv/xls download
buttons.

Thanks for writing a great and extremely useful plugin for this!

I was having an issue where the filenames for CSV and XLS files were
being downloaded as the full text of the USE_TITLE_FOR_FILENAME
function. This commit ensures that the xls and csv mime types will be
included regardless of the specific text in the csv/xls download
buttons.
@eleland
Copy link
Contributor Author

eleland commented Dec 5, 2015

As a follow-up here, it seems like the "downloadCSV"/"downloadXLS" strings are used in the export-csv.js library, and this PR ensures that these libs play nicely out of the box.
https://github.com/highcharts/export-csv/blob/master/export-csv.js#L290

@A----
Copy link
Owner

A---- commented Dec 5, 2015

Hi!

Glad it is useful!
I don't understand how you could end up with the full-text function for filename or how your fix does change this. I may have overlooked something, though.

The TRANSLATION_KEY_TO_MIME_TYPES constants are for changing the behavior of the menu items, so that they use exportChartLocal(…) instead of the exportChart provided by either exporting or export-csv official modules.

Then there is this line: https://github.com/A----/highcharts-export-clientside/blob/master/highcharts-export-clientside.js#L46
With which you should end up in the same state as with your fix, as long as you included the dependencies before this plugin. Which you probably did or it would fuck up elsewhere.

The reason for that strange key addition to TRANSLATION_KEY_TO_MIME_TYPES is that, at one time, there was no textKey (i18n support, downloadCSV) defined by export-csv.

Can you work out a JSFiddle with your issue?

@eleland
Copy link
Contributor Author

eleland commented Dec 7, 2015

Sure, here is a jsfiddle I based off the one from this export lib's homepage:
http://jsfiddle.net/tzjzpj3f/
The only changes I made were to remove the programmatic buttons and add this highcharts plot config attribute:

  exporting: {
    filename: Highcharts.exporting.USE_TITLE_FOR_FILENAME
  },

When I download a CSV of XLS from there, I get the filename
function (options, chartOptions) {- var title = this.title - this.title.textStr.replace(- -g, '-').toLowerCase() - 'chart';- return title;- }

Thanks!

@A----
Copy link
Owner

A---- commented Dec 7, 2015

Huh, yeah. I get it now :)

@A---- A---- merged commit 2fa7598 into A----:master Oct 25, 2018
@eleland eleland deleted the csv-xls-filenames-bugfix branch October 25, 2018 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants