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.

Sign up for GitHub

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

replace manage facility admin screen dropdowns with combos #6439

Merged
merged 14 commits into from
Sep 7, 2023
Merged

replace manage facility admin screen dropdowns with combos #6439

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
8c8fdb1
refactor using truss
fzhao99 Aug 29, 2023
c972ca6
add tests
fzhao99 Aug 29, 2023
a285c5c
temporarily wrap things in act
fzhao99 Aug 31, 2023
3497425
fix accessibility violations and refactor tests
fzhao99 Aug 31, 2023
d9b61f0
scope down unnecessary changes
fzhao99 Aug 31, 2023
20c9677
use recommended userEvent setup pattern
fzhao99 Aug 31, 2023
c424b12
fix mocks
fzhao99 Aug 31, 2023
b449eef
move gql alias to before each hook
fzhao99 Aug 31, 2023
397f579
switch selector
fzhao99 Aug 31, 2023
641eb26
rework e2e to work with combo box
fzhao99 Aug 31, 2023
831d535
add org util query to simplify selectors
fzhao99 Sep 5, 2023
9130d59
add extracted combo box component
fzhao99 Sep 6, 2023
54ed7ed
add forwardRef hook
fzhao99 Sep 6, 2023
4a21a24
relax label selectors
fzhao99 Sep 6, 2023
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
44 changes: 30 additions & 14 deletions cypress/e2e/11-support-manage-facility.cy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,31 @@
import {loginHooks, testNumber} from "../support/e2e";
import {addMockFacility, whoAmI} from "../utils/testing-data-utils";
import {graphqlURL} from "../utils/request-utils";
import {aliasGraphqlOperations} from "../utils/graphql-test-utils";
import { loginHooks, testNumber } from "../support/e2e";
import { addMockFacility, whoAmI } from "../utils/testing-data-utils";
import { graphqlURL } from "../utils/request-utils";
import { aliasGraphqlOperations } from "../utils/graphql-test-utils";

loginHooks();
describe("Support admin: manage facility", () => {
let organizationId="";
let organizationId = "";
let facilityId = "";
let facilityCreated = {
id:"",
name:`RainbowCenter-${testNumber()}`,
}
id: "",
name: `RainbowCenter-${testNumber()}`
};

before(() => {
addMockFacility(facilityCreated.name).then(response=>{
addMockFacility(facilityCreated.name).then(response => {
facilityCreated.id = response.body.data.addFacility.id;
});

whoAmI().then((res) => {
organizationId = res.body.data.whoami.organization.id;
facilityId = res.body.data.whoami.organization.facilities[0].id;
});
});

beforeEach(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moving this hook into a before each as recommended here

cy.intercept("POST", graphqlURL, (req) => {
aliasGraphqlOperations(req)
aliasGraphqlOperations(req);
});
});

Expand All @@ -38,11 +40,25 @@ describe("Support admin: manage facility", () => {

// selects organization
cy.wait("@GetAllOrganizations");
cy.get("div.bg-base-lightest select").first().select(organizationId);

// selects facility
// selects org combo box
cy.get("[data-testid=\"org-selection-container\"] " +
"> [data-testid=\"combo-box\"] ")
.within(() => {
// within the org selection box
cy.get("[data-testid=\"combo-box-input\"]").click();
fzhao99 marked this conversation as resolved.
Show resolved Hide resolved
cy.get(`[data-testid=\"combo-box-option-${organizationId}\"]`).click();
});

// selects facility combo box
cy.wait("@GetFacilitiesByOrgId");
cy.get("div.bg-base-lightest select").eq(1).select(facilityCreated.id);
cy.get("[data-testid=\"facility-selection-container\"] " +
"> [data-testid=\"combo-box\"]")
.within(() => {
// within the org selection box
cy.get("[data-testid=\"combo-box-input\"]").click();
cy.get(`[data-testid=\"combo-box-option-${facilityCreated.id}\"]`).click();
});

// clicks search button
cy.get("button").contains("Search").click();
Expand All @@ -62,7 +78,7 @@ describe("Support admin: manage facility", () => {
cy.contains(`Delete ${facilityCreated.name}`).should("not.exist");
});

it("Deletes a facility",()=>{
it("Deletes a facility", () => {
cy.get("button").contains("Delete facility").click();
cy.get("button").contains("Yes, delete facility").click();
cy.get(".Toastify").contains(`Facility ${facilityCreated.name} successfully deleted`);
Expand Down
67 changes: 47 additions & 20 deletions frontend/src/app/supportAdmin/ManageFacility/FacilitySelectFilter.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,33 @@
import { fireEvent, render, screen, waitFor } from "@testing-library/react";
import { act, render, screen, waitFor, within } from "@testing-library/react";
import { MemoryRouter } from "react-router-dom";
import React from "react";
import { ComboBoxRef } from "@trussworks/react-uswds";
import userEvent from "@testing-library/user-event";

import { Option } from "../../commonComponents/Dropdown";

import FacilitySelectFilter from "./FacilitySelectFilter";
import { initialState, ManageFacilityState } from "./ManageFacility";

export const getOrgComboBoxElements = () => {
const orgSelectionDiv = screen.getByTestId("org-selection-container");
const orgComboBoxInput = screen.getByLabelText("Organization *");
const orgComboBoxList = within(orgSelectionDiv).getByTestId(
"combo-box-option-list"
);
return [orgComboBoxInput, orgComboBoxList] as const;
};

export const getFacilityComboBoxElements = () => {
const facilitySelectionDiv = screen.getByTestId(
"facility-selection-container"
);
const facilityComboBoxInput = screen.getByLabelText("Testing facility *");
const facilityComboBoxList = within(facilitySelectionDiv).getByTestId(
"combo-box-option-list"
);
return [facilityComboBoxInput, facilityComboBoxList] as const;
};
describe("FacilitySelectFilter", () => {
const handleClearFilter = jest.fn();
const handleSelectOrg = jest.fn();
Expand All @@ -22,7 +44,9 @@ describe("FacilitySelectFilter", () => {
orgOptions: Option[],
facilityOptions: Option[],
manageFacilityState: ManageFacilityState
) =>
) => {
const orgRef = React.createRef<ComboBoxRef>();
const facilityRef = React.createRef<ComboBoxRef>();
render(
<MemoryRouter>
<FacilitySelectFilter
Expand All @@ -34,50 +58,56 @@ describe("FacilitySelectFilter", () => {
onSearch={handleSearch}
manageFacilityState={manageFacilityState}
loading={true}
orgRef={orgRef}
facilityRef={facilityRef}
/>
</MemoryRouter>
);
};
const user = userEvent.setup();

it("disables controls when loading data", () => {
renderWithMocks([], [], initialState);

expect(
screen.getByRole("combobox", { name: /organization/i })
).toBeDisabled();
expect(screen.getByRole("combobox", { name: /facility/i })).toBeDisabled();
const [orgDropdown] = getOrgComboBoxElements();
const [facilityDropdown] = getFacilityComboBoxElements();

expect(facilityDropdown).toBeDisabled();
expect(orgDropdown).toBeDisabled();
});

it("calls handleClearFilter upon clicking clear filters button", async () => {
renderWithMocks(mockOrganizationOptions, mockFacilityOptions, {
orgId: "123",
facilityId: "",
facilityId: undefined,
facility: undefined,
});
const clearFiltersBtn = screen.getByRole("button", {
name: /clear facility selection filters/i,
});
expect(clearFiltersBtn).toBeEnabled();
fireEvent.click(clearFiltersBtn);
await act(() => user.click(clearFiltersBtn));
await waitFor(() => expect(handleClearFilter).toHaveBeenCalled());
});

it("calls event handlers when organization is selected", async () => {
renderWithMocks(mockOrganizationOptions, mockFacilityOptions, initialState);
const orgDropdown = screen.getByRole("combobox", { name: /organization/i });
fireEvent.change(orgDropdown, { target: { value: "123" } });

const [, orgDropdown] = getOrgComboBoxElements();

await act(() => user.selectOptions(orgDropdown, ["organization-123"]));
await waitFor(() => expect(handleSelectOrg).toHaveBeenCalled());
});

it("calls event handlers when facility is selected", async () => {
renderWithMocks(mockOrganizationOptions, mockFacilityOptions, {
orgId: "123",
facilityId: "",
facilityId: undefined,
facility: undefined,
});
const facilityDropdown = screen.getByRole("combobox", {
name: /facility/i,
});
fireEvent.change(facilityDropdown, { target: { value: "123" } });

const [, facilityDropdown] = getFacilityComboBoxElements();
await act(() => user.selectOptions(facilityDropdown, ["facility-123"]));
await waitFor(() => expect(handleSelectFacility).toHaveBeenCalled());
});

Expand All @@ -87,15 +117,12 @@ describe("FacilitySelectFilter", () => {
facilityId: "123",
facility: undefined,
});
const facilityDropdown = screen.getByRole("combobox", {
name: /facility/i,
});
fireEvent.change(facilityDropdown, { target: { value: "123" } });

const searchBtn = screen.getByRole("button", {
name: /search/i,
});
fireEvent.click(searchBtn);
expect(searchBtn).toBeEnabled();
await act(() => user.click(searchBtn));

await waitFor(() => expect(handleSearch).toHaveBeenCalled());
});
Expand Down
81 changes: 47 additions & 34 deletions frontend/src/app/supportAdmin/ManageFacility/FacilitySelectFilter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
import { faSlidersH } from "@fortawesome/free-solid-svg-icons";
import React from "react";
import React, { Ref } from "react";
import { ComboBox, ComboBoxRef, Label } from "@trussworks/react-uswds";

import Dropdown, { Option } from "../../commonComponents/Dropdown";
import Button from "../../commonComponents/Button/Button";
import SupportHomeLink from "../SupportHomeLink";
import { Option } from "../../commonComponents/Dropdown";
import Required from "../../commonComponents/Required";

import { ManageFacilityState } from "./ManageFacility";

export interface FacilitySelectFilterProps {
organizationOptions: Option[];
facilityOptions: Option[];
manageFacilityState: ManageFacilityState;
onClearFilter: () => void;
onSelectOrg: (e: any) => void;
onSelectFacility: (e: any) => void;
onSelectOrg: (selectedOrg: string | undefined) => void;
onSelectFacility: (selectedFacility: string | undefined) => void;
onSearch: (e: any) => void;
manageFacilityState: ManageFacilityState;
loading: boolean;
facilityRef: Ref<ComboBoxRef> | undefined;
orgRef: Ref<ComboBoxRef> | undefined;
}

const FacilitySelectFilter: React.FC<FacilitySelectFilterProps> = ({
Expand All @@ -27,6 +31,8 @@ const FacilitySelectFilter: React.FC<FacilitySelectFilterProps> = ({
onSelectFacility,
onSearch,
loading,
facilityRef,
orgRef,
}) => {
/**
* HTML
Expand All @@ -43,7 +49,7 @@ const FacilitySelectFilter: React.FC<FacilitySelectFilterProps> = ({
<div className="desktop:grid-col-auto tablet:grid-col-auto mobile:grid-col-12 margin-top-2 tablet:margin-top-0">
<Button
icon={faSlidersH}
disabled={manageFacilityState.orgId === ""}
disabled={manageFacilityState.orgId === undefined}
onClick={onClearFilter}
ariaLabel="Clear facility selection filters"
>
Expand All @@ -58,40 +64,47 @@ const FacilitySelectFilter: React.FC<FacilitySelectFilterProps> = ({
className="bg-base-lightest padding-left-3 padding-right-3 padding-bottom-1"
>
<div className="grid-row grid-gap padding-bottom-2 flex-align-end">
<div className="desktop:grid-col-4 tablet:grid-col-4 mobile:grid-col-1">
<Dropdown
label="Organization"
options={[
{
label: "- Select -",
value: "",
},
...organizationOptions,
]}
onChange={onSelectOrg}
selectedValue={manageFacilityState.orgId}
<div
data-testid={"org-selection-container"}
className="desktop:grid-col-4 tablet:grid-col-4 mobile:grid-col-1 margin-top-1em"
>
<Label htmlFor="manage-facility-org-select">
Organization <Required />
</Label>
<ComboBox
name={"Organization"}
id={"manage-facility-org-select"}
options={organizationOptions}
onChange={(val) => {
onSelectOrg(val);
}}
disabled={loading}
required={true}
Copy link
Contributor

Choose a reason for hiding this comment

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

The required true was needed to pass a11y scan. Does the combobox handle the required in a different way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Truss doesn't give us a required prop in their out-of-the-box component, so I added our custom required UI in the heading piece. The automated scan with non-pro Axe tools didn't surface any issues but if you see one with your tools lmk and I'm happy to try and fix it.

Screenshot 2023-09-06 at 9 13 54 AM

Worth calling out that we might want to add new component we can extend with things like the required status since I'm guessing this won't be the last time this comes up. If folks are in agreement that this is useful happy to refactor the code a bit to make that extensible

Copy link
Contributor

Choose a reason for hiding this comment

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

@BobanL do you remember how you found the issue 6408?. Want to make sure we don't need the required at the input level before resolving this combo! :D

Copy link
Contributor

Choose a reason for hiding this comment

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

This happens when you test this as a form. I ran through it and it looks like it's still being flagged. I think for the purpose of accessibility we might need to figure out how to add required.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh actually Truss gives us an escape hatch for this. Will push a fix soon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2023-09-06 at 4 35 30 PM

The latest commit adds to prop but if someone could run the scan for me again would appreciate it!

Copy link
Contributor

Choose a reason for hiding this comment

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

Tested it with axe and didn't find any problems!

Copy link
Collaborator

Choose a reason for hiding this comment

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

When I ran it on the latest changes I saw this error but am ignoring because I believe this is a false positive jsyk
Screenshot 2023-09-07 at 09 42 39

ref={orgRef}
/>
</div>
<div className="desktop:grid-col-4 tablet:grid-col-4 mobile:grid-col-1">
<Dropdown
label="Testing facility"
options={[
{
label: "- Select -",
value: "",
},
...facilityOptions,
]}
onChange={onSelectFacility}
selectedValue={manageFacilityState.facilityId}
<div
data-testid={"facility-selection-container"}
className="desktop:grid-col-4 tablet:grid-col-4 mobile:grid-col-1 margin-top-1em"
>
<Label htmlFor="manage-facility-facility-select">
Testing facility <Required />
</Label>

<ComboBox
name={"Facility"}
id={"manage-facility-facility-select"}
options={facilityOptions}
onChange={(val) => onSelectFacility(val)}
disabled={loading}
required={true}
ref={facilityRef}
/>
</div>
<div className="desktop:grid-col-4 tablet:grid-col-4 mobile:grid-col-1">
<Button onClick={onSearch} ariaLabel="Search facility selection">
<div className="desktop:grid-col-4 tablet:grid-col-4 mobile:grid-col-1 ">
<Button
onClick={onSearch}
disabled={manageFacilityState.facilityId === undefined}
ariaLabel="Search facility selection"
>
Search
</Button>
</div>
Expand Down
Loading