Merge pull request #1208 from MxFr/performance-eventkey

Improve performance for addEventKeys
FormidableLabs · Dec 20, 2018 · a4e2b08 · a4e2b08
2 parents 3d4802d + b552135
commit a4e2b08
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 23 deletions.
diff --git a/packages/victory-core/src/victory-util/data.js b/packages/victory-core/src/victory-util/data.js
@@ -85,10 +85,17 @@ function getEventKey(key) {
 
 // Returns data with an eventKey prop added to each datum
 function addEventKeys(props, data) {
+  const hasEventKeyAccessor = !!props.eventKey;
   const eventKeyAccessor = getEventKey(props.eventKey);
-  return data.map((datum, index) => {
-    const eventKey = datum.eventKey || eventKeyAccessor(datum) || index;
-    return assign({ eventKey }, datum);
+  return data.map((datum) => {
+    if (datum.eventKey) {
+      return datum;
+    } else if (hasEventKeyAccessor) {
+      const eventKey = eventKeyAccessor(datum);
+      return eventKey ? assign({ eventKey }, datum) : datum;
+    } else {
+      return datum;
+    }
   });
 }
 

diff --git a/packages/victory-scatter/src/helper-methods.js b/packages/victory-scatter/src/helper-methods.js
@@ -78,7 +78,7 @@ const getBaseProps = (props, fallbackProps) => {
   } };
 
   return data.reduce((childProps, datum, index) => {
-    const eventKey = datum.eventKey;
+    const eventKey = datum.eventKey || index;
     const { x, y } = Helpers.scalePoint(props, datum);
     const dataProps = {
       x, y, datum, data, index, scale, polar, origin,

diff --git a/packages/victory-voronoi/src/helper-methods.js b/packages/victory-voronoi/src/helper-methods.js
@@ -53,7 +53,7 @@ const getBaseProps = (props, fallbackProps) => {
 
   return data.reduce((childProps, datum, index) => {
     const polygon = without(polygons[index], "data");
-    const eventKey = datum.eventKey;
+    const eventKey = datum.eventKey || index;
     const { x, y } = Helpers.scalePoint(props, datum);
     const dataProps = {
       x, y, datum, data, index, scale, polygon, origin,

diff --git a/test/client/spec/mock-components.js b/test/client/spec/mock-components.js
@@ -61,6 +61,7 @@ class MockVictoryComponent extends React.Component {
             index,
             datum,
             data,
+            eventKey: index,
             style: {}
           }
         }

diff --git a/test/client/spec/victory-core/victory-util/add-events.spec.js b/test/client/spec/victory-core/victory-util/add-events.spec.js
@@ -54,6 +54,40 @@ describe("victory-util/add-events", () => {
     expectEventsTriggered(getDataComponents, dataComponentIsAltered, [true, true], wrapper);
   });
 
+  it("should set up events on data components scoped with an event key", () => {
+    const wrapper = mount(
+      <EventedMockVictoryComponent
+        data={[{ x: 1, y: 2 }, { x: 3, y: 4 }]}
+        events={[
+          {
+            target: "data",
+            eventKey: "1",
+            eventHandlers: {
+              onClick: () => {
+                return [{
+                  target: "data",
+                  mutation: () => {
+                    return { style: { fill: "tomato" } };
+                  }
+                }];
+              }
+            }
+          }
+        ]}
+      />
+    );
+
+    const dataComponentIsAltered = (dataComponent) => {
+      return get(dataComponent.props(), "style.fill") === "tomato";
+    };
+
+    expectEventsTriggered(getDataComponents, dataComponentIsAltered, [false, false], wrapper);
+    getDataComponents(wrapper).at(0).simulate("click");
+    expectEventsTriggered(getDataComponents, dataComponentIsAltered, [false, false], wrapper);
+    getDataComponents(wrapper).at(1).simulate("click");
+    expectEventsTriggered(getDataComponents, dataComponentIsAltered, [false, true], wrapper);
+  });
+
   it("should set up events on data components to target labels", () => {
     const wrapper = mount(
       <EventedMockVictoryComponent

diff --git a/test/client/spec/victory-core/victory-util/data.spec.js b/test/client/spec/victory-core/victory-util/data.spec.js
@@ -129,7 +129,7 @@ describe("victory-util/data", () => {
         const props = { data: createData(dataset) };
         const formatted = Data.formatData(dataset, props);
         expect(formatted).to.be.an.array;
-        expect(formatted[0]).to.have.keys(["_x", "_y", "x", "y", "eventKey"]);
+        expect(formatted[0]).to.have.keys(["_x", "_y", "x", "y"]);
       });
     });
   });
@@ -141,8 +141,8 @@ describe("victory-util/data", () => {
           const data = createData([{ x: "kittens", y: 3 }, { x: "cats", y: 5 }]);
           const props = { data, x: "x", y: "y" };
           const expectedReturnWithEventKeys = [
-             { _x: 1, x: "kittens", xName: "kittens", _y: 3, y: 3, eventKey: 0 },
-             { _x: 2, x: "cats", xName: "cats", _y: 5, y: 5, eventKey: 1 }
+             { _x: 1, x: "kittens", xName: "kittens", _y: 3, y: 3 },
+             { _x: 2, x: "cats", xName: "cats", _y: 5, y: 5 }
           ];
           const returnData = Data.getData(props);
           expect(returnData).to.eql(expectedReturnWithEventKeys);
@@ -186,9 +186,9 @@ describe("victory-util/data", () => {
           const returnData = Data.getData({ data });
 
           expect(returnData).to.eql([
-            { _x: 2, x: 2, _y: 2, y: 2, eventKey: 0 },
-            { _x: 1, x: 1, _y: 3, y: 3, eventKey: 1 },
-            { _x: 3, x: 3, _y: 1, y: 1, eventKey: 2 }
+            { _x: 2, x: 2, _y: 2, y: 2 },
+            { _x: 1, x: 1, _y: 3, y: 3 },
+            { _x: 3, x: 3, _y: 1, y: 1 }
           ]);
         });
 
@@ -202,9 +202,9 @@ describe("victory-util/data", () => {
           const returnData = Data.getData({ data, sortKey: "order" });
 
           expect(returnData).to.eql([
-            { _x: 3, x: 3, _y: 3, y: 3, order: 1, eventKey: 0 },
-            { _x: 1, x: 1, _y: 1, y: 1, order: 2, eventKey: 1 },
-            { _x: 2, x: 2, _y: 2, y: 2, order: 3, eventKey: 2 }
+            { _x: 3, x: 3, _y: 3, y: 3, order: 1 },
+            { _x: 1, x: 1, _y: 1, y: 1, order: 2 },
+            { _x: 2, x: 2, _y: 2, y: 2, order: 3 }
           ]);
         });
 
@@ -218,9 +218,9 @@ describe("victory-util/data", () => {
           const returnData = Data.getData({ data, sortKey: "order", sortOrder: "descending" });
 
           expect(returnData).to.eql([
-            { _x: 2, x: 2, _y: 2, y: 2, order: 3, eventKey: 0 },
-            { _x: 1, x: 1, _y: 1, y: 1, order: 2, eventKey: 1 },
-            { _x: 3, x: 3, _y: 3, y: 3, order: 1, eventKey: 2 }
+            { _x: 2, x: 2, _y: 2, y: 2, order: 3 },
+            { _x: 1, x: 1, _y: 1, y: 1, order: 2 },
+            { _x: 3, x: 3, _y: 3, y: 3, order: 1 }
 
           ]);
         });
@@ -236,16 +236,16 @@ describe("victory-util/data", () => {
           const returnDataX = Data.getData({ data, sortKey: "x" });
 
           expect(returnDataX).to.eql([
-            { _x: 1, x: 20, _y: 3, y: 20, eventKey: 0 },
-            { _x: 2, x: 10, _y: 2, y: 10, eventKey: 1 },
-            { _x: 3, x: 30, _y: 1, y: 30, eventKey: 2 }
+            { _x: 1, x: 20, _y: 3, y: 20 },
+            { _x: 2, x: 10, _y: 2, y: 10 },
+            { _x: 3, x: 30, _y: 1, y: 30 }
           ]);
 
           const returnDataY = Data.getData({ data, sortKey: "y" });
           expect(returnDataY).to.eql([
-            { _x: 3, x: 30, _y: 1, y: 30, eventKey: 0 },
-            { _x: 2, x: 10, _y: 2, y: 10, eventKey: 1 },
-            { _x: 1, x: 20, _y: 3, y: 20, eventKey: 2 }
+            { _x: 3, x: 30, _y: 1, y: 30 },
+            { _x: 2, x: 10, _y: 2, y: 10 },
+            { _x: 1, x: 20, _y: 3, y: 20 }
           ]);
         });
       });