Simplifying the Fields (Controls) interface (#1868)

* Simplifying the Field interface / logic * Moving ControlLabelWithTooltip where it belongs * Progress * Rename FieldClass->FieldType
apache · Jan 4, 2017 · 7aab8b0 · 7aab8b0
1 parent 861a3bd
commit 7aab8b0
Show file tree

Hide file tree

Showing 7 changed files with 111 additions and 119 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -19,3 +19,9 @@
   padding: 0 0 0 .5em;
   font-size: 14px;
 }
+
+.checkbox {
+    float: left;
+    margin-top: 0px;
+    margin-right: 3px;
+}