diff --git a/client/src/components/settings/GeneralTab.jsx b/client/src/components/settings/GeneralTab.jsx index f2a480f56..127a32445 100644 --- a/client/src/components/settings/GeneralTab.jsx +++ b/client/src/components/settings/GeneralTab.jsx @@ -2,8 +2,10 @@ import { useState, useEffect, useMemo } from 'react'; import { Save } from 'lucide-react'; import toast from '../ui/Toast'; import FormField from '../ui/FormField'; +import UnsavedChangesConfirm from '../ui/UnsavedChangesConfirm'; import BrailleSpinner from '../BrailleSpinner'; import ThemePickerPanel from '../ThemePickerPanel'; +import useUnsavedChangesGuard from '../../hooks/useUnsavedChangesGuard'; import { getSettings, updateSettings } from '../../services/api'; // Coordinate inputs are free text so a partially-typed "-" or "37." isn't @@ -20,25 +22,54 @@ const parseCoord = (v) => { export function GeneralTab() { const [loading, setLoading] = useState(true); const [timezone, setTimezone] = useState(''); + const [savedTimezone, setSavedTimezone] = useState(null); const [saving, setSaving] = useState(false); const [lat, setLat] = useState(''); const [lon, setLon] = useState(''); + const [savedLocation, setSavedLocation] = useState(null); const [savingLocation, setSavingLocation] = useState(false); const detectedTz = Intl.DateTimeFormat().resolvedOptions().timeZone; const allTimezones = useMemo(() => Intl.supportedValuesOf?.('timeZone') ?? [], []); + const timezoneDirty = savedTimezone !== null && timezone !== savedTimezone; + const locationDirty = savedLocation !== null + && (lat !== savedLocation.lat || lon !== savedLocation.lon); + const dirty = timezoneDirty || locationDirty; + const hasDiscardableChanges = (timezoneDirty && !saving) + || (locationDirty && !savingLocation); + const routeGuard = useUnsavedChangesGuard(dirty); useEffect(() => { + let current = true; getSettings({ silent: true }) .then(settings => { - setTimezone(settings?.timezone || ''); - setLat(settings?.location?.lat != null ? String(settings.location.lat) : ''); - setLon(settings?.location?.lon != null ? String(settings.location.lon) : ''); + if (!current) return; + const nextTimezone = settings?.timezone || ''; + const nextLat = settings?.location?.lat != null ? String(settings.location.lat) : ''; + const nextLon = settings?.location?.lon != null ? String(settings.location.lon) : ''; + setTimezone(nextTimezone); + setSavedTimezone(nextTimezone); + setLat(nextLat); + setLon(nextLon); + setSavedLocation({ lat: nextLat, lon: nextLon }); }) - .catch(() => toast.error('Failed to load settings')) - .finally(() => setLoading(false)); + .catch(() => { + if (!current) return; + // The empty fields remain usable after a failed load, so treat the + // displayed fallback as the baseline instead of leaving edits outside + // dirty tracking. + setSavedTimezone(''); + setSavedLocation({ lat: '', lon: '' }); + toast.error('Failed to load settings'); + }) + .finally(() => { + if (current) setLoading(false); + }); + return () => { current = false; }; }, []); const handleSaveLocation = async () => { + const submittedLat = lat; + const submittedLon = lon; // Both-or-neither: weather needs a full pair, and a half-set pair would // silently mix a custom value with the tool's default coordinate. if (isBlank(lat) !== isBlank(lon)) { @@ -62,6 +93,11 @@ export function GeneralTab() { setSavingLocation(true); try { await updateSettings({ location: { lat: parsedLat, lon: parsedLon } }, { silent: true }); + const nextLat = parsedLat === null ? '' : String(parsedLat); + const nextLon = parsedLon === null ? '' : String(parsedLon); + setSavedLocation({ lat: nextLat, lon: nextLon }); + setLat(current => current === submittedLat ? nextLat : current); + setLon(current => current === submittedLon ? nextLon : current); toast.success(parsedLat === null ? 'Location cleared' : `Location set to ${parsedLat}, ${parsedLon}`); } catch (err) { toast.error(err.message || 'Failed to save location'); @@ -71,6 +107,7 @@ export function GeneralTab() { }; const handleSave = async (tz) => { + const submittedTimezone = timezone; const tzToSave = tz || detectedTz; if (!tzToSave) { toast.error('Timezone is required.'); @@ -95,7 +132,8 @@ export function GeneralTab() { setSaving(true); try { await updateSettings({ timezone: tzToSave }, { silent: true }); - setTimezone(tzToSave); + setSavedTimezone(tzToSave); + setTimezone(current => current === submittedTimezone ? tzToSave : current); toast.success(`Timezone set to ${tzToSave}`); } catch (err) { toast.error(err.message || 'Failed to save timezone'); @@ -104,10 +142,29 @@ export function GeneralTab() { } }; + const discardAndExit = () => { + // A stale /settings/:tab URL can fall back to General on both sides of the + // navigation, preserving this component instance. Reset the sections that + // are not actively being persisted before releasing the parked route. + if (!saving) setTimezone(savedTimezone ?? ''); + if (!savingLocation) { + setLat(savedLocation?.lat ?? ''); + setLon(savedLocation?.lon ?? ''); + } + routeGuard.proceed(); + }; + if (loading) return ; return (
+

Interface Theme

@@ -128,8 +185,9 @@ export function GeneralTab() { type="text" value={timezone} onChange={e => setTimezone(e.target.value)} + disabled={saving} placeholder={detectedTz} - className="w-full sm:flex-1 sm:max-w-xs min-w-0 px-3 py-2 bg-port-bg border border-port-border rounded-lg text-white text-sm" + className="w-full sm:flex-1 sm:max-w-xs min-w-0 px-3 py-2 bg-port-bg border border-port-border rounded-lg text-white text-sm disabled:opacity-50" list="tz-list" /> + {timezoneDirty && ( + + Unsaved changes + + )} {!timezone && ( + {locationDirty && ( + + Unsaved changes + + )}
diff --git a/client/src/components/settings/GeneralTab.test.jsx b/client/src/components/settings/GeneralTab.test.jsx new file mode 100644 index 000000000..5560c4662 --- /dev/null +++ b/client/src/components/settings/GeneralTab.test.jsx @@ -0,0 +1,295 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { StrictMode } from 'react'; +import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react'; +import { createMemoryRouter, RouterProvider } from 'react-router'; + +vi.mock('../../services/api', () => ({ + getSettings: vi.fn(), + updateSettings: vi.fn(), +})); +vi.mock('../ui/Toast', () => ({ + default: Object.assign(vi.fn(), { + success: vi.fn(), + error: vi.fn(), + }), +})); +vi.mock('../ThemePickerPanel', () => ({ + default: () =>
Theme picker
, +})); + +import { getSettings, updateSettings } from '../../services/api'; +import { GeneralTab } from './GeneralTab'; + +const SETTINGS = { + timezone: 'UTC', + location: { lat: 37.7749, lon: -122.4194 }, +}; +const CONFIRM = 'Discard your unsaved General settings changes?'; + +const timezoneCard = () => screen.getByRole('heading', { name: 'Timezone' }).parentElement; +const locationCard = () => screen.getByRole('heading', { name: 'Location' }).parentElement; + +const renderTab = async ({ expectedTimezone = 'UTC' } = {}) => { + const router = createMemoryRouter([ + { path: '/settings/general', element: }, + { path: '/settings/security', element:
Security settings
}, + ], { initialEntries: ['/settings/general'] }); + render(); + if (expectedTimezone === null) { + await screen.findByLabelText('Timezone (IANA)'); + } else { + await screen.findByDisplayValue(expectedTimezone); + } + return router; +}; + +const navigate = (router, to) => act(async () => { await router.navigate(to); }); +const deferred = () => { + let resolve; + let reject; + const promise = new Promise((settle, fail) => { + resolve = settle; + reject = fail; + }); + return { promise, resolve, reject }; +}; + +beforeEach(() => { + vi.clearAllMocks(); + getSettings.mockResolvedValue(SETTINGS); + updateSettings.mockResolvedValue({}); +}); + +describe('GeneralTab unsaved changes', () => { + it('guards edits made against the displayed fallback after loading fails', async () => { + getSettings.mockRejectedValueOnce(new Error('settings offline')); + const router = await renderTab({ expectedTimezone: null }); + fireEvent.change(screen.getByLabelText('Timezone (IANA)'), { + target: { value: 'America/New_York' }, + }); + + expect(within(timezoneCard()).getByText('Unsaved changes')).toBeInTheDocument(); + await navigate(router, '/settings/security'); + expect(screen.getByText(CONFIRM)).toBeInTheDocument(); + }); + + it('ignores an older StrictMode load failure after the current load succeeds', async () => { + const olderLoad = deferred(); + const currentLoad = deferred(); + getSettings + .mockReturnValueOnce(olderLoad.promise) + .mockReturnValueOnce(currentLoad.promise); + const router = createMemoryRouter([ + { path: '/settings/general', element: }, + ], { initialEntries: ['/settings/general'] }); + render(); + + await act(async () => { currentLoad.resolve(SETTINGS); }); + expect(await screen.findByDisplayValue('UTC')).toBeInTheDocument(); + await act(async () => { olderLoad.reject(new Error('stale settings request')); }); + + expect(screen.queryByText('Unsaved changes')).toBeNull(); + expect(screen.getByLabelText('Timezone (IANA)')).toHaveValue('UTC'); + expect(screen.getByLabelText('Latitude (-90 to 90)')).toHaveValue(String(SETTINGS.location.lat)); + }); + + it('marks each edited section dirty, arms beforeunload, and clears when values are restored', async () => { + const add = vi.spyOn(window, 'addEventListener'); + const remove = vi.spyOn(window, 'removeEventListener'); + await renderTab(); + + fireEvent.change(screen.getByLabelText('Timezone (IANA)'), { + target: { value: 'America/New_York' }, + }); + expect(within(timezoneCard()).getByText('Unsaved changes')).toBeInTheDocument(); + expect(screen.getByRole('status', { name: 'Timezone has unsaved changes' })).toBeInTheDocument(); + expect(within(locationCard()).queryByText('Unsaved changes')).toBeNull(); + + fireEvent.change(screen.getByLabelText('Latitude (-90 to 90)'), { + target: { value: '40.7128' }, + }); + expect(within(locationCard()).getByText('Unsaved changes')).toBeInTheDocument(); + expect(screen.getByRole('status', { name: 'Location has unsaved changes' })).toBeInTheDocument(); + await waitFor(() => { + expect(add.mock.calls.some(([type]) => type === 'beforeunload')).toBe(true); + }); + + fireEvent.change(screen.getByLabelText('Timezone (IANA)'), { + target: { value: SETTINGS.timezone }, + }); + fireEvent.change(screen.getByLabelText('Latitude (-90 to 90)'), { + target: { value: String(SETTINGS.location.lat) }, + }); + expect(screen.queryByText('Unsaved changes')).toBeNull(); + await waitFor(() => { + expect(remove.mock.calls.some(([type]) => type === 'beforeunload')).toBe(true); + }); + }); + + it('keeps the current route and draft when navigation is canceled', async () => { + const router = await renderTab(); + fireEvent.change(screen.getByLabelText('Timezone (IANA)'), { + target: { value: 'America/New_York' }, + }); + + await navigate(router, '/settings/security'); + expect(screen.getByText(CONFIRM)).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Keep editing' })); + + await waitFor(() => expect(screen.queryByText(CONFIRM)).toBeNull()); + expect(router.state.location.pathname).toBe('/settings/general'); + expect(screen.getByLabelText('Timezone (IANA)')).toHaveValue('America/New_York'); + }); + + it('discards the draft and runs the parked Settings navigation', async () => { + const router = await renderTab(); + fireEvent.change(screen.getByLabelText('Longitude (-180 to 180)'), { + target: { value: '-74.006' }, + }); + + await navigate(router, '/settings/security'); + expect(screen.getByText(CONFIRM)).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Discard' })); + + expect(await screen.findByText('Security settings')).toBeInTheDocument(); + expect(router.state.location.pathname).toBe('/settings/security'); + }); + + it('resets the draft when Settings preserves General across a stale tab route', async () => { + const router = createMemoryRouter([ + { path: '/settings/:tab', element: }, + ], { initialEntries: ['/settings/old-tab'] }); + render(); + const timezoneInput = await screen.findByDisplayValue('UTC'); + fireEvent.change(timezoneInput, { target: { value: 'America/New_York' } }); + + await navigate(router, '/settings/general'); + expect(screen.getByText(CONFIRM)).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Discard' })); + + await waitFor(() => expect(router.state.location.pathname).toBe('/settings/general')); + expect(screen.getByLabelText('Timezone (IANA)')).toHaveValue('UTC'); + expect(screen.queryByText('Unsaved changes')).toBeNull(); + }); + + it('advances only the successful section baseline', async () => { + const router = await renderTab(); + fireEvent.change(screen.getByLabelText('Timezone (IANA)'), { + target: { value: 'America/New_York' }, + }); + fireEvent.change(screen.getByLabelText('Latitude (-90 to 90)'), { + target: { value: '40.7128' }, + }); + + await act(async () => { + fireEvent.click(within(timezoneCard()).getByRole('button', { name: 'Save' })); + }); + expect(updateSettings).toHaveBeenCalledWith( + { timezone: 'America/New_York' }, + { silent: true }, + ); + expect(within(timezoneCard()).queryByText('Unsaved changes')).toBeNull(); + expect(within(locationCard()).getByText('Unsaved changes')).toBeInTheDocument(); + + await navigate(router, '/settings/security'); + expect(screen.getByText(CONFIRM)).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Keep editing' })); + + await act(async () => { + fireEvent.click(within(locationCard()).getByRole('button', { name: 'Save' })); + }); + expect(updateSettings).toHaveBeenLastCalledWith( + { location: { lat: 40.7128, lon: SETTINGS.location.lon } }, + { silent: true }, + ); + expect(screen.queryByText('Unsaved changes')).toBeNull(); + }); + + it('locks each section during its save without hiding another section\'s discard prompt', async () => { + const timezoneSave = deferred(); + const locationSave = deferred(); + updateSettings + .mockReturnValueOnce(timezoneSave.promise) + .mockReturnValueOnce(locationSave.promise); + const router = await renderTab(); + const timezoneInput = screen.getByLabelText('Timezone (IANA)'); + const latitudeInput = screen.getByLabelText('Latitude (-90 to 90)'); + const longitudeInput = screen.getByLabelText('Longitude (-180 to 180)'); + fireEvent.change(timezoneInput, { target: { value: 'America/New_York' } }); + fireEvent.change(latitudeInput, { target: { value: '40.7128' } }); + + fireEvent.click(within(timezoneCard()).getByRole('button', { name: 'Save' })); + expect(timezoneInput).toBeDisabled(); + expect(latitudeInput).not.toBeDisabled(); + await navigate(router, '/settings/security'); + expect(screen.getByText(CONFIRM)).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Keep editing' })); + + await act(async () => { timezoneSave.resolve({}); }); + fireEvent.click(within(locationCard()).getByRole('button', { name: 'Save' })); + expect(latitudeInput).toBeDisabled(); + expect(longitudeInput).toBeDisabled(); + await navigate(router, '/settings/security'); + expect(screen.queryByText(CONFIRM)).toBeNull(); + await act(async () => { locationSave.resolve({}); }); + expect(await screen.findByText('Security settings')).toBeInTheDocument(); + }); + + it('discards only the unsaved section while another section save is in flight', async () => { + const timezoneSave = deferred(); + updateSettings.mockReturnValueOnce(timezoneSave.promise); + const router = createMemoryRouter([ + { path: '/settings/:tab', element: }, + ], { initialEntries: ['/settings/old-tab'] }); + render(); + const timezoneInput = await screen.findByDisplayValue('UTC'); + const latitudeInput = screen.getByLabelText('Latitude (-90 to 90)'); + fireEvent.change(timezoneInput, { target: { value: 'America/New_York' } }); + fireEvent.change(latitudeInput, { target: { value: '40.7128' } }); + fireEvent.click(within(timezoneCard()).getByRole('button', { name: 'Save' })); + + await navigate(router, '/settings/general'); + expect(screen.getByText(CONFIRM)).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Discard' })); + + await waitFor(() => expect(router.state.location.pathname).toBe('/settings/general')); + expect(timezoneInput).toHaveValue('America/New_York'); + expect(timezoneInput).toBeDisabled(); + expect(latitudeInput).toHaveValue(String(SETTINGS.location.lat)); + await act(async () => { timezoneSave.resolve({}); }); + expect(timezoneInput).toHaveValue('America/New_York'); + expect(screen.queryByText('Unsaved changes')).toBeNull(); + }); + + it('keeps a failed timezone save dirty and guarded', async () => { + updateSettings.mockRejectedValueOnce(new Error('timezone offline')); + const router = await renderTab(); + fireEvent.change(screen.getByLabelText('Timezone (IANA)'), { + target: { value: 'America/New_York' }, + }); + + await act(async () => { + fireEvent.click(within(timezoneCard()).getByRole('button', { name: 'Save' })); + }); + expect(within(timezoneCard()).getByText('Unsaved changes')).toBeInTheDocument(); + + await navigate(router, '/settings/security'); + expect(screen.getByText(CONFIRM)).toBeInTheDocument(); + }); + + it('keeps a failed location save dirty and guarded', async () => { + updateSettings.mockRejectedValueOnce(new Error('location offline')); + const router = await renderTab(); + fireEvent.change(screen.getByLabelText('Longitude (-180 to 180)'), { + target: { value: '-74.006' }, + }); + + await act(async () => { + fireEvent.click(within(locationCard()).getByRole('button', { name: 'Save' })); + }); + expect(within(locationCard()).getByText('Unsaved changes')).toBeInTheDocument(); + + await navigate(router, '/settings/security'); + expect(screen.getByText(CONFIRM)).toBeInTheDocument(); + }); +});