fix(citizen-pwa): P0-P3 bug fixes and polish sweep#92
Conversation
…t button\n\n- LookupScreen: ensureSignedIn() before requestLookup (fixes CORS/auth error)\n- Step3Review: remove danger-500 override on Submit (navy per DESIGN.md)\n- SettingsPage: add Sign out to Account section\n- SettingsPage: Add to Home Screen when deferredInstallPrompt available\n- SettingsPage: disable push toggle + hint when browser permission is denied\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ve nav('/reports/{id}') from onSuccess callback â�� it was racing the\nRevealSheet render away before users could see their reference/secret\ncodes. RevealSheet now handles dismissal via onClose -> nav('/').\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n\n- main.tsx: retry SW registration up to 3 times (1s/2s backoff); dispatch\n sw-registration-failed event on exhaustion for future banner wiring\n- index.html: add apple-mobile-web-app-capable, status-bar-style, title,\n apple-touch-icon â�� required for iOS home-screen install\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…markers\n\n- MapTab/index.tsx: import FilterBar, make filters stateful, render FilterBar\n absolutely above map with pointer-events-auto wrapper\n- IncidentLayer.tsx + MyReportLayer.tsx: add cursor:pointer to marker div\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ly\n\nWhen auto-location triggers GPS on mount but the device denies/times out,\nlocationMethod stays 'gps' with no location and no method-picker visible.\nNow renders a 'Switch to manual location' button when GPS failed.\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-state polish\n\nDeleteAccountFlow: proper fixed-overlay backdrop, Escape-key dismiss,\nclick-outside close, autoFocus on first interactive element per dialog\nstate, aria-modal + aria-labelledby, semantic danger-600 color classes\ninstead of inline style. (P1-5, P3-16)\n\nFeedTab: swap green CheckCircle 'All clear' empty-state for neutral\nInfo icon + 'No incidents' heading; fix badge text-[10px] â�� text-xs;\nauthority-navy (001e40) for selected filter chips; caption text-[11px]\nâ�� text-xs. (P3-6, P3-7, P3-12, P3-13)\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…\nAlertsTab: border-l-4 â�� border-l-2 (P3-9, stay within 1px design-law\nspirit; 2px is minimal visual cue, not a heavy stripe).\n\nLookupScreen: add 'Check another report' button below result card so\nusers can look up a second incident without refreshing. (P2-14)\n\nMapTab IncidentLayer + FilterBar: medium severity color #a73400 â�� #7c3500\nto meet WCAG AA 4.5:1 contrast on white backgrounds. (P2-12)\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…els\n\nStep1Evidence: px-3 py-3 on incident type buttons (P2-1)\nStep3Review: py-3 â�� py-4 on all summary rows, mb-3 below consent (P2-4, P2-5)\nFeedTab/AlertsTab/ProfileTab/Step*: animate-pulse â�� motion-safe:animate-pulse\n respects prefers-reduced-motion for all skeleton loaders (P3-3)\nToggle: remove redundant role=group wrapper with duplicate aria-label; the\n role=switch button already carries the label (P3-14)\nContactFields: Tagalog sub-labels '/ Pangalan' and '/ Numero ng telepono'\n added to name and phone fields per PRODUCT.md inclusive-by-default (P3-15)\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…net 4.6 <noreply@anthropic.com>
Reviewer's GuideThis PR implements a broad bug-fix and polish sweep for the Citizen PWA, focusing on critical reliability (submission flow, auth, SW registration, GPS fallback), UX/a11y of destructive flows and settings (delete account modal, push toggle, sign-out, A2HS), map and feed usability (filter wiring, marker affordances, empty states, colors), motion/accessibility preferences, and documentation of QA coverage. Sequence diagram for report submission success RevealSheet flowsequenceDiagram
actor User
participant SubmitReportForm
participant WizardContainer
participant SubmissionPanel
participant RevealSheetSuccess
participant WizardSnapshot
participant Nav as Router
participant ReportApi
User->>SubmitReportForm: Complete steps and click Submit
SubmitReportForm->>WizardContainer: onSubmit()
WizardContainer->>SubmissionPanel: Render with draft, secret, onSuccess
User->>SubmissionPanel: Confirm submission
SubmissionPanel->>ReportApi: submitReport(draft, secret)
ReportApi-->>SubmissionPanel: success
SubmissionPanel->>WizardSnapshot: clear()
SubmissionPanel-->>WizardContainer: onSuccess()
WizardContainer->>SubmissionPanel: Render RevealSheetSuccess
SubmissionPanel->>RevealSheetSuccess: referenceCode, municipalityId, secretCode, onClose
User->>RevealSheetSuccess: Tap close button
RevealSheetSuccess-->>SubmissionPanel: onClose()
SubmissionPanel->>Nav: navigate("/")
Sequence diagram for lookup request with ensureSignedInsequenceDiagram
actor User
participant LookupScreen
participant FirebaseConfig
participant EnsureSignedIn
participant Fns as FirebaseFunctions
participant CloudFn as requestLookup
User->>LookupScreen: Enter reference and submit
LookupScreen->>FirebaseConfig: hasFirebaseConfig()
alt config missing
FirebaseConfig-->>LookupScreen: false
LookupScreen-->>User: Show FIREBASE_ENV_ERROR_MESSAGE
else config present
FirebaseConfig-->>LookupScreen: true
LookupScreen->>EnsureSignedIn: ensureSignedIn()
EnsureSignedIn-->>LookupScreen: user authenticated
LookupScreen->>Fns: fns()
Fns-->>LookupScreen: functions instance
LookupScreen->>CloudFn: httpsCallable("requestLookup", payload)
CloudFn-->>LookupScreen: result
LookupScreen-->>User: Show lookup result
end
User->>LookupScreen: Click "Check another report"
LookupScreen->>LookupScreen: setResult(null)
LookupScreen-->>User: Show lookup form again
Updated class diagram for key Citizen PWA componentsclassDiagram
class DeleteAccountFlow {
+step: string
+typed: string
+error: string
+onGoodbye(): void
+goIdle(): void
+handleConfirm(): Promise~void~
}
class SettingsPage {
+enabled: boolean
+user: object
+requestPermission(): Promise~boolean~
+disable(): Promise~void~
}
class Toggle {
+checked: boolean
+label: string
+disabled: boolean
+onChange(next: boolean): void
}
class MapTab {
+filters: Filters
+isOffline: boolean
+selectedPin: SelectedPin
+sheetPhase: string
}
class FilterBar {
+filters: Filters
+disabled: boolean
+onChange(next: Filters): void
}
class IncidentLayer {
+filters: Filters
+onPinTap(incident: PublicIncident): void
}
class MyReportLayer {
+onPinTap(report: MyReport): void
}
class SubmitReportForm {
}
class WizardContainer {
+draft: object
+secret: string
}
class SubmissionPanel {
+draft: object
+secret: string
+onSuccess(): void
}
class RevealSheetSuccess {
+referenceCode: string
+municipalityId: string
+secretCode: string
+onClose(): void
}
class LookupScreen {
+result: LookupResult
+error: string
}
class ContactFields {
+name: string
+phoneNumber: string
}
DeleteAccountFlow --> SettingsPage : used in
SettingsPage --> Toggle : renders
MapTab --> FilterBar : renders
MapTab --> IncidentLayer : configures
MapTab --> MyReportLayer : configures
SubmitReportForm --> WizardContainer : contains
WizardContainer --> SubmissionPanel : renders
SubmissionPanel --> RevealSheetSuccess : shows on success
SubmitReportForm --> ContactFields : uses
LookupScreen --> SettingsPage : navigates from
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
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 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMultiple QA-driven UI, accessibility, and infra refinements across the Citizen PWA: iOS PWA meta tags, motion-safe animations, minor visual/tokens tweaks, marker cursor affordance, Delete Account dialog accessibility and Escape handling, GPS fallback, service-worker registration retries, bilingual labels, notification/install UI, and added QA documentation. ChangesQA refinements: single change DAG
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 38 minutes and 20 seconds.Comment |
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="apps/citizen-pwa/src/components/Toggle.tsx" line_range="34" />
<code_context>
].join(' ')
return (
- <div className="flex items-center gap-3" role="group" aria-label={label}>
+ <div className="flex items-center gap-3">
<button
type="button"
</code_context>
<issue_to_address>
**issue (bug_risk):** The Toggle's accessible label appears to have been removed, which may break screen-reader labeling.
Previously, `label` was passed to `aria-label` on the `role="group"` wrapper, giving the control an accessible name. After this change, `label` is unused and the `role="switch"` button has no label, so it will be announced as an unlabeled switch by screen readers. Please reintroduce an accessible name (for example `aria-label={label}` on the button, or another appropriate labeling strategy).
</issue_to_address>
### Comment 2
<location path="docs/progress.md" line_range="52" />
<code_context>
+
## 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.
</code_context>
<issue_to_address>
**issue (typo):** Possible typo in the name "Sourcery-ai".
If you’re referring to the code review tool, its official name is “Sourcery AI” (no hyphen, capitalized “AI”).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| ].join(' ') | ||
|
|
||
| return ( | ||
| <div className="flex items-center gap-3" role="group" aria-label={label}> |
There was a problem hiding this comment.
issue (bug_risk): The Toggle's accessible label appears to have been removed, which may break screen-reader labeling.
Previously, label was passed to aria-label on the role="group" wrapper, giving the control an accessible name. After this change, label is unused and the role="switch" button has no label, so it will be announced as an unlabeled switch by screen readers. Please reintroduce an accessible name (for example aria-label={label} on the button, or another appropriate labeling strategy).
There was a problem hiding this comment.
Actionable comments posted: 1
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/MapTab/index.tsx (1)
233-233:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHumanize the time-window copy.
filters.windowrenders raw tokens like7dand30d, which reads awkwardly in the empty-state message. Reuse the existing user-facing labels here so the copy says “last 7 days” / “last 30 days.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/citizen-pwa/src/components/MapTab/index.tsx` at line 233, The empty-state message is showing raw tokens (filters.window) like "7d" — update the JSX in the MapTab component so it uses the same user-facing label used by the time-window control instead of the raw token; replace "No reported incidents in this area in the last {filters.window}." with "No reported incidents in this area in the last {humanLabel}" where humanLabel is obtained by reusing the existing time-window label mapping/helper used by the filter control (the same source used to render the window selector) so tokens like "7d"/"30d" become "7 days"/"30 days".
🤖 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/DeleteAccountFlow.tsx`:
- Around line 35-45: The Escape/backdrop dismissal currently calls goIdle() even
while deletion is in flight; change the dismissal guards to ignore events when
step === 'submitting' so the dialog cannot close once handleConfirm() has moved
to 'submitting'. Specifically, update the useEffect's onKey handler to only call
goIdle() when step is neither 'idle' nor 'submitting' (check step before calling
goIdle), and likewise update the backdrop/close button click handler(s) (the
same handler that currently invokes goIdle on backdrop clicks) to return early
if step === 'submitting', ensuring the UI stays open until the deletion
completes or explicitly transitions out of 'submitting'.
---
Outside diff comments:
In `@apps/citizen-pwa/src/components/MapTab/index.tsx`:
- Line 233: The empty-state message is showing raw tokens (filters.window) like
"7d" — update the JSX in the MapTab component so it uses the same user-facing
label used by the time-window control instead of the raw token; replace "No
reported incidents in this area in the last {filters.window}." with "No reported
incidents in this area in the last {humanLabel}" where humanLabel is obtained by
reusing the existing time-window label mapping/helper used by the filter control
(the same source used to render the window selector) so tokens like "7d"/"30d"
become "7 days"/"30 days".
🪄 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: cdc6566b-206f-4d4b-97fe-ff36427a3998
📒 Files selected for processing (19)
apps/citizen-pwa/index.htmlapps/citizen-pwa/src/components/AlertsTab.tsxapps/citizen-pwa/src/components/DeleteAccountFlow.tsxapps/citizen-pwa/src/components/FeedTab.tsxapps/citizen-pwa/src/components/LookupScreen.tsxapps/citizen-pwa/src/components/MapTab/FilterBar.tsxapps/citizen-pwa/src/components/MapTab/IncidentLayer.tsxapps/citizen-pwa/src/components/MapTab/MyReportLayer.tsxapps/citizen-pwa/src/components/MapTab/index.tsxapps/citizen-pwa/src/components/ProfileTab.tsxapps/citizen-pwa/src/components/SubmitReportForm/ContactFields.tsxapps/citizen-pwa/src/components/SubmitReportForm/Step1Evidence.tsxapps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsxapps/citizen-pwa/src/components/SubmitReportForm/Step3Review.tsxapps/citizen-pwa/src/components/SubmitReportForm/index.tsxapps/citizen-pwa/src/components/Toggle.tsxapps/citizen-pwa/src/main.tsxapps/citizen-pwa/src/pages/SettingsPage.tsxdocs/progress.md
- DeleteAccountFlow: prevent Escape/backdrop dismissal while submitting (CodeRabbit) - MapTab: humanize time-window labels in empty state (CodeRabbit) - progress.md: fix Sourcery AI capitalization (Sourcery) - Toggle.tsx: already correct — aria-label present on role=switch button
There was a problem hiding this comment.
Actionable comments posted: 2
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/MapTab/index.tsx (1)
233-241:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEmpty state message is misleading when a non-
"all"severity filter is active.
visibleIncidentsis already severity-filtered byusePublicIncidents, so whenseverity !== 'all'the message "No reported incidents in this area in the last {window}" may display even though medium/low incidents exist — the user has no indication their filter is responsible.🔧 Proposed fix
- No reported incidents in this area in the last {WINDOW_LABELS[filters.window]}. + No{filters.severity !== 'all' ? ` ${filters.severity}` : ''} incidents reported in this area in the last {WINDOW_LABELS[filters.window]}.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/citizen-pwa/src/components/MapTab/index.tsx` around lines 233 - 241, The empty-state message for the MapTab component incorrectly assumes no incidents exist when showEmpty is true even though visibleIncidents is already severity-filtered by usePublicIncidents; update the JSX that renders the <div> (the block controlled by showEmpty) to check filters.severity and render a different message when filters.severity !== 'all' that includes the active severity (filters.severity) or instructs the user to clear/change the severity filter, while keeping the existing WINDOW_LABELS[filters.window] text for the time window; ensure you reference showEmpty, visibleIncidents/usePublicIncidents, filters.severity, filters.window, and WINDOW_LABELS when making the conditional message change.
🤖 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/DeleteAccountFlow.tsx`:
- Around line 92-104: The destructive "Yes, delete my account →" button
currently has autoFocus; move the autoFocus attribute to the least-destructive
control (the Cancel button) so keyboard/AT users land on Cancel by default.
Update the JSX so the confirm button (which calls setStep('confirm'),
setTyped(''), setError(null)) no longer has autoFocus or the eslint-disable
comment, and instead add autoFocus on the Cancel button element; keep all
existing handlers and classNames unchanged except for the autofocus relocation.
- Around line 156-162: The Cancel button currently calls goIdle()
unconditionally which allows the modal to be dismissed while step ===
'submitting' and then be force-reopened when the async deletion fails; update
the Cancel button handler in DeleteAccountFlow (and/or the goIdle() call site)
to be guarded by the step state: either disable the button and make its onClick
a no-op when step === 'submitting', or return early from goIdle() if step ===
'submitting'; also ensure the button has an appropriate disabled/aria-busy state
so UI and a11y reflect that the operation is in progress.
---
Outside diff comments:
In `@apps/citizen-pwa/src/components/MapTab/index.tsx`:
- Around line 233-241: The empty-state message for the MapTab component
incorrectly assumes no incidents exist when showEmpty is true even though
visibleIncidents is already severity-filtered by usePublicIncidents; update the
JSX that renders the <div> (the block controlled by showEmpty) to check
filters.severity and render a different message when filters.severity !== 'all'
that includes the active severity (filters.severity) or instructs the user to
clear/change the severity filter, while keeping the existing
WINDOW_LABELS[filters.window] text for the time window; ensure you reference
showEmpty, visibleIncidents/usePublicIncidents, filters.severity,
filters.window, and WINDOW_LABELS when making the conditional message change.
🪄 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: 37f51422-1589-4a81-91b4-5008275cdb03
📒 Files selected for processing (4)
apps/citizen-pwa/src/components/DeleteAccountFlow.tsxapps/citizen-pwa/src/components/MapTab/index.tsxdocs/progress.mddocs/qa-findings-2026-05-03.md
- DeleteAccountFlow: move autoFocus to Cancel button; guard Cancel during submitting - MapTab: severity-aware empty state message when filter is active
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/DeleteAccountFlow.tsx`:
- Around line 130-141: The confirmation input (id "delete-confirm") stays
editable during the async submitting state and loses focus recovery after an
error; update the input in DeleteAccountFlow.tsx to set disabled={submitting}
(or equivalent) so users cannot type while deletion is in progress, and
replace/remove the unreliable autoFocus by adding a ref (e.g., confirmInputRef)
plus a useEffect that watches step === 'confirm' to call
confirmInputRef.current?.focus() when entering the confirm step (so focus is
restored after handleConfirm fails and step resets); keep the value handlers
(typed/setTyped) intact and ensure the disabled check uses the same submitting
state used by the submit button.
In `@apps/citizen-pwa/src/components/MapTab/index.tsx`:
- Around line 239-241: The filter-guidance hint is currently gated by the
overall empty condition (showEmpty) which also requires myReports.length === 0,
so when a user has personal reports the severity-filter hint is suppressed;
update the MapTab rendering to compute and use an independent condition (e.g., a
boolean like filterOnlyEmpty or showFilterHint) that checks the active filters
against the public incidents collection (use filters.severity, filters.window,
WINDOW_LABELS and the public incidents list or filteredPublicIncidents result)
and render the "Try clearing the severity filter." message whenever no public
incidents match the current filters, regardless of myReports length, while
keeping the existing full-empty message logic (showEmpty) for truly empty maps.
🪄 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: 3314450f-e825-4660-ab49-1fea295345ef
📒 Files selected for processing (2)
apps/citizen-pwa/src/components/DeleteAccountFlow.tsxapps/citizen-pwa/src/components/MapTab/index.tsx
- DeleteAccountFlow: replace autoFocus with ref+useEffect focus management; disable input during submitting - MapTab: show severity filter hint independently from showEmpty so it appears even when myReports exist
Summary
Comprehensive bug fix and polish sweep for the Citizen PWA covering P0 critical bugs through P3 design/a11y issues.
P0 — Critical Bugs
apple-mobile-web-app-capable, status bar style, touch icon)P1 — Major Issues
cursor:pointeron map markersP2 — UX Polish
px-3 py-3spacing on incident type buttonspy-4on summary rows,mb-3below consent#a73400→#7c3500(4.5:1 on white)P3 — Design & Accessibility
prefers-reduced-motion—animate-pulse→motion-safe:animate-pulsetext-xseverywhereborder-l-4→border-l-2role=groupwrapper from toggle/ Pangalanand/ Numero ng teleponoVerification
docs/qa-findings-2026-05-03.mdSummary by Sourcery
Apply a QA-driven bugfix and polish sweep to the Citizen PWA covering critical submission, auth, PWA, and UX/a11y issues.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Summary by CodeRabbit
New Features
Improvements
Documentation