From 54bf64ada47fdac1443c8d6bedb1142c176089d9 Mon Sep 17 00:00:00 2001
From: Jacob Shandling <61553566+jacobshandling@users.noreply.github.com>
Date: Fri, 16 Dec 2022 10:32:10 -0800
Subject: [PATCH] UI: Fix line height issue on error in registration flow via
 replacing InputFieldWithIcon with InputField  (#8819)

* Replace InputFieldWithIcon with InputField on setup page

* same refactor on OrgDetails and FleetDetails

* Fix admin setup page spacing issue

* change file

* Remove redundant registration flow placeholders per style guide - info already in labels

* Update reg flow tests getByPlaceHolder ->  getByLabel

* update e2e tests with getByPlaceHolderText -> getByLabelText

Co-authored-by: Jacob Shandling <jacob@fleetdm.com>
---
 ...eight-change-on-error-in-registration-page |  1 +
 cypress/integration/all/setup/setup.spec.ts   | 10 ++++----
 .../AdminDetails/AdminDetails.jsx             | 20 ++++++---------
 .../AdminDetails/AdminDetails.tests.jsx       | 25 ++++++-------------
 .../FleetDetails/FleetDetails.jsx             |  6 ++---
 .../OrgDetails/OrgDetails.jsx                 | 10 ++++----
 .../forms/RegistrationForm/_styles.scss       |  2 +-
 .../forms/fields/InputField/_styles.scss      |  1 +
 8 files changed, 32 insertions(+), 43 deletions(-)
 create mode 100644 changes/fix-label-height-change-on-error-in-registration-page

diff --git a/changes/fix-label-height-change-on-error-in-registration-page b/changes/fix-label-height-change-on-error-in-registration-page
new file mode 100644
index 0000000000..5dd59e7bf0
--- /dev/null
+++ b/changes/fix-label-height-change-on-error-in-registration-page
@@ -0,0 +1 @@
+- Fix an issue where the height of the label for some input fields changed when an error message is displayed
diff --git a/cypress/integration/all/setup/setup.spec.ts b/cypress/integration/all/setup/setup.spec.ts
index 842626ab6d..924fdedf8f 100644
--- a/cypress/integration/all/setup/setup.spec.ts
+++ b/cypress/integration/all/setup/setup.spec.ts
@@ -4,22 +4,22 @@ const { GOOD_PASSWORD } = CONSTANTS;
 
 const fillOutForm = () => {
   // Page 1
-  cy.findByPlaceholderText(/full name/i).type("Test name");
+  cy.findByLabelText(/full name/i).type("Test name");
 
-  cy.findByPlaceholderText(/email/i).type("test@example.com");
+  cy.findByLabelText(/email/i).type("test@example.com");
 
-  cy.findByPlaceholderText(/^password/i)
+  cy.findByLabelText(/^password/i)
     .first()
     .type(GOOD_PASSWORD);
 
-  cy.findByPlaceholderText(/confirm password/i)
+  cy.findByLabelText(/confirm password/i)
     .last()
     .type(GOOD_PASSWORD);
 
   cy.contains("button:enabled", /next/i).click();
 
   // Page 2
-  cy.findByPlaceholderText(/organization name/i).type("Fleet Test");
+  cy.findByLabelText(/organization name/i).type("Fleet Test");
 
   cy.contains("button:enabled", /next/i).click();
 
diff --git a/frontend/components/forms/RegistrationForm/AdminDetails/AdminDetails.jsx b/frontend/components/forms/RegistrationForm/AdminDetails/AdminDetails.jsx
index 92dcc42f3a..b83595035b 100644
--- a/frontend/components/forms/RegistrationForm/AdminDetails/AdminDetails.jsx
+++ b/frontend/components/forms/RegistrationForm/AdminDetails/AdminDetails.jsx
@@ -4,7 +4,7 @@ import PropTypes from "prop-types";
 import Form from "components/forms/Form";
 import formFieldInterface from "interfaces/form_field";
 import Button from "components/buttons/Button";
-import InputFieldWithIcon from "components/forms/fields/InputFieldWithIcon";
+import InputField from "components/forms/fields/InputField";
 import helpers from "./helpers";
 
 const formFields = ["name", "password", "password_confirmation", "email"];
@@ -44,9 +44,9 @@ class AdminDetails extends Component {
     return (
       <form onSubmit={handleSubmit} className={className} autoComplete="off">
         <div className="registration-fields">
-          <InputFieldWithIcon
+          <InputField
             {...fields.name}
-            placeholder="Full name"
+            label="Full name"
             tabIndex={tabIndex}
             autofocus={currentPage}
             ref={(input) => {
@@ -56,25 +56,21 @@ class AdminDetails extends Component {
               maxLength: "80",
             }}
           />
-          <InputFieldWithIcon
-            {...fields.email}
-            placeholder="Email"
-            tabIndex={tabIndex}
-          />
-          <InputFieldWithIcon
+          <InputField {...fields.email} label="Email" tabIndex={tabIndex} />
+          <InputField
             {...fields.password}
-            placeholder="Password"
+            label="Password"
             type="password"
             tabIndex={tabIndex}
             hint={[
               "Must include 12 characters, at least 1 number (e.g. 0 - 9), and at least 1 symbol (e.g. &*#)",
             ]}
           />
-          <InputFieldWithIcon
+          <InputField
             {...fields.password_confirmation}
-            placeholder="Confirm password"
             type="password"
             tabIndex={tabIndex}
+            label="Confirm password"
           />
         </div>
         <Button
diff --git a/frontend/components/forms/RegistrationForm/AdminDetails/AdminDetails.tests.jsx b/frontend/components/forms/RegistrationForm/AdminDetails/AdminDetails.tests.jsx
index c12fa4d575..4ede3f37dd 100644
--- a/frontend/components/forms/RegistrationForm/AdminDetails/AdminDetails.tests.jsx
+++ b/frontend/components/forms/RegistrationForm/AdminDetails/AdminDetails.tests.jsx
@@ -10,8 +10,8 @@ describe("AdminDetails - form", () => {
   it("renders", () => {
     render(<AdminDetails handleSubmit={onSubmitSpy} />);
 
-    expect(screen.getByPlaceholderText("Password")).toBeInTheDocument();
-    expect(screen.getByPlaceholderText("Confirm password")).toBeInTheDocument();
+    expect(screen.getByLabelText("Password")).toBeInTheDocument();
+    expect(screen.getByLabelText("Confirm password")).toBeInTheDocument();
     expect(
       screen.getByRole("textbox", { name: "Full name" })
     ).toBeInTheDocument();
@@ -55,11 +55,8 @@ describe("AdminDetails - form", () => {
       <AdminDetails handleSubmit={onSubmitSpy} currentPage />
     );
 
-    await user.type(screen.getByPlaceholderText("Password"), "p@ssw0rd");
-    await user.type(
-      screen.getByPlaceholderText("Confirm password"),
-      "password123"
-    );
+    await user.type(screen.getByLabelText("Password"), "p@ssw0rd");
+    await user.type(screen.getByLabelText("Confirm password"), "password123");
     await user.click(screen.getByRole("button", { name: "Next" }));
     // then
     expect(onSubmitSpy).not.toHaveBeenCalled();
@@ -73,11 +70,8 @@ describe("AdminDetails - form", () => {
       <AdminDetails handleSubmit={onSubmitSpy} currentPage />
     );
 
-    await user.type(screen.getByPlaceholderText("Password"), "passw0rd");
-    await user.type(
-      screen.getByPlaceholderText("Confirm password"),
-      "passw0rd"
-    );
+    await user.type(screen.getByLabelText("Password"), "passw0rd");
+    await user.type(screen.getByLabelText("Confirm password"), "passw0rd");
     await user.click(screen.getByRole("button", { name: "Next" }));
     // then
     expect(onSubmitSpy).not.toHaveBeenCalled();
@@ -95,11 +89,8 @@ describe("AdminDetails - form", () => {
       screen.getByRole("textbox", { name: "Email" }),
       "hi@gnar.dog"
     );
-    await user.type(screen.getByPlaceholderText("Password"), "password123#");
-    await user.type(
-      screen.getByPlaceholderText("Confirm password"),
-      "password123#"
-    );
+    await user.type(screen.getByLabelText("Password"), "password123#");
+    await user.type(screen.getByLabelText("Confirm password"), "password123#");
     await user.type(
       screen.getByRole("textbox", { name: "Full name" }),
       "Gnar Dog"
diff --git a/frontend/components/forms/RegistrationForm/FleetDetails/FleetDetails.jsx b/frontend/components/forms/RegistrationForm/FleetDetails/FleetDetails.jsx
index 208bb16064..e9912e80e2 100644
--- a/frontend/components/forms/RegistrationForm/FleetDetails/FleetDetails.jsx
+++ b/frontend/components/forms/RegistrationForm/FleetDetails/FleetDetails.jsx
@@ -5,7 +5,7 @@ import Form from "components/forms/Form";
 import formFieldInterface from "interfaces/form_field";
 import Button from "components/buttons/Button";
 import helpers from "components/forms/RegistrationForm/FleetDetails/helpers";
-import InputFieldWithIcon from "components/forms/fields/InputFieldWithIcon";
+import InputField from "components/forms/fields/InputField";
 
 const formFields = ["server_url"];
 const { validate } = helpers;
@@ -41,9 +41,9 @@ class FleetDetails extends Component {
     return (
       <form onSubmit={handleSubmit} className={className} autoComplete="off">
         <div className="registration-fields">
-          <InputFieldWithIcon
+          <InputField
             {...fields.server_url}
-            placeholder="Fleet web address"
+            label="Fleet web address"
             tabIndex={tabIndex}
             hint={[
               "Don’t include ",
diff --git a/frontend/components/forms/RegistrationForm/OrgDetails/OrgDetails.jsx b/frontend/components/forms/RegistrationForm/OrgDetails/OrgDetails.jsx
index dbeee1e11e..810fe7dbed 100644
--- a/frontend/components/forms/RegistrationForm/OrgDetails/OrgDetails.jsx
+++ b/frontend/components/forms/RegistrationForm/OrgDetails/OrgDetails.jsx
@@ -5,7 +5,7 @@ import Form from "components/forms/Form";
 import formFieldInterface from "interfaces/form_field";
 import Button from "components/buttons/Button";
 import helpers from "components/forms/RegistrationForm/OrgDetails/helpers";
-import InputFieldWithIcon from "components/forms/fields/InputFieldWithIcon";
+import InputField from "components/forms/fields/InputField";
 
 const formFields = ["org_name", "org_logo_url"];
 const { validate } = helpers;
@@ -42,17 +42,17 @@ class OrgDetails extends Component {
     return (
       <form onSubmit={handleSubmit} className={className} autoComplete="off">
         <div className="registration-fields">
-          <InputFieldWithIcon
+          <InputField
             {...fields.org_name}
-            placeholder="Organization name"
+            label="Organization name"
             tabIndex={tabIndex}
             ref={(input) => {
               this.firstInput = input;
             }}
           />
-          <InputFieldWithIcon
+          <InputField
             {...fields.org_logo_url}
-            placeholder="Organization logo URL (optional)"
+            label="Organization logo URL (optional)"
             tabIndex={tabIndex}
             hint="Personalize Fleet with your brand.  For best results, use a square image at least 150px wide, like https://fleetdm.com/logo.png."
           />
diff --git a/frontend/components/forms/RegistrationForm/_styles.scss b/frontend/components/forms/RegistrationForm/_styles.scss
index e10cf2e070..9da41c1d6e 100644
--- a/frontend/components/forms/RegistrationForm/_styles.scss
+++ b/frontend/components/forms/RegistrationForm/_styles.scss
@@ -33,7 +33,7 @@
 
     &--admin {
       left: 0;
-      top: unquote("max(56%, 480px)");
+      top: unquote("max(56%, 560px)");
       margin: auto;
     }
 
diff --git a/frontend/components/forms/fields/InputField/_styles.scss b/frontend/components/forms/fields/InputField/_styles.scss
index e07edbc4d7..697d9e2fcf 100644
--- a/frontend/components/forms/fields/InputField/_styles.scss
+++ b/frontend/components/forms/fields/InputField/_styles.scss
@@ -10,6 +10,7 @@
   box-sizing: border-box;
   height: 40px;
   transition: border-color 100ms;
+  width: 100%;
 
   &::placeholder {
     color: $ui-fleet-black-50;