From 19247fd4a236862e5eccec7fce2500775e78b69d Mon Sep 17 00:00:00 2001 From: Doug Schrashun Date: Wed, 6 Nov 2024 09:16:17 -0500 Subject: [PATCH 1/5] wip --- frontend/src/components/Header.tsx | 19 ++++++++++++++++-- frontend/src/components/search/SearchBar.tsx | 21 +++++++++++++++++--- frontend/src/hooks/useSearchParamUpdater.ts | 1 + 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/frontend/src/components/Header.tsx b/frontend/src/components/Header.tsx index 7292075de..ee78cf668 100644 --- a/frontend/src/components/Header.tsx +++ b/frontend/src/components/Header.tsx @@ -5,6 +5,7 @@ import { assetPath } from "src/utils/assetPath"; import { useTranslations } from "next-intl"; import Link from "next/link"; +import { usePathname, useRouter } from "next/navigation"; import { useEffect, useRef, useState } from "react"; import { GovBanner, @@ -33,8 +34,12 @@ const Header = ({ logoPath, locale }: Props) => { const primaryLinksRef = useRef([]); const { featureFlagsManager } = useFeatureFlags(); + const router = useRouter(); + const path = usePathname(); + console.log("&&& path", path); useEffect(() => { + console.log("&&& on search", path.includes("search")); primaryLinksRef.current = [ { i18nKey: t("nav_link_home"), href: "/" }, { i18nKey: t("nav_link_process"), href: "/process" }, @@ -45,18 +50,28 @@ const Header = ({ logoPath, locale }: Props) => { i18nKey: t("nav_link_search"), href: "/search", }; + if (featureFlagsManager.isFeatureEnabled("showSearchV0")) { primaryLinksRef.current.splice(1, 0, searchNavLink); } - }, [featureFlagsManager, t]); + }, [featureFlagsManager, t, path]); const navItems = primaryLinksRef.current.map((link) => ( - + { + // if (path.includes("search") && link.href.includes("search")) { + // return router.push("/search?refresh=true").then(() => undefined); + // } + // }} + > {link.i18nKey} )); const language = locale && locale.match("/^es/") ? "spanish" : "english"; + console.log("&&& refresh nav items?", navItems); return ( <>
{ updateQueryParams("", "query", queryTerm, false); }; - const t = useTranslations("Search"); + console.log("!!! search bar render query", query); + console.log("!!! search bar render queryTerm", queryTerm); + + // useEffect(() => { + // console.log("CHECKING", searchParams.get("refresh")); + // if (searchParams.get("refresh")) { + // console.log("UPDATDIHNG"); + // updateQueryTerm(""); + // } + // }, [searchParams, updateQueryTerm]); + + // useEffect(() => console.log("***"), []); return (
diff --git a/frontend/src/hooks/useSearchParamUpdater.ts b/frontend/src/hooks/useSearchParamUpdater.ts index 0e65f269f..4ce3bd72c 100644 --- a/frontend/src/hooks/useSearchParamUpdater.ts +++ b/frontend/src/hooks/useSearchParamUpdater.ts @@ -49,6 +49,7 @@ export function useSearchParamUpdater() { }; return { + searchParams, updateQueryParams, }; } From 9d8fd2f9f421ea36b17f20b43b948e3ae19d5bf7 Mon Sep 17 00:00:00 2001 From: Doug Schrashun Date: Wed, 6 Nov 2024 11:49:15 -0500 Subject: [PATCH 2/5] this seems to work --- frontend/src/components/Header.tsx | 114 +++++++++++-------- frontend/src/components/search/SearchBar.tsx | 29 +++-- frontend/src/hooks/useSearchParamUpdater.ts | 3 +- 3 files changed, 83 insertions(+), 63 deletions(-) diff --git a/frontend/src/components/Header.tsx b/frontend/src/components/Header.tsx index ee78cf668..78f5b5f2c 100644 --- a/frontend/src/components/Header.tsx +++ b/frontend/src/components/Header.tsx @@ -5,8 +5,8 @@ import { assetPath } from "src/utils/assetPath"; import { useTranslations } from "next-intl"; import Link from "next/link"; -import { usePathname, useRouter } from "next/navigation"; -import { useEffect, useRef, useState } from "react"; +import { usePathname } from "next/navigation"; +import { useCallback, useMemo, useState } from "react"; import { GovBanner, NavMenuButton, @@ -15,63 +15,84 @@ import { Header as USWDSHeader, } from "@trussworks/react-uswds"; -type PrimaryLinks = { - i18nKey: string; - href: string; -}[]; +type PrimaryLink = { + text?: string; + href?: string; +}; type Props = { logoPath?: string; locale?: string; }; -const Header = ({ logoPath, locale }: Props) => { +const NavLinks = ({ + mobileExpanded, + onToggleMobileNav, +}: { + mobileExpanded: boolean; + onToggleMobileNav: () => unknown; +}) => { const t = useTranslations("Header"); - const [isMobileNavExpanded, setIsMobileNavExpanded] = useState(false); - const handleMobileNavToggle = () => { - setIsMobileNavExpanded(!isMobileNavExpanded); - }; - - const primaryLinksRef = useRef([]); const { featureFlagsManager } = useFeatureFlags(); - const router = useRouter(); const path = usePathname(); - console.log("&&& path", path); - useEffect(() => { - console.log("&&& on search", path.includes("search")); - primaryLinksRef.current = [ - { i18nKey: t("nav_link_home"), href: "/" }, - { i18nKey: t("nav_link_process"), href: "/process" }, - { i18nKey: t("nav_link_research"), href: "/research" }, - { i18nKey: t("nav_link_subscribe"), href: "/subscribe" }, + const getSearchLink = useCallback( + (onSearch: boolean, enabled: boolean) => { + if (!enabled) { + return {}; + } + return { + text: t("nav_link_search"), + href: onSearch ? "/search?refresh=true" : "/search", + }; + }, + [t], + ); + + const navLinkList = useMemo(() => { + return [ + { text: t("nav_link_home"), href: "/" }, + getSearchLink( + path.includes("/search"), + featureFlagsManager.isFeatureEnabled("showSearchV0"), + ), + { text: t("nav_link_process"), href: "/process" }, + { text: t("nav_link_research"), href: "/research" }, + { text: t("nav_link_subscribe"), href: "/subscribe" }, ]; - const searchNavLink = { - i18nKey: t("nav_link_search"), - href: "/search", - }; + }, [t, path, featureFlagsManager, getSearchLink]); - if (featureFlagsManager.isFeatureEnabled("showSearchV0")) { - primaryLinksRef.current.splice(1, 0, searchNavLink); - } - }, [featureFlagsManager, t, path]); + const navItems = useMemo(() => { + return navLinkList.map((link: PrimaryLink) => { + if (!link.text || !link.href) { + return <>; + } + return ( + + {link.text} + + ); + }); + }, [navLinkList]); - const navItems = primaryLinksRef.current.map((link) => ( - { - // if (path.includes("search") && link.href.includes("search")) { - // return router.push("/search?refresh=true").then(() => undefined); - // } - // }} - > - {link.i18nKey} - - )); + return ( + + ); +}; + +const Header = ({ logoPath, locale }: Props) => { + const t = useTranslations("Header"); + const [isMobileNavExpanded, setIsMobileNavExpanded] = + useState(false); + const handleMobileNavToggle = () => { + setIsMobileNavExpanded(!isMobileNavExpanded); + }; const language = locale && locale.match("/^es/") ? "spanish" : "english"; - console.log("&&& refresh nav items?", navItems); return ( <>
{ label={t("nav_menu_toggle")} />
- + />
diff --git a/frontend/src/components/search/SearchBar.tsx b/frontend/src/components/search/SearchBar.tsx index 278bd1f94..dadc600e2 100644 --- a/frontend/src/components/search/SearchBar.tsx +++ b/frontend/src/components/search/SearchBar.tsx @@ -4,17 +4,15 @@ import { QueryContext } from "src/app/[locale]/search/QueryProvider"; import { useSearchParamUpdater } from "src/hooks/useSearchParamUpdater"; import { useTranslations } from "next-intl"; -import { useSearchParams } from "next/navigation"; -import { useContext, useEffect } from "react"; +import { useContext, useEffect, useRef } from "react"; import { Icon } from "@trussworks/react-uswds"; interface SearchBarProps { query: string | null | undefined; } -// what can we look at to determine if there is a page refresh so we can clear the queryTerm -// let's try making the nav take you to like "search-opportunities?navLink=true" which redirects to '/search" but we've already captured the state to know to reset the form???? export default function SearchBar({ query }: SearchBarProps) { + const inputRef = useRef(null); const { queryTerm, updateQueryTerm } = useContext(QueryContext); const { updateQueryParams, searchParams } = useSearchParamUpdater(); const t = useTranslations("Search"); @@ -23,18 +21,18 @@ export default function SearchBar({ query }: SearchBarProps) { updateQueryParams("", "query", queryTerm, false); }; - console.log("!!! search bar render query", query); - console.log("!!! search bar render queryTerm", queryTerm); + useEffect(() => { + if (searchParams.get("refresh") && inputRef.current) { + updateQueryTerm(""); + inputRef.current.value = ""; + } + }, [searchParams, updateQueryTerm]); - // useEffect(() => { - // console.log("CHECKING", searchParams.get("refresh")); - // if (searchParams.get("refresh")) { - // console.log("UPDATDIHNG"); - // updateQueryTerm(""); - // } - // }, [searchParams, updateQueryTerm]); - - // useEffect(() => console.log("***"), []); + useEffect(() => { + if (searchParams.get("refresh") && queryTerm) { + updateQueryParams("", "refresh"); + } + }, [queryTerm, searchParams, updateQueryParams]); return (
@@ -51,6 +49,7 @@ export default function SearchBar({ query }: SearchBarProps) {
, // Key of the parameter. key: string, - queryTerm: string | null | undefined, + queryTerm?: string | null, // Determines whether the state update scrolls the user to the top. This // is useful for components that are expected to be "under the fold." scroll = false, From f2221de869d6108877c736c43f48d99eeb198b8a Mon Sep 17 00:00:00 2001 From: Doug Schrashun Date: Wed, 6 Nov 2024 13:03:47 -0500 Subject: [PATCH 3/5] fix tests --- frontend/tests/components/Header.test.tsx | 16 ++++++++++++++++ .../tests/components/search/SearchBar.test.tsx | 2 ++ frontend/tests/pages/search/page.test.tsx | 8 ++++++++ 3 files changed, 26 insertions(+) diff --git a/frontend/tests/components/Header.test.tsx b/frontend/tests/components/Header.test.tsx index 1f8fb07f5..27f475809 100644 --- a/frontend/tests/components/Header.test.tsx +++ b/frontend/tests/components/Header.test.tsx @@ -1,6 +1,8 @@ import userEvent from "@testing-library/user-event"; import { render, screen } from "tests/react-utils"; +import { ReadonlyURLSearchParams, usePathname } from "next/navigation"; + import Header from "src/components/Header"; const props = { @@ -17,6 +19,20 @@ const props = { ], }; +const mockedPath = "/fakepath"; + +const getMockedPath = () => mockedPath; + +jest.mock("src/hooks/useSearchParamUpdater", () => ({ + useSearchParamUpdater: () => ({ + searchParams: new ReadonlyURLSearchParams(), + }), +})); + +jest.mock("next/navigation", () => ({ + usePathname: () => getMockedPath(), +})); + describe("Header", () => { it("toggles the mobile nav menu", async () => { render(
); diff --git a/frontend/tests/components/search/SearchBar.test.tsx b/frontend/tests/components/search/SearchBar.test.tsx index 5824645e1..36f0a4b1c 100644 --- a/frontend/tests/components/search/SearchBar.test.tsx +++ b/frontend/tests/components/search/SearchBar.test.tsx @@ -5,6 +5,7 @@ import { axe } from "jest-axe"; import QueryProvider from "src/app/[locale]/search/QueryProvider"; import { render, screen } from "tests/react-utils"; +import { ReadonlyURLSearchParams } from "next/navigation"; import React from "react"; import SearchBar from "src/components/search/SearchBar"; @@ -15,6 +16,7 @@ const mockUpdateQueryParams = jest.fn(); jest.mock("src/hooks/useSearchParamUpdater", () => ({ useSearchParamUpdater: () => ({ updateQueryParams: mockUpdateQueryParams, + searchParams: new ReadonlyURLSearchParams(), }), })); diff --git a/frontend/tests/pages/search/page.test.tsx b/frontend/tests/pages/search/page.test.tsx index fd9d127df..37a3c0e40 100644 --- a/frontend/tests/pages/search/page.test.tsx +++ b/frontend/tests/pages/search/page.test.tsx @@ -4,6 +4,8 @@ import Search from "src/app/[locale]/search/page"; import { SEARCH_NO_STATUS_VALUE } from "src/constants/search"; import { useTranslationsMock } from "src/utils/testing/intlMocks"; +import { ReadonlyURLSearchParams } from "next/navigation"; + // test without feature flag functionality jest.mock("src/hoc/search/withFeatureFlag", () => jest.fn((Component: React.Component) => Component), @@ -18,6 +20,12 @@ jest.mock("next-intl", () => ({ useTranslations: () => useTranslationsMock(), })); +jest.mock("src/hooks/useSearchParamUpdater", () => ({ + useSearchParamUpdater: () => ({ + searchParams: new ReadonlyURLSearchParams(), + }), +})); + // // currently, with Suspense mocked out below to always show fallback content, // // the components making the fetch calls are never being rendered so we do not need to mock them out // // uncomment this if we figure out a way to properly test the underlying async components From 5b30855c8e00094ae1e820ec0355ebf98a723258 Mon Sep 17 00:00:00 2001 From: Doug Schrashun Date: Wed, 6 Nov 2024 13:15:08 -0500 Subject: [PATCH 4/5] remove feature flag on search, add all-checks job --- frontend/package.json | 1 + frontend/src/components/Header.tsx | 14 +++----------- frontend/tests/components/Header.test.tsx | 2 +- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/frontend/package.json b/frontend/package.json index 32937c8f7..d958c762f 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -6,6 +6,7 @@ "node": ">=20.0.0" }, "scripts": { + "all-checks": "npm run test && npm run lint && npm run ts:check && npm run build", "build": "next build", "dev": "next dev", "debug": "NODE_OPTIONS='--inspect' next dev", diff --git a/frontend/src/components/Header.tsx b/frontend/src/components/Header.tsx index 78f5b5f2c..fb7874e83 100644 --- a/frontend/src/components/Header.tsx +++ b/frontend/src/components/Header.tsx @@ -1,6 +1,5 @@ "use client"; -import { useFeatureFlags } from "src/hooks/useFeatureFlags"; import { assetPath } from "src/utils/assetPath"; import { useTranslations } from "next-intl"; @@ -33,14 +32,10 @@ const NavLinks = ({ onToggleMobileNav: () => unknown; }) => { const t = useTranslations("Header"); - const { featureFlagsManager } = useFeatureFlags(); const path = usePathname(); const getSearchLink = useCallback( - (onSearch: boolean, enabled: boolean) => { - if (!enabled) { - return {}; - } + (onSearch: boolean) => { return { text: t("nav_link_search"), href: onSearch ? "/search?refresh=true" : "/search", @@ -52,15 +47,12 @@ const NavLinks = ({ const navLinkList = useMemo(() => { return [ { text: t("nav_link_home"), href: "/" }, - getSearchLink( - path.includes("/search"), - featureFlagsManager.isFeatureEnabled("showSearchV0"), - ), + getSearchLink(path.includes("/search")), { text: t("nav_link_process"), href: "/process" }, { text: t("nav_link_research"), href: "/research" }, { text: t("nav_link_subscribe"), href: "/subscribe" }, ]; - }, [t, path, featureFlagsManager, getSearchLink]); + }, [t, path, getSearchLink]); const navItems = useMemo(() => { return navLinkList.map((link: PrimaryLink) => { diff --git a/frontend/tests/components/Header.test.tsx b/frontend/tests/components/Header.test.tsx index 27f475809..7d20c3078 100644 --- a/frontend/tests/components/Header.test.tsx +++ b/frontend/tests/components/Header.test.tsx @@ -1,7 +1,7 @@ import userEvent from "@testing-library/user-event"; import { render, screen } from "tests/react-utils"; -import { ReadonlyURLSearchParams, usePathname } from "next/navigation"; +import { ReadonlyURLSearchParams } from "next/navigation"; import Header from "src/components/Header"; From a1ac516c29647770b943ea72ba45430b3c1f5be4 Mon Sep 17 00:00:00 2001 From: Doug Schrashun Date: Wed, 6 Nov 2024 13:52:47 -0500 Subject: [PATCH 5/5] add header test --- frontend/src/components/search/SearchBar.tsx | 7 +++-- frontend/tests/components/Header.test.tsx | 30 +++++++++++++------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/frontend/src/components/search/SearchBar.tsx b/frontend/src/components/search/SearchBar.tsx index dadc600e2..ba6917ac6 100644 --- a/frontend/src/components/search/SearchBar.tsx +++ b/frontend/src/components/search/SearchBar.tsx @@ -21,6 +21,8 @@ export default function SearchBar({ query }: SearchBarProps) { updateQueryParams("", "query", queryTerm, false); }; + // if we have "refresh=true" query param, clear the input + // this supports the expected refresh of the input if the user clicks the search link while on the search page useEffect(() => { if (searchParams.get("refresh") && inputRef.current) { updateQueryTerm(""); @@ -28,11 +30,12 @@ export default function SearchBar({ query }: SearchBarProps) { } }, [searchParams, updateQueryTerm]); + // removes the "refresh" param once a user has dirtied the input useEffect(() => { - if (searchParams.get("refresh") && queryTerm) { + if (searchParams.get("refresh") && inputRef.current?.value) { updateQueryParams("", "refresh"); } - }, [queryTerm, searchParams, updateQueryParams]); + }, [searchParams, updateQueryParams]); return (
diff --git a/frontend/tests/components/Header.test.tsx b/frontend/tests/components/Header.test.tsx index 7d20c3078..371bdf9bf 100644 --- a/frontend/tests/components/Header.test.tsx +++ b/frontend/tests/components/Header.test.tsx @@ -7,19 +7,10 @@ import Header from "src/components/Header"; const props = { logoPath: "/img/logo.svg", - primaryLinks: [ - { - i18nKey: "nav_link_home", - href: "/", - }, - { - i18nKey: "nav_link_health", - href: "/health", - }, - ], + locale: "en", }; -const mockedPath = "/fakepath"; +let mockedPath = "/fakepath"; const getMockedPath = () => mockedPath; @@ -66,4 +57,21 @@ describe("Header", () => { expect(govBanner).toHaveAttribute("aria-expanded", "true"); }); + + it("displays a search link without refresh param if not currently on search page", () => { + render(
); + + const searchLink = screen.getByRole("link", { name: "Search" }); + expect(searchLink).toBeInTheDocument(); + expect(searchLink).toHaveAttribute("href", "/search"); + }); + + it("displays a search link with refresh param if currently on search page", () => { + mockedPath = "/search"; + render(
); + + const searchLink = screen.getByRole("link", { name: "Search" }); + expect(searchLink).toBeInTheDocument(); + expect(searchLink).toHaveAttribute("href", "/search?refresh=true"); + }); });