From d9018d036c3cc1fe1e971d5bffa41d762f0ba06a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 05:45:38 +0000 Subject: [PATCH 1/2] Make TrainerRoad metrics configurable via settings Add a Settings modal (gear icon in the header) with a "Show TrainerRoad metrics" toggle, persisted to localStorage via a new useSettings hook that mirrors the existing use-section-state persistence pattern. The setting is opt-in (off by default) and gates visibility of the TR-RPE and TR-LGT fields in the form as well as their lines in generated markdown, while preserving any previously-entered values. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NzWVv8oVBPDR14qbN1ZoQa --- CLAUDE.md | 10 ++ client/src/components/ui/settings-panel.tsx | 81 +++++++++++++ client/src/hooks/use-settings.ts | 75 ++++++++++++ client/src/pages/home.tsx | 95 +++++++++++---- client/src/test/hooks/use-settings.test.ts | 121 ++++++++++++++++++++ client/src/test/utils.test.ts | 52 +++++++-- 6 files changed, 401 insertions(+), 33 deletions(-) create mode 100644 client/src/components/ui/settings-panel.tsx create mode 100644 client/src/hooks/use-settings.ts create mode 100644 client/src/test/hooks/use-settings.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index c43794a..059dad3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -48,6 +48,7 @@ client/src/ ├── components/ui/ # shadcn/ui components ├── hooks/ │ ├── use-form-persistence.ts # Auto-save/restore form draft to localStorage +│ ├── use-settings.ts # Persisted app settings (e.g. TrainerRoad metrics visibility) │ ├── use-theme.ts # Dark/light theme toggle │ └── use-toast.ts # Toast notification system └── test/ # Vitest tests @@ -65,7 +66,9 @@ scripts/ # AWS S3 deployment automation - `shared/schema-static.ts` - Zod schema defining workout data structure - `client/src/hooks/use-form-persistence.ts` - Debounced form draft auto-save/restore - `client/src/hooks/use-section-state.ts` - Persisted open/closed state for collapsible form sections +- `client/src/hooks/use-settings.ts` - Persisted app settings (TrainerRoad metrics visibility) - `client/src/components/ui/collapsible-section.tsx` - Reusable `
`-based collapsible section component +- `client/src/components/ui/settings-panel.tsx` - Settings modal (opened via gear icon in the header) ### PWA Setup - `vite-plugin-pwa` generates the service worker (Workbox) and injects manifest link automatically @@ -93,6 +96,13 @@ scripts/ # AWS S3 deployment automation - Section IDs: `daily-notes`, `core-metrics`, `fueling`, `performance-metrics`, `recovery-metrics`, `reflection`, `rest-day`, `activity` - Visible sections depend on `entryType` — `expandAllOrCollapseAll` only affects currently visible sections; `daily-notes` is included in all three entry type section lists +### Settings Persistence +- `useSettings({ key, defaults })` manages persisted app-wide settings, opened via the gear icon in the header (`SettingsPanel`) +- localStorage key: `"pedalnotes-settings"`; stored as `{ version: 1, data: Settings }` (same envelope shape as section state) +- On mount: reads stored state, merges defaults for any missing keys, drops unknown keys; version mismatch or malformed data falls back to defaults and overwrites stored value +- `setSetting(key, value)` — writes persist immediately (no debounce); `QuotaExceededError` on write is caught and settings still work in memory +- Current setting: `showTrainerRoadMetrics` (boolean, **default `false` — opt-in**). When off, the `trainerRoadRpe` (TR-RPE, Core Metrics) and `trainerRoadLgt` (TR-LGT, Recovery Metrics) fields are hidden from the form and omitted from generated markdown, even if values were previously entered — same "hidden fields retain their values but don't appear in markdown output" behavior used for `entryType` switching. `generateMarkdown`/`generateCyclingMarkdown`/`generateRestMarkdown`/`generateOtherMarkdown` all take a `showTrainerRoadMetrics` parameter to enforce this. + ### Entry Types Three entry types supported via `entryType` field (default: `cycling`): - **`cycling`** — full cycling workout form (Core Metrics, Fueling, Performance, Recovery, Reflection) diff --git a/client/src/components/ui/settings-panel.tsx b/client/src/components/ui/settings-panel.tsx new file mode 100644 index 0000000..2649e93 --- /dev/null +++ b/client/src/components/ui/settings-panel.tsx @@ -0,0 +1,81 @@ +import { useEffect } from "react"; +import { X } from "lucide-react"; + +interface SettingsPanelProps { + open: boolean; + onOpenChange: (open: boolean) => void; + showTrainerRoadMetrics: boolean; + onShowTrainerRoadMetricsChange: (value: boolean) => void; +} + +export function SettingsPanel({ + open, + onOpenChange, + showTrainerRoadMetrics, + onShowTrainerRoadMetricsChange, +}: SettingsPanelProps) { + useEffect(() => { + if (!open) return; + function handleKeyDown(e: KeyboardEvent) { + if (e.key === "Escape") onOpenChange(false); + } + document.addEventListener("keydown", handleKeyDown); + return () => document.removeEventListener("keydown", handleKeyDown); + }, [open, onOpenChange]); + + if (!open) return null; + + return ( +
onOpenChange(false)} + > +
e.stopPropagation()} + > +
+

+ Settings +

+ +
+ +
+
+

Show TrainerRoad metrics

+

+ Show TR-RPE and TR-LGT fields on the form and in generated markdown. +

+
+ +
+
+
+ ); +} diff --git a/client/src/hooks/use-settings.ts b/client/src/hooks/use-settings.ts new file mode 100644 index 0000000..6e9e82f --- /dev/null +++ b/client/src/hooks/use-settings.ts @@ -0,0 +1,75 @@ +import { useState } from "react"; + +export interface Settings { + showTrainerRoadMetrics: boolean; +} + +interface PersistedSettings { + version: 1; + data: Settings; +} + +interface UseSettingsOptions { + key: string; + defaults: Settings; +} + +export function useSettings(options: UseSettingsOptions): { + settings: Settings; + setSetting: (key: K, value: Settings[K]) => void; +} { + const { key, defaults } = options; + + const [settings, setSettings] = useState(() => { + try { + const stored = localStorage.getItem(key); + if (stored) { + const parsed: unknown = JSON.parse(stored); + if ( + typeof parsed !== "object" || + parsed === null || + (parsed as Record).version !== 1 || + typeof (parsed as Record).data !== "object" || + (parsed as Record).data === null + ) { + // Malformed or wrong version — fall back to defaults and overwrite + persistRaw(key, defaults); + return { ...defaults }; + } + const data = (parsed as PersistedSettings).data; + // Merge: use defaults for missing keys, drop unknown keys + const merged: Settings = { ...defaults }; + for (const settingKey of Object.keys(defaults) as (keyof Settings)[]) { + if (typeof data[settingKey] === "boolean") { + merged[settingKey] = data[settingKey]; + } + } + return merged; + } + } catch (err) { + console.error("[use-settings] Failed to restore settings:", err); + } + return { ...defaults }; + }); + + function setSetting(settingKey: K, value: Settings[K]) { + setSettings((prev) => { + const next = { ...prev, [settingKey]: value }; + persistRaw(key, next); + return next; + }); + } + + return { settings, setSetting }; +} + +function persistRaw(key: string, data: Settings) { + try { + const toStore: PersistedSettings = { version: 1, data }; + localStorage.setItem(key, JSON.stringify(toStore)); + } catch (err) { + if (err instanceof DOMException && err.name === "QuotaExceededError") { + console.error("[use-settings] localStorage quota exceeded"); + } + } +} diff --git a/client/src/pages/home.tsx b/client/src/pages/home.tsx index 15b6e18..af19cfe 100644 --- a/client/src/pages/home.tsx +++ b/client/src/pages/home.tsx @@ -11,10 +11,12 @@ import { Form, FormControl, FormField, FormItem, FormLabel, FormMessage } from " import { Card, CardContent } from "@/components/ui/card"; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; import { CollapsibleSection } from "@/components/ui/collapsible-section"; +import { SettingsPanel } from "@/components/ui/settings-panel"; import { useToast } from "@/hooks/use-toast"; import { useTheme } from "@/hooks/use-theme"; import { useFormPersistence } from "@/hooks/use-form-persistence"; import { useSectionState, type SectionId } from "@/hooks/use-section-state"; +import { useSettings, type Settings } from "@/hooks/use-settings"; import { Zap, Copy, @@ -38,7 +40,8 @@ import { Dumbbell, ClipboardList, Scale, - NotebookPen + NotebookPen, + Settings as SettingsIcon } from "lucide-react"; const SECTION_DEFAULTS: Record = { @@ -52,6 +55,10 @@ const SECTION_DEFAULTS: Record = { "activity": true, }; +const SETTINGS_DEFAULTS: Settings = { + showTrainerRoadMetrics: false, +}; + const FIELD_TO_SECTION: Partial> = { goal: "core-metrics", rpe: "core-metrics", @@ -191,7 +198,7 @@ function formatWorkoutDate(dateStr: string): string { return `${day}.${month}.${year}`; } -function generateCyclingMarkdown(data: InsertWorkout): string { +function generateCyclingMarkdown(data: InsertWorkout, showTrainerRoadMetrics: boolean): string { let markdown = ""; if (data.goal) markdown += `G: ${data.goal}\n`; @@ -204,11 +211,11 @@ function generateCyclingMarkdown(data: InsertWorkout): string { if (data.normalizedPower) markdown += `NP: ${data.normalizedPower}\n`; if (data.tss) markdown += `TSS: ${data.tss}\n`; if (data.avgHeartRate) markdown += `Hr: ${data.avgHeartRate}\n`; - if (data.trainerRoadRpe) markdown += `TR-RPE: ${data.trainerRoadRpe}\n`; + if (showTrainerRoadMetrics && data.trainerRoadRpe) markdown += `TR-RPE: ${data.trainerRoadRpe}\n`; if (data.hrv) markdown += `HRV: ${data.hrv}\n`; if (data.rMSSD) markdown += `rMSSD: ${data.rMSSD}\n`; if (data.rhr) markdown += `RHR: ${data.rhr}\n`; - if (data.trainerRoadLgt && data.trainerRoadLgt !== 'G') { + if (showTrainerRoadMetrics && data.trainerRoadLgt && data.trainerRoadLgt !== 'G') { markdown += `TR-LGT: ${data.trainerRoadLgt}\n`; } @@ -232,13 +239,13 @@ function generateCyclingMarkdown(data: InsertWorkout): string { return markdown; } -function generateRestMarkdown(data: InsertWorkout): string { +function generateRestMarkdown(data: InsertWorkout, showTrainerRoadMetrics: boolean): string { let markdown = "Rest Day\n\n"; if (data.hrv) markdown += `HRV: ${data.hrv}\n`; if (data.rMSSD) markdown += `rMSSD: ${data.rMSSD}\n`; if (data.rhr) markdown += `RHR: ${data.rhr}\n`; - if (data.trainerRoadLgt && data.trainerRoadLgt !== 'G') { + if (showTrainerRoadMetrics && data.trainerRoadLgt && data.trainerRoadLgt !== 'G') { markdown += `TR-LGT: ${data.trainerRoadLgt}\n`; } if (data.weight) markdown += `Weight: ${data.weight}\n`; @@ -246,13 +253,13 @@ function generateRestMarkdown(data: InsertWorkout): string { return markdown; } -function generateOtherMarkdown(data: InsertWorkout): string { +function generateOtherMarkdown(data: InsertWorkout, showTrainerRoadMetrics: boolean): string { let markdown = ""; if (data.hrv) markdown += `HRV: ${data.hrv}\n`; if (data.rMSSD) markdown += `rMSSD: ${data.rMSSD}\n`; if (data.rhr) markdown += `RHR: ${data.rhr}\n`; - if (data.trainerRoadLgt && data.trainerRoadLgt !== 'G') { + if (showTrainerRoadMetrics && data.trainerRoadLgt && data.trainerRoadLgt !== 'G') { markdown += `TR-LGT: ${data.trainerRoadLgt}\n`; } @@ -265,7 +272,7 @@ function generateOtherMarkdown(data: InsertWorkout): string { return markdown; } -function generateMarkdown(data: InsertWorkout): string { +function generateMarkdown(data: InsertWorkout, showTrainerRoadMetrics: boolean): string { let markdown = `---\n## ${formatWorkoutDate(data.workoutDate)}\n\n`; if (data.dailyNotes) { @@ -274,14 +281,14 @@ function generateMarkdown(data: InsertWorkout): string { switch (data.entryType) { case "rest": - markdown += generateRestMarkdown(data); + markdown += generateRestMarkdown(data, showTrainerRoadMetrics); break; case "other": - markdown += generateOtherMarkdown(data); + markdown += generateOtherMarkdown(data, showTrainerRoadMetrics); break; case "cycling": default: - markdown += generateCyclingMarkdown(data); + markdown += generateCyclingMarkdown(data, showTrainerRoadMetrics); break; } @@ -307,6 +314,12 @@ export default function Home() { defaults: SECTION_DEFAULTS, }); + const { settings, setSetting } = useSettings({ + key: "pedalnotes-settings", + defaults: SETTINGS_DEFAULTS, + }); + const [settingsOpen, setSettingsOpen] = useState(false); + const isQuickMode = useMemo( () => new URLSearchParams(window.location.search).get('quick') === '1', [] @@ -345,8 +358,8 @@ export default function Home() { } useEffect(() => { - setMarkdownOutput((isDirty || wasRestored || isQuickMode) ? generateMarkdown(watchedValues) : ""); - }, [watchedValues, isDirty, wasRestored, isQuickMode]); + setMarkdownOutput((isDirty || wasRestored || isQuickMode) ? generateMarkdown(watchedValues, settings.showTrainerRoadMetrics) : ""); + }, [watchedValues, isDirty, wasRestored, isQuickMode, settings.showTrainerRoadMetrics]); useEffect(() => { if (wasRestored) { @@ -362,7 +375,7 @@ export default function Home() { await form.trigger(); autoExpandErrorSections(); } - const exportMarkdown = generateMarkdown(form.getValues()); + const exportMarkdown = generateMarkdown(form.getValues(), settings.showTrainerRoadMetrics); try { // Try modern clipboard API first if (navigator.clipboard && window.isSecureContext) { @@ -411,7 +424,7 @@ export default function Home() { await form.trigger(); autoExpandErrorSections(); } - const exportMarkdown = generateMarkdown(form.getValues()); + const exportMarkdown = generateMarkdown(form.getValues(), settings.showTrainerRoadMetrics); const blob = new Blob([exportMarkdown], { type: 'text/markdown' }); const url = URL.createObjectURL(blob); const a = document.createElement('a'); @@ -448,6 +461,13 @@ export default function Home() { > Full form + +
+ + +
@@ -772,7 +807,7 @@ export default function Home() { !!watchedValues.goal || watchedValues.rpe !== 5 || watchedValues.feel !== "N" || - watchedValues.trainerRoadRpe !== undefined + (settings.showTrainerRoadMetrics && watchedValues.trainerRoadRpe !== undefined) } > + {settings.showTrainerRoadMetrics && ( + )}
@@ -1193,6 +1230,7 @@ export default function Home() { )} /> + {settings.showTrainerRoadMetrics && ( )} /> + )}
)} @@ -1444,6 +1483,12 @@ export default function Home() { + setSetting("showTrainerRoadMetrics", value)} + /> ); } diff --git a/client/src/test/hooks/use-settings.test.ts b/client/src/test/hooks/use-settings.test.ts new file mode 100644 index 0000000..3815041 --- /dev/null +++ b/client/src/test/hooks/use-settings.test.ts @@ -0,0 +1,121 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import { useSettings, type Settings } from "@/hooks/use-settings"; + +const SETTINGS_KEY = "pedalnotes-settings-test"; + +const defaults: Settings = { + showTrainerRoadMetrics: false, +}; + +function makeStored(data: Partial, version = 1) { + return JSON.stringify({ version, data }); +} + +function renderHookWithDefaults() { + return renderHook(() => useSettings({ key: SETTINGS_KEY, defaults })); +} + +beforeEach(() => { + localStorage.clear(); +}); + +afterEach(() => { + localStorage.clear(); +}); + +describe("useSettings", () => { + it("returns defaults when no localStorage entry exists", () => { + const { result } = renderHookWithDefaults(); + expect(result.current.settings).toEqual(defaults); + }); + + it("restores persisted state on mount", () => { + localStorage.setItem(SETTINGS_KEY, makeStored({ showTrainerRoadMetrics: true })); + + const { result } = renderHookWithDefaults(); + expect(result.current.settings).toEqual({ showTrainerRoadMetrics: true }); + }); + + it("setSetting updates a single setting and persists", () => { + const { result } = renderHookWithDefaults(); + + act(() => { + result.current.setSetting("showTrainerRoadMetrics", true); + }); + + expect(result.current.settings.showTrainerRoadMetrics).toBe(true); + const stored = JSON.parse(localStorage.getItem(SETTINGS_KEY)!); + expect(stored.data.showTrainerRoadMetrics).toBe(true); + }); + + it("discards malformed localStorage data and falls back to defaults", () => { + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + localStorage.setItem(SETTINGS_KEY, "not valid json {{{{"); + + const { result } = renderHookWithDefaults(); + expect(result.current.settings).toEqual(defaults); + + consoleSpy.mockRestore(); + }); + + it("discards data with wrong version and falls back to defaults", () => { + const badVersion = JSON.stringify({ version: 99, data: { showTrainerRoadMetrics: true } }); + localStorage.setItem(SETTINGS_KEY, badVersion); + + const { result } = renderHookWithDefaults(); + expect(result.current.settings).toEqual(defaults); + + // Should overwrite with version 1 + const stored = JSON.parse(localStorage.getItem(SETTINGS_KEY)!); + expect(stored.version).toBe(1); + }); + + it("merges defaults for missing keys (forward compatibility)", () => { + localStorage.setItem(SETTINGS_KEY, makeStored({})); + + const { result } = renderHookWithDefaults(); + + expect(result.current.settings.showTrainerRoadMetrics).toBe(false); // from defaults + }); + + it("drops unknown keys silently", () => { + const withExtra = { showTrainerRoadMetrics: true, unknownSetting: true } as unknown as Partial; + localStorage.setItem(SETTINGS_KEY, makeStored(withExtra)); + + const { result } = renderHookWithDefaults(); + + expect((result.current.settings as Record)["unknownSetting"]).toBeUndefined(); + expect(result.current.settings.showTrainerRoadMetrics).toBe(true); + }); + + it("catches QuotaExceededError on write without crashing", () => { + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + vi.spyOn(Storage.prototype, "setItem").mockImplementation(() => { + throw new DOMException("QuotaExceededError", "QuotaExceededError"); + }); + + const { result } = renderHookWithDefaults(); + + expect(() => { + act(() => { + result.current.setSetting("showTrainerRoadMetrics", true); + }); + }).not.toThrow(); + + vi.restoreAllMocks(); + consoleSpy.mockRestore(); + }); + + it("persists version:1 wrapper in localStorage", () => { + const { result } = renderHookWithDefaults(); + + act(() => { + result.current.setSetting("showTrainerRoadMetrics", true); + }); + + const stored = JSON.parse(localStorage.getItem(SETTINGS_KEY)!); + expect(stored.version).toBe(1); + expect(typeof stored.data).toBe("object"); + }); +}); diff --git a/client/src/test/utils.test.ts b/client/src/test/utils.test.ts index f923659..7160358 100644 --- a/client/src/test/utils.test.ts +++ b/client/src/test/utils.test.ts @@ -83,7 +83,7 @@ function formatWorkoutDate(dateStr: string): string { return `${day}.${month}.${year}`; } -function generateCyclingMarkdown(data: InsertWorkout): string { +function generateCyclingMarkdown(data: InsertWorkout, showTrainerRoadMetrics = true): string { let markdown = ""; if (data.goal) markdown += `G: ${data.goal}\n`; @@ -96,11 +96,11 @@ function generateCyclingMarkdown(data: InsertWorkout): string { if (data.normalizedPower) markdown += `NP: ${data.normalizedPower}\n`; if (data.tss) markdown += `TSS: ${data.tss}\n`; if (data.avgHeartRate) markdown += `Hr: ${data.avgHeartRate}\n`; - if (data.trainerRoadRpe) markdown += `TR-RPE: ${data.trainerRoadRpe}\n`; + if (showTrainerRoadMetrics && data.trainerRoadRpe) markdown += `TR-RPE: ${data.trainerRoadRpe}\n`; if (data.hrv) markdown += `HRV: ${data.hrv}\n`; if (data.rMSSD) markdown += `rMSSD: ${data.rMSSD}\n`; if (data.rhr) markdown += `RHR: ${data.rhr}\n`; - if (data.trainerRoadLgt && data.trainerRoadLgt !== 'G') { + if (showTrainerRoadMetrics && data.trainerRoadLgt && data.trainerRoadLgt !== 'G') { markdown += `TR-LGT: ${data.trainerRoadLgt}\n`; } @@ -122,13 +122,13 @@ function generateCyclingMarkdown(data: InsertWorkout): string { return markdown; } -function generateRestMarkdown(data: InsertWorkout): string { +function generateRestMarkdown(data: InsertWorkout, showTrainerRoadMetrics = true): string { let markdown = "Rest Day\n\n"; if (data.hrv) markdown += `HRV: ${data.hrv}\n`; if (data.rMSSD) markdown += `rMSSD: ${data.rMSSD}\n`; if (data.rhr) markdown += `RHR: ${data.rhr}\n`; - if (data.trainerRoadLgt && data.trainerRoadLgt !== 'G') { + if (showTrainerRoadMetrics && data.trainerRoadLgt && data.trainerRoadLgt !== 'G') { markdown += `TR-LGT: ${data.trainerRoadLgt}\n`; } if (data.weight) markdown += `Weight: ${data.weight}\n`; @@ -148,7 +148,7 @@ function generateOtherMarkdown(data: InsertWorkout): string { return markdown; } -function generateMarkdown(data: InsertWorkout): string { +function generateMarkdown(data: InsertWorkout, showTrainerRoadMetrics = true): string { let markdown = `---\n## ${formatWorkoutDate(data.workoutDate)}\n\n`; if (data.dailyNotes) { @@ -157,14 +157,14 @@ function generateMarkdown(data: InsertWorkout): string { switch (data.entryType) { case "rest": - markdown += generateRestMarkdown(data); + markdown += generateRestMarkdown(data, showTrainerRoadMetrics); break; case "other": markdown += generateOtherMarkdown(data); break; case "cycling": default: - markdown += generateCyclingMarkdown(data); + markdown += generateCyclingMarkdown(data, showTrainerRoadMetrics); break; } @@ -393,3 +393,39 @@ describe('generateMarkdown — cycling daily notes', () => { expect(md).not.toContain('- '); }); }); + +describe('generateMarkdown — TrainerRoad metrics gating', () => { + it('includes TR-RPE and TR-LGT for cycling when showTrainerRoadMetrics is true', () => { + const md = generateMarkdown( + { ...baseCyclingWorkout, trainerRoadRpe: 3, trainerRoadLgt: 'Y' }, + true + ); + expect(md).toContain('TR-RPE: 3'); + expect(md).toContain('TR-LGT: Y'); + }); + + it('omits TR-RPE and TR-LGT for cycling when showTrainerRoadMetrics is false, even with values set', () => { + const md = generateMarkdown( + { ...baseCyclingWorkout, trainerRoadRpe: 3, trainerRoadLgt: 'Y' }, + false + ); + expect(md).not.toContain('TR-RPE'); + expect(md).not.toContain('TR-LGT'); + }); + + it('omits TR-LGT for rest when showTrainerRoadMetrics is false, even with a value set', () => { + const md = generateMarkdown( + { entryType: 'rest', workoutDate: '2026-04-13', trainerRoadLgt: 'R' }, + false + ); + expect(md).not.toContain('TR-LGT'); + }); + + it('includes TR-LGT for rest when showTrainerRoadMetrics is true', () => { + const md = generateMarkdown( + { entryType: 'rest', workoutDate: '2026-04-13', trainerRoadLgt: 'R' }, + true + ); + expect(md).toContain('TR-LGT: R'); + }); +}); From 284621699375a3b52cf9980cd9014423c90ee81f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 05:52:37 +0000 Subject: [PATCH 2/2] Address review: trap modal focus, fix stale storage, align other-entry test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - settings-panel.tsx: trap Tab focus within the dialog while open (was escaping to background form controls), focus the close button on open, and restore focus to the trigger on close. - use-settings.ts: overwrite malformed localStorage data with defaults on parse failure, instead of leaving the bad value in place to re-trigger the same parse error on every mount. - utils.test.ts: align the test-local generateOtherMarkdown with production (home.tsx) — it reuses HRV/rMSSD/RHR/TR-LGT like rest entries — and thread showTrainerRoadMetrics through it so TR-LGT gating is covered for entryType "other", not just cycling/rest. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NzWVv8oVBPDR14qbN1ZoQa --- client/src/components/ui/settings-panel.tsx | 42 +++++++++++++++++++-- client/src/hooks/use-settings.ts | 1 + client/src/test/hooks/use-settings.test.ts | 6 ++- client/src/test/utils.test.ts | 42 ++++++++++++++++----- 4 files changed, 78 insertions(+), 13 deletions(-) diff --git a/client/src/components/ui/settings-panel.tsx b/client/src/components/ui/settings-panel.tsx index 2649e93..4d683d3 100644 --- a/client/src/components/ui/settings-panel.tsx +++ b/client/src/components/ui/settings-panel.tsx @@ -1,6 +1,9 @@ -import { useEffect } from "react"; +import { useEffect, useRef } from "react"; import { X } from "lucide-react"; +const FOCUSABLE_SELECTOR = + 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'; + interface SettingsPanelProps { open: boolean; onOpenChange: (open: boolean) => void; @@ -14,13 +17,45 @@ export function SettingsPanel({ showTrainerRoadMetrics, onShowTrainerRoadMetricsChange, }: SettingsPanelProps) { + const dialogRef = useRef(null); + useEffect(() => { if (!open) return; + + const previouslyFocused = document.activeElement as HTMLElement | null; + const getFocusable = () => + dialogRef.current + ? Array.from(dialogRef.current.querySelectorAll(FOCUSABLE_SELECTOR)) + : []; + + getFocusable()[0]?.focus(); + function handleKeyDown(e: KeyboardEvent) { - if (e.key === "Escape") onOpenChange(false); + if (e.key === "Escape") { + onOpenChange(false); + return; + } + if (e.key !== "Tab") return; + + const focusable = getFocusable(); + if (focusable.length === 0) return; + const first = focusable[0]; + const last = focusable[focusable.length - 1]; + + if (e.shiftKey && document.activeElement === first) { + e.preventDefault(); + last.focus(); + } else if (!e.shiftKey && document.activeElement === last) { + e.preventDefault(); + first.focus(); + } } + document.addEventListener("keydown", handleKeyDown); - return () => document.removeEventListener("keydown", handleKeyDown); + return () => { + document.removeEventListener("keydown", handleKeyDown); + previouslyFocused?.focus(); + }; }, [open, onOpenChange]); if (!open) return null; @@ -31,6 +66,7 @@ export function SettingsPanel({ onClick={() => onOpenChange(false)} >
{ expect(stored.data.showTrainerRoadMetrics).toBe(true); }); - it("discards malformed localStorage data and falls back to defaults", () => { + it("discards malformed localStorage data, falls back to defaults, and overwrites the bad value", () => { const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); localStorage.setItem(SETTINGS_KEY, "not valid json {{{{"); const { result } = renderHookWithDefaults(); expect(result.current.settings).toEqual(defaults); + // Should overwrite the malformed value so subsequent mounts don't re-hit the parse error + const stored = JSON.parse(localStorage.getItem(SETTINGS_KEY)!); + expect(stored).toEqual({ version: 1, data: defaults }); + consoleSpy.mockRestore(); }); diff --git a/client/src/test/utils.test.ts b/client/src/test/utils.test.ts index 7160358..bb97096 100644 --- a/client/src/test/utils.test.ts +++ b/client/src/test/utils.test.ts @@ -136,9 +136,16 @@ function generateRestMarkdown(data: InsertWorkout, showTrainerRoadMetrics = true return markdown; } -function generateOtherMarkdown(data: InsertWorkout): string { +function generateOtherMarkdown(data: InsertWorkout, showTrainerRoadMetrics = true): string { let markdown = ""; + if (data.hrv) markdown += `HRV: ${data.hrv}\n`; + if (data.rMSSD) markdown += `rMSSD: ${data.rMSSD}\n`; + if (data.rhr) markdown += `RHR: ${data.rhr}\n`; + if (showTrainerRoadMetrics && data.trainerRoadLgt && data.trainerRoadLgt !== 'G') { + markdown += `TR-LGT: ${data.trainerRoadLgt}\n`; + } + if (data.activityGoal) markdown += `G: ${data.activityGoal}\n`; if (data.activityNotes) { @@ -160,7 +167,7 @@ function generateMarkdown(data: InsertWorkout, showTrainerRoadMetrics = true): s markdown += generateRestMarkdown(data, showTrainerRoadMetrics); break; case "other": - markdown += generateOtherMarkdown(data); + markdown += generateOtherMarkdown(data, showTrainerRoadMetrics); break; case "cycling": default: @@ -311,25 +318,26 @@ describe('generateMarkdown — other output', () => { expect(md).toContain('- Calves and quads were tender'); }); - it('excludes all metrics and cycling fields', () => { + it('includes reused recovery metrics but excludes cycling-only fields', () => { const md = generateMarkdown({ ...baseOther, activityGoal: 'Yoga', goal: 'leaked', rpe: 5, feel: 'N', - hrv: 'leaked', + hrv: 'Recovered', rMSSD: 40, rhr: 60, weight: 80, normalizedPower: 200, tss: 50, }); - expect(md).not.toContain('R:'); - expect(md).not.toContain('F:'); - expect(md).not.toContain('HRV:'); - expect(md).not.toContain('rMSSD:'); - expect(md).not.toContain('RHR:'); + expect(md).toContain('HRV: Recovered'); + expect(md).toContain('rMSSD: 40'); + expect(md).toContain('RHR: 60'); + // Use line-start matches for R:/F: since "RHR:" otherwise falsely matches a naive "R:" substring check + expect(md).not.toMatch(/(^|\n)R: /); + expect(md).not.toMatch(/(^|\n)F: /); expect(md).not.toContain('Weight:'); expect(md).not.toContain('NP:'); expect(md).not.toContain('TSS:'); @@ -428,4 +436,20 @@ describe('generateMarkdown — TrainerRoad metrics gating', () => { ); expect(md).toContain('TR-LGT: R'); }); + + it('omits TR-LGT for other when showTrainerRoadMetrics is false, even with a value set', () => { + const md = generateMarkdown( + { entryType: 'other', workoutDate: '2026-04-13', trainerRoadLgt: 'R' }, + false + ); + expect(md).not.toContain('TR-LGT'); + }); + + it('includes TR-LGT for other when showTrainerRoadMetrics is true', () => { + const md = generateMarkdown( + { entryType: 'other', workoutDate: '2026-04-13', trainerRoadLgt: 'R' }, + true + ); + expect(md).toContain('TR-LGT: R'); + }); });