Skip to content

fix(citizen-pwa): QA round 1 + 2 offline shell, a11y, wizard resumability, auth phone preservation#91

Merged
Exc1D merged 5 commits intomainfrom
fix/citizen-pwa-auth-and-wizard-followups
May 2, 2026
Merged

fix(citizen-pwa): QA round 1 + 2 offline shell, a11y, wizard resumability, auth phone preservation#91
Exc1D merged 5 commits intomainfrom
fix/citizen-pwa-auth-and-wizard-followups

Conversation

@Exc1D
Copy link
Copy Markdown
Owner

@Exc1D Exc1D commented May 2, 2026

Summary

Two rounds of QA staging follow-ups bundled into a single PR. 9 issues from the round-1 (10-subagent) sweep + the round-2 auth/wizard sweep, scoped to citizen-pwa.

Round 1 (commit 013e8b5) — 4 issues, 3 source files:

  • Offline shell: precache /, /index.html, manifest, icons; navigation fallback to cached /index.html; cache version v1v2 (apps/citizen-pwa/public/sw.js)
  • Settings WCAG-AA contrast: 6× text-[#768081]text-surface-600, 1× text-[#a3adae]text-surface-500 (SettingsPage.tsx)
  • Settings <main id="main-content"> landmark (same file)
  • Wizard Step 2 silent submit: bottom action region always rendered with role="status" hint until a location method is picked (Step2WhoWhere.tsx)

Round 2 (commit cdcf72b) — 5 real bugs, 6 files (4 modified + 2 new):

  • RegisterPage phone input a11y: id/name/autoComplete="tel" + real <label htmlFor>
  • Phone preservation Login ↔ Register via sessionStorage["bantayog.last-phone"] + useState lazy initializer
  • Step 1 Skip-photo silent submit: reportType defaults to '' instead of seeded 'flood'; handleNext validates with inline role="alert"; type buttons clear the error
  • Wizard resumability: new wizard-snapshot service (localforage, 24h TTL, distinct from draft-store); WizardContainer loads on mount, saves on every step/formData change, clears on submit success and Step-1 back-to-home; Step1Evidence accepts initialReportType prop
  • Photos intentionally not persisted in the snapshot (per-keystroke blob writes would bloat IDB; user re-attaches if needed)

Booked, NOT in this PR (documented in docs/progress.md)

  • A-5 RegisterPage architecturelinkWithPhoneNumber requires currentUser but the codebase has no signInAnonymously upstream. Fresh users hitting /register directly get "Not signed in" with no recovery. Design decision needed (link vs. fresh sign-in vs. anon-then-link).
  • Step 2 back-from-Step-3 field lossStep2WhoWhere mounts fresh on back navigation; needs initialValues props.
  • <main> landmark sweep for the other 9 non-shell pages (Register, Login, NotFound, Goodbye, Onboarding, wizard root, Lookup, Tracking, Receipt, IncidentDetail).
  • /reports/[uuid] 404 is already fixed in main by commit 90f29ef (two-step report_lookup); needs a staging redeploy to verify.
  • Phone Auth OPERATION_NOT_ALLOWED is a Firebase Console config (Authentication → Sign-in method), not code.

Test plan

Local gate (already verified on the merge tip):

  • pnpm --filter @bantayog/citizen-pwa lint — 0 errors (14 pre-existing warnings auto-fixed by lint-staged)
  • pnpm --filter @bantayog/citizen-pwa typecheck — clean
  • npx vitest run — 327/327 pass (54 files; +8 new wizard-snapshot tests)
  • pnpm build — clean; SW + manifest + icons all in dist/

Manual verification (after staging redeploy):

  • Chrome DevTools → Network → Offline → reload / → app shell renders (not the dinosaur)
  • Lighthouse a11y on /settings → score ≥ 95, no contrast violations on section headers / helper text
  • /report → Step 2 (no GPS/Manual picked yet) → "Pick a location method above…" hint visible
  • /login → enter +639XXXXXXXXX → navigate /register → confirm phone preserved
  • /report → tap "Skip photo for now" without picking a type → confirm "Please select an incident type to continue." inline error
  • /report → pick Flood → Continue → refresh → confirm wizard resumes at Step 2 with Flood retained on Back
  • /report → complete + submit → refresh after success → /report starts fresh (snapshot cleared)

Rollback

git revert -m 1 <merge-sha>
firebase deploy --only hosting:citizen-pwa

🤖 Generated with Claude Code

Summary by Sourcery

Improve the citizen PWA’s offline behavior, accessibility, and report wizard resiliency, including auth phone preservation and mid-form snapshotting.

New Features:

  • Add a wizard snapshot service to persist in-progress report steps with a 24-hour TTL so the multi-step form can resume after refresh or accidental close.

Bug Fixes:

  • Ensure the report wizard no longer silently advances when skipping the photo step without selecting an incident type, showing an inline validation error instead.
  • Fix the Step 2 location step so the action area is always visible, guiding users with a status hint until a location method is chosen.
  • Preserve the entered phone number when users move between Login and Register, including across page reloads.
  • Make the Register page phone input accessible with proper labeling and autocomplete attributes.
  • Ensure offline visits to the PWA load the cached app shell instead of the browser’s default offline error page.

Enhancements:

  • Restore previously selected incident types when navigating back in the report wizard by threading initial values into Step 1.
  • Improve Settings page semantics and contrast by adding a main landmark and replacing hard-coded low-contrast colors with design tokens.
  • Document architectural decisions and UX/a11y patterns around auth flows, offline shell behavior, and multi-step wizard persistence in the project learnings and progress docs.

Tests:

  • Add unit tests for the wizard snapshot service to cover load, save, TTL expiry, and failure scenarios.

Summary by CodeRabbit

  • New Features

    • Resume incomplete reports via a persisted wizard snapshot; wizard load/save is gated to avoid clobbering resumed state
    • Enhanced offline support: app-shell precaching and SPA-friendly navigation fallback
    • Persist last-entered phone across auth flows
  • Bug Fixes

    • Require incident type selection before progressing (affects skip/continue paths)
    • Always-render primary action area with guidance when location method is unset
  • Tests

    • Added unit tests covering snapshot persistence and TTL behavior
  • Documentation

    • Expanded PWA, auth, and multi-step form learnings
  • Accessibility

    • Use semantic main element and theme color tokens for improved contrast

Exc1D and others added 3 commits May 2, 2026 21:19
…y, wizard step 2

Four issues from the 2026-05-02 staging QA pass, kept tight at 3 source files:

- sw.js: precache app shell on install (`/`, `/index.html`, manifest, icons)
  and serve cached `/index.html` for navigation requests that fail the
  network. Cold offline boots now render the SPA shell instead of the
  browser's default offline page. Cache version bumped v1 → v2 so the
  existing activate-cleanup evicts the old cache.
- SettingsPage.tsx: replace 6× `text-[#768081]` and 1× `text-[#a3adae]`
  (3.1:1 / 4.0:1 against their backgrounds — fail WCAG AA) with
  `text-surface-600` / `text-surface-500` (~7.4:1 / ~5.8:1 — pass AA).
  Outer `<div>` → `<main id="main-content">` so the skip-link target works
  on this non-shell route too.
- Step2WhoWhere.tsx: the bottom action region was conditionally rendered on
  `locationMethod !== null`, so before users picked GPS/Manual they saw no
  button and no hint — read as silent failure. Always render the region;
  show a `role="status"` hint until a method is chosen, then render the
  existing Button.

Gate: lint + typecheck clean, 319/319 vitest pass, `pnpm build` emits all
five precache URLs. The broader `<main>` landmark sweep across other
non-shell screens (Register, Login, NotFound, Goodbye, Onboarding, the
wizard root, Lookup, Tracking, Receipt, IncidentDetail) is booked as a
follow-up — out of scope here per the 3-file budget.

Manual verification still required after staging redeploy: offline reload,
Lighthouse a11y on /settings, Step 2 hint visibility.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…p, login↔register phone preservation

Five real bugs from the round-2 staging QA pass, scoped to the
citizen-pwa. The other four findings break out as: Firebase Phone Auth
disabled (Console config, not code), report-tracking 404 fixed in code
by `90f29ef` and waiting on a staging redeploy, Register architecture
(`linkWithPhoneNumber` requires `currentUser` but no `signInAnonymously`
exists upstream — design decision, not a small fix), and Step 2
back-from-Step-3 field loss (deferred — needs `initialValues` props on
`Step2WhoWhere`).

- services/wizard-snapshot.ts (NEW) + .test.ts (8 tests): single-snapshot
  store on its own localforage instance with a 24 h TTL. Distinct from
  `draft-store` because in-progress wizard state has different semantics
  — no publicRef/secretHash yet. Best-effort persistence: storage errors
  fall through silently rather than block step transitions.
- SubmitReportForm/index.tsx: load snapshot on mount, save on every
  step/formData change (gated on `hasLoadedSnapshot` so the initial
  empty state can't clobber a fresh resume), clear on submit success
  and on Step-1 back-to-home. Photo files are NOT persisted (per-keystroke
  blob writes would bloat the snapshot); user re-attaches if needed.
- Step1Evidence.tsx: default `reportType` to `''` instead of seeded
  `'flood'`, accept `initialReportType` so back-from-Step-2 keeps the
  pick. `handleNext` validates and shows inline `role="alert"` error
  when the type is missing — closes the silent path where the
  "Skip photo for now" button bypassed the disabled main button.
- LoginPage.tsx + RegisterPage.tsx: both seed `phone` from
  `sessionStorage["bantayog.last-phone"]` via a `useState` lazy
  initializer, write back on every keystroke. RegisterPage's existing
  `?resume=registration` effect still wins when `currentUser.phoneNumber`
  is present.
- RegisterPage.tsx: phone input gains `id="register-phone"`,
  `name="phone"`, `autoComplete="tel"`, and a real `<label htmlFor>` —
  closes the a11y warning.

Gate: lint + typecheck clean, 327/327 vitest pass (54 files, +8 new
snapshot tests), `pnpm build` clean (SubmitReportForm chunk +1 KB).

Manual verification still required after staging redeploy: see
docs/progress.md for the four-item checklist.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
# Conflicts:
#	docs/learnings.md
#	docs/progress.md
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 2, 2026

Reviewer's Guide

This PR implements two QA rounds for the citizen PWA, focusing on offline shell behavior, a11y improvements, auth phone-number preservation across Login/Register, and wizard resumability via a new snapshot service, plus validation fixes for the report wizard.

Sequence diagram for wizard resumability using wizardSnapshot

sequenceDiagram
  actor User
  participant Browser
  participant WizardContainer
  participant wizardSnapshot

  User->>Browser: Open /report or refresh
  Browser->>WizardContainer: Mount component
  WizardContainer->>wizardSnapshot: load()
  wizardSnapshot-->>WizardContainer: snapshot or null
  alt snapshot exists and fresh
    WizardContainer->>WizardContainer: set step from snapshot.step
    WizardContainer->>WizardContainer: set formData.step1 (photoFile null)
    WizardContainer->>WizardContainer: set formData.step2
  else no snapshot
    WizardContainer->>WizardContainer: keep default step=1, empty formData
  end
  WizardContainer->>WizardContainer: set hasLoadedSnapshot = true
  WizardContainer-->>User: Render wizard (Step1/2/3)

  loop On every step or formData change
    WizardContainer->>wizardSnapshot: save({ step, step1: { reportType }, step2 })
    wizardSnapshot->>wizardSnapshot: stamp updatedAt
    wizardSnapshot-->>WizardContainer: persist best-effort
  end

  alt User taps Back on Step1
    WizardContainer->>wizardSnapshot: clear()
    wizardSnapshot-->>WizardContainer: snapshot removed (best-effort)
    WizardContainer->>Browser: navigate('/')
  else User completes submission
    WizardContainer->>wizardSnapshot: clear()
    wizardSnapshot-->>WizardContainer: snapshot removed (best-effort)
    WizardContainer->>Browser: navigate('/reports/{publicRef}')
  end
Loading

Sequence diagram for phone preservation between LoginPage and RegisterPage

sequenceDiagram
  actor User
  participant LoginPage
  participant sessionStorage
  participant RegisterPage

  User->>LoginPage: Visit /login
  LoginPage->>sessionStorage: getItem(bantayog.last-phone)
  alt value exists
    sessionStorage-->>LoginPage: "+639XXXXXXXXX"
    LoginPage->>LoginPage: phone state = stored value
  else no value or error
    sessionStorage-->>LoginPage: null or throw
    LoginPage->>LoginPage: phone state = "+63"
  end
  LoginPage-->>User: Phone input prefilled

  User->>LoginPage: Type phone number
  LoginPage->>LoginPage: setPhone(next)
  LoginPage->>sessionStorage: setItem(bantayog.last-phone, next) (best-effort)

  User->>LoginPage: Navigate to /register
  User->>RegisterPage: Visit /register
  RegisterPage->>RegisterPage: Determine isResume from searchParams
  alt isResume
    RegisterPage->>RegisterPage: phone state = "+63"
  else not resume
    RegisterPage->>sessionStorage: getItem(bantayog.last-phone)
    alt value exists
      sessionStorage-->>RegisterPage: "+639XXXXXXXXX"
      RegisterPage->>RegisterPage: phone state = stored value
    else no value or error
      sessionStorage-->>RegisterPage: null or throw
      RegisterPage->>RegisterPage: phone state = "+63"
    end
  end
  RegisterPage-->>User: Phone input shows preserved value

  User->>RegisterPage: Edit phone (optional)
  RegisterPage->>RegisterPage: setPhone(next)
  RegisterPage->>sessionStorage: setItem(bantayog.last-phone, next) (best-effort)
Loading

Class diagram for wizardSnapshot service and SubmitReportForm components

classDiagram
  class SerializableStep1 {
    +string reportType
  }

  class SerializableStep2 {
    +number location_lat
    +number location_lng
    +string reporterName
    +string reporterMsisdn
    +number patientCount
    +string locationMethod  // 'gps' | 'manual'
    +string municipalityId
    +string municipalityLabel
    +string barangayId
    +string nearestLandmark
  }

  class WizardSnapshot {
    +number step  // 1 | 2 | 3
    +SerializableStep1 step1
    +SerializableStep2 step2
    +number updatedAt
  }

  class WizardSnapshotStore {
    +load() WizardSnapshot
    +save(step, step1, step2) void
    +clear() void
  }

  class WizardContainer {
    -boolean hasLoadedSnapshot
    -number step  // 1 | 2 | 3
    -FormData formData
    -Draft draft
    -string secret
    -boolean isCreatingDraft
    -string draftError
    +handleStep1Next(step1Data)
    +handleStep2Next(step2Data)
    +handleStep2Back()
    +handleStep1Back()
  }

  class Step1EvidenceProps {
    +function onNext(data)
    +function onBack()
    +boolean isSubmitting
    +string initialReportType
  }

  class Step1Evidence {
    -string reportType
    -string reportTypeError
    -File photoFile
    -string photoError
    +handleNext()
  }

  class Step2WhoWhere {
    -string locationMethod  // gps | manual | null
  }

  class LoginPage {
    -string phone
    +setPhone(next)
  }

  class RegisterPage {
    -string phone
    +setPhone(next)
  }

  WizardSnapshotStore --> WizardSnapshot : uses
  WizardContainer --> WizardSnapshotStore : load/save/clear
  WizardContainer --> Step1Evidence : renders
  WizardContainer --> Step2WhoWhere : renders
  Step1Evidence ..> Step1EvidenceProps : implements
  WizardSnapshot --> SerializableStep1 : has
  WizardSnapshot --> SerializableStep2 : has
  LoginPage --> SessionStorageHelper : read/write last phone
  RegisterPage --> SessionStorageHelper : read/write last phone

  class SessionStorageHelper {
    +getLastPhone() string
    +setLastPhone(phone) void
  }
Loading

File-Level Changes

Change Details Files
Add resumable report wizard state via a dedicated wizard snapshot store, and wire it into the SubmitReportForm flow.
  • Introduce wizard-snapshot service backed by localforage with a 24h TTL, encapsulating load/save/clear and handling IDB failures gracefully.
  • Gate snapshot saving behind a hasLoadedSnapshot flag to avoid clobbering restored state, and load snapshots on WizardContainer mount while omitting photo blobs from persistence.
  • Clear snapshots when the user abandons from Step 1 or after successful submission, and block wizard rendering with a shell-only placeholder until snapshot load completes.
  • Pass initialReportType from WizardContainer into Step1Evidence so resuming or navigating back to Step 1 preserves the selected incident type.
apps/citizen-pwa/src/components/SubmitReportForm/index.tsx
apps/citizen-pwa/src/components/SubmitReportForm/Step1Evidence.tsx
apps/citizen-pwa/src/services/wizard-snapshot.ts
apps/citizen-pwa/src/services/wizard-snapshot.test.ts
docs/progress.md
docs/learnings.md
Tighten Step 1 and Step 2 wizard UX/a11y by requiring explicit incident-type selection and always showing a next-step affordance.
  • Change Step1Evidence to default reportType to an empty value, add reportTypeError state, and validate on Next with an inline role="alert" error message; clear the error when the user selects a type.
  • Update Step2WhoWhere bottom action region to always render, showing either a role="status" hint when no location method is chosen or the Review Report button when the precondition is met.
apps/citizen-pwa/src/components/SubmitReportForm/Step1Evidence.tsx
apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx
docs/progress.md
docs/learnings.md
Improve offline shell behavior by precaching the app shell and providing navigation fallbacks in the service worker.
  • Bump shell cache name from bantayog_shell_v1 to bantayog_shell_v2 and introduce a PRECACHE_URLS list for /, /index.html, manifest, and icons.
  • Update install handler to pre-cache the app shell and only call skipWaiting after precache completes.
  • Enhance fetch handler to fall back to cached /index.html for failed navigation requests and to return cached responses for GET requests when available.
apps/citizen-pwa/public/sw.js
docs/progress.md
docs/learnings.md
Preserve phone numbers across Login and Register flows and improve RegisterPage phone field accessibility.
  • Seed LoginPage and RegisterPage phone state from sessionStorage["bantayog.last-phone"] via lazy useState initializers with try/catch guards, defaulting to +63.
  • Write phone changes back to sessionStorage on each keystroke in both pages, while keeping RegisterPage’s resume-from-currentUser behavior intact.
  • Wrap the RegisterPage phone input with a proper label, and add id, name, and autoComplete="tel" attributes for better a11y semantics.
apps/citizen-pwa/src/pages/LoginPage.tsx
apps/citizen-pwa/src/pages/RegisterPage.tsx
docs/progress.md
docs/learnings.md
Improve Settings page accessibility via contrast fixes and a proper main landmark.
  • Replace hard-coded low-contrast hex text colors with design-system Tailwind tokens text-surface-600 and text-surface-500 for section labels and helper text.
  • Change the Settings root from a div to to align with skip-link targeting and provide a proper landmark.
apps/citizen-pwa/src/pages/SettingsPage.tsx
docs/progress.md
docs/learnings.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 2, 2026

Warning

Rate limit exceeded

@Exc1D has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 14 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: dda829ce-2ce8-4c27-a9c6-969d6416f80a

📥 Commits

Reviewing files that changed from the base of the PR and between 67664f2 and 5666500.

📒 Files selected for processing (6)
  • apps/citizen-pwa/public/sw.js
  • apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx
  • apps/citizen-pwa/src/components/SubmitReportForm/index.tsx
  • apps/citizen-pwa/src/pages/RegisterPage.tsx
  • apps/citizen-pwa/src/services/phone-session-storage.ts
  • docs/progress.md
📝 Walkthrough

Walkthrough

This PR adds a localforage-backed wizard snapshot service with TTL and tests, wires resume/save/clear logic into the SubmitReportForm, validates Step 1 incident type, persists last-entered phone via sessionStorage, updates the service worker to precache the app shell and serve cached /index.html for navigation fallbacks, and applies minor UI/accessibility/style tweaks and docs.

Changes

Citizen PWA: Wizard Resume, Phone Persistence, SW, and UI tweaks

Layer / File(s) Summary
Data Shape & Storage API
apps/citizen-pwa/src/services/wizard-snapshot.ts
Adds WizardSnapshot interface, TTL and storage key constants, _resetWizardSnapshotCache(), and wizardSnapshot with load(), save(), clear() using a cached localforage instance.
Unit Tests for Storage
apps/citizen-pwa/src/services/wizard-snapshot.test.ts
New Vitest suite exercising load/save/clear, TTL staleness, and resilient error paths with a mocked in-memory localforage.
Component Wiring — Load & Gate Saves
apps/citizen-pwa/src/components/SubmitReportForm/index.tsx
Loads snapshot on mount into step/formData (omitting photoFile), introduces hasLoadedSnapshot to gate subsequent saves, shows loading placeholder until loaded, clears snapshot on abandon and after successful submit, and passes initialReportType to Step1.
Step 1: Initialization & Validation
apps/citizen-pwa/src/components/SubmitReportForm/Step1Evidence.tsx
Adds optional initialReportType prop, initializes reportType from it (no longer hardcoded), introduces reportTypeError and prevents advancing when empty; clears error on selection.
Step 2: Stable Action Region
apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx
Always renders the sticky bottom action area; when locationMethod === null shows a role="status" hint, otherwise renders existing submit button with unchanged disabled/label logic.
Phone Persistence Helpers
apps/citizen-pwa/src/services/phone-session-storage.ts
Adds getStoredPhone() and setStoredPhone(phone) using sessionStorage with best-effort/error-swallowing behavior and +63 default.
Auth Pages: Seed & Persist Phone
apps/citizen-pwa/src/pages/LoginPage.tsx, apps/citizen-pwa/src/pages/RegisterPage.tsx
Login/Register now initialize phone from getStoredPhone() (Register uses lazy initializer unless resuming) and persist changes via setStoredPhone(...); Register phone input gains an associated <label id="register-phone">.
Service Worker: Precache & Navigation Fallback
apps/citizen-pwa/public/sw.js
Bumps cache name to bantayog_shell_v2, adds PRECACHE_URLS and precaches app-shell URLs during install before skipWaiting(). Fetch handler's network-failure path now returns cached /index.html for navigation requests and falls back to cached matches for other GET requests.
Accessibility & Styling
apps/citizen-pwa/src/pages/SettingsPage.tsx
Wraps content in <main id="main-content"> and replaces ad-hoc hex color classes with design tokens (text-surface-600, text-surface-500).
Docs & Progress
docs/learnings.md, docs/progress.md
Adds learnings about SW precaching, wizard snapshot practices, phone persistence, ARIA/usability rules, and three new progress entries documenting QA and follow-ups.

Sequence Diagrams

sequenceDiagram
    actor User
    participant Browser
    participant WizardComponent as SubmitReportForm
    participant SnapshotService as wizardSnapshot
    participant LocalForage

    User->>Browser: Open wizard
    Browser->>WizardComponent: mount
    WizardComponent->>SnapshotService: load()
    SnapshotService->>LocalForage: getItem(snapshotKey)
    LocalForage-->>SnapshotService: snapshot or null
    SnapshotService-->>WizardComponent: Snapshot | null
    WizardComponent->>WizardComponent: rehydrate step & formData
    WizardComponent->>SnapshotService: save({step, step1, step2})
    SnapshotService->>LocalForage: setItem(snapshotKey, payload+updatedAt)
    LocalForage-->>SnapshotService: success
    User->>WizardComponent: abandon or submit
    WizardComponent->>SnapshotService: clear()
    SnapshotService->>LocalForage: removeItem(snapshotKey)
Loading
sequenceDiagram
    actor User
    participant Browser
    participant ServiceWorker as SW
    participant Cache
    participant Network

    User->>Browser: Request SPA route (navigate)
    Browser->>SW: fetch(request)
    SW->>Network: fetch(request)
    Network-->>SW: network error
    SW->>Cache: match("/index.html")
    Cache-->>SW: cached /index.html
    SW-->>Browser: respond with /index.html
    Browser->>Browser: bootstrap SPA and render route
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~40 minutes

Possibly related PRs

Poem

🐰 I hopped to save your wizard's state,

Cookies, caches, stored the date.
Phones remembered, forms resume,
Offline shells hum in the gloom.
A bouncing patch, now PWA blooms.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title comprehensively covers all major changes: offline shell (service worker), a11y (accessibility), wizard resumability, and auth phone preservation, accurately reflecting the multi-faceted nature of this QA follow-up PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/citizen-pwa-auth-and-wizard-followups

Tip

💬 Introducing Slack Agent: Turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value).


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 11 minutes and 14 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • In WizardContainer, the hasLoadedSnapshot gate currently returns an empty div with aria-hidden="true"; consider rendering a minimal loading/skeleton state with an accessible status instead so users aren’t presented with a blank screen while the snapshot loads.
  • The sessionStorage["bantayog.last-phone"] key and access pattern are duplicated in LoginPage and RegisterPage; extracting this into a small shared helper (with the try/catch and default +63) would reduce drift and centralize the persistence logic.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `WizardContainer`, the `hasLoadedSnapshot` gate currently returns an empty `div` with `aria-hidden="true"`; consider rendering a minimal loading/skeleton state with an accessible status instead so users aren’t presented with a blank screen while the snapshot loads.
- The `sessionStorage["bantayog.last-phone"]` key and access pattern are duplicated in `LoginPage` and `RegisterPage`; extracting this into a small shared helper (with the try/catch and default `+63`) would reduce drift and centralize the persistence logic.

## Individual Comments

### Comment 1
<location path="apps/citizen-pwa/public/sw.js" line_range="22-29" />
<code_context>
+  '/icons/icon-512.png',
+]
+
 self.addEventListener('install', (event) => {
-  self.skipWaiting()
+  event.waitUntil(
+    caches
+      .open(CACHE_NAME)
+      .then((cache) => cache.addAll(PRECACHE_URLS))
+      .then(() => self.skipWaiting()),
+  )
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider hardening install-time precaching so a single failure doesn’t block the entire service worker installation.

`cache.addAll(PRECACHE_URLS)` will reject the entire `install` if any resource fails (e.g. a transient failure fetching `manifest.webmanifest`), which prevents the new SW from ever activating. To avoid users being stuck on an old SW due to minor precache hiccups, consider handling failures (e.g. wrapping `addAll` in a `catch` and still calling `self.skipWaiting()`, or using `Promise.allSettled` with individual `cache.add` calls).

```suggestion
self.addEventListener('install', (event) => {
  event.waitUntil(
    caches
      .open(CACHE_NAME)
      .then((cache) =>
        Promise.all(
          PRECACHE_URLS.map((url) =>
            cache.add(url).catch((error) => {
              // Log and continue on individual precache failures so that
              // a single bad asset does not block the entire SW install.
              console.warn('[sw] Failed to precache', url, error)
            }),
          ),
        ),
      )
      .catch((error) => {
        // Even if opening the cache or the bulk precache logic fails,
        // we still want the new service worker to activate.
        console.error('[sw] Precache during install encountered an error', error)
      })
      .then(() => self.skipWaiting()),
  )
})
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread apps/citizen-pwa/public/sw.js
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/citizen-pwa/src/components/SubmitReportForm/index.tsx`:
- Around line 56-72: The effect currently calls wizardSnapshot.load().then(...)
but lacks a .catch(), so if load() rejects or the then-handler throws,
setHasLoadedSnapshot(true) never runs and the wizard stays hidden; update the
promise chain on wizardSnapshot.load() to append a .catch(error => { if
(!cancelled) { process the error if needed (e.g., log) } }) and ensure
setHasLoadedSnapshot(true) is executed in a finally-like step (either by using
.finally(() => { if (!cancelled) setHasLoadedSnapshot(true) }) or by repeating
the flag set in both then and catch); keep existing uses of setStep and
setFormData inside the successful path (inside the then handler) and preserve
the cancelled guard.

In `@apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx`:
- Around line 315-318: The paragraph element in Step2WhoWhere (the <p> with
role="status") redundantly includes aria-live="polite"; remove the explicit
aria-live attribute from that element so it relies on the implicit
aria-live/aria-atomic behavior provided by role="status" to reduce noise and
keep accessibility semantics correct.

In `@apps/citizen-pwa/src/services/wizard-snapshot.test.ts`:
- Around line 55-65: The TTL test currently can pass falsely if the test
STORAGE_KEY drifts from production; update the test to exercise the TTL branch
by using the public API and fake timers: in the test for wizardSnapshot.load()
replace direct store writes to mockInstance._store.set(STORAGE_KEY, ...) with
wizardSnapshot.save(staleSnapshot) (use a WizardSnapshot object), call
vi.useFakeTimers({ now: Date.now() }) in beforeEach, advance time or call
vi.setSystemTime(Date.now() + 25 * 60 * 60 * 1000) to simulate >24h, then assert
wizardSnapshot.load() returns null and that the underlying mock store no longer
has the production key; restore timers with vi.useRealTimers() in afterEach.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 35ab3216-8ce5-40f3-81a7-e4d6fec8188e

📥 Commits

Reviewing files that changed from the base of the PR and between c6db91f and 2200b33.

📒 Files selected for processing (11)
  • apps/citizen-pwa/public/sw.js
  • apps/citizen-pwa/src/components/SubmitReportForm/Step1Evidence.tsx
  • apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx
  • apps/citizen-pwa/src/components/SubmitReportForm/index.tsx
  • apps/citizen-pwa/src/pages/LoginPage.tsx
  • apps/citizen-pwa/src/pages/RegisterPage.tsx
  • apps/citizen-pwa/src/pages/SettingsPage.tsx
  • apps/citizen-pwa/src/services/wizard-snapshot.test.ts
  • apps/citizen-pwa/src/services/wizard-snapshot.ts
  • docs/learnings.md
  • docs/progress.md

Comment thread apps/citizen-pwa/src/components/SubmitReportForm/index.tsx
Comment thread apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx Outdated
Comment thread apps/citizen-pwa/src/services/wizard-snapshot.test.ts
…lience, a11y loading state, phone helper extraction, snapshot catch guard, TTL test fix
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/citizen-pwa/src/components/SubmitReportForm/index.tsx (1)

129-149: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clear the wizard snapshot as soon as createDraft() succeeds.

Right now the snapshot survives until server confirmation. If the user refreshes from queued or failed_retryable, the in-memory draft is gone but the step-3 snapshot is still there, so reopening /report can create a second draft with a new clientDraftRef instead of resuming the existing submission.

♻️ Suggested lifecycle fix
       const { draft: created, secret: draftSecret } = await createDraft({
         reportType: formData.step1.reportType as ReportType,
         // barangayId holds the barangay name when selected; fall back to municipality label
         barangay: formData.step2.barangayId ?? formData.step2.municipalityLabel ?? '',
@@
         ...(photo ? { photo } : {}),
       })
 
+      await wizardSnapshot.clear()
       setDraft(created)
       setSecret(draftSecret)

Also applies to: 181-183

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/citizen-pwa/src/components/SubmitReportForm/index.tsx` around lines 129
- 149, After createDraft() succeeds and you call setDraft(created) and
setSecret(draftSecret), immediately clear the persisted wizard snapshot (i.e.,
invoke whatever function or storage-removal you use to persist the step-3 wizard
snapshot) so the UI can't later resume into a stale snapshot and accidentally
create a second draft; do this right after setDraft/setSecret in the createDraft
success block in SubmitReportForm (the same change must also be applied to the
other creation branch around the second occurrence at the other block near the
referenced lines). Ensure you clear the snapshot before any further
navigation/confirmation logic and keep the clearing call tied to the createDraft
success path so clientDraftRef remains authoritative.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/citizen-pwa/public/sw.js`:
- Around line 22-36: The install handler currently treats all PRECACHE_URLS as
optional and proceeds even if core shell files fail; update it to split assets
into REQUIRED_URLS (e.g., "/", "/index.html", core shell files) and
OPTIONAL_URLS (manifest, icons), open CACHE_NAME and call
cache.addAll(REQUIRED_URLS) so any failure rejects the install (thus keeping the
old worker), then individually cache.add(optional) with per-item catch for
best-effort; adjust the install flow inside the self.addEventListener('install',
...) block to reject the waitUntil promise on required failures and only swallow
errors for optional assets.

In `@apps/citizen-pwa/src/components/SubmitReportForm/index.tsx`:
- Around line 58-67: The restore logic only sets formData.step2 to snap.step2
but Step2WhoWhere still uses its own local state and never receives those
values; update the restore to fully hydrate all Step 2 fields (e.g., location,
contact, etc.) into formData by mapping snap.step2 properties into the same
shape Step2WhoWhere expects and ensure Step2WhoWhere receives those values as
initialValues (or refactor Step2WhoWhere to read from formData.step2 instead of
local state); specifically modify the wizardSnapshot.load() continuation that
calls setFormData and the Step2WhoWhere mount to pass/consume formData.step2 so
Step 2 is repopulated on reload or navigation.

In `@apps/citizen-pwa/src/pages/RegisterPage.tsx`:
- Around line 188-190: The phone input is being saved raw via setPhone and
setStoredPhone; update the input handler in RegisterPage (the onChange that sets
next, setPhone and setStoredPhone) to validate and normalize the value first:
trim whitespace, strip non-digit characters (or apply a limited allowed
pattern), enforce a max length (e.g. 15) and reject or truncate oversized
values, and only call setStoredPhone when the normalized value passes
validation; wrap the storage call inside a try/catch that logs errors instead of
swallowing them to handle sessionStorage failures. Ensure you modify the handler
where setPhone and setStoredPhone are called so the normalized value is used
consistently.

In `@apps/citizen-pwa/src/services/phone-session-storage.ts`:
- Around line 4-17: The current empty catch blocks in getStoredPhone and
setStoredPhone swallow all errors; change them to catch (e: unknown) and
distinguish known benign storage failures (e.g., DOMException names like
"QuotaExceededError", "NS_ERROR_DOM_QUOTA_REACHED", "SecurityError" or messages
containing "Quota" / "private" / "denied") and ignore those, but for any other
unexpected error call console.warn (or your module logger) with a clear message
including KEY and the error, then return DEFAULT_PHONE from getStoredPhone (and
silently return from setStoredPhone) so callers still get best-effort behavior
while unexpected failures are visible in logs.

In `@docs/progress.md`:
- Around line 60-65: Update the Round 1 note in the progress entry for Step 2 so
it matches the current markup in the Step2WhoWhere component: remove the
statement claiming the Step 2 hint uses role="status" and aria-live="polite"
(since that attribute was already removed and recorded on 2026-05-03) and
replace it with a brief note that the hint is no longer rendered with those ARIA
attributes, or delete the redundant sentence so the entry is not
self-contradictory.

---

Outside diff comments:
In `@apps/citizen-pwa/src/components/SubmitReportForm/index.tsx`:
- Around line 129-149: After createDraft() succeeds and you call
setDraft(created) and setSecret(draftSecret), immediately clear the persisted
wizard snapshot (i.e., invoke whatever function or storage-removal you use to
persist the step-3 wizard snapshot) so the UI can't later resume into a stale
snapshot and accidentally create a second draft; do this right after
setDraft/setSecret in the createDraft success block in SubmitReportForm (the
same change must also be applied to the other creation branch around the second
occurrence at the other block near the referenced lines). Ensure you clear the
snapshot before any further navigation/confirmation logic and keep the clearing
call tied to the createDraft success path so clientDraftRef remains
authoritative.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8893a8aa-2ed2-4cac-b0f0-256e7c54d910

📥 Commits

Reviewing files that changed from the base of the PR and between 2200b33 and 67664f2.

📒 Files selected for processing (9)
  • apps/citizen-pwa/public/sw.js
  • apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx
  • apps/citizen-pwa/src/components/SubmitReportForm/index.tsx
  • apps/citizen-pwa/src/pages/LoginPage.tsx
  • apps/citizen-pwa/src/pages/RegisterPage.tsx
  • apps/citizen-pwa/src/services/phone-session-storage.ts
  • apps/citizen-pwa/src/services/wizard-snapshot.test.ts
  • docs/learnings.md
  • docs/progress.md

Comment thread apps/citizen-pwa/public/sw.js
Comment thread apps/citizen-pwa/src/components/SubmitReportForm/index.tsx
Comment thread apps/citizen-pwa/src/pages/RegisterPage.tsx Outdated
Comment thread apps/citizen-pwa/src/services/phone-session-storage.ts
Comment thread docs/progress.md Outdated
@Exc1D Exc1D changed the title fix(citizen-pwa): QA round 1 + 2 — offline shell, a11y, wizard resumability, auth phone preservation fix(citizen-pwa): QA round 1 + 2 offline shell, a11y, wizard resumability, auth phone preservation May 2, 2026
- SW: split precache into REQUIRED_URLS (/, /index.html) and OPTIONAL_URLS.
  Required failures reject install; optional failures are best-effort.
- Wizard: clear snapshot immediately after createDraft() succeeds to prevent
  stale resumes creating duplicate drafts.
- Step2WhoWhere: accept initialValues prop and hydrate from snapshot/formData.
- RegisterPage: normalize phone input (strip non-digits, trim, 16-char max)
  before persisting to sessionStorage.
- phone-session-storage: distinguish benign storage errors from unexpected
  ones; log unexpected errors instead of swallowing silently.
- docs/progress.md: remove outdated aria-live mention from Step 2 entry.

Verification: lint 0 errors, typecheck clean, 243/243 tests pass.
@Exc1D Exc1D merged commit 520710e into main May 2, 2026
14 checks passed
@Exc1D Exc1D deleted the fix/citizen-pwa-auth-and-wizard-followups branch May 2, 2026 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant