From fde764323671f176a6df85939d0ceecf59ebf24c Mon Sep 17 00:00:00 2001 From: Vera Liu Date: Sun, 16 Oct 2016 11:25:23 -0700 Subject: [PATCH 1/4] Adjusted top margin of heatmap plot to get it working in V2 Problem: The heatmap in V2 was shifted towards the top margin of slice container, this was because in v2 slice name header was part of the container body, while in v1 the header was separately defined in explore.html template. Solution: To get heatmap properly shown in V2, we need to take into account the height of the slice_name header. Adding to margin_top will shift the plot in V1 too, but it won't make a big difference to the look. Ideally when we renovate slice container in future PR we would defined a height for slice_name header and take it into account for all visualization files. --- .../assets/javascripts/explorev2/components/ChartContainer.jsx | 2 +- caravel/assets/visualizations/heatmap.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/caravel/assets/javascripts/explorev2/components/ChartContainer.jsx b/caravel/assets/javascripts/explorev2/components/ChartContainer.jsx index 32430736bf38e..b73f545b31eb5 100644 --- a/caravel/assets/javascripts/explorev2/components/ChartContainer.jsx +++ b/caravel/assets/javascripts/explorev2/components/ChartContainer.jsx @@ -38,7 +38,7 @@ class ChartContainer extends React.Component { width: () => this.chartContainerRef.getBoundingClientRect().width, - height: () => parseInt(this.props.height, 10) - 100, + height: () => parseInt(this.props.height, 10), selector: `#${this.props.sliceContainerId}`, diff --git a/caravel/assets/visualizations/heatmap.js b/caravel/assets/visualizations/heatmap.js index 2cd4df237b10b..11f06fcdf7532 100644 --- a/caravel/assets/visualizations/heatmap.js +++ b/caravel/assets/visualizations/heatmap.js @@ -11,7 +11,7 @@ require('./heatmap.css'); function heatmapVis(slice) { function refresh() { const margin = { - top: 10, + top: 50, right: 10, bottom: 35, left: 35, From 91a60f0f458f6b38a426ef9ed5732fbfc9824183 Mon Sep 17 00:00:00 2001 From: Vera Liu Date: Mon, 24 Oct 2016 18:54:16 -0700 Subject: [PATCH 2/4] Added panel header height to margin_top for explore v2 --- .../javascripts/explorev2/components/ChartContainer.jsx | 2 +- caravel/assets/visualizations/heatmap.js | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/caravel/assets/javascripts/explorev2/components/ChartContainer.jsx b/caravel/assets/javascripts/explorev2/components/ChartContainer.jsx index b73f545b31eb5..32430736bf38e 100644 --- a/caravel/assets/javascripts/explorev2/components/ChartContainer.jsx +++ b/caravel/assets/javascripts/explorev2/components/ChartContainer.jsx @@ -38,7 +38,7 @@ class ChartContainer extends React.Component { width: () => this.chartContainerRef.getBoundingClientRect().width, - height: () => parseInt(this.props.height, 10), + height: () => parseInt(this.props.height, 10) - 100, selector: `#${this.props.sliceContainerId}`, diff --git a/caravel/assets/visualizations/heatmap.js b/caravel/assets/visualizations/heatmap.js index 11f06fcdf7532..211d824d712f1 100644 --- a/caravel/assets/visualizations/heatmap.js +++ b/caravel/assets/visualizations/heatmap.js @@ -10,8 +10,11 @@ require('./heatmap.css'); // https://jsfiddle.net/cyril123/h0reyumq/ function heatmapVis(slice) { function refresh() { + // Header for panel in explore v2 + const header = document.getElementsByClassName('panel-title'); + const headerHeight = header ? header[0].offsetHeight : 0; const margin = { - top: 50, + top: 20 + headerHeight, right: 10, bottom: 35, left: 35, From 0b147f724adb791ccb90bf06d2f3f045944ea9e6 Mon Sep 17 00:00:00 2001 From: Vera Liu Date: Mon, 24 Oct 2016 21:13:56 -0700 Subject: [PATCH 3/4] Use getBoundingClientRect to get header height --- caravel/assets/visualizations/heatmap.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/caravel/assets/visualizations/heatmap.js b/caravel/assets/visualizations/heatmap.js index 211d824d712f1..657ac9d78477a 100644 --- a/caravel/assets/visualizations/heatmap.js +++ b/caravel/assets/visualizations/heatmap.js @@ -12,7 +12,7 @@ function heatmapVis(slice) { function refresh() { // Header for panel in explore v2 const header = document.getElementsByClassName('panel-title'); - const headerHeight = header ? header[0].offsetHeight : 0; + const headerHeight = header ? header[0].getBoundingClientRect().height : 0; const margin = { top: 20 + headerHeight, right: 10, From a62d9adf6ca6551e35243fcde6e34ff9e08b2509 Mon Sep 17 00:00:00 2001 From: Vera Liu Date: Mon, 24 Oct 2016 21:36:30 -0700 Subject: [PATCH 4/4] Use slice-header for id of panel-title --- .../javascripts/explorev2/components/ChartContainer.jsx | 7 ++++++- caravel/assets/visualizations/heatmap.js | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/caravel/assets/javascripts/explorev2/components/ChartContainer.jsx b/caravel/assets/javascripts/explorev2/components/ChartContainer.jsx index 32430736bf38e..241500efd9625 100644 --- a/caravel/assets/javascripts/explorev2/components/ChartContainer.jsx +++ b/caravel/assets/javascripts/explorev2/components/ChartContainer.jsx @@ -59,7 +59,12 @@ class ChartContainer extends React.Component { {this.props.sliceName} +
+ {this.props.sliceName} +
} >