search

statgen / locuszoom

A Javascript/d3 embeddable plugin for interactively visualizing statistical genetic data from customizable sources.
https://statgen.github.io/locuszoom/
MIT License
154 stars 29 forks source link

Download as image fails for large or complex plots #164

Closed abought closed 5 years ago

abought commented 5 years ago

Reported by @pjvandehaar

Summary

The LocusZoom "download as image" button does not work for extremely large region sizes.

Steps to reproduce

An example page where this occurs is: http://pheweb.sph.umich.edu/SAIGE-UKB/region/008/4:160747832-161147832

(zoom out several times, eg to a region size of 1.2 Mb)

Initial workup

The problem seems to be that links are specified as a data: url, and the link is too long. Other options- notably the Blob interface- may yield better results. This behavior seems to be browser dependent as an implementation detail. https://stackoverflow.com/questions/695151/data-protocol-url-size-limitations

pjvandehaar commented 5 years ago

FileSaver.js should work. It's ~3KB minified, or ~1.5KB if I remove support for downloading URLs.

I'm happy to implement this change. Do you think that adding FileSaver as a dependency is a good approach? Or adding a copy of FileSaver with the non-Blob functionality removed?

abought commented 5 years ago

I'm happy to implement this change. Do you think that adding FileSaver as a dependency is a good approach? Or adding a copy of FileSaver with the non-Blob functionality removed?

That seemed an interesting library! I'll try to look at it tonight/tomorrow and follow up. (I'm also kind of curious about StreamSaver, because there are some clever tricks for larger files and wheeeee)

In general, I think my order of thoughts would be as follows:

  1. Can we do this without a dependency using just blobs and raw JS (I need to experiment a bit!)
  2. If we do add a dependency, will it work without modification? (for a 3kb library, I'd far prefer an extra 1.5kb as opposed to maintaining a fork; that never ends well)
abought commented 5 years ago

The proposed no-library fix seems to work for me in limited local testing, but my browser was acting a bit differently than @pjvandehaar 's. Care to give it a try before I close this ticket?

The fix is on the develop branch, so remember to run npx gulp build first.

Tested in Chrome, FF, and Safari on localhost.

abought commented 5 years ago

Thanks for the testing, Peter! I've incorporated your suggestion, and also added a Promise polyfill for the IE users you uncovered. Changes are now available on the develop branch. Feel free to reopen if you find any issues.