From c6a79ec2c870ae5ed358adacae3fec85f01881d6 Mon Sep 17 00:00:00 2001
From: dmt0 <dmitry@plot.ly>
Date: Fri, 29 Jun 2018 16:04:01 -0400
Subject: [PATCH 1/2] Move group and filter transforms to main

---
 dev/App.js                                    |  66 +++++----
 dev/mocks.json                                |   2 +
 dev/percy/panelTest.json                      |  17 ++-
 .../containers/TransformAccordion.js          |  77 ++++++++---
 .../containers/TransformAccordionDev.js       | 130 ------------------
 src/components/fields/FilterOperation.js      |   9 +-
 src/default_panels/GraphTransformsPanel.js    |  56 ++++----
 src/default_panels/GraphTransformsPanelDev.js |  91 ------------
 8 files changed, 145 insertions(+), 303 deletions(-)
 delete mode 100644 src/components/containers/TransformAccordionDev.js
 delete mode 100644 src/default_panels/GraphTransformsPanelDev.js

diff --git a/dev/App.js b/dev/App.js
index 8c8e38b79..85da19506 100644
--- a/dev/App.js
+++ b/dev/App.js
@@ -7,7 +7,6 @@ import brace from 'brace'; // eslint-disable-line no-unused-vars
 import AceEditor from 'react-ace';
 import Select from 'react-select';
 import PlotlyEditor, {DefaultEditor, Panel} from '../src';
-import GraphTransformsPanel from '../src/default_panels/GraphTransformsPanelDev';
 import Inspector from 'react-inspector';
 import tips from './tips';
 import 'brace/mode/json';
@@ -43,38 +42,38 @@ const dataSourceOptions = Object.keys(dataSources).map(name => ({
 const config = {mapboxAccessToken: ACCESS_TOKENS.MAPBOX, editable: true};
 
 const traceTypesConfig = {
-    traces: _ => ([
-        {
-          value: 'scattergl',
-          icon: 'scatter',
-          label: _('Scatter'),
-        },
-        {
-          value: 'bar',
-          label: _('Bar'),
-        },
-        {
-          value: 'histogram',
-          label: _('Histogram'),
-        },
-        {
-          value: 'table',
-          label: _('Table'),
-        },
-        {
-          value: 'pie',
-          label: _('Pie'),
-        },
-        {
-          value: 'box',
-          label: _('Box'),
-        },
-        {
-          value: 'histogram2d',
-          label: _('Histogram 2D'),
-        },
-    ]),
-    complex: true
+  traces: _ => [
+    {
+      value: 'scattergl',
+      icon: 'scatter',
+      label: _('Scatter'),
+    },
+    {
+      value: 'bar',
+      label: _('Bar'),
+    },
+    {
+      value: 'histogram',
+      label: _('Histogram'),
+    },
+    {
+      value: 'table',
+      label: _('Table'),
+    },
+    {
+      value: 'pie',
+      label: _('Pie'),
+    },
+    {
+      value: 'box',
+      label: _('Box'),
+    },
+    {
+      value: 'histogram2d',
+      label: _('Histogram 2D'),
+    },
+  ],
+  complex: true,
 };
 
 class App extends Component {
@@ -160,7 +159,6 @@ class App extends Component {
           // makeDefaultTrace={() => ({type: 'scattergl', mode: 'markers'})}
         >
           <DefaultEditor>
-            <GraphTransformsPanel group="Dev" name="Transforms" />
             <Panel group="Dev" name="JSON">
               <div className="mocks">
                 <Select
diff --git a/dev/mocks.json b/dev/mocks.json
index fce795041..372cbc945 100644
--- a/dev/mocks.json
+++ b/dev/mocks.json
@@ -1,6 +1,8 @@
 [
   "/mocks/aggregate.json",
   "/percy/panelTest.json",
+  "/percy/histogram.json",
+  "/percy/histogram2d.json",
   "0.json",
   "1.json",
   "10.json",
diff --git a/dev/percy/panelTest.json b/dev/percy/panelTest.json
index ae6532910..c8fb46e36 100644
--- a/dev/percy/panelTest.json
+++ b/dev/percy/panelTest.json
@@ -101,7 +101,22 @@
         "color": "#1f77b4",
         "thickness": 2,
         "width": 4
-      }
+      },
+      "transforms": [
+        {
+          "type": "groupby",
+          "groupssrc": "ints"
+        },
+        {
+          "type": "filter",
+          "targetsrc": "ints",
+          "operation": "[]",
+          "value": [
+            "0",
+            ""
+          ]
+        }
+      ]
     }
   ],
   "layout": {
diff --git a/src/components/containers/TransformAccordion.js b/src/components/containers/TransformAccordion.js
index 71eb48dc8..15b53353c 100644
--- a/src/components/containers/TransformAccordion.js
+++ b/src/components/containers/TransformAccordion.js
@@ -14,6 +14,8 @@ class TransformAccordion extends Component {
       fullContainer,
       fullContainer: {transforms = []},
       localize: _,
+      container,
+      dataSourceOptions,
     } = this.context;
     const {children} = this.props;
 
@@ -23,7 +25,9 @@ class TransformAccordion extends Component {
       {label: _('Aggregate'), type: 'aggregate'},
     ];
 
-    if (['scatter', 'bar', 'scattergl'].indexOf(fullContainer.type) === -1) {
+    const transformableCharts = ['scatter', 'bar', 'scattergl'];
+
+    if (!transformableCharts.includes(fullContainer.type)) {
       return (
         <FoldEmpty
           icon={PlotScatterIcon}
@@ -32,6 +36,28 @@ class TransformAccordion extends Component {
       );
     }
 
+    const transformBy =
+      container.transforms &&
+      container.transforms.map(tr => {
+        let foldNameSuffix = '';
+        if (tr.groupssrc) {
+          const groupssrc =
+            dataSourceOptions &&
+            dataSourceOptions.find(d => d.value === tr.groupssrc);
+          foldNameSuffix = `: ${
+            groupssrc && groupssrc.label ? groupssrc.label : tr.groupssrc
+          }`;
+        } else if (tr.targetsrc) {
+          const targetsrc =
+            dataSourceOptions &&
+            dataSourceOptions.find(d => d.value === tr.targetsrc);
+          foldNameSuffix = `: ${
+            targetsrc && targetsrc.label ? targetsrc.label : tr.targetsrc
+          }`;
+        }
+        return foldNameSuffix;
+      });
+
     const filteredTransforms = transforms.filter(({type}) => Boolean(type));
     const content =
       filteredTransforms.length &&
@@ -39,32 +65,41 @@ class TransformAccordion extends Component {
         <TransformFold
           key={i}
           transformIndex={i}
-          name={transformTypes.filter(({type}) => type === tr.type)[0].label}
+          name={`${
+            transformTypes.filter(({type}) => type === tr.type)[0].label
+          }${transformBy[i]}`}
           canDelete={true}
         >
           {children}
         </TransformFold>
       ));
 
-    const handlers = transformTypes.map(({label, type}) => {
-      return {
-        label,
-        handler: context => {
-          const {fullContainer, updateContainer} = context;
-          if (updateContainer) {
-            const transformIndex = Array.isArray(fullContainer.transforms)
-              ? fullContainer.transforms.length
-              : 0;
-            const key = `transforms[${transformIndex}]`;
-            updateContainer({[key]: {type}});
-          }
-        },
-      };
-    });
-
     const addAction = {
-      label: _('Aggregation'),
-      handler: handlers[2].handler,
+      label: _('Transform'),
+      handler: transformTypes.map(({label, type}) => {
+        return {
+          label,
+          handler: context => {
+            const {fullContainer, updateContainer} = context;
+            if (updateContainer) {
+              const transformIndex = Array.isArray(fullContainer.transforms)
+                ? fullContainer.transforms.length
+                : 0;
+              const key = `transforms[${transformIndex}]`;
+
+              const payload = {type};
+              const firstDataSource = dataSourceOptions[0].value;
+              if (type === 'filter') {
+                payload.targetsrc = firstDataSource;
+              } else {
+                payload.groupssrc = firstDataSource;
+              }
+
+              updateContainer({[key]: payload});
+            }
+          },
+        };
+      }),
     };
 
     return (
@@ -78,6 +113,8 @@ class TransformAccordion extends Component {
 TransformAccordion.contextTypes = {
   fullContainer: PropTypes.object,
   localize: PropTypes.func,
+  container: PropTypes.object,
+  dataSourceOptions: PropTypes.array,
 };
 
 TransformAccordion.propTypes = {
diff --git a/src/components/containers/TransformAccordionDev.js b/src/components/containers/TransformAccordionDev.js
deleted file mode 100644
index 6915ad6db..000000000
--- a/src/components/containers/TransformAccordionDev.js
+++ /dev/null
@@ -1,130 +0,0 @@
-import PlotlyFold from './PlotlyFold';
-import PlotlyPanel from './PlotlyPanel';
-import PropTypes from 'prop-types';
-import React, {Component} from 'react';
-import {connectTransformToTrace} from 'lib';
-import FoldEmpty from './FoldEmpty';
-import {PlotScatterIcon} from 'plotly-icons';
-
-const TransformFold = connectTransformToTrace(PlotlyFold);
-
-class TransformAccordion extends Component {
-  render() {
-    const {
-      fullContainer,
-      fullContainer: {transforms = []},
-      localize: _,
-      container,
-      dataSourceOptions,
-    } = this.context;
-    const {children} = this.props;
-
-    const transformTypes = [
-      {label: _('Filter'), type: 'filter'},
-      {label: _('Split'), type: 'groupby'},
-      {label: _('Aggregate'), type: 'aggregate'},
-    ];
-
-    const transformableCharts = [
-      'scatter',
-      'bar',
-      'line',
-      'area',
-      'scattergl',
-      'box',
-      'histogram',
-      'histogram2d',
-      'histogram2dcontour',
-      'candlestick',
-      'ohlc',
-    ];
-
-    if (transformableCharts.indexOf(fullContainer.type) === -1) {
-      return (
-        <FoldEmpty
-          icon={PlotScatterIcon}
-          messagePrimary={_('No transforms available for this trace type')}
-        />
-      );
-    }
-
-    const transformBy =
-      container.transforms &&
-      container.transforms.map(tr => {
-        let retValue = '';
-        if (tr.groupssrc) {
-          retValue = `: ${
-            dataSourceOptions.find(d => d.value === tr.groupssrc).label
-          }`;
-        } else if (tr.targetsrc) {
-          retValue = `: ${
-            dataSourceOptions.find(d => d.value === tr.targetsrc).label
-          }`;
-        }
-        return retValue;
-      });
-
-    const filteredTransforms = transforms.filter(({type}) => Boolean(type));
-    const content =
-      filteredTransforms.length &&
-      filteredTransforms.map((tr, i) => (
-        <TransformFold
-          key={i}
-          transformIndex={i}
-          name={`${
-            transformTypes.filter(({type}) => type === tr.type)[0].label
-          }${transformBy[i]}`}
-          canDelete={true}
-        >
-          {children}
-        </TransformFold>
-      ));
-
-    const addAction = {
-      label: _('Transform'),
-      handler: transformTypes.map(({label, type}) => {
-        return {
-          label,
-          handler: context => {
-            const {fullContainer, updateContainer} = context;
-            if (updateContainer) {
-              const transformIndex = Array.isArray(fullContainer.transforms)
-                ? fullContainer.transforms.length
-                : 0;
-              const key = `transforms[${transformIndex}]`;
-
-              const payload = {type};
-              const firstDataSource = dataSourceOptions[0].value;
-              if (type === 'filter') {
-                payload.targetsrc = firstDataSource;
-              } else {
-                payload.groupssrc = firstDataSource;
-              }
-
-              updateContainer({[key]: payload});
-            }
-          },
-        };
-      }),
-    };
-
-    return (
-      <PlotlyPanel addAction={addAction}>
-        {content ? content : null}
-      </PlotlyPanel>
-    );
-  }
-}
-
-TransformAccordion.contextTypes = {
-  fullContainer: PropTypes.object,
-  localize: PropTypes.func,
-  container: PropTypes.object,
-  dataSourceOptions: PropTypes.array,
-};
-
-TransformAccordion.propTypes = {
-  children: PropTypes.node,
-};
-
-export default TransformAccordion;
diff --git a/src/components/fields/FilterOperation.js b/src/components/fields/FilterOperation.js
index dfae81d15..c6a304230 100644
--- a/src/components/fields/FilterOperation.js
+++ b/src/components/fields/FilterOperation.js
@@ -160,10 +160,11 @@ class UnconnectedFilterValue extends Component {
   }
 
   render() {
-    const {
-      localize: _,
-      container: {operation},
-    } = this.context;
+    const {localize: _, container} = this.context;
+
+    const operation =
+      container && container.operation ? container.operation : '=';
+
     const {fullValue, attr, defaultValue} = this.props;
     const op = findOperation(operation);
 
diff --git a/src/default_panels/GraphTransformsPanel.js b/src/default_panels/GraphTransformsPanel.js
index 904c5a51a..16072c92c 100644
--- a/src/default_panels/GraphTransformsPanel.js
+++ b/src/default_panels/GraphTransformsPanel.js
@@ -7,6 +7,8 @@ import {
   DataSelector,
   Dropdown,
   PlotlySection,
+  FilterOperation,
+  FilterValue,
 } from '../components';
 import {connectAggregationToTransform} from '../lib';
 
@@ -22,28 +24,30 @@ export class Aggregations extends Component {
       return null;
     }
 
-    return aggregations.map(({target}, i) => (
-      <AggregationSection show key={i} aggregationIndex={i}>
-        <Dropdown
-          attr="func"
-          label={target}
-          options={[
-            {label: _('Count'), value: 'count'},
-            {label: _('Sum'), value: 'sum'},
-            {label: _('Average'), value: 'avg'},
-            {label: _('Median'), value: 'median'},
-            {label: _('Mode'), value: 'mode'},
-            {label: _('RMS'), value: 'rms'},
-            {label: _('Standard Deviation'), value: 'stddev'},
-            {label: _('Min'), value: 'min'},
-            {label: _('Max'), value: 'max'},
-            {label: _('First'), value: 'first'},
-            {label: _('Last'), value: 'last'},
-          ]}
-          clearable={false}
-        />
-      </AggregationSection>
-    ));
+    return aggregations
+      .filter(aggr => aggr.target.match(/transforms\[\d*\]\./gi) === null)
+      .map(({target}, i) => (
+        <AggregationSection show key={i} aggregationIndex={i}>
+          <Dropdown
+            attr="func"
+            label={target}
+            options={[
+              {label: _('Count'), value: 'count'},
+              {label: _('Sum'), value: 'sum'},
+              {label: _('Average'), value: 'avg'},
+              {label: _('Median'), value: 'median'},
+              {label: _('Mode'), value: 'mode'},
+              {label: _('RMS'), value: 'rms'},
+              {label: _('Standard Deviation'), value: 'stddev'},
+              {label: _('Min'), value: 'min'},
+              {label: _('Max'), value: 'max'},
+              {label: _('First'), value: 'first'},
+              {label: _('Last'), value: 'last'},
+            ]}
+            clearable={false}
+          />
+        </AggregationSection>
+      ));
   }
 }
 
@@ -67,7 +71,13 @@ const GraphTransformsPanel = (props, {localize: _}) => {
 
         <DataSelector label={_('By')} attr="groups" />
 
-        <Aggregations />
+        <DataSelector label={_('Target')} attr="target" />
+        <FilterOperation label={_('Operator')} attr="operation" />
+        <FilterValue label={_('Value')} attr="value" />
+
+        <PlotlySection name={_('Aggregations')} attr="aggregations">
+          <Aggregations />
+        </PlotlySection>
       </TransformAccordion>
     </TraceAccordion>
   );
diff --git a/src/default_panels/GraphTransformsPanelDev.js b/src/default_panels/GraphTransformsPanelDev.js
deleted file mode 100644
index 39ff8b8a1..000000000
--- a/src/default_panels/GraphTransformsPanelDev.js
+++ /dev/null
@@ -1,91 +0,0 @@
-import React, {Component} from 'react';
-import PropTypes from 'prop-types';
-import {
-  Radio,
-  // TransformAccordion,
-  TraceAccordion,
-  DataSelector,
-  Dropdown,
-  PlotlySection,
-  FilterOperation,
-  FilterValue,
-} from '../components';
-import TransformAccordion from '../components/containers/TransformAccordionDev';
-import {connectAggregationToTransform} from '../lib';
-
-const AggregationSection = connectAggregationToTransform(PlotlySection);
-
-export class Aggregations extends Component {
-  render() {
-    const {
-      fullContainer: {aggregations = []},
-    } = this.context;
-    const {localize: _} = this.context;
-    if (aggregations.length === 0) {
-      return null;
-    }
-
-    return aggregations
-      .filter(aggr => aggr.target.match(/transforms\[\d*\]\./gi) === null)
-      .map(({target}, i) => (
-        <AggregationSection show key={i} aggregationIndex={i}>
-          <Dropdown
-            attr="func"
-            label={target}
-            options={[
-              {label: _('Count'), value: 'count'},
-              {label: _('Sum'), value: 'sum'},
-              {label: _('Average'), value: 'avg'},
-              {label: _('Median'), value: 'median'},
-              {label: _('Mode'), value: 'mode'},
-              {label: _('RMS'), value: 'rms'},
-              {label: _('Standard Deviation'), value: 'stddev'},
-              {label: _('Min'), value: 'min'},
-              {label: _('Max'), value: 'max'},
-              {label: _('First'), value: 'first'},
-              {label: _('Last'), value: 'last'},
-            ]}
-            clearable={false}
-          />
-        </AggregationSection>
-      ));
-  }
-}
-
-Aggregations.plotly_editor_traits = {no_visibility_forcing: true};
-Aggregations.contextTypes = {
-  fullContainer: PropTypes.object,
-  localize: PropTypes.func,
-};
-
-const GraphTransformsPanel = (props, {localize: _}) => {
-  return (
-    <TraceAccordion>
-      <TransformAccordion>
-        <Radio
-          attr="enabled"
-          options={[
-            {label: _('Enabled'), value: true},
-            {label: _('Disabled'), value: false},
-          ]}
-        />
-
-        <DataSelector label={_('By')} attr="groups" />
-
-        <DataSelector label={_('Target')} attr="target" />
-        <FilterOperation label={_('Operator')} attr="operation" />
-        <FilterValue label={_('Value')} attr="value" />
-
-        <PlotlySection name={_('Aggregations')} attr="aggregations">
-          <Aggregations />
-        </PlotlySection>
-      </TransformAccordion>
-    </TraceAccordion>
-  );
-};
-
-GraphTransformsPanel.contextTypes = {
-  localize: PropTypes.func,
-};
-
-export default GraphTransformsPanel;

From 97ea7b4d06abf8fcc840fe7ba1c39eee1d0d1f49 Mon Sep 17 00:00:00 2001
From: dmt0 <dmitry@plot.ly>
Date: Tue, 3 Jul 2018 15:48:49 -0400
Subject: [PATCH 2/2] percy improvements

---
 dev/percy/histogram.json      | 14 ++++----------
 dev/percy/histogram2d.json    |  8 ++++----
 dev/percy/panelTest.json      |  9 +++++++--
 src/__percy__/panels.percy.js | 22 ++++++++++++++++++----
 src/lib/test-utils.js         |  2 +-
 5 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/dev/percy/histogram.json b/dev/percy/histogram.json
index d8f399464..7f79fb40f 100644
--- a/dev/percy/histogram.json
+++ b/dev/percy/histogram.json
@@ -7,12 +7,9 @@
       "x": [
         1,
         2,
-        3,
-        4,
-        5,
-        6
+        3
       ],
-      "xsrc": "ints",
+      "xsrc": "x1",
       "autobinx": true,
       "xbins": {
         "start": -0.5,
@@ -21,13 +18,10 @@
       },
       "y": [
         2,
-        1,
         3,
-        5,
-        4,
-        6
+        4
       ],
-      "ysrc": "jagged ints",
+      "ysrc": "y1",
       "error_x": {
         "visible": true,
         "symmetric": true
diff --git a/dev/percy/histogram2d.json b/dev/percy/histogram2d.json
index 48056d0b6..85c37b028 100644
--- a/dev/percy/histogram2d.json
+++ b/dev/percy/histogram2d.json
@@ -9,13 +9,13 @@
         2,
         3
       ],
-      "xsrc": "ints",
+      "xsrc": "x1",
       "y": [
-        1,
         2,
-        3
+        3,
+        4
       ],
-      "ysrc": "ints",
+      "ysrc": "y1",
       "autobinx": false,
       "xbins": {
         "start": 0.5,
diff --git a/dev/percy/panelTest.json b/dev/percy/panelTest.json
index c8fb46e36..1496c3e4f 100644
--- a/dev/percy/panelTest.json
+++ b/dev/percy/panelTest.json
@@ -16,6 +16,7 @@
       "transforms": [
         {
           "type": "aggregate",
+          "enabled": true,
           "groups": [
             1,
             2,
@@ -105,15 +106,19 @@
       "transforms": [
         {
           "type": "groupby",
-          "groupssrc": "ints"
+          "groupssrc": "ints",
+          "groups": [],
+          "enabled": true
         },
         {
+          "enabled": true,
           "type": "filter",
+          "target": "x",
           "targetsrc": "ints",
           "operation": "[]",
           "value": [
             "0",
-            ""
+            "1"
           ]
         }
       ]
diff --git a/src/__percy__/panels.percy.js b/src/__percy__/panels.percy.js
index e19a8b657..9bd071ff6 100644
--- a/src/__percy__/panels.percy.js
+++ b/src/__percy__/panels.percy.js
@@ -1,5 +1,4 @@
-import React from 'react';
-import {TestEditor, plotly, setupGraphDiv} from 'lib/test-utils';
+import {TestEditor, setupGraphDiv, fixtures, plotly} from 'lib/test-utils';
 
 import {PanelMenuWrapper} from '../components';
 
@@ -19,10 +18,25 @@ const panelsToTest = {
   histogram2d: ['GraphCreatePanel', 'StyleTracesPanel'],
 };
 
-const panelFixture = (Panel, group, name, data) => {
+window.URL.createObjectURL = function() {
+  return null;
+};
+
+const panelFixture = (Panel, group, name, figure) => {
+  const gd = setupGraphDiv(figure);
+  gd._context = plotly.setPlotConfig();
+  gd._context.setBackground = () => {
+    return null;
+  };
+
   return (
     <div className="plotly_editor">
-      <TestEditor plotly={plotly} graphDiv={setupGraphDiv(data)}>
+      <TestEditor
+        plotly={plotly}
+        graphDiv={gd}
+        dataSources={fixtures.scatter().dataSources}
+        dataSourceOptions={fixtures.scatter().dataSourceOptions}
+      >
         <PanelMenuWrapper>
           <Panel group={group} name={name} />
         </PanelMenuWrapper>
diff --git a/src/lib/test-utils.js b/src/lib/test-utils.js
index c78081275..66a1c80f2 100644
--- a/src/lib/test-utils.js
+++ b/src/lib/test-utils.js
@@ -1,4 +1,4 @@
-import plotly from 'plotly.js/dist/plotly-basic';
+import plotly from 'plotly.js/dist/plotly-cartesian';
 import {extendDeep} from 'plotly.js/src/lib/extend';
 import EditorControls from '../EditorControls';
 import {configure, mount, shallow} from 'enzyme';