Polish Clean Air Forum selfie filter: branding copy, loader UX, camer…#3781
Conversation
…a reliability
- Correct the filter card wordmark ("Africa Clean Air Forum") and event
date range (13TH-16TH JULY)
- Stop echoing the submitter's display name/email in the "sent to wall"
toast — the wall submission still carries it, just not shown in-app
- Add a bordered status-banner palette (AppAlertColors) and inline
loading state to the share sheet's message banner
- Harden the camera screen against overlapping sessions, hung native
camera calls, and lock-screen interruptions mid-capture
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughUpdates Clean Air Forum branding text, adds alert colors and loading-aware inline messaging, hardens camera open/capture lifecycle handling, and changes selfie submission so it starts earlier and reports status through the shared banner. ChangesClean Air Forum branding, camera reliability, and wall submission with inline messaging
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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. Comment |
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)
src/mobile/lib/src/app/dashboard/widgets/clean_air_forum_filter_tab.dart (1)
256-302: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winQueue the latest wall submission instead of returning early
_isSendingToWallonly blocks concurrent runs, so a second selfie picked while the first upload is in flight can capture the newer card but still exit atif (_isSendingToWall) return;. That makes the success message refer to the older photo, and consent off/on will submit the same selfie again. Store the latest pending bytes and rerun once the current submission settles.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mobile/lib/src/app/dashboard/widgets/clean_air_forum_filter_tab.dart` around lines 256 - 302, _queue the latest wall submission instead of dropping it when `_isSendingToWall` is true. In `_submitToConferenceWall`, keep the most recent `Uint8List` in a pending field (set from `_submitCurrentSelfieToWall`/retry path) and, in the `finally` block, rerun the submission once the in-flight upload finishes if a newer payload was queued. This should be wired through `_submitCurrentSelfieToWall`, `_submitToConferenceWall`, and the `_isSendingToWall` guard so the latest selfie is submitted and the message/analytics match the newest card.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mobile/lib/src/app/dashboard/widgets/clean_air_forum_camera_screen.dart`:
- Around line 165-176: Guard the AppLifecycleState.resumed reopen path in
CleanAirForumCameraScreen so it does not call _openCamera(_cameraIndex) when no
cameras were discovered. In the resume branch, check that _cameras is not empty
before reopening, since _openCamera indexes into _cameras directly and can crash
after permission denial or on devices without a camera. Keep the reopen call
unawaited for consistency with the existing camera open flows, and make the
check alongside the current _controller null/initialized guard in the lifecycle
handler.
- Around line 93-122: The _openCamera flow in clean_air_forum_camera_screen.dart
needs an in-flight guard because clearing _controller before initialization
completes allows resumed or retry-triggered calls to start another open while
the previous CameraController is still disposing or initializing. Add a
dedicated “camera opening”/“initializing” state check around _openCamera, set it
before teardown/init starts, and clear it on success, timeout, and dispose paths
so only one CameraController session can be active or transitioning at a time.
---
Outside diff comments:
In `@src/mobile/lib/src/app/dashboard/widgets/clean_air_forum_filter_tab.dart`:
- Around line 256-302: _queue the latest wall submission instead of dropping it
when `_isSendingToWall` is true. In `_submitToConferenceWall`, keep the most
recent `Uint8List` in a pending field (set from
`_submitCurrentSelfieToWall`/retry path) and, in the `finally` block, rerun the
submission once the in-flight upload finishes if a newer payload was queued.
This should be wired through `_submitCurrentSelfieToWall`,
`_submitToConferenceWall`, and the `_isSendingToWall` guard so the latest selfie
is submitted and the message/analytics match the newest card.
🪄 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: CHILL
Plan: Pro
Run ID: 2d03585c-1c90-4011-8512-07f9aec6eb4d
⛔ Files ignored due to path filters (1)
src/mobile/assets/icons/alert-circle.svgis excluded by!**/*.svg
📒 Files selected for processing (6)
src/mobile/lib/src/app/dashboard/utils/clean_air_forum_branding.dartsrc/mobile/lib/src/app/dashboard/widgets/air_quality_share_sheet.dartsrc/mobile/lib/src/app/dashboard/widgets/clean_air_forum_camera_screen.dartsrc/mobile/lib/src/app/dashboard/widgets/clean_air_forum_filter_tab.dartsrc/mobile/lib/src/app/dashboard/widgets/share_sheet_widgets.dartsrc/mobile/lib/src/meta/utils/colors.dart
There was a problem hiding this comment.
Pull request overview
Improves the Clean Air Forum selfie filter/share-sheet UX and reliability in the mobile app by aligning branding copy, adding clearer inline status feedback, and hardening camera/wall-submission flows.
Changes:
- Introduces a new alert/banner color palette (AppAlertColors) and updates the share sheet inline banner to support loading + bordered success/error states.
- Adjusts Clean Air Forum filter behavior to submit to the wall immediately upon consent/photo changes, and avoids displaying submitter identity in success copy.
- Improves camera-session reliability by ensuring proper teardown between sessions and bounding camera operations with timeouts.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mobile/lib/src/meta/utils/colors.dart | Adds AppAlertColors for theme-aware bordered success/error banner styling. |
| src/mobile/lib/src/app/dashboard/widgets/share_sheet_widgets.dart | Extends share-sheet messenger API with loading and updates inline banner visuals (spinner/icons, borders). |
| src/mobile/lib/src/app/dashboard/widgets/clean_air_forum_filter_tab.dart | Auto-submits to wall on consent/selfie changes; revises messaging to avoid echoing user identity. |
| src/mobile/lib/src/app/dashboard/widgets/clean_air_forum_camera_screen.dart | Prevents overlapping camera sessions; adds timeouts and recovery paths for hung capture/init flows. |
| src/mobile/lib/src/app/dashboard/widgets/air_quality_share_sheet.dart | Adds inline banner loading state support and updates accessibility announcement call. |
| src/mobile/lib/src/app/dashboard/utils/clean_air_forum_branding.dart | Updates CAF wordmark copy and date range string. |
| src/mobile/assets/icons/alert-circle.svg | Adds alert icon asset used by the updated inline banner. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Future<void> _submitCurrentSelfieToWall() async { | ||
| if (widget.selfieFile == null) return; | ||
| final imageBytes = await captureShareBoundary(context, _filterKey); |
| // interruption case that surfaced it. | ||
| final controller = _controller; | ||
| if (controller == null || !controller.value.isInitialized) { | ||
| _openCamera(_cameraIndex); |
- Guard _openCamera against re-entrancy: retry-on-failure and lifecycle-resume could both call it close together, opening two CameraController sessions at once and tripping the Camera2 HAL - Don't reopen the camera on resume when no cameras were discovered (permission denied / cameraless device) — indexed into an empty list - Check _isSendingToWall before rasterizing the filter card, not just before the network call, so a fast consent+photo-change race doesn't do a wasted capture pass - Wrap the inactive-branch controller.dispose() in unawaited(), matching the rest of the file
…a reliability
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
How should this be manually tested?
What are the relevant tickets?
Screenshots (optional)
Summary by CodeRabbit