pissang commented on a change in pull request #14542:
URL: https://github.com/apache/echarts/pull/14542#discussion_r603765920
##########
File path: src/core/echarts.ts
##########
@@ -2157,34 +2157,44 @@ class ECharts extends Eventful<ECEventDefinition> {
if (model.preventAutoZ) {
return;
}
- const z = model.get('z');
- const zlevel = model.get('zlevel');
// Set z and zlevel
- view.group.traverse(function (el: Displayable) {
- if (!el.isGroup) {
- z != null && (el.z = z);
- zlevel != null && (el.zlevel = zlevel);
-
- // TODO if textContent is on group.
- const label = el.getTextContent();
- const labelLine = el.getTextGuideLine();
- if (label) {
- label.z = el.z;
- label.zlevel = el.zlevel;
- // lift z2 of text content
- // TODO if el.emphasis.z2 is spcefied, what about
textContent.
- label.z2 = el.z2 + 2;
- }
- if (labelLine) {
- const showAbove = el.textGuideLineConfig &&
el.textGuideLineConfig.showAbove;
- labelLine.z = el.z;
- labelLine.zlevel = el.zlevel;
- labelLine.z2 = el.z2 + (showAbove ? 1 : -1);
- }
- }
- });
+ _updateZ(view.group, model.get('z'), model.get('zlevel'));
};
+ function _updateZ(el: Element, z: number, zlevel: number) {
+ // Group may also have textContent
+ const label = el.getTextContent();
+ const labelLine = el.getTextGuideLine();
+ const isGroup = el.isGroup;
+
+ // always set z and zlevel if label/labelLine exists
+ if (label || labelLine) {
+ if (label) {
+ label.z = z + 1;
+ label.zlevel = zlevel;
+ // lift z2 of text content
+ // TODO if el.emphasis.z2 is spcefied, what about
textContent.
+ isGroup || (label.z2 = (el as Displayable).z2 + 2);
+ }
+ if (labelLine) {
+ const showAbove = el.textGuideLineConfig &&
el.textGuideLineConfig.showAbove;
+ labelLine.z = z;
+ labelLine.zlevel = zlevel;
+ isGroup || (labelLine.z2 = (el as Displayable).z2 +
(showAbove ? 1 : -1));
+ }
+ }
+
+ if (isGroup) {
+ // set z & zlevel of children elements of Group
+ el.traverse((childEl: Element) => _updateZ(childEl, z,
zlevel));
Review comment:
Perhaps we can return max z2 of all elements in the group and update z2
of label and labelLine based on this max z2 value.
For example:
```ts
if (isGroup) {
const children = el.childrenRef();
let maxZ2 = -Infinity;
for (let i = 0; i < children.length; i++); {
maxZ2 =_updateZ(childEl, z, zlevel, maxZ2);
}
if (isFinite(maxZ2)) {
label.z2 = maxZ2 + 2;
labelLine.....
}
}
else {
return Math.max(el.z2, maxZ2);
}
```
Also this code may cause each group node to be traversed too many times
because for each group node it will call `traverse` and visits all its
descendants when it's visited. We need to loop the childrenRef() we want to do
it recursively.
##########
File path: src/core/echarts.ts
##########
@@ -2157,34 +2157,44 @@ class ECharts extends Eventful<ECEventDefinition> {
if (model.preventAutoZ) {
return;
}
- const z = model.get('z');
- const zlevel = model.get('zlevel');
// Set z and zlevel
- view.group.traverse(function (el: Displayable) {
- if (!el.isGroup) {
- z != null && (el.z = z);
- zlevel != null && (el.zlevel = zlevel);
-
- // TODO if textContent is on group.
- const label = el.getTextContent();
- const labelLine = el.getTextGuideLine();
- if (label) {
- label.z = el.z;
- label.zlevel = el.zlevel;
- // lift z2 of text content
- // TODO if el.emphasis.z2 is spcefied, what about
textContent.
- label.z2 = el.z2 + 2;
- }
- if (labelLine) {
- const showAbove = el.textGuideLineConfig &&
el.textGuideLineConfig.showAbove;
- labelLine.z = el.z;
- labelLine.zlevel = el.zlevel;
- labelLine.z2 = el.z2 + (showAbove ? 1 : -1);
- }
- }
- });
+ _updateZ(view.group, model.get('z'), model.get('zlevel'));
};
+ function _updateZ(el: Element, z: number, zlevel: number) {
+ // Group may also have textContent
+ const label = el.getTextContent();
+ const labelLine = el.getTextGuideLine();
+ const isGroup = el.isGroup;
+
+ // always set z and zlevel if label/labelLine exists
+ if (label || labelLine) {
+ if (label) {
+ label.z = z + 1;
Review comment:
I think `label.z` is better to be same with user-configured `z` so it
can be more predictable to developers. Also, we hope elements can be on top of
other labels when it's hovered, which can't be done if we lift the `z` by
default.
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
