Merge pull request #316 from FormidableLabs/bug/bar-bugs

Bug/bar bugs

src/victory-primitives/bar.js

@@ -11,6 +11,8 @@ export default class Bar extends React.Component {
   static propTypes = {
     ...CommonProps,
     alignment: PropTypes.oneOf(["start", "middle", "end"]),
+    barRatio: PropTypes.number,
+    cornerRadius: PropTypes.number,
     datum: PropTypes.object,
     horizontal: PropTypes.bool,
     padding: PropTypes.oneOfType([
@@ -64,32 +66,42 @@ export default class Bar extends React.Component {
     const alignment = props.alignment || "middle";
     const size = alignment === "middle" ? width / 2 : width;
     const sign = horizontal ? -1 : 1;
-    const x0 = alignment === "start" ? x : x - (sign * size);
-    const x1 = alignment === "end" ? x : x + (sign * size);
     return {
-      y0: Math.round(y0),
-      y1: Math.round(y),
-      x0: Math.round(x0),
-      x1: Math.round(x1)
+      x0: alignment === "start" ? x : x - (sign * size),
+      x1: alignment === "end" ? x : x + (sign * size),
+      y0,
+      y1: y
     };
   }
 
   getVerticalBarPath(props, width) {
     const { x0, x1, y0, y1 } = this.getPosition(props, width);
+    const cornerRadius = props.cornerRadius || 0;
+    const sign = y0 > y1 ? 1 : -1;
+    const direction = sign > 0 ? "0 0 1" : "0 0 0";
+    const arc = `${cornerRadius} ${cornerRadius} ${direction}`;
     return `M ${x0}, ${y0}
-      L ${x0}, ${y1}
-      L ${x1}, ${y1}
+      L ${x0}, ${y1 + sign * cornerRadius}
+      A ${arc}, ${x0 + cornerRadius}, ${y1}
+      L ${x1 - cornerRadius}, ${y1}
+      A ${arc}, ${x1}, ${y1 + sign * cornerRadius}
       L ${x1}, ${y0}
       L ${x0}, ${y0}
       z`;
   }
 
   getHorizontalBarPath(props, width) {
     const { x0, x1, y0, y1 } = this.getPosition(props, width);
+    const cornerRadius = props.cornerRadius || 0;
+    const sign = y1 > y0 ? 1 : -1;
+    const direction = sign > 0 ? "0 0 1" : "0 0 0";
+    const arc = `${cornerRadius} ${cornerRadius} ${direction}`;
     return `M ${y0}, ${x0}
       L ${y0}, ${x1}
-      L ${y1}, ${x1}
-      L ${y1}, ${x0}
+      L ${y1 - sign * cornerRadius}, ${x1}
+      A ${arc}, ${y1}, ${x1 + cornerRadius}
+      L ${y1}, ${x0 - cornerRadius}
+      A ${arc}, ${y1 - sign * cornerRadius}, ${x0}
       L ${y0}, ${x0}
       z`;
   }
@@ -145,7 +157,13 @@ export default class Bar extends React.Component {
     }
   }
 
-  getVerticalPolarBarPath(props) {
+  getPathMoves(path) {
+    const moves = path.match(/[A-Z]/g);
+    const coords = path.split(/[A-Z]/);
+    return moves.map((m, i) => ({ command: m, coords: coords[i + 1].split(",") }));
+  }
+
+  getVerticalPolarBarPath(props) { // eslint-disable-line max-statements
     const { datum, scale, style, index, alignment } = props;
     const r1 = scale.y(datum._y0 || 0);
     const r2 = scale.y(datum._y1 !== undefined ? datum._y1 : datum._y);
@@ -161,11 +179,28 @@ export default class Bar extends React.Component {
       start = this.getStartAngle(props, index);
       end = this.getEndAngle(props, index);
     }
+    const cornerRadius = props.cornerRadius || 0;
     const path = d3Shape.arc()
       .innerRadius(r1)
       .outerRadius(r2)
       .startAngle(this.transformAngle(start))
       .endAngle(this.transformAngle(end));
+    if (cornerRadius) {
+      const withCorners = d3Shape.arc()
+        .innerRadius(r1)
+        .outerRadius(r2)
+        .startAngle(this.transformAngle(start))
+        .endAngle(this.transformAngle(end))
+        .cornerRadius(cornerRadius);
+      const moves = this.getPathMoves(path());
+      const cornerMoves = this.getPathMoves(withCorners());
+      // eslint-disable-next-line no-magic-numbers
+      const totalMoves = cornerMoves.slice(0, 4).concat(moves.slice(2));
+      return totalMoves.reduce((memo, move) => {
+        memo += `${move.command} ${move.coords.join()}`;
+        return memo;
+      }, "");
+    }
     return path();
   }
 
@@ -187,10 +222,10 @@ export default class Bar extends React.Component {
     const range = scale.x.range();
     const extent = Math.abs(range[1] - range[0]);
     const bars = data.length + 2;
-    const barRatio = 0.5;
+    const barRatio = props.barRatio || 0.5;
     // eslint-disable-next-line no-magic-numbers
     const defaultWidth = data.length < 2 ? 8 : (barRatio * extent / bars);
-    return Math.max(1, Math.round(defaultWidth));
+    return Math.max(1, defaultWidth);
   }
 
   // Overridden in victory-core-native

src/victory-primitives/flyout.js

@@ -76,18 +76,18 @@ export default class Flyout extends React.Component {
     const rightEdge = x + (width / 2);
     const leftEdge = x - (width / 2);
     const direction = orientation === "top" ? "0 0 0" : "0 0 1";
-    const arc = `${Math.round(cornerRadius)} ${Math.round(cornerRadius)} ${direction}`;
-    return `M ${Math.round(x - pointerWidth / 2)}, ${Math.round(pointerEdge)}
-      L ${Math.round(x)}, ${Math.round(y)}
-      L ${Math.round(x + pointerWidth / 2)}, ${Math.round(pointerEdge)}
-      L ${Math.round(rightEdge - cornerRadius)}, ${Math.round(pointerEdge)}
-      A ${arc} ${Math.round(rightEdge)}, ${Math.round(pointerEdge - sign * cornerRadius)}
-      L ${Math.round(rightEdge)}, ${Math.round(oppositeEdge + sign * cornerRadius)}
-      A ${arc} ${Math.round(rightEdge - cornerRadius)}, ${Math.round(oppositeEdge)}
-      L ${Math.round(leftEdge + cornerRadius)}, ${Math.round(oppositeEdge)}
-      A ${arc} ${Math.round(leftEdge)}, ${Math.round(oppositeEdge + sign * cornerRadius)}
-      L ${Math.round(leftEdge)}, ${Math.round(pointerEdge - sign * cornerRadius)}
-      A ${arc} ${Math.round(leftEdge + cornerRadius)}, ${Math.round(pointerEdge)}
+    const arc = `${cornerRadius} ${cornerRadius} ${direction}`;
+    return `M ${x - pointerWidth / 2}, ${pointerEdge}
+      L ${x}, ${y}
+      L ${x + pointerWidth / 2}, ${pointerEdge}
+      L ${rightEdge - cornerRadius}, ${pointerEdge}
+      A ${arc} ${rightEdge}, ${pointerEdge - sign * cornerRadius}
+      L ${rightEdge}, ${oppositeEdge + sign * cornerRadius}
+      A ${arc} ${rightEdge - cornerRadius}, ${oppositeEdge}
+      L ${leftEdge + cornerRadius}, ${oppositeEdge}
+      A ${arc} ${leftEdge}, ${oppositeEdge + sign * cornerRadius}
+      L ${leftEdge}, ${pointerEdge - sign * cornerRadius}
+      A ${arc} ${leftEdge + cornerRadius}, ${pointerEdge}
       z`;
   }
 
@@ -101,18 +101,18 @@ export default class Flyout extends React.Component {
     const bottomEdge = y + height / 2;
     const topEdge = y - height / 2;
     const direction = orientation === "right" ? "0 0 0" : "0 0 1";
-    const arc = `${Math.round(cornerRadius)} ${Math.round(cornerRadius)} ${direction}`;
-    return `M ${Math.round(pointerEdge)}, ${Math.round(y - pointerWidth / 2)}
-      L ${Math.round(x)}, ${Math.round(y)}
-      L ${Math.round(pointerEdge)}, ${Math.round(y + pointerWidth / 2)}
-      L ${Math.round(pointerEdge)}, ${Math.round(bottomEdge - cornerRadius)}
-      A ${arc} ${Math.round(pointerEdge + sign * cornerRadius)}, ${Math.round(bottomEdge)}
+    const arc = `${cornerRadius} ${cornerRadius} ${direction}`;
+    return `M ${pointerEdge}, ${y - pointerWidth / 2}
+      L ${x}, ${y}
+      L ${pointerEdge}, ${y + pointerWidth / 2}
+      L ${pointerEdge}, ${bottomEdge - cornerRadius}
+      A ${arc} ${pointerEdge + sign * cornerRadius}, ${bottomEdge}
       L ${oppositeEdge - sign * cornerRadius}, ${bottomEdge}
-      A ${arc} ${Math.round(oppositeEdge)}, ${Math.round(bottomEdge - cornerRadius)}
+      A ${arc} ${oppositeEdge}, ${bottomEdge - cornerRadius}
       L ${oppositeEdge}, ${topEdge + cornerRadius}
-      A ${arc} ${Math.round(oppositeEdge - sign * cornerRadius)}, ${Math.round(topEdge)}
-      L ${Math.round(pointerEdge + sign * cornerRadius)}, ${Math.round(topEdge)}
-      A ${arc} ${Math.round(pointerEdge)}, ${Math.round(topEdge + cornerRadius)}
+      A ${arc} ${oppositeEdge - sign * cornerRadius}, ${topEdge}
+      L ${pointerEdge + sign * cornerRadius}, ${topEdge}
+      A ${arc} ${pointerEdge}, ${topEdge + cornerRadius}
       z`;
   }
 

src/victory-primitives/path-helpers.js

@@ -3,8 +3,6 @@ import { range } from "lodash";
 
 export default {
   circle(x, y, size) {
-    x = Math.round(x);
-    y = Math.round(y);
     return `M ${x}, ${y}
       m ${-size}, 0
       a ${size}, ${size} 0 1,0 ${size * 2},0
@@ -13,12 +11,10 @@ export default {
 
 
   square(x, y, size) {
-    x = Math.round(x);
-    y = Math.round(y);
     const baseSize = 0.87 * size; // eslint-disable-line no-magic-numbers
     const x0 = x - baseSize;
     const y1 = y + baseSize;
-    const distance = Math.floor(x + baseSize - x0);
+    const distance = x + baseSize - x0;
     return `M ${x0}, ${y1}
       h${distance}
       v-${distance}
@@ -27,8 +23,6 @@ export default {
   },
 
   diamond(x, y, size) {
-    x = Math.round(x);
-    y = Math.round(y);
     const baseSize = 0.87 * size; // eslint-disable-line no-magic-numbers
     const length = Math.sqrt(2 * (baseSize * baseSize));
     return `M ${x}, ${y + length}
@@ -40,8 +34,6 @@ export default {
   },
 
   triangleDown(x, y, size) {
-    x = Math.round(x);
-    y = Math.round(y);
     const height = size / 2 * Math.sqrt(3);
     const x0 = x - size;
     const x1 = x + size;
@@ -54,8 +46,6 @@ export default {
   },
 
   triangleUp(x, y, size) {
-    x = Math.round(x);
-    y = Math.round(y);
     const height = size / 2 * Math.sqrt(3);
     const x0 = x - size;
     const x1 = x + size;
@@ -68,8 +58,6 @@ export default {
   },
 
   plus(x, y, size) {
-    x = Math.round(x);
-    y = Math.round(y);
     const baseSize = 1.1 * size; // eslint-disable-line no-magic-numbers
     const distance = baseSize / 1.5; // eslint-disable-line no-magic-numbers
     return `
@@ -89,9 +77,7 @@ export default {
   },
 
   star(x, y, size) {
-    x = Math.round(x);
-    y = Math.round(y);
-    const baseSize = Math.round(1.35 * size); // eslint-disable-line no-magic-numbers
+    const baseSize = 1.35 * size; // eslint-disable-line no-magic-numbers
     const angle = Math.PI / 5; // eslint-disable-line no-magic-numbers
     const starCoords = range(10).map((index) => { // eslint-disable-line no-magic-numbers
       const length = index % 2 === 0 ? baseSize : baseSize / 2;

src/victory-util/domain.js

@@ -78,8 +78,8 @@ export default {
       const maxData = flatData.map((datum) => {
         return datum[`_${currentAxis}1`] || datum[`_${currentAxis}`] || 0;
       });
-      const min = Collection.getMinValue([...domain, ...minData]);
-      const max = Collection.getMaxValue([...domain, ...maxData]);
+      const min = Collection.getMinValue([...domain, ...minData, ...maxData]);
+      const max = Collection.getMaxValue([...domain, ...maxData, ...minData]);
       return [min, max];
     };
     const categoryDomain = this.getDomainFromCategories(props, axis);

test/client/spec/svg-test-helper.js

@@ -1,7 +1,6 @@
 import { property, max, min } from "lodash";
 
 const FLYOUT_SEQUENCE = ["M", "L", "L", "L", "A", "L", "A", "L", "A", "L", "A", "z"];
-const RECTANGLE_SEQUENCE = ["M", "L", "L", "L", "L", "z"];
 
 const parseSvgPathCommands = (commandStr) => {
   const matches = commandStr.match(
@@ -54,8 +53,6 @@ const expectations = {
   getBarShape(wrapper) {
     const commands = getPathCommandsFromWrapper(wrapper);
 
-    expect(exhibitsShapeSequence(wrapper, RECTANGLE_SEQUENCE, commands)).to.equal(true);
-
     const points = commands.filter((command) => { return command.name !== "z"; });
     const verticalPoints = points.map(property("args.1"));
     const horizontalPoints = points.map(property("args.0"));

test/client/spec/victory-primitives/bar.spec.js

@@ -26,8 +26,7 @@ describe("victory-primitives/bar", () => {
   it("should render a vertical bar", () => {
     const wrapper = shallow(<Bar {...baseProps}/>);
     const barShape = SvgTestHelper.getBarShape(wrapper);
-
-    expect(barShape.height).to.eql(10);
+    expect(Math.round(barShape.height)).to.eql(10);
   });
 
   it("should render a horizontal bar", () => {
@@ -36,7 +35,7 @@ describe("victory-primitives/bar", () => {
     const wrapper = shallow(<Bar {...props}/>);
     const barShape = SvgTestHelper.getBarShape(wrapper);
 
-    expect(barShape.width).to.eql(10);
+    expect(Math.round(barShape.width)).to.eql(10);
   });
 
   it("should render a default bar width when one is not provided", () => {
@@ -49,15 +48,15 @@ describe("victory-primitives/bar", () => {
     const wrapper = shallow(<Bar {...props}/>);
     const barShape = SvgTestHelper.getBarShape(wrapper);
 
-    expect(barShape.width).to.eql(1);
+    expect(Math.floor(barShape.width)).to.eql(2);
   });
 
   it("should allow override of width by passing a style", () => {
-    const props = Object.assign({}, baseProps, { style: { width: 1 } });
+    const props = Object.assign({}, baseProps, { style: { width: 3 } });
 
     const wrapper = shallow(<Bar {...props}/>);
     const barShape = SvgTestHelper.getBarShape(wrapper);
 
-    expect(barShape.width).to.eql(1);
+    expect(Math.floor(barShape.width)).to.eql(3);
   });
 });

test/client/spec/victory-primitives/path-helper.spec.js

@@ -58,7 +58,7 @@ describe("path-helpers", () => {
     it("draws a path for a star at the correct location", () => {
       const pathResult = PathHelpers.star(0, 0, 1);
       const angle = Math.PI / 5;
-      const baseSize = Math.round(1.35 * size);
+      const baseSize = 1.35 * size;
       expect(pathResult).to.contain(`M ${(baseSize) * Math.sin(angle) + x },
         ${(baseSize) * Math.cos(angle) + y}`);
     });