-
Notifications
You must be signed in to change notification settings - Fork 19.7k
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
Add polyline edge layout for the tree series #7090 #11808
Conversation
src/chart/tree/TreeSeries.js
Outdated
@@ -139,6 +139,9 @@ export default SeriesModel.extend({ | |||
// the layout of the tree, two value can be selected, 'orthogonal' or 'radial' | |||
layout: 'orthogonal', | |||
|
|||
// value can be 'polyline' | |||
edgeLayout: 'curve', |
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 think edgeShape
is better. edgeLayout
is more like the position
of edge
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.
yeah, good suggestion!
…edge of source when the only child be removed
ctx.lineTo(midPoint[0], point[1]); | ||
} | ||
} | ||
} |
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.
Top and bottom horizontal lines are better to be connected with the vertical line.
ctx.moveTo(); // top line end point
ctx.lineTo(); // top line connect point
ctx.lineTo(); // bottom line connect point
ctx.lineTo(); // bottom line end point
It will have a nicer corner than current if the line is thicker.
src/chart/tree/TreeView.js
Outdated
var location = parsePercent(shape.forkPosition, 1); | ||
var midPoint = computeMidPoints(parentPoint, orient, ptMax, location); | ||
|
||
ctx.moveTo(parentPoint[0], parentPoint[1]); |
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
src/chart/tree/TreeView.js
Outdated
function drawEdge( | ||
seriesModel, node, virtualRoot, symbolEl, sourceOldLayout, | ||
sourceLayout, targetLayout, group, seriesScope | ||
) { |
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 indent is not good. The back brace should better align with the start of the sentence function
,
and the rest of the code in the function should reduce 4 space.
src/chart/tree/TreeView.js
Outdated
} | ||
var parentPoint = shape.parentPoint; | ||
var orient = shape.orient; | ||
var location = parsePercent(shape.forkPosition, 1); |
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 location
is not good.
src/chart/tree/TreeView.js
Outdated
var ptMin = [Infinity, Infinity]; | ||
var ptMax = [-Infinity, -Infinity]; | ||
var points = shape.childPoints; | ||
for (var i = 0; i < points.length; i++) { |
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 think ptMin
ptMax
and the switch case
of 'TB' 'BT' 'LR' 'RL' and computeMidPoints
are not needed.
The code can be simplified if you wish. And some critical case need to be considered. For example:
buildPath: function (ctx, shape) {
var childPoints = shape.childPoints;
var parentPoint = shape.parentPoint;
var childLen = childPoints.length;
var firstChildPos = childPoints[1];
var lastChildPos = childPoints[childLen - 1];
if (!childLen || !parentPoint) {
return;
}
if (childLen === 1) {
ctx.moveTo(firstChildPos[0], firstChildPos[1]);
ctx.lineTo(parentPoint[0], parentPoint[1]);
return;
}
var forkDim = (orient === 'TB' || orient === 'BT') ? 1 : 0;
var otherDim = 1 - forkDim;
var forkPosition = parsePercent(shape.forkPosition, 1);
var tmpPt = [];
ctx.moveTo(parentPoint[0], parentPoint[1]);
tmpPt[forkDim] = parentPoint[forkDim] + (firstChildPos[forkDim] - parentPoint[forkDim]) * forkPosition;
tmpPt[otherDim] = parentPoint[otherDim];
ctx.lineTo(tmpPt[0], tmpPt[1]);
ctx.moveTo(firstChildPos[0], firstChildPos[1]);
tmpPt[otherDim] = firstChildPos[otherDim];
ctx.lineTo(tmpPt[0], tmpPt[1]);
tmpPt[otherDim] = lastChildPos[otherDim];
ctx.lineTo(tmpPt[0], tmpPt[1]);
ctx.lineTo(lastChildPos[0], lastChildPos[1]);
for (var i = 1; i < childLen - 1; i++) {
var childPos = childPoints[i];
ctx.moveTo(childPos[0], childPos[1]);
tmpPt[otherDim] = childPos[otherDim];
ctx.lineTo(tmpPt[0], tmpPt[1]);
}
}
edgeLayout
in the TreeSeries.js.Fix #7090.