Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SCC-4384: My Account 2.0 VQA/a11y round 1 #412

Merged
merged 17 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/components/MyAccount/ProfileHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ const ProfileHeader = ({ patron }: { patron: Patron }) => {
sx={{
border: "none",
h2: { border: "none", paddingTop: 0 },
marginBottom: "l",
}}
>
{profileData}
Expand Down
4 changes: 2 additions & 2 deletions src/components/MyAccount/ProfileTabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -49,7 +49,7 @@ const ProfileTabs = ({ activePath }: ProfileTabsPropsType) => {
: []),
{
label: "Account settings",
content: <NewAccountSettingsTab />,
content: <AccountSettingsTab />,
urlPath: "settings",
},
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { StatusBanner } from "./StatusBanner"

type StatusType = "" | "failure" | "success"

const NewAccountSettingsTab = () => {
const AccountSettingsTab = () => {
const {
updatedAccountData: { patron, pickupLocations },
} = useContext(PatronDataContext)
Expand Down Expand Up @@ -41,7 +41,7 @@ const NewAccountSettingsTab = () => {
<StatusBanner status={status} statusMessage={statusMessage} />
</div>
)}
<Flex flexDir="column" sx={{ marginTop: "xl", gap: "s" }}>
<Flex flexDir="column" sx={{ marginTop: "m", gap: "s" }}>
<SettingsInputForm
patronData={patron}
settingsState={settingsState}
Expand Down Expand Up @@ -73,4 +73,4 @@ const NewAccountSettingsTab = () => {
)
}

export default NewAccountSettingsTab
export default AccountSettingsTab
46 changes: 26 additions & 20 deletions src/components/MyAccount/Settings/AddButton.tsx
Original file line number Diff line number Diff line change
@@ -1,30 +1,36 @@
import { Button } from "@nypl/design-system-react-components"
import { forwardRef } from "react"

type AddButtonProps = {
inputType?: string
label: string
onClick: () => void
}

const AddButton = ({ inputType, label, onClick }: AddButtonProps) => {
return (
<Button
id={inputType ? `add-${inputType}-button` : "add-button"}
buttonType="text"
onClick={onClick}
size="large"
sx={{
justifyContent: "flex-start",
width: { base: "100%", md: "300px" },
paddingLeft: { base: "m", md: "unset" },
paddingTop: "xs",
paddingBottom: "xs",
paddingRight: "xs",
}}
>
{label}
</Button>
)
}
const AddButton = forwardRef<HTMLButtonElement, AddButtonProps>(
({ inputType, label, onClick }, ref) => {
return (
<Button
ref={ref}
id={inputType ? `add-${inputType}-button` : "add-button"}
buttonType="text"
onClick={onClick}
size="large"
sx={{
justifyContent: "flex-start",
width: { base: "100%", md: "300px" },
paddingLeft: "xs",
paddingTop: "xs",
paddingBottom: "xs",
paddingRight: "xs",
}}
>
{label}
</Button>
)
}
)

AddButton.displayName = "AddButton"

export default AddButton
44 changes: 27 additions & 17 deletions src/components/MyAccount/Settings/EditButton.tsx
Original file line number Diff line number Diff line change
@@ -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 (
<Button
id={buttonId}
buttonType="text"
onClick={onClick}
sx={{
paddingLeft: "xs",
paddingRight: "xs",
marginLeft: "xxl",
}}
>
<Icon name="editorMode" align="left" size="medium" />
Edit
</Button>
)
}
const EditButton = forwardRef<HTMLButtonElement, EditButtonProps>(
({ buttonLabel, buttonId, onClick }, ref) => {
return (
<Button
ref={ref}
id={buttonId}
aria-label={buttonLabel}
buttonType="text"
onClick={onClick}
sx={{
marginTop: { base: "unset", lg: "-xs" },
paddingTop: "0",
paddingBottom: "0",
paddingLeft: "xs",
paddingRight: "xs",
}}
>
<Icon name="editorMode" align="left" size="medium" />
Edit
</Button>
)
}
)

EditButton.displayName = "EditButton"

export default EditButton
37 changes: 32 additions & 5 deletions src/components/MyAccount/Settings/EmailForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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("[email protected]")
).toBeInTheDocument()
Expand All @@ -60,12 +62,37 @@ describe("email form", () => {
expect(screen.queryByText(/edit/)).not.toBeInTheDocument()
})

it("manages focus", async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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(
Expand All @@ -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
)
})
Expand All @@ -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: "[email protected]" } })

fireEvent.click(screen.getByRole("button", { name: /save changes/i }))
Expand All @@ -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: "[email protected]" } })

fireEvent.click(screen.getByRole("button", { name: /cancel/i }))
Expand Down
13 changes: 13 additions & 0 deletions src/components/MyAccount/Settings/HomeLibraryForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 }))
Expand Down
13 changes: 13 additions & 0 deletions src/components/MyAccount/Settings/NotificationForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
17 changes: 16 additions & 1 deletion src/components/MyAccount/Settings/PasswordForm.test.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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")
Expand Down
Loading
Loading