Use @vx/responsive to provide Chart width on Explore page #6104

kristw · 2018-10-15T05:50:40Z

Use ParentSize to provide chart width in ExploreChartPanel.
Remove complicated logic in Chart.jsx for getting element width.
Remove unnecessary jQuery usage.

@graceguo-supercat @williaster @conglei @michellethomas

codecov-io · 2018-10-15T06:07:06Z

Codecov Report

Merging #6104 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #6104   +/-   ##
=======================================
  Coverage   77.23%   77.23%           
=======================================
  Files          47       47           
  Lines        9349     9349           
=======================================
  Hits         7221     7221           
  Misses       2128     2128

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 af0ffa4...fae29eb. Read the comment docs.

superset/assets/src/explore/components/ExploreChartPanel.jsx

williaster · 2018-10-16T01:31:05Z

superset/assets/src/explore/components/ExploreChartPanel.jsx

+          datasource={this.props.datasource}
+          formData={this.props.form_data}
+          width={Math.floor(width)}
+          height={this.getHeight()}


should this now take the height from ParentSize into account?

I just tried. It doesn't work out of the box. May need to change the css of the container (.panel-header and .panel-body) into flexbox similar to what I did with WithLegend to make sure ParentSize computes the correct height. Now the height is larger than it should be.

I prefer not to deal with it now and leave this for future PR.

williaster

LGTM.

one question for testing, did you test the explore url with &standalone=true to test standalone mode?

kristw · 2018-10-17T05:06:37Z

Good call @williaster . Just verify the standalone mode and is good to go.

* use parent size to compute width * simplify this.width() and this.height() * remove unnecessary jquery * fix indent and import * adjust headerheight from 100 to 80 * fix test