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

add new promise-based toImage and downloadImage to Plotly #446

Merged
merged 7 commits into from May 16, 2016

Conversation

timelyportfolio
Copy link
Contributor

@timelyportfolio timelyportfolio commented Apr 19, 2016

ref original pull timelyportfolio#1 for very long discussion and commits


Addresses


Summary of Changes

  • attach promise-based toImage directly to Plotly
  • attach promise-based downloadImage directly to Plotly
  • preserve existing unofficial Plotly.Snapshot for backward compatibility with exception of adding promise option in svgToImg

Example Usage

Plotly.toImage

var img = document.createElement('img');
document.body.appendChild(img);

Plotly.plot()
    .then(Plotly.toImage)
    .then(function(url){
        img.src = url;
    });

// all the options
Plotly.plot()
    .then(function(gd){
        return Plotly.toImage({
            format: 'png', //also can use 'jpeg', 'webp', 'svg'
            height: 500,
            width: 700
        });
    })
    .then(function(url){
        img.src = url;
    });

Plotly.downloadImage

Plotly.plot()
    .then(Plotly.downloadImage)
    .then(function(filename){
        console.log(filename);
    });

// all the options
Plotly.plot()
    .then(function(gd){
        return Plotly.downloadImage({
            filename: 'myspecialplot',
            format: 'png', //also can use 'jpeg', 'webp', 'svg'
            height: 500,
            width: 700
        });
    })
    .then(function(filename){
        console.log(filename);
    });

@timelyportfolio timelyportfolio changed the title add new promise-based toImage and downloadImage to Plotly add new promise-based toImage and downloadImage to Plotly Apr 19, 2016
opts.format = opts.format || 'png';

if(
(opts.width && isNumeric(opts.width) && opts.width < 1) ||
Copy link
Contributor

@mdtusz mdtusz Apr 19, 2016

Choose a reason for hiding this comment

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

🐄 Let's set these as values above so the if is a bit more clean.

Or define a quick helper function to call on opts.____.

Copy link
Contributor

Choose a reason for hiding this comment

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

and isNumeric(undefined || null) returns false.

So, opts.width && isNumeric(opts.width) is equivalent to isNumeric(opts.width).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but the opts.width and opts.height might be undefined or null, and those are valid options. In the case of null || undefined we use layout height and width.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. I see. 👍 My mistake

* LICENSE file in the root directory of this source tree.
*/

/*eslint dot-notation: [2, {"allowPattern": "^catch$"}]*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove the {"allowKeywords": false} option in the eslintrc file and remove this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, this was just copy/paste.

@etpinard
Copy link
Contributor

@timelyportfolio looking good. This will make a lot folks happy.

You'll need one good linting commit. You can run npm run lint to check for linting mistakes locally.

Let me know if you need any clarifications about #446 (comment)

@timelyportfolio timelyportfolio force-pushed the feature/snapshot-pull branch 2 times, most recently from 0a059ce to 6124b99 Compare April 20, 2016 17:39
@timelyportfolio
Copy link
Contributor Author

@etpinard, how should I handle these eslint errors?

.catch is a syntax error  dot-notation

@etpinard
Copy link
Contributor

@timelyportfolio

how should I handle these eslint errors

We should remove the {"allowKeywords": false} option in the eslintrc file. That it,

"dot-notation": [2, {"allowKeywords": false}]

should become

"dot-notation": [2]

After some research, this only breaks things in ECMAScript version 3. So, let's not worry about it.

@@ -55,17 +55,15 @@ modeBarButtons.toImage = {
return;
}

if(gd._snapshotInProgress) {
Lib.notifier('Snapshotting is still in progress - please hold', 'long');
Copy link
Contributor

Choose a reason for hiding this comment

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

@timelyportfolio why did you review this?

Copy link
Contributor

Choose a reason for hiding this comment

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

@mdtusz
Copy link
Contributor

mdtusz commented Apr 27, 2016

Seems to work just peachy on Safari. 👍

@timelyportfolio timelyportfolio force-pushed the feature/snapshot-pull branch 2 times, most recently from fb6eeea to bc96343 Compare April 28, 2016 01:53
@timelyportfolio
Copy link
Contributor Author

@etpinard, @mdtusz - rebased and squashed into one commit

* By Eli Grey, http://eligrey.com
* License: MIT
* See https://github.com/eligrey/FileSaver.js/blob/master/LICENSE.md
*/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@etpinard, is this ok for attribution?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Nicely done. You'll need to add this block:

/**
 * Copyright 2012-2016, Plotly, Inc.
 * All rights reserved.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

at the top of the file to make npm run test-syntax happy.

@etpinard
Copy link
Contributor

@timelyportfolio @mdtusz I tested out toImage and downloadImage in IE11, looks like toImage is broken in IE11.

First, I discovered a bug in tosvg.js fixed in #489 .

After fixing that bug, svgtoimg.js break before getting to img.onload:

image

Maybe IE11 doesn't like encodeURIComponent ??

@timelyportfolio
Copy link
Contributor Author

@etpinard, and the fun continues :). I have had this error before in an unrelated project, but I cannot find any notes. So, before IE was not supported at all, but it seems we should take the opportunity to get this working in all the browsers. Maybe I write a function for both encodeURIComponent and createObjectURL and pick based on browser. Does that sound like a reasonable approach? I intentionally included the FileSaver bit for IE >10, so that we might be able to get it working.

@mdtusz
Copy link
Contributor

mdtusz commented Apr 28, 2016

It's possible we just need to specify the encoding for it to work in IE 11 as shown here.

@etpinard
Copy link
Contributor

Maybe I write a function for both encodeURIComponent and createObjectURL and pick based on browser. Does that sound like a reasonable approach?

Yes. That sounds good. We have a lib function Lib.IE setup. Feel free to use it.

@timelyportfolio
Copy link
Contributor Author

@mdtusz bringing back good memories, but that just goes back to base64 which is what we were doing with createObjectURL, or am I missing something.

@timelyportfolio
Copy link
Contributor Author

@etpinard, @mdtusz Ok, I think this is ready. Let me know if you want me to rebase and squash. Thanks.

@etpinard
Copy link
Contributor

@timelyportfolio well deserved 🍻

@timelyportfolio
Copy link
Contributor Author

@etpinard and @mdtusz, thanks so much for your patience and help. This was far more difficult than I could have imagined.

@yniknafs
Copy link

@etpinard @timelyportfolio

Hi. Thanks for all your help on here. I've tried using the latest release to save an SVG. I call the downloadImage function, using the "svg" format option.

However, the SVG that is dowloaded can be viewed by chrome, but not illustrator or adobe acrobat. Any thoughts?

I looks like you worked on addressing this directly, but I'm still struggling with it. Really want to be able to download my plotly plots as some form of SVG graphic.

@mdtusz
Copy link
Contributor

mdtusz commented Jun 30, 2016

@yniknafs could you share the data and layout from your example if possible, or better yet, the downloaded svg file for me to inspect? Support with Illustrator is mostly there for most plots, but there are still some edge cases of things that Illustrator's svg parser/renderer doesn't like, and unfortunately, there are very sparse error messages and documentation on what exactly their requirements are - it certainly doesn't adhere to the svg spec.

@yniknafs
Copy link

yniknafs commented Jun 30, 2016

@mdtusz Thanks so much for the reply. We have a number of plots (bar, box, pie) on the site we are developing, and the svg --> illustrator wasn't working with any of the plots.

I have attached the plot. The data is coming from a seql db, can't really paste here, but you can see it in the attached plot.

SVG was downloaded using the modebar (changed modebar code to have "format: 'svg';" in /modebar/buttons.js).

Here is the code to call the plot.

var expression_layout = {
    title: 'TPM vs. Transcripts',
    xaxis: {
        title: 'Transcripts',
        showgrid: false,
        zeroline: false
    },
    yaxis: {
        title: 'TPM',
        showline: false
    },
    boxmode: 'group'
};

function redraw_expression_plot(json_response) {
    var data = [];
    for (var key in json_response) {
        var trace = {
            y: json_response[key][1],
            x: json_response[key][0],
            type: 'box',
            name: key
        }
        data.push(trace);
    }
    expression_layout.yaxis.title = "TPM";
    Plotly.newPlot('expression_plot', data, expression_layout);
    highlight_linear();
    highlight_box();
}

newplot.svg.zip

@mdtusz
Copy link
Contributor

mdtusz commented Jun 30, 2016

@yniknafs using Adobe Illustrator CC, I'm able to open the svg and it looks ok to me. I don't have Acrobat installed however so I can't verify that either.

screen shot 2016-06-30 at 11 56 33 am

The main issue we encountered from before was with respect to opacities and quotation marks, but looking through the SVG neither of those issues seem to be present. What sort of warnings does Illustrator spit out at you when you attempt to open it?

@yniknafs
Copy link

@mdtusz thanks for that. quite interesting. I just download Adobe Illustrator CC and it worked.

I have Adobe CS6 installed. And it did not work (see images). Any chance you know why its not working with the CS version? Will be a little limiting to force users to have CC (I'm sure lots of people using our tool will have some illustrator CS5/6 installed).

image

image

@rreusser
Copy link
Contributor

@yniknafs @mdtusz FWIW, a quick sanity-check with the W3C validator shows the file is 100% valid. A workaround: I was able to open it in Illustrator CS5 by first running it through Inkscape to convert to PDF. For mac/linux (and probably windows too), Inkscape can be invoked from the command line:

$ inkscape /Users/rreusser/Desktop/newplot.svg --export-pdf=/Users/rreusser/Desktop/newplot.pdf

which is nice, though plenty weird that you must pass it a full path to the file. It's not a great solution, but if it's the blocker, it's something.

@a59
Copy link

a59 commented Sep 22, 2016

Please could someone give an example of how to use this with the Python Notebook API.

@etpinard
Copy link
Contributor

@a59 see here

@a59
Copy link

a59 commented Sep 22, 2016

Thanks for the pointer. That's very useful.

However, it doesn't seem to be working quite as advertised. I am on OS X and have tried both Safari and Firefox. I do get a picture which is downloaded to my 'Downloads' folder after the Dialog, but it is given the name 'Unknown' and no extension. I even tried setting the option filename='plot' in the call.

@a59
Copy link

a59 commented Sep 22, 2016

My apologies, it works in Firefox.

ETA. Spoke too soon. The .svg is mangled from Firefox.

@etpinard
Copy link
Contributor

@a59 http://community.plot.ly/ is the best place to look for help.

@amirmog
Copy link

amirmog commented Oct 11, 2016

Hello folks,
I'm still encountering that after I download the image, not all data is exported into the .png file, in a nutshell, I'm drawing bar charts and some of the bars are missing.

Essentially it's the same issue as was originally raised here:

#50
and
#42

I'm not sure if this pull request is supposed to fix this or not, or it's already included in the Plotly CDN?

Any help would be appreciated.

Thanks,

@etpinard
Copy link
Contributor

etpinard commented Oct 11, 2016

@amirmog are you referring to issue #50 ? Ticket #446 is this PR which added a top-level API for to-image exports.

Sharing a reproducible example would help us help you. Thanks.

@amirmog
Copy link

amirmog commented Oct 11, 2016

@etpinard I apologize yes, it was the #50 and #42

I'm using the plotly CDN ( https://cdn.plot.ly/plotly-latest.js )
and when I download the image produced by the default download to image button ("Download plot as a png") in the chart toolbar (modeBar), only some of the bars endup being in the .png file.

This is what the chart looks like:

screen shot 2016-10-11 at 2 02 41 pm

This is what the downloaded png looks like:

newplot 7

Please let me know what other info I can provide.

Thanks.

@vsabeti
Copy link

vsabeti commented Oct 20, 2016

PNG toImage for IE Edge works if img.crossOrigin is set to 'Anonymous'. The tainted canvas is fixed for the Edge browser (and works with this one change), but apparently will not be fixed for IE11. https://connect.microsoft.com/IE/feedback/details/828416/cavas-todataurl-method-doesnt-work-after-draw-svg-file-to-canvas
It would be great if in a future plotlyjs release, toImage png format is allowed for Edge while still restricting IE11 and below to be svg format only.

@etpinard
Copy link
Contributor

@vsabeti thanks for your comments. I'll it over to #699

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants