vegas-viz / Vegas

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

Create the targeted element before the script #108

Closed rgbkrk closed 7 years ago

rgbkrk commented 7 years ago

This doesn't solve the issue with iframe resizing, though it's certainly something that should be done to ensure the element targeted for vega exists.

dbtsai commented 7 years ago

This looks good to me. However, this changes the output HTML which breaks the test. Can you fix it in StaticHTMLRendererSpec? Thanks.

codecov-io commented 7 years ago

Codecov Report

Merging #108 into master will decrease coverage by 29.29%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #108      +/-   ##
==========================================
- Coverage   70.25%   40.95%   -29.3%     
==========================================
  Files          27       28       +1     
  Lines        1785     3064    +1279     
  Branches        6        3       -3     
==========================================
+ Hits         1254     1255       +1     
- Misses        531     1809    +1278
Impacted Files Coverage Δ
...c/main/scala/vegas/render/StaticHTMLRenderer.scala 100% <100%> (ø) :arrow_up:
core/src/main/scala/vegas/spec/Spec.scala 0% <0%> (-69.43%) :arrow_down:
spec/src/main/scala/vegas/spec/Spec.scala 69.42% <0%> (ø)
...os/src/main/scala/vegas/macros/AliasWithLens.scala 100% <0%> (+100%) :arrow_up:

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 03286dd...4c554d7. Read the comment docs.

rgbkrk commented 7 years ago

All set, want me to squash this down to one commit?

dbtsai commented 7 years ago

@rgbkrk Thanks for this PR. Merged into master. Github can squash when merging :)