-
Notifications
You must be signed in to change notification settings - Fork 19.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix bug #7340 #11295
fix bug #7340 #11295
Conversation
@@ -0,0 +1,402 @@ | |||
<!-- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of this file has typo
var chart2 = echarts.init(document.getElementById('plain2'), null, {}); | ||
|
||
var option2 = { | ||
tooltip: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong indent.
], function (echarts) { | ||
var graphic = echarts.graphic; | ||
|
||
var chart = echarts.init(document.getElementById('plain'), null, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong indent
<div class="chart" id="plain"></div> | ||
<h1>The style with legend symble should be series[i].itemStyle</h1> | ||
<div class="chart" id="plain1"></div> | ||
<h1>The style with legend symble should be legend.itemStyle</h1> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description should better be more clear:
what style it will be (for example, what color). When running the test case manually,
I can not know what color of the legend border is expected.
It seems that it is not the same color as the border of the series shape in the second and third chart.
If need multiple line desription,
testHelper.createTest
can be used, which support multiple line description
(take dataZoom-action.html
and tmp-base.html
as examples)
@@ -283,6 +283,14 @@ var LegendModel = echarts.extendComponentModel({ | |||
// 图例关闭时候的颜色 | |||
inactiveColor: '#ccc', | |||
|
|||
// 图例关闭时候的颜色 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use English please.
Although the previous comment is Chinese, we should change it to English gradually,
usually when modifying the file.
@@ -74,6 +77,25 @@ export default function (seriesType) { | |||
// Set data all color for legend | |||
dataAll.setItemVisual(rawIdx, 'color', singleDataColor); | |||
} | |||
|
|||
if (!singleDataBorderColor) { | |||
var borderColor = itemModel.get('itemStyle.borderColor') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some historical reason, we have both 'barBorderColor'
and 'borderColor'
.
|
||
var borderColorAccessPath = (seriesModel.visualBorderColorAccessPath || 'itemStyle.borderColor').split('.'); | ||
var borderColor = seriesModel.get(borderColorAccessPath) // Set in itemStyle | ||
|| seriesModel.getColorFromPalette( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should not repeat it, now that it both used in two places.
|
||
var borderColorAccessPath = (seriesModel.visualBorderColorAccessPath || 'itemStyle.borderColor').split('.'); | ||
var borderColor = seriesModel.get(borderColorAccessPath) // Set in itemStyle | ||
|| seriesModel.getColorFromPalette( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should not repeat it, now that it both used in two places.
@@ -33,6 +33,15 @@ export default { | |||
|
|||
// FIXME Set color function or use the platte color | |||
data.setVisual('color', color); | |||
|
|||
var borderColorAccessPath = (seriesModel.visualBorderColorAccessPath || 'itemStyle.borderColor').split('.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For historical reason, we have both 'borderColor' and 'barBorderColor'.
@@ -41,16 +50,23 @@ export default { | |||
data.setItemVisual( | |||
idx, 'color', color(seriesModel.getDataParams(idx)) | |||
); | |||
data.setItemVisual( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer do not support set callback function for borderColor
until it is really needed.
This kind of callback function brings some complexity.
Hi @yufeng04 . You need to add more detail in the title and description of pull request |
No description provided.