100pah commented on a change in pull request #11485: Fix #10910
URL: 
https://github.com/apache/incubator-echarts/pull/11485#discussion_r344188997
 
 

 ##########
 File path: src/chart/line/LineView.js
 ##########
 @@ -453,6 +453,9 @@ export default ChartView.extend({
         }
 
         var visualColor = getVisualGradient(data, coordSys) || 
data.getVisual('color');
+        if (typeof visualColor === 'function') {
+            visualColor = visualColor(seriesModel.getDataParams(0)) || 
'#000000';
+        }
 
 
 Review comment:
   Use `seriesModel.getDataParams(0)` will cause that line color will be 
changed when using `dataZoom`.
   Check this example please:
   ```js
               colors = ['red', 'blue', 'yellow', 'green'];
               option = {
                   xAxis: {},
                   yAxis: {},
                   dataZoom: {},
                   series: {
                       type: 'line',
                       itemStyle: {
                           color: function (params) {
                               return colors[params.dataIndex];
                           }
                       },
                       data: [[11, 22], [33, 44], [22, 33]]
                   }
               };
   ```
   
   I suggest that if the `itemStyle.color` is a `function`, we use the default 
color for line.
   If line needs be colored, use `lineStyle.color` in this case.
   By convention, the data.getVisual('color') should not be a function in the 
view stage.
   The function should be processed in the visual stage.
   I thinks we probably change the code here:
   
<https://github.com/apache/incubator-echarts/blob/0ebdeaf42d8a8e3830c160fa9cd27bcc434ce502/src/visual/seriesColor.js#L35>
   
   If the color is a function, we still use the palette color. @pissang 
   Or, to avoid some potential issue, save the original palette in another 
filed and use it in `LineView`.
   
   

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@echarts.apache.org
For additional commands, e-mail: commits-h...@echarts.apache.org

Reply via email to