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/08/14 16:02:11 UTC
[GitHub] [echarts] susiwen8 opened a new pull request #15534: Feat: new selectedMode and selectable for item
susiwen8 opened a new pull request #15534:
URL: https://github.com/apache/echarts/pull/15534
<!-- Please fill in the following information to help us review your PR more efficiently. -->
## Brief Information
This pull request is in the type of:
- [ ] bug fixing
- [x] new feature
- [ ] others
### What does this PR do?
Since `5.0`, ECharts supports item `select`, It would be nice to have `selectable` option for each item. The idea came from #14712
Also add `series` for `selectedMode`, it's would make whole series `selected` by on click.
## Details
### After: How is it fixed in this PR?
## 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.
## 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.
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] susiwen8 commented on a change in pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780319342
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
Add `isSelectSeries` @pissang
--
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 pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1006664798
It's more common to use `enabled / disabled` instead of `enable / disable`. For example in highcharts https://api.highcharts.com/highcharts/boost.enabled
--
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 edited a comment on pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1005382047
Sorry for the late review. Seems to be a very promise feature. Take a quick look and here is one suggestion: Could `selectable` be in the root instead of under `itemStyle`. It's not a style related option and not all series use `itemStyle`. I think there are two options:
```
data: [{ selectable: false }]
```
Or
```
data: [ select: { disabled: true } ]
```
The later one can be designed together with https://github.com/apache/echarts/issues/14952 . Emphasis state can also be disabled.
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780875405
##########
File path: src/model/Series.ts
##########
@@ -558,6 +569,14 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
const data = this.getData(dataType);
+ if (data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])) {
Review comment:
It's better to do this check after `selectedMap` check because `getItemModel` and `Model#get` is much more expensive than simply object access from `selectedMap`. We need to avoid this call as much as possible.
##########
File path: src/model/Series.ts
##########
@@ -558,6 +569,14 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
const data = this.getData(dataType);
+ if (data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])) {
Review comment:
It's better to do this check after `selectedMap` check because `getItemModel` and `Model#get` is much more expensive than simply property access from `selectedMap`. We need to avoid this call as much as possible.
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1011139696
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] pissang commented on a change in pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r781715721
##########
File path: src/model/Series.ts
##########
@@ -569,16 +569,22 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
const data = this.getData(dataType);
- if (data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])) {
- return false;
- }
-
+
if (selectedMap === 'all') {
- return true;
+ return !data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled']);
}
const nameOrId = getSelectionKey(data, dataIndex);
- return selectedMap[nameOrId] || false;
+
+ if (selectedMap[nameOrId] != null) {
Review comment:
`selectedMap[nameOrId] != null` is a redundant check. The whole logic can be simplified to:
```ts
return (selectedMap === 'all' || selectedMap[getSelectionKey(data, dataIndex)])
&& !data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])
```
Whener you see a long line like `data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])` multiple times in one function. Try to reduce it to one. Always try to keep the code as neat as possible.
--
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 pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1010571651
@susiwen8 BTW it's not correct to use `chore` prefix in your commit message here. `chore` means `updating grunt tasks etc; no production code change`
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780875405
##########
File path: src/model/Series.ts
##########
@@ -558,6 +569,14 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
const data = this.getData(dataType);
+ if (data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])) {
Review comment:
We can optimize it by doing this check after `selectedMap` check because `getItemModel` and `Model#get` is much more expensive than simply property access from `selectedMap`. We need to avoid this call as much as possible.
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780735571
##########
File path: src/model/Series.ts
##########
@@ -558,6 +569,14 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
const data = this.getData(dataType);
+ if ((data.getItemModel(dataIndex) as Model).get(['select', 'disabled']) === true) {
+ return false;
+ }
Review comment:
@plainheart I think https://github.com/apache/echarts/pull/15534#discussion_r780309973 can explain it.
BTW we usually don't check `true` explicitly unless the value can have other types than boolean. Check `truthy` is more commonly used for boolean only value.
```ts
if (...get(['select', 'disabled']) { ... }
```
--
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] susiwen8 commented on a change in pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r783103143
##########
File path: src/model/Series.ts
##########
@@ -599,9 +618,12 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
}
else if (selectedMode === 'single' || selectedMode === true) {
+ if (zrUtil.isString(option.selectedMap)) {
+ option.selectedMap = {};
+ }
Review comment:
resolved
--
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] susiwen8 commented on a change in pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780263814
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
But if some datas have specified `select.disable = false`? Just like the test case I have added. @pissang
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780337067
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
selected states should be saved in the option because it is the single source of truth. Why not using selectedMap: 'all'?
--
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 removed a comment on pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang removed a comment on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1007495563
selected states should be saved in the option because it's is the single source of truth. Why not using selectedMap: 'all'?
--
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 pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1005852456
@susiwen8 We aready have `select` for each data item. Where we can sepcify selected color and other styles. That's why I prefer using `select.disabled`. And it will be more consistent with `emphasis.disabled`.
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780689028
##########
File path: src/model/Series.ts
##########
@@ -582,26 +601,42 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
private _innerSelect(data: SeriesData, innerDataIndices: number[]) {
- const selectedMode = this.option.selectedMode;
+ const option = this.option;
+ const selectedMode = option.selectedMode;
const len = innerDataIndices.length;
if (!selectedMode || !len) {
return;
}
- if (selectedMode === 'multiple') {
- const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ if (selectedMode === 'series') {
+ option.selectedMap = 'all';
+ }
+ else if (selectedMode === 'multiple') {
+ if (!isObject(option.selectedMap)) {
+ option.selectedMap = {};
+ }
+ const selectedMap = option.selectedMap;
for (let i = 0; i < len; i++) {
const dataIndex = innerDataIndices[i];
+ if ((data.getItemModel(dataIndex) as Model).get(['select', 'disabled']) === true) {
+ return;
Review comment:
Just take a shallow review. Should it be `continue` rather than `return`?
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780337067
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
selected states should be saved in the option because it's is the single source of truth. Why not using selectedMap: 'all'?
--
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 edited a comment on pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1010571651
@susiwen8 BTW it's not correct to use `chore` prefix in the commit message here. `chore` means `updating grunt tasks etc; no production code change`
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang merged pull request #15534:
URL: https://github.com/apache/echarts/pull/15534
--
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 edited a comment on pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1005382047
Sorry for the late review. Seems to be a very promise feature. Take a quick look and here is one suggestion:
Could `selectable` be in the root instead of under `itemStyle`. It's not a style related option and not all series use `itemStyle`. I think there are two options:
```
data: [{ selectable: false }]
```
Or
```
data: [ select: { disabled: true } ]
```
The later one can be designed together with https://github.com/apache/echarts/issues/14952 . Emphasis state can also be disabled.
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780690167
##########
File path: src/model/Series.ts
##########
@@ -56,6 +56,7 @@ import { defaultSeriesFormatTooltip } from '../component/tooltip/seriesFormatToo
import {ECSymbol} from '../util/symbol';
import {Group} from '../util/graphic';
import {LegendIconParams} from '../component/legend/LegendModel';
+import { isObject, isString } from 'zrender/src/core/util';
Review comment:
Duplicated import: line 20 has imported utils from zrender.
##########
File path: src/model/Series.ts
##########
@@ -558,6 +569,14 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
const data = this.getData(dataType);
+ if ((data.getItemModel(dataIndex) as Model).get(['select', 'disabled']) === true) {
+ return false;
+ }
Review comment:
It seems this logic isn't necessary as if a data item disables `select` state, the `selectedMap` won't contain it. (See `_innerSelect` function). Correct me if I'm missing something.
##########
File path: src/model/Series.ts
##########
@@ -582,26 +601,42 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
private _innerSelect(data: SeriesData, innerDataIndices: number[]) {
- const selectedMode = this.option.selectedMode;
+ const option = this.option;
+ const selectedMode = option.selectedMode;
const len = innerDataIndices.length;
if (!selectedMode || !len) {
return;
}
- if (selectedMode === 'multiple') {
- const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ if (selectedMode === 'series') {
+ option.selectedMap = 'all';
+ }
+ else if (selectedMode === 'multiple') {
+ if (!isObject(option.selectedMap)) {
+ option.selectedMap = {};
+ }
+ const selectedMap = option.selectedMap;
for (let i = 0; i < len; i++) {
const dataIndex = innerDataIndices[i];
+ if ((data.getItemModel(dataIndex) as Model).get(['select', 'disabled']) === true) {
+ return;
+ }
// TODO diffrent types of data share same object.
const nameOrId = getSelectionKey(data, dataIndex);
selectedMap[nameOrId] = true;
this._selectedDataIndicesMap[nameOrId] = data.getRawIndex(dataIndex);
}
}
else if (selectedMode === 'single' || selectedMode === true) {
+ if (isString(option.selectedMap)) {
+ option.selectedMap = {};
+ }
const lastDataIndex = innerDataIndices[len - 1];
+ if ((data.getItemModel(lastDataIndex) as Model).get(['select', 'disabled']) === true) {
Review comment:
Current `as Model` lacks the type check. Perhaps we can remove this `as` and specify a generic type for `getItemModel`.
```ts
data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])
```
Besides, I personally think we can remove `=== true`, that is, it allows the **truthy** value, e.g, `1`. And this also reduces the code slightly.
##########
File path: src/model/Series.ts
##########
@@ -582,26 +601,42 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
private _innerSelect(data: SeriesData, innerDataIndices: number[]) {
- const selectedMode = this.option.selectedMode;
+ const option = this.option;
+ const selectedMode = option.selectedMode;
const len = innerDataIndices.length;
if (!selectedMode || !len) {
return;
}
- if (selectedMode === 'multiple') {
- const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ if (selectedMode === 'series') {
+ option.selectedMap = 'all';
+ }
+ else if (selectedMode === 'multiple') {
+ if (!isObject(option.selectedMap)) {
+ option.selectedMap = {};
+ }
+ const selectedMap = option.selectedMap;
for (let i = 0; i < len; i++) {
const dataIndex = innerDataIndices[i];
+ if ((data.getItemModel(dataIndex) as Model).get(['select', 'disabled']) === true) {
+ return;
Review comment:
Just take a shallow review. Should it be `break` rather than `return`?
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780875628
##########
File path: src/model/Series.ts
##########
@@ -582,26 +601,42 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
private _innerSelect(data: SeriesData, innerDataIndices: number[]) {
- const selectedMode = this.option.selectedMode;
+ const option = this.option;
+ const selectedMode = option.selectedMode;
const len = innerDataIndices.length;
if (!selectedMode || !len) {
return;
}
- if (selectedMode === 'multiple') {
- const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ if (selectedMode === 'series') {
+ option.selectedMap = 'all';
+ }
+ else if (selectedMode === 'multiple') {
+ if (!zrUtil.isObject(option.selectedMap)) {
+ option.selectedMap = {};
+ }
+ const selectedMap = option.selectedMap;
for (let i = 0; i < len; i++) {
const dataIndex = innerDataIndices[i];
+ if (data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])) {
Review comment:
I think we can remove the check here and only do the check in the `isSelected` method. The downside the stored `selectedMap` and `_selectedDataIndicesMap` will ignore the `select.disabled`. But it becomes consistent between `'series'`, `'multiple'` and `'single'` and it's simpler.
##########
File path: src/model/Series.ts
##########
@@ -558,6 +569,14 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
const data = this.getData(dataType);
+ if (data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])) {
Review comment:
It's better to do this check after `selectedMap` check because `getItemModel` and `Model#get` is much more expensive than simply object access in `selectedMap`. We need to avoid this call as much as possible.
--
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 pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1007495563
selected states should be saved in the option because it's is the single source of truth. Why not using selectedMap: 'all'?
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780671438
##########
File path: src/model/Series.ts
##########
@@ -588,18 +604,33 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
- const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ if (selectedMode === 'series') {
+ this.option.selectedMap = 'all';
+ }
+ else if (selectedMode === 'multiple') {
+ if (isString(this.option.selectedMap)) {
+ this.option.selectedMap = {};
+ }
+ const selectedMap = (this.option.selectedMap || (this.option.selectedMap = {}));
for (let i = 0; i < len; i++) {
const dataIndex = innerDataIndices[i];
+ if ((data.getItemModel(dataIndex) as Model).get(['select', 'disabled']) === true) {
+ return;
+ }
// TODO diffrent types of data share same object.
const nameOrId = getSelectionKey(data, dataIndex);
selectedMap[nameOrId] = true;
this._selectedDataIndicesMap[nameOrId] = data.getRawIndex(dataIndex);
}
}
else if (selectedMode === 'single' || selectedMode === true) {
+ if (isString(this.option.selectedMap)) {
Review comment:
It's unecessary here?
##########
File path: src/model/Series.ts
##########
@@ -515,11 +515,19 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
unselect(innerDataIndices: number[], dataType?: SeriesDataType): void {
- const selectedMap = this.option.selectedMap;
+ const selectedMap = this.option.selectedMap as Dictionary<boolean>;
Review comment:
Assign `this.option` to a temporal variable will have less code.
##########
File path: src/model/Series.ts
##########
@@ -588,18 +604,33 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
- const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ if (selectedMode === 'series') {
+ this.option.selectedMap = 'all';
+ }
+ else if (selectedMode === 'multiple') {
+ if (isString(this.option.selectedMap)) {
Review comment:
It will be simpler to check `isObject` instead of checking `string` and `null` twice
```ts
const selectedMap = this.option.selectedMap;
if (!isObject(selectedMap)) {
selectedMap = this.option.selectedMap = {};
}
```
##########
File path: src/model/Series.ts
##########
@@ -588,18 +604,33 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
- const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ if (selectedMode === 'series') {
+ this.option.selectedMap = 'all';
Review comment:
Assign `this.option` to a temporal variable will have less code.
##########
File path: src/model/Series.ts
##########
@@ -588,18 +604,33 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
- const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ if (selectedMode === 'series') {
+ this.option.selectedMap = 'all';
Review comment:
Seems `getSelectedDataIndices` doesn't handle the case `selectedMap: 'all'` as I suggested.
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r782787776
##########
File path: src/model/Series.ts
##########
@@ -599,9 +618,12 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
}
else if (selectedMode === 'single' || selectedMode === true) {
+ if (zrUtil.isString(option.selectedMap)) {
+ option.selectedMap = {};
+ }
Review comment:
This seems unnecessary as the following lines 626-628 will always override this assignment. Right?
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-898912096
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).
The pull request is marked to be `PR: author is committer` because you are a committer of this project.
--
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] susiwen8 commented on pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1006614328
@pissang Please check again
--
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] susiwen8 commented on pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1006668727
@pissang Oops I was meant to use `disabled`
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780735571
##########
File path: src/model/Series.ts
##########
@@ -558,6 +569,14 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
const data = this.getData(dataType);
+ if ((data.getItemModel(dataIndex) as Model).get(['select', 'disabled']) === true) {
+ return false;
+ }
Review comment:
I think https://github.com/apache/echarts/pull/15534#discussion_r780309973 can explain it.
BTW we usually don't check `true` explicitly unless the value can have other types than boolean. Check `truthy` is more commonly used for boolean only value.
```ts
if (...get(['select', 'disabled']) { ... }
```
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780735392
##########
File path: src/model/Series.ts
##########
@@ -558,6 +569,14 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
const data = this.getData(dataType);
+ if ((data.getItemModel(dataIndex) as Model).get(['select', 'disabled']) === true) {
+ return false;
+ }
Review comment:
I think https://github.com/apache/echarts/pull/15534#discussion_r780309973 can explain 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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780671917
##########
File path: src/legacy/dataSelectAction.ts
##########
@@ -86,7 +86,7 @@ function handleSeriesLegacySelectEvents(
type: legacyEventName,
seriesId: seriesModel.id,
name: isArray(dataIndex) ? data.getName(dataIndex[0]) : data.getName(dataIndex),
- selected: extend({}, seriesModel.option.selectedMap)
+ selected: extend({}, seriesModel.option as Dictionary<boolean>)
Review comment:
Must be very careful when using `as`. Here the code is obviously wrong
--
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] susiwen8 commented on a change in pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780674516
##########
File path: src/model/Series.ts
##########
@@ -588,18 +604,33 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
- const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ if (selectedMode === 'series') {
+ this.option.selectedMap = 'all';
+ }
+ else if (selectedMode === 'multiple') {
+ if (isString(this.option.selectedMap)) {
+ this.option.selectedMap = {};
+ }
+ const selectedMap = (this.option.selectedMap || (this.option.selectedMap = {}));
for (let i = 0; i < len; i++) {
const dataIndex = innerDataIndices[i];
+ if ((data.getItemModel(dataIndex) as Model).get(['select', 'disabled']) === true) {
+ return;
+ }
// TODO diffrent types of data share same object.
const nameOrId = getSelectionKey(data, dataIndex);
selectedMap[nameOrId] = true;
this._selectedDataIndicesMap[nameOrId] = data.getRawIndex(dataIndex);
}
}
else if (selectedMode === 'single' || selectedMode === true) {
+ if (isString(this.option.selectedMap)) {
Review comment:
Yes, It is, cause user may update `selectedMode` from `series` to `single`. I will add a test case for 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] susiwen8 commented on a change in pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780263814
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
But if some data have specify `select.disable = false`? Just like the test case I have added. @pissang
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r781715721
##########
File path: src/model/Series.ts
##########
@@ -569,16 +569,22 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
const data = this.getData(dataType);
- if (data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])) {
- return false;
- }
-
+
if (selectedMap === 'all') {
- return true;
+ return !data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled']);
}
const nameOrId = getSelectionKey(data, dataIndex);
- return selectedMap[nameOrId] || false;
+
+ if (selectedMap[nameOrId] != null) {
Review comment:
`selectedMap[nameOrId] != null` is a redundant check. The whole logic can be simplified to:
```ts
return (selectedMap === 'all' || selectedMap[getSelectionKey(data, dataIndex)])
&& !data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])
```
Whener you see a long line like `data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])` multiple times in one function. Try to reduce it to one. And always try to keep the code as neat as possible.
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r781715721
##########
File path: src/model/Series.ts
##########
@@ -569,16 +569,22 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
const data = this.getData(dataType);
- if (data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])) {
- return false;
- }
-
+
if (selectedMap === 'all') {
- return true;
+ return !data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled']);
}
const nameOrId = getSelectionKey(data, dataIndex);
- return selectedMap[nameOrId] || false;
+
+ if (selectedMap[nameOrId] != null) {
Review comment:
`selectedMap[nameOrId] != null` is a redundant check. The whole logic can be simplified to:
```ts
return (selectedMap === 'all' || selectedMap[getSelectionKey(data, dataIndex)])
&& !data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])
```
Whener you see a long line like `data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])` multiple times in one function. Try to reduce it to one.
--
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 removed a comment on pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang removed a comment on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1007495563
selected states should be saved in the option because it's is the single source of truth. Why not using selectedMap: 'all'?
--
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] susiwen8 commented on a change in pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780662103
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
Right, using `selectedMap` now @pissang
--
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 pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1005382047
Sorry for the late review. Seems to be a very promise feature. Take a quick look and here is one suggestion: Could `selectable` be in the root instead of under `itemStyle`. It's not a style related option and not all series use `itemStyle`. I think there are two options:
```
data: [{ selectable: false }]
```
Or
```
data: [ select: { disabled: true } ]
```
The later one can be designed together with https://github.com/apache/echarts/issues/14952 . Emphasi state can also be disabled.
--
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 edited a comment on pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1005382047
Sorry for the late review. Seems to be a very promise feature. Take a quick look and here is one suggestion:
Could `selectable` be in the root instead of under `itemStyle`. It's not a style related option and not all series use `itemStyle`. I think there are two options:
```
data: [{ selectable: false }]
```
Or
```
data: [ select: { disabled: true } ]
```
This can be designed together with https://github.com/apache/echarts/issues/14952 . Emphasis state can also be disabled.
--
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] susiwen8 commented on a change in pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780263814
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
But if some datas have specified `select.disable = false`? Just like the one in test case. @pissang
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780054834
##########
File path: src/model/Series.ts
##########
@@ -519,7 +519,18 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
if (!selectedMap) {
return;
}
+ const selectedMode = this.option.selectedMode;
+
const data = this.getData(dataType);
+ if (selectedMode === 'series') {
+ data.each(idx => {
Review comment:
It's simpler and faster to just assign an empty object to `selectedMap` and `_selectedDataIndicesMap` if wan't to unselect all.
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
I think we should use a special reserved value to indicate all data are selected in this series. For example:
```js
selectedMap = 'all'
```
It will be much faster and simpler than traverse all data. Also it has benefits that when new data is added. It's selected automatically. However current implentation will not include this new data.
Then we can check if selectedMap is 'all' in `isSelected`. Also `_selectedDataIndicesMap` is not necessary in this case. `getSelectedDataIndices ` can return all indices of 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.
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] susiwen8 commented on a change in pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r778873173
##########
File path: src/util/types.ts
##########
@@ -618,6 +618,7 @@ export type OptionDataItemObject<T> = {
groupId?: OptionId;
value?: T[] | T;
selected?: boolean;
+ selectable?: boolean;
Review comment:
Not sure I put `selectable` at right type. @pissang
--
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 edited a comment on pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1005852456
@susiwen8 We aready have `select` for each data item. Where we can sepcify selected color and other styles. From my side I prefer using `select.disabled`. And it will be more consistent with `emphasis.disabled`.
--
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] susiwen8 commented on a change in pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780674782
##########
File path: src/legacy/dataSelectAction.ts
##########
@@ -86,7 +86,7 @@ function handleSeriesLegacySelectEvents(
type: legacyEventName,
seriesId: seriesModel.id,
name: isArray(dataIndex) ? data.getName(dataIndex[0]) : data.getName(dataIndex),
- selected: extend({}, seriesModel.option.selectedMap)
+ selected: extend({}, seriesModel.option as Dictionary<boolean>)
Review comment:
Sorry, my bad
--
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] susiwen8 commented on a change in pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r781185083
##########
File path: src/model/Series.ts
##########
@@ -558,6 +569,14 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
const data = this.getData(dataType);
+ if (data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])) {
Review comment:
If we put `selectedMap` at front, it will select the one whose `selected` is disable.
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r781222031
##########
File path: src/model/Series.ts
##########
@@ -558,6 +569,14 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
const data = this.getData(dataType);
+ if (data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])) {
Review comment:
How this will happen? disabled is still checked. But it's only checked when it's in the selectedMap or selectedMap is 'all'
--
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] susiwen8 commented on pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1005750238
@pissang I prefer first one `data: [{ selectable: false }]`
However the second proposal can provide more fine grade control for item selected state, such as specify `select color` for single data item
--
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 edited a comment on pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1005382047
Sorry for the late review. Seems to be a very promise feature. Take a quick look and here is one suggestion:
Could `selectable` be in the root instead of under `itemStyle`. It's not a style related option and not all series use `itemStyle`. I think there are two options:
```
data: [{ selectable: false }]
```
Or
```
data: [ select: { disabled: true } ]
```
This can be designed together with https://github.com/apache/echarts/issues/14952 . Emphasis state can also be disabled:
```
emphasis: { disabled: true }
```
Or
```
hoverEmphasis: false
```
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780309973
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
It doesn't matter, disabled can be checked in the isSelected method
--
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] susiwen8 commented on a change in pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r781225402
##########
File path: src/model/Series.ts
##########
@@ -558,6 +569,14 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
}
const data = this.getData(dataType);
+ if (data.getItemModel<StatesOptionMixin<unknown, unknown>>(dataIndex).get(['select', 'disabled'])) {
Review comment:
Ok, I get 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] susiwen8 commented on a change in pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780263814
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
But if some data have specify `select.disable = false`? Just like the test case I have added. @pissang
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
But if some datas have specified `select.disable = false`? Just like the test case I have added. @pissang
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
But if some datas have specified `select.disable = false`? Just like the one in test case. @pissang
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
Add `isSelectSeries` @pissang
--
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 pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1010566668
LGTM now. @plainheart any more comments?
--
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 #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#discussion_r780054834
##########
File path: src/model/Series.ts
##########
@@ -519,7 +519,18 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
if (!selectedMap) {
return;
}
+ const selectedMode = this.option.selectedMode;
+
const data = this.getData(dataType);
+ if (selectedMode === 'series') {
+ data.each(idx => {
Review comment:
It's simpler and faster to just assign an empty object to `selectedMap` and `_selectedDataIndicesMap` if wan't to unselect all.
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
I think we should use a special reserved value to indicate all data are selected in this series. For example:
```js
selectedMap = 'all'
```
It will be much faster and simpler than traverse all data. Also it has benefits that when new data is added. It's selected automatically. However current implentation will not include this new data.
Then we can check if selectedMap is 'all' in `isSelected`. Also `_selectedDataIndicesMap` is not necessary in this case. `getSelectedDataIndices ` can return all indices of data.
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
It doesn't matter, disabled can be checked in the isSelected method
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
selected states should be saved in the option because it's is the single source of truth. Why not using selectedMap: 'all'?
##########
File path: src/model/Series.ts
##########
@@ -588,10 +599,26 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption> extends ComponentMode
return;
}
- if (selectedMode === 'multiple') {
+ if (selectedMode === 'series') {
+ const selectedMap = this.option.selectedMap || (this.option.selectedMap = {});
+ const self = this;
+ data.each(idx => {
Review comment:
selected states should be saved in the option because it is the single source of truth. Why not using selectedMap: 'all'?
--
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 pull request #15534: Feat: new selectedMode and selectable for item
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #15534:
URL: https://github.com/apache/echarts/pull/15534#issuecomment-1007495563
selected states should be saved in the option because it's is the single source of truth. Why not using selectedMap: 'all'?
--
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