jathanasiou commented on a change in pull request #296:
URL: https://github.com/apache/brooklyn-ui/pull/296#discussion_r729815705
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -819,23 +849,11 @@ export function D3Blueprint(container, options) {
.attr('opacity', 1)
.attr('stroke', 'red')
.attr('d', function(d) {
- let targetNode = nodeForEntity(d.target);
- let sourceNode = nodeForEntity(d.source);
- let sourceY = sourceNode.y + (d.source.isMemberSpec() ?
_configHolder.nodes.memberspec.circle.cy : 0);
- let targetY = targetNode.y + (d.target.isMemberSpec() ?
_configHolder.nodes.memberspec.circle.cy : 0);
- let dx = targetNode.x - sourceNode.x;
- let dy = targetY - sourceY;
- let dr = Math.sqrt(dx * dx + dy * dy);
- let sweep = dx * dy > 0 ? 0 : 1;
- _mirror.attr('d', `M ${sourceNode.x},${sourceY} A ${dr},${dr}
0 0,${sweep} ${targetNode.x},${targetY}`);
-
- let m =
_mirror._groups[0][0].getPointAtLength(_mirror._groups[0][0].getTotalLength() -
_configHolder.nodes.child.circle.r - 20);
-
- dx = m.x - sourceNode.x;
- dy = m.y - sourceY;
- dr = Math.sqrt(dx * dx + dy * dy);
-
- return `M ${sourceNode.x},${sourceY} A ${dr},${dr} 0
0,${sweep} ${m.x},${m.y}`;
+ const [x1,y1, dr, sweep, x2, y2, isLeftToRight] =
getArcTransitionParameters(d);
+
+ d.isLeftToRight = isLeftToRight;
Review comment:
What's the reasoning for assigning the (generic) function as an instance
member here? Wouldn't it be more appropriate to have
```
d.isLeftToRight = () => isLeftToRight(d);
// to avoid awkward cases like
d.isLeftToRight(d)
```
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -807,6 +807,36 @@ export function D3Blueprint(container, options) {
let relationArcs = _relationArcs.selectAll('.relation').data(arcsData,
(d) => getPathId(d));
+ const getArcTransitionParameters = (d) => {
+ let targetNode = nodeForEntity(d.target);
+ let sourceNode = nodeForEntity(d.source);
+ let sourceY = sourceNode.y + (d.source.isMemberSpec() ?
_configHolder.nodes.memberspec.circle.cy : 0);
+ let targetY = targetNode.y + (d.target.isMemberSpec() ?
_configHolder.nodes.memberspec.circle.cy : 0);
+ let dx = targetNode.x - sourceNode.x;
+ let dy = targetY - sourceY;
+ let dr = Math.sqrt(dx * dx + dy * dy);
+ let sweep = dx * dy > 0 ? 0 : 1;
+ _mirror.attr('d', `M ${sourceNode.x},${sourceY} A ${dr},${dr} 0
0,${sweep} ${targetNode.x},${targetY}`);
+
+ let m =
_mirror._groups[0][0].getPointAtLength(_mirror._groups[0][0].getTotalLength() -
_configHolder.nodes.child.circle.r - 20);
+
+ dx = m.x - sourceNode.x;
+ dy = m.y - sourceY;
+ dr = Math.sqrt(dx * dx + dy * dy);
Review comment:
since this formula is reused a quick lambda might be useful
```js
const deltaR = (dx, dy) => Math.sqrt(dx * dx + dy * dy);
```
--
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]
