flutter: context cleanup batch 3 — post-await captures + scoped ignores #60
Conversation
…nce.share/shareXFiles in QR + ShareService; remove local stub
…hare_plus directly
…g and DeleteFamilyDialog
…or) + fix const-eval in common widgets
…pture and fix duplicates
…ignores; fix template onCancel and batch dialogs snackbars
Summary of ChangesHello @zensgit, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on improving the stability and maintainability of the Flutter application by addressing Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull Request Overview
This PR continues the effort to reduce use_build_context_synchronously warnings by moving context-dependent captures (Navigator/ScaffoldMessenger) to after awaits with mounted guards and adding scoped ignores where necessary.
- Capture Navigator/ScaffoldMessenger references before async gaps to avoid context usage after awaits
- Add mounted checks after async operations with early returns
- Use scoped
// ignore: use_build_context_synchronouslycomments where analyzer false-positives persist despite proper guards
Reviewed Changes
Copilot reviewed 36 out of 40 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/widgets/batch_operation_bar.dart | Add scoped ignores and post-await captures for batch operations |
| lib/widgets/dialogs/delete_family_dialog.dart | Capture navigator/messenger post-await with mounted guards |
| lib/widgets/theme_share_dialog.dart | Pre-capture messenger references before async operations |
| lib/widgets/custom_theme_editor.dart | Capture navigator/messenger before awaits in save theme |
| lib/screens/admin/template_admin_page.dart | Add scoped ignores and post-await captures for save/delete operations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // ignore: use_build_context_synchronously | ||
| final navigator = Navigator.of(context); | ||
| // ignore: use_build_context_synchronously | ||
| final messenger = ScaffoldMessenger.of(context); |
There was a problem hiding this comment.
[nitpick] Instead of using multiple ignore comments, consider capturing both navigator and messenger before the async gap to avoid needing the ignores.
Copilot uses AI. Check for mistakes.
| // ignore: use_build_context_synchronously | ||
| final messenger = ScaffoldMessenger.of(context); |
There was a problem hiding this comment.
[nitpick] Similar to the previous comment, this ignore could be avoided by capturing the messenger reference before the async operation instead of after.
Copilot uses AI. Check for mistakes.
| // ignore: use_build_context_synchronously | ||
| final messenger = ScaffoldMessenger.of(context); |
There was a problem hiding this comment.
The ignore comment is unnecessary here since messenger is captured after the mounted check but there's no async gap between the check and this line.
Copilot uses AI. Check for mistakes.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request focuses on improving the codebase by addressing use_build_context_synchronously warnings in the Flutter app and adding logging to the Rust backend. The effort to clean up context usage across async gaps is commendable. I've noticed that in some places, the correct pattern of capturing context-dependent objects before await calls has been applied successfully. However, in other areas, the lint warning was suppressed with // ignore comments, which is not the ideal solution. I've provided detailed suggestions on how to refactor these parts to follow best practices, ensuring better stability and maintainability. Additionally, I've included a suggestion for the Rust code to use a more idiomatic error handling pattern. Overall, these are valuable improvements, and with the suggested changes, the code quality will be even higher.
| let family = match family_service.create_family(user_id, family_request).await { | ||
| Ok(f) => f, | ||
| Err(e) => { | ||
| tracing::error!(target: "auth_service", error = ?e, user_id = %user_id, "register_with_family: create_family failed"); | ||
| return Err(e); | ||
| } | ||
| }; |
There was a problem hiding this comment.
While this match block works for logging the error before propagating it, a more idiomatic and concise way to achieve this in Rust is by using map_err. This approach avoids the verbosity of a match statement when you just want to perform a side-effect on the error case before letting it propagate with ?.
let family = family_service.create_family(user_id, family_request).await.map_err(|e| {
tracing::error!(target: "auth_service", error = ?e, user_id = %user_id, "register_with_family: create_family failed");
e
})?;…vigator capture and mounted guards
1 participant
Purpose
Changes
Validation
Notes