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/08/07 05:40:51 UTC
[GitHub] [incubator-echarts] easonyq opened a new pull request #13094: Fix legend color with customized itemStyle
easonyq opened a new pull request #13094:
URL: https://github.com/apache/incubator-echarts/pull/13094
<!-- 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?
Fix the bug that when `itemStyle` with functional `color` is used, the color of legend will not be consist with the customized color.
### Fixed issues
#12977
## Details
### Before: What was the problem?
When user customized the color of bar, the color of legend is not consist with it.
### After: How is it fixed in this PR?
<!-- THE RESULT AFTER FIXING AND A SIMPLE EXPLANATION ABOUT HOW IT IS FIXED. -->
<!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
The color of legend is consist with bar color customized by user
(The first color is white, so it can not be seen very clearly)
## Usage
### Are there any API changes?
- [ ] The API has been changed.
<!-- LIST THE API CHANGES HERE -->
No
### Related test cases or examples to use the new APIs
NA.
## Others
### Merging options
- [x] Please squash the commits into a single one when merge.
### Other information
I use `const dataParams = seriesModel.getDataParams(dataIndex);` in my PR, and I found that `dataParams.color` is just the final result of color, no matter it is a function or a value, or user simply hadn't used `color` in `itemStyle`.
As a result, it is not necessary anymore to retrieve or do some transforming calculation from LegendView, just use this 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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] easonyq edited a comment on pull request #13094: Fix legend color with customized itemStyle
Posted by GitBox <gi...@apache.org>.
easonyq edited a comment on pull request #13094:
URL: https://github.com/apache/incubator-echarts/pull/13094#issuecomment-673353051
Excuse me, but I cannot quite understand the **several** color that color callback returns. Is that mean the color callback within `itemStyle` of a single data serie will return different value according to different input parameters? Like this?
```javascript
series: [{
name: 'Forest',
type: 'bar',
barGap: 0,
data: [320, 332, 301, 334, 390],
itemStyle: {
color: (params) => {
if (params.someProperty)
return 'white'
else
return 'black';
}
}
},
```
If so, the chart will display different color for a single item. Let's take my screenshot as an example, the color of *Forest* (the first bar) in 2012 (the first data) can be white, but the color of *Forest* in 2013 (the second data) can be black.
If this is the case, I don't regard this as a valid configuration because the fact we take for granted that different color means different series (white stands for Forest, green stands for Wetland). If different colors exist in one data serie, it is quite wierd. (Both white and black stands for Forest??)
Despite, I still think that the current situation isn't good enough, for the bar color is not consist with the legend color. I agree with @pissang that this misleading (if it really exists) is much less important than the display error happened now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on pull request #13094: Fix legend color with customized itemStyle
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #13094:
URL: https://github.com/apache/incubator-echarts/pull/13094#issuecomment-673278481
I'm not sure, but is it relevant to @Ovilia 's PR https://github.com/apache/incubator-echarts/pull/12444 ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang edited a comment on pull request #13094: Fix legend color with customized itemStyle
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #13094:
URL: https://github.com/apache/incubator-echarts/pull/13094#issuecomment-673278481
<del>I'm not sure, but is it relevant to @Ovilia 's PR https://github.com/apache/incubator-echarts/pull/12444 ?</del>
Had a further review. Seems it's trying to solve the same issue at https://github.com/apache/incubator-echarts/pull/12133 from @susiwen8 .
I remember we had a discussion privately about how should we choose the color of the legend if `itemStyle.color` is a callback.
I preferred choosing the first color callback given, which is the same strategy in this PR. But @100pah thought it will be misleading if the callback returns several different colors and choose the color from the default palette.
Perhaps we can have more discussions here. My opinion is still not changed. The misleading is much less than the current strategy if we choose the color from callback returns.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] easonyq edited a comment on pull request #13094: Fix legend color with customized itemStyle
Posted by GitBox <gi...@apache.org>.
easonyq edited a comment on pull request #13094:
URL: https://github.com/apache/incubator-echarts/pull/13094#issuecomment-674625760
I understood what you mean.
But according to this `color` function, user did not give us enough information about **which color is normal** and **which color is highlight**. This is the key point that we can not figure out which color we should use in legend unless he provide this extra information.
Still, I think no matter which color we used in legend, if we apply this PR, we are using **at least** one color of the bar. But the current situation is that we are using a **totally different** color. It is worse for sure.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang edited a comment on pull request #13094: Fix legend color with customized itemStyle
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #13094:
URL: https://github.com/apache/incubator-echarts/pull/13094#issuecomment-673278481
<del>I'm not sure, but is it relevant to @Ovilia 's PR https://github.com/apache/incubator-echarts/pull/12444 ?</del>
Had a further review. Seems it's trying to solve the same issue at https://github.com/apache/incubator-echarts/pull/12133 from @susiwen8 .
I remember we had a discussion privately about how should we choose the color of the legend if `itemStyle.color` is a callback.
I preferred choosing the first color callback returns, which is the same strategy in this PR. But @100pah thought it will be misleading if the callback returns several different colors and choose the color from the default palette.
Perhaps we can have more discussions here. My opinion is still not changed. The misleading is much less than the current strategy if we choose the color from callback returns.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] easonyq edited a comment on pull request #13094: Fix legend color with customized itemStyle
Posted by GitBox <gi...@apache.org>.
easonyq edited a comment on pull request #13094:
URL: https://github.com/apache/incubator-echarts/pull/13094#issuecomment-674625760
I understood what you mean.
But according to this `color` function, user did not give us enough information about **which color is normal** and **which color is highlight**. This is the key point that we can not figure out which color we should use in legend unless he provide this extra information.
Still, I think no matter which color we used in legend, if we apply this PR, we are using **at least** one color of the bar. But the current situation is we are using a **totally different** color. It is worse for sure.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] easonyq commented on pull request #13094: Fix legend color with customized itemStyle
Posted by GitBox <gi...@apache.org>.
easonyq commented on pull request #13094:
URL: https://github.com/apache/incubator-echarts/pull/13094#issuecomment-674625760
I understood what you mean.
But according to this `color` function, user did not give us enough information about **which color is normal** and **which color is highlight**. This is the key point that we can not distinguish which color we should use in legend unless he provide this extra information.
Still, I think no matter which color we used in legend, if we apply this PR, we are using **at least** one color of the bar. But the current situation is we are using a **totally different** color. It is worse for sure.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on pull request #13094: Fix legend color with customized itemStyle
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #13094:
URL: https://github.com/apache/incubator-echarts/pull/13094#issuecomment-673848795
That's a typical scenario. Usually, the different color is for highlighting the data.
```js
itemStyle: {
color: (params) => {
// Needs alert the data exceed the threshold.
if (params.someProperty > someThreshold)
return 'red';
else
return defaultColor;
}
}
```
This reminds me of what @100pah is really concerned about. Because the highlighted data may be changed. It can be the first data or not. If we only pick the first color callback returns. The color of the legend is not consistent if we use `dataZoom` to slide.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] easonyq commented on pull request #13094: Fix legend color with customized itemStyle
Posted by GitBox <gi...@apache.org>.
easonyq commented on pull request #13094:
URL: https://github.com/apache/incubator-echarts/pull/13094#issuecomment-673353051
Excuse me, but I cannot quite understand the **several** color that color callback returns. Is that mean the color callback within `itemStyle` of a single data serie will return different value according to different input parameters? Like this?
```javascript
series: [{
name: 'Forest',
type: 'bar',
barGap: 0,
data: [320, 332, 301, 334, 390],
itemStyle: {
color: (params) => {
if (params.someProperty)
return 'white'
else
return 'black';
}
}
},
```
If so, the chart will display different color for a single item. Let's take my screenshot for example, the color of *Forest* (the first bar) in 2012 (the first data) can be white, but the color of *Forest* in 2013 (the second data) can be black.
If this is the case, I don't regard this as a valid configuration because the fact we take for granted that different color means different series (white stands for Forest, green stands for Wetland). If different colors exist in one data serie, it is quite wierd. (Both white and black stands for Forest??)
Despite, I still think that the current situation isn't good enough, for the bar color is not consist with the legend color. I agree with @pissang that this misleading (if it really exists) is much less important than the display error happened now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang edited a comment on pull request #13094: Fix legend color with customized itemStyle
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #13094:
URL: https://github.com/apache/incubator-echarts/pull/13094#issuecomment-673278481
<del>I'm not sure, but is it relevant to @Ovilia 's PR https://github.com/apache/incubator-echarts/pull/12444 ?</del>
Had a further review. Seems it's trying to solve the same issue at https://github.com/apache/incubator-echarts/pull/12133 from @susiwen8 .
I remember we had a discussion privately about how should we choose the color of the legend if `itemStyle.color` is a callback.
I preferred choosing the first color callback given, which is the same strategy in this PR. But @100pah thought it will be misleading if the callback returns several different colors and choose the color from the default palette.
My opinion is still not changed. The misleading is much less than the current strategy if we choose the color from callback returns.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org