From da2a3aa1f0cf4bc6b028cc856028acf14b823fe1 Mon Sep 17 00:00:00 2001
From: RachelElysia <71795832+RachelElysia@users.noreply.github.com>
Date: Wed, 1 Sep 2021 10:57:25 -0400
Subject: [PATCH] App Settings Page: New advance options UI (#1832)

---
 changes/1832-app-settings-ui                  |  1 +
 .../integration/all/app/settingsflow.spec.ts  | 10 +--
 .../admin/AppConfigForm/AppConfigForm.jsx     | 63 +++----------------
 .../AppConfigForm/AppConfigForm.tests.jsx     | 20 +-----
 .../forms/admin/AppConfigForm/_styles.scss    | 37 ++++-------
 5 files changed, 32 insertions(+), 99 deletions(-)
 create mode 100644 changes/1832-app-settings-ui

diff --git a/changes/1832-app-settings-ui b/changes/1832-app-settings-ui
new file mode 100644
index 000000000..70e3b1a43
--- /dev/null
+++ b/changes/1832-app-settings-ui
@@ -0,0 +1 @@
+Cleaner advanced options UI
\ No newline at end of file
diff --git a/cypress/integration/all/app/settingsflow.spec.ts b/cypress/integration/all/app/settingsflow.spec.ts
index 59d25106c..7d7a68026 100644
--- a/cypress/integration/all/app/settingsflow.spec.ts
+++ b/cypress/integration/all/app/settingsflow.spec.ts
@@ -66,8 +66,6 @@ describe("Settings flow", () => {
       .click()
       .type("rachelspassword");
 
-    cy.get("#advanced-options").click();
-
     cy.findByLabelText(/domain/i)
       .click()
       .type("http://www.fleetdm.com");
@@ -75,12 +73,16 @@ describe("Settings flow", () => {
     // can't grab button from the label because the button is a child element and doesn't have a for attribute
     // couldn't figure out how to write a for attribute on kolide button
     // Repeated Error Message: Timed out retrying after 4000ms: Found a label with the text of: /verify ssl certs/i, however no form control was found associated to that label. Make sure you're using the "for" attribute or "aria-labelledby" attribute correctly.
-    cy.get(".kolide-slider__dot").click({ multiple: true });
+    cy.findByLabelText(/verify ssl certs/i).check({ force: true });
+    cy.findByLabelText(/enable starttls/i).check({ force: true });
+    cy.findByLabelText(/^host expiry$/i).check({ force: true });
 
     cy.findByLabelText(/host expiry window/i)
       .click()
       .type("{selectall}{backspace}5");
 
+    cy.findByLabelText(/disable live queries/i).check({ force: true });
+
     // Update settings
     cy.findByRole("button", { name: /update settings/i }).click();
 
@@ -133,8 +135,6 @@ describe("Settings flow", () => {
       "rachelsusername"
     );
 
-    cy.get("#advanced-options").click();
-
     cy.findByLabelText(/host expiry window/i).should("have.value", "5");
 
     cy.getEmails().then((response) => {
diff --git a/frontend/components/forms/admin/AppConfigForm/AppConfigForm.jsx b/frontend/components/forms/admin/AppConfigForm/AppConfigForm.jsx
index 83c6c7e7e..5c1d197e2 100644
--- a/frontend/components/forms/admin/AppConfigForm/AppConfigForm.jsx
+++ b/frontend/components/forms/admin/AppConfigForm/AppConfigForm.jsx
@@ -60,25 +60,6 @@ const formFields = [
   "agent_options",
   "enable_analytics",
 ];
-const Header = ({ showAdvancedOptions }) => {
-  const CaratIcon = (
-    <Button
-      className={`button button--unstyled ${
-        showAdvancedOptions ? "upcarat" : "downcarat"
-      }`}
-    />
-  );
-
-  return (
-    <span>
-      Advanced Options {CaratIcon}{" "}
-      <small>Most users do not need to modify these options.</small>
-    </span>
-  );
-};
-
-Header.propTypes = { showAdvancedOptions: PropTypes.bool.isRequired };
-
 class AppConfigForm extends Component {
   static propTypes = {
     fields: PropTypes.shape({
@@ -120,21 +101,10 @@ class AppConfigForm extends Component {
     super(props);
 
     this.state = {
-      showAdvancedOptions: false,
       showUsageStatsPreviewModal: false,
     };
   }
 
-  onToggleAdvancedOptions = (evt) => {
-    evt.preventDefault();
-
-    const { showAdvancedOptions } = this.state;
-
-    this.setState({ showAdvancedOptions: !showAdvancedOptions });
-
-    return false;
-  };
-
   toggleUsageStatsPreviewModal = () => {
     const { showUsageStatsPreviewModal } = this.state;
     this.setState({
@@ -144,29 +114,26 @@ class AppConfigForm extends Component {
 
   renderAdvancedOptions = () => {
     const { fields } = this.props;
-    const { showAdvancedOptions } = this.state;
-
-    if (!showAdvancedOptions) {
-      return false;
-    }
 
     return (
       <div>
+        <p className={`${baseClass}__section-description`}>
+          Most users do not need to modify these options.
+        </p>
         <div className={`${baseClass}__inputs`}>
           <div className={`${baseClass}__smtp-section`}>
             <InputField {...fields.domain} label="Domain" />
-            <Slider {...fields.verify_ssl_certs} label="Verify SSL Certs?" />
-            <Slider {...fields.enable_start_tls} label="Enable STARTTLS?" />
-            <Slider {...fields.host_expiry_enabled} label="Host Expiry" />
+            <Checkbox {...fields.verify_ssl_certs}>Verify SSL certs</Checkbox>
+            <Checkbox {...fields.enable_start_tls}>Enable STARTTLS</Checkbox>
+            <Checkbox {...fields.host_expiry_enabled}>Host expiry</Checkbox>
             <InputField
               {...fields.host_expiry_window}
               disabled={!fields.host_expiry_enabled.value}
               label="Host Expiry Window"
             />
-            <Slider
-              {...fields.live_query_disabled}
-              label="Disable Live Queries?"
-            />
+            <Checkbox {...fields.live_query_disabled}>
+              Disable live queries
+            </Checkbox>
           </div>
         </div>
 
@@ -248,13 +215,11 @@ class AppConfigForm extends Component {
   render() {
     const { fields, handleSubmit, smtpConfigured, enrollSecret } = this.props;
     const {
-      onToggleAdvancedOptions,
       renderAdvancedOptions,
       renderSmtpSection,
       toggleUsageStatsPreviewModal,
       renderUsageStatsPreviewModal,
     } = this;
-    const { showAdvancedOptions } = this.state;
 
     return (
       <>
@@ -554,15 +519,7 @@ class AppConfigForm extends Component {
           </div>
 
           <div className={`${baseClass}__section`}>
-            <h2>
-              <a
-                id="advanced-options"
-                onClick={onToggleAdvancedOptions}
-                className={`${baseClass}__show-options`}
-              >
-                <Header showAdvancedOptions={showAdvancedOptions} />
-              </a>
-            </h2>
+            <h2>Advanced options</h2>
             {renderAdvancedOptions()}
           </div>
           <Button type="submit" variant="brand">
diff --git a/frontend/components/forms/admin/AppConfigForm/AppConfigForm.tests.jsx b/frontend/components/forms/admin/AppConfigForm/AppConfigForm.tests.jsx
index c020324ca..c8e7afb1c 100644
--- a/frontend/components/forms/admin/AppConfigForm/AppConfigForm.tests.jsx
+++ b/frontend/components/forms/admin/AppConfigForm/AppConfigForm.tests.jsx
@@ -7,7 +7,7 @@ import { itBehavesLikeAFormInputElement } from "test/helpers";
 
 describe("AppConfigForm - form", () => {
   const defaultProps = {
-    formData: { org_name: "Kolide" },
+    formData: { org_name: "Fleet" },
     handleSubmit: noop,
     smtpConfigured: false,
     enrollSecret: [
@@ -84,21 +84,7 @@ describe("AppConfigForm - form", () => {
     });
   });
 
-  it("does not render advanced options by default", () => {
-    expect(form.find({ name: "domain" }).length).toEqual(0);
-    expect(form.find("Slider").length).toEqual(0);
-  });
-
   describe("Advanced options", () => {
-    beforeAll(() => {
-      form.find(".app-config-form__show-options").simulate("click");
-    });
-
-    it('renders advanced options when "Advanced Options" is clicked', () => {
-      expect(form.find({ name: "domain" }).hostNodes().length).toEqual(1);
-      expect(form.find("Slider").length).toEqual(4);
-    });
-
     it("disables host expiry window by default", () => {
       const InputField = form.find({ name: "host_expiry_window" });
       const inputElement = InputField.find("input");
@@ -109,11 +95,11 @@ describe("AppConfigForm - form", () => {
     it("enables host expiry window", () => {
       form
         .find({ name: "host_expiry_enabled" })
-        .find("button")
+        .find("Checkbox")
         .simulate("click");
       const InputField = form.find({ name: "host_expiry_window" });
       const inputElement = InputField.find("input");
-      expect(inputElement.hasClass("input-field--disabled")).toBe(false);
+      expect(inputElement.hasClass("input-field--disabled")).toBe(true);
     });
 
     it("renders live query disabled input", () => {
diff --git a/frontend/components/forms/admin/AppConfigForm/_styles.scss b/frontend/components/forms/admin/AppConfigForm/_styles.scss
index d9ae23da9..b05214ea1 100644
--- a/frontend/components/forms/admin/AppConfigForm/_styles.scss
+++ b/frontend/components/forms/admin/AppConfigForm/_styles.scss
@@ -14,15 +14,14 @@
       margin: 0;
       font-size: $x-small;
     }
-
   }
-  
+
   &__learn-more {
     color: $core-vibrant-blue;
     font-size: $x-small;
     font-weight: $bold;
     text-decoration: none;
-    
+
     &--inline {
       margin-left: $pad-medium;
     }
@@ -33,7 +32,6 @@
     }
   }
 
-
   &__enroll-secret-label {
     font-size: 15px;
 
@@ -196,25 +194,14 @@
   &__smtp-section {
     @include clearfix;
 
-    .slide-wrapper {
-      .slider-option {
-        font-size: 18px;
-        font-weight: $regular;
-        text-align: left;
-        vertical-align: text-bottom;
-
-        &--off {
-          color: $core-fleet-blue;
-        }
-
-        &--on {
-          color: $core-vibrant-blue;
-        }
-      }
-
-      .button {
-        margin: 0 10px;
-      }
+    .fleet-checkbox__tick {
+      position: relative;
+      left: 150px;
+      top: 12px;
+    }
+    .fleet-checkbox__label {
+      position: relative;
+      left: -48px;
     }
   }
 
@@ -224,7 +211,9 @@
   }
 
   &__yaml {
-    width: calc(100% - 2px); // because the outline extended beyond the sticky page description
+    width: calc(
+      100% - 2px
+    ); // because the outline extended beyond the sticky page description
   }
 
   &__usage-stats-preview-modal {