plainheart commented on code in PR #17451:
URL: https://github.com/apache/echarts/pull/17451#discussion_r1114118541
##########
src/chart/graph/GraphSeries.ts:
##########
@@ -69,7 +69,7 @@ export interface GraphNodeStateOption<TCbParams = never> {
interface ExtraEmphasisState {
- focus?: DefaultEmphasisFocus | 'adjacency'
+ focus?: DefaultEmphasisFocus | 'adjacency' | 'trajectory'
Review Comment:
Why do you add `'trajectory'` for the `graph` series? I think you are just
adding support for the `sankey` series. If you also plan to support this
feature for the `graph` series, it's not enough to only add typings.
##########
src/chart/sankey/SankeyView.ts:
##########
@@ -231,7 +231,9 @@ class SankeyView extends ChartView {
const focus = emphasisModel.get('focus');
toggleHoverEmphasis(
curve,
- focus === 'adjacency' ? edge.getAdjacentDataIndices() : focus,
+ focus === 'adjacency' ? edge.getAdjacentDataIndices()
+ : focus === 'trajectory' ? edge.getFullPathDataIndices()
+ : focus,
Review Comment:
Code style preference
```suggestion
focus === 'adjacency'
? edge.getAdjacentDataIndices()
: focus === 'trajectory'
? edge.getFullPathDataIndices()
: focus,
```
##########
src/data/Graph.ts:
##########
@@ -387,6 +387,51 @@ class GraphNode {
}
return dataIndices;
}
+
+ getFullPathDataIndices(): {node: number[], edge: number[]} {
Review Comment:
Let's keep the naming style consistent. Please use `trajectory` instead of
`fullPath`.
##########
src/data/Graph.ts:
##########
@@ -387,6 +387,51 @@ class GraphNode {
}
return dataIndices;
}
+
+ getFullPathDataIndices(): {node: number[], edge: number[]} {
+ const connectedEdgesMap: {[key: number]: boolean} = {};
+ const connectedNodesMap: {[key: number]: boolean} = {};
Review Comment:
It's better to use a HashMap.
```ts
const connectedEdgesMap = zrUtil.createHashMap<boolean, number>();
const connectedNodesMap = zrUtil.createHashMap<boolean, number>();
```
##########
src/data/Graph.ts:
##########
@@ -429,6 +474,47 @@ class GraphEdge {
node: [this.node1.dataIndex, this.node2.dataIndex]
};
}
+
+ getFullPathDataIndices(): {node: number[], edge: number[]} {
+ const connectedEdgesMap: {[key: number]: boolean} = {};
+ const connectedNodesMap: {[key: number]: boolean} = {};
Review Comment:
The same to the above comments.
##########
test/emphasis-fullPath.html:
##########
@@ -0,0 +1,62 @@
+
Review Comment:
nitpick: It would be better if we rename this test case to
`sankey-emphasis.html` and add a child case named `emphasis: 'trajectory'`. You
can refer to `test/sankey.html`.
##########
src/chart/sankey/SankeyView.ts:
##########
@@ -283,7 +285,9 @@ class SankeyView extends ChartView {
const focus = emphasisModel.get('focus');
toggleHoverEmphasis(
rect,
- focus === 'adjacency' ? node.getAdjacentDataIndices() : focus,
+ focus === 'adjacency' ? node.getAdjacentDataIndices()
+ : focus === 'trajectory' ? node.getFullPathDataIndices()
+ : focus,
Review Comment:
Code style preference
```suggestion
focus === 'adjacency'
? node.getAdjacentDataIndices()
: focus === 'trajectory'
? node.getFullPathDataIndices()
: focus,
```
##########
src/data/Graph.ts:
##########
@@ -429,6 +474,47 @@ class GraphEdge {
node: [this.node1.dataIndex, this.node2.dataIndex]
};
}
+
+ getFullPathDataIndices(): {node: number[], edge: number[]} {
Review Comment:
Please use `trajectory` instead of `fullPath`.
##########
src/data/Graph.ts:
##########
@@ -387,6 +387,51 @@ class GraphNode {
}
return dataIndices;
}
+
+ getFullPathDataIndices(): {node: number[], edge: number[]} {
+ const connectedEdgesMap: {[key: number]: boolean} = {};
+ const connectedNodesMap: {[key: number]: boolean} = {};
+
+ for (let i = 0; i < this.edges.length; i++) {
+ const adjacentEdge = this.edges[i];
+ if (adjacentEdge.dataIndex < 0) {
+ continue;
+ }
+
+ connectedEdgesMap[adjacentEdge.dataIndex] = true;
+
+ const sourceNodesQueue = [adjacentEdge.node1];
+ const targetNodesQueue = [adjacentEdge.node2];
+
+ let nodeIteratorIndex = 0;
+ while (nodeIteratorIndex < sourceNodesQueue.length) {
+ const sourceNode = sourceNodesQueue[nodeIteratorIndex];
+ nodeIteratorIndex++;
+ connectedNodesMap[sourceNode.dataIndex] = true;
+
+ for (let j = 0; j < sourceNode.inEdges.length; j++) {
+ connectedEdgesMap[sourceNode.inEdges[j].dataIndex] = true;
+ sourceNodesQueue.push(sourceNode.inEdges[j].node1);
+ }
+ }
+
+ nodeIteratorIndex = 0;
+ while (nodeIteratorIndex < targetNodesQueue.length) {
+ const targetNode = targetNodesQueue[nodeIteratorIndex];
+ nodeIteratorIndex++;
+ connectedNodesMap[targetNode.dataIndex] = true;
+ for (let j = 0; j < targetNode.outEdges.length; j++) {
+ connectedEdgesMap[targetNode.outEdges[j].dataIndex] = true;
+ targetNodesQueue.push(targetNode.outEdges[j].node2);
+ }
+ }
+ }
+
+ return {
+ edge: Object.keys(connectedEdgesMap).map(Number),
+ node: Object.keys(connectedNodesMap).map(Number)
Review Comment:
Just use `connectedEdgesMap.keys()` and `connectedNodesMap.keys()`
##########
src/data/Graph.ts:
##########
@@ -429,6 +474,47 @@ class GraphEdge {
node: [this.node1.dataIndex, this.node2.dataIndex]
};
}
+
+ getFullPathDataIndices(): {node: number[], edge: number[]} {
+ const connectedEdgesMap: {[key: number]: boolean} = {};
+ const connectedNodesMap: {[key: number]: boolean} = {};
+
+ connectedEdgesMap[this.dataIndex] = true;
+
+ const sourceNodes = [this.node1];
+ const targetNodes = [this.node2];
+
+ let nodeIteratorIndex = 0;
+ while (nodeIteratorIndex < sourceNodes.length) {
+ const sourceNode = sourceNodes[nodeIteratorIndex];
+ nodeIteratorIndex++;
+
+ connectedNodesMap[sourceNode.dataIndex] = true;
+
+ for (let j = 0; j < sourceNode.inEdges.length; j++) {
+ connectedEdgesMap[sourceNode.inEdges[j].dataIndex] = true;
+ sourceNodes.push(sourceNode.inEdges[j].node1);
+ }
+ }
+
+ nodeIteratorIndex = 0;
+ while (nodeIteratorIndex < targetNodes.length) {
+ const targetNode = targetNodes[nodeIteratorIndex];
+ nodeIteratorIndex++;
+
+ connectedNodesMap[targetNode.dataIndex] = true;
+
+ for (let j = 0; j < targetNode.outEdges.length; j++) {
+ connectedEdgesMap[targetNode.outEdges[j].dataIndex] = true;
+ targetNodes.push(targetNode.outEdges[j].node2);
+ }
+ }
+
+ return {
+ edge: Object.keys(connectedEdgesMap).map(Number),
+ node: Object.keys(connectedNodesMap).map(Number)
Review Comment:
The same to the above comments.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
