Merge pull request #418 from NYPL/SCC-4389/my-account-2.0-qa

SCC-4389: My Account 2.0 QA round 1 and VQA/a11y round 2
NYPL · Dec 16, 2024 · 11aceac · 11aceac
2 parents 9f1e092 + 0cc96cb
commit 11aceac
Show file tree

Hide file tree

Showing 9 changed files with 227 additions and 100 deletions.
diff --git a/src/components/MyAccount/Settings/AddButton.tsx b/src/components/MyAccount/Settings/AddButton.tsx
@@ -4,18 +4,20 @@ import { forwardRef } from "react"
 type AddButtonProps = {
   inputType?: string
   label: string
+  isDisabled?: boolean
   onClick: () => void
 }
 
 const AddButton = forwardRef<HTMLButtonElement, AddButtonProps>(
-  ({ inputType, label, onClick }, ref) => {
+  ({ inputType, label, isDisabled, onClick }, ref) => {
     return (
       <Button
         ref={ref}
         id={inputType ? `add-${inputType}-button` : "add-button"}
         buttonType="text"
         onClick={onClick}
         size="large"
+        isDisabled={isDisabled}
         sx={{
           justifyContent: "flex-start",
           width: { base: "100%", md: "300px" },

diff --git a/src/components/MyAccount/Settings/EditButton.tsx b/src/components/MyAccount/Settings/EditButton.tsx
@@ -5,13 +5,15 @@ type EditButtonProps = {
   buttonId: string
   buttonLabel: string
   onClick: () => void
+  isDisabled?: boolean
 }
 
 const EditButton = forwardRef<HTMLButtonElement, EditButtonProps>(
-  ({ buttonLabel, buttonId, onClick }, ref) => {
+  ({ buttonLabel, buttonId, isDisabled, onClick }, ref) => {
     return (
       <Button
         ref={ref}
+        isDisabled={isDisabled}
         id={buttonId}
         aria-label={buttonLabel}
         buttonType="text"

diff --git a/src/components/MyAccount/Settings/EmailForm.test.tsx b/src/components/MyAccount/Settings/EmailForm.test.tsx
@@ -67,7 +67,9 @@ describe("email form", () => {
     fireEvent.click(screen.getByRole("button", { name: /edit/i }))
 
     await waitFor(() =>
-      expect(screen.getByLabelText("Update email address 2")).toHaveFocus()
+      expect(
+        screen.getByLabelText("Update primary email address")
+      ).toHaveFocus()
     )
 
     fireEvent.click(screen.getByRole("button", { name: /cancel/i }))
@@ -82,7 +84,7 @@ describe("email form", () => {
 
     await waitFor(() => expect(screen.getAllByRole("textbox")[2]).toHaveFocus())
 
-    fireEvent.click(screen.getAllByLabelText("Remove email")[1])
+    fireEvent.click(screen.getByLabelText("Remove email 2"))
 
     await waitFor(() => expect(screen.getAllByRole("textbox")[1]).toHaveFocus())
   })
@@ -121,7 +123,7 @@ describe("email form", () => {
 
     fireEvent.click(screen.getByRole("button", { name: /edit/i }))
 
-    fireEvent.click(screen.getByLabelText("Remove email"))
+    fireEvent.click(screen.getByLabelText("Remove email 2"))
 
     expect(
       screen.queryByDisplayValue("[email protected]")

diff --git a/src/components/MyAccount/Settings/NotificationForm.test.tsx b/src/components/MyAccount/Settings/NotificationForm.test.tsx
@@ -1,7 +1,10 @@
 import { render, screen, fireEvent, waitFor } from "@testing-library/react"
 import { filteredPickupLocations } from "../../../../__test__/fixtures/processedMyAccountData"
 import { PatronDataProvider } from "../../../context/PatronDataContext"
-import { processedPatron } from "../../../../__test__/fixtures/processedMyAccountData"
+import {
+  processedPatron,
+  emptyPatron,
+} from "../../../../__test__/fixtures/processedMyAccountData"
 import { pickupLocations } from "../../../../__test__/fixtures/rawSierraAccountData"
 import SettingsSelectForm from "./SettingsSelectForm"
 import type { Patron } from "../../../types/myAccountTypes"
@@ -34,33 +37,79 @@ describe("notification preference form", () => {
     (pref) => pref.code === processedPatron.notificationPreference
   )?.name
 
-  const component = (
-    <PatronDataProvider
-      testSpy={accountFetchSpy}
-      value={{
-        patron: processedPatron,
-        pickupLocations: filteredPickupLocations,
-      }}
-    >
-      <SettingsSelectForm
-        patronData={processedPatron}
-        settingsState={mockSettingsState}
-        pickupLocations={pickupLocations}
-        type="notification"
-      />
-    </PatronDataProvider>
-  )
+  const processedPatronWithNone = {
+    notificationPreference: "-",
+    username: "pastadisciple",
+    name: "Strega Nonna",
+    barcode: "23333121538324",
+    formattedBarcode: "2 3333 12153 8324",
+    expirationDate: "March 28, 2025",
+    emails: ["[email protected]", "[email protected]"],
+    phones: [
+      {
+        number: "123-456-7890",
+        type: "t",
+      },
+    ],
+    homeLibrary: { code: "sn   ", name: "SNFL (formerly Mid-Manhattan)" },
+    id: 6742743,
+  } as Patron
+
+  const processedPatronWithNothing = {
+    notificationPreference: "-",
+    username: "pastadisciple",
+    name: "Strega Nonna",
+    barcode: "23333121538324",
+    formattedBarcode: "2 3333 12153 8324",
+    expirationDate: "March 28, 2025",
+    emails: [],
+    phones: [],
+    homeLibrary: { code: "sn   ", name: "SNFL (formerly Mid-Manhattan)" },
+    id: 6742743,
+  } as Patron
+
+  const processedPatronWithNoPhone = {
+    notificationPreference: "-",
+    username: "pastadisciple",
+    name: "Strega Nonna",
+    barcode: "23333121538324",
+    formattedBarcode: "2 3333 12153 8324",
+    expirationDate: "March 28, 2025",
+    emails: ["[email protected]", "[email protected]"],
+    phones: [],
+    homeLibrary: { code: "sn   ", name: "SNFL (formerly Mid-Manhattan)" },
+    id: 6742743,
+  } as Patron
+
+  const component = (patron) => {
+    return (
+      <PatronDataProvider
+        testSpy={accountFetchSpy}
+        value={{
+          patron: patron,
+          pickupLocations: filteredPickupLocations,
+        }}
+      >
+        <SettingsSelectForm
+          patronData={patron}
+          settingsState={mockSettingsState}
+          pickupLocations={pickupLocations}
+          type="notification"
+        />
+      </PatronDataProvider>
+    )
+  }
 
   it("renders correctly with initial preference", () => {
-    render(component)
+    render(component(processedPatron))
 
     expect(screen.getByText(processedPatronPref)).toBeInTheDocument()
 
     expect(screen.getByRole("button", { name: /edit/i })).toBeInTheDocument()
   })
 
   it("allows editing when edit button is clicked", () => {
-    render(component)
+    render(component(processedPatron))
     fireEvent.click(screen.getByRole("button", { name: /edit/i }))
 
     expect(
@@ -78,7 +127,7 @@ describe("notification preference form", () => {
   })
 
   it("manages focus", async () => {
-    render(component)
+    render(component(processedPatron))
     fireEvent.click(screen.getByRole("button", { name: /edit/i }))
 
     await waitFor(() => expect(screen.getByRole("combobox")).toHaveFocus())
@@ -90,8 +139,36 @@ describe("notification preference form", () => {
     )
   })
 
+  it("disables editing the none preference when user has no phone or email", () => {
+    render(component(processedPatronWithNothing))
+
+    expect(screen.getByLabelText("Edit notification")).toBeInTheDocument()
+
+    expect(screen.getByLabelText("Edit notification")).toBeDisabled()
+
+    expect(screen.getByText("None")).toBeInTheDocument()
+
+    expect(
+      screen.getByText(
+        "Please set a phone number or email address to choose a notification preference."
+      )
+    ).toBeInTheDocument()
+  })
+
+  it("disables the corresponding dropdown field when user doesn't have that contact method", () => {
+    render(component(processedPatronWithNoPhone))
+
+    fireEvent.click(screen.getByRole("button", { name: /edit/i }))
+
+    const phoneOption = screen.getByRole("option", { name: "Phone" })
+    expect(phoneOption).toBeDisabled()
+
+    const emailOption = screen.getByRole("option", { name: "Email" })
+    expect(emailOption).not.toBeDisabled()
+  })
+
   it("calls submit with valid data", async () => {
-    render(component)
+    render(component(processedPatron))
 
     fireEvent.click(screen.getByRole("button", { name: /edit/i }))
 
@@ -116,48 +193,15 @@ describe("notification preference form", () => {
   })
 
   it("displays none as an option if that's already user's selection", async () => {
-    const processedPatronWithNone = {
-      notificationPreference: "-",
-      username: "pastadisciple",
-      name: "Strega Nonna",
-      barcode: "23333121538324",
-      formattedBarcode: "2 3333 12153 8324",
-      expirationDate: "March 28, 2025",
-      emails: ["[email protected]", "[email protected]"],
-      phones: [
-        {
-          number: "123-456-7890",
-          type: "t",
-        },
-      ],
-      homeLibrary: { code: "sn   ", name: "SNFL (formerly Mid-Manhattan)" },
-      id: 6742743,
-    } as Patron
-    const component = (
-      <PatronDataProvider
-        testSpy={accountFetchSpy}
-        value={{
-          patron: processedPatronWithNone,
-          pickupLocations: filteredPickupLocations,
-        }}
-      >
-        <SettingsSelectForm
-          patronData={processedPatronWithNone}
-          settingsState={mockSettingsState}
-          pickupLocations={pickupLocations}
-          type="notification"
-        />
-      </PatronDataProvider>
-    )
-    render(component)
+    render(component(processedPatronWithNone))
 
     fireEvent.click(screen.getByRole("button", { name: /edit/i }))
 
     expect(screen.getByText("None")).toBeInTheDocument()
   })
 
   it("cancels editing and reverts state", () => {
-    render(component)
+    render(component(processedPatron))
 
     fireEvent.click(screen.getByRole("button", { name: /edit/i }))
 

diff --git a/src/components/MyAccount/Settings/PhoneForm.test.tsx b/src/components/MyAccount/Settings/PhoneForm.test.tsx
@@ -84,7 +84,7 @@ describe("phone form", () => {
 
     await waitFor(() => expect(screen.getAllByRole("textbox")[1]).toHaveFocus())
 
-    fireEvent.click(screen.getByLabelText("Remove phone"))
+    fireEvent.click(screen.getByLabelText("Remove phone 2"))
 
     await waitFor(() => expect(screen.getByRole("textbox")).toHaveFocus())
   })
@@ -135,7 +135,7 @@ describe("phone form", () => {
       processedPatron.phones.length + 1
     )
 
-    fireEvent.click(screen.getByLabelText("Remove phone"))
+    fireEvent.click(screen.getByLabelText("Remove phone 2"))
     expect(screen.getAllByRole("textbox").length).toBe(
       processedPatron.phones.length
     )

diff --git a/src/components/MyAccount/Settings/SettingsInputForm.tsx b/src/components/MyAccount/Settings/SettingsInputForm.tsx
@@ -53,6 +53,12 @@ const SettingsInputForm = ({
     }
   }
 
+  const focusFirstInput = () => {
+    if (inputRefs.current[0]) {
+      inputRefs?.current[0]?.focus()
+    }
+  }
+
   useEffect(() => {
     focusLastInput()
   }, [tempInputs.length])
@@ -222,7 +228,9 @@ const SettingsInputForm = ({
                   {index == 0 && <div style={{ width: "57px" }}> </div>}
                   {index !== 0 && (
                     <Button
-                      aria-label={`Remove ${formUtils.inputLabel.toLowerCase()}`}
+                      aria-label={`Remove ${formUtils.inputLabel.toLowerCase()} ${
+                        index + 1
+                      }`}
                       buttonType="text"
                       id="remove-input-btn"
                       onClick={() => handleRemove(index)}
@@ -243,7 +251,7 @@ const SettingsInputForm = ({
               <div style={{ width: "72px" }}> </div>
             </Flex>
           </Flex>
-        ) : isEmail || tempInputs.length !== 0 ? (
+        ) : tempInputs.length !== 0 ? (
           <Flex
             marginLeft={{ base: "m", lg: "unset" }}
             flexDir="row"
@@ -284,22 +292,25 @@ const SettingsInputForm = ({
                 onClick={() => {
                   setIsEditing(true)
                   setEditingField(inputType)
-                  setTimeout(() => focusLastInput(), 0)
+                  setTimeout(() => focusFirstInput(), 0)
                 }}
               />
             )}
           </Flex>
         ) : (
           // User has no phone or email.
-          <AddButton
-            inputType={inputType}
-            onClick={() => {
-              setIsEditing(true)
-              setEditingField(inputType)
-              handleAdd()
-            }}
-            label={formUtils.addButtonLabel}
-          />
+          <Box sx={{ marginTop: { base: "unset", lg: "-xs" } }}>
+            <AddButton
+              isDisabled={editingField !== ""}
+              inputType={inputType}
+              onClick={() => {
+                setIsEditing(true)
+                setEditingField(inputType)
+                handleAdd()
+              }}
+              label={formUtils.addButtonLabel}
+            />
+          </Box>
         )}
 
         {isEditing && (