From 46ba343accd8e07e4084ada412e18b6588aab523 Mon Sep 17 00:00:00 2001 From: Ryan Heisler Date: Sat, 2 Sep 2023 17:29:02 -0400 Subject: [PATCH] Make loading work correctly. WIP tests still throw warnings about updating an unmounted component --- src/pages/Page.test.tsx | 57 ++++++++++++++++++++++++++--------------- src/pages/Page.tsx | 23 +++++++++-------- src/setupTests.ts | 1 + 3 files changed, 50 insertions(+), 31 deletions(-) diff --git a/src/pages/Page.test.tsx b/src/pages/Page.test.tsx index 8fca4d5..02cd81a 100644 --- a/src/pages/Page.test.tsx +++ b/src/pages/Page.test.tsx @@ -73,7 +73,7 @@ describe(Page.name, () => { liveRegion = screen.getByTestId("page-live-region"); }); - test("adds loading text to live region after timeout", async () => { + test("anounces 'loading' after a timeout", async () => { await user.click(screen.getByRole("button", { name: "START LOADING" })); const loadingText = await within(liveRegion).findByText("loading"); expect(loadingText).toBeInTheDocument(); @@ -85,30 +85,51 @@ describe(Page.name, () => { expect(loadingText).toBeVisible(); }); + test("Hides loading indicator when loading finishes", async () => { + await user.click(screen.getByRole("button", { name: "START LOADING" })); + await asyncTimeout(parseInt(process.env.REACT_APP_LOADING_TIMEOUT!) * 2); + await user.click(screen.getByRole("button", { name: "STOP LOADING" })); + + expect(screen.queryByText("Loading...")).not.toBeInTheDocument(); + + await waitFor(() => { + expect(liveRegion.textContent).toEqual(""); + }); + }); + + test("Announces 'finished loading' when loading finishes", async () => { + await user.click(screen.getByRole("button", { name: "START LOADING" })); + await asyncTimeout(parseInt(process.env.REACT_APP_LOADING_TIMEOUT!) * 2); + await user.click(screen.getByRole("button", { name: "STOP LOADING" })); + + const finishedLoadingAnnouncment = await within(liveRegion).findByText("finished loading"); + expect(finishedLoadingAnnouncment).toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "START LOADING" })); + + await waitFor(() => { + expect(liveRegion.textContent).toEqual(""); + }); + }); + test("does not show loading text or indicator if loading finished within the timeout period", async () => { const originalAppLoadingTimeout = process.env.REACT_APP_LOADING_TIMEOUT; - process.env.REACT_APP_LOADING_TIMEOUT = "250"; + process.env.REACT_APP_LOADING_TIMEOUT = "125"; await user.click(screen.getByRole("button", { name: "START LOADING" })); + await user.click(screen.getByRole("button", { name: "STOP LOADING" })); - await asyncTimeout(parseInt(process.env.REACT_APP_LOADING_TIMEOUT!) * 2); - - const loadingText = within(liveRegion).queryByText("loading"); + const loadingText = within(liveRegion).queryByText("loading", { exact: true }); expect(loadingText).not.toBeInTheDocument(); + const finishedLoadingText = within(liveRegion).queryByText("finished loading"); + expect(finishedLoadingText).not.toBeInTheDocument(); + const loadingIndicator = screen.queryByText("Loading..."); expect(loadingIndicator).not.toBeInTheDocument(); - expect(screen.getByText("children")).toBeVisible(); - process.env.REACT_APP_LOADING_TIMEOUT = originalAppLoadingTimeout; }); - - test("Hides loading indicator and shows children when loading finishes", async () => { - await user.click(screen.getByRole("button", { name: "START LOADING" })); - - expect((await screen.findByText("children"))).toBeVisible(); - }); }); describe("under error conditions", () => { @@ -176,15 +197,9 @@ const ContextProvider = ({ children, freshDocument }: { freshDocument: boolean } const ChildComponentThatLoadsData = () => { const { startLoading, finishLoading } = useContext(LoadingContext); - const loadFor10Milliseconds = useCallback(() => { - startLoading(); - setTimeout(() => { - finishLoading(); - }, 10); - }, [startLoading, finishLoading]); - return (<> children - + + ); }; diff --git a/src/pages/Page.tsx b/src/pages/Page.tsx index 1f3de3a..92b4cd2 100644 --- a/src/pages/Page.tsx +++ b/src/pages/Page.tsx @@ -16,19 +16,26 @@ export const Page = ({ title, children }: PageProps) => { const { documentHasBeenLoaded } = useContext(DocumentStateContext); const [loadingIndicatorNeeded, setLoadingIndicatorNeeded] = useState(false); const loadingTimerRef: React.MutableRefObject = useRef(); + const [liveRegionText, setLiveRegionText] = useState(""); const setIsLoading = useCallback((isLoading: boolean) => { if (isLoading) { loadingTimerRef.current = setTimeout(() => { + setLiveRegionText("loading"); setLoadingIndicatorNeeded(true); }, parseInt(process.env.REACT_APP_LOADING_TIMEOUT!)); } else { - if (loadingIndicatorNeeded) { - setLoadingIndicatorNeeded(false); - } if (loadingTimerRef.current) { clearTimeout(loadingTimerRef.current); } + if (loadingIndicatorNeeded) { + setLiveRegionText("finished loading"); + setLoadingIndicatorNeeded(false); + + setTimeout(() => { + setLiveRegionText(""); + }, parseInt(process.env.REACT_APP_LIVE_REGION_CLEAR_TIMEOUT!)); + } } }, [loadingIndicatorNeeded]); @@ -46,13 +53,9 @@ export const Page = ({ title, children }: PageProps) => {
- {loadingIndicatorNeeded - ? "Loading..." - : children - } -
{loadingIndicatorNeeded ? "loading" : ""} -
+ {loadingIndicatorNeeded && Loading...} + {children} +
{liveRegionText}
); diff --git a/src/setupTests.ts b/src/setupTests.ts index b2a393f..a25c705 100644 --- a/src/setupTests.ts +++ b/src/setupTests.ts @@ -10,6 +10,7 @@ import { toHaveNoViolations } from "jest-axe"; process.env.REACT_APP_API = "test"; process.env.REACT_APP_FOCUS_TIMEOUT = "10"; process.env.REACT_APP_LOADING_TIMEOUT = "10"; +process.env.REACT_APP_LIVE_REGION_CLEAR_TIMEOUT = "10"; setupTestingServer();