Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(heatmap) Skip rendering for data out of axis content in heatmap (#12969) #12991

Merged
merged 2 commits into from Jul 21, 2020
Merged

fix(heatmap) Skip rendering for data out of axis content in heatmap (#12969) #12991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
493f54c
fix #12969 - Skip rendering for data out of axis content in heatmap
quillblue Jul 19, 2020
f8031ef
Update HeatmapView.ts according to code review comment
quillblue Jul 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 13 additions & 5 deletions src/chart/heatmap/HeatmapView.ts
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

import {__DEV__} from '../../config';
import { __DEV__ } from '../../config';
import * as graphic from '../../util/graphic';
import HeatmapLayer from './HeatmapLayer';
import * as zrUtil from 'zrender/src/core/util';
Expand Down Expand Up @@ -163,6 +163,8 @@ class HeatmapView extends ChartView {
const coordSys = seriesModel.coordinateSystem as Cartesian2D | Calendar;
let width;
let height;
let xAxisExtent;
let yAxisExtent;

if (isCoordinateSystemType<Cartesian2D>(coordSys, 'cartesian2d')) {
const xAxis = coordSys.getAxis('x');
Expand All @@ -179,6 +181,8 @@ class HeatmapView extends ChartView {

width = xAxis.getBandWidth();
height = yAxis.getBandWidth();
xAxisExtent = xAxis.scale.getExtent();
yAxisExtent = yAxis.scale.getExtent();
}

const group = this.group;
Expand All @@ -203,14 +207,18 @@ class HeatmapView extends ChartView {
let rect;

if (isCoordinateSystemType<Cartesian2D>(coordSys, 'cartesian2d')) {
// Ignore empty data
if (isNaN(data.get(dataDims[2], idx) as number)) {
const dataDimX = data.get(dataDims[0], idx);
const dataDimY = data.get(dataDims[1], idx);

// Ignore empty data and out of extent data
if (isNaN(data.get(dataDims[2], idx) as number) || dataDimX < xAxisExtent[0] || dataDimX > xAxisExtent[1]
|| dataDimY < yAxisExtent[0] || dataDimY > yAxisExtent[1]) {
continue;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data.get(dataDims[0], idx) is better to be assigned to a temporal variable.

Also, yAxis should also be checked?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, have modified accordingly.

}

const point = coordSys.dataToPoint([
data.get(dataDims[0], idx),
data.get(dataDims[1], idx)
dataDimX,
dataDimY
]);

rect = new graphic.Rect({
Expand Down
218 changes: 141 additions & 77 deletions test/heatmap.html
View file
Open in desktop
@@ -1,4 +1,3 @@

<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
Expand All @@ -19,91 +18,156 @@
-->

<html>
<head>
Copy link
Contributor

@pissang pissang Jul 19, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should create a new test case instead of modifying the exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the new case is almost seprate except under same page and share same data. Changes are more likely a false detection (may be due to formatter, actually I didn't change the original case any)

<meta charset="utf-8">
<script src="lib/esl.js"></script>
<script src="lib/config.js"></script>
<meta name="viewport" content="width=device-width, initial-scale=1" />
</head>
<body>
<style>
html, body, #main {
width: 100%;
height: 100%;
margin: 0;
}
</style>
<div id="main"></div>
<script>

require([
'echarts'
// 'echarts/chart/heatmap',
// 'echarts/component/legend',
// 'echarts/component/visualMap',
// 'echarts/component/grid',
// 'echarts/component/polar',
// 'echarts/component/tooltip'
], function (echarts) {
<head>
<meta charset="utf-8">
<script src="lib/esl.js"></script>
<script src="lib/config.js"></script>
<meta name="viewport" content="width=device-width, initial-scale=1" />
</head>

var chart = echarts.init(document.getElementById('main'));
<body>
<style>
html,
body,
#main,
#main2 {
width: 100%;
height: 100%;
margin: 0;
}
</style>
<div id="main"></div>
<div id="main2"></div>
<script>

var hours = ['12a', '1a', '2a', '3a', '4a', '5a', '6a',
'7a', '8a', '9a','10a','11a',
'12p', '1p', '2p', '3p', '4p', '5p',
'6p', '7p', '8p', '9p', '10p', '11p'];
var days = ['Saturday', 'Friday', 'Thursday',
'Wednesday', 'Tuesday', 'Monday', 'Sunday'];
require([
'echarts'
// 'echarts/chart/heatmap',
// 'echarts/component/legend',
// 'echarts/component/visualMap',
// 'echarts/component/grid',
// 'echarts/component/polar',
// 'echarts/component/tooltip'
], function (echarts) {

var data = [[0,0,5],[0,1,1],[0,2,0],[0,3,0],[0,4,0],[0,5,0],[0,6,0],[0,7,0],[0,8,0],[0,9,0],[0,10,0],[0,11,2],[0,12,4],[0,13,1],[0,14,1],[0,15,3],[0,16,4],[0,17,6],[0,18,4],[0,19,4],[0,20,3],[0,21,3],[0,22,2],[0,23,5],[1,0,7],[1,1,0],[1,2,0],[1,3,0],[1,4,0],[1,5,0],[1,6,0],[1,7,0],[1,8,0],[1,9,0],[1,10,5],[1,11,2],[1,12,2],[1,13,6],[1,14,9],[1,15,11],[1,16,6],[1,17,7],[1,18,8],[1,19,12],[1,20,5],[1,21,5],[1,22,7],[1,23,2],[2,0,1],[2,1,1],[2,2,0],[2,3,0],[2,4,0],[2,5,0],[2,6,0],[2,7,0],[2,8,0],[2,9,0],[2,10,3],[2,11,2],[2,12,1],[2,13,9],[2,14,8],[2,15,10],[2,16,6],[2,17,5],[2,18,5],[2,19,5],[2,20,7],[2,21,4],[2,22,2],[2,23,4],[3,0,7],[3,1,3],[3,2,0],[3,3,0],[3,4,0],[3,5,0],[3,6,0],[3,7,0],[3,8,1],[3,9,0],[3,10,5],[3,11,4],[3,12,7],[3,13,14],[3,14,13],[3,15,12],[3,16,9],[3,17,5],[3,18,5],[3,19,10],[3,20,6],[3,21,4],[3,22,4],[3,23,1],[4,0,1],[4,1,3],[4,2,0],[4,3,0],[4,4,0],[4,5,1],[4,6,0],[4,7,0],[4,8,0],[4,9,2],[4,10,4],[4,11,4],[4,12,2],[4,13,4],[4,14,4],[4,15,14],[4,16,12],[4,17,1],[4,18,8],[4,19,5],[4,20,3],[4,21,7],[4,22,3],[4,23,0],[5,0,2],[5,1,1],[5,2,0],[5,3,3],[5,4,0],[5,5,0],[5,6,0],[5,7,0],[5,8,2],[5,9,0],[5,10,4],[5,11,1],[5,12,5],[5,13,10],[5,14,5],[5,15,7],[5,16,11],[5,17,6],[5,18,0],[5,19,5],[5,20,3],[5,21,4],[5,22,2],[5,23,0],[6,0,1],[6,1,0],[6,2,0],[6,3,0],[6,4,0],[6,5,0],[6,6,0],[6,7,0],[6,8,0],[6,9,0],[6,10,1],[6,11,0],[6,12,2],[6,13,1],[6,14,3],[6,15,4],[6,16,0],[6,17,0],[6,18,0],[6,19,0],[6,20,1],[6,21,2],[6,22,2],[6,23,6]];
var chart = echarts.init(document.getElementById('main'));
var chartTwo = echarts.init(document.getElementById('main2'));

data = data.map(function (item) {
return [item[1], item[0], item[2] || '-'];
});
var hours = ['12a', '1a', '2a', '3a', '4a', '5a', '6a',
'7a', '8a', '9a', '10a', '11a',
'12p', '1p', '2p', '3p', '4p', '5p',
'6p', '7p', '8p', '9p', '10p', '11p'];
var days = ['Saturday', 'Friday', 'Thursday',
'Wednesday', 'Tuesday', 'Monday', 'Sunday'];

chart.setOption({
aria: {
show: true
},
tooltip: {
position: 'top'
},
animation: false,
grid: {
height: 300
},
xAxis: {
type: 'category',
data: hours
},
yAxis: {
type: 'category',
data: days
var data = [[0, 0, 5], [0, 1, 1], [0, 2, 0], [0, 3, 0], [0, 4, 0], [0, 5, 0], [0, 6, 0], [0, 7, 0], [0, 8, 0], [0, 9, 0], [0, 10, 0], [0, 11, 2], [0, 12, 4], [0, 13, 1], [0, 14, 1], [0, 15, 3], [0, 16, 4], [0, 17, 6], [0, 18, 4], [0, 19, 4], [0, 20, 3], [0, 21, 3], [0, 22, 2], [0, 23, 5], [1, 0, 7], [1, 1, 0], [1, 2, 0], [1, 3, 0], [1, 4, 0], [1, 5, 0], [1, 6, 0], [1, 7, 0], [1, 8, 0], [1, 9, 0], [1, 10, 5], [1, 11, 2], [1, 12, 2], [1, 13, 6], [1, 14, 9], [1, 15, 11], [1, 16, 6], [1, 17, 7], [1, 18, 8], [1, 19, 12], [1, 20, 5], [1, 21, 5], [1, 22, 7], [1, 23, 2], [2, 0, 1], [2, 1, 1], [2, 2, 0], [2, 3, 0], [2, 4, 0], [2, 5, 0], [2, 6, 0], [2, 7, 0], [2, 8, 0], [2, 9, 0], [2, 10, 3], [2, 11, 2], [2, 12, 1], [2, 13, 9], [2, 14, 8], [2, 15, 10], [2, 16, 6], [2, 17, 5], [2, 18, 5], [2, 19, 5], [2, 20, 7], [2, 21, 4], [2, 22, 2], [2, 23, 4], [3, 0, 7], [3, 1, 3], [3, 2, 0], [3, 3, 0], [3, 4, 0], [3, 5, 0], [3, 6, 0], [3, 7, 0], [3, 8, 1], [3, 9, 0], [3, 10, 5], [3, 11, 4], [3, 12, 7], [3, 13, 14], [3, 14, 13], [3, 15, 12], [3, 16, 9], [3, 17, 5], [3, 18, 5], [3, 19, 10], [3, 20, 6], [3, 21, 4], [3, 22, 4], [3, 23, 1], [4, 0, 1], [4, 1, 3], [4, 2, 0], [4, 3, 0], [4, 4, 0], [4, 5, 1], [4, 6, 0], [4, 7, 0], [4, 8, 0], [4, 9, 2], [4, 10, 4], [4, 11, 4], [4, 12, 2], [4, 13, 4], [4, 14, 4], [4, 15, 14], [4, 16, 12], [4, 17, 1], [4, 18, 8], [4, 19, 5], [4, 20, 3], [4, 21, 7], [4, 22, 3], [4, 23, 0], [5, 0, 2], [5, 1, 1], [5, 2, 0], [5, 3, 3], [5, 4, 0], [5, 5, 0], [5, 6, 0], [5, 7, 0], [5, 8, 2], [5, 9, 0], [5, 10, 4], [5, 11, 1], [5, 12, 5], [5, 13, 10], [5, 14, 5], [5, 15, 7], [5, 16, 11], [5, 17, 6], [5, 18, 0], [5, 19, 5], [5, 20, 3], [5, 21, 4], [5, 22, 2], [5, 23, 0], [6, 0, 1], [6, 1, 0], [6, 2, 0], [6, 3, 0], [6, 4, 0], [6, 5, 0], [6, 6, 0], [6, 7, 0], [6, 8, 0], [6, 9, 0], [6, 10, 1], [6, 11, 0], [6, 12, 2], [6, 13, 1], [6, 14, 3], [6, 15, 4], [6, 16, 0], [6, 17, 0], [6, 18, 0], [6, 19, 0], [6, 20, 1], [6, 21, 2], [6, 22, 2], [6, 23, 6]];

data = data.map(function (item) {
return [item[1], item[0], item[2] || '-'];
});


chart.setOption({
aria: {
show: true
},
tooltip: {
position: 'top'
},
animation: false,
grid: {
height: 300
},
xAxis: {
type: 'category',
data: hours,
},
yAxis: {
type: 'category',
data: days
},
visualMap: {
min: 1,
max: 10,
calculable: true
},
series: [{
name: 'Punch Card',
type: 'heatmap',
data: data,
label: {
normal: {
show: true
}
},
visualMap: {
min: 1,
max: 10,
calculable: true
itemStyle: {
emphasis: {
shadowBlur: 10,
shadowColor: 'rgba(0, 0, 0, 0.5)'
}
}
}]
});

chartTwo.setOption({
aria: {
show: true
},
title: {
left: 'center',
text: 'Heatmap with min/max value configed in Axis (tested with dataZoom)',
},
tooltip: {
position: 'top'
},
toolbox: {
feature: {
dataZoom: {},
restore: {}
}
},
animation: false,
grid: {
height: 300
},
xAxis: {
type: 'category',
data: hours,
min: 2,
max: 20
},
yAxis: {
type: 'category',
data: days,
max: 6,
min: 1
},
visualMap: {
min: 1,
max: 10,
calculable: true
},
series: [{
name: 'Punch Card',
type: 'heatmap',
data: data,
label: {
normal: {
show: true
}
},
series: [{
name: 'Punch Card',
type: 'heatmap',
data: data,
label: {
normal: {
show: true
}
},
itemStyle: {
emphasis: {
shadowBlur: 10,
shadowColor: 'rgba(0, 0, 0, 0.5)'
}
itemStyle: {
emphasis: {
shadowBlur: 10,
shadowColor: 'rgba(0, 0, 0, 0.5)'
}
}]
});
}
}]
});
});

</script>
</body>

</script>
</body>
</html>