[Mobile] Add "Share air quality" from location detail sheet#3657
Merged
Conversation
Contributor
|
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 ignored due to path filters (1)
📒 Files selected for processing (7)
💤 Files with no reviewable changes (6)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an air quality share feature to the mobile app's analytics detail view: a new ChangesAir Quality Share Feature
Auth Layer Test Reorganization
Sequence Diagram(s)sequenceDiagram
actor User
participant AnalyticsSpecifics
participant ShareBottomSheet
participant _ShareCardSheet
participant AirQualityShareCard
participant RepaintBoundary
participant AirQualityShareService
participant SharePlus
User->>AnalyticsSpecifics: tap Share button
AnalyticsSpecifics->>ShareBottomSheet: showModalBottomSheet (quick text / share card)
User->>ShareBottomSheet: select "Share card"
ShareBottomSheet->>_ShareCardSheet: open sheet with measurement
_ShareCardSheet->>AirQualityShareCard: render preview in RepaintBoundary
User->>_ShareCardSheet: tap Share
_ShareCardSheet->>RepaintBoundary: toImage → toByteData (PNG)
RepaintBoundary-->>_ShareCardSheet: Uint8List imageBytes
_ShareCardSheet->>AirQualityShareService: shareMeasurementCard(imageBytes, measurement)
AirQualityShareService->>AirQualityShareService: buildShareMessage(measurement)
AirQualityShareService->>SharePlus: Share.shareXFiles([XFile(png)], text)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
16 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
src/mobile/test/app/auth/lifecycle/app_resume_session_test.dart (1)
182-182: ⚡ Quick winReplace fixed sleep with deterministic queue draining in the test.
Line 182 uses a hardcoded
10msdelay, which is timing-sensitive in CI. Prefer draining the event queue (or using stream expectations) so the test is deterministic.Proposed patch
- await Future<void>.delayed(const Duration(milliseconds: 10)); + await pumpEventQueue();🤖 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/test/app/auth/lifecycle/app_resume_session_test.dart` at line 182, The test at line 182 uses a hardcoded 10 millisecond delay which is timing-sensitive and unreliable in CI environments. Replace the `Future<void>.delayed(const Duration(milliseconds: 10))` call with a deterministic approach such as draining the event queue (e.g., using `pumpAndSettle` if this is a Flutter widget test) or waiting on specific stream expectations that indicate the actual event you're waiting for has occurred. This ensures the test waits for the actual condition to complete rather than relying on arbitrary timing.src/mobile/lib/src/app/auth/lifecycle/app_resume_session_guard.dart (1)
17-25: ⚡ Quick winAvoid stale auth/lifecycle snapshots across
await.Line 18 (
currentState) and Line 19 (isMounted) are evaluated before the async refresh. If state flips during the await (e.g., logout/guest transition), Line 25 can still notify expiry from stale state. Prefer passing live getters (or re-reading current auth state post-refresh) before notifying.🤖 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/auth/lifecycle/app_resume_session_guard.dart` around lines 17 - 25, The handleAuthState method uses currentState and isMounted parameters that are evaluated before the async refreshTokenIfNeeded call, making them stale by the time line 25 executes notifySessionExpired. During the await, the auth state could change (e.g., logout or guest transition), so the notification would be based on outdated snapshots. After the await completes and before calling notifySessionExpired, re-read the current auth state and isMounted status to ensure they reflect the actual live state rather than the stale parameters captured at method entry.
🤖 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/auth/lifecycle/app_resume_session_guard.dart`:
- Around line 23-26: The refreshTokenIfNeeded() call in the resume guard can
throw exceptions from refresh, storage, or network operations that currently
escape unhandled. Wrap the token refresh logic (the refreshTokenIfNeeded() call
and the null check) in a try/catch block. In the catch block, check if isMounted
and then call _sessionExpiryNotifier.notifySessionExpired() to treat any refresh
failure as a controlled session-expiry signal rather than an unhandled lifecycle
error.
In `@src/mobile/lib/src/app/dashboard/widgets/air_quality_share_card.dart`:
- Around line 76-84: The Text widgets displaying locationName and category text
lack proper overflow handling, which can cause excessive wrapping and layout
instability in the share card. Add maxLines and overflow properties to both Text
widgets: the locationName Text widget needs maxLines to prevent excessive
wrapping despite being in an Expanded parent, and the category text Text widget
needs both maxLines and overflow since it has no width constraint and will
overflow in the Row if the string is long enough. This ensures proper text
truncation and maintains visual polish for the share card preview.
In `@src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dart`:
- Around line 150-167: The analytics event `air_quality_shared` is missing from
the successful share completion paths. Add tracking instrumentation at three
locations in the file: after the _shareQuickText method completes successfully
(around lines 176-181), after the showModalBottomSheet returns from a card share
(around lines 150-167), and at the third share completion location (around lines
455-460). In each case, after successful share completion, emit an
`air_quality_shared` analytics event that includes the required metadata
consisting of site ID, name, slug, and category extracted from the
widget.measurement object and related properties.
- Around line 480-483: The `_captureCard()` method creates a `ui.Image` object
from the boundary toImage call but does not dispose of it, causing native
resource accumulation when the share flow is triggered repeatedly. After
extracting the byte data from the image using toByteData, you must explicitly
dispose of the image object to free native resources. Use a try-finally pattern
to ensure the image.dispose() call happens regardless of whether toByteData
succeeds or fails, so the uint8list is returned but the image is properly
cleaned up.
- Around line 176-182: Add exception handling to the `_shareQuickText` method by
adding a catch block after the try statement that catches exceptions from the
`shareMeasurement` call and surfaces a user-facing error message to the user.
Apply the same fix to the `_shareCard` method which has the same pattern with
`shareMeasurementCard`, ensuring both methods handle share operation failures
with appropriate error dialogs. Additionally, modify the `_captureCard` method
to properly dispose of the ui.Image object it creates to prevent native memory
leaks during repeated card share operations.
In `@src/mobile/lib/src/app/shared/services/air_quality_share_service.dart`:
- Around line 64-73: The share text message constructed in the lines list (which
includes the air quality category, PM2.5 value, location description, and health
tip) is missing the required canonical URL. Add the HTTPS billboard link in the
format https://airqo.net/billboard/grid/{slug} to the lines list, where slug
should be generated to match the website's slug generation logic. Include this
URL as an additional line in the lines list before the final "Shared from the
AirQo app." line, ensuring the returned share message satisfies the share-link
requirement.
In `@src/mobile/pubspec_overrides.yaml`:
- Around line 1-6: The dependency_overrides section in pubspec_overrides.yaml
that redirects google_mlkit_translation to the stub package is currently global
and will apply to all builds (including CI and production), but the comment
indicates it is intended only for local simulator debugging. Either remove this
override from the committed pubspec_overrides.yaml file and document the manual
process for developers to apply it locally during development, or add
pubspec_overrides.yaml to .gitignore so local overrides do not leak into regular
builds. Ensure the real google_mlkit_translation plugin is used in CI and
production builds.
---
Nitpick comments:
In `@src/mobile/lib/src/app/auth/lifecycle/app_resume_session_guard.dart`:
- Around line 17-25: The handleAuthState method uses currentState and isMounted
parameters that are evaluated before the async refreshTokenIfNeeded call, making
them stale by the time line 25 executes notifySessionExpired. During the await,
the auth state could change (e.g., logout or guest transition), so the
notification would be based on outdated snapshots. After the await completes and
before calling notifySessionExpired, re-read the current auth state and
isMounted status to ensure they reflect the actual live state rather than the
stale parameters captured at method entry.
In `@src/mobile/test/app/auth/lifecycle/app_resume_session_test.dart`:
- Line 182: The test at line 182 uses a hardcoded 10 millisecond delay which is
timing-sensitive and unreliable in CI environments. Replace the
`Future<void>.delayed(const Duration(milliseconds: 10))` call with a
deterministic approach such as draining the event queue (e.g., using
`pumpAndSettle` if this is a Flutter widget test) or waiting on specific stream
expectations that indicate the actual event you're waiting for has occurred.
This ensures the test waits for the actual condition to complete rather than
relying on arbitrary timing.
🪄 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: 233246ca-47d4-451a-9ab7-f8e8f3edc58d
⛔ Files ignored due to path filters (1)
src/mobile/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
src/mobile/.gitignoresrc/mobile/ios/Runner.xcodeproj/project.pbxprojsrc/mobile/lib/main.dartsrc/mobile/lib/src/app/auth/bloc/auth_bloc.dartsrc/mobile/lib/src/app/auth/lifecycle/app_resume_session_guard.dartsrc/mobile/lib/src/app/dashboard/widgets/air_quality_share_card.dartsrc/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dartsrc/mobile/lib/src/app/shared/services/air_quality_share_service.dartsrc/mobile/local_packages/google_mlkit_translation_stub/README.mdsrc/mobile/local_packages/google_mlkit_translation_stub/lib/google_mlkit_translation.dartsrc/mobile/local_packages/google_mlkit_translation_stub/pubspec.yamlsrc/mobile/pubspec.yamlsrc/mobile/pubspec_overrides.yamlsrc/mobile/test/app/auth/bloc/auth_bloc_test.dartsrc/mobile/test/app/auth/integration/session_expiry_flow_test.dartsrc/mobile/test/app/auth/lifecycle/app_resume_session_test.dartsrc/mobile/test/app/auth/repository/base_repository_auth_test.dartsrc/mobile/test/app/auth/repository/secure_storage_repository_test.dartsrc/mobile/test/app/auth/services/auth_helper_test.dartsrc/mobile/test/app/auth/services/auth_token_storage_test.dart
💤 Files with no reviewable changes (1)
- src/mobile/ios/Runner.xcodeproj/project.pbxproj
Comment on lines
+23
to
+26
| final token = await _tokenRefresher.refreshTokenIfNeeded(); | ||
| if (token == null && isMounted) { | ||
| _sessionExpiryNotifier.notifySessionExpired(); | ||
| } |
Contributor
There was a problem hiding this comment.
Handle refresh exceptions in the resume guard.
Line 23 can throw (refresh/storage/network), and that currently escapes the guard path. Wrap this branch in try/catch and treat failures as a controlled session-expiry signal instead of an unhandled lifecycle error.
Proposed patch
Future<void> handleAuthState(
AuthState currentState, {
required bool isMounted,
}) async {
if (currentState is! AuthLoaded) return;
-
- final token = await _tokenRefresher.refreshTokenIfNeeded();
- if (token == null && isMounted) {
- _sessionExpiryNotifier.notifySessionExpired();
- }
+ try {
+ final token = await _tokenRefresher.refreshTokenIfNeeded();
+ if (token == null && isMounted) {
+ _sessionExpiryNotifier.notifySessionExpired();
+ }
+ } catch (_) {
+ if (isMounted) {
+ _sessionExpiryNotifier.notifySessionExpired();
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| final token = await _tokenRefresher.refreshTokenIfNeeded(); | |
| if (token == null && isMounted) { | |
| _sessionExpiryNotifier.notifySessionExpired(); | |
| } | |
| Future<void> handleAuthState( | |
| AuthState currentState, { | |
| required bool isMounted, | |
| }) async { | |
| if (currentState is! AuthLoaded) return; | |
| try { | |
| final token = await _tokenRefresher.refreshTokenIfNeeded(); | |
| if (token == null && isMounted) { | |
| _sessionExpiryNotifier.notifySessionExpired(); | |
| } | |
| } catch (_) { | |
| if (isMounted) { | |
| _sessionExpiryNotifier.notifySessionExpired(); | |
| } | |
| } | |
| } |
🤖 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/auth/lifecycle/app_resume_session_guard.dart` around
lines 23 - 26, The refreshTokenIfNeeded() call in the resume guard can throw
exceptions from refresh, storage, or network operations that currently escape
unhandled. Wrap the token refresh logic (the refreshTokenIfNeeded() call and the
null check) in a try/catch block. In the catch block, check if isMounted and
then call _sessionExpiryNotifier.notifySessionExpired() to treat any refresh
failure as a controlled session-expiry signal rather than an unhandled lifecycle
error.
Comment on lines
+150
to
+167
| if (selectedAction == _ShareAction.quickText) { | ||
| await _shareQuickText(shareOrigin); | ||
| return; | ||
| } | ||
|
|
||
| await showModalBottomSheet<void>( | ||
| context: context, | ||
| useRootNavigator: true, | ||
| isScrollControlled: true, | ||
| showDragHandle: true, | ||
| backgroundColor: Colors.transparent, | ||
| builder: (_) => _ShareCardSheet( | ||
| measurement: widget.measurement, | ||
| fallbackLocationName: widget.fallbackLocationName, | ||
| sharePositionOrigin: shareOrigin, | ||
| ), | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Successful shares are not emitting the required analytics event.
I don’t see an air_quality_shared tracking call in either the quick-text or card share success path. The linked objective requires this event with metadata (site ID, name, slug, category), so instrumentation should be added after successful completion.
Also applies to: 176-181, 455-460
🤖 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/analytics_specifics.dart` around
lines 150 - 167, The analytics event `air_quality_shared` is missing from the
successful share completion paths. Add tracking instrumentation at three
locations in the file: after the _shareQuickText method completes successfully
(around lines 176-181), after the showModalBottomSheet returns from a card share
(around lines 150-167), and at the third share completion location (around lines
455-460). In each case, after successful share completion, emit an
`air_quality_shared` analytics event that includes the required metadata
consisting of site ID, name, slug, and category extracted from the
widget.measurement object and related properties.
Comment on lines
+176
to
+182
| try { | ||
| await AirQualityShareService.shareMeasurement( | ||
| widget.measurement, | ||
| fallbackLocationName: widget.fallbackLocationName, | ||
| sharePositionOrigin: shareOrigin, | ||
| ); | ||
| } finally { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dart | sed -n '170,190p'Repository: airqo-platform/AirQo-frontend
Length of output: 622
🏁 Script executed:
cat -n src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dart | sed -n '450,470p'Repository: airqo-platform/AirQo-frontend
Length of output: 667
🏁 Script executed:
wc -l src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dartRepository: airqo-platform/AirQo-frontend
Length of output: 143
🏁 Script executed:
cat -n src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dart | sed -n '160,189p'Repository: airqo-platform/AirQo-frontend
Length of output: 987
🏁 Script executed:
cat -n src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dart | sed -n '430,468p'Repository: airqo-platform/AirQo-frontend
Length of output: 1303
🏁 Script executed:
cat -n src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dart | sed -n '470,520p'Repository: airqo-platform/AirQo-frontend
Length of output: 2240
🏁 Script executed:
rg -n "air_quality_shared|firebaseAnalytics|logEvent" src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dartRepository: airqo-platform/AirQo-frontend
Length of output: 55
🏁 Script executed:
rg -n "image\.dispose\(\)" src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dartRepository: airqo-platform/AirQo-frontend
Length of output: 55
🏁 Script executed:
cat -n src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dart | sed -n '1,50p'Repository: airqo-platform/AirQo-frontend
Length of output: 2029
🏁 Script executed:
rg -n "catch|Exception" src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dart | head -20Repository: airqo-platform/AirQo-frontend
Length of output: 55
🏁 Script executed:
cat -n src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dart | sed -n '169,189p'Repository: airqo-platform/AirQo-frontend
Length of output: 679
🏁 Script executed:
ast-grep --pattern 'Future<void> _shareQuickText($_) { $$$ }'Repository: airqo-platform/AirQo-frontend
Length of output: 55
Add exception handling to both share methods and dispose the captured image.
The _shareQuickText method at line 176 lacks a catch block, so shareMeasurement exceptions bubble up unhandled. The _shareCard method has the same issue with shareMeasurementCard at line 455—it has a try-finally for state cleanup, but no catch for the share operation itself. Both should surface a user-facing error message on failure.
Additionally, _captureCard creates a ui.Image at line 480 without disposing it, which increases native memory pressure during repeated card shares.
Suggested fix
Future<void> _shareQuickText(Rect? shareOrigin) async {
try {
await AirQualityShareService.shareMeasurement(
widget.measurement,
fallbackLocationName: widget.fallbackLocationName,
sharePositionOrigin: shareOrigin,
);
+ } catch (_) {
+ if (!mounted) return;
+ ScaffoldMessenger.of(context).showSnackBar(
+ const SnackBar(content: Text('Could not share right now. Please try again.')),
+ );
} finally {
if (mounted) {
setState(() {
_isSharing = false;
});
}
}
} await AirQualityShareService.shareMeasurementCard(
imageBytes,
widget.measurement,
fallbackLocationName: widget.fallbackLocationName,
sharePositionOrigin: widget.sharePositionOrigin,
);
+ } catch (_) {
+ if (!mounted) return;
+ ScaffoldMessenger.of(context).showSnackBar(
+ const SnackBar(content: Text('Could not share card right now. Please try again.')),
+ );
} finally { Future<Uint8List?> _captureCard() async {
final pixelRatio =
MediaQuery.of(context).devicePixelRatio.clamp(2.0, 3.0).toDouble();
await Future<void>.delayed(const Duration(milliseconds: 16));
final boundary =
_cardKey.currentContext?.findRenderObject() as RenderRepaintBoundary?;
if (boundary == null) return null;
final image = await boundary.toImage(pixelRatio: pixelRatio);
final byteData = await image.toByteData(format: ui.ImageByteFormat.png);
+ image.dispose();
return byteData?.buffer.asUint8List();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| await AirQualityShareService.shareMeasurement( | |
| widget.measurement, | |
| fallbackLocationName: widget.fallbackLocationName, | |
| sharePositionOrigin: shareOrigin, | |
| ); | |
| } finally { | |
| try { | |
| await AirQualityShareService.shareMeasurement( | |
| widget.measurement, | |
| fallbackLocationName: widget.fallbackLocationName, | |
| sharePositionOrigin: shareOrigin, | |
| ); | |
| } catch (_) { | |
| if (!mounted) return; | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| const SnackBar(content: Text('Could not share right now. Please try again.')), | |
| ); | |
| } finally { |
🤖 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/analytics_specifics.dart` around
lines 176 - 182, Add exception handling to the `_shareQuickText` method by
adding a catch block after the try statement that catches exceptions from the
`shareMeasurement` call and surfaces a user-facing error message to the user.
Apply the same fix to the `_shareCard` method which has the same pattern with
`shareMeasurementCard`, ensuring both methods handle share operation failures
with appropriate error dialogs. Additionally, modify the `_captureCard` method
to properly dispose of the ui.Image object it creates to prevent native memory
leaks during repeated card share operations.
Comment on lines
+480
to
+483
| final image = await boundary.toImage(pixelRatio: pixelRatio); | ||
| final byteData = await image.toByteData(format: ui.ImageByteFormat.png); | ||
|
|
||
| return byteData?.buffer.asUint8List(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the specific file
find . -name "analytics_specifics.dart" -type fRepository: airqo-platform/AirQo-frontend
Length of output: 141
🏁 Script executed:
# Check the exact content at lines 475-490 to see full context
if [ -f "src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dart" ]; then
sed -n '475,490p' "src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dart" | cat -n
fiRepository: airqo-platform/AirQo-frontend
Length of output: 747
🏁 Script executed:
# Search for other instances of toImage() in the codebase to check if this is a common pattern
rg "toImage\(" --type dart -B2 -A4Repository: airqo-platform/AirQo-frontend
Length of output: 766
🏁 Script executed:
# Check for any existing dispose() calls on ui.Image in the file
rg "image\.dispose\(\)" src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dartRepository: airqo-platform/AirQo-frontend
Length of output: 55
🏁 Script executed:
# Get a broader view of the _captureCard method
if [ -f "src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dart" ]; then
rg "_captureCard" --type dart -B5 -A15 src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dart
fiRepository: airqo-platform/AirQo-frontend
Length of output: 1214
Dispose ui.Image after PNG extraction to prevent native resource accumulation.
The _captureCard() method creates a ui.Image at line 480 but never disposes it. Since this runs in the share flow and can be triggered repeatedly, undisposed images accumulate native memory and can cause performance degradation.
Suggested fix
Future<Uint8List?> _captureCard() async {
@@
final image = await boundary.toImage(pixelRatio: pixelRatio);
- final byteData = await image.toByteData(format: ui.ImageByteFormat.png);
-
- return byteData?.buffer.asUint8List();
+ try {
+ final byteData = await image.toByteData(format: ui.ImageByteFormat.png);
+ return byteData?.buffer.asUint8List();
+ } finally {
+ image.dispose();
+ }
}🤖 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/analytics_specifics.dart` around
lines 480 - 483, The `_captureCard()` method creates a `ui.Image` object from
the boundary toImage call but does not dispose of it, causing native resource
accumulation when the share flow is triggered repeatedly. After extracting the
byte data from the image using toByteData, you must explicitly dispose of the
image object to free native resources. Use a try-finally pattern to ensure the
image.dispose() call happens regardless of whether toByteData succeeds or fails,
so the uint8list is returned but the image is properly cleaned up.
Comment on lines
+64
to
+73
| final lines = <String>[ | ||
| 'Air quality in $locationName is $category.', | ||
| if (pm25Value != null) 'PM2.5: ${pm25Value.toStringAsFixed(1)} µg/m³', | ||
| if (locationDescription.isNotEmpty) 'Location: $locationDescription', | ||
| if (healthTip != null && healthTip.isNotEmpty) healthTip, | ||
| '', | ||
| 'Shared from the AirQo app.', | ||
| ]; | ||
|
|
||
| return lines.join('\n'); |
Contributor
There was a problem hiding this comment.
Share payload is missing the required canonical URL + slug.
Per the PR objectives, the share text should include an HTTPS billboard link in the format https://airqo.net/billboard/grid/{slug} with website-matching slug generation. The current message has no URL, so this flow won’t satisfy the share-link requirement.
Suggested fix
static String buildShareMessage(
Measurement measurement, {
String? fallbackLocationName,
}) {
@@
final lines = <String>[
'Air quality in $locationName is $category.',
if (pm25Value != null) 'PM2.5: ${pm25Value.toStringAsFixed(1)} µg/m³',
if (locationDescription.isNotEmpty) 'Location: $locationDescription',
if (healthTip != null && healthTip.isNotEmpty) healthTip,
+ 'https://airqo.net/billboard/grid/${_buildSlug(locationName)}',
'',
'Shared from the AirQo app.',
];
return lines.join('\n');
}
+
+ static String _buildSlug(String input) {
+ final normalized = input.toLowerCase().trim();
+ final slug = normalized
+ .replaceAll(RegExp(r'[^a-z0-9\s-]'), '')
+ .replaceAll(RegExp(r'\s+'), '-')
+ .replaceAll(RegExp(r'-{2,}'), '-');
+ return slug;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| final lines = <String>[ | |
| 'Air quality in $locationName is $category.', | |
| if (pm25Value != null) 'PM2.5: ${pm25Value.toStringAsFixed(1)} µg/m³', | |
| if (locationDescription.isNotEmpty) 'Location: $locationDescription', | |
| if (healthTip != null && healthTip.isNotEmpty) healthTip, | |
| '', | |
| 'Shared from the AirQo app.', | |
| ]; | |
| return lines.join('\n'); | |
| final lines = <String>[ | |
| 'Air quality in $locationName is $category.', | |
| if (pm25Value != null) 'PM2.5: ${pm25Value.toStringAsFixed(1)} µg/m³', | |
| if (locationDescription.isNotEmpty) 'Location: $locationDescription', | |
| if (healthTip != null && healthTip.isNotEmpty) healthTip, | |
| 'https://airqo.net/billboard/grid/${_buildSlug(locationName)}', | |
| '', | |
| 'Shared from the AirQo app.', | |
| ]; | |
| return lines.join('\n'); | |
| } | |
| static String _buildSlug(String input) { | |
| final normalized = input.toLowerCase().trim(); | |
| final slug = normalized | |
| .replaceAll(RegExp(r'[^a-z0-9\s-]'), '') | |
| .replaceAll(RegExp(r'\s+'), '-') | |
| .replaceAll(RegExp(r'-{2,}'), '-'); | |
| return slug; | |
| } |
🤖 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/shared/services/air_quality_share_service.dart` around
lines 64 - 73, The share text message constructed in the lines list (which
includes the air quality category, PM2.5 value, location description, and health
tip) is missing the required canonical URL. Add the HTTPS billboard link in the
format https://airqo.net/billboard/grid/{slug} to the lines list, where slug
should be generated to match the website's slug generation logic. Include this
URL as an additional line in the lines list before the final "Shared from the
AirQo app." line, ensuring the returned share message satisfies the share-link
requirement.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds “Share air quality” entry points to the mobile location detail sheet (including quick text share and an image-based share card flow), and also introduces significant auth/session resiliency work (token refresh abstractions + lifecycle guard) along with new test coverage. It additionally includes an iOS simulator build workaround via a stubbed MLKit translation dependency.
Changes:
- Added share UI/actions from the location detail sheet, plus a share-card preview and image sharing flow (via
share_plus). - Introduced an
AppResumeSessionGuard+TokenRefresherabstraction and extensive auth/session tests. - Added an iOS simulator compatibility workaround by swapping
google_mlkit_translationto a local stub through dependency overrides.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mobile/lib/src/app/shared/services/air_quality_share_service.dart | New share service to build and dispatch share payloads via native share sheet. |
| src/mobile/lib/src/app/dashboard/widgets/analytics_specifics.dart | Adds Share button + bottom sheets for quick text/card sharing and captures a share card image. |
| src/mobile/lib/src/app/dashboard/widgets/air_quality_share_card.dart | New widget that renders the shareable “air quality card” UI. |
| src/mobile/pubspec.yaml | Adds share_plus dependency. |
| src/mobile/pubspec.lock | Locks share_plus and updates dependency resolution (including MLKit translation source). |
| src/mobile/pubspec_overrides.yaml | Forces MLKit translation to a local stub via dependency overrides. |
| src/mobile/local_packages/google_mlkit_translation_stub/README.md | Documents the local stub purpose/scope. |
| src/mobile/local_packages/google_mlkit_translation_stub/pubspec.yaml | Pubspec for the local MLKit translation stub package. |
| src/mobile/local_packages/google_mlkit_translation_stub/lib/google_mlkit_translation.dart | Stub implementation of the limited MLKit translation API surface used by the app. |
| src/mobile/lib/src/app/auth/lifecycle/app_resume_session_guard.dart | New lifecycle helper to refresh token on resume and notify session expiry. |
| src/mobile/lib/src/app/auth/bloc/auth_bloc.dart | Injects TokenRefresher to decouple startup refresh logic from AuthHelper. |
| src/mobile/lib/main.dart | Uses AppResumeSessionGuard on app resume instead of calling AuthHelper directly. |
| src/mobile/test/app/auth/services/auth_token_storage_test.dart | New tests for token sanitization + storage behaviors. |
| src/mobile/test/app/auth/services/auth_helper_test.dart | New tests for refresh token behavior (fail/timeout/network/malformed/valid). |
| src/mobile/test/app/auth/repository/secure_storage_repository_test.dart | New tests for secure storage save/get/delete behavior. |
| src/mobile/test/app/auth/repository/base_repository_auth_test.dart | New tests for authenticated GET handling, session-expiry classification, and token header persistence. |
| src/mobile/test/app/auth/lifecycle/app_resume_session_test.dart | New tests validating resume guard session-expiry notification behavior and deduping. |
| src/mobile/test/app/auth/integration/session_expiry_flow_test.dart | New integration-style test for 401 → session-expired → cleanup → guest fallback flow. |
| src/mobile/test/app/auth/bloc/auth_bloc_test.dart | New tests for AppStarted/session-expired/logout flows and storage cleanup. |
| src/mobile/ios/Runner.xcodeproj/project.pbxproj | Adjusts iOS build settings to allow simulator builds. |
| src/mobile/.gitignore | Normalizes ignore entries and adds ignores for local docs files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+50
to
+74
| static String buildShareMessage( | ||
| Measurement measurement, { | ||
| String? fallbackLocationName, | ||
| }) { | ||
| final locationName = measurement.siteDetails?.searchName ?? | ||
| measurement.siteDetails?.name ?? | ||
| fallbackLocationName ?? | ||
| 'this location'; | ||
| final category = measurement.aqiCategory ?? 'Unavailable'; | ||
| final pm25Value = measurement.pm25?.value; | ||
| final locationDescription = _locationDescription(measurement); | ||
| final healthTip = measurement.healthTips?.firstOrNull?.tagLine ?? | ||
| measurement.healthTips?.firstOrNull?.description; | ||
|
|
||
| final lines = <String>[ | ||
| 'Air quality in $locationName is $category.', | ||
| if (pm25Value != null) 'PM2.5: ${pm25Value.toStringAsFixed(1)} µg/m³', | ||
| if (locationDescription.isNotEmpty) 'Location: $locationDescription', | ||
| if (healthTip != null && healthTip.isNotEmpty) healthTip, | ||
| '', | ||
| 'Shared from the AirQo app.', | ||
| ]; | ||
|
|
||
| return lines.join('\n'); | ||
| } |
Comment on lines
+480
to
+483
| final image = await boundary.toImage(pixelRatio: pixelRatio); | ||
| final byteData = await image.toByteData(format: ui.ImageByteFormat.png); | ||
|
|
||
| return byteData?.buffer.asUint8List(); |
Comment on lines
+246
to
+252
| InkWell( | ||
| onTap: () => Navigator.pop(context), | ||
| child: Icon( | ||
| Icons.close, | ||
| color: nameColor, | ||
| ), | ||
| ), |
Comment on lines
+2
to
+6
| # the real MLKit translation plugin does not currently build cleanly for iOS | ||
| # simulator, so we swap it with a stub package to unblock auth debugging. | ||
| dependency_overrides: | ||
| google_mlkit_translation: | ||
| path: local_packages/google_mlkit_translation_stub |
Comment on lines
+89
to
+93
| server.listen((request) async { | ||
| expect(request.uri.path, '/api/v2/users/token/refresh'); | ||
| await Future<void>.delayed(const Duration(seconds: 11)); | ||
| request.response.statusCode = HttpStatus.ok; | ||
| request.response.write('{"success":true,"token":"fresh-token"}'); |
This was referenced Jul 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
This PR adds air quality sharing from the location detail sheet in the mobile app.
It includes:
Shareaction in the location detail sheetWhy
Users should be able to share the current air quality reading from a location in a way that works both as simple text and as a more visual card.
What I validated
Notes
Closes #3593
Summary by CodeRabbit
New Features
Dependencies
share_pluslibrary for enhanced sharing capabilitiesTests