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 2020/01/03 06:26:35 UTC
[GitHub] [incubator-echarts] yufeng04 opened a new pull request #11973: fix
bug #11528 Modify default value of tooltip and label
yufeng04 opened a new pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973
<!-- 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?
Modify default value of tooltip and label when using default configuration.
<!-- USE ONCE SENTENCE TO DESCRIBE WHAT THIS PR DOES. -->
### Fixed issues
fixed issues #11528
<!--
- #xxxx: ...
-->
## Details
### Before: What was the problem?
In scatter chart, the label and tooltip use the value of 'y' when using default configuration.
<!-- DESCRIBE THE BUG OR REQUIREMENT HERE. -->
<!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
### After: How is it fixed in this PR?
Select the last value of data dimension as the label and tooltip by default
<!-- THE RESULT AFTER FIXING AND A SIMPLE EXPLANATION ABOUT HOW IT IS FIXED. -->
<!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
## Usage
### Are there any API changes?
- [ ] The API has been changed.
<!-- LIST THE API CHANGES HERE -->
### 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r362733325
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
@100pah It's a change since we introduced `encode` in 4.0 . It breaks all `scatter` series. Not only the `bmap` one.
In the 3.x version and earlier. We use the first two values to represent the coordinates and the third one as the value. It will be displayed in label and tooltip.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] github-actions[bot] closed pull request #11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #11973:
URL: https://github.com/apache/echarts/pull/11973
--
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] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r362733325
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
@100pah It's a change since we introduced `encode` in 4.0 . It breaks all `scatter` series. Not only the `bmap` one.
In the 3.x version and earlier. We use the first two values to represent the `[x, y]` coordinates and the third one as the value. It will be displayed in the label and tooltip.
In the logic of https://github.com/apache/incubator-echarts/blob/master/src/data/helper/dimensionHelper.js#L53 . I think the third value should be an extra dimension besides [x, y] coords. But I'm not sure why the label is still using the `y` value.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] LianJilu commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
LianJilu commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r362723277
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
Or can we modify the bmap dimension #11939
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r363576700
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
> So can we solve it by using the dimension that has not been encoded yet?
For example, If we have three value in each item of `series.data`.
```js
[x, y, z]
```
Because the data is not shared by other series. After taking `[x, y]` and encoded to xAxis/yAxis. We can easily guess the third value `z` is an extra value which can be displayed as label and tooltip. We can treat it as a function `z = f(x, y)`
But in the `dataset`, because each dimension may be taken by other series. We can not simply use the third dimension, or the last dimension. So can we take a further step and find if any dimension in the `dataset` has been taken by other series and being encoded?
>I think the concept and behavior of the attribute encode should better be consistent either using series.data or dataset. Otherwise it may bring confuse and more learning cost to users. But for the "default strategy when no encode" I think probably we need to trade it differently for series.data and dataset if necessary.
I agree with this. If developer has specified the encode property. We should do it consistently on `series.data` and `dataset`
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r363202621
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
> encode is not necessary in dataset case.
I mean encode is a very common used concept and configuration in the scene using dataset. It's necessary when we want to use another dimension to represent the value besides the `x/y` dimension.
But in the code without `dataset`, which is more common in our examples. It's not necessary to use encode to specify the value dimension in our earlier version. That's why our examples are broken.
> when we guessing which dimension can be the "default label", we detect that if the data is from dataset, use the current way, otherwise, if the data is from series itself (that means the data will not shared by other series), use the laster dimension as the label.
Perhaps it can solve the issue. Can you show a possible code to achieve this?
But I think the key issue is how to deal with the data from series and data from `dataset`. It seems currently we use the same encoding strategy on them. But they are different because data from `dataset` can be shared by 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r363202621
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
> encode is not necessary in dataset case.
I mean encode is a very common used concept and configuration in the scene using dataset. It's necessary when we want to use another dimension to represent the value besides the `x/y` dimension.
But in the code without `dataset`, which is more common in our examples. It's not necessary to use encode to specify the value dimension in our earlier version. That's why our examples are broken.
> when we guessing which dimension can be the "default label", we detect that if the data is from dataset, use the current way, otherwise, if the data is from series itself (that means the data will not shared by other series), use the laster dimension as the label.
Perhaps it can solve the issue. Can you show a possible code to achieve this?
I think the key issue is how to deal with the data from series and data from `dataset`. It seems currently we use the same encoding strategy on them. But they are different because data from `dataset` can be shared by different series. So can we use the dimension that has not been encoded yet?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r363202621
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
> encode is not necessary in dataset case.
I mean encode is a very common used concept and configuration in the scene using dataset. It's necessary when we want to use another dimension to represent the value besides the `x/y` dimension.
But in the code without `dataset`, which is more common in our examples. It's not necessary to use encode to specify the value dimension in our earlier version. That's why our examples are broken.
> when we guessing which dimension can be the "default label", we detect that if the data is from dataset, use the current way, otherwise, if the data is from series itself (that means the data will not shared by other series), use the laster dimension as the label.
Perhaps it can solve the issue. Can you show a possible code to achieve this?
I think the key issue is how to deal with the data from series and data from `dataset`. It seems currently we use the same encoding strategy on them. But they are different because data from `dataset` can be shared by 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r362733325
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
@100pah It's a change since we introduced `encode` in 4.0 . It breaks all `scatter` series. Not only the `bmap` one.
In the 3.x version and earlier. We use the first two values to represent the coordinates and the third one as the value. It will be displayed in the label and tooltip.
In the logic of https://github.com/apache/incubator-echarts/blob/master/src/data/helper/dimensionHelper.js#L53 . I think the third value should be an extra dimension besides [x, y] coords. But I'm not sure why the label is still using the `y` value.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] 100pah commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
100pah commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r363178407
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
@pissang
> In the dataset case. encode is a necessary configuration.
`encode` is not necessary in `dataset` case.
```js
option = {
xAxis: {},
yAxis: {},
dataset: {
source: [
// series0x series0y series1x series1y series2x series2y
[11, 22, 33, 99, 33, 91 ],
[121, 62, 93, 99, 83, 91 ],
[16, 52, 33, 99, 33, 91 ],
]
},
series: [{
type: 'scatter'
}, {
type: 'scatter'
}, {
type: 'scatter'
}]
};
```
is a common usage.
A possible way for this issue could be:
when we guessing which dimension can be the "default label", we detect that if the data is from dataset, use the current way, otherwise, if the data is from series itself (that means the data will not shared by other series), use the laster dimension as the label.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r363576700
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
> So can we solve it by using the dimension that has not been encoded yet?
For example, If we have three value in each item of `series.data`.
```js
[x, y, z]
```
Because the data is not shared by other series. After taking `[x, y]` and encoded to xAxis/yAxis. We can easily guess the third value `z` is an extra value which can be displayed as label and tooltip. We can treat it as a function `z = f(x, y)`
But in the `dataset`, because each dimension may be taken by other series. We can not simply use the third dimension, or the last dimension. So can we take a further step and find if any dimension in the `dataset` has been taken by other series and being encoded?
>I think the concept and behavior of the attribute encode should better be consistent either using series.data or dataset. Otherwise it may bring confuse and more learning cost to users. But for the "default strategy when no encode" I think probably we need to trade it differently for series.data and dataset if necessary.
I agree with this. If developer has specified the `encode` property. We should do it consistently on `series.data` and `dataset`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r362733325
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
@100pah It's a change since we introduced `encode` in 4.0 . It breaks all `scatter` series. Not only the `bmap` one.
In the `dataset` case. `encode` is a necessary configuration. But in the case without `dataset`. encode is not used so frequently. That's why some of our case broken.
In the 3.x version and earlier. We use the first two values to represent the `[x, y]` coordinates and the third one as the value. It will be displayed in the label and tooltip.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] github-actions[bot] commented on pull request #11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11973:
URL: https://github.com/apache/echarts/pull/11973#issuecomment-1006933855
This PR has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If you wish not to mark it as stale, please leave a comment in this PR. We are sorry for this but 2 years is a long time and the code base has been changed a lot. Thanks for your contribution anyway.
--
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] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r362733325
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
@100pah It's a change since we introduced `encode` in 4.0 . It breaks all `scatter` series. Not only the `bmap` one.
In the 3.x version and earlier. We use the first two values to represent the `[x, y]` coordinates and the third one as the value. It will be displayed in the label and tooltip.
Maybe we can use the first value which is not encoded to coords to be displayed as label.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r362733325
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
@100pah It's a change since we introduced `encode` in 4.0 . It breaks all `scatter` series. Not only the `bmap` one.
In the `dataset` case. `encode` is a necessary configuration. But in the case without `dataset`. `encode` is not used so frequently. That's why some of our case broken.
In the 3.x version and earlier. We use the first two values to represent the `[x, y]` coordinates and the third one as the value. It will be displayed in the label and tooltip.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] 100pah commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
100pah commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r362718449
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
(1) the logic `dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : ...` is to calculate a "default label" , so the code should be placed on where the `defaultedLabel` calculated rather here.
(2) We should not simply use the last dimension at the default label. Consider the dataset case:
```js
dataset: {
source: [
// series0x series0y series1x series1y series2x series2y
[11, 22, 33, 99, 33, 91 ],
[121, 62, 93, 99, 83, 91 ],
[16, 52, 33, 99, 33, 91 ],
]
}
```
If using the last dimension as the label, all of the series will use the `series2y` as their label.
It a little difficulty to guess which dimension can be the default label.
Users can set label dimension by
```js
encode: {label: 2} means dimesion 2
```
or
```js
formatter: '{@[2]}' // means dimesion 2
```
So do we need to fix that on code?
We might only fix #11528 on example?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r362733325
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
@100pah It's a change since we introduced `encode` in 4.0 . It breaks all `scatter` series. Not only the `bmap` one.
In the 3.x version and earlier. We use the first two values to represent the `[x, y]` coordinates and the third one as the value. It will be displayed in the label and tooltip.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r363202621
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
> encode is not necessary in dataset case.
I mean encode is a very common used concept and configuration in the scene using dataset. It's necessary when we want to use another dimension to represent the value besides the `x/y` dimension.
But in the code without `dataset`, which is more common in our examples. It's not necessary to use encode to specify the value dimension in our earlier version. That's why our examples are broken.
> when we guessing which dimension can be the "default label", we detect that if the data is from dataset, use the current way, otherwise, if the data is from series itself (that means the data will not shared by other series), use the laster dimension as the label.
Perhaps it can solve the issue. Can you show a possible code to achieve this?
I think the key issue is how to deal with the data from series and data from `dataset`. It seems currently we use the same encoding strategy on them. But they are different because data from `dataset` can be shared by different series.
So can we solve it by using the dimension that has not been encoded yet?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] 100pah commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
100pah commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r363364428
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
@pissang
> So can we solve it by using the dimension that has not been encoded yet?
I am not understand what it means yet. Could you tell me more?
> It seems currently we use the same encoding strategy on them.
The attribute `encode` and the encode strategy is introduced to echarts earlier than `dataset`. It originally to enable users specify any columns to map x/y/labels/tooltips, rather than have to travel and exchange the order of the columns of the raw data. That especially helps the scenario that users need different types of mappings in a single case.
I think the concept and behavior of the attribute `encode` should better be consistent either using `series.data` or `dataset`. Otherwise it may bring confuse and more learning cost to users.
But for the "default strategy when no encode" I think probably we need to trade it differently for `series.data` and `dataset` if necessary.
@yufeng04 @pissang
I think the fix could be:
(1) Add `isFromDataset` method to `List` class:
```js
+ listProto.isFromDataset = function () {
+ var provider = this._rawData;
+ // There might be no `getSource` in some legacy provider input from upper application.
+ return provider.getSource && provider.getSource().fromDataset;
+ };
```
(2) Modify the logic about "defaultedLabel" in `summarizeDimensions` of `dimensionHelper.js`:
```js
export function summarizeDimensions(data) {
var summary = {};
var encode = summary.encode = {};
var notExtraCoordDimMap = createHashMap();
var defaultedLabel = [];
var defaultedTooltip = [];
// See the comment of `List.js#userOutput`.
var userOutput = summary.userOutput = {
dimensionNames: data.dimensions.slice(),
encode: {}
};
+ var fromDataSet = this.isFromDataSet();
+ var defaultedLabelDimNonDataset;
each(data.dimensions, function (dimName) {
var dimItem = data.getDimensionInfo(dimName);
var coordDim = dimItem.coordDim;
if (coordDim) {
if (__DEV__) {
assert(OTHER_DIMENSIONS.get(coordDim) == null);
}
var coordDimIndex = dimItem.coordDimIndex;
getOrCreateEncodeArr(encode, coordDim)[coordDimIndex] = dimName;
if (!dimItem.isExtraCoord) {
notExtraCoordDimMap.set(coordDim, 1);
// Use the last coord dim (and label friendly) as default label,
// because when dataset is used, it is hard to guess which dimension
// can be value dimension. If both show x, y on label is not look good,
// and conventionally y axis is focused more.
- if (mayLabelDimType(dimItem.type)) {
+ if (fromDataset && mayLabelDimType(dimItem.type)) {
defaultedLabel[0] = dimName;
}
// User output encode do not contain generated coords.
// And it only has index. User can use index to retrieve value from the raw item array.
getOrCreateEncodeArr(userOutput.encode, coordDim)[coordDimIndex] = dimItem.index;
}
if (dimItem.defaultTooltip) {
defaultedTooltip.push(dimName);
}
}
+ // For data specified on `series` directly, we use different strategy
+ // to guess defualt label from data specified on `dataset`. That is,
+ // Find the last possible dimension. For example, consider the case
+ // [[latitude, longitude, val], ...], the `val` would be the best choise.
+ if (!fromDataSet && !dimItem.isExtraCoord) {
+ defaultedLabelDimNonDataset = dimName;
+ }
OTHER_DIMENSIONS.each(function (v, otherDim) {
var encodeArr = getOrCreateEncodeArr(encode, otherDim);
var dimIndex = dimItem.otherDims[otherDim];
if (dimIndex != null && dimIndex !== false) {
encodeArr[dimIndex] = dimItem.name;
}
});
});
+ if (defaultedLabelDimNonDataset != null)
+ defaultedLabel[0] = defaultedLabelDimNonDataset;
+ }
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r363576700
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
> So can we solve it by using the dimension that has not been encoded yet?
For example, If we have three value in each item of `series.data`.
```js
[x, y, z]
```
Because the data is not shared by other series. After taking `[x, y]` and encoded to xAxis/yAxis. We can easily guess the third value `z` is an extra value which can be displayed as label and tooltip. We can treat it as a function `z = f(x, y)`
But in the `dataset`, because each dimension may be taken by other series. We can not simply use the third dimension, or the last dimension. So can we take a further step and find if any dimension in the `dataset` has been taken by other series and being encoded?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r362733325
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
@100pah It's a change since we introduced `encode` in 4.0 . It breaks all `scatter` series. Not only the `bmap` one.
In the 3.x version and earlier. We use the first two values to represent the coordinates and the third one as the value. It will be displayed in the label and tooltip.
In the logic of https://github.com/apache/incubator-echarts/blob/master/src/data/helper/dimensionHelper.js#L53 . I think the third value should be an extra dimension besides [x, y] coords. But I'm not sure why the label still using the `y` value.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] github-actions[bot] commented on pull request #11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11973:
URL: https://github.com/apache/echarts/pull/11973#issuecomment-1013472446
This PR has been automatically closed because it has not had recent activity. Sorry for that and we are looking forward to your next 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] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r362733325
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
@100pah It's a change since we introduced `encode` in 4.0 . It breaks all `scatter` series. Not only the `bmap` one.
In the 3.x version and earlier. We use the first two values to represent the `[x, y]` coordinates and the third one as the value. It will be displayed in the label and tooltip.
Maybe we can use the first value which is not encoded to coords to be displayed as label..
In https://github.com/apache/incubator-echarts/blob/master/src/data/helper/dimensionHelper.js#L52
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request
#11973: fix bug #11528 Modify default value of tooltip and label
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #11973: fix bug #11528 Modify default value of tooltip and label
URL: https://github.com/apache/incubator-echarts/pull/11973#discussion_r362733325
##########
File path: src/chart/helper/labelHelper.js
##########
@@ -25,7 +25,8 @@ import {retrieveRawValue} from '../../data/helper/dataProvider';
* @return {string} label string. Not null/undefined
*/
export function getDefaultLabel(data, dataIndex) {
- var labelDims = data.mapDimension('defaultedLabel', true);
+ var dataDimsLen = data.dimensions.length;
+ var labelDims = data.mapDimension(dataDimsLen > 2 ? data.dimensions[dataDimsLen - 1] : 'defaultedLabel', true);
Review comment:
@100pah It's a change since we introduced `encode` in 4.0 . It breaks all `scatter` series. Not only the `bmap` one.
In the 3.x version and earlier. We use the first two values to represent the `[x, y]` coordinates and the third one as the value. It will be displayed in the label and tooltip.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org