From 7aab8b0ae3d6816cf861c09193c9e2e166e2d18a Mon Sep 17 00:00:00 2001
From: Maxime Beauchemin <maximebeauchemin@gmail.com>
Date: Wed, 4 Jan 2017 15:46:52 -0800
Subject: [PATCH] Simplifying the Fields (Controls) interface (#1868)

* Simplifying the Field interface / logic

* Moving ControlLabelWithTooltip where it belongs

* Progress

* Rename FieldClass->FieldType
---
 .../explorev2/components/CheckboxField.jsx    |   6 +-
 .../explorev2/components/FieldSet.jsx         |  75 +++--------
 .../explorev2/components/SelectField.jsx      |  14 +-
 .../explorev2/components/TextAreaField.jsx    |   2 -
 .../explorev2/components/TextField.jsx        |   5 -
 .../assets/javascripts/explorev2/main.css     |   6 +
 .../javascripts/explorev2/stores/fields.js    | 122 ++++++++++++------
 7 files changed, 111 insertions(+), 119 deletions(-)

diff --git a/superset/assets/javascripts/explorev2/components/CheckboxField.jsx b/superset/assets/javascripts/explorev2/components/CheckboxField.jsx
index fb9ce69db9797..98a3013bb0d4e 100644
--- a/superset/assets/javascripts/explorev2/components/CheckboxField.jsx
+++ b/superset/assets/javascripts/explorev2/components/CheckboxField.jsx
@@ -1,6 +1,5 @@
 import React, { PropTypes } from 'react';
 import { Checkbox } from 'react-bootstrap';
-import ControlLabelWithTooltip from './ControlLabelWithTooltip';
 
 const propTypes = {
   name: PropTypes.string.isRequired,
@@ -24,12 +23,9 @@ export default class CheckboxField extends React.Component {
   render() {
     return (
       <Checkbox
-        inline
         checked={this.props.value}
         onChange={this.onToggle.bind(this)}
-      >
-        <ControlLabelWithTooltip label={this.props.label} description={this.props.description} />
-      </Checkbox>
+      />
     );
   }
 }
diff --git a/superset/assets/javascripts/explorev2/components/FieldSet.jsx b/superset/assets/javascripts/explorev2/components/FieldSet.jsx
index b155b6e8bd068..b2a11823aa2c2 100644
--- a/superset/assets/javascripts/explorev2/components/FieldSet.jsx
+++ b/superset/assets/javascripts/explorev2/components/FieldSet.jsx
@@ -3,7 +3,16 @@ import TextField from './TextField';
 import CheckboxField from './CheckboxField';
 import TextAreaField from './TextAreaField';
 import SelectField from './SelectField';
-import { fieldTypes } from '../stores/fields';
+
+import ControlLabelWithTooltip from './ControlLabelWithTooltip';
+
+const fieldMap = {
+  TextField,
+  CheckboxField,
+  TextAreaField,
+  SelectField,
+};
+const fieldTypes = Object.keys(fieldMap);
 
 const propTypes = {
   name: PropTypes.string.isRequired,
@@ -18,68 +27,22 @@ const propTypes = {
     PropTypes.string,
     PropTypes.number,
     PropTypes.bool,
-    PropTypes.array]).isRequired,
+    PropTypes.array]),
 };
 
 const defaultProps = {
-  choices: null,
-  description: null,
-  places: null,
-  validators: null,
   onChange: () => {},
 };
 
-export default class FieldSet extends React.Component {
-  renderCheckBoxField() {
-    return (
-      <CheckboxField
-        {...this.props}
-      />);
-  }
-
-  renderTextAreaField() {
-    return (
-      <TextAreaField
-        {...this.props}
-      />);
-  }
-
-  renderSelectField(selectProps) {
-    return (
-      <SelectField
-        {...this.props}
-        {...selectProps}
-      />);
-  }
-
-  renderTextField() {
-    return (
-      <TextField
-        {...this.props}
-      />);
-  }
-
+export default class FieldSet extends React.PureComponent {
   render() {
-    const type = this.props.type;
-    const selectProps = {
-      SelectCustomMultiField: { multi: true, freeForm: true },
-      SelectMultipleSortableField: { multi: true, freeForm: false },
-      SelectField: { multi: false, freeForm: false },
-      FreeFormSelectField: { multi: false, freeForm: true },
-    };
-    let field;
-
-    if (type === 'CheckboxField') {
-      field = this.renderCheckBoxField();
-    } else if (Object.keys(selectProps).includes(type)) {
-      field = this.renderSelectField(selectProps[type]);
-    } else if (['TextField', 'IntegerField'].includes(type)) {
-      field = this.renderTextField();
-    } else if (type === 'TextAreaField') {
-      field = this.renderTextAreaField();
-    }
-
-    return field;
+    const FieldType = fieldMap[this.props.type];
+    return (
+      <div>
+        <ControlLabelWithTooltip label={this.props.label} description={this.props.description} />
+        <FieldType {...this.props} />
+      </div>
+    );
   }
 }
 
diff --git a/superset/assets/javascripts/explorev2/components/SelectField.jsx b/superset/assets/javascripts/explorev2/components/SelectField.jsx
index b3514449ee563..f9686e06bd687 100644
--- a/superset/assets/javascripts/explorev2/components/SelectField.jsx
+++ b/superset/assets/javascripts/explorev2/components/SelectField.jsx
@@ -1,6 +1,4 @@
 import React, { PropTypes } from 'react';
-import ControlLabelWithTooltip from './ControlLabelWithTooltip';
-import { slugify } from '../../modules/utils';
 import Select, { Creatable } from 'react-select';
 
 
@@ -87,17 +85,7 @@ export default class SelectField extends React.Component {
     //  Tab, comma or Enter will trigger a new option created for FreeFormSelect
     const selectWrap = this.props.freeForm ?
       (<Creatable {...selectProps} />) : (<Select {...selectProps} />);
-    if (this.props.label) {
-      return (
-        <div id={`formControlsSelect-${slugify(this.props.label)}`}>
-          <ControlLabelWithTooltip
-            label={this.props.label}
-            description={this.props.description}
-          />
-          {selectWrap}
-        </div>
-      );
-    }
+
     return (
       <div>
         {selectWrap}
diff --git a/superset/assets/javascripts/explorev2/components/TextAreaField.jsx b/superset/assets/javascripts/explorev2/components/TextAreaField.jsx
index c0fc5b2b95912..bd80b13ea49cb 100644
--- a/superset/assets/javascripts/explorev2/components/TextAreaField.jsx
+++ b/superset/assets/javascripts/explorev2/components/TextAreaField.jsx
@@ -1,6 +1,5 @@
 import React, { PropTypes } from 'react';
 import { FormGroup, FormControl } from 'react-bootstrap';
-import ControlLabelWithTooltip from './ControlLabelWithTooltip';
 
 const propTypes = {
   name: PropTypes.string.isRequired,
@@ -24,7 +23,6 @@ export default class TextAreaField extends React.Component {
   render() {
     return (
       <FormGroup controlId="formControlsTextarea">
-        <ControlLabelWithTooltip label={this.props.label} description={this.props.description} />
         <FormControl
           componentClass="textarea"
           placeholder="textarea"
diff --git a/superset/assets/javascripts/explorev2/components/TextField.jsx b/superset/assets/javascripts/explorev2/components/TextField.jsx
index 68b54e1267015..3d61376113726 100644
--- a/superset/assets/javascripts/explorev2/components/TextField.jsx
+++ b/superset/assets/javascripts/explorev2/components/TextField.jsx
@@ -1,6 +1,5 @@
 import React, { PropTypes } from 'react';
 import { FormGroup, FormControl } from 'react-bootstrap';
-import ControlLabelWithTooltip from './ControlLabelWithTooltip';
 
 const propTypes = {
   name: PropTypes.string.isRequired,
@@ -24,10 +23,6 @@ export default class TextField extends React.Component {
   render() {
     return (
       <FormGroup controlId="formInlineName" bsSize="small">
-        <ControlLabelWithTooltip
-          label={this.props.label}
-          description={this.props.description}
-        />
         <FormControl
           type="text"
           placeholder=""
diff --git a/superset/assets/javascripts/explorev2/main.css b/superset/assets/javascripts/explorev2/main.css
index d59b4acb79bc1..145b70711c4e8 100644
--- a/superset/assets/javascripts/explorev2/main.css
+++ b/superset/assets/javascripts/explorev2/main.css
@@ -19,3 +19,9 @@
   padding: 0 0 0 .5em;
   font-size: 14px;
 }
+
+.checkbox {
+    float: left;
+    margin-top: 0px;
+    margin-right: 3px;
+}
diff --git a/superset/assets/javascripts/explorev2/stores/fields.js b/superset/assets/javascripts/explorev2/stores/fields.js
index 6c50a8b977459..ba47a1b1fa55b 100644
--- a/superset/assets/javascripts/explorev2/stores/fields.js
+++ b/superset/assets/javascripts/explorev2/stores/fields.js
@@ -1,16 +1,6 @@
 import { formatSelectOptionsForRange, formatSelectOptions } from '../../modules/utils';
 import visTypes from './visTypes';
 
-export const fieldTypes = [
-  'CheckboxField',
-  'FreeFormSelectField',
-  'IntegerField',
-  'SelectCustomMultiField',
-  'SelectField',
-  'SelectMultipleSortableField',
-  'TextAreaFeild',
-  'TextField',
-];
 const D3_FORMAT_DOCS = 'D3 format syntax: https://github.com/d3/d3-format';
 
 // input choices & options
@@ -55,7 +45,9 @@ export const fields = {
   },
 
   metrics: {
-    type: 'SelectMultipleSortableField',
+    type: 'SelectField',
+    multi: true,
+    freeForm: false,
     label: 'Metrics',
     choices: [],
     default: [],
@@ -63,7 +55,9 @@ export const fields = {
   },
 
   order_by_cols: {
-    type: 'SelectMultipleSortableField',
+    type: 'SelectField',
+    multi: true,
+    freeForm: false,
     label: 'Ordering',
     choices: [],
     default: [],
@@ -237,7 +231,9 @@ export const fields = {
   },
 
   groupby: {
-    type: 'SelectMultipleSortableField',
+    type: 'SelectField',
+    multi: true,
+    freeForm: false,
     label: 'Group by',
     choices: [],
     default: [],
@@ -245,7 +241,9 @@ export const fields = {
   },
 
   columns: {
-    type: 'SelectMultipleSortableField',
+    type: 'SelectField',
+    multi: true,
+    freeForm: false,
     label: 'Columns',
     choices: [],
     default: [],
@@ -253,7 +251,9 @@ export const fields = {
   },
 
   all_columns: {
-    type: 'SelectMultipleSortableField',
+    type: 'SelectField',
+    multi: true,
+    freeForm: false,
     label: 'Columns',
     choices: [],
     default: [],
@@ -277,7 +277,9 @@ export const fields = {
   },
 
   druid_time_origin: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Origin',
     choices: [
       ['', 'default'],
@@ -289,7 +291,9 @@ export const fields = {
   },
 
   bottom_margin: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Bottom Margin',
     choices: formatSelectOptions(['auto', 50, 75, 100, 125, 150, 200]),
     default: 'auto',
@@ -297,7 +301,9 @@ export const fields = {
   },
 
   granularity: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Time Granularity',
     default: 'one day',
     choices: formatSelectOptions([
@@ -338,7 +344,9 @@ export const fields = {
   },
 
   link_length: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Link Length',
     default: '200',
     choices: formatSelectOptions(['10', '25', '50', '75', '100', '150', '200', '250']),
@@ -346,7 +354,9 @@ export const fields = {
   },
 
   charge: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Charge',
     default: '-500',
     choices: formatSelectOptions([
@@ -389,7 +399,9 @@ export const fields = {
   },
 
   resample_rule: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Resample Rule',
     default: null,
     choices: formatSelectOptions(['', '1T', '1H', '1D', '7D', '1M', '1AS']),
@@ -397,7 +409,9 @@ export const fields = {
   },
 
   resample_how: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Resample How',
     default: null,
     choices: formatSelectOptions(['', 'mean', 'sum', 'median']),
@@ -405,7 +419,9 @@ export const fields = {
   },
 
   resample_fillmethod: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Resample Fill Method',
     default: null,
     choices: formatSelectOptions(['', 'ffill', 'bfill']),
@@ -413,7 +429,9 @@ export const fields = {
   },
 
   since: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Since',
     default: '7 days ago',
     choices: formatSelectOptions([
@@ -431,7 +449,9 @@ export const fields = {
   },
 
   until: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Until',
     default: 'now',
     choices: formatSelectOptions([
@@ -445,14 +465,18 @@ export const fields = {
   },
 
   max_bubble_size: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Max Bubble Size',
     default: '25',
     choices: formatSelectOptions(['5', '10', '15', '25', '50', '75', '100']),
   },
 
   whisker_options: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Whisker/outlier options',
     default: 'Tukey',
     description: 'Determines how whiskers and outliers are calculated.',
@@ -472,7 +496,9 @@ export const fields = {
   },
 
   number_format: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Number format',
     default: D3_TIME_FORMAT_OPTIONS[0],
     choices: D3_TIME_FORMAT_OPTIONS,
@@ -480,14 +506,18 @@ export const fields = {
   },
 
   row_limit: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Row limit',
     default: null,
     choices: formatSelectOptions(ROW_LIMIT_OPTIONS),
   },
 
   limit: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Series limit',
     choices: formatSelectOptions(SERIES_LIMITS),
     default: 50,
@@ -613,7 +643,9 @@ export const fields = {
   },
 
   table_timestamp_format: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Table Timestamp Format',
     default: 'smart_date',
     choices: TIME_STAMP_OPTIONS,
@@ -621,7 +653,9 @@ export const fields = {
   },
 
   series_height: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Series Height',
     default: '25',
     choices: formatSelectOptions(['10', '25', '40', '50', '75', '100', '150', '200']),
@@ -629,7 +663,9 @@ export const fields = {
   },
 
   page_length: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Page Length',
     default: 0,
     choices: formatSelectOptions([0, 10, 25, 40, 50, 75, 100, 150, 200]),
@@ -637,7 +673,9 @@ export const fields = {
   },
 
   x_axis_format: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'X axis format',
     default: 'smart_date',
     choices: TIME_STAMP_OPTIONS,
@@ -645,7 +683,9 @@ export const fields = {
   },
 
   y_axis_format: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Y axis format',
     default: '.3s',
     choices: D3_TIME_FORMAT_OPTIONS,
@@ -868,7 +908,9 @@ export const fields = {
   },
 
   mapbox_label: {
-    type: 'SelectMultipleSortableField',
+    type: 'SelectField',
+    multi: true,
+    freeForm: false,
     label: 'label',
     choices: [],
     default: [],
@@ -894,7 +936,9 @@ export const fields = {
   },
 
   clustering_radius: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'Clustering Radius',
     default: '60',
     choices: formatSelectOptions([
@@ -972,7 +1016,9 @@ export const fields = {
   },
 
   mapbox_color: {
-    type: 'FreeFormSelectField',
+    type: 'SelectField',
+    multi: false,
+    freeForm: true,
     label: 'RGB Color',
     default: 'rgb(0, 122, 135)',
     choices: [