From 238e363bb6434f6da5d69a4e7c5da7155265ab24 Mon Sep 17 00:00:00 2001 From: Emma Mansell <73774046+7emansell@users.noreply.github.com> Date: Tue, 3 Dec 2024 12:36:14 -0600 Subject: [PATCH 01/17] VQA fixes --- src/components/MyAccount/ProfileHeader.tsx | 1 + src/components/MyAccount/ProfileTabs.tsx | 4 +-- ...SettingsTab.tsx => AccountSettingsTab.tsx} | 6 ++-- .../MyAccount/Settings/AddButton.tsx | 2 +- .../MyAccount/Settings/EditButton.tsx | 7 +++-- .../MyAccount/Settings/PasswordForm.tsx | 15 ++++++--- .../MyAccount/Settings/SettingsInputForm.tsx | 31 ++++++++++++------- .../MyAccount/Settings/SettingsLabel.tsx | 8 ++--- .../MyAccount/Settings/SettingsSelectForm.tsx | 26 +++++++++------- .../MyAccount/Settings/StatusBanner.tsx | 12 ++----- .../MyAccount/Settings/UsernameForm.tsx | 8 +++-- 11 files changed, 70 insertions(+), 50 deletions(-) rename src/components/MyAccount/Settings/{NewAccountSettingsTab.tsx => AccountSettingsTab.tsx} (93%) 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..48bf98be7 100644 --- a/src/components/MyAccount/Settings/AddButton.tsx +++ b/src/components/MyAccount/Settings/AddButton.tsx @@ -16,7 +16,7 @@ const AddButton = ({ inputType, label, onClick }: AddButtonProps) => { sx={{ justifyContent: "flex-start", width: { base: "100%", md: "300px" }, - paddingLeft: { base: "m", md: "unset" }, + paddingLeft: { base: "m", md: "xs" }, paddingTop: "xs", paddingBottom: "xs", paddingRight: "xs", diff --git a/src/components/MyAccount/Settings/EditButton.tsx b/src/components/MyAccount/Settings/EditButton.tsx index edff21264..88ad45160 100644 --- a/src/components/MyAccount/Settings/EditButton.tsx +++ b/src/components/MyAccount/Settings/EditButton.tsx @@ -2,19 +2,22 @@ import { Button, Icon } from "@nypl/design-system-react-components" type EditButtonProps = { buttonId: string + buttonLabel: string onClick: () => void } -const EditButton = ({ buttonId, onClick }: EditButtonProps) => { +const EditButton = ({ buttonId, buttonLabel, onClick }: EditButtonProps) => { return ( Date: Tue, 3 Dec 2024 15:56:27 -0500 Subject: [PATCH 06/17] Updating tests with new explicitly numbered labels --- .../MyAccount/Settings/EmailForm.test.tsx | 12 +++++++----- .../MyAccount/Settings/PhoneForm.test.tsx | 17 +++++++++-------- .../MyAccount/Settings/SettingsInputForm.tsx | 2 +- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/components/MyAccount/Settings/EmailForm.test.tsx b/src/components/MyAccount/Settings/EmailForm.test.tsx index 9636b63db..fdeab09b8 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() @@ -65,7 +67,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: "invalid-email" } }) expect( @@ -84,7 +86,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 +108,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 +130,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/PhoneForm.test.tsx b/src/components/MyAccount/Settings/PhoneForm.test.tsx index 685d5d043..b858d678f 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() @@ -67,7 +69,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: "invalid-phone" } }) expect( @@ -86,7 +88,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 +102,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 +111,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 +121,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 +143,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/SettingsInputForm.tsx b/src/components/MyAccount/Settings/SettingsInputForm.tsx index 6f236f536..c6aa36c54 100644 --- a/src/components/MyAccount/Settings/SettingsInputForm.tsx +++ b/src/components/MyAccount/Settings/SettingsInputForm.tsx @@ -215,7 +215,7 @@ const SettingsInputForm = ({ labelText={ index == 0 ? formUtils.primaryLabelText - : `${formUtils.labelText} ${index}` + : `${formUtils.labelText} ${index + 1}` } showLabel={false} isInvalid={error && !validateInput(input, tempInputs)} From 4d6a4006f00697336fb5a087fa9135383e7af4ac Mon Sep 17 00:00:00 2001 From: Emma Mansell <73774046+7emansell@users.noreply.github.com> Date: Tue, 3 Dec 2024 15:57:11 -0500 Subject: [PATCH 07/17] And username test --- src/components/MyAccount/Settings/UsernameForm.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/MyAccount/Settings/UsernameForm.test.tsx b/src/components/MyAccount/Settings/UsernameForm.test.tsx index 479b37210..a628c9c7e 100644 --- a/src/components/MyAccount/Settings/UsernameForm.test.tsx +++ b/src/components/MyAccount/Settings/UsernameForm.test.tsx @@ -120,7 +120,7 @@ describe("username form", () => { fireEvent.click(screen.getByRole("button", { name: /edit/i })) - fireEvent.click(screen.getByLabelText("Remove username")) + fireEvent.click(screen.getByLabelText("Delete username from your account")) expect( screen.queryByDisplayValue(processedPatron.username) From 612e0274be7fb2770df9ea596568abb74eecf6ed Mon Sep 17 00:00:00 2001 From: Emma Mansell <73774046+7emansell@users.noreply.github.com> Date: Thu, 5 Dec 2024 11:14:35 -0500 Subject: [PATCH 08/17] Reducing refs and styling corrections --- .../MyAccount/Settings/EditButton.tsx | 4 +-- .../MyAccount/Settings/PasswordForm.tsx | 36 +++++++++++-------- .../MyAccount/Settings/SettingsInputForm.tsx | 4 +-- .../MyAccount/Settings/SettingsSelectForm.tsx | 13 ++++--- .../MyAccount/Settings/UsernameForm.tsx | 22 ++++++------ 5 files changed, 41 insertions(+), 38 deletions(-) diff --git a/src/components/MyAccount/Settings/EditButton.tsx b/src/components/MyAccount/Settings/EditButton.tsx index 4bdd2b50b..1f4d7d92a 100644 --- a/src/components/MyAccount/Settings/EditButton.tsx +++ b/src/components/MyAccount/Settings/EditButton.tsx @@ -17,9 +17,7 @@ const EditButton = forwardRef( buttonType="text" onClick={onClick} sx={{ - _focus: { - outline: "2px green solid", - }, + marginTop: { base: "unset", lg: "-xs" }, paddingTop: "0", paddingBottom: "0", paddingLeft: "xs", diff --git a/src/components/MyAccount/Settings/PasswordForm.tsx b/src/components/MyAccount/Settings/PasswordForm.tsx index 656e6068e..95e6f3dfc 100644 --- a/src/components/MyAccount/Settings/PasswordForm.tsx +++ b/src/components/MyAccount/Settings/PasswordForm.tsx @@ -41,9 +41,11 @@ const PasswordFormField = forwardRef( > { }) const { setStatus, setStatusMessage, editingField, setEditingField } = settingsState - const editingRef = useRef() - const inputRef = useRef() + const focusRef = useRef() const cancelEditing = () => { setIsEditing(false) setEditingField("") setTimeout(() => { - editingRef.current?.focus() + focusRef.current?.focus() }, 0) } @@ -152,12 +153,19 @@ const PasswordForm = ({ patronData, settingsState }: PasswordFormProps) => { return ( <> {isLoading ? ( - div": { marginTop: "-xs" } }} - contentSize={2} - showImage={false} - headingSize={0} - /> + + + div": { marginTop: "-xs" } }} + contentSize={2} + showImage={false} + headingSize={0} + /> + ) : isEditing ? ( <> @@ -168,7 +176,7 @@ const PasswordForm = ({ patronData, settingsState }: PasswordFormProps) => { }} > } label="Enter current pin/password" name="currentPassword" handler={handleInputChange} @@ -242,14 +250,14 @@ const PasswordForm = ({ patronData, settingsState }: PasswordFormProps) => { {editingField === "" && ( } buttonLabel="Edit password" buttonId="edit-password-button" onClick={() => { setIsEditing(true) setEditingField("password") setTimeout(() => { - inputRef.current?.focus() + focusRef.current?.focus() }, 0) }} /> diff --git a/src/components/MyAccount/Settings/SettingsInputForm.tsx b/src/components/MyAccount/Settings/SettingsInputForm.tsx index c6aa36c54..70d4b6b9a 100644 --- a/src/components/MyAccount/Settings/SettingsInputForm.tsx +++ b/src/components/MyAccount/Settings/SettingsInputForm.tsx @@ -189,7 +189,7 @@ const SettingsInputForm = ({ {isLoading ? ( div": { marginTop: "-xs" } }} + sx={{ "> div": { marginTop: "-s" } }} contentSize={2} showImage={false} headingSize={0} @@ -240,7 +240,7 @@ const SettingsInputForm = ({ ))} - + () - const inputRef = useRef() + const focusRef = useRef() const notificationPreferenceMap = patronData.notificationPreference === "-" @@ -92,7 +91,7 @@ const SettingsSelectForm = ({ setIsEditing(false) setEditingField("") setTimeout(() => { - editingRef.current?.focus() + focusRef.current?.focus() }, 0) } @@ -150,7 +149,7 @@ const SettingsSelectForm = ({ {isLoading ? ( div": { marginTop: "-xs" } }} + sx={{ "> div": { marginTop: "-s" } }} contentSize={2} showImage={false} headingSize={0} @@ -164,7 +163,7 @@ const SettingsSelectForm = ({ width="-webkit-fill-available" > } + ref={selectRef} width={{ base: "100%", md: "max-content" }} name={`select-${type}`} id={formUtils.selectorId} @@ -192,14 +193,14 @@ const SettingsSelectForm = ({ {editingField === "" && ( } + ref={editingRef} buttonLabel={`Edit ${type}`} buttonId={`edit-${type}-button`} onClick={() => { setIsEditing(true) setEditingField(type) setTimeout(() => { - focusRef.current?.focus() + selectRef.current?.focus() }, 0) }} /> diff --git a/src/components/MyAccount/Settings/UsernameForm.tsx b/src/components/MyAccount/Settings/UsernameForm.tsx index 527239738..5e740b07b 100644 --- a/src/components/MyAccount/Settings/UsernameForm.tsx +++ b/src/components/MyAccount/Settings/UsernameForm.tsx @@ -46,7 +46,9 @@ const UsernameForm = ({ patron, usernameState }: UsernameFormProps) => { const currentUsernameNotDeleted = tempUsername !== null const { setUsernameStatus, setUsernameStatusMessage } = usernameState - const focusRef = useRef() + const inputRef = useRef() + const editingRef = useRef() + const addButtonRef = useRef() const validateUsername = (username: string) => { const usernameRegex = /^[a-zA-Z0-9]{5,15}$/ @@ -58,7 +60,7 @@ const UsernameForm = ({ patron, usernameState }: UsernameFormProps) => { setIsEditing(false) setError(false) setTimeout(() => { - focusRef.current?.focus() + editingRef.current?.focus() }, 0) } @@ -119,7 +121,7 @@ const UsernameForm = ({ patron, usernameState }: UsernameFormProps) => { sx={{ width: { base: "87%", md: "300px" }, }} - ref={focusRef as React.Ref} + ref={inputRef} value={tempUsername} id="username-input" labelText="Username" @@ -130,7 +132,9 @@ const UsernameForm = ({ patron, usernameState }: UsernameFormProps) => { showHelperInvalidText={true} onChange={handleInputChange} isClearable - isClearableCallback={() => setError(true)} + isClearableCallback={() => { + setError(true) + }} />