feat(marker): markPoint markLine markArea add z2 option to support change level #20782
Conversation
echarts-bot
bot
added
PR: author is committer
PR: awaiting doc
|
Thanks for your contribution! The pull request is marked to be Document changes are required in this PR. Please also make a PR to apache/echarts-doc for document changes and update the issue id in the PR description. When the doc PR is merged, the maintainers will remove the |
|
The changes brought by this PR can be previewed at: https://echarts.apache.org/examples/editor?version=PR-20782@785b25a |
| @@ -143,12 +143,14 @@ class MarkPointView extends MarkerView { | |||
| } | |||
|
|
|||
| const style = itemModel.getModel('itemStyle').getItemStyle(); | |||
| const z2 = itemModel.getModel('z2').option; | |||
There was a problem hiding this comment.
Usually be
const z2 = itemModel.get('z2');
// or const z2 = itemModel.get('z2', true); (no need to create a temporary model object, tho there is probably no performance issue here, handy optimization would be better.)
| const style = areaData.getItemModel<MarkAreaMergedItemOption>(idx).getModel('itemStyle').getItemStyle(); | ||
| const itemModel = areaData.getItemModel<MarkAreaMergedItemOption>(idx); | ||
| const style = itemModel.getModel('itemStyle').getItemStyle(); | ||
| const z2 = itemModel.getModel('z2').option; |
There was a problem hiding this comment.
| // lineData.setItemVisual(idx, { | ||
| // color: lineColor || fromData.getItemVisual(idx, 'color') | ||
| // }); | ||
| lineData.setItemLayout(idx, [ | ||
| fromData.getItemLayout(idx), | ||
| toData.getItemLayout(idx) | ||
| ]); | ||
| const z2 = itemModel.getModel('z2').option; |
There was a problem hiding this comment.
| @@ -80,7 +81,7 @@ class Symbol extends graphic.Group { | |||
| ); | |||
|
|
|||
| symbolPath.attr({ | |||
| z2: 100, | |||
There was a problem hiding this comment.
-
Symbolis used not only by markPoint, but also by line series, scatter series, etc.
In the current PR, only markPoint allows users to set z2, which results inz2: 100being missing in other cases. Is it intentional? If not, backward compatibility should be maintained. -
If users do not specify
z2, here a undefined-z2 will be set to an zr element, which is error-prone (z2 is not meant to be a non-number yet in zrender) and might degrade the performance (theoretically).
Summary: I think every place a z2 is about to set to a zr element, use a default number value to make sure it's a number.
e.g.,
symbolPath.attr({
zr: retrieve2(z2, 100),
...
})graphic.updateProps(polygon, {
retrieve2(z2, 0),
// ...
})The other place that sets z2 to zr elements in this PR follows this suggestion.
Brief Information
This pull request is in the type of:
What does this PR do?
#6691
markPoint markLine markArea add z2 option to support change level.
Fixed issues
Details
Before: What was the problem?
After: How does it behave after the fixing?
Document Info
One of the following should be checked.
Misc
ZRender Changes
Related test cases or examples to use the new APIs
N.A.
Others
Merging options
Other information