diff --git a/src/components/MyAccount/ProfileHeader.tsx b/src/components/MyAccount/ProfileHeader.tsx index 2acda8126..40a3704fd 100644 --- a/src/components/MyAccount/ProfileHeader.tsx +++ b/src/components/MyAccount/ProfileHeader.tsx @@ -95,6 +95,7 @@ const ProfileHeader = ({ patron }: { patron: Patron }) => { sx={{ border: "none", h2: { border: "none", paddingTop: 0 }, + marginBottom: "l", }} > {profileData} diff --git a/src/components/MyAccount/ProfileTabs.tsx b/src/components/MyAccount/ProfileTabs.tsx index 7148f3fdf..9da9789dc 100644 --- a/src/components/MyAccount/ProfileTabs.tsx +++ b/src/components/MyAccount/ProfileTabs.tsx @@ -6,7 +6,7 @@ import RequestsTab from "./RequestsTab/RequestsTab" import FeesTab from "./FeesTab/FeesTab" import { PatronDataContext } from "../../context/PatronDataContext" import { useContext } from "react" -import NewAccountSettingsTab from "./Settings/NewAccountSettingsTab" +import AccountSettingsTab from "./Settings/AccountSettingsTab" interface ProfileTabsPropsType { activePath: string @@ -49,7 +49,7 @@ const ProfileTabs = ({ activePath }: ProfileTabsPropsType) => { : []), { label: "Account settings", - content: , + content: , urlPath: "settings", }, ] diff --git a/src/components/MyAccount/Settings/NewAccountSettingsTab.tsx b/src/components/MyAccount/Settings/AccountSettingsTab.tsx similarity index 93% rename from src/components/MyAccount/Settings/NewAccountSettingsTab.tsx rename to src/components/MyAccount/Settings/AccountSettingsTab.tsx index 7a4ae694f..1147919b0 100644 --- a/src/components/MyAccount/Settings/NewAccountSettingsTab.tsx +++ b/src/components/MyAccount/Settings/AccountSettingsTab.tsx @@ -8,7 +8,7 @@ import { StatusBanner } from "./StatusBanner" type StatusType = "" | "failure" | "success" -const NewAccountSettingsTab = () => { +const AccountSettingsTab = () => { const { updatedAccountData: { patron, pickupLocations }, } = useContext(PatronDataContext) @@ -41,7 +41,7 @@ const NewAccountSettingsTab = () => { )} - + { ) } -export default NewAccountSettingsTab +export default AccountSettingsTab diff --git a/src/components/MyAccount/Settings/AddButton.tsx b/src/components/MyAccount/Settings/AddButton.tsx index 33f1da869..104dd2cef 100644 --- a/src/components/MyAccount/Settings/AddButton.tsx +++ b/src/components/MyAccount/Settings/AddButton.tsx @@ -1,4 +1,5 @@ import { Button } from "@nypl/design-system-react-components" +import { forwardRef } from "react" type AddButtonProps = { inputType?: string @@ -6,25 +7,30 @@ type AddButtonProps = { onClick: () => void } -const AddButton = ({ inputType, label, onClick }: AddButtonProps) => { - return ( - - ) -} +const AddButton = forwardRef( + ({ inputType, label, onClick }, ref) => { + return ( + + ) + } +) + +AddButton.displayName = "AddButton" export default AddButton diff --git a/src/components/MyAccount/Settings/EditButton.tsx b/src/components/MyAccount/Settings/EditButton.tsx index edff21264..1f4d7d92a 100644 --- a/src/components/MyAccount/Settings/EditButton.tsx +++ b/src/components/MyAccount/Settings/EditButton.tsx @@ -1,26 +1,36 @@ import { Button, Icon } from "@nypl/design-system-react-components" +import { forwardRef } from "react" type EditButtonProps = { buttonId: string + buttonLabel: string onClick: () => void } -const EditButton = ({ buttonId, onClick }: EditButtonProps) => { - return ( - - ) -} +const EditButton = forwardRef( + ({ buttonLabel, buttonId, onClick }, ref) => { + return ( + + ) + } +) + +EditButton.displayName = "EditButton" export default EditButton diff --git a/src/components/MyAccount/Settings/EmailForm.test.tsx b/src/components/MyAccount/Settings/EmailForm.test.tsx index 9636b63db..cd3c8d0f4 100644 --- a/src/components/MyAccount/Settings/EmailForm.test.tsx +++ b/src/components/MyAccount/Settings/EmailForm.test.tsx @@ -48,7 +48,9 @@ describe("email form", () => { render(component) fireEvent.click(screen.getByRole("button", { name: /edit/i })) - expect(screen.getAllByLabelText("Update emails")[0]).toBeInTheDocument() + expect( + screen.getByLabelText("Update primary email address") + ).toBeInTheDocument() expect( screen.getByDisplayValue("streganonna@gmail.com") ).toBeInTheDocument() @@ -60,12 +62,37 @@ describe("email form", () => { expect(screen.queryByText(/edit/)).not.toBeInTheDocument() }) + it("manages focus", async () => { + render(component) + fireEvent.click(screen.getByRole("button", { name: /edit/i })) + + await waitFor(() => + expect(screen.getByLabelText("Update email address 2")).toHaveFocus() + ) + + fireEvent.click(screen.getByRole("button", { name: /cancel/i })) + + await waitFor(() => + expect(screen.getByRole("button", { name: /edit/i })).toHaveFocus() + ) + + fireEvent.click(screen.getByRole("button", { name: /edit/i })) + + fireEvent.click(screen.getByText("+ Add an email address")) + + await waitFor(() => expect(screen.getAllByRole("textbox")[2]).toHaveFocus()) + + fireEvent.click(screen.getAllByLabelText("Remove email")[1]) + + await waitFor(() => expect(screen.getAllByRole("textbox")[1]).toHaveFocus()) + }) + it("validates email input correctly", () => { render(component) fireEvent.click(screen.getByRole("button", { name: /edit/i })) - const input = screen.getAllByLabelText("Update emails")[0] + const input = screen.getByLabelText("Update primary email address") fireEvent.change(input, { target: { value: "invalid-email" } }) expect( @@ -84,7 +111,7 @@ describe("email form", () => { screen.getByRole("button", { name: /\+ add an email address/i }) ) - expect(screen.getAllByLabelText("Update emails").length).toBe( + expect(screen.getAllByRole("textbox").length).toBe( processedPatron.emails.length + 1 ) }) @@ -106,7 +133,7 @@ describe("email form", () => { fireEvent.click(screen.getByRole("button", { name: /edit/i })) - const input = screen.getAllByLabelText("Update emails")[0] + const input = screen.getByLabelText("Update primary email address") fireEvent.change(input, { target: { value: "newemail@example.com" } }) fireEvent.click(screen.getByRole("button", { name: /save changes/i })) @@ -128,7 +155,7 @@ describe("email form", () => { fireEvent.click(screen.getByRole("button", { name: /edit/i })) - const input = screen.getAllByLabelText("Update emails")[0] + const input = screen.getByLabelText("Update primary email address") fireEvent.change(input, { target: { value: "modified@example.com" } }) fireEvent.click(screen.getByRole("button", { name: /cancel/i })) diff --git a/src/components/MyAccount/Settings/HomeLibraryForm.test.tsx b/src/components/MyAccount/Settings/HomeLibraryForm.test.tsx index 6375ef7d4..8303ecf3c 100644 --- a/src/components/MyAccount/Settings/HomeLibraryForm.test.tsx +++ b/src/components/MyAccount/Settings/HomeLibraryForm.test.tsx @@ -48,6 +48,19 @@ describe("home library form", () => { expect(screen.getByRole("button", { name: /edit/i })).toBeInTheDocument() }) + it("manages focus", async () => { + render(component) + fireEvent.click(screen.getByRole("button", { name: /edit/i })) + + await waitFor(() => expect(screen.getByRole("combobox")).toHaveFocus()) + + fireEvent.click(screen.getByRole("button", { name: /cancel/i })) + + await waitFor(() => + expect(screen.getByRole("button", { name: /edit/i })).toHaveFocus() + ) + }) + it("allows editing when edit button is clicked", () => { render(component) fireEvent.click(screen.getByRole("button", { name: /edit/i })) diff --git a/src/components/MyAccount/Settings/NotificationForm.test.tsx b/src/components/MyAccount/Settings/NotificationForm.test.tsx index a603255e8..72b9f1989 100644 --- a/src/components/MyAccount/Settings/NotificationForm.test.tsx +++ b/src/components/MyAccount/Settings/NotificationForm.test.tsx @@ -77,6 +77,19 @@ describe("notification preference form", () => { expect(screen.queryByText(/edit/)).not.toBeInTheDocument() }) + it("manages focus", async () => { + render(component) + fireEvent.click(screen.getByRole("button", { name: /edit/i })) + + await waitFor(() => expect(screen.getByRole("combobox")).toHaveFocus()) + + fireEvent.click(screen.getByRole("button", { name: /cancel/i })) + + await waitFor(() => + expect(screen.getByRole("button", { name: /edit/i })).toHaveFocus() + ) + }) + it("calls submit with valid data", async () => { render(component) diff --git a/src/components/MyAccount/Settings/PasswordForm.test.tsx b/src/components/MyAccount/Settings/PasswordForm.test.tsx index 5c5dce990..40cf762ba 100644 --- a/src/components/MyAccount/Settings/PasswordForm.test.tsx +++ b/src/components/MyAccount/Settings/PasswordForm.test.tsx @@ -1,5 +1,5 @@ import React from "react" -import { render, fireEvent, waitFor } from "../../../utils/testUtils" +import { render, fireEvent, waitFor, screen } from "../../../utils/testUtils" import PasswordForm from "./PasswordForm" import { filteredPickupLocations, @@ -38,6 +38,21 @@ beforeEach(() => { }) describe("Pin/password form", () => { + it("manages focus", async () => { + render(component) + fireEvent.click(screen.getByRole("button", { name: /edit/i })) + + await waitFor(() => + expect(screen.getByLabelText("Enter current pin/password")).toHaveFocus() + ) + + fireEvent.click(screen.getByRole("button", { name: /cancel/i })) + + await waitFor(() => + expect(screen.getByRole("button", { name: /edit/i })).toHaveFocus() + ) + }) + it("disables submit button if any form field is empty", async () => { const { getByText, getByLabelText } = render(component) const button = getByText("Edit") diff --git a/src/components/MyAccount/Settings/PasswordForm.tsx b/src/components/MyAccount/Settings/PasswordForm.tsx index e99427c21..eb5dc17d5 100644 --- a/src/components/MyAccount/Settings/PasswordForm.tsx +++ b/src/components/MyAccount/Settings/PasswordForm.tsx @@ -1,5 +1,6 @@ -import { useContext, useState } from "react" +import { forwardRef, useContext, useRef, useState } from "react" import { PatronDataContext } from "../../../context/PatronDataContext" +import type { TextInputRefType } from "@nypl/design-system-react-components" import { Banner, Flex, @@ -30,37 +31,38 @@ export const passwordFormMessages = { INVALID: "Invalid new pin/password.", } -const PasswordFormField = ({ - label, - handler, - name, - isInvalid, -}: PasswordFormFieldProps) => { - return ( - - - - - ) -} +const PasswordFormField = forwardRef( + ({ label, handler, name, isInvalid }: PasswordFormFieldProps, ref) => { + return ( + + + + + ) + } +) + +PasswordFormField.displayName = "PasswordFormField" const PasswordForm = ({ patronData, settingsState }: PasswordFormProps) => { const { getMostUpdatedSierraAccountData } = useContext(PatronDataContext) @@ -74,10 +76,15 @@ const PasswordForm = ({ patronData, settingsState }: PasswordFormProps) => { }) const { setStatus, setStatusMessage, editingField, setEditingField } = settingsState + const editingRef = useRef() + const inputRef = useRef() const cancelEditing = () => { setIsEditing(false) setEditingField("") + setTimeout(() => { + editingRef.current?.focus() + }, 0) } const validateForm = @@ -146,18 +153,30 @@ const PasswordForm = ({ patronData, settingsState }: PasswordFormProps) => { return ( <> {isLoading ? ( - + + + div": { marginTop: "-s" } }} + contentSize={2} + showImage={false} + headingSize={0} + /> + ) : isEditing ? ( <> - + { /> { @@ -231,10 +251,15 @@ const PasswordForm = ({ patronData, settingsState }: PasswordFormProps) => { {editingField === "" && ( { setIsEditing(true) setEditingField("password") + setTimeout(() => { + inputRef.current?.focus() + }, 0) }} /> )} diff --git a/src/components/MyAccount/Settings/PhoneForm.test.tsx b/src/components/MyAccount/Settings/PhoneForm.test.tsx index 685d5d043..160574f94 100644 --- a/src/components/MyAccount/Settings/PhoneForm.test.tsx +++ b/src/components/MyAccount/Settings/PhoneForm.test.tsx @@ -50,7 +50,9 @@ describe("phone form", () => { render(component) fireEvent.click(screen.getByRole("button", { name: /edit/i })) - expect(screen.getAllByLabelText("Update phones")[0]).toBeInTheDocument() + expect( + screen.getByLabelText("Update primary phone number") + ).toBeInTheDocument() expect( screen.getByDisplayValue(processedPatron.phones[0].number) ).toBeInTheDocument() @@ -62,12 +64,37 @@ describe("phone form", () => { expect(screen.queryByText(/edit/)).not.toBeInTheDocument() }) + it("manages focus", async () => { + render(component) + fireEvent.click(screen.getByRole("button", { name: /edit/i })) + + await waitFor(() => + expect(screen.getByLabelText("Update primary phone number")).toHaveFocus() + ) + + fireEvent.click(screen.getByRole("button", { name: /cancel/i })) + + await waitFor(() => + expect(screen.getByRole("button", { name: /edit/i })).toHaveFocus() + ) + + fireEvent.click(screen.getByRole("button", { name: /edit/i })) + + fireEvent.click(screen.getByText("+ Add a phone number")) + + await waitFor(() => expect(screen.getAllByRole("textbox")[1]).toHaveFocus()) + + fireEvent.click(screen.getByLabelText("Remove phone")) + + await waitFor(() => expect(screen.getByRole("textbox")).toHaveFocus()) + }) + it("validates phone input correctly", () => { render(component) fireEvent.click(screen.getByRole("button", { name: /edit/i })) - const input = screen.getAllByLabelText("Update phones")[0] + const input = screen.getByLabelText("Update primary phone number") fireEvent.change(input, { target: { value: "invalid-phone" } }) expect( @@ -86,7 +113,7 @@ describe("phone form", () => { screen.getByRole("button", { name: /\+ add a phone number/i }) ) - expect(screen.getAllByLabelText("Update phones").length).toBe( + expect(screen.getAllByRole("textbox").length).toBe( processedPatron.phones.length + 1 ) }) @@ -100,7 +127,7 @@ describe("phone form", () => { screen.getByRole("button", { name: /\+ add a phone number/i }) ) - const input = screen.getAllByLabelText("Update phones")[1] + const input = screen.getByLabelText("Update phone number 2") fireEvent.change(input, { target: { value: "5106974153" } }) @@ -109,8 +136,7 @@ describe("phone form", () => { ) fireEvent.click(screen.getByLabelText("Remove phone")) - - expect(screen.getAllByLabelText("Update phones").length).toBe( + expect(screen.getAllByRole("textbox").length).toBe( processedPatron.phones.length ) }) @@ -120,7 +146,7 @@ describe("phone form", () => { fireEvent.click(screen.getByRole("button", { name: /edit/i })) - const input = screen.getAllByLabelText("Update phones")[0] + const input = screen.getByLabelText("Update primary phone number") fireEvent.change(input, { target: { value: "1234" } }) fireEvent.click(screen.getByRole("button", { name: /save changes/i })) @@ -142,7 +168,7 @@ describe("phone form", () => { fireEvent.click(screen.getByRole("button", { name: /edit/i })) - const input = screen.getAllByLabelText("Update phones")[0] + const input = screen.getByLabelText("Update primary phone number") fireEvent.change(input, { target: { value: "4534" } }) fireEvent.click(screen.getByRole("button", { name: /cancel/i })) diff --git a/src/components/MyAccount/Settings/SaveCancelButtons.tsx b/src/components/MyAccount/Settings/SaveCancelButtons.tsx index e6bd00711..3d77a7d96 100644 --- a/src/components/MyAccount/Settings/SaveCancelButtons.tsx +++ b/src/components/MyAccount/Settings/SaveCancelButtons.tsx @@ -4,7 +4,13 @@ type SaveCancelButtonProps = { isDisabled?: boolean onCancel: () => void onSave: () => void - inputType?: "phones" | "emails" + inputType: + | "emails" + | "phones" + | "password" + | "username" + | "library" + | "notification" } const SaveCancelButtons = ({ diff --git a/src/components/MyAccount/Settings/SettingsInputForm.tsx b/src/components/MyAccount/Settings/SettingsInputForm.tsx index 14cd6acc9..52f763604 100644 --- a/src/components/MyAccount/Settings/SettingsInputForm.tsx +++ b/src/components/MyAccount/Settings/SettingsInputForm.tsx @@ -8,7 +8,8 @@ import { Form, Box, } from "@nypl/design-system-react-components" -import { useContext, useState } from "react" +import type { TextInputRefType } from "@nypl/design-system-react-components" +import { useContext, useEffect, useRef, useState } from "react" import { PatronDataContext } from "../../../context/PatronDataContext" import SaveCancelButtons from "./SaveCancelButtons" import SettingsLabel from "./SettingsLabel" @@ -42,9 +43,26 @@ const SettingsInputForm = ({ const [tempInputs, setTempInputs] = useState([...inputs]) + const inputRefs = useRef>([]) + const editingRef = useRef() + + const focusLastInput = () => { + const lastIndex = tempInputs.length - 1 + if (inputRefs.current[lastIndex]) { + inputRefs.current[lastIndex]?.focus() + } + } + + useEffect(() => { + focusLastInput() + }, [tempInputs.length]) + const formUtils = { regex: isEmail ? /^[^@]+@[^@]+\.[^@]+$/ : /^\+?[1-9]\d{1,14}$/, - labelText: `Update ${inputType}`, + labelText: isEmail ? "Update email address" : "Update phone number", + primaryLabelText: isEmail + ? "Update primary email address" + : "Update primary phone number", addButtonLabel: isEmail ? "+ Add an email address" : "+ Add a phone number", errorMessage: `Please enter a valid and unique ${ isEmail ? "email address" : "phone number" @@ -82,6 +100,9 @@ const SettingsInputForm = ({ const updatedInputs = tempInputs.filter((_, i) => i !== index) setTempInputs(updatedInputs) + // Remove its corresponding ref. + inputRefs.current = inputRefs.current.filter((_, i) => i !== index) + // Immediately revalidate remaining inputs. const hasInvalidInput = updatedInputs.some( (input) => !validateInput(input, updatedInputs) @@ -100,18 +121,14 @@ const SettingsInputForm = ({ setError(hasInvalidInput) } - const handleClearableCallback = (index) => { - const updatedInputs = [...tempInputs] - updatedInputs[index] = "" - setTempInputs(updatedInputs) - setError(true) - } - const cancelEditing = () => { setTempInputs([...inputs]) setIsEditing(false) setEditingField("") setError(false) + setTimeout(() => { + editingRef.current?.focus() + }, 0) } const submitInputs = async () => { @@ -166,12 +183,16 @@ const SettingsInputForm = ({ width="100%" > - {isLoading ? ( - + div": { marginTop: "-s" } }} + contentSize={2} + showImage={false} + headingSize={0} + /> ) : isEditing ? ( (inputRefs.current[index] = el)} name={`input-${index}`} value={input} id={`${inputType}-text-input-${index}`} - labelText={formUtils.labelText} + labelText={ + index == 0 + ? formUtils.primaryLabelText + : `${formUtils.labelText} ${index + 1}` + } showLabel={false} isInvalid={error && !validateInput(input, tempInputs)} invalidText={formUtils.errorMessage} onChange={(e) => handleInputChange(e, index)} isRequired - isClearable - isClearableCallback={() => handleClearableCallback(index)} /> + {index == 0 &&
} {index !== 0 && (