From ca6b2b408c9a8b249b070af44e8ba004a2360bfa Mon Sep 17 00:00:00 2001 From: JC Quirin <2581274+jcq@users.noreply.github.com> Date: Fri, 22 Dec 2023 11:22:51 -0500 Subject: [PATCH 1/7] initial implementation of a LocaleSwitcher component - this wraps the `react-uswds` `LanguageSelector` component - it is functional, but I have some UX concerns - we will likely want to tweak styling --- app/src/components/Header.tsx | 5 ++-- app/src/components/LocaleSwitcher.tsx | 37 +++++++++++++++++++++++++++ app/src/i18n/navigation.ts | 7 +++++ 3 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 app/src/components/LocaleSwitcher.tsx create mode 100644 app/src/i18n/navigation.ts diff --git a/app/src/components/Header.tsx b/app/src/components/Header.tsx index 10a93db4..a265bd23 100644 --- a/app/src/components/Header.tsx +++ b/app/src/components/Header.tsx @@ -8,6 +8,7 @@ import { Title, Header as USWDSHeader, } from "@trussworks/react-uswds"; +import LocaleSwitcher from "./LocaleSwitcher"; const primaryLinks = [ { @@ -28,11 +29,11 @@ const Header = () => { setIsMobileNavExpanded(!isMobileNavExpanded); }; - const navItems = primaryLinks.map((link) => ( + const navItems = [...primaryLinks.map((link) => ( {t(link.i18nKey)} - )); + )), ]; return ( <> diff --git a/app/src/components/LocaleSwitcher.tsx b/app/src/components/LocaleSwitcher.tsx new file mode 100644 index 00000000..32a58612 --- /dev/null +++ b/app/src/components/LocaleSwitcher.tsx @@ -0,0 +1,37 @@ +"use client"; + +import { usePathname, useRouter } from "src/i18n/navigation"; + +import { useLocale } from "next-intl"; +import { useTransition } from "react"; +import { LanguageDefinition, LanguageSelector } from "@trussworks/react-uswds"; + +export default function LocaleSwitcher() { + const locale = useLocale(); + const router = useRouter(); + const [, startTransition] = useTransition(); + const pathname = usePathname(); + + const selectLocale = (newLocale: string) => { + startTransition(() => { + router.replace(pathname, { locale: newLocale }); + }); + }; + + const langs: LanguageDefinition[] = [ + { + attr: "en-US", + label: "English", + on_click: () => selectLocale("es-US"), + }, + { + attr: "es-US", + label: "Español", + label_local: "Spanish", + on_click: () => selectLocale("en-US"), + }, + ]; + + + return ; +} diff --git a/app/src/i18n/navigation.ts b/app/src/i18n/navigation.ts new file mode 100644 index 00000000..2e286141 --- /dev/null +++ b/app/src/i18n/navigation.ts @@ -0,0 +1,7 @@ +import {createSharedPathnamesNavigation} from 'next-intl/navigation'; + +export const locales = ['en', 'de'] as const; +export const localePrefix = 'always'; // Default + +export const {Link, redirect, usePathname, useRouter} = + createSharedPathnamesNavigation({locales, localePrefix}); \ No newline at end of file From eab4c4d602799f0cbd6698625f24bd9f88c13b2c Mon Sep 17 00:00:00 2001 From: JC Quirin <2581274+jcq@users.noreply.github.com> Date: Thu, 4 Jan 2024 10:43:11 -0500 Subject: [PATCH 2/7] added utils for mocking next/router and next/navigation --- app/package-lock.json | 20 +++++++++++++++-- app/package.json | 1 + app/tests/jest.setup.ts | 2 ++ app/tests/next-router-utils.ts | 40 ++++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 app/tests/next-router-utils.ts diff --git a/app/package-lock.json b/app/package-lock.json index 5b91fc53..93afbda8 100644 --- a/app/package-lock.json +++ b/app/package-lock.json @@ -47,6 +47,7 @@ "jest-axe": "^8.0.0", "jest-cli": "^29.5.0", "jest-environment-jsdom": "^29.5.0", + "next-router-mock": "^0.9.10", "postcss": "^8.4.31", "postcss-loader": "^7.1.0", "postcss-preset-env": "^9.0.0", @@ -15926,7 +15927,6 @@ "version": "2.3.2", "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.2.tgz", "integrity": "sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==", - "dev": true, "hasInstallScript": true, "optional": true, "os": [ @@ -20484,6 +20484,16 @@ "react": "^16.8.0 || ^17.0.0 || ^18.0.0" } }, + "node_modules/next-router-mock": { + "version": "0.9.10", + "resolved": "https://registry.npmjs.org/next-router-mock/-/next-router-mock-0.9.10.tgz", + "integrity": "sha512-bK6sRb/xGNFgHVUZuvuApn6KJBAKTPiP36A7a9mO77U4xQO5ukJx9WHlU67Tv8AuySd09pk0+Hu8qMVIAmLO6A==", + "dev": true, + "peerDependencies": { + "next": ">=10.0.0", + "react": ">=17.0.0" + } + }, "node_modules/no-case": { "version": "3.0.4", "resolved": "https://registry.npmjs.org/no-case/-/no-case-3.0.4.tgz", @@ -37603,7 +37613,6 @@ "version": "2.3.2", "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.2.tgz", "integrity": "sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==", - "dev": true, "optional": true }, "function-bind": { @@ -40894,6 +40903,13 @@ "use-intl": "^3.2.1" } }, + "next-router-mock": { + "version": "0.9.10", + "resolved": "https://registry.npmjs.org/next-router-mock/-/next-router-mock-0.9.10.tgz", + "integrity": "sha512-bK6sRb/xGNFgHVUZuvuApn6KJBAKTPiP36A7a9mO77U4xQO5ukJx9WHlU67Tv8AuySd09pk0+Hu8qMVIAmLO6A==", + "dev": true, + "requires": {} + }, "no-case": { "version": "3.0.4", "resolved": "https://registry.npmjs.org/no-case/-/no-case-3.0.4.tgz", diff --git a/app/package.json b/app/package.json index 6275cde1..93dbb3c5 100644 --- a/app/package.json +++ b/app/package.json @@ -57,6 +57,7 @@ "jest-axe": "^8.0.0", "jest-cli": "^29.5.0", "jest-environment-jsdom": "^29.5.0", + "next-router-mock": "^0.9.10", "postcss": "^8.4.31", "postcss-loader": "^7.1.0", "postcss-preset-env": "^9.0.0", diff --git a/app/tests/jest.setup.ts b/app/tests/jest.setup.ts index ce69f56e..ddda8846 100644 --- a/app/tests/jest.setup.ts +++ b/app/tests/jest.setup.ts @@ -1,5 +1,7 @@ import "@testing-library/jest-dom"; +import './next-router-utils' + import { toHaveNoViolations } from "jest-axe"; expect.extend(toHaveNoViolations); diff --git a/app/tests/next-router-utils.ts b/app/tests/next-router-utils.ts new file mode 100644 index 00000000..e51c6c39 --- /dev/null +++ b/app/tests/next-router-utils.ts @@ -0,0 +1,40 @@ +/* eslint-disable */ + +// Taken from https://github.com/vercel/next.js/discussions/42527#discussioncomment-7234041 +// This mocks important pieces from both next/router and next/navigation to enable testing of components +// that use next/router and next/navigation hooks. + +import mockRouter from "next-router-mock"; +import { createDynamicRouteParser } from "next-router-mock/dynamic-routes"; + +jest.mock("next/router", () => jest.requireActual("next-router-mock")); + +mockRouter.useParser( + createDynamicRouteParser([ + // @see https://github.com/scottrippey/next-router-mock#dynamic-routes + ]) +); + +jest.mock("next/navigation", () => { + const actual = jest.requireActual("next/navigation"); + const nextRouterMock = jest.requireActual("next-router-mock"); + const { useRouter } = nextRouterMock; + const usePathname = jest.fn().mockImplementation(() => { + const router = useRouter(); + return router.asPath; + }); + + const useSearchParams = jest.fn().mockImplementation(() => { + const router = useRouter(); + return new URLSearchParams(router.query); + }); + + return { + ...actual, + useRouter: jest.fn().mockImplementation(useRouter), + usePathname, + useSearchParams, + }; +}); + +export { mockRouter }; \ No newline at end of file From 26f29bf0d3ac71c8962e11d4aa7a7e89cf618650 Mon Sep 17 00:00:00 2001 From: JC Quirin <2581274+jcq@users.noreply.github.com> Date: Thu, 4 Jan 2024 14:45:07 -0500 Subject: [PATCH 3/7] added comments to LocaleSwitcher added test for LocaleSwitcher --- app/src/components/LocaleSwitcher.tsx | 12 ++++++-- app/tests/components/LocaleSwitcher.test.tsx | 29 ++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 app/tests/components/LocaleSwitcher.test.tsx diff --git a/app/src/components/LocaleSwitcher.tsx b/app/src/components/LocaleSwitcher.tsx index 32a58612..b1b14a54 100644 --- a/app/src/components/LocaleSwitcher.tsx +++ b/app/src/components/LocaleSwitcher.tsx @@ -18,16 +18,22 @@ export default function LocaleSwitcher() { }); }; + // This should be modified to fit the project's language requirements + // If you have more than two languages, it will render as a dropdown + // The react-uswds component will just display the `label` for the current language; + // USWDS guidance is to display "Language" in the current language as the label, which isn't currently possible + // See https://designsystem.digital.gov/components/language-selector/ + // We're using two languages by default here, but implementing such that it displays the language to which it will switch rather than the current language const langs: LanguageDefinition[] = [ { attr: "en-US", - label: "English", + label: "Español", + label_local: "Spanish", on_click: () => selectLocale("es-US"), }, { attr: "es-US", - label: "Español", - label_local: "Spanish", + label: "English", on_click: () => selectLocale("en-US"), }, ]; diff --git a/app/tests/components/LocaleSwitcher.test.tsx b/app/tests/components/LocaleSwitcher.test.tsx new file mode 100644 index 00000000..09c54e3a --- /dev/null +++ b/app/tests/components/LocaleSwitcher.test.tsx @@ -0,0 +1,29 @@ +import userEvent from "@testing-library/user-event"; +import { mockRouter } from "tests/next-router-utils"; +import { render, screen } from "tests/react-utils"; + +import LocaleSwitcher from "src/components/LocaleSwitcher"; + +describe("LocaleSwitcher", () => { + it("renders the language selector and updates routes when switching language", async () => { + // Set the initial url + await mockRouter.push("/initial-path"); + + render(); + + // We start in English, so we see the toggle to switch to Spanish + await userEvent.click(screen.getByRole("button", { name: "Español" })); + + // Ensure the router was updated + // This is the best we can do for testing given constraints listed below + expect(mockRouter).toMatchObject({ + asPath: "/es-US/initial-path", + pathname: "/es-US/initial-path", + }); + + // This won't actually work because the NextIntlProvider relies on middleware that isn't available in tests + // expect( + // await screen.findByRole("button", { name: "English" }) + // ).toBeInTheDocument(); + }); +}); From 9f33c6e2e9e7a3872808da593e6c20f46882937e Mon Sep 17 00:00:00 2001 From: JC Quirin <2581274+jcq@users.noreply.github.com> Date: Thu, 4 Jan 2024 17:06:46 -0500 Subject: [PATCH 4/7] code formatting --- app/src/components/Header.tsx | 14 +++++--- app/src/components/LocaleSwitcher.tsx | 1 - app/src/i18n/navigation.ts | 14 ++++---- app/tests/jest.setup.ts | 3 +- app/tests/next-router-utils.ts | 46 +++++++++++++-------------- 5 files changed, 40 insertions(+), 38 deletions(-) diff --git a/app/src/components/Header.tsx b/app/src/components/Header.tsx index a265bd23..ec8ad65c 100644 --- a/app/src/components/Header.tsx +++ b/app/src/components/Header.tsx @@ -8,6 +8,7 @@ import { Title, Header as USWDSHeader, } from "@trussworks/react-uswds"; + import LocaleSwitcher from "./LocaleSwitcher"; const primaryLinks = [ @@ -29,11 +30,14 @@ const Header = () => { setIsMobileNavExpanded(!isMobileNavExpanded); }; - const navItems = [...primaryLinks.map((link) => ( - - {t(link.i18nKey)} - - )), ]; + const navItems = [ + ...primaryLinks.map((link) => ( + + {t(link.i18nKey)} + + )), + , + ]; return ( <> diff --git a/app/src/components/LocaleSwitcher.tsx b/app/src/components/LocaleSwitcher.tsx index b1b14a54..eb8d9234 100644 --- a/app/src/components/LocaleSwitcher.tsx +++ b/app/src/components/LocaleSwitcher.tsx @@ -38,6 +38,5 @@ export default function LocaleSwitcher() { }, ]; - return ; } diff --git a/app/src/i18n/navigation.ts b/app/src/i18n/navigation.ts index 2e286141..951d5840 100644 --- a/app/src/i18n/navigation.ts +++ b/app/src/i18n/navigation.ts @@ -1,7 +1,7 @@ -import {createSharedPathnamesNavigation} from 'next-intl/navigation'; - -export const locales = ['en', 'de'] as const; -export const localePrefix = 'always'; // Default - -export const {Link, redirect, usePathname, useRouter} = - createSharedPathnamesNavigation({locales, localePrefix}); \ No newline at end of file +import { createSharedPathnamesNavigation } from "next-intl/navigation"; + +export const locales = ["en", "de"] as const; +export const localePrefix = "always"; // Default + +export const { Link, redirect, usePathname, useRouter } = + createSharedPathnamesNavigation({ locales, localePrefix }); diff --git a/app/tests/jest.setup.ts b/app/tests/jest.setup.ts index ddda8846..605240e7 100644 --- a/app/tests/jest.setup.ts +++ b/app/tests/jest.setup.ts @@ -1,6 +1,5 @@ import "@testing-library/jest-dom"; - -import './next-router-utils' +import "./next-router-utils"; import { toHaveNoViolations } from "jest-axe"; diff --git a/app/tests/next-router-utils.ts b/app/tests/next-router-utils.ts index e51c6c39..4d421eda 100644 --- a/app/tests/next-router-utils.ts +++ b/app/tests/next-router-utils.ts @@ -10,31 +10,31 @@ import { createDynamicRouteParser } from "next-router-mock/dynamic-routes"; jest.mock("next/router", () => jest.requireActual("next-router-mock")); mockRouter.useParser( - createDynamicRouteParser([ - // @see https://github.com/scottrippey/next-router-mock#dynamic-routes - ]) + createDynamicRouteParser([ + // @see https://github.com/scottrippey/next-router-mock#dynamic-routes + ]) ); jest.mock("next/navigation", () => { - const actual = jest.requireActual("next/navigation"); - const nextRouterMock = jest.requireActual("next-router-mock"); - const { useRouter } = nextRouterMock; - const usePathname = jest.fn().mockImplementation(() => { - const router = useRouter(); - return router.asPath; - }); - - const useSearchParams = jest.fn().mockImplementation(() => { - const router = useRouter(); - return new URLSearchParams(router.query); - }); - - return { - ...actual, - useRouter: jest.fn().mockImplementation(useRouter), - usePathname, - useSearchParams, - }; + const actual = jest.requireActual("next/navigation"); + const nextRouterMock = jest.requireActual("next-router-mock"); + const { useRouter } = nextRouterMock; + const usePathname = jest.fn().mockImplementation(() => { + const router = useRouter(); + return router.asPath; + }); + + const useSearchParams = jest.fn().mockImplementation(() => { + const router = useRouter(); + return new URLSearchParams(router.query); + }); + + return { + ...actual, + useRouter: jest.fn().mockImplementation(useRouter), + usePathname, + useSearchParams, + }; }); -export { mockRouter }; \ No newline at end of file +export { mockRouter }; From 519294c3b5f7838d14becb3f9578a9590b634c16 Mon Sep 17 00:00:00 2001 From: JC Quirin <2581274+jcq@users.noreply.github.com> Date: Fri, 5 Jan 2024 07:57:58 -0500 Subject: [PATCH 5/7] style workarounds for extra class on LanguageSelector --- app/src/components/LocaleSwitcher.tsx | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/app/src/components/LocaleSwitcher.tsx b/app/src/components/LocaleSwitcher.tsx index eb8d9234..d084671c 100644 --- a/app/src/components/LocaleSwitcher.tsx +++ b/app/src/components/LocaleSwitcher.tsx @@ -3,9 +3,18 @@ import { usePathname, useRouter } from "src/i18n/navigation"; import { useLocale } from "next-intl"; -import { useTransition } from "react"; +import { CSSProperties, useTransition } from "react"; import { LanguageDefinition, LanguageSelector } from "@trussworks/react-uswds"; +// Currently, the `react-uswds` component erroneously sets 'usa-language-container' class +// on both the container and the button, which causes incorrect positioning relative to nav items +const styleFixes: CSSProperties = { + display: "block", + top: "auto", + marginLeft: "auto", + marginTop: "auto", +}; + export default function LocaleSwitcher() { const locale = useLocale(); const router = useRouter(); @@ -38,5 +47,7 @@ export default function LocaleSwitcher() { }, ]; - return ; + return ( + + ); } From a0a9a1acce717afabb4279ea5cf20df5a735cc27 Mon Sep 17 00:00:00 2001 From: JC Quirin <2581274+jcq@users.noreply.github.com> Date: Fri, 5 Jan 2024 11:49:14 -0500 Subject: [PATCH 6/7] added a11y test for LocaleSwitcher this currently fails due to issue w/ `react-uswds` component --- app/tests/components/LocaleSwitcher.test.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/tests/components/LocaleSwitcher.test.tsx b/app/tests/components/LocaleSwitcher.test.tsx index 09c54e3a..81b7dccf 100644 --- a/app/tests/components/LocaleSwitcher.test.tsx +++ b/app/tests/components/LocaleSwitcher.test.tsx @@ -1,4 +1,5 @@ import userEvent from "@testing-library/user-event"; +import { axe } from "jest-axe"; import { mockRouter } from "tests/next-router-utils"; import { render, screen } from "tests/react-utils"; @@ -26,4 +27,13 @@ describe("LocaleSwitcher", () => { // await screen.findByRole("button", { name: "English" }) // ).toBeInTheDocument(); }); + + // This fails when in 2-language mode because the react-uswds component sets aria-controls + // w/o corresponding element in the DOM + it("passes accessibility scan", async () => { + const { container } = render(); + const results = await axe(container); + + expect(results).toHaveNoViolations(); + }); }); From 2182989afbfdc944210f2644d83c4120dcc41f4e Mon Sep 17 00:00:00 2001 From: JC Quirin <2581274+jcq@users.noreply.github.com> Date: Wed, 10 Jan 2024 10:56:40 -0500 Subject: [PATCH 7/7] utilize existing `locales` export for next-intl/navigation --- app/src/i18n/navigation.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/i18n/navigation.ts b/app/src/i18n/navigation.ts index 951d5840..7dea3254 100644 --- a/app/src/i18n/navigation.ts +++ b/app/src/i18n/navigation.ts @@ -1,6 +1,7 @@ import { createSharedPathnamesNavigation } from "next-intl/navigation"; -export const locales = ["en", "de"] as const; +import { locales } from "./config"; + export const localePrefix = "always"; // Default export const { Link, redirect, usePathname, useRouter } =