vegas-viz / Vegas

The missing MatPlotLib for Scala + Spark
MIT License
730 stars 98 forks source link

Remove all use of jQuery, wrap invocation in IIFE #112

Closed rgbkrk closed 7 years ago

rgbkrk commented 7 years ago

This fixes some issues folks were having when there were multiple vegas plots on one page while also making a non-jquery way of plotting.

If people like this change, I'll update for the tests too.

codecov-io commented 7 years ago

Codecov Report

Merging #112 into master will decrease coverage by 0.03%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #112      +/-   ##
==========================================
- Coverage   40.95%   40.92%   -0.04%     
==========================================
  Files          28       28              
  Lines        3064     3064              
  Branches        3        3              
==========================================
- Hits         1255     1254       -1     
- Misses       1809     1810       +1
Impacted Files Coverage Δ
...c/main/scala/vegas/render/StaticHTMLRenderer.scala 100% <100%> (ø) :arrow_up:
...os/src/main/scala/vegas/macros/AliasWithLens.scala 0% <0%> (-100%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 12b30c8...071919c. Read the comment docs.

rgbkrk commented 7 years ago

Hey look it passes! 😉

rgbkrk commented 7 years ago

I'm tempted to do this:

  def frameHTML(name: String = defaultName) = {
    val frameName = "frame-" + name
    s"""
      |  <iframe id="${frameName}" sandbox="allow-scripts allow-same-origin" style="border: none; width: 100%" srcdoc="${xml.Utility.escape(pageHTML(name))}"></iframe>
      |  <script>
      |    (function() {
      |      var iframe = document.querySelector('#${frameName}');
      |      function resizeIFrame() {
      |        var height = iframe.contentWindow.document.body.scrollHeight || '400'; // Fallback in case of no scroll height
      |        iframe.style.height = height + 'px';
      |      }
      |      iframe.onload = resizeIFrame;
      |    })(); // IIFE
      |  </script>
    """.stripMargin
    }

I'm guessing that the recursive setTimeout is accouting for waiting on vega to finish drawing as well. It seems like I should work on using the iframe postMessage mechanics to communicate the height with the callback from embed. That can be a later PR though.

aishfenton commented 7 years ago

@rgbkrk yes setTimeout was done to wait for vega rendering to complete. But totally agree a callback would be better (I just didn't know how to do that).

I'm going to merge and release what's here

aishfenton commented 7 years ago

merged

rgbkrk commented 7 years ago

yes setTimeout was done to wait for vega rendering to complete. But totally agree a callback would be better (I just didn't know how to do that).

That's ok, I'm happy to contribute that.