You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by GitBox <gi...@apache.org> on 2022/02/11 06:26:53 UTC
[GitHub] [echarts] Rain120 opened a new pull request #16493: [Feature] add borderRadius option to heatmap
Rain120 opened a new pull request #16493:
URL: https://github.com/apache/echarts/pull/16493
Issue: https://github.com/Rain120/Web-Study/issues/16
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Rain120 commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
Rain120 commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r806439593
##########
File path: src/chart/heatmap/HeatmapSeries.ts
##########
@@ -41,14 +41,18 @@ import type Calendar from '../../coord/calendar/Calendar';
type HeatmapDataValue = OptionDataValue[];
+export type BorderRadius = number | number[];
Review comment:
@plainheart I've fixed it, review it in your free time plz.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] pissang commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r806406304
##########
File path: test/heatmap-geo.html
##########
@@ -5,109 +5,99 @@
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
-"License"); you may not use this file except in compliance
Review comment:
@Rain120 There are still unexpected style changes on this file
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] pissang commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r805573629
##########
File path: test/heatmap-geo.html
##########
@@ -5,109 +5,99 @@
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
-"License"); you may not use this file except in compliance
Review comment:
Please avoid using any tool to format the code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] echarts-bot[bot] commented on pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#issuecomment-1035915879
Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout [the coding standard](https://echarts.apache.org/en/coding-standard.html) and Wiki about [How to make a pull request](https://github.com/apache/echarts/wiki/How-to-make-a-pull-request).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] pissang merged pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
pissang merged pull request #16493:
URL: https://github.com/apache/echarts/pull/16493
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Rain120 commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
Rain120 commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r805564618
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -41,6 +41,8 @@ interface GeoLikeCoordSys extends CoordinateSystem {
getViewRect(): graphic.BoundingRect
}
+type Shape = graphic.Rect['shape'];
Review comment:
I've fixed in the next commit.
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -41,6 +41,8 @@ interface GeoLikeCoordSys extends CoordinateSystem {
getViewRect(): graphic.BoundingRect
}
+type Shape = graphic.Rect['shape'];
Review comment:
I've fixed it in the next commit.
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -41,6 +41,8 @@ interface GeoLikeCoordSys extends CoordinateSystem {
getViewRect(): graphic.BoundingRect
}
+type Shape = graphic.Rect['shape'];
Review comment:
I've fix it in the next commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Rain120 commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
Rain120 commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r804649156
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -260,10 +271,17 @@ class HeatmapView extends ChartView {
if (isNaN(data.get(dataDims[1], idx) as number)) {
continue;
}
+ const shape: RectLike & {
Review comment:
copy that
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -245,13 +248,21 @@ class HeatmapView extends ChartView {
dataDimY
]);
+ const shape: RectShape & {
Review comment:
copy that
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -31,9 +31,10 @@ import type ContinuousModel from '../../component/visualMap/ContinuousModel';
import { CoordinateSystem, isCoordinateSystemType } from '../../coord/CoordinateSystem';
import { StageHandlerProgressParams, Dictionary, OptionDataValue } from '../../util/types';
import type Cartesian2D from '../../coord/cartesian/Cartesian2D';
-import type Calendar from '../../coord/calendar/Calendar';
+import Calendar from '../../coord/calendar/Calendar';
import { setLabelStyle, getLabelStatesModels } from '../../label/labelStyle';
import Element from 'zrender/src/Element';
+import { RectLike, RectShape } from 'zrender';
Review comment:
copy that
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -205,6 +206,8 @@ class HeatmapView extends ChartView {
let emphasisStyle = seriesModel.getModel(['emphasis', 'itemStyle']).getItemStyle();
let blurStyle = seriesModel.getModel(['blur', 'itemStyle']).getItemStyle();
let selectStyle = seriesModel.getModel(['select', 'itemStyle']).getItemStyle();
+ const itemStyle = seriesModel.getModel(['itemStyle']);
Review comment:
copy that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] pissang commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r806406304
##########
File path: test/heatmap-geo.html
##########
@@ -5,109 +5,99 @@
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
-"License"); you may not use this file except in compliance
Review comment:
@Rain120 There are still unexpected style changes on this file
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] pissang commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r805573629
##########
File path: test/heatmap-geo.html
##########
@@ -5,109 +5,99 @@
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
-"License"); you may not use this file except in compliance
Review comment:
Please avoid using any autoformatting tool to format the code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] pissang commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r804385000
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -31,9 +31,10 @@ import type ContinuousModel from '../../component/visualMap/ContinuousModel';
import { CoordinateSystem, isCoordinateSystemType } from '../../coord/CoordinateSystem';
import { StageHandlerProgressParams, Dictionary, OptionDataValue } from '../../util/types';
import type Cartesian2D from '../../coord/cartesian/Cartesian2D';
-import type Calendar from '../../coord/calendar/Calendar';
+import Calendar from '../../coord/calendar/Calendar';
import { setLabelStyle, getLabelStatesModels } from '../../label/labelStyle';
import Element from 'zrender/src/Element';
+import { RectLike, RectShape } from 'zrender';
Review comment:
Avoid importing from the 'zrender'. It may import the whole package.
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -205,6 +206,8 @@ class HeatmapView extends ChartView {
let emphasisStyle = seriesModel.getModel(['emphasis', 'itemStyle']).getItemStyle();
let blurStyle = seriesModel.getModel(['blur', 'itemStyle']).getItemStyle();
let selectStyle = seriesModel.getModel(['select', 'itemStyle']).getItemStyle();
+ const itemStyle = seriesModel.getModel(['itemStyle']);
Review comment:
Also `borderRadius` should support to be configured per data item. Like `emphasisStyle`, we can override the `borderRadius` in the `data.hasItemOption` branch below.
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -260,10 +271,17 @@ class HeatmapView extends ChartView {
if (isNaN(data.get(dataDims[1], idx) as number)) {
continue;
}
+ const shape: RectLike & {
Review comment:
Try using the type `graphic.Rect['shape']`
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -245,13 +248,21 @@ class HeatmapView extends ChartView {
dataDimY
]);
+ const shape: RectShape & {
Review comment:
The type can be `graphic.Rect['shape']`
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -205,6 +206,8 @@ class HeatmapView extends ChartView {
let emphasisStyle = seriesModel.getModel(['emphasis', 'itemStyle']).getItemStyle();
let blurStyle = seriesModel.getModel(['blur', 'itemStyle']).getItemStyle();
let selectStyle = seriesModel.getModel(['select', 'itemStyle']).getItemStyle();
+ const itemStyle = seriesModel.getModel(['itemStyle']);
Review comment:
No need to get a temporary model here. Just `seriesModel.get(['itemStyle', borderRadius']);`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Rain120 commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
Rain120 commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r805584783
##########
File path: test/heatmap-geo.html
##########
@@ -5,109 +5,99 @@
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
-"License"); you may not use this file except in compliance
Review comment:
srr, my fault
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] echarts-bot[bot] commented on pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#issuecomment-1039881852
Congratulations! Your PR has been merged. Thanks for your contribution! 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Rain120 commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
Rain120 commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r806434591
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -22,7 +22,7 @@ import { toggleHoverEmphasis } from '../../util/states';
import HeatmapLayer from './HeatmapLayer';
import * as zrUtil from 'zrender/src/core/util';
import ChartView from '../../view/Chart';
-import HeatmapSeriesModel, { HeatmapDataItemOption } from './HeatmapSeries';
+import HeatmapSeriesModel, { BorderRadius, HeatmapDataItemOption } from './HeatmapSeries';
Review comment:
ok
##########
File path: src/chart/heatmap/HeatmapSeries.ts
##########
@@ -41,14 +41,18 @@ import type Calendar from '../../coord/calendar/Calendar';
type HeatmapDataValue = OptionDataValue[];
+export type BorderRadius = number | number[];
Review comment:
@plainheart I've fixed it, review it in your free time plz.
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -31,7 +31,7 @@ import type ContinuousModel from '../../component/visualMap/ContinuousModel';
import { CoordinateSystem, isCoordinateSystemType } from '../../coord/CoordinateSystem';
import { StageHandlerProgressParams, Dictionary, OptionDataValue } from '../../util/types';
import type Cartesian2D from '../../coord/cartesian/Cartesian2D';
-import type Calendar from '../../coord/calendar/Calendar';
+import Calendar from '../../coord/calendar/Calendar';
Review comment:
I had lost it by accident.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Rain120 commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
Rain120 commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r805532283
##########
File path: test/heatmap-border-radius.html
##########
@@ -0,0 +1,166 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+'License'); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+<html>
+
+<head>
+ <meta charset='utf-8'>
+ <script src='lib/simpleRequire.js'></script>
+ <script src='lib/config.js'></script>
+ <meta name='viewport' content='width=device-width, initial-scale=1' />
+</head>
+
+<body>
+ <style>
+ html,
+ body,
+ #main,
+ #main2 {
+ width: 100%;
+ height: 100%;
+ margin: 0;
+ }
+ </style>
+ <div id='main'></div>
+ <div id='main2'></div>
+ <script>
+
+ require([
+ 'echarts'
+ ], function (echarts) {
+
+ var chart = echarts.init(document.getElementById('main'));
+ var chartTwo = echarts.init(document.getElementById('main2'));
+
+ var hours = ['12a', '1a', '2a', '3a', '4a', '5a', '6a',
Review comment:
I had done to combining all tests demo about heatmap-border-radius.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Rain120 commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
Rain120 commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r805815949
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -260,30 +265,39 @@ class HeatmapView extends ChartView {
if (isNaN(data.get(dataDims[1], idx) as number)) {
continue;
}
+ const shape: RectShape = coordSys.dataToRect([data.get(dataDims[0], idx)]).contentShape;
Review comment:
I've fix it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] pissang commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r805507967
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -225,6 +228,17 @@ class HeatmapView extends ChartView {
for (let idx = start; idx < end; idx++) {
let rect;
const style = data.getItemVisual(idx, 'style');
+ const itemModel = data.getItemModel<HeatmapDataItemOption>(idx);
+
+ // borderRadius must be set before the Rect draw.
Review comment:
It's not necessary to fetch the borderRadius before creating `Rect`. You can still change the shape after `Rect` created. I will describe it bellow.
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -245,13 +259,19 @@ class HeatmapView extends ChartView {
dataDimY
]);
+ const shape: Shape = {
+ x: Math.floor(Math.round(point[0]) - width / 2),
+ y: Math.floor(Math.round(point[1]) - height / 2),
+ width: Math.ceil(width),
+ height: Math.ceil(height)
+ };
+
+ if (borderRadius || Array.isArray(borderRadius)) {
Review comment:
As described above. The `r` can still be changed after `Rect` created. So it's not necessary to prepare the `shape` with `r` in two different branches. You only need to use
```
rect.shape.r = borderRadius;
```
to update the `r` around the line https://github.com/apache/echarts/blob/04392f5336f0f895c37b3e3f36294b67fc5fdf5c/src/chart/heatmap/HeatmapView.ts#L326
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -41,6 +41,8 @@ interface GeoLikeCoordSys extends CoordinateSystem {
getViewRect(): graphic.BoundingRect
}
+type Shape = graphic.Rect['shape'];
Review comment:
It's better to be named `RectShape`
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -245,13 +259,19 @@ class HeatmapView extends ChartView {
dataDimY
]);
+ const shape: Shape = {
+ x: Math.floor(Math.round(point[0]) - width / 2),
+ y: Math.floor(Math.round(point[1]) - height / 2),
+ width: Math.ceil(width),
+ height: Math.ceil(height)
+ };
+
+ if (borderRadius || Array.isArray(borderRadius)) {
Review comment:
Also using `isArray` method in the `zrender/src/core/util` instead of `Array.isArray`.
In fact the `Array.isArray` check can be removed because we usually don't do strict type checking in the logic code. Usually we only do the check that may have exceptions and break the whole app.
For example in this case `borderRadius` we won't reject the value like `{ 0: 5, 1: 5, 2: 5, 3: 5 }` or `TypedArray`. It make users code can still run even if they made a small mistake.
Instead we are trying to leave this kind of checking to the TypeScript. Which can provide strict enough type check and won't bring any extra code to the lib.
##########
File path: test/heatmap-border-radius.html
##########
@@ -0,0 +1,166 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+'License'); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+<html>
+
+<head>
+ <meta charset='utf-8'>
+ <script src='lib/simpleRequire.js'></script>
+ <script src='lib/config.js'></script>
+ <meta name='viewport' content='width=device-width, initial-scale=1' />
+</head>
+
+<body>
+ <style>
+ html,
+ body,
+ #main,
+ #main2 {
+ width: 100%;
+ height: 100%;
+ margin: 0;
+ }
+ </style>
+ <div id='main'></div>
+ <div id='main2'></div>
+ <script>
+
+ require([
+ 'echarts'
+ ], function (echarts) {
+
+ var chart = echarts.init(document.getElementById('main'));
+ var chartTwo = echarts.init(document.getElementById('main2'));
+
+ var hours = ['12a', '1a', '2a', '3a', '4a', '5a', '6a',
Review comment:
Try using `npm run mktest` command to create a new test case and put all heatmap borderRadius related test cases in one file.
##########
File path: test/heatmap-calendar.html
##########
@@ -0,0 +1,106 @@
+<!DOCTYPE html>
+<!--
Review comment:
There is already a `calendar-heatmap` case. Add a new `heatmap-calendar` may be confusing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] pissang merged pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
pissang merged pull request #16493:
URL: https://github.com/apache/echarts/pull/16493
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Rain120 commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
Rain120 commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r805564189
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -245,13 +259,19 @@ class HeatmapView extends ChartView {
dataDimY
]);
+ const shape: Shape = {
+ x: Math.floor(Math.round(point[0]) - width / 2),
+ y: Math.floor(Math.round(point[1]) - height / 2),
+ width: Math.ceil(width),
+ height: Math.ceil(height)
+ };
+
+ if (borderRadius || Array.isArray(borderRadius)) {
Review comment:
It means, we just need to get the value and assign it, the value about type check uses the TypeScript compiler, bcs we just care about the value. wonderful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] echarts-bot[bot] commented on pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#issuecomment-1039881852
Congratulations! Your PR has been merged. Thanks for your contribution! 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Rain120 commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
Rain120 commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r805553077
##########
File path: test/heatmap-calendar.html
##########
@@ -0,0 +1,106 @@
+<!DOCTYPE html>
+<!--
Review comment:
I've already deleted it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] plainheart commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r806421923
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -22,7 +22,7 @@ import { toggleHoverEmphasis } from '../../util/states';
import HeatmapLayer from './HeatmapLayer';
import * as zrUtil from 'zrender/src/core/util';
import ChartView from '../../view/Chart';
-import HeatmapSeriesModel, { HeatmapDataItemOption } from './HeatmapSeries';
+import HeatmapSeriesModel, { BorderRadius, HeatmapDataItemOption } from './HeatmapSeries';
Review comment:
The imported type `BorderRadius` is unused and can be removed here.
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -31,7 +31,7 @@ import type ContinuousModel from '../../component/visualMap/ContinuousModel';
import { CoordinateSystem, isCoordinateSystemType } from '../../coord/CoordinateSystem';
import { StageHandlerProgressParams, Dictionary, OptionDataValue } from '../../util/types';
import type Cartesian2D from '../../coord/cartesian/Cartesian2D';
-import type Calendar from '../../coord/calendar/Calendar';
+import Calendar from '../../coord/calendar/Calendar';
Review comment:
May I know why you removed the `type` keyword here?
##########
File path: src/chart/heatmap/HeatmapSeries.ts
##########
@@ -41,14 +41,18 @@ import type Calendar from '../../coord/calendar/Calendar';
type HeatmapDataValue = OptionDataValue[];
+export type BorderRadius = number | number[];
Review comment:
This definition for the `BorderRadius` type is unnecessary and can be merged into line 49 below.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Rain120 commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
Rain120 commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r806456097
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -31,7 +31,7 @@ import type ContinuousModel from '../../component/visualMap/ContinuousModel';
import { CoordinateSystem, isCoordinateSystemType } from '../../coord/CoordinateSystem';
import { StageHandlerProgressParams, Dictionary, OptionDataValue } from '../../util/types';
import type Cartesian2D from '../../coord/cartesian/Cartesian2D';
-import type Calendar from '../../coord/calendar/Calendar';
+import Calendar from '../../coord/calendar/Calendar';
Review comment:
I had lost it by accident.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] pissang commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r805573629
##########
File path: test/heatmap-geo.html
##########
@@ -5,109 +5,99 @@
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
-"License"); you may not use this file except in compliance
Review comment:
Please avoid using any autoformat tool to format the code.
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -260,30 +265,39 @@ class HeatmapView extends ChartView {
if (isNaN(data.get(dataDims[1], idx) as number)) {
continue;
}
+ const shape: RectShape = coordSys.dataToRect([data.get(dataDims[0], idx)]).contentShape;
Review comment:
This change of creating a temporary variable `shape` can be undo. So there is no necessity of using type `RectShape`.
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -260,30 +265,39 @@ class HeatmapView extends ChartView {
if (isNaN(data.get(dataDims[1], idx) as number)) {
continue;
}
+ const shape: RectShape = coordSys.dataToRect([data.get(dataDims[0], idx)]).contentShape;
rect = new graphic.Rect({
z2: 1,
- shape: coordSys.dataToRect([data.get(dataDims[0], idx)]).contentShape,
+ shape,
style
});
}
- const itemModel = data.getItemModel<HeatmapDataItemOption>(idx);
-
// Optimization for large datset
if (data.hasItemOption) {
+ const itemModel = data.getItemModel<HeatmapDataItemOption>(idx);
Review comment:
There is already an `itemModel`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] plainheart commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r806421923
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -22,7 +22,7 @@ import { toggleHoverEmphasis } from '../../util/states';
import HeatmapLayer from './HeatmapLayer';
import * as zrUtil from 'zrender/src/core/util';
import ChartView from '../../view/Chart';
-import HeatmapSeriesModel, { HeatmapDataItemOption } from './HeatmapSeries';
+import HeatmapSeriesModel, { BorderRadius, HeatmapDataItemOption } from './HeatmapSeries';
Review comment:
The imported type `BorderRadius` is unused and can be removed here.
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -31,7 +31,7 @@ import type ContinuousModel from '../../component/visualMap/ContinuousModel';
import { CoordinateSystem, isCoordinateSystemType } from '../../coord/CoordinateSystem';
import { StageHandlerProgressParams, Dictionary, OptionDataValue } from '../../util/types';
import type Cartesian2D from '../../coord/cartesian/Cartesian2D';
-import type Calendar from '../../coord/calendar/Calendar';
+import Calendar from '../../coord/calendar/Calendar';
Review comment:
May I know why you removed the `type` keyword here?
##########
File path: src/chart/heatmap/HeatmapSeries.ts
##########
@@ -41,14 +41,18 @@ import type Calendar from '../../coord/calendar/Calendar';
type HeatmapDataValue = OptionDataValue[];
+export type BorderRadius = number | number[];
Review comment:
This definition for the `BorderRadius` type is unnecessary and can be merged into line 49 below.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Rain120 commented on a change in pull request #16493: [Feature] add borderRadius option to heatmap
Posted by GitBox <gi...@apache.org>.
Rain120 commented on a change in pull request #16493:
URL: https://github.com/apache/echarts/pull/16493#discussion_r806434591
##########
File path: src/chart/heatmap/HeatmapView.ts
##########
@@ -22,7 +22,7 @@ import { toggleHoverEmphasis } from '../../util/states';
import HeatmapLayer from './HeatmapLayer';
import * as zrUtil from 'zrender/src/core/util';
import ChartView from '../../view/Chart';
-import HeatmapSeriesModel, { HeatmapDataItemOption } from './HeatmapSeries';
+import HeatmapSeriesModel, { BorderRadius, HeatmapDataItemOption } from './HeatmapSeries';
Review comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org