Refactor react components fixes #1988 (#1989)

* Move react help text to constants fixes #1988 * Move help from forms to constants * Rename handle change methods
mozilla · Nov 26, 2019 · d6f0baa · d6f0baa
1 parent 36e3ba9
commit d6f0baa
Show file tree

Hide file tree

Showing 8 changed files with 266 additions and 272 deletions.
diff --git a/app/experimenter/static/js/components/AddonForm.js b/app/experimenter/static/js/components/AddonForm.js
@@ -1,11 +1,15 @@
-import { List, Map } from "immutable";
 import PropTypes from "prop-types";
 import React from "react";
+import { List, Map } from "immutable";
 import { Row, Col } from "react-bootstrap";
 
 import BranchManager from "experimenter/components/BranchManager";
 import DesignInput from "experimenter/components/DesignInput";
 import GenericBranchFields from "experimenter/components/GenericBranchFields";
+import {
+  ADDON_EXPERIMENT_ID_HELP,
+  ADDON_RELEASE_URL_HELP,
+} from "experimenter/components/constants";
 
 export default class AddonForm extends React.PureComponent {
   static propTypes = {
@@ -32,22 +36,7 @@ export default class AddonForm extends React.PureComponent {
           }}
           value={this.props.data.get("addon_experiment_id")}
           error={this.props.errors.get("addon_experiment_id", "")}
-          helpContent={
-            <div>
-              <p>
-                Enter the <code>activeExperimentName</code> as it appears in the
-                add-on. It may appear in <code>manifest.json</code> as
-                <code> applications.gecko.id </code>
-                <a
-                  target="_blank"
-                  rel="noreferrer noopener"
-                  href="https://mana.mozilla.org/wiki/display/FIREFOX/Pref-Flip+and+Add-On+Experiments#Pref-FlipandAdd-OnExperiments-Add-ons"
-                >
-                  See here for more info.
-                </a>
-              </p>
-            </div>
-          }
+          helpContent={ADDON_EXPERIMENT_ID_HELP}
         />
 
         <DesignInput
@@ -58,23 +47,7 @@ export default class AddonForm extends React.PureComponent {
           }}
           value={this.props.data.get("addon_release_url")}
           error={this.props.errors.get("addon_release_url", "")}
-          helpContent={
-            <div>
-              <p>
-                Enter the URL where the release build of your add-on can be
-                found. This is often attached to a bugzilla ticket. This MUST BE
-                the release signed add-on (not the test add-on) that you want
-                deployed.&nbsp;
-                <a
-                  target="_blank"
-                  rel="noreferrer noopener"
-                  href="https://mana.mozilla.org/wiki/display/FIREFOX/Pref-Flip+and+Add-On+Experiments#Pref-FlipandAdd-OnExperiments-Add-ons"
-                >
-                  See here for more info.
-                </a>
-              </p>
-            </div>
-          }
+          helpContent={ADDON_RELEASE_URL_HELP}
         />
 
         <hr className="heavy-line my-5" />

diff --git a/app/experimenter/static/js/components/Branch.js b/app/experimenter/static/js/components/Branch.js
@@ -1,8 +1,10 @@
-import { boundClass } from "autobind-decorator";
-import { Map } from "immutable";
 import PropTypes from "prop-types";
 import React from "react";
 import { Button, Row, Col } from "react-bootstrap";
+import { Map } from "immutable";
+import { boundClass } from "autobind-decorator";
+
+import DesignInput from "experimenter/components/DesignInput";
 
 @boundClass
 class Branch extends React.PureComponent {
@@ -15,7 +17,7 @@ class Branch extends React.PureComponent {
     remove: PropTypes.func,
   };
 
-  handleChange(key, value) {
+  handleBranchFieldChange(key, value) {
     const { onChange, branch } = this.props;
     onChange(branch.set(key, value));
   }
@@ -28,6 +30,22 @@ class Branch extends React.PureComponent {
     return <h4>Branch {index}</h4>;
   }
 
+  renderField(name, label, value, error, help) {
+    return (
+      <DesignInput
+        label={label}
+        name={`variants[${this.props.index}][${name}]`}
+        id={`variants-${this.props.index}-${name}`}
+        value={value}
+        onChange={value => {
+          this.handleBranchFieldChange(name, value);
+        }}
+        error={error}
+        helpContent={help}
+      />
+    );
+  }
+
   renderRemoveButton() {
     const { branch, index } = this.props;
     if (branch.get("is_control")) {
@@ -59,10 +77,11 @@ class Branch extends React.PureComponent {
         </Row>
 
         <BranchFields
-          handleChange={this.handleChange}
-          index={this.props.index}
           branch={this.props.branch}
           errors={this.props.errors}
+          onChange={this.handleBranchFieldChange}
+          index={this.props.index}
+          renderField={this.renderField}
         />
 
         <hr className="heavy-line my-5" />

diff --git a/app/experimenter/static/js/components/BranchManager.js b/app/experimenter/static/js/components/BranchManager.js
@@ -16,11 +16,6 @@ class BranchManager extends React.PureComponent {
     handleErrorsChange: PropTypes.func,
   };
 
-  handleChange(index, value) {
-    const { branches, handleDataChange } = this.props;
-    handleDataChange("variants", branches.set(index, value));
-  }
-
   addBranch() {
     const {
       branches,
@@ -43,6 +38,11 @@ class BranchManager extends React.PureComponent {
     handleErrorsChange("variants", errors.delete(index));
   }
 
+  handleBranchChange(index, value) {
+    const { branches, handleDataChange } = this.props;
+    handleDataChange("variants", branches.set(index, value));
+  }
+
   renderBranch(branch, index) {
     const { branchFieldsComponent, errors } = this.props;
 
@@ -57,7 +57,7 @@ class BranchManager extends React.PureComponent {
         }}
         errors={errors.get(index, new Map())}
         onChange={value => {
-          this.handleChange(index, value);
+          this.handleBranchChange(index, value);
         }}
       />
     );

diff --git a/app/experimenter/static/js/components/GenericBranchFields.js b/app/experimenter/static/js/components/GenericBranchFields.js
@@ -1,95 +1,47 @@
-import { boundClass } from "autobind-decorator";
-import { Map } from "immutable";
 import PropTypes from "prop-types";
 import React from "react";
+import { Map } from "immutable";
+import { boundClass } from "autobind-decorator";
 
-import DesignInput from "experimenter/components/DesignInput";
+import {
+  BRANCH_RATIO_HELP,
+  BRANCH_NAME_HELP,
+  BRANCH_DESCRIPTION_HELP,
+} from "experimenter/components/constants";
 
 @boundClass
 class GenericBranchFields extends React.PureComponent {
   static propTypes = {
     branch: PropTypes.instanceOf(Map),
     errors: PropTypes.instanceOf(Map),
-    handleChange: PropTypes.func,
     index: PropTypes.number,
+    renderField: PropTypes.func,
   };
 
-  renderField(name, label) {
-    let helpContent;
-    switch (name) {
-      case "ratio":
-        helpContent = (
-          <div>
-            <p>
-              Choose the size of this branch represented as a whole number. The
-              size of all branches together must be equal to 100. It does not
-              have to be exact, so these sizes are simply a recommendation of
-              the relative distribution of the branches.
-            </p>
-            <p>
-              <strong>Example:</strong> 50
-            </p>
-          </div>
-        );
-        break;
-
-      case "name":
-        helpContent = (
-          <div>
-            <p>
-              The control group should represent the users receiving the
-              existing, unchanged version of what you're testing. For example,
-              if you're testing making a button larger to see if users click on
-              it more often, the control group would receive the existing button
-              size. You should name your control branch based on the experience
-              or functionality that group of users will be receiving. Don't name
-              it 'Control Group', we already know it's the control group!
-            </p>
-            <p>
-              <strong>Example:</strong> Normal Button Size
-            </p>
-          </div>
-        );
-        break;
-
-      case "description":
-        helpContent = (
-          <div>
-            <p>
-              Describe the experience or functionality the control group will
-              receive in more detail.
-            </p>
-            <p>
-              <strong>Example:</strong> The control group will receive the
-              existing 80px sign in button located at the top right of the
-              screen.
-            </p>
-          </div>
-        );
-        break;
-    }
-
-    return (
-      <DesignInput
-        label={label}
-        name={`variants[${this.props.index}][${name}]`}
-        id={`variants-${this.props.index}-${name}`}
-        value={this.props.branch.get(name)}
-        onChange={value => {
-          this.props.handleChange(name, value);
-        }}
-        error={this.props.errors.get(name)}
-        helpContent={helpContent}
-      />
-    );
-  }
-
   render() {
     return (
       <React.Fragment>
-        {this.renderField("ratio", "Branch Size")}
-        {this.renderField("name", "Name")}
-        {this.renderField("description", "Description")}
+        {this.props.renderField(
+          "ratio",
+          "Branch Size",
+          this.props.branch.get("ratio"),
+          this.props.errors.get("ratio"),
+          BRANCH_RATIO_HELP,
+        )}
+        {this.props.renderField(
+          "name",
+          "Name",
+          this.props.branch.get("name"),
+          this.props.errors.get("name"),
+          BRANCH_NAME_HELP,
+        )}
+        {this.props.renderField(
+          "description",
+          "Description",
+          this.props.branch.get("description"),
+          this.props.errors.get("description"),
+          BRANCH_DESCRIPTION_HELP,
+        )}
       </React.Fragment>
     );
   }

diff --git a/app/experimenter/static/js/components/GenericForm.js b/app/experimenter/static/js/components/GenericForm.js
@@ -1,10 +1,11 @@
-import { List, Map } from "immutable";
 import PropTypes from "prop-types";
 import React from "react";
+import { List, Map } from "immutable";
 
 import BranchManager from "experimenter/components/BranchManager";
 import DesignInput from "experimenter/components/DesignInput";
 import GenericBranchFields from "experimenter/components/GenericBranchFields";
+import { DESIGN_HELP } from "experimenter/components/constants";
 
 export default class GenericForm extends React.PureComponent {
   static propTypes = {
@@ -27,11 +28,7 @@ export default class GenericForm extends React.PureComponent {
           error={this.props.errors.get("design", "")}
           as="textarea"
           rows="10"
-          helpContent={
-            <div>
-              <p>Specify the design of the experiment.</p>
-            </div>
-          }
+          helpContent={DESIGN_HELP}
         />
 
         <hr className="heavy-line my-5" />