-
Notifications
You must be signed in to change notification settings - Fork 0
flutter: SharePlus migration step 2 — switch to SharePlus.instance + ShareParams #62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…hareParams (share_service + QR widget)
…re; invite_member_dialog const fix; prep PR
…nsts; coalesce PermissionGuard children
…ger/navigator; finalize; tests green
|
Batch 8 additions included here for efficiency:\n- ThemeShareDialog: messenger capture + mounted guards\n- InviteMemberDialog: const-eval fix in _buildInfoRow\n- PermissionGuard: non-null children for Elevated/Text/Filled\n- LoadingIndicator/QRCode: remove invalid consts\n- TemplateAdminPage: messenger/navigator capture + mounted guards\n\nAll tests pass locally; analyzer warnings reduced. Remaining conservative warnings (e.g., delete_family_dialog) planned for follow-up. Next, I will add lightweight smoke tests for Share service (text and files) using SharePlus platform interface fakes, and submit in a small PR. |
…ePlus.instance; adjust tests to use platform delegate
… to _doShare; null-safe tags
… internals; fix ShareResult ctor
…re paths use _doShare
…plicitly earlier already applied
…Test only; simplify screenshot stub; run tests
…eResult; remove stray lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR migrates the SharePlus API usage from deprecated static methods to the instance-based form using SharePlus.instance with ShareParams. The migration focuses strictly on API compatibility while maintaining existing functionality.
- Switch from
Share.share()andShare.shareXFiles()toSharePlus.instance.share()withShareParams - Add test infrastructure for the new API in ShareService
- Update QR code sharing implementation to use the new API pattern
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| jive-flutter/lib/services/share_service.dart | Core migration from static Share methods to SharePlus.instance with ShareParams, includes test infrastructure |
| jive-flutter/lib/widgets/qr_code_generator.dart | Update QR code sharing to use new SharePlus API |
| jive-flutter/test/services/share_service_test.dart | New test file for ShareService with ShareParams validation |
| Other files | Unrelated cleanup changes (const/non-const widgets, context.mounted fixes) |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| child: Center( | ||
| child: CircularProgressIndicator(), | ||
| child: const Center( | ||
| child: const CircularProgressIndicator(), |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double const keyword is redundant. Remove the second 'const' keyword from the CircularProgressIndicator constructor.
| child: const CircularProgressIndicator(), | |
| child: CircularProgressIndicator(), |
Copilot uses AI. Check for mistakes.
| 🏠 账本:$familyName | ||
| ${transaction.tags.isNotEmpty ? '🏷️ 标签:${transaction.tags.join(', ')}' : ''} | ||
| ${transaction.tags?.isNotEmpty == true ? '🏷️ 标签:${transaction.tags!.join(', ')}' : ''} |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The null-aware check followed by a force unwrap (!.) is risky. Consider using transaction.tags?.join(', ') directly or a more defensive pattern to avoid potential null reference exceptions.
| ${transaction.tags?.isNotEmpty == true ? '🏷️ 标签:${transaction.tags!.join(', ')}' : ''} | |
| ${transaction.tags?.isNotEmpty == true ? '🏷️ 标签:${transaction.tags?.join(', ')}' : ''} |
Copilot uses AI. Check for mistakes.
| try { | ||
| final xFiles = images.map((file) => XFile(file.path)).toList(); | ||
| await Share.shareXFiles(xFiles, text: text); | ||
| final List<XFile> xFiles = images.map((file) => XFile(file.path)).toList(); |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The explicit type annotation List<XFile> is redundant since the type can be inferred from the map operation and toList() call.
| final List<XFile> xFiles = images.map((file) => XFile(file.path)).toList(); | |
| final xFiles = images.map((file) => XFile(file.path)).toList(); |
Copilot uses AI. Check for mistakes.
Scope
Notes
Follow-ups