diff --git a/apps/citizen-pwa/public/sw.js b/apps/citizen-pwa/public/sw.js index 8a9eac0b..a9ad6faf 100644 --- a/apps/citizen-pwa/public/sw.js +++ b/apps/citizen-pwa/public/sw.js @@ -1,4 +1,4 @@ -const CACHE_NAME = 'bantayog_shell_v1' +const CACHE_NAME = 'bantayog_shell_v2' // WARNING: This must match the localforage instance name in // apps/citizen-pwa/src/services/draft-store.ts ('bantayog-drafts'). // The SW uses raw IndexedDB; localforage wraps IndexedDB with its own @@ -8,8 +8,35 @@ const CACHE_NAME = 'bantayog_shell_v1' const DB_NAME = 'bantayog-drafts' const DB_STORE = 'drafts' +// Precache the app shell so a cold offline navigation falls back to +// index.html instead of the browser's default offline page. Vite's hashed +// JS/CSS chunks are still cached opportunistically by the fetch handler. +// +// REQUIRED: core shell files — install fails if any of these can't be cached +// so the old worker stays active rather than serving a broken shell. +const REQUIRED_URLS = ['/', '/index.html'] +// OPTIONAL: manifest + icons — best-effort; a transient 404 here should not +// block the entire install. +const OPTIONAL_URLS = ['/manifest.webmanifest', '/icons/icon-192.png', '/icons/icon-512.png'] + self.addEventListener('install', (event) => { - self.skipWaiting() + event.waitUntil( + caches + .open(CACHE_NAME) + .then(async (cache) => { + // Required URLs must all succeed; let any failure reject install. + await cache.addAll(REQUIRED_URLS) + // Optional URLs are best-effort; log and continue on individual failure. + await Promise.allSettled( + OPTIONAL_URLS.map((url) => + cache.add(url).catch((err) => { + console.warn('[SW] optional precache failed for', url, err) + }), + ), + ) + }) + .then(() => self.skipWaiting()), + ) }) self.addEventListener('activate', (event) => { @@ -41,9 +68,17 @@ self.addEventListener('fetch', (event) => { } return response }) - .catch(() => { + .catch(async () => { + // Navigation requests (SPA routes) fall back to the cached shell so + // React Router can render the right view instead of the browser + // showing its default offline page. + if (event.request.mode === 'navigate') { + const shell = await caches.match('/index.html') + if (shell) return shell + } if (event.request.method === 'GET') { - return caches.match(event.request) + const cached = await caches.match(event.request) + if (cached) return cached } throw new Error('not found') }), diff --git a/apps/citizen-pwa/src/components/SubmitReportForm/Step1Evidence.tsx b/apps/citizen-pwa/src/components/SubmitReportForm/Step1Evidence.tsx index e18ac7bb..7426da84 100644 --- a/apps/citizen-pwa/src/components/SubmitReportForm/Step1Evidence.tsx +++ b/apps/citizen-pwa/src/components/SubmitReportForm/Step1Evidence.tsx @@ -21,6 +21,7 @@ interface Step1EvidenceProps { onNext: (data: { reportType: string; photoFile: File | null }) => void onBack: () => void isSubmitting?: boolean + initialReportType?: string } const INCIDENT_TYPES = [ @@ -98,8 +99,17 @@ const INCIDENT_TYPES = [ }, ] as const -export function Step1Evidence({ onNext, onBack, isSubmitting = false }: Step1EvidenceProps) { - const [reportType, setReportType] = useState('flood') +export function Step1Evidence({ + onNext, + onBack, + isSubmitting = false, + initialReportType = '', +}: Step1EvidenceProps) { + // Default to empty so the user must actively pick a type. This is what + // gates the "Skip photo for now" path — without it, Skip silently + // submitted whatever the seeded default was. + const [reportType, setReportType] = useState(initialReportType) + const [reportTypeError, setReportTypeError] = useState(null) const [photoFile, setPhotoFile] = useState(null) const [photoError, setPhotoError] = useState(null) const fileInputRef = useRef(null) @@ -170,6 +180,11 @@ export function Step1Evidence({ onNext, onBack, isSubmitting = false }: Step1Evi } const handleNext = () => { + if (!reportType) { + setReportTypeError('Please select an incident type to continue.') + return + } + setReportTypeError(null) onNext({ reportType, photoFile }) } @@ -293,6 +308,7 @@ export function Step1Evidence({ onNext, onBack, isSubmitting = false }: Step1Evi type="button" onClick={() => { setReportType(value) + setReportTypeError(null) }} className={`flex flex-col items-center justify-center gap-2 min-h-[80px] rounded-xl border-2 transition-all active:scale-95 ${ isSelected @@ -310,6 +326,11 @@ export function Step1Evidence({ onNext, onBack, isSubmitting = false }: Step1Evi ) })} + {reportTypeError && ( +

+ {reportTypeError} +

+ )} diff --git a/apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx b/apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx index 17ffccf7..61c9c8fd 100644 --- a/apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx +++ b/apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx @@ -24,9 +24,24 @@ interface Step2WhoWhereProps { }) => void onBack: () => void isSubmitting?: boolean + initialValues?: { + location?: { lat: number; lng: number } + reporterName?: string + reporterMsisdn?: string + patientCount?: number + locationMethod?: 'gps' | 'manual' + municipalityId?: string + barangayId?: string + nearestLandmark?: string + } } -export function Step2WhoWhere({ onNext, onBack, isSubmitting = false }: Step2WhoWhereProps) { +export function Step2WhoWhere({ + onNext, + onBack, + isSubmitting = false, + initialValues, +}: Step2WhoWhereProps) { const { location, locationMethod, @@ -54,6 +69,28 @@ export function Step2WhoWhere({ onNext, onBack, isSubmitting = false }: Step2Who const [hasMemory, setHasMemory] = useState(false) const [municipalityError, setMunicipalityError] = useState(null) + // Hydrate from snapshot when resuming or navigating back to Step 2. + useEffect(() => { + if (!initialValues) return + + if (initialValues.locationMethod) setLocationMethod(initialValues.locationMethod) + // eslint-disable-next-line react-hooks/set-state-in-effect + if (initialValues.reporterName) setReporterName(initialValues.reporterName) + + if (initialValues.reporterMsisdn) setReporterMsisdn(initialValues.reporterMsisdn) + if (initialValues.patientCount) { + setPatientCount(initialValues.patientCount) + + setAnyoneHurt(initialValues.patientCount > 0) + } + + if (initialValues.nearestLandmark) setNearestLandmark(initialValues.nearestLandmark) + // Municipality / barangay are handled via useMunicipalityBarangays; those + // hooks don't expose setters, so we rely on localStorage/sessionStorage + // pre-fill below for reporter fields, and the user re-selects location. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []) + useEffect(() => { try { const savedName = localStorage.getItem('bantayog.reporter.name') @@ -307,9 +344,15 @@ export function Step2WhoWhere({ onNext, onBack, isSubmitting = false }: Step2Who )} - {/* Bottom action */} - {locationMethod !== null && ( -
+ {/* Bottom action — always rendered so users always see what to do next. + Before a location method is picked, show a polite hint instead of + hiding the entire region (which previously felt like silent failure). */} +
+ {locationMethod === null ? ( +

+ Pick a location method above (GPS or Manual) to continue. +

+ ) : ( -
- )} + )} +
) } diff --git a/apps/citizen-pwa/src/components/SubmitReportForm/index.tsx b/apps/citizen-pwa/src/components/SubmitReportForm/index.tsx index be983b7a..071bf5b0 100644 --- a/apps/citizen-pwa/src/components/SubmitReportForm/index.tsx +++ b/apps/citizen-pwa/src/components/SubmitReportForm/index.tsx @@ -4,6 +4,7 @@ import { normalizeMsisdn } from '@bantayog/shared-validators' import type { ReportType } from '@bantayog/shared-types' import { createDraft } from '../../services/submit-report.js' import type { Draft } from '../../services/draft-store.js' +import { wizardSnapshot } from '../../services/wizard-snapshot.js' import { useSubmissionMachine } from '../../hooks/useSubmissionMachine.js' import { Step1Evidence } from './Step1Evidence.js' import { Step2WhoWhere } from './Step2WhoWhere.js' @@ -41,6 +42,7 @@ export function SubmitReportForm() { function WizardContainer() { const nav = useNavigate() + const [hasLoadedSnapshot, setHasLoadedSnapshot] = useState(false) const [step, setStep] = useState<1 | 2 | 3>(1) const [formData, setFormData] = useState({ step1: null, step2: null }) const [draft, setDraft] = useState(null) @@ -48,6 +50,43 @@ function WizardContainer() { const [isCreatingDraft, setIsCreatingDraft] = useState(false) const [draftError, setDraftError] = useState(null) + // Resume an in-progress wizard from a prior session (refresh, accidental close). + // photoFile is intentionally not persisted — File can't be reliably serialized; + // the user re-attaches if needed by going back to Step 1. + useEffect(() => { + let cancelled = false + void wizardSnapshot + .load() + .then((snap) => { + if (cancelled) return + if (snap) { + setStep(snap.step) + setFormData({ + step1: snap.step1 ? { reportType: snap.step1.reportType, photoFile: null } : null, + step2: snap.step2 ?? null, + }) + } + setHasLoadedSnapshot(true) + }) + .catch(() => { + if (!cancelled) setHasLoadedSnapshot(true) + }) + return () => { + cancelled = true + } + }, []) + + // Persist after load + on every step/formData change. The hasLoadedSnapshot + // gate prevents the initial empty state from clobbering a fresh resume. + useEffect(() => { + if (!hasLoadedSnapshot) return + void wizardSnapshot.save({ + step, + step1: formData.step1 ? { reportType: formData.step1.reportType } : null, + step2: formData.step2, + }) + }, [hasLoadedSnapshot, step, formData]) + const handleStep1Next = (data: Step1Data) => { setFormData((prev) => ({ ...prev, step1: data })) setStep(2) @@ -108,6 +147,8 @@ function WizardContainer() { setDraft(created) setSecret(draftSecret) + // Prevent stale snapshot from causing a second draft on refresh. + await wizardSnapshot.clear() } catch (err: unknown) { setDraftError(err instanceof Error ? err.message : 'Failed to create draft') } finally { @@ -116,15 +157,31 @@ function WizardContainer() { } const handleStep1Back = () => { + // User abandoned the wizard from Step 1 — drop the snapshot so a fresh + // /report visit starts clean rather than resuming the old draft. + void wizardSnapshot.clear() void nav('/') } + if (!hasLoadedSnapshot) { + return ( +
+
+
+ ) + } + if (draft) { return ( { + void wizardSnapshot.clear() void nav(`/reports/${publicRef}`) }} /> @@ -137,6 +194,7 @@ function WizardContainer() { onNext={handleStep1Next} onBack={handleStep1Back} isSubmitting={isCreatingDraft} + initialReportType={formData.step1?.reportType ?? ''} /> ) } @@ -147,6 +205,7 @@ function WizardContainer() { onNext={handleStep2Next} onBack={handleStep2Back} isSubmitting={isCreatingDraft} + {...(formData.step2 ? { initialValues: formData.step2 } : {})} /> ) } diff --git a/apps/citizen-pwa/src/pages/LoginPage.tsx b/apps/citizen-pwa/src/pages/LoginPage.tsx index 2388317a..5df38d51 100644 --- a/apps/citizen-pwa/src/pages/LoginPage.tsx +++ b/apps/citizen-pwa/src/pages/LoginPage.tsx @@ -10,12 +10,15 @@ import { import { useToast } from '../hooks/useToast.js' import { Toast } from '../components/Toast.js' import { auth, hasFirebaseConfig } from '../services/firebase.js' +import { getStoredPhone, setStoredPhone } from '../services/phone-session-storage.js' export function LoginPage() { const navigate = useNavigate() const { show, message, type, toast } = useToast() const [step, setStep] = useState<'phone' | 'otp'>('phone') - const [phone, setPhone] = useState('+63') + // Seed from sessionStorage so users who bounce between /login and /register + // (e.g. wrong-account → register flow) keep the phone they already typed. + const [phone, setPhone] = useState(() => getStoredPhone()) const [otp, setOtp] = useState('') const [loading, setLoading] = useState(false) const [verificationId, setVerificationId] = useState(null) @@ -151,7 +154,9 @@ export function LoginPage() { type="tel" value={phone} onChange={(e) => { - setPhone(e.target.value) + const next = e.target.value + setPhone(next) + setStoredPhone(next) }} placeholder="+63 XXX XXX XXXX" className="w-full pl-10 pr-4 py-3 border border-surface-300 rounded-lg focus:ring-2 focus:ring-brand-500 focus:border-brand-500 outline-none" diff --git a/apps/citizen-pwa/src/pages/RegisterPage.tsx b/apps/citizen-pwa/src/pages/RegisterPage.tsx index a210073e..17f8aaf7 100644 --- a/apps/citizen-pwa/src/pages/RegisterPage.tsx +++ b/apps/citizen-pwa/src/pages/RegisterPage.tsx @@ -6,6 +6,7 @@ import { auth } from '../services/firebase.js' import { registerCitizen } from '../services/callables.js' import { useToast } from '../hooks/useToast.js' import { Toast } from '../components/Toast.js' +import { getStoredPhone, setStoredPhone } from '../services/phone-session-storage.js' type Step = 'phone' | 'otp' | 'name' | 'consent' @@ -18,7 +19,13 @@ export function RegisterPage() { const isResume = searchParams.get('resume') === 'registration' const [step, setStep] = useState(isResume ? 'consent' : 'phone') - const [phone, setPhone] = useState('+63') + // Seed from sessionStorage so users who bounce here from /login (or back) + // do not have to retype the +63 prefix. The resume effect below still + // overrides this with `currentUser.phoneNumber` when applicable. + const [phone, setPhone] = useState(() => { + if (isResume) return '+63' + return getStoredPhone() + }) const [otp, setOtp] = useState('') const [displayName, setDisplayName] = useState('') const [consentGiven, setConsentGiven] = useState(false) @@ -165,14 +172,30 @@ export function RegisterPage() { {step === 'phone' && (
-

+

+ { - setPhone(e.target.value) + const raw = e.target.value + // Strip non-digits except the leading +, trim whitespace, + // enforce a sensible max length to avoid junk in storage. + const normalized = raw + .trim() + .replace(/[^+\d]/g, '') + .slice(0, 16) + setPhone(normalized) + if (normalized.length <= 16) { + setStoredPhone(normalized) + } }} placeholder="+63XXXXXXXXXX" className="w-full h-14 rounded-xl border border-[#d5dedd] px-4 text-base focus:border-[#0f9488] focus:outline-none mb-4" diff --git a/apps/citizen-pwa/src/pages/SettingsPage.tsx b/apps/citizen-pwa/src/pages/SettingsPage.tsx index 5820ed25..9fbd724b 100644 --- a/apps/citizen-pwa/src/pages/SettingsPage.tsx +++ b/apps/citizen-pwa/src/pages/SettingsPage.tsx @@ -115,7 +115,7 @@ export function SettingsPage() { } return ( -
+
{/* Back header */}
- + Receive a JSON export of your profile, reports, and media.
@@ -236,7 +236,7 @@ export function SettingsPage() {
{/* Danger Zone section */} -

+

Danger Zone

@@ -249,6 +249,6 @@ export function SettingsPage() {
-
+ ) } diff --git a/apps/citizen-pwa/src/services/phone-session-storage.ts b/apps/citizen-pwa/src/services/phone-session-storage.ts new file mode 100644 index 00000000..46b7c1d0 --- /dev/null +++ b/apps/citizen-pwa/src/services/phone-session-storage.ts @@ -0,0 +1,38 @@ +const KEY = 'bantayog.last-phone' +const DEFAULT_PHONE = '+63' + +function isBenignStorageError(err: unknown): boolean { + if (!(err instanceof Error)) return false + const name = err.name + const msg = err.message.toLowerCase() + return ( + name === 'QuotaExceededError' || + name === 'NS_ERROR_DOM_QUOTA_REACHED' || + name === 'SecurityError' || + msg.includes('quota') || + msg.includes('private') || + msg.includes('denied') + ) +} + +export function getStoredPhone(): string { + try { + return sessionStorage.getItem(KEY) ?? DEFAULT_PHONE + } catch (err: unknown) { + if (!isBenignStorageError(err)) { + console.warn('[phone-session-storage] Unexpected error reading last-phone:', err) + } + return DEFAULT_PHONE + } +} + +export function setStoredPhone(phone: string): void { + try { + sessionStorage.setItem(KEY, phone) + } catch (err: unknown) { + if (!isBenignStorageError(err)) { + console.warn('[phone-session-storage] Unexpected error writing last-phone:', err) + } + // Private mode / quota / security errors — best effort persistence. + } +} diff --git a/apps/citizen-pwa/src/services/wizard-snapshot.test.ts b/apps/citizen-pwa/src/services/wizard-snapshot.test.ts new file mode 100644 index 00000000..5785352e --- /dev/null +++ b/apps/citizen-pwa/src/services/wizard-snapshot.test.ts @@ -0,0 +1,112 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' + +const { mockInstance } = vi.hoisted(() => { + const store = new Map() + const mockInstance = { + getItem: vi.fn((key: string) => Promise.resolve(store.get(key) ?? null)), + setItem: vi.fn((key: string, value: unknown) => { + store.set(key, value) + return Promise.resolve(value) + }), + removeItem: vi.fn((key: string) => { + store.delete(key) + return Promise.resolve() + }), + _store: store, + } + return { mockInstance } +}) + +vi.mock('localforage', () => ({ + default: { createInstance: () => mockInstance }, +})) + +import { + wizardSnapshot, + _resetWizardSnapshotCache, + type WizardSnapshot, +} from './wizard-snapshot.js' + +const STORAGE_KEY = 'wizard-in-progress' + +beforeEach(() => { + mockInstance._store.clear() + vi.clearAllMocks() + _resetWizardSnapshotCache() + vi.useFakeTimers({ now: Date.now() }) +}) + +afterEach(() => { + vi.useRealTimers() +}) + +describe('wizardSnapshot.load', () => { + it('returns null when no snapshot exists', async () => { + expect(await wizardSnapshot.load()).toBeNull() + }) + + it('returns the snapshot when fresh', async () => { + await wizardSnapshot.save({ + step: 2, + step1: { reportType: 'flood' }, + step2: null, + }) + const loaded = await wizardSnapshot.load() + expect(loaded?.step).toBe(2) + expect(loaded?.step1?.reportType).toBe('flood') + expect(loaded?.step2).toBeNull() + }) + + it('returns null when snapshot exceeds 24h TTL and clears stale data', async () => { + await wizardSnapshot.save({ step: 1, step1: null, step2: null }) + vi.setSystemTime(Date.now() + 25 * 60 * 60 * 1000) + expect(await wizardSnapshot.load()).toBeNull() + expect(mockInstance.removeItem).toHaveBeenCalledOnce() + }) + + it('returns null when localforage rejects on read', async () => { + mockInstance.getItem.mockRejectedValueOnce(new Error('idb broken')) + expect(await wizardSnapshot.load()).toBeNull() + }) +}) + +describe('wizardSnapshot.save', () => { + it('persists step + formData and stamps updatedAt', async () => { + const before = Date.now() + await wizardSnapshot.save({ + step: 3, + step1: { reportType: 'fire' }, + step2: { + location: { lat: 14.1, lng: 122.9 }, + reporterName: 'Juan', + reporterMsisdn: '+639171234567', + patientCount: 0, + locationMethod: 'gps', + }, + }) + const stored = mockInstance._store.get(STORAGE_KEY) as WizardSnapshot + expect(stored.step).toBe(3) + expect(stored.step2?.reporterName).toBe('Juan') + expect(stored.updatedAt).toBeGreaterThanOrEqual(before) + }) + + it('does not throw when localforage rejects on save', async () => { + mockInstance.setItem.mockRejectedValueOnce(new Error('idb broken')) + await expect( + wizardSnapshot.save({ step: 1, step1: null, step2: null }), + ).resolves.toBeUndefined() + }) +}) + +describe('wizardSnapshot.clear', () => { + it('removes the snapshot', async () => { + await wizardSnapshot.save({ step: 1, step1: { reportType: 'flood' }, step2: null }) + await wizardSnapshot.clear() + expect(await wizardSnapshot.load()).toBeNull() + }) + + it('does not throw when removeItem rejects', async () => { + mockInstance.removeItem.mockRejectedValueOnce(new Error('idb broken')) + await expect(wizardSnapshot.clear()).resolves.toBeUndefined() + }) +}) diff --git a/apps/citizen-pwa/src/services/wizard-snapshot.ts b/apps/citizen-pwa/src/services/wizard-snapshot.ts new file mode 100644 index 00000000..06225e74 --- /dev/null +++ b/apps/citizen-pwa/src/services/wizard-snapshot.ts @@ -0,0 +1,81 @@ +import localforage from 'localforage' + +interface SerializableStep1 { + reportType: string +} + +interface SerializableStep2 { + location: { lat: number; lng: number } + reporterName: string + reporterMsisdn: string + patientCount: number + locationMethod: 'gps' | 'manual' + municipalityId?: string + municipalityLabel?: string + barangayId?: string + nearestLandmark?: string +} + +export interface WizardSnapshot { + step: 1 | 2 | 3 + step1: SerializableStep1 | null + step2: SerializableStep2 | null + updatedAt: number +} + +const TTL_MS = 24 * 60 * 60 * 1000 +const STORAGE_KEY = 'wizard-in-progress' + +interface SnapshotStorage { + getItem(key: string): Promise + setItem(key: string, value: WizardSnapshot): Promise + removeItem(key: string): Promise +} + +let _storage: SnapshotStorage | undefined + +export function _resetWizardSnapshotCache(): void { + _storage = undefined +} + +function getStorage(): SnapshotStorage { + _storage ??= localforage.createInstance({ + name: 'bantayog-wizard', + storeName: 'snapshot', + }) as unknown as SnapshotStorage + return _storage +} + +export const wizardSnapshot = { + async load(): Promise { + let snapshot: WizardSnapshot | null + try { + snapshot = await getStorage().getItem(STORAGE_KEY) + } catch { + return null + } + if (!snapshot) return null + if (Date.now() - snapshot.updatedAt > TTL_MS) { + await this.clear() + return null + } + return snapshot + }, + + async save(partial: Omit): Promise { + const toSave: WizardSnapshot = { ...partial, updatedAt: Date.now() } + try { + await getStorage().setItem(STORAGE_KEY, toSave) + } catch { + // Best-effort persistence — don't block step transitions if IDB fails. + } + }, + + async clear(): Promise { + try { + await getStorage().removeItem(STORAGE_KEY) + } catch { + // Best-effort. + } + }, +} diff --git a/docs/learnings.md b/docs/learnings.md index 2f0faef4..a5dacafe 100644 --- a/docs/learnings.md +++ b/docs/learnings.md @@ -7,6 +7,11 @@ - Passing navigation callbacks as props (e.g., `onReportSimilar={() => void navigate(...)}`) avoids `useNavigate` being called in components tested without a Router context — the pattern is cleaner than wrapping every test with a MemoryRouter. - `subscribeAlerts` from `@bantayog/shared-firebase` takes a raw `Firestore` instance; the citizen-pwa's `db()` helper satisfies this directly. - RevealSheet: spring-eased slide-up (`cubic-bezier(0.34, 1.56, 0.64, 1)`) + `max-height: 90svh` scroll guard are the minimum polish required on any bottom-sheet component. +- `role="status"` implicitly carries `aria-live="polite"` + `aria-atomic="true"` per WAI-ARIA spec; adding explicit `aria-live` is redundant noise. +- Async state gates in React (e.g. `hasLoadedSnapshot`) must always resolve — both `.then()` and `.catch()` paths must flip the gate flag, or the component freezes on rejection. +- `cache.addAll()` rejects the entire install if any URL fails; use `Promise.allSettled(cache.add(url).catch(...))` for resilient SW precaching. +- When two files share the same `sessionStorage` key + try/catch + default pattern, extract a shared helper before the pattern drifts. +- TTL tests that write directly to mock stores bypass the code under test. Use `vi.useFakeTimers()` + the public `save()` API + `vi.setSystemTime()` to actually exercise the TTL branch. ## Process @@ -72,6 +77,9 @@ - In `onAuthStateChanged`, guard `.then`/`.catch` with an `active` flag + uid check to prevent stale promises overwriting state. - `awaitFreshAuthToken` must start `getIdToken(true)` inside the Promise constructor so rejection can unsubscribe and reject. - Null-check `awaitFreshAuthToken` before invoking `httpsCallable`; missing user = opaque failure. +- `linkWithPhoneNumber` requires a non-null `currentUser`. If a `/register` route uses it without an upstream `signInAnonymously()` or prior `signInWithPhoneNumber()`, fresh users get "Not signed in" with no recovery path. Either guard the route, sign in anonymously on mount, or use `signInWithPhoneNumber` and link later. +- For surviving phone numbers across adjacent auth flows (login → register), prefer `sessionStorage["bantayog.last-phone"]` over React Router navigation state. State is dropped on reload; sessionStorage isn't, and survives the full-page reCAPTCHA round-trips that phone auth needs. +- `useState(() => sessionStorage.getItem(...))` lazy initializer is safer than `useEffect` + `setState` for storage seeds — no flash of default value, no `react-hooks/set-state-in-effect` warning, and any throw inside the initializer is caught by React (just guard with try/catch for private-mode/security errors). ## Phase 6 Responder App @@ -144,3 +152,19 @@ - Service Worker cannot use Firebase JS SDK (requires bundling); use Firestore REST API (`firestore.googleapis.com/v1/projects/...`) for SW background sync writes. - Idempotency key on the SW write ensures dedup if both SW and in-app machine both succeed for the same draft. - Image compression in the browser: canvas `toBlob('image/jpeg', quality)` is the reliable cross-browser path (avoids `createImageBitmap` + `OffscreenCanvas` compatibility issues). +- Opportunistic dynamic caching alone is not enough for an offline-first PWA — a cold offline boot has nothing to fall back to. Precache the app shell (`/`, `/index.html`, manifest, key icons) on `install` and serve cached `/index.html` for any navigation request that fails the network. SPA shell + React Router then handle per-route offline UI. +- When bumping a SW cache version, the existing `activate` cleanup must already filter for the cache prefix (`bantayog_shell_*` here); otherwise the old cache lingers and precache never re-runs. + +## UX / A11y + +- Conditional rendering of primary action buttons (`{state !== null && }`) reads as "silent failure" — users see no button and no instruction. Prefer always-rendering the action region with one of two states: a `role="status"` hint when the precondition is unmet, or the action button when it is. Pattern matches the offline-banner pattern in `CitizenShell`. +- For WCAG-AA contrast, prefer Tailwind theme tokens (`text-surface-600`, `text-surface-500`) over arbitrary `text-[#hex]` so contrast becomes part of the design-system contract instead of a per-page coincidence. The `surface-400` / `surface-300` tokens (3.1:1 / 4.0:1) are decorative-only — never use them for body text on light backgrounds. +- Routes that bypass the shell (`/settings`, `/register`, `/login`, full-page wizards) need their own `
` element; the skip link inside `CitizenShell` already targets `#main-content`, so reusing the same ID keeps that link consistent across shell and non-shell pages. + +## Wizard / Multi-step Forms + +- "In-progress wizard state" and "finalized draft awaiting submission" are different concerns — keep them in separate stores. `draft-store` requires `publicRef`, `secretHash`, `correlationId`, `idempotencyKey` (all populated at submit); shoehorning partial wizard state into it forces placeholder values that pollute the queued-draft list. A small dedicated `wizard-snapshot` store (localforage instance, 24h TTL, single-record `wizard-in-progress` key) is cleaner. +- Default a required selector to `''`, not a "first" value. A seeded default like `useState('flood')` looks harmless but it lets bypass paths (a "Skip" button calling the same `handleNext`) silently submit the seeded value. Combine with handleNext-level validation + inline `role="alert"` error. +- Do not persist `File`/`Blob` in IDB at per-keystroke cadence. Snapshot writes happen on every step transition; a 5 MB photo encoded into the snapshot bloats every write. Persist scalar form data only; let the user re-attach photos on resume (acceptable since photos are usually optional). +- Mid-form persistence requires accepting initial-value props on each step component. Without `initialReportType` etc., a back-navigation from Step 2 to Step 1 (or a refresh that resumes mid-wizard) re-mounts the step with fresh defaults and silently loses the user's input. +- Snapshot save effect must be gated on a `hasLoadedSnapshot` boolean, otherwise the initial empty `formData` clobbers the just-loaded snapshot before the resume effect commits. Two-effect pattern: load on mount, then enable saves. diff --git a/docs/progress.md b/docs/progress.md index f7db48dd..94205a3d 100644 --- a/docs/progress.md +++ b/docs/progress.md @@ -1,5 +1,86 @@ # Progress +## 2026-05-03 — PR #91 Review Follow-ups (branch: `fix/citizen-pwa-auth-and-wizard-followups`) + +Addressed all Sourcery-ai and CodeRabbit review comments on PR #91. + +| Source | Issue | Fix | Files | +| ---------- | ------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------- | +| Sourcery | SW install blocked if any precache URL fails | `cache.addAll` → `Promise.allSettled(cache.add(url).catch(...))` — individual failures logged, SW still activates | `apps/citizen-pwa/public/sw.js` | +| Sourcery | `hasLoadedSnapshot` gate rendered empty `aria-hidden` div | Replaced with spinner + `role="status"` + `aria-label="Loading report wizard"` | `components/SubmitReportForm/index.tsx` | +| Sourcery | `sessionStorage["bantayog.last-phone"]` duplicated in LoginPage + RegisterPage | Extracted `services/phone-session-storage.ts` (`getStoredPhone` / `setStoredPhone`); both pages use shared helper | `services/phone-session-storage.ts` (NEW) + `pages/LoginPage.tsx` + `pages/RegisterPage.tsx` | +| CodeRabbit | Missing `.catch()` on `wizardSnapshot.load()` — rejection freezes wizard forever | Added `.catch(() => { if (!cancelled) setHasLoadedSnapshot(true) })` | `components/SubmitReportForm/index.tsx` | +| CodeRabbit | `aria-live="polite"` redundant alongside `role="status"` (implicit per WAI-ARIA spec) | Removed explicit `aria-live` attribute | `components/SubmitReportForm/Step2WhoWhere.tsx` | +| CodeRabbit | TTL test false-positive if `STORAGE_KEY` constant drifts from production | Test now uses `vi.useFakeTimers` + public `save()` API + `vi.setSystemTime()` instead of direct store manipulation | `services/wizard-snapshot.test.ts` | + +**Gate:** `pnpm --filter @bantayog/citizen-pwa lint typecheck` clean, +`npx vitest run` 243/243 pass. + +--- + +## 2026-05-02 — Citizen PWA Auth + Wizard Resumability (branch: `fix/citizen-pwa-auth-and-wizard-followups`) + +Round 2 of the QA staging pass. 5 real code bugs from a 9-issue list — the +remaining 4 are either Firebase Console config (Phone Auth disabled) or +already-fixed-in-code waiting on a staging redeploy (commit `90f29ef` +two-step `report_lookup` for tracking 404). + +| # | Issue | Fix | Files | +| ---------- | ---------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------- | +| A-2 | RegisterPage phone input lacked `id`/`name`/`autocomplete="tel"` (a11y warning) | Added all three + a real `