Fix drawing backward links on difficult scenarios (#45)

microsoft · Feb 13, 2018 · c5ca98c · c5ca98c
1 parent e5b604a
commit c5ca98c
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 43 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 1.7.1
+
+* Fix drawing backward links on difficult graph (with many backward links)
+
 ## 1.7.0
 
 * Options to configure to display cycles

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "powerbi-visuals-sankey",
-  "version": "1.7.0",
+  "version": "1.7.1",
   "description": "Sankey is a type of flow diagram in which the width of the series is in proportion to the quantity of the flow.  Use it to find major contributions to an overall flow.",
   "repository": {
     "type": "git",

diff --git a/pbiviz.json b/pbiviz.json
@@ -1,10 +1,10 @@
 {
   "visual": {
     "name": "SankeyDiagram",
-    "displayName": "Sankey 1.7.0",
+    "displayName": "Sankey 1.7.1",
     "guid": "SankeyDiagram1446463184954",
     "visualClassName": "SankeyDiagram",
-    "version": "1.7.0",
+    "version": "1.7.1",
     "description": "Sankey is a type of flow diagram in which the width of the series is in proportion to the quantity of the flow.  Use it to find major contributions to an overall flow.",
     "supportUrl": "http://community.powerbi.com",
     "gitHubUrl": "https://github.com/Microsoft/powerbi-visuals-sankey"

diff --git a/src/visual.ts b/src/visual.ts
@@ -406,43 +406,6 @@ module powerbi.extensibility.visual {
             SankeyDiagram.updateValueOfNode(link.source);
         }
 
-        // in this method we breaking simple cycles for typical displaying with twice rendering onr node in cycle
-        private processCyclesForBackwardLinks(cycles: SankeyDiagramCycleDictionary, nodes: SankeyDiagramNode[], links: SankeyDiagramLink[], settings: SankeyDiagramSettings): SankeyDiagramLink[] {
-            let createdNodes: SankeyDiagramNode[] = [];
-            for (let nodeName in cycles) {
-                let firstCyclesNode: SankeyDiagramNode = cycles[nodeName][0];
-
-                firstCyclesNode.links.some( (link: SankeyDiagramLink, index: number) => {
-                    let linkToNodeFromCycle = cycles[nodeName].some((node: SankeyDiagramNode) => {
-                        return node === link.destination;
-                    });
-
-                    if (link.source === firstCyclesNode && linkToNodeFromCycle) {
-                        SankeyDiagram.swapNodes(link);
-                    }
-                    if (link.destination === link.source) {
-                        link.direction = SankeyLinkDirrections.SelfLink;
-                    }
-
-                    let exsistCycles = this.checkCycles(nodes);
-
-                    // if inverting of link doesn't increase cycles on graph revert link inversion
-                    if (Object.keys(exsistCycles).length >= Object.keys(cycles).length) {
-                        SankeyDiagram.swapNodes(link);
-                        return false;
-                    }
-
-                    SankeyDiagram.updateValueOfNode(link.destination);
-                    SankeyDiagram.updateValueOfNode(link.source);
-                    return true;
-                });
-
-                SankeyDiagram.updateValueOfNode(firstCyclesNode);
-            }
-
-            return links;
-        }
-
         private processCyclesForwardLinks(cycles: SankeyDiagramCycleDictionary, nodes: SankeyDiagramNode[], links: SankeyDiagramLink[], settings: SankeyDiagramSettings): SankeyDiagramLink[] {
             let createdNodes: SankeyDiagramNode[] = [];
             for (let nodeName in cycles) {
@@ -484,6 +447,32 @@ module powerbi.extensibility.visual {
             return links;
         }
 
+        // in this method we breaking simple cycles
+        private processCyclesForBackwardLinks(cycles: SankeyDiagramCycleDictionary, nodes: SankeyDiagramNode[], links: SankeyDiagramLink[], settings: SankeyDiagramSettings): SankeyDiagramLink[] {
+            let createdNodes: SankeyDiagramNode[] = [];
+            for (let nodeName in cycles) {
+                let firstCyclesNode: SankeyDiagramNode = cycles[nodeName][cycles[nodeName].length - 1];
+
+                // make output links as backward links for node
+                let outputLinks = firstCyclesNode.links.filter((link: SankeyDiagramLink) => {
+                    if (link.source === firstCyclesNode || link.source === link.destination) {
+                        return true;
+                    }
+                    return false;
+                });
+
+                outputLinks.forEach( (link: SankeyDiagramLink) => {
+                    link.direction === SankeyLinkDirrections.Backward;
+                    SankeyDiagram.swapNodes(link);
+                });
+
+                SankeyDiagram.updateValueOfNode(firstCyclesNode);
+                SankeyDiagram.fixLinksCount(firstCyclesNode);
+            }
+
+            return links;
+        }
+
         private checkNodePositionSettings(nodes: SankeyDiagramNode[], settings: SankeyDiagramSettings) {
             let nodePositions: SankeyDiagramNodePositionSetting[] = settings._nodePositions;
 
@@ -875,7 +864,8 @@ module powerbi.extensibility.visual {
             node.links.forEach( (currentValue: SankeyDiagramLink) => {
                 node.inputWeight +=
                 currentValue.destination === node &&
-                currentValue.destination !== currentValue.source
+                currentValue.destination !== currentValue.source &&
+                currentValue.direction === SankeyLinkDirrections.Forward
                 ?
                 currentValue.weigth
                 :
@@ -1288,13 +1278,15 @@ module powerbi.extensibility.visual {
 
                 node.x *= scale.x;
 
-                let selfLinkHeight: number =  d3.sum( node.links.filter( l => l.direction === SankeyLinkDirrections.SelfLink ).map( l => l.weigth / 2 ) ) ;
+                let selfLinkHeight: number =  d3.max( node.links.filter( l => l.direction === SankeyLinkDirrections.SelfLink ).map( l => l.weigth ) ) ;
 
+                if (!selfLinkHeight) {
+                    selfLinkHeight = 0;
+                }
                 if (ignoreSelfLinkWeight && selfLinkHeight > 0) {
                     selfLinkHeight = node.width;
                 }
 
-                // because  backward links doesn't take a place
                 node.height = ( Math.max( node.inputWeight, node.outputWeight, node.inputWeight + selfLinkHeight, node.outputWeight + selfLinkHeight )
                 ) * scale.y;