make sure tickFormat is called with correct arguments, refactor

demo/components/victory-chart-demo.js

@@ -206,13 +206,14 @@ class App extends React.Component {
           </VictoryChart>
 
           <VictoryChart style={chartStyle}>
+            <VictoryAxis tickFormat={(t, i, ts) => `${t}s ${i} ${ts[0]}`}/>
             <VictoryBar
               groupComponent={<VictoryClipContainer/>}
               style={{ data: { fill: "tomato" } }}
               data={[
-                { x: 1, y: 1 },
-                { x: 2, y: 2 },
-                { x: 3, y: 7 }
+                { x: "one", y: 1 },
+                { x: "two", y: 2 },
+                { x: "three", y: 7 }
               ]}
             />
          </VictoryChart>

src/components/victory-axis/helper-methods.js

@@ -1,4 +1,5 @@
 import { includes, defaults, defaultsDeep, isFunction, range, without } from "lodash";
+import Axis from "../../helpers/axis";
 import { Helpers, Scale, Domain } from "victory-core";
 
 const orientationSign = {
@@ -224,7 +225,7 @@ export default {
         axisLabel: axisLabelProps,
         ticks: this.getTickProps(tickLayout, styles.tickStyle, tick),
         tickLabels: this.getTickLabelProps(
-          tickLayout, styles.labelStyle, anchors, tick, tickFormat(tick, index)
+          tickLayout, styles.labelStyle, anchors, tick, tickFormat(tick, index, ticks)
         ),
         grid: this.getGridProps(gridLayout, styles.gridStyle, tick)
       };
@@ -244,7 +245,7 @@ export default {
     const scale = this.getScale(props);
     const domain = this.getDomain(props);
     const ticks = this.getTicks(props, scale);
-    const tickFormat = this.getTickFormat(props, scale, ticks);
+    const tickFormat = Axis.getTickFormat(props, scale);
     const anchors = this.getAnchors(orientation, isVertical);
 
     return {
@@ -306,20 +307,6 @@ export default {
     };
   },
 
-  getTickFormat(props, scale) {
-    if (props.tickFormat && isFunction(props.tickFormat)) {
-      return props.tickFormat;
-    } else if (props.tickFormat && Array.isArray(props.tickFormat)) {
-      return (x, index) => props.tickFormat[index];
-    } else if (Helpers.stringTicks(props)) {
-      return (x, index) => props.tickValues[index];
-    } else if (scale.tickFormat && isFunction(scale.tickFormat)) {
-      return scale.tickFormat();
-    } else {
-      return (x) => x;
-    }
-  },
-
   getLabelPadding(props, style) {
     const labelStyle = style.axisLabel || {};
     if (typeof labelStyle.padding !== "undefined" && labelStyle.padding !== null) {

src/components/victory-chart/helper-methods.js

@@ -1,4 +1,4 @@
-import { isFunction, flow, invert, sortBy, values } from "lodash";
+import { invert, sortBy, values } from "lodash";
 import Axis from "../../helpers/axis";
 import Wrapper from "../../helpers/wrapper";
 import React from "react";
@@ -123,62 +123,24 @@ export default {
     return this.getTicksFromAxis(...args) || this.getTicksFromData(...args);
   },
 
-  getArrayFormatter(tickValues, tickFormat) {
-    // For tickFormat lists, use the list in order
-    if (Array.isArray(tickFormat)) {
-      return (index) => () => tickFormat[index];
-    }
-
-    // For non-numerical tickValues lists, use the list in order
-    if (Array.isArray(tickValues) && !Collection.containsNumbers(tickValues)) {
-      return (index) => () => tickValues[index];
-    }
-
-    // identity
-    return () => (tick) => tick;
-  },
-
-  getFunctionFormatter(tickFormat) {
-    return isFunction(tickFormat) ? tickFormat : identity;
-  },
-
-  getStringMapFormatter(stringMap) {
-    // When string maps are present, convert tick to string
-    if (stringMap !== null) {
+  getTickFormat(component, axis, calculatedProps) {
+    const currentAxis = Helpers.getCurrentAxis(axis, calculatedProps.horizontal);
+    const stringMap = calculatedProps.stringMap[currentAxis];
+    const tickValues = component.props.tickValues;
+    const useIdentity = tickValues && !Collection.containsStrings(tickValues) &&
+      !Collection.containsDates(tickValues);
+    if (useIdentity) {
+      return identity;
+    } else if (stringMap !== null) {
       const tickValueArray = sortBy(values(stringMap), (n) => n);
       const invertedStringMap = invert(stringMap);
       const dataNames = tickValueArray.map((tick) => invertedStringMap[tick]);
       // string ticks should have one tick of padding at the beginning
       const dataTicks = ["", ...dataNames, ""];
       return (x) => dataTicks[x];
+    } else {
+      return calculatedProps.scale[currentAxis].tickFormat() || identity;
     }
-
-    return identity;
-  },
-
-  getScaleFormatter(stringMap, tickFormat, tickValues, calculatedProps, currentAxis) { // eslint-disable-line
-    if (stringMap || tickFormat || tickValues) {
-      return identity;
-    }
-
-    return calculatedProps.scale[currentAxis].tickFormat();
-  },
-
-  getTickFormat(component, axis, calculatedProps) {
-    const { tickFormat, tickValues } = component.props;
-    const currentAxis = Helpers.getCurrentAxis(axis, calculatedProps.horizontal);
-    const stringMap = calculatedProps.stringMap[currentAxis];
-
-    return (tick, index) => {
-      const tickFormatterPipeline = [
-        this.getScaleFormatter(stringMap, tickFormat, tickValues, calculatedProps, currentAxis),
-        this.getStringMapFormatter(stringMap),
-        this.getArrayFormatter(tickValues, tickFormat)(index),
-        this.getFunctionFormatter(tickFormat)
-      ];
-
-      return flow(tickFormatterPipeline)(tick);
-    };
   },
 
   createStringMap(props, axis, childComponents) {

src/components/victory-chart/victory-chart.js

@@ -98,11 +98,13 @@ export default class VictoryChart extends React.Component {
   }
 
   getAxisProps(child, props, calculatedProps) {
-    const { domain, scale, originSign } = calculatedProps;
+    const { domain, scale, originSign, stringMap } = calculatedProps;
     const axis = child.type.getAxis(child.props);
     const axisOffset = ChartHelpers.getAxisOffset(props, calculatedProps);
     const tickValues = ChartHelpers.getTicks(calculatedProps, axis, child);
-    const tickFormat = ChartHelpers.getTickFormat(child, axis, calculatedProps);
+    const tickFormat = child.props.tickFormat ?
+      Axis.getTickFormat(child.props, scale, stringMap) :
+      ChartHelpers.getTickFormat(child, axis, calculatedProps);
     const offsetY = axis === "y" ? undefined : axisOffset.y;
     const offsetX = axis === "x" ? undefined : axisOffset.x;
     const crossAxis = child.props.crossAxis === false ? false : true;

src/helpers/axis.js

@@ -1,5 +1,5 @@
 import { Collection } from "victory-core";
-import { identity } from "lodash";
+import { identity, isFunction, invert } from "lodash";
 import React from "react";
 
 export default {
@@ -166,5 +166,26 @@ export default {
    */
   stringTicks(props) {
     return props.tickValues !== undefined && Collection.containsStrings(props.tickValues);
+  },
+
+  getTickFormat(props, scale, stringMap) {
+    const stringTicks = this.stringTicks(props);
+    const axis = this.getAxis(props);
+    const applyStringTicks = (tick, index, ticks) => {
+      const invertedStringMap = invert(stringMap[axis]);
+      const stringTickArray = ticks.map((t) => invertedStringMap[t]);
+      return props.tickFormat(invertedStringMap[tick], index, stringTickArray);
+    };
+    if (props.tickFormat && isFunction(props.tickFormat)) {
+      return stringMap && stringMap[axis] ? applyStringTicks : props.tickFormat;
+    } else if (props.tickFormat && Array.isArray(props.tickFormat)) {
+      return (x, index) => props.tickFormat[index];
+    } else if (stringTicks) {
+      return (x, index) => props.tickValues[index];
+    } else if (scale.tickFormat && isFunction(scale.tickFormat)) {
+      return scale.tickFormat();
+    } else {
+      return (x) => x;
+    }
   }
 };

test/client/spec/components/victory-axis/helper-methods.spec.js

@@ -137,54 +137,4 @@ describe("victory-axis/helper-methods", () => {
       expect(tickResult).to.be.an("array").and.not.have.members([0]);
     });
   });
-
-  describe("getTickFormat", () => {
-    let sandbox;
-    const scale = Scale.getBaseScale({ scale: { x: "linear" } }, "x");
-    const ticks = [1, 2, 3, 4, 5];
-    beforeEach(() => {
-      sandbox = sinon.sandbox.create();
-      sandbox.spy(Helpers, "stringTicks");
-      sandbox.stub(scale, "tickFormat");
-    });
-
-    afterEach(() => {
-      sandbox.restore();
-    });
-
-    it("returns tickFormat function from props", () => {
-      const props = { tickFormat: (x) => x * 5 };
-      const tickProps = { scale, ticks };
-      const formatResult = AxisHelpers.getTickFormat(props, tickProps);
-      expect(Helpers.stringTicks).notCalled;
-      expect(scale.tickFormat).notCalled;
-      expect(formatResult).to.eql(props.tickFormat);
-    });
-
-    it("converts tickFormat array from props to a function", () => {
-      const props = { tickFormat: [1, 2, 3, 4, 5] };
-      const tickProps = { scale, ticks };
-      const formatResult = AxisHelpers.getTickFormat(props, tickProps);
-      expect(Helpers.stringTicks).notCalled;
-      expect(scale.tickFormat).notCalled;
-      expect(formatResult).to.be.a("function");
-    });
-
-    it("converts tickFormat string array from props to a function", () => {
-      const props = { tickValues: ["cats", "dogs", "birds"] };
-      const tickProps = { scale, ticks };
-      const formatResult = AxisHelpers.getTickFormat(props, tickProps);
-      expect(Helpers.stringTicks).calledWith(props).and.returned(true);
-      expect(scale.tickFormat).notCalled;
-      expect(formatResult).to.be.a("function");
-    });
-
-    it("calculates a tick format from scale", () => {
-      const props = {};
-      const tickProps = { scale, ticks };
-      const formatResult = AxisHelpers.getTickFormat(props, tickProps);
-      expect(Helpers.stringTicks).calledWith(props).and.returned(false);
-      expect(formatResult).to.be.a("function");
-    });
-  });
 });

test/client/spec/components/victory-chart/helper-methods.spec.js

@@ -186,57 +186,6 @@ describe("victory-chart/helpers-methods", () => {
       );
       expect(formatResult(1)).to.equal("scaleFormatTick");
     });
-
-    it("uses custom tickFormat", () => {
-      const props = { tickFormat: (tick) => tick + 1 };
-      const victoryAxis = getVictoryAxis(props);
-      const formatResult = Helpers.getTickFormat(victoryAxis, "x", { stringMap: nullStringMap });
-      expect(formatResult).to.be.a("function");
-      expect(formatResult(1)).to.equal(2);
-    });
-
-    it("passes string map result to custom tickFormat prop", () => {
-      const props = { tickFormat: (tick) => `${tick} yo` };
-      const victoryAxis = getVictoryAxis(props);
-      const formatResult = Helpers.getTickFormat(victoryAxis, "x", { stringMap });
-      expect(formatResult).to.be.a("function");
-      expect(formatResult(1)).to.equal("a yo");
-    });
-
-    it("uses string array tickValues for tick formatting", () => {
-      const props = { tickValues: ["orange", "banana", "apple"] };
-      const victoryAxis = getVictoryAxis(props);
-      const formatResult = Helpers.getTickFormat(victoryAxis, "x", { stringMap: nullStringMap });
-      expect(formatResult).to.be.a("function");
-      expect(formatResult(5, 0)).to.equal("orange");
-      expect(formatResult(5, 2)).to.equal("apple");
-    });
-
-    it("uses string array tickFormat for tick formatting", () => {
-      const props = { tickFormat: ["orange", "banana", "apple"] };
-      const victoryAxis = getVictoryAxis(props);
-      const formatResult = Helpers.getTickFormat(victoryAxis, "x", { stringMap: nullStringMap });
-      expect(formatResult).to.be.a("function");
-      expect(formatResult(5, 0)).to.equal("orange");
-      expect(formatResult(5, 2)).to.equal("apple");
-    });
-
-    it("uses tickFormat array over tickValues", () => {
-      const props = { tickValues: [1, 2, 3], tickFormat: ["orange", "banana", "apple"] };
-      const victoryAxis = getVictoryAxis(props);
-      const formatResult = Helpers.getTickFormat(victoryAxis, "x", { stringMap: nullStringMap });
-      expect(formatResult).to.be.a("function");
-      expect(formatResult(5, 0)).to.equal("orange");
-      expect(formatResult(5, 2)).to.equal("apple");
-    });
-
-    it("runs tickFormat fn after tick values are applied", () => {
-      const props = { tickValues: ["a", "b", "c"], tickFormat: (tick) => `${tick} yo` };
-      const victoryAxis = getVictoryAxis(props);
-      const formatResult = Helpers.getTickFormat(victoryAxis, "x", { stringMap: nullStringMap });
-      expect(formatResult).to.be.a("function");
-      expect(formatResult(1, 1)).to.equal("b yo");
-    });
   });
 
   describe("getTicks", () => {

test/client/spec/helpers/axis.spec.js

@@ -1,6 +1,7 @@
 /* global sinon */
 /* eslint-disable no-unused-expressions,react/no-multi-comp */
 import Axis from "src/helpers/axis";
+import { Scale } from "victory-core";
 import React from "react";
 import { VictoryAxis, VictoryBar } from "src/index";
 
@@ -33,4 +34,54 @@ describe("helpers/axis", () => {
       expect(componentResult).to.eql(independentAxis);
     });
   });
+
+  describe("getTickFormat", () => {
+    let sandbox;
+    const scale = Scale.getBaseScale({ scale: { x: "linear" } }, "x");
+    const ticks = [1, 2, 3, 4, 5];
+    beforeEach(() => {
+      sandbox = sinon.sandbox.create();
+      sandbox.spy(Axis, "stringTicks");
+      sandbox.stub(scale, "tickFormat");
+    });
+
+    afterEach(() => {
+      sandbox.restore();
+    });
+
+    it("returns tickFormat function from props", () => {
+      const props = { tickFormat: (x) => x * 5 };
+      const tickProps = { scale, ticks };
+      const formatResult = Axis.getTickFormat(props, tickProps);
+      expect(Axis.stringTicks).notCalled;
+      expect(scale.tickFormat).notCalled;
+      expect(formatResult).to.eql(props.tickFormat);
+    });
+
+    it("converts tickFormat array from props to a function", () => {
+      const props = { tickFormat: [1, 2, 3, 4, 5] };
+      const tickProps = { scale, ticks };
+      const formatResult = Axis.getTickFormat(props, tickProps);
+      expect(Axis.stringTicks).notCalled;
+      expect(scale.tickFormat).notCalled;
+      expect(formatResult).to.be.a("function");
+    });
+
+    it("converts tickFormat string array from props to a function", () => {
+      const props = { tickValues: ["cats", "dogs", "birds"] };
+      const tickProps = { scale, ticks };
+      const formatResult = Axis.getTickFormat(props, tickProps);
+      expect(Axis.stringTicks).calledWith(props).and.returned(true);
+      expect(scale.tickFormat).notCalled;
+      expect(formatResult).to.be.a("function");
+    });
+
+    it("calculates a tick format from scale", () => {
+      const props = {};
+      const tickProps = { scale, ticks };
+      const formatResult = Axis.getTickFormat(props, tickProps);
+      expect(Axis.stringTicks).calledWith(props).and.returned(false);
+      expect(formatResult).to.be.a("function");
+    });
+  });
 });