Merge pull request #54 from spotify/sankey-label-change

Fix for non-string SankeyDiagram nodeLabelText prop
spotify · Mar 12, 2018 · fac4214 · fac4214
2 parents c1f4b64 + 2e12b41
commit fac4214
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 35 deletions.
diff --git a/src/SankeyDiagram.js b/src/SankeyDiagram.js
@@ -90,37 +90,41 @@ const SankeyNodeLabel = props => {
   const distance = getWithNode(props.nodeLabelDistance) || 0;
   const labelContent = getWithNode(getLabelText);
   // don't render empty labels
-  if(_.isNull(labelContent) || _.isUndefined(labelContent) || labelContent === false || labelContent === "") {
+  if (_.isNull(labelContent) || _.isUndefined(labelContent) || labelContent === false || labelContent === "") {
     return null;
   }
 
+  // if `labelContent` is a string or number, it is rendered as text within a SVG <text> element
+  // otherwise, it is rendered as arbitrary SVG content
+  // allows users to render components inside a node label (eg. to add icon or link)
+  const isTextLabel = _.isString(labelContent) || _.isNumber(labelContent);
+  if(!isTextLabel)  {
+    return labelContent;
+  }
+
   const baseClassName = `sankey-node-label ${getWithNode(props.nodeLabelClassName)}`;
   const baseStyle = getWithNode(props.nodeLabelStyle);
   let position;
   let textStyle;
-  let translate;
 
-  // use placement prop to determine x, y, alignmentBaseline and
+  // use placement prop to determine x, y, alignmentBaseline and textAnchor
   if (placement === "above") {
     // render label above node, centered horizontally
     textStyle = {alignmentBaseline: "baseline", textAnchor: "middle", ...baseStyle};
-    translate = '-50%, -100%';
     position = {
       x: node.x0 + Math.abs(node.x1 - node.x0) / 2,
       y: node.y0 - distance
     };
   } else if (placement === "below") {
     // render label above node, centered horizontally
     textStyle = {alignmentBaseline: "hanging", textAnchor: "middle", ...baseStyle};
-    translate = '-50%, 0';
     position = {
       x: node.x0 + Math.abs(node.x1 - node.x0) / 2,
       y: node.y1 + distance
     };
   } else if (placement === "before") {
     // render label before (to left of) node, centered vertically
     textStyle = {alignmentBaseline: "middle", textAnchor: "end", ...baseStyle};
-    translate = '-100%, -50%';
     position = {
       x: node.x0 - distance,
       y: node.y0 + Math.abs(node.y1 - node.y0) / 2
@@ -130,39 +134,18 @@ const SankeyNodeLabel = props => {
       console.warn(`${placement} is not a valid value for nodeLabelPlacement - defaulting to "after"`);
     // render label after (to right of) node, centered vertically
     textStyle = {alignmentBaseline: "middle", textAnchor: "start", ...baseStyle};
-    translate = '0, -50%';
     position = {
       x: node.x1 + distance,
       y: node.y0 + Math.abs(node.y1 - node.y0) / 2
     };
   }
 
-  // if `labelContent` is a string or number, it is rendered as text within a SVG <text> element
-  // otherwise, it is rendered as arbitrary HTML content inside of a <foreignObject />
-  // allows users to render arbitrary components inside a node label (eg. to add an icon or link)
-  const isTextLabel = _.isString(labelContent) || _.isNumber(labelContent);
-
-  if(isTextLabel) {
-    const className = `${baseClassName} sankey-node-label-text`;
-    return (
-      <text {...position} className={className} style={textStyle}>
-        {labelContent}
-      </text>
-    );
-
-  } else {
-    const className = `${baseClassName} sankey-node-label-html`;
-    // wrap HTML labels in a div with "inline-block" so that translation (%) is relative to width of its content
-    const style = {...baseStyle, display: "inline-block", transform: `translate(${translate})`};
-    // give foreignObject container a large width/height to prevent unintentional line breaks/cut off content
-    return (
-      <foreignObject {...position} style={{overflow: "visible"}} width="5000" height="5000">
-        <div className={className} style={style}>
-          {labelContent}
-        </div>
-      </foreignObject>
-    );
-  }
+  const className = `${baseClassName} sankey-node-label-text`;
+  return (
+    <text {...position} className={className} style={textStyle}>
+      {labelContent}
+    </text>
+  );
 };
 
 const SankeyLinkLabel = props => {
@@ -466,7 +449,10 @@ export default class SankeyDiagram extends React.Component {
     /**
      * Accessor function `nodeLabelText(node, graph)` which returns the content to be used for node labels.
      * The function may return a string/number (rendered as SVG `<text>`),
-     * or arbitrary React HTML element(s) (rendered as HTML wrapped in SVG `<foreignObject>`).
+     * or arbitrary React SVG element(s) (rendered as-is inside the SVG).
+     * NOTE: in the latter case (returning arbitrary SVG), `nodeLabelPlacement`, `nodeLabelDistance`,
+     * `nodeLabelClassName` and `nodeLabelStyle` props will not be applied -
+     * user is responsible for all positioning and attributes on this element.
      */
     nodeLabelText: PropTypes.func,
     /**

diff --git a/tests/jsdom/spec/SankeyDiagram.spec.js b/tests/jsdom/spec/SankeyDiagram.spec.js
@@ -610,7 +610,7 @@ describe("SankeyDiagram", () => {
       id: "lemons",
       name: "Sour Lemons"
     };
-    it("renders a node label", () => {
+    it("renders a node label in a <text> element", () => {
       const label = mount(<SankeyNodeLabel {...{node: basicNodeObj, nodeLabelText: () => "ok"}} />);
       const text = label.find("text");
       expect(text).to.have.length(1);
@@ -643,6 +643,20 @@ describe("SankeyDiagram", () => {
       expect(textWithId).to.have.length(1);
       expect(textWithId.text()).to.equal("lemons");
     });
+    it("renders nodeLabelText as-is (not wrapped in <text>), if it returns an element instead of string", () => {
+      const label = mount(
+        <SankeyNodeLabel
+          {...{
+            node: basicNodeObj,
+            nodeLabelText: node => <rect x={node.x0} y={node.y0} width={50} height={20} />
+          }}
+        />
+      );
+      expect(label.find("text")).to.have.length(0);
+      expect(label.find("rect")).to.have.length(1);
+      expect(label.find("rect").props().width).to.equal(50);
+    });
+
     it("passes nodeLabelClassName and nodeLabelStyle through to the text element", () => {
       const nodeLabelClassName = "my-fun-node-label";
       const nodeLabelStyle = {fill: "salmon"};