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 2021/06/07 08:01:22 UTC
[GitHub] [echarts] ssthouse opened a new pull request #15095: feat: pie-chart support minus value
ssthouse opened a new pull request #15095:
URL: https://github.com/apache/echarts/pull/15095
<!-- Please fill in the following information to help us review your PR more efficiently. -->
## Brief Information
This pull request is in the type of:
- [x] bug fixing
- [ ] new feature
- [ ] others
### What does this PR do?
<!-- USE ONCE SENTENCE TO DESCRIBE WHAT THIS PR DOES. -->
Support minus value in Pie Chart.
### Fixed issues
- #15055
## Details
### Before: What was the problem?
<!-- DESCRIBE THE BUG OR REQUIREMENT HERE. -->
the minus value slice fillup the entire pie chart;
<!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
### After: How is it fixed in this PR?
<!-- THE RESULT AFTER FIXING AND A SIMPLE EXPLANATION ABOUT HOW IT IS FIXED. -->
<!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
## Misc
<!-- ADD RELATED ISSUE ID WHEN APPLICABLE -->
- [ ] The API has been changed (apache/echarts-doc#xxx).
- [ ] This PR depends on ZRender changes (ecomfe/zrender#xxx).
### Related test cases or examples to use the new APIs
NA.
add test file: `pie-minus-value.html` in test folder;
## Others
### Merging options
- [ ] Please squash the commits into a single one when merge.
### Other information
--
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.
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] ssthouse commented on pull request #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-855828637
> @ssthouse Could you try to filter negative data with pie series in `processor/dataFilter.ts`? If it works, you don't need to change pieView and it would be simpler. Please be careful when checking the legend of negative data.
I'll try~
--
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.
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 edited a comment on pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-867390596
@ssthouse I think we can wait for the developers' feedback.
--
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.
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 pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-867390596
@ssthouse I think we can wait for the developers' feedback. So I'm going to merge 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.
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 pull request #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-856408943
@ssthouse Should register a new processor to filter the negative data. See https://github.com/apache/echarts/blob/master/src/chart/pie/install.ts#L36 about how to register a processor
--
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.
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] ssthouse commented on a change in pull request #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on a change in pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#discussion_r646464989
##########
File path: src/chart/pie/pieLayout.ts
##########
@@ -72,8 +72,13 @@ export default function pieLayout(
data.each(valueDim, function (value: number) {
!isNaN(value) && validDataCount++;
});
+ let sum = 0;
+ data.each(valueDim, function (value: number) {
+ if (!isNaN(value) && value > 0) {
+ sum += value;
+ }
+ });
Review comment:
fixed
--
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.
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 edited a comment on pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-862152229
@ssthouse Here is a common way to set style from the option:
```ts
sector.useStyle(seriesModel.getModel('emptyCircleStyle').getItemStyle());
```
Some notes:
+ Need to use `useStyle` here so the style properties can be reset to default when developers didn't set it in the new options.
+ `seriesModel.getModel('emptyCircleStyle')` get a model subset which only includes `emptyCircleStyle` option.
+ `getItemStyle` get the full mapped style object from the model. It includes fill and stroke. Meanwhile, there are other two methods that can get a mapped style object. `getLineStyle` which only includes stroke. `getAreaStyle` which only includes fill.
--
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.
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 #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#discussion_r647935424
##########
File path: test/pie-minus-value.html
##########
@@ -0,0 +1,105 @@
+<!--
+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>
+ h1 {
+ line-height: 60px;
+ height: 60px;
+ background: #146402;
+ text-align: center;
+ font-weight: bold;
+ color: #eee;
+ font-size: 14px;
+ }
+ .chart {
+ height: 600px;
+ }
+ </style>
+
+ <div id="main" class="chart"></div>
+ <script>
+ require(["echarts"], function (echarts) {
+ var chart = echarts.init(document.getElementById("main"));
+
+ chart.setOption({
+ aria: {
+ enabled: true,
+ },
+ title: {
+ text: `The minus value slice should render like 0 value,
+ while the tooltip should show the origin minus value`,
+ x: "center",
+ },
+ tooltip: {
+ trigger: "item",
+ formatter: "{a} <br/>{b} : {c} ({d}%)",
+ },
+ legend: {
+ orient: "vertical",
+ left: "left",
+ data: [
+ "直接访问",
+ "邮件营销",
+ "联盟广告",
+ "视频广告(value is 0)",
+ "搜索引擎(value is minus)",
+ ],
+ },
+ series: [
+ {
+ name: "访问来源",
+ type: "pie",
+ radius: "55%",
+ center: ["50%", "60%"],
+ selectedMode: "single",
+ data: [
+ { value: 335, name: "直接访问" },
Review comment:
> When all the data are negative, the pie chart is empty.
> I'm not sure if this is the expected result 🐒
At least, it's unexpected for me. This will break the previous behavior (See below). And I think it's meaningful when all value is negative. In that case, considering them as `0` or filtering them may be not a good way.
<img src="https://user-images.githubusercontent.com/26999792/121666461-dda29380-cadb-11eb-9603-a9cdc3cffc4d.png" width="400">
--
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.
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 edited a comment on pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-862152229
@ssthouse Here is a common way to set style from the option:
```ts
sector.useStyle(seriesModel.getModel('emptyCircleStyle').getItemStyle());
```
Some notes:
+ Need to use `useStyle` here so the style properties can be reset to default when developers didn't set it in the new options. Meanwhile `setStyle` will merge the new style to the existing style.
+ `seriesModel.getModel('emptyCircleStyle')` get a model subset which only includes `emptyCircleStyle` option.
+ `getItemStyle` get the full mapped style object from the option. It includes fill and stroke. Meanwhile, there are other two methods that can get a mapped style object. `getLineStyle` which only includes stroke. `getAreaStyle` which only includes fill.
--
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.
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] ssthouse commented on a change in pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on a change in pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#discussion_r652531368
##########
File path: src/processor/negativeDataFilter.ts
##########
@@ -0,0 +1,44 @@
+/*
+* 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.
+*/
+
+import { StageHandler } from '../util/types';
+
+export default function negativeDataFilter(seriesType: string): StageHandler {
+ return {
+ seriesType: seriesType,
+ reset: function (seriesModel, ecModel) {
+ const legendModels = ecModel.findComponents({
+ mainType: 'legend'
+ });
+ if (!legendModels || !legendModels.length) {
Review comment:
fixed
--
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.
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 edited a comment on pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-862152229
@ssthouse Here is a common way to set style from the option:
```ts
sector.useStyle(seriesModel.getModel('emptyCircleStyle').getItemStyle());
```
Some notes:
+ Need to use `useStyle` here so the style properties can be reset to default when developers didn't set it in the new options. Meanwhile `setStyle` will merge the new style to the existing style.
+ `seriesModel.getModel('emptyCircleStyle')` get a model subset which only includes `emptyCircleStyle` option.
+ `getItemStyle` get the full mapped style object from the model. It includes fill and stroke. Meanwhile, there are other two methods that can get a mapped style object. `getLineStyle` which only includes stroke. `getAreaStyle` which only includes fill.
--
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.
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] ssthouse commented on a change in pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on a change in pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#discussion_r652532454
##########
File path: src/chart/pie/PieView.ts
##########
@@ -242,6 +243,29 @@ class PieView extends ChartView {
}
}
+ // remove empty-circle(which is not instance of PiePiece) if it exists
+ group.children().forEach(child => {
+ if (!(child instanceof PiePiece)) {
Review comment:
fixed
--
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.
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] yangkai-bj commented on pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
yangkai-bj commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-919790484
在Pie中强制过滤负数,是否简单粗暴了一些,在饼图和圆环图中负数确实导致图形绘制的问题,但玫瑰图可以真实反映负数,是否可以将PIE中"是否过滤负值"作为一个用户选项.
日常涉及规模数据的时候一般都是非负数,使用Pie绘制很好理解,但涉及增量的占比,负增数据无法避免,饼图和圆环图都无法绘制,往往需要对原始数据进行变形(将所有值+abs(最小负值))后绘制,绘制后的图表让人很难理解,5.1.2以前的玫瑰图(roseType: "area")可以真实体现原始数据中的负数.
--
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 pull request #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
plainheart commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-856448011
It seems this will break the case in which all the number is negative. Besides, if the negative value was considered as `0`, how does the tooltip show? Should it be `0`(as the pie shows) or the original negative value like `-1548`?
--
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.
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] Ovilia commented on a change in pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
Ovilia commented on a change in pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#discussion_r649791842
##########
File path: test/pie-minus-value.html
##########
@@ -0,0 +1,105 @@
+<!--
+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>
+ h1 {
+ line-height: 60px;
+ height: 60px;
+ background: #146402;
+ text-align: center;
+ font-weight: bold;
+ color: #eee;
+ font-size: 14px;
+ }
+ .chart {
+ height: 600px;
+ }
+ </style>
+
+ <div id="main" class="chart"></div>
+ <script>
+ require(["echarts"], function (echarts) {
+ var chart = echarts.init(document.getElementById("main"));
+
+ chart.setOption({
+ aria: {
+ enabled: true,
+ },
+ title: {
+ text: `The minus value slice should render like 0 value,
+ while the tooltip should show the origin minus value`,
+ x: "center",
+ },
+ tooltip: {
+ trigger: "item",
+ formatter: "{a} <br/>{b} : {c} ({d}%)",
+ },
+ legend: {
+ orient: "vertical",
+ left: "left",
+ data: [
+ "直接访问",
+ "邮件营销",
+ "联盟广告",
+ "视频广告(value is 0)",
+ "搜索引擎(value is minus)",
+ ],
+ },
+ series: [
+ {
+ name: "访问来源",
+ type: "pie",
+ radius: "55%",
+ center: ["50%", "60%"],
+ selectedMode: "single",
+ data: [
+ { value: 335, name: "直接访问" },
Review comment:
By default (if [series-pie.stillShowZeroSum](https://echarts.apache.org/zh/option.html#series-pie.stillShowZeroSum) is not set to be `false`), when all data is negative, it should behave the same as when all data is zero.
##########
File path: test/pie-minus-value.html
##########
@@ -0,0 +1,105 @@
+<!--
+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>
+ h1 {
+ line-height: 60px;
+ height: 60px;
+ background: #146402;
+ text-align: center;
+ font-weight: bold;
+ color: #eee;
+ font-size: 14px;
+ }
+ .chart {
+ height: 600px;
+ }
+ </style>
+
+ <div id="main" class="chart"></div>
+ <script>
+ require(["echarts"], function (echarts) {
+ var chart = echarts.init(document.getElementById("main"));
+
+ chart.setOption({
+ aria: {
+ enabled: true,
+ },
+ title: {
+ text: `The minus value slice should render like 0 value,
+ while the tooltip should show the origin minus value`,
+ x: "center",
+ },
+ tooltip: {
+ trigger: "item",
+ formatter: "{a} <br/>{b} : {c} ({d}%)",
+ },
+ legend: {
+ orient: "vertical",
+ left: "left",
+ data: [
+ "直接访问",
+ "邮件营销",
+ "联盟广告",
+ "视频广告(value is 0)",
+ "搜索引擎(value is minus)",
+ ],
+ },
+ series: [
+ {
+ name: "访问来源",
+ type: "pie",
+ radius: "55%",
+ center: ["50%", "60%"],
+ selectedMode: "single",
+ data: [
+ { value: 335, name: "直接访问" },
Review comment:
By default (if [series-pie.stillShowZeroSum](https://echarts.apache.org/zh/option.html#series-pie.stillShowZeroSum) is not set to be `false`), when all data is negative, it should behave the same as when all data is zero.
##########
File path: test/pie-minus-value.html
##########
@@ -0,0 +1,105 @@
+<!--
+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>
+ h1 {
+ line-height: 60px;
+ height: 60px;
+ background: #146402;
+ text-align: center;
+ font-weight: bold;
+ color: #eee;
+ font-size: 14px;
+ }
+ .chart {
+ height: 600px;
+ }
+ </style>
+
+ <div id="main" class="chart"></div>
+ <script>
+ require(["echarts"], function (echarts) {
+ var chart = echarts.init(document.getElementById("main"));
+
+ chart.setOption({
+ aria: {
+ enabled: true,
+ },
+ title: {
+ text: `The minus value slice should render like 0 value,
+ while the tooltip should show the origin minus value`,
+ x: "center",
+ },
+ tooltip: {
+ trigger: "item",
+ formatter: "{a} <br/>{b} : {c} ({d}%)",
+ },
+ legend: {
+ orient: "vertical",
+ left: "left",
+ data: [
+ "直接访问",
+ "邮件营销",
+ "联盟广告",
+ "视频广告(value is 0)",
+ "搜索引擎(value is minus)",
+ ],
+ },
+ series: [
+ {
+ name: "访问来源",
+ type: "pie",
+ radius: "55%",
+ center: ["50%", "60%"],
+ selectedMode: "single",
+ data: [
+ { value: 335, name: "直接访问" },
Review comment:
If the canvas is blank, it looks like there is an error to the users. So perhaps we could display an empty light gray circle when no data (sum is zero or all is `'-'` or negative). We should also provide options to configure the color.
The legend color doesn't look right to me. They should have different colors for different series.
--
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.
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] ssthouse commented on a change in pull request #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on a change in pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#discussion_r646463595
##########
File path: src/chart/pie/PieSeries.ts
##########
@@ -170,7 +170,13 @@ class PieSeriesModel extends SeriesModel<PieSeriesOption> {
const valueList: number[] = [];
data.each(data.mapDimension('value'), function (value: number) {
- valueList.push(value);
+ // handle minue value condition: teat minus value as 0 when calculation percent
+ if (!isNaN(value) && value < 0) {
+ valueList.push(0);
+ }
+ else {
+ valueList.push(value);
+ }
Review comment:
fixed
--
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.
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 #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#discussion_r647935424
##########
File path: test/pie-minus-value.html
##########
@@ -0,0 +1,105 @@
+<!--
+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>
+ h1 {
+ line-height: 60px;
+ height: 60px;
+ background: #146402;
+ text-align: center;
+ font-weight: bold;
+ color: #eee;
+ font-size: 14px;
+ }
+ .chart {
+ height: 600px;
+ }
+ </style>
+
+ <div id="main" class="chart"></div>
+ <script>
+ require(["echarts"], function (echarts) {
+ var chart = echarts.init(document.getElementById("main"));
+
+ chart.setOption({
+ aria: {
+ enabled: true,
+ },
+ title: {
+ text: `The minus value slice should render like 0 value,
+ while the tooltip should show the origin minus value`,
+ x: "center",
+ },
+ tooltip: {
+ trigger: "item",
+ formatter: "{a} <br/>{b} : {c} ({d}%)",
+ },
+ legend: {
+ orient: "vertical",
+ left: "left",
+ data: [
+ "直接访问",
+ "邮件营销",
+ "联盟广告",
+ "视频广告(value is 0)",
+ "搜索引擎(value is minus)",
+ ],
+ },
+ series: [
+ {
+ name: "访问来源",
+ type: "pie",
+ radius: "55%",
+ center: ["50%", "60%"],
+ selectedMode: "single",
+ data: [
+ { value: 335, name: "直接访问" },
Review comment:
> When all the data are negative, the pie chart is empty.
> I'm not sure if this is the expected result 🐒
At least, it's unexpected for me. This will break the previous behavior. And I think it's meaningful when all value is negative. In that case, considering them as `0` may be not a good way.
--
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.
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 #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-868193487
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.
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 #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#discussion_r652519737
##########
File path: src/chart/pie/PieView.ts
##########
@@ -242,6 +243,29 @@ class PieView extends ChartView {
}
}
+ // remove empty-circle(which is not instance of PiePiece) if it exists
+ group.children().forEach(child => {
+ if (!(child instanceof PiePiece)) {
Review comment:
It's not a good way to check if it's the empty-circle element by checking it's not a piece of the pie. It may have issues if we add other elements that are not a piece of the pie further.
A better way is to save this empty-circle element as a private property of PieView. It's simpler and more robust.
##########
File path: src/processor/negativeDataFilter.ts
##########
@@ -0,0 +1,44 @@
+/*
+* 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.
+*/
+
+import { StageHandler } from '../util/types';
+
+export default function negativeDataFilter(seriesType: string): StageHandler {
+ return {
+ seriesType: seriesType,
+ reset: function (seriesModel, ecModel) {
+ const legendModels = ecModel.findComponents({
+ mainType: 'legend'
+ });
+ if (!legendModels || !legendModels.length) {
Review comment:
I think it's not necessary to check if there is `legend`?
--
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.
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 #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
pissang merged pull request #15095:
URL: https://github.com/apache/echarts/pull/15095
--
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.
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 edited a comment on pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-867390596
@ssthouse I think we can wait for the developers' feedback.
Seems there is still a blocking change request from @Ovilia
--
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.
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] ssthouse commented on a change in pull request #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on a change in pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#discussion_r647497436
##########
File path: test/pie-minus-value.html
##########
@@ -0,0 +1,105 @@
+<!--
+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>
+ h1 {
+ line-height: 60px;
+ height: 60px;
+ background: #146402;
+ text-align: center;
+ font-weight: bold;
+ color: #eee;
+ font-size: 14px;
+ }
+ .chart {
+ height: 600px;
+ }
+ </style>
+
+ <div id="main" class="chart"></div>
+ <script>
+ require(["echarts"], function (echarts) {
+ var chart = echarts.init(document.getElementById("main"));
+
+ chart.setOption({
+ aria: {
+ enabled: true,
+ },
+ title: {
+ text: `The minus value slice should render like 0 value,
+ while the tooltip should show the origin minus value`,
+ x: "center",
+ },
+ tooltip: {
+ trigger: "item",
+ formatter: "{a} <br/>{b} : {c} ({d}%)",
+ },
+ legend: {
+ orient: "vertical",
+ left: "left",
+ data: [
+ "直接访问",
+ "邮件营销",
+ "联盟广告",
+ "视频广告(value is 0)",
+ "搜索引擎(value is minus)",
+ ],
+ },
+ series: [
+ {
+ name: "访问来源",
+ type: "pie",
+ radius: "55%",
+ center: ["50%", "60%"],
+ selectedMode: "single",
+ data: [
+ { value: 335, name: "直接访问" },
Review comment:
When all the data are negative, the pie chart is empty.
I'm not sure if this is the expected result 🐒
--
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.
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] Ovilia commented on a change in pull request #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
Ovilia commented on a change in pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#discussion_r646427323
##########
File path: src/chart/pie/pieLayout.ts
##########
@@ -72,8 +72,13 @@ export default function pieLayout(
data.each(valueDim, function (value: number) {
!isNaN(value) && validDataCount++;
});
+ let sum = 0;
+ data.each(valueDim, function (value: number) {
+ if (!isNaN(value) && value > 0) {
+ sum += value;
+ }
+ });
Review comment:
Maybe you don't need this extra loop? Just put this in the previous loop.
##########
File path: src/chart/pie/PieSeries.ts
##########
@@ -170,7 +170,13 @@ class PieSeriesModel extends SeriesModel<PieSeriesOption> {
const valueList: number[] = [];
data.each(data.mapDimension('value'), function (value: number) {
- valueList.push(value);
+ // handle minue value condition: teat minus value as 0 when calculation percent
+ if (!isNaN(value) && value < 0) {
+ valueList.push(0);
+ }
+ else {
+ valueList.push(value);
+ }
Review comment:
This could be simplified to be `valueList.push(isNaN(value) ? value : Math.max(value, 0));`
--
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.
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 pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-862152229
@ssthouse Here is a common way to set style from the option:
```ts
sector.useStyle(seriesModel.getModel('emptyCircleStyle').getItemStyle());
```
Some note:
+ Need to use `useStyle` here so the style properties can be reset to default when developers didn't set it in the new options.
+ `getItemStyle` get the full mapped style object from the model. It includes fill and stroke. Meanwhile, there are other two methods that can get a mapped style object. `getLineStyle` which only includes stroke. `getAreaStyle` which only includes fill.
--
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.
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] ssthouse commented on a change in pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on a change in pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#discussion_r650693834
##########
File path: test/pie-minus-value.html
##########
@@ -0,0 +1,105 @@
+<!--
+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>
+ h1 {
+ line-height: 60px;
+ height: 60px;
+ background: #146402;
+ text-align: center;
+ font-weight: bold;
+ color: #eee;
+ font-size: 14px;
+ }
+ .chart {
+ height: 600px;
+ }
+ </style>
+
+ <div id="main" class="chart"></div>
+ <script>
+ require(["echarts"], function (echarts) {
+ var chart = echarts.init(document.getElementById("main"));
+
+ chart.setOption({
+ aria: {
+ enabled: true,
+ },
+ title: {
+ text: `The minus value slice should render like 0 value,
+ while the tooltip should show the origin minus value`,
+ x: "center",
+ },
+ tooltip: {
+ trigger: "item",
+ formatter: "{a} <br/>{b} : {c} ({d}%)",
+ },
+ legend: {
+ orient: "vertical",
+ left: "left",
+ data: [
+ "直接访问",
+ "邮件营销",
+ "联盟广告",
+ "视频广告(value is 0)",
+ "搜索引擎(value is minus)",
+ ],
+ },
+ series: [
+ {
+ name: "访问来源",
+ type: "pie",
+ radius: "55%",
+ center: ["50%", "60%"],
+ selectedMode: "single",
+ data: [
+ { value: 335, name: "直接访问" },
Review comment:
@Ovilia this sounds reasonable, I'll give it a try~
> display an empty light gray circle when no data
For the color issue, all three legends' are in blue because they are all filtered by `negativeDataFilter`. And the filtered legend's default color is blue
--
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.
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 edited a comment on pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-867390596
@ssthouse I think we can wait for the developers' feedback. Seems there is still a blocking change request from @Ovilia
--
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.
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] ssthouse commented on pull request #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-856483466
> It seems this will break the case in which all the number is negative. Besides, if the negative value was considered as `0`, how does the tooltip show? Should it be `0`(as the pie shows) or the original negative value like `-1548`?
@plainheart hi, In my previous edition(see the first two commits of this PR).
the negative value is considered as `0` when calculating pie slices. And the tooltip shows the negative value label; like below:
---
according to @Ovilia and @pissang's suggestion, I moved the filter logic to `processor/negativeDataFilter`( see the 3rd commit of this PR).
and in this edition, the negative value will only be shown in legend and not in pie slices. like below:
I'm not sure which result is expected since they're both reasonable in some way，hope you guys can give me a conclusion😂
--
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.
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 #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-855696700
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.
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 #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#discussion_r647935424
##########
File path: test/pie-minus-value.html
##########
@@ -0,0 +1,105 @@
+<!--
+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>
+ h1 {
+ line-height: 60px;
+ height: 60px;
+ background: #146402;
+ text-align: center;
+ font-weight: bold;
+ color: #eee;
+ font-size: 14px;
+ }
+ .chart {
+ height: 600px;
+ }
+ </style>
+
+ <div id="main" class="chart"></div>
+ <script>
+ require(["echarts"], function (echarts) {
+ var chart = echarts.init(document.getElementById("main"));
+
+ chart.setOption({
+ aria: {
+ enabled: true,
+ },
+ title: {
+ text: `The minus value slice should render like 0 value,
+ while the tooltip should show the origin minus value`,
+ x: "center",
+ },
+ tooltip: {
+ trigger: "item",
+ formatter: "{a} <br/>{b} : {c} ({d}%)",
+ },
+ legend: {
+ orient: "vertical",
+ left: "left",
+ data: [
+ "直接访问",
+ "邮件营销",
+ "联盟广告",
+ "视频广告(value is 0)",
+ "搜索引擎(value is minus)",
+ ],
+ },
+ series: [
+ {
+ name: "访问来源",
+ type: "pie",
+ radius: "55%",
+ center: ["50%", "60%"],
+ selectedMode: "single",
+ data: [
+ { value: 335, name: "直接访问" },
Review comment:
> When all the data are negative, the pie chart is empty.
> I'm not sure if this is the expected result 🐒
At least, it's unexpected for me. This will break the previous behavior. And I think it's meaningful when all value is negative. In that case, considering them as `0` or filtering them may be not a good way.
--
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.
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] ssthouse commented on pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-861560031
@Ovilia Hi~
I added an empty light gray circle when the data count is 0 or all data are filtered (by `negativeDataFilter.ts`). It looks like this:
<img width="500" alt="wecom-temp-441629a7e1e40f5ef839a4027ad86d34" src="https://user-images.githubusercontent.com/10973821/122072227-f687ad80-ce29-11eb-83f0-8b6660641f2b.png">
---
This is what it looks when data are dynamically changed to be all negative.
---
This is what it looks when two data are negative and the other two are 0 (so they are summed to be 0):
--
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.
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 pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-861999831
@ssthouse Awesome! Can we have the ability to configure if display this empty circle and the style. Perhaps the options can be:
```
showEmptyCircle: true,
emptyCircleStyle: {
// same with itemStyle
}
```
--
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.
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 pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-919797529
@yangkai-bj 谢谢建议，欢迎 PR 增加该配置
--
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] ssthouse commented on pull request #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-856439119
@pissang hi, I create a new filter: `negativeDataFilter.ts` and register it in `pie/install.ts`, please check~
--
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.
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] ssthouse commented on pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-862191994
@pissang Thanks for your suggestions, really helpful!
I have added the config options, please check~
--
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.
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] Ovilia commented on a change in pull request #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
Ovilia commented on a change in pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#discussion_r647379047
##########
File path: src/processor/negativeDataFilter.ts
##########
@@ -0,0 +1,44 @@
+import { StageHandler } from '../util/types';
Review comment:
This should appear after the license header.
##########
File path: test/pie-minus-value.html
##########
@@ -0,0 +1,105 @@
+<!--
+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>
+ h1 {
+ line-height: 60px;
+ height: 60px;
+ background: #146402;
+ text-align: center;
+ font-weight: bold;
+ color: #eee;
+ font-size: 14px;
+ }
+ .chart {
+ height: 600px;
+ }
+ </style>
+
+ <div id="main" class="chart"></div>
+ <script>
+ require(["echarts"], function (echarts) {
+ var chart = echarts.init(document.getElementById("main"));
+
+ chart.setOption({
+ aria: {
+ enabled: true,
+ },
+ title: {
+ text: `The minus value slice should render like 0 value,
+ while the tooltip should show the origin minus value`,
+ x: "center",
+ },
+ tooltip: {
+ trigger: "item",
+ formatter: "{a} <br/>{b} : {c} ({d}%)",
+ },
+ legend: {
+ orient: "vertical",
+ left: "left",
+ data: [
+ "直接访问",
+ "邮件营销",
+ "联盟广告",
+ "视频广告(value is 0)",
+ "搜索引擎(value is minus)",
+ ],
+ },
+ series: [
+ {
+ name: "访问来源",
+ type: "pie",
+ radius: "55%",
+ center: ["50%", "60%"],
+ selectedMode: "single",
+ data: [
+ { value: 335, name: "直接访问" },
Review comment:
As @plainheart suggested, I think we should add a test case that all data are negative.
##########
File path: src/chart/pie/pieLayout.ts
##########
@@ -30,9 +30,9 @@ const RADIAN = Math.PI / 180;
function getViewRect(seriesModel: PieSeriesModel, api: ExtensionAPI) {
return layout.getLayoutRect(
seriesModel.getBoxLayoutParams(), {
- width: api.getWidth(),
- height: api.getHeight()
- }
+ width: api.getWidth(),
+ height: api.getHeight()
+ }
Review comment:
I think these three lines (or perhaps this file?) should not be changed.
--
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.
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] Ovilia commented on pull request #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
Ovilia commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-856711383
I think it's a little better if not show in the tooltip but show in legend.
--
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.
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] ssthouse commented on pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-862135253
> @ssthouse Awesome! Can we have the ability to configure if display this empty circle and the style. Perhaps the options can be:
>
> ```js
> showEmptyCircle: true,
> emptyCircleStyle: {
> // same with itemStyle
> }
> ```
@pissang Yes, sure~
I have a small question that may need your help. 🙏
About the option `emptyCircleStyle`. Currently, I implement the light-gray circle by creating a simple `graphic.Sector`, and the type of itemStyle (`ItemStyleOption`) seems can not be directly applied to `Sector`.
I checked the sector's API and found it accepts `PathStyleProps`, maybe we can expose `emptyCircleStyle: PathStyleProps` ?
--
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.
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] yangkai-bj commented on pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
yangkai-bj commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-919790688
--
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] ssthouse commented on a change in pull request #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on a change in pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#discussion_r647494510
##########
File path: src/chart/pie/pieLayout.ts
##########
@@ -30,9 +30,9 @@ const RADIAN = Math.PI / 180;
function getViewRect(seriesModel: PieSeriesModel, api: ExtensionAPI) {
return layout.getLayoutRect(
seriesModel.getBoxLayoutParams(), {
- width: api.getWidth(),
- height: api.getHeight()
- }
+ width: api.getWidth(),
+ height: api.getHeight()
+ }
Review comment:
fixed
##########
File path: src/processor/negativeDataFilter.ts
##########
@@ -0,0 +1,44 @@
+import { StageHandler } from '../util/types';
Review comment:
fixed
--
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.
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] ssthouse commented on pull request #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-855842641
@Ovilia hi, do you mean adding filter logic like this?
So I can discard the changed I've made in `src/chart/pie/pieLayout.ts` and `src/chart/pie/PieSeries.ts`
--
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.
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] Ovilia commented on pull request #15095: feat: pie-chart support minus value
Posted by GitBox <gi...@apache.org>.
Ovilia commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-855814004
@ssthouse Could you try to filter negative data with pie series in `processor/dataFilter.ts`? If it works, you don't need to change pieView and it would be simpler. Please be careful when checking the legend of negative data.
--
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.
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 edited a comment on pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-862152229
@ssthouse Here is a common way to set style from the option:
```ts
sector.useStyle(seriesModel.getModel('emptyCircleStyle').getItemStyle());
```
Some notes:
+ Need to use `useStyle` here so the style properties can be reset to default when developers didn't set it in the new options. Meanwhile `setStyle` will merge the new style to the existing style.
+ `seriesModel.getModel('emptyCircleStyle')` get a model subset which only includes `emptyCircleStyle` option.
+ `getItemStyle` get the full mapped style object from the option. It includes fill and stroke. Meanwhile, there are other two methods that can also get a mapped style object. `getLineStyle` which only includes stroke. `getAreaStyle` which only includes fill.
--
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.
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] ssthouse commented on pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
ssthouse commented on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-863711271
@pissang Should I add this config now or we wait for developers‘ feedback after release?
--
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.
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 edited a comment on pull request #15095: feat: pie-chart support negative value
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15095:
URL: https://github.com/apache/echarts/pull/15095#issuecomment-861999831
@ssthouse Awesome! Can we have the ability to configure if display this empty circle and the style. Perhaps the options can be:
```js
showEmptyCircle: true,
emptyCircleStyle: {
// same with itemStyle
}
```
--
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.
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