[explore-v2] control panel fixes (#1529)

* make fieldset conditions more clear * make label required * use render* pattern * use slugify util for turning labels into ids * use field rather than html * don't need panel-title class here
apache · Nov 3, 2016 · 4156ad5 · 4156ad5
1 parent ae46561
commit 4156ad5
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 18 deletions.
diff --git a/caravel/assets/javascripts/components/InfoTooltipWithTrigger.jsx b/caravel/assets/javascripts/components/InfoTooltipWithTrigger.jsx
@@ -1,5 +1,6 @@
 import React, { PropTypes } from 'react';
 import { Tooltip, OverlayTrigger } from 'react-bootstrap';
+import { slugify } from '../modules/utils';
 
 const propTypes = {
   label: PropTypes.string.isRequired,
@@ -10,7 +11,7 @@ export default function InfoTooltipWithTrigger({ label, tooltip }) {
   return (
     <OverlayTrigger
       placement="right"
-      overlay={<Tooltip id={`${label}-tooltip`}>{tooltip}</Tooltip>}
+      overlay={<Tooltip id={`${slugify(label)}-tooltip`}>{tooltip}</Tooltip>}
     >
       <i className="fa fa-question-circle-o" />
     </OverlayTrigger>

diff --git a/caravel/assets/javascripts/explorev2/components/ControlLabelWithTooltip.jsx b/caravel/assets/javascripts/explorev2/components/ControlLabelWithTooltip.jsx
@@ -3,12 +3,11 @@ import { ControlLabel } from 'react-bootstrap';
 import InfoTooltipWithTrigger from '../../components/InfoTooltipWithTrigger';
 
 const propTypes = {
-  label: PropTypes.string,
+  label: PropTypes.string.isRequired,
   description: PropTypes.string,
 };
 
 const defaultProps = {
-  label: null,
   description: null,
 };
 

diff --git a/caravel/assets/javascripts/explorev2/components/ControlPanelSection.jsx b/caravel/assets/javascripts/explorev2/components/ControlPanelSection.jsx
@@ -16,12 +16,12 @@ const defaultProps = {
 };
 
 export default class ControlPanelSection extends React.Component {
-  header() {
+  renderHeader() {
     const { label, tooltip } = this.props;
     let header;
     if (label) {
       header = (
-        <div className="panel-title">
+        <div>
           {label} &nbsp;
           {tooltip && <InfoTooltipWithTrigger label={label} tooltip={tooltip} />}
         </div>
@@ -32,7 +32,7 @@ export default class ControlPanelSection extends React.Component {
 
   render() {
     return (
-      <Panel header={this.header()}>
+      <Panel header={this.renderHeader()}>
         {this.props.children}
       </Panel>
     );

diff --git a/caravel/assets/javascripts/explorev2/components/FieldSet.jsx b/caravel/assets/javascripts/explorev2/components/FieldSet.jsx
@@ -40,22 +40,25 @@ export default class FieldSet extends React.Component {
 
   render() {
     const type = this.props.type;
-    let html;
+    const selectTypes = [
+      'SelectField',
+      'SelectCustomMultiField',
+      'SelectMultipleSortableField',
+      'FreeFormSelectField',
+    ];
+    let field;
 
     if (type === 'CheckboxField') {
-      html = this.renderCheckBoxField();
-    } else if (type === 'SelectField' ||
-               type === 'SelectCustomMultiField' ||
-               type === 'SelectMultipleSortableField' ||
-               type === 'FreeFormSelectField') {
-      html = this.renderSelectField();
-    } else if (type === 'TextField' || type === 'IntegerField') {
-      html = this.renderTextField();
+      field = this.renderCheckBoxField();
+    } else if (selectTypes.includes(type)) {
+      field = this.renderSelectField();
+    } else if (['TextField', 'IntegerField'].includes(type)) {
+      field = this.renderTextField();
     } else if (type === 'TextAreaField') {
-      this.renderTextAreaField();
+      field = this.renderTextAreaField();
     }
 
-    return html;
+    return field;
   }
 }
 

diff --git a/caravel/assets/javascripts/explorev2/components/SelectField.jsx b/caravel/assets/javascripts/explorev2/components/SelectField.jsx
@@ -1,6 +1,7 @@
 import React, { PropTypes } from 'react';
 import { FormGroup, FormControl } from 'react-bootstrap';
 import ControlLabelWithTooltip from './ControlLabelWithTooltip';
+import { slugify } from '../../modules/utils';
 
 const propTypes = {
   label: PropTypes.string,
@@ -14,7 +15,7 @@ const defaultProps = {
 
 export default function SelectField({ label, description }) {
   return (
-    <FormGroup controlId={`formControlsSelect-${label}`}>
+    <FormGroup controlId={`formControlsSelect-${slugify(label)}`}>
       <ControlLabelWithTooltip label={label} description={description} />
       <FormControl componentClass="select" placeholder="select">
         <option value="select">select</option>

diff --git a/caravel/assets/javascripts/modules/utils.js b/caravel/assets/javascripts/modules/utils.js
@@ -142,3 +142,13 @@ export function formatSelectOptions(options) {
      [opt, opt]
   );
 }
+
+export function slugify(string) {
+  // slugify('My Neat Label! '); returns 'my-neat-label'
+  return string
+          .toString()
+          .toLowerCase()
+          .trim()
+          .replace(/[\s\W-]+/g, '-') // replace spaces, non-word chars, w/ a single dash (-)
+          .replace(/-$/, ''); // remove last floating dash
+}