Implement LocaleSwitcher Component #273
Conversation
- this wraps the `react-uswds` `LanguageSelector` component - it is functional, but I have some UX concerns - we will likely want to tweak styling
added test for LocaleSwitcher
this currently fails due to issue w/ `react-uswds` component
| // 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(); | ||
| }); |
There was a problem hiding this comment.
Left this in to note an a11y issue with the current version of the react-uswds component.
| 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 |
There was a problem hiding this comment.
Could we open a PR to fix that for them? They're usually pretty open to contributions.
There was a problem hiding this comment.
app/src/i18n/navigation.ts
Outdated
There was a problem hiding this comment.
Is this file needed? I think the middleware handles prefixing and automatically redirecting to the expected locale prefix based on the preferred locale. The localePrefix setting in this file also looks different than what's set in the middleware.
There was a problem hiding this comment.
This file is for re-exporting some enhanced versions of NextJS navigation functions. Its usage is documented here:
https://next-intl-docs.vercel.app/docs/routing/navigation#shared-pathnames
That said, I totally forgot to update to our actual locales and instead just copy-pasted what they had in the docs. 😓
I'm updating it to just pull locales from the existing export so they are only defined once.
I'm not entirely certain of their reasons for keeping this as its own file vs simply adding these exports to config.ts. My guess is that it is largely for slightly clearer code since it's specifically re-exporting stuff from next/navigation. I'm sort of ambivalent as to where we'd prefer to put it. 🤷 Thoughts?
There was a problem hiding this comment.
Is it accurate to say that these utilities are only needed for the locale switcher component?
It seems like the language is preserved fine when using next/navigation for routing elsewhere, so am wondering if these could be co-located in the component's file.
I was able to test usage of normal next/navigation routing by adding the following to the homepage and observing the URL and header's translated text. Seems to work:
// TestButton.tsx
"use client";
import { useRouter } from "next/navigation";
export default function TestButton() {
const router = useRouter();
return (
<button onClick={() => router.push("/health")}>Go to health page</button>
);
}CleanShot.2024-01-11.at.10.17.45.mp4
Ticket
Resolves #237
Changes
LocaleSwitchercomponent and added it as the final item in the nav inHeaderreact-uswdsLanguageSelectorcomponentreact-uswdscomponent erroneously applying.usa-language-containerclass to button as well as wrapping divreact-uswdscomponent erroneously applying aaria-controlsprop even in dual-language mode when the corresponding element isn't present (it is in dropdown/multiple-language mode)Context for reviewers
This can be seen in the nav when running the app in localhost. However, there are multiple issues with the implementation from
react-uswdsthat make me think we may want to re-implement our own until such time as that version gets updated. I do also still have some UX concerns over both the basic USWDS implementation and the specific one fromreact-uswds.Testing