fix(water-log): address code-review findings from i18n PR
Deploy to route.crispygoat.com / deploy (push) Successful in 4m9s
Deploy to route.crispygoat.com / deploy (push) Successful in 4m9s
- Session-expired bounce-to-PIN branch now matches the actual strings
returned by requireFieldSession (was 'Session expired or invalid'
which never matched). Added 'Session not found' and 'User is inactive'
to the intercept list. The user-visible 'Your session expired. Please
sign in again.' string is now in the dictionary (en + es).
- LangProvider moved up to WaterFieldClient. The orchestrator (which
was sitting OUTSIDE the provider in b29caa0) now calls useLang()
directly, so the loadHeadgates fallback error string is always
rendered in the user's chosen language — even if they toggled
after signing in.
- getClientLangSnapshot now memoizes the cookie value (React 19
enforces getSnapshot purity stricter than 18).
- LanguageToggle switched from aria-pressed (toggle pattern) to
role="radiogroup" + role="radio" + aria-checked (mutually-exclusive
selector pattern). Per-button aria-labels dropped — the visible
EN/ES text is read by SR as the radio name. Group label switched
to t.common.a11yLanguage for Spanish parity.
- Notes textarea now has aria-label={t.log.notes} (was unlabeled —
SR would announce 'Edit text' with no context).
- 1 new test: lock in that wl_lang=<script>...</script> falls through
to the navigator (injection guard on the cookie regex).
Refs /tmp/refactor-water-log.md (review followups to Commit B)
This commit is contained in:
@@ -7,20 +7,32 @@
|
||||
* Used in both the mobile app (header of the headgate list) and
|
||||
* the admin client (settings page).
|
||||
*
|
||||
* ARIA: implemented as a `radiogroup` with two `radio` items. We
|
||||
* intentionally avoid the `aria-pressed` toggle pattern because the
|
||||
* two buttons are mutually exclusive — pressing one deselects the
|
||||
* other. The `radiogroup` pattern communicates that to assistive
|
||||
* tech and matches the segmented visual intent.
|
||||
*
|
||||
* The pill is intentionally narrow so it can fit in a sticky nav bar
|
||||
* next to the logout icon. The active button has the brand-green
|
||||
* background; the inactive is a subtle pill outline.
|
||||
*/
|
||||
|
||||
import { useLang } from "@/lib/water-log/lang-context";
|
||||
import type { Lang } from "@/lib/water-log/i18n";
|
||||
|
||||
type Props = {
|
||||
/** Visual size. `sm` fits inside a 44pt iOS-style nav bar. */
|
||||
size?: "sm" | "md";
|
||||
};
|
||||
|
||||
const LANG_OPTIONS: ReadonlyArray<{ lang: Lang; label: string }> = [
|
||||
{ lang: "en", label: "EN" },
|
||||
{ lang: "es", label: "ES" },
|
||||
];
|
||||
|
||||
export function LanguageToggle({ size = "sm" }: Props) {
|
||||
const { lang, setLang } = useLang();
|
||||
const { lang, setLang, t } = useLang();
|
||||
|
||||
const isSmall = size === "sm";
|
||||
const containerStyle: React.CSSProperties = {
|
||||
@@ -63,29 +75,23 @@ export function LanguageToggle({ size = "sm" }: Props) {
|
||||
|
||||
return (
|
||||
<div
|
||||
role="group"
|
||||
aria-label="Language"
|
||||
role="radiogroup"
|
||||
aria-label={t.common.a11yLanguage}
|
||||
data-testid="language-toggle"
|
||||
style={containerStyle}
|
||||
>
|
||||
{LANG_OPTIONS.map((opt) => (
|
||||
<button
|
||||
key={opt.lang}
|
||||
type="button"
|
||||
onClick={() => setLang("en")}
|
||||
aria-pressed={lang === "en"}
|
||||
aria-label="English"
|
||||
style={lang === "en" ? activeStyle : inactiveStyle}
|
||||
role="radio"
|
||||
aria-checked={lang === opt.lang}
|
||||
onClick={() => setLang(opt.lang)}
|
||||
style={lang === opt.lang ? activeStyle : inactiveStyle}
|
||||
>
|
||||
EN
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setLang("es")}
|
||||
aria-pressed={lang === "es"}
|
||||
aria-label="Español"
|
||||
style={lang === "es" ? activeStyle : inactiveStyle}
|
||||
>
|
||||
ES
|
||||
{opt.label}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -12,14 +12,47 @@
|
||||
* `/water` flow is now a clean three-screen Apple-HIG experience:
|
||||
* PIN → Headgates → Log
|
||||
*
|
||||
* This client component also owns the `<LangProvider>` wrapping
|
||||
* the tree so `MobileWaterApp` (and any of its descendants,
|
||||
* including the orchestrator's `loadHeadgates` catch branch) can
|
||||
* call `useLang()` directly. The initial language comes from
|
||||
* `useSyncExternalStore` against the `wl_lang` cookie (server
|
||||
* returns `"es"`, client agrees after the first paint).
|
||||
*
|
||||
* The wrapping div uses `100dvh` so the iOS Safari address bar
|
||||
* collapse doesn't trigger layout jumps, and `100vh` as a
|
||||
* fallback for older browsers.
|
||||
*/
|
||||
import { Suspense } from "react";
|
||||
import { Suspense, useSyncExternalStore } from "react";
|
||||
import { MobileWaterApp } from "./mobile/MobileWaterApp";
|
||||
import { LangProvider } from "@/lib/water-log/lang-context";
|
||||
import { detectInitialLang, type Lang } from "@/lib/water-log/i18n";
|
||||
|
||||
const subscribeNoop = () => () => {};
|
||||
const getServerLangSnapshot = (): Lang => "es";
|
||||
|
||||
// Memoize the cookie read so `getSnapshot` always returns the same
|
||||
// reference between renders (React 19 enforces getSnapshot purity for
|
||||
// `useSyncExternalStore` more strictly than 18).
|
||||
let cachedSnapshot: { cookie: string; lang: Lang } | null = null;
|
||||
const getClientLangSnapshot = (): Lang => {
|
||||
if (typeof document === "undefined") return "es";
|
||||
const cookie = document.cookie;
|
||||
if (cachedSnapshot && cachedSnapshot.cookie === cookie) {
|
||||
return cachedSnapshot.lang;
|
||||
}
|
||||
const lang = detectInitialLang();
|
||||
cachedSnapshot = { cookie, lang };
|
||||
return lang;
|
||||
};
|
||||
|
||||
export default function WaterFieldClient() {
|
||||
const lang = useSyncExternalStore(
|
||||
subscribeNoop,
|
||||
getClientLangSnapshot,
|
||||
getServerLangSnapshot,
|
||||
);
|
||||
|
||||
return (
|
||||
<div
|
||||
className="fixed inset-0 bg-[#F2F2F7]"
|
||||
@@ -44,7 +77,9 @@ export default function WaterFieldClient() {
|
||||
</div>
|
||||
}
|
||||
>
|
||||
<LangProvider initialLang={lang}>
|
||||
<MobileWaterApp />
|
||||
</LangProvider>
|
||||
</Suspense>
|
||||
</div>
|
||||
);
|
||||
|
||||
@@ -341,6 +341,7 @@ export function MobileWaterLogForm({
|
||||
rows={3}
|
||||
maxLength={NOTES_MAX}
|
||||
placeholder={t.log.notesPlaceholder}
|
||||
aria-label={t.log.notes}
|
||||
className="min-h-[68px] w-full resize-none bg-transparent text-[16px] leading-[1.4] text-[#1d1d1f] outline-none placeholder:text-[rgba(60,60,67,0.4)]"
|
||||
style={{
|
||||
fontFamily:
|
||||
|
||||
@@ -27,7 +27,7 @@
|
||||
* - `submitWaterEntry` → posts a new reading
|
||||
* - `logoutWater` → clears the cookie + session row
|
||||
*/
|
||||
import { useCallback, useEffect, useState, useSyncExternalStore } from "react";
|
||||
import { useCallback, useEffect, useState } from "react";
|
||||
import {
|
||||
verifyWaterPin,
|
||||
submitWaterEntry,
|
||||
@@ -41,25 +41,25 @@ import { MobileWaterLogForm } from "./LogForm";
|
||||
import { MobileWaterSuccessScreen } from "./SuccessScreen";
|
||||
import { WL_SESSION_COOKIE } from "@/lib/water-log/session";
|
||||
import { TUXEDO_BRAND_ID } from "@/lib/water-log/brand";
|
||||
import { LangProvider } from "@/lib/water-log/lang-context";
|
||||
import {
|
||||
detectInitialLang,
|
||||
getT,
|
||||
type Lang,
|
||||
} from "@/lib/water-log/i18n";
|
||||
import { useLang } from "@/lib/water-log/lang-context";
|
||||
|
||||
// Server-side auth errors that should bounce the user to the PIN
|
||||
// screen rather than surface as a read error. Matched against the
|
||||
// exact strings returned by `requireFieldSession` in
|
||||
// src/actions/water-log/auth.ts.
|
||||
const SESSION_ERRORS = [
|
||||
"Session expired",
|
||||
"Session not found",
|
||||
"User is inactive",
|
||||
"Not logged in",
|
||||
];
|
||||
|
||||
export function MobileWaterApp() {
|
||||
const [screen, setScreen] = useState<Screen>("pin");
|
||||
// Language comes from `document.cookie` (the wl_lang cookie) which
|
||||
// doesn't exist on the server. We use `useSyncExternalStore` so
|
||||
// the server snapshot ("es") and the client snapshot (cookie or
|
||||
// navigator) are read from stable getSnapshot functions — no
|
||||
// hydration mismatch, no cascading setState in an effect.
|
||||
const lang = useSyncExternalStore(
|
||||
subscribeNoop,
|
||||
getClientLangSnapshot,
|
||||
getServerLangSnapshot,
|
||||
);
|
||||
// `useLang()` reads from the <LangProvider> in WaterFieldClient,
|
||||
// so it's always up-to-date when the user toggles via the toggle
|
||||
// in the headgate list header.
|
||||
const { t } = useLang();
|
||||
const [irrigatorName, setIrrigatorName] = useState<string | null>(null);
|
||||
const [headgates, setHeadgates] = useState<FieldHeadgate[]>([]);
|
||||
const [loadingHeadgates, setLoadingHeadgates] = useState(false);
|
||||
@@ -73,10 +73,6 @@ export function MobileWaterApp() {
|
||||
const [logFormKey, setLogFormKey] = useState(0);
|
||||
|
||||
// ── Headgate fetch (shared by initial load + pull-to-refresh) ──
|
||||
// `lang` is read here (instead of via `useLang`) because this
|
||||
// orchestrator lives outside the <LangProvider>; the provider
|
||||
// wraps each child screen. We just want the fallback error string
|
||||
// in the user's chosen language.
|
||||
const loadHeadgates = useCallback(async () => {
|
||||
setLoadingHeadgates(true);
|
||||
setHeadgateError(null);
|
||||
@@ -85,14 +81,12 @@ export function MobileWaterApp() {
|
||||
setHeadgates(hgs.filter((h) => h.active));
|
||||
} catch (err) {
|
||||
setHeadgateError(
|
||||
err instanceof Error
|
||||
? err.message
|
||||
: getT(lang).headgates.loadError,
|
||||
err instanceof Error ? err.message : t.headgates.loadError,
|
||||
);
|
||||
} finally {
|
||||
setLoadingHeadgates(false);
|
||||
}
|
||||
}, [lang]);
|
||||
}, [t.headgates.loadError]);
|
||||
|
||||
// ── Initial bootstrap: if `wl_session` exists, jump straight to
|
||||
// the headgate list. We defer the cookie read into an effect
|
||||
@@ -145,7 +139,7 @@ export function MobileWaterApp() {
|
||||
const handleLogSubmit = useCallback(
|
||||
async (input: { measurement: number; unit: string; notes: string }) => {
|
||||
if (!selectedHeadgate) {
|
||||
throw new Error("No headgate selected");
|
||||
throw new Error(t.log.noHeadgateSelected);
|
||||
}
|
||||
const result = await submitWaterEntry(
|
||||
selectedHeadgate.id,
|
||||
@@ -154,15 +148,11 @@ export function MobileWaterApp() {
|
||||
input.notes,
|
||||
);
|
||||
if (!result.success) {
|
||||
// Session gone? Bounce back to PIN.
|
||||
if (
|
||||
result.error === "Session expired or invalid" ||
|
||||
result.error === "Not logged in"
|
||||
) {
|
||||
if (SESSION_ERRORS.includes(result.error)) {
|
||||
setScreen("pin");
|
||||
throw new Error("Your session expired. Please sign in again.");
|
||||
throw new Error(t.log.sessionExpired);
|
||||
}
|
||||
throw new Error(result.error);
|
||||
throw new Error(t.log.submitError);
|
||||
}
|
||||
setSubmittedLog({
|
||||
headgateName: selectedHeadgate.name,
|
||||
@@ -172,7 +162,7 @@ export function MobileWaterApp() {
|
||||
});
|
||||
setScreen("success");
|
||||
},
|
||||
[selectedHeadgate],
|
||||
[selectedHeadgate, t.log.noHeadgateSelected, t.log.sessionExpired, t.log.submitError],
|
||||
);
|
||||
|
||||
// ── Success actions ───────────────────────────────────────────
|
||||
@@ -270,32 +260,9 @@ export function MobileWaterApp() {
|
||||
break;
|
||||
}
|
||||
|
||||
// `initialLang` is computed at module-load time — the value will
|
||||
// match the first client render (SSR returns "es" because
|
||||
// `document` is undefined; the client effect then syncs if the
|
||||
// cookie disagrees). This avoids the hydration mismatch that a
|
||||
// `useState(detectInitialLang)` initializer would cause.
|
||||
return (
|
||||
<LangProvider initialLang={lang}>{screenElement}</LangProvider>
|
||||
);
|
||||
// `initialLang` is computed by the parent <LangProvider> in
|
||||
// WaterFieldClient — we just return the screen element here.
|
||||
return screenElement;
|
||||
}
|
||||
|
||||
/**
|
||||
* Subscribers don't make sense for the cookie source — it's read
|
||||
* once on mount and updated when the user toggles via the
|
||||
* `LanguageToggle` component (which writes to the same cookie +
|
||||
* dispatches a manual re-render via setState in the provider).
|
||||
*/
|
||||
const subscribeNoop = () => () => {};
|
||||
|
||||
const getServerLangSnapshot = (): Lang => "es";
|
||||
|
||||
const getClientLangSnapshot = (): Lang => {
|
||||
// Cache the value so we don't re-read the cookie on every render.
|
||||
// The cookie is only mutated by the LanguageToggle's setLang →
|
||||
// setLangCookie path, which causes a re-render through the
|
||||
// provider's setState.
|
||||
return detectInitialLang();
|
||||
};
|
||||
|
||||
export default MobileWaterApp;
|
||||
@@ -49,6 +49,8 @@ export type Labels = {
|
||||
a11yBackToHeadgates: string;
|
||||
a11yChangeHeadgate: string;
|
||||
a11ySignOut: string;
|
||||
/** ARIA group label for the language toggle. */
|
||||
a11yLanguage: string;
|
||||
};
|
||||
/** The brand strip shown on the PIN and Success screens. */
|
||||
brand: {
|
||||
@@ -94,6 +96,12 @@ export type Labels = {
|
||||
implausiblyLarge: string;
|
||||
submit: string;
|
||||
submitting: string;
|
||||
/** Shown when a session expires mid-shift; bounce user to PIN. */
|
||||
sessionExpired: string;
|
||||
/** Generic submit-failure fallback (e.g. server-side validation). */
|
||||
submitError: string;
|
||||
/** Defensive guard — shouldn't ever happen. */
|
||||
noHeadgateSelected: string;
|
||||
normalRange: (low: number, high: number, unit: string) => string;
|
||||
alertAbove: (high: number, unit: string) => string;
|
||||
alertBelow: (low: number, unit: string) => string;
|
||||
@@ -143,6 +151,7 @@ export const LABELS: Record<Lang, Labels> = {
|
||||
a11yBackToHeadgates: "Back to headgates",
|
||||
a11yChangeHeadgate: "Change headgate",
|
||||
a11ySignOut: "Sign out",
|
||||
a11yLanguage: "Language",
|
||||
},
|
||||
brand: {
|
||||
waterLog: "TUXEDO WATER LOG",
|
||||
@@ -187,6 +196,9 @@ export const LABELS: Record<Lang, Labels> = {
|
||||
implausiblyLarge: "That value is implausibly large",
|
||||
submit: "Submit Log",
|
||||
submitting: "Submitting…",
|
||||
sessionExpired: "Your session expired. Please sign in again.",
|
||||
submitError: "Something went wrong submitting that entry. Try again.",
|
||||
noHeadgateSelected: "Pick a headgate first.",
|
||||
normalRange: (low, high, unit) =>
|
||||
`Normal range: ${low} – ${high} ${unit}`,
|
||||
alertAbove: (high, unit) => `Alert above ${high} ${unit}`,
|
||||
@@ -230,6 +242,7 @@ export const LABELS: Record<Lang, Labels> = {
|
||||
a11yBackToHeadgates: "Volver a compuertas",
|
||||
a11yChangeHeadgate: "Cambiar compuerta",
|
||||
a11ySignOut: "Cerrar sesión",
|
||||
a11yLanguage: "Idioma",
|
||||
},
|
||||
brand: {
|
||||
waterLog: "REGISTRO DE AGUA TUXEDO",
|
||||
@@ -274,6 +287,10 @@ export const LABELS: Record<Lang, Labels> = {
|
||||
implausiblyLarge: "Ese valor es demasiado grande",
|
||||
submit: "Enviar registro",
|
||||
submitting: "Enviando…",
|
||||
sessionExpired: "Tu sesión expiró. Inicia sesión otra vez.",
|
||||
submitError:
|
||||
"Algo salió mal al enviar ese registro. Inténtalo de nuevo.",
|
||||
noHeadgateSelected: "Elige una compuerta primero.",
|
||||
normalRange: (low, high, unit) =>
|
||||
`Rango normal: ${low} – ${high} ${unit}`,
|
||||
alertAbove: (high, unit) => `Alerta arriba de ${high} ${unit}`,
|
||||
|
||||
@@ -193,6 +193,16 @@ describe("detectInitialLang()", () => {
|
||||
stub.setNav({ language: "fr-FR" });
|
||||
expect(detectInitialLang()).toBe("es");
|
||||
});
|
||||
|
||||
it("ignores a wl_lang value that contains a JS payload (injection guard)", () => {
|
||||
// Important: the regex in `detectInitialLang` only matches the
|
||||
// exact 2-letter codes. Anything else must fall through to the
|
||||
// navigator/default. This locks in that we never feed untrusted
|
||||
// cookie contents into React state.
|
||||
document.cookie = `${WL_LANG_COOKIE}=<script>alert(1)</script>; path=/`;
|
||||
stub.setNav({ language: "en-US" });
|
||||
expect(detectInitialLang()).toBe("en");
|
||||
});
|
||||
});
|
||||
|
||||
// ── setLangCookie ───────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user