From 5f98f39c85ef767b5c9b8a8bb655397e2d7fd14f Mon Sep 17 00:00:00 2001
From: Maxime Beauchemin <maximebeauchemin@gmail.com>
Date: Fri, 19 May 2017 09:52:26 -0700
Subject: [PATCH 1/3] [explore] viz type selector as modal

---
 .../explore/components/Control.jsx            |  2 +
 superset/assets/javascripts/explore/main.css  | 36 +++++++
 .../javascripts/explore/stores/controls.jsx   |  9 +-
 .../javascripts/explore/stores/visTypes.js    |  2 +-
 .../components/controls/VizTypeControl.jsx    | 95 +++++++++++++++++++
 superset/assets/package.json                  |  8 +-
 .../components/VizTypeControl_spec.jsx        | 37 ++++++++
 7 files changed, 177 insertions(+), 12 deletions(-)
 create mode 100644 superset/assets/javascripts/explorev2/components/controls/VizTypeControl.jsx
 create mode 100644 superset/assets/spec/javascripts/explorev2/components/VizTypeControl_spec.jsx

diff --git a/superset/assets/javascripts/explore/components/Control.jsx b/superset/assets/javascripts/explore/components/Control.jsx
index cd9e732dfd998..ec7d1169ac009 100644
--- a/superset/assets/javascripts/explore/components/Control.jsx
+++ b/superset/assets/javascripts/explore/components/Control.jsx
@@ -7,6 +7,7 @@ import HiddenControl from './controls/HiddenControl';
 import SelectControl from './controls/SelectControl';
 import TextAreaControl from './controls/TextAreaControl';
 import TextControl from './controls/TextControl';
+import VizTypeControl from './controls/VizTypeControl';
 
 const controlMap = {
   CheckboxControl,
@@ -15,6 +16,7 @@ const controlMap = {
   SelectControl,
   TextAreaControl,
   TextControl,
+  VizTypeControl,
 };
 const controlTypes = Object.keys(controlMap);
 
diff --git a/superset/assets/javascripts/explore/main.css b/superset/assets/javascripts/explore/main.css
index 403221cb3de6b..05805a65a465a 100644
--- a/superset/assets/javascripts/explore/main.css
+++ b/superset/assets/javascripts/explore/main.css
@@ -34,3 +34,39 @@
 .background-transparent {
   background-color: transparent !important;
 }
+.viztype-selector-container {
+    float: left;
+    border: 1px solid #ccc;
+    cursor: pointer;
+    position:relative;
+    margin: 5px;
+    box-shadow: 0 2px 4px 0 rgba(0, 0, 0, 0.2), 0 3px 8px 0 rgba(0, 0, 0, 0.19);
+}
+.viztype-selector-container:hover {
+  border: 1px solid black;
+}
+.viztype-label {
+  text-align: center;
+  position: absolute;
+  left: 0;
+  top: 70;
+  width: 100%;
+}
+
+.viztype-label span {
+  background-color:rgba(0, 0, 0, 0.5);
+  color: white;
+  font-size: 12px;
+  display:inline-block;
+}
+.l-m-5 {
+  margin-left: 5px !important;
+}
+.l-m-2 {
+  margin-left: 1px !important;
+}
+.viztype-selector-container.selected {
+  border: 1px solid #00A699;
+  cursor: not-allowed !important;
+  box-shadow: none;
+}
diff --git a/superset/assets/javascripts/explore/stores/controls.jsx b/superset/assets/javascripts/explore/stores/controls.jsx
index 693201d5c4914..9a96bbbb43d35 100644
--- a/superset/assets/javascripts/explore/stores/controls.jsx
+++ b/superset/assets/javascripts/explore/stores/controls.jsx
@@ -1,6 +1,5 @@
 import React from 'react';
 import { formatSelectOptionsForRange, formatSelectOptions } from '../../modules/utils';
-import visTypes from './visTypes';
 import * as v from '../validators';
 
 const D3_FORMAT_DOCS = 'D3 format syntax: https://github.com/d3/d3-format';
@@ -48,15 +47,9 @@ export const controls = {
   },
 
   viz_type: {
-    type: 'SelectControl',
+    type: 'VizTypeControl',
     label: 'Visualization Type',
-    clearable: false,
     default: 'table',
-    choices: Object.keys(visTypes).map(vt => [
-      vt,
-      visTypes[vt].label,
-      `/static/assets/images/viz_thumbnails/${vt}.png`,
-    ]),
     description: 'The type of visualization to display',
   },
 
diff --git a/superset/assets/javascripts/explore/stores/visTypes.js b/superset/assets/javascripts/explore/stores/visTypes.js
index 83a5afb3d8ccb..8e57633c89126 100644
--- a/superset/assets/javascripts/explore/stores/visTypes.js
+++ b/superset/assets/javascripts/explore/stores/visTypes.js
@@ -141,7 +141,7 @@ const visTypes = {
   },
 
   dual_line: {
-    label: 'Time Series - Dual Axis Line Chart',
+    label: 'Dual Axis Line Chart',
     requiresTime: true,
     controlPanelSections: [
       {
diff --git a/superset/assets/javascripts/explorev2/components/controls/VizTypeControl.jsx b/superset/assets/javascripts/explorev2/components/controls/VizTypeControl.jsx
new file mode 100644
index 0000000000000..cf1e360cc1445
--- /dev/null
+++ b/superset/assets/javascripts/explorev2/components/controls/VizTypeControl.jsx
@@ -0,0 +1,95 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import { Button, Label, FormControl, Modal } from 'react-bootstrap';
+import visTypes from '../../stores/visTypes';
+
+const IMG_SIZE = 150;
+
+const propTypes = {
+  description: PropTypes.string,
+  label: PropTypes.string,
+  name: PropTypes.string.isRequired,
+  onChange: PropTypes.func,
+  value: PropTypes.string.isRequired,
+};
+
+const defaultProps = {
+  onChange: () => {},
+};
+
+export default class VizTypeControl extends React.PureComponent {
+  constructor(props) {
+    super(props);
+    this.state = {
+      showModal: false,
+      filter: '',
+    };
+    this.toggleModal = this.toggleModal.bind(this);
+    this.changeSearch = this.changeSearch.bind(this);
+  }
+  onChange(vizType) {
+    this.props.onChange(vizType);
+    this.setState({ showModal: false });
+  }
+  toggleModal() {
+    this.setState({ showModal: !this.state.showModal });
+  }
+  changeSearch(event) {
+    this.setState({ filter: event.target.value });
+  }
+  render() {
+    const filter = this.state.filter;
+    return (
+      <div>
+        <Label>{visTypes[this.props.value].label}</Label>
+        <Button onClick={this.toggleModal} bsSize="sm" className="l-m-5">
+          <i className="fa fa-line-chart l-m-2" />
+          <i className="fa fa-area-chart l-m-2" />
+          <i className="fa fa-bar-chart l-m-2" />
+          <i className="fa fa-pie-chart l-m-2" />
+        </Button>
+        <Modal show={this.state.showModal} onHide={this.toggleModal} bsSize="lg">
+          <Modal.Header closeButton>
+            <Modal.Title>Select a visualization type</Modal.Title>
+          </Modal.Header>
+          <Modal.Body>
+            <div>
+              <FormControl
+                id="formControlsText"
+                type="text"
+                bsSize="sm"
+                value={this.state.filter}
+                placeholder="Search / Filter"
+                onChange={this.changeSearch}
+              />
+            </div>
+            <div className="clearfix">
+              {Object.keys(visTypes)
+                .filter(vt => (
+                  filter.length === 0 || visTypes[vt].label.toLowerCase().includes(filter)))
+                .map(vt => (
+                  <div
+                    className={`viztype-selector-container ${vt === this.props.value ? 'selected' : ''}`}
+                    onClick={this.onChange.bind(this, vt)}
+                    key={`viztype-img-${vt}`}
+                  >
+                    <img
+                      alt={`viz-type-${vt}`}
+                      width={IMG_SIZE}
+                      height={IMG_SIZE}
+                      className={`viztype-selector ${this.props.value === vt ? 'selected' : ''}`}
+                      src={`/static/assets/images/viz_thumbnails/${vt}.png`}
+                    />
+                    <div className="viztype-label">
+                      <Label>{visTypes[vt].label}</Label>
+                    </div>
+                  </div>))}
+            </div>
+          </Modal.Body>
+        </Modal>
+      </div>);
+  }
+}
+
+VizTypeControl.propTypes = propTypes;
+VizTypeControl.defaultProps = defaultProps;
diff --git a/superset/assets/package.json b/superset/assets/package.json
index 17e2510ed1316..8630df9127f6c 100644
--- a/superset/assets/package.json
+++ b/superset/assets/package.json
@@ -9,7 +9,7 @@
   },
   "scripts": {
     "test": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/browser.js --recursive spec/**/*_spec.*",
-    "cover": "babel-node ./node_modules/.bin/istanbul cover _mocha -- --require spec/helpers/browser.js --recursive spec/**/*_spec.*",
+    "cover": "babel-node node_modules/.bin/babel-istanbul cover _mocha -- --require spec/helpers/browser.js --recursive spec/**/*_spec.*",
     "dev": "NODE_ENV=dev webpack --watch --colors --progress --debug --output-pathinfo --devtool inline-source-map",
     "prod": "NODE_ENV=production node --max_old_space_size=4096 ./node_modules/webpack/bin/webpack.js -p --colors --progress",
     "build": "NODE_ENV=production webpack --colors --progress",
@@ -59,6 +59,7 @@
     "immutability-helper": "^2.0.0",
     "immutable": "^3.8.1",
     "jquery": "^3.2.1",
+    "jsdom": "9.12.0",
     "lodash.throttle": "^4.1.1",
     "mapbox-gl": "^0.26.0",
     "moment": "^2.14.1",
@@ -98,9 +99,10 @@
   "devDependencies": {
     "babel-cli": "^6.14.0",
     "babel-core": "^6.10.4",
+    "babel-istanbul": "^0.12.2",
     "babel-loader": "^6.2.4",
     "babel-plugin-css-modules-transform": "^1.1.0",
-    "babel-polyfill": "^6.14.0",
+    "babel-polyfill": "^6.23.0",
     "babel-preset-airbnb": "^2.1.1",
     "babel-preset-es2015": "^6.14.0",
     "babel-preset-react": "^6.11.1",
@@ -119,7 +121,7 @@
     "ignore-styles": "^5.0.1",
     "imports-loader": "^0.7.1",
     "istanbul": "^1.0.0-alpha",
-    "jsdom": "^9.12.0",
+    "jsdom": "9.12.0",
     "json-loader": "^0.5.4",
     "less": "^2.6.1",
     "less-loader": "^4.0.3",
diff --git a/superset/assets/spec/javascripts/explorev2/components/VizTypeControl_spec.jsx b/superset/assets/spec/javascripts/explorev2/components/VizTypeControl_spec.jsx
new file mode 100644
index 0000000000000..c7f7203790e93
--- /dev/null
+++ b/superset/assets/spec/javascripts/explorev2/components/VizTypeControl_spec.jsx
@@ -0,0 +1,37 @@
+import React from 'react';
+import sinon from 'sinon';
+import { expect } from 'chai';
+import { describe, it, beforeEach } from 'mocha';
+import { shallow } from 'enzyme';
+import { Modal } from 'react-bootstrap';
+import VizTypeControl from '../../../../javascripts/explorev2/components/controls/VizTypeControl';
+
+const defaultProps = {
+  name: 'viz_type',
+  label: 'Visualization Type',
+  value: 'table',
+  onChange: sinon.spy(),
+};
+
+describe('VizTypeControl', () => {
+  let wrapper;
+
+  beforeEach(() => {
+    wrapper = shallow(<VizTypeControl {...defaultProps} />);
+  });
+
+  it('renders a Modal', () => {
+    expect(wrapper.find(Modal)).to.have.lengthOf(1);
+  });
+
+  it('calls onChange when toggled', () => {
+    const select = wrapper.find('.viztype-selector-container').first();
+    select.simulate('click');
+    expect(defaultProps.onChange.called).to.equal(true);
+  });
+  it('filters images based on text input', () => {
+    expect(wrapper.find('img').length).to.be.above(20);
+    wrapper.setState({ filter: 'time' });
+    expect(wrapper.find('img').length).to.be.below(10);
+  });
+});

From 021dc8010e9baba1ca7663b7f63818d487f82746 Mon Sep 17 00:00:00 2001
From: Maxime Beauchemin <maximebeauchemin@gmail.com>
Date: Tue, 23 May 2017 18:18:06 -0700
Subject: [PATCH 2/3] Addressing comments

---
 .../components/controls/VizTypeControl.jsx    | 77 +++++++++++--------
 superset/assets/javascripts/explore/main.css  | 45 +++++------
 .../components/VizTypeControl_spec.jsx        |  2 +-
 3 files changed, 65 insertions(+), 59 deletions(-)
 rename superset/assets/javascripts/{explorev2 => explore}/components/controls/VizTypeControl.jsx (52%)

diff --git a/superset/assets/javascripts/explorev2/components/controls/VizTypeControl.jsx b/superset/assets/javascripts/explore/components/controls/VizTypeControl.jsx
similarity index 52%
rename from superset/assets/javascripts/explorev2/components/controls/VizTypeControl.jsx
rename to superset/assets/javascripts/explore/components/controls/VizTypeControl.jsx
index cf1e360cc1445..807aeac335bff 100644
--- a/superset/assets/javascripts/explorev2/components/controls/VizTypeControl.jsx
+++ b/superset/assets/javascripts/explore/components/controls/VizTypeControl.jsx
@@ -1,9 +1,8 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import { Button, Label, FormControl, Modal } from 'react-bootstrap';
+import { Label, Row, Col, FormControl, Modal } from 'react-bootstrap';
 import visTypes from '../../stores/visTypes';
-
-const IMG_SIZE = 150;
+import ControlHeader from '../ControlHeader';
 
 const propTypes = {
   description: PropTypes.string,
@@ -37,17 +36,52 @@ export default class VizTypeControl extends React.PureComponent {
   changeSearch(event) {
     this.setState({ filter: event.target.value });
   }
+  renderVizType(vizType) {
+    const vt = vizType;
+    return (
+      <div
+        className={`viztype-selector-container ${vt === this.props.value ? 'selected' : ''}`}
+        onClick={this.onChange.bind(this, vt)}
+      >
+        <img
+          alt={`viz-type-${vt}`}
+          width="100%"
+          className={`viztype-selector ${this.props.value === vt ? 'selected' : ''}`}
+          src={`/static/assets/images/viz_thumbnails/${vt}.png`}
+        />
+        <div className="viztype-label">
+          <strong>{visTypes[vt].label}</strong>
+        </div>
+      </div>);
+  }
   render() {
     const filter = this.state.filter;
+    const filteredVizTypes = Object.keys(visTypes)
+      .filter(vt => filter.length === 0 || visTypes[vt].label.toLowerCase().includes(filter));
+
+    const imgPerRow = 4;
+    const rows = [];
+    for (let i = 0; i <= filteredVizTypes.length; i += imgPerRow) {
+      rows.push(
+        <Row>
+          {filteredVizTypes.slice(i, i + imgPerRow).map(vt => (
+            <Col md={3} key={`grid-col-${vt}`}>
+              {this.renderVizType(vt)}
+            </Col>
+          ))}
+        </Row>);
+    }
     return (
       <div>
-        <Label>{visTypes[this.props.value].label}</Label>
-        <Button onClick={this.toggleModal} bsSize="sm" className="l-m-5">
-          <i className="fa fa-line-chart l-m-2" />
-          <i className="fa fa-area-chart l-m-2" />
-          <i className="fa fa-bar-chart l-m-2" />
-          <i className="fa fa-pie-chart l-m-2" />
-        </Button>
+        <ControlHeader
+          {...this.props}
+          rightNode={
+            <a onClick={this.toggleModal}>edit</a>
+          }
+        />
+        <Label onClick={this.toggleModal} style={{ cursor: 'pointer' }}>
+          {visTypes[this.props.value].label}
+        </Label>
         <Modal show={this.state.showModal} onHide={this.toggleModal} bsSize="lg">
           <Modal.Header closeButton>
             <Modal.Title>Select a visualization type</Modal.Title>
@@ -63,28 +97,7 @@ export default class VizTypeControl extends React.PureComponent {
                 onChange={this.changeSearch}
               />
             </div>
-            <div className="clearfix">
-              {Object.keys(visTypes)
-                .filter(vt => (
-                  filter.length === 0 || visTypes[vt].label.toLowerCase().includes(filter)))
-                .map(vt => (
-                  <div
-                    className={`viztype-selector-container ${vt === this.props.value ? 'selected' : ''}`}
-                    onClick={this.onChange.bind(this, vt)}
-                    key={`viztype-img-${vt}`}
-                  >
-                    <img
-                      alt={`viz-type-${vt}`}
-                      width={IMG_SIZE}
-                      height={IMG_SIZE}
-                      className={`viztype-selector ${this.props.value === vt ? 'selected' : ''}`}
-                      src={`/static/assets/images/viz_thumbnails/${vt}.png`}
-                    />
-                    <div className="viztype-label">
-                      <Label>{visTypes[vt].label}</Label>
-                    </div>
-                  </div>))}
-            </div>
+            {rows}
           </Modal.Body>
         </Modal>
       </div>);
diff --git a/superset/assets/javascripts/explore/main.css b/superset/assets/javascripts/explore/main.css
index 05805a65a465a..51eb621bbad43 100644
--- a/superset/assets/javascripts/explore/main.css
+++ b/superset/assets/javascripts/explore/main.css
@@ -34,39 +34,32 @@
 .background-transparent {
   background-color: transparent !important;
 }
-.viztype-selector-container {
-    float: left;
-    border: 1px solid #ccc;
-    cursor: pointer;
-    position:relative;
-    margin: 5px;
-    box-shadow: 0 2px 4px 0 rgba(0, 0, 0, 0.2), 0 3px 8px 0 rgba(0, 0, 0, 0.19);
-}
-.viztype-selector-container:hover {
-  border: 1px solid black;
+
+a {
+  cursor: pointer;
 }
+
 .viztype-label {
   text-align: center;
-  position: absolute;
-  left: 0;
-  top: 70;
-  width: 100%;
 }
 
-.viztype-label span {
-  background-color:rgba(0, 0, 0, 0.5);
-  color: white;
-  font-size: 12px;
-  display:inline-block;
+.viztype-selector-container {
+  cursor: pointer;
+  margin-top: 10px;
+  margin-bottom: 10px;
+  padding: 5px;
+  border: 1px solid #aaa;
 }
-.l-m-5 {
-  margin-left: 5px !important;
+
+.viztype-selector-container:hover {
+  border: 1px solid #000;
 }
-.l-m-2 {
-  margin-left: 1px !important;
+
+.viztype-selector-container.selected {
+  cursor: not-allowed;
+  opacity: 0.5;
 }
 .viztype-selector-container.selected {
-  border: 1px solid #00A699;
-  cursor: not-allowed !important;
-  box-shadow: none;
+  cursor: not-allowed;
+  border: 1px solid #aaa;
 }
diff --git a/superset/assets/spec/javascripts/explorev2/components/VizTypeControl_spec.jsx b/superset/assets/spec/javascripts/explorev2/components/VizTypeControl_spec.jsx
index c7f7203790e93..ba0ba5814630f 100644
--- a/superset/assets/spec/javascripts/explorev2/components/VizTypeControl_spec.jsx
+++ b/superset/assets/spec/javascripts/explorev2/components/VizTypeControl_spec.jsx
@@ -4,7 +4,7 @@ import { expect } from 'chai';
 import { describe, it, beforeEach } from 'mocha';
 import { shallow } from 'enzyme';
 import { Modal } from 'react-bootstrap';
-import VizTypeControl from '../../../../javascripts/explorev2/components/controls/VizTypeControl';
+import VizTypeControl from '../../../../javascripts/explore/components/controls/VizTypeControl';
 
 const defaultProps = {
   name: 'viz_type',

From 90f4b751dd41209b5eecde3b5a7d10d258ddcd53 Mon Sep 17 00:00:00 2001
From: Maxime Beauchemin <maximebeauchemin@gmail.com>
Date: Tue, 30 May 2017 09:11:01 -0700
Subject: [PATCH 3/3] Adressing comments

---
 superset/assets/javascripts/explore/main.css           | 4 ----
 superset/assets/stylesheets/less/cosmo/bootswatch.less | 5 +++++
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/superset/assets/javascripts/explore/main.css b/superset/assets/javascripts/explore/main.css
index 51eb621bbad43..ab04eaa45688a 100644
--- a/superset/assets/javascripts/explore/main.css
+++ b/superset/assets/javascripts/explore/main.css
@@ -35,10 +35,6 @@
   background-color: transparent !important;
 }
 
-a {
-  cursor: pointer;
-}
-
 .viztype-label {
   text-align: center;
 }
diff --git a/superset/assets/stylesheets/less/cosmo/bootswatch.less b/superset/assets/stylesheets/less/cosmo/bootswatch.less
index 7cb1eb4641b67..402dd5dcdb24a 100644
--- a/superset/assets/stylesheets/less/cosmo/bootswatch.less
+++ b/superset/assets/stylesheets/less/cosmo/bootswatch.less
@@ -422,3 +422,8 @@ hr {
   }
 }
 .space-loop(6);
+
+a {
+  cursor: pointer;
+}
+