Make loading work correctly. WIP tests still throw warnings about upd…

…ating an unmounted component
washtenawIdtreble · Sep 2, 2023 · 46ba343 · 46ba343
1 parent 2a6ae5f
commit 46ba343
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 31 deletions.
diff --git 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
- <button onClick={loadFor10Milliseconds}>START LOADING</button>
+ <button onClick={startLoading}>START LOADING</button>
+ <button onClick={finishLoading}>STOP LOADING</button>
  </>);
 };
diff --git 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<NodeJS.Timeout | undefined> = 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) => {
  <HeadingCheck/>
  <LoadingContextProvider setIsLoading={setIsLoading}>
  <div className={"page-container"}>
- {loadingIndicatorNeeded
- ? "Loading..."
- : children
- }
- <div aria-live={"polite"}
- data-testid={"page-live-region"}>{loadingIndicatorNeeded ? "loading" : ""}
- </div>
+ {loadingIndicatorNeeded && <span>Loading...</span>}
+ {children}
+ <div aria-live={"polite"} data-testid={"page-live-region"}>{liveRegionText}</div>
  </div>
  </LoadingContextProvider>
  </>);

diff --git 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();