-
Notifications
You must be signed in to change notification settings - Fork 0
シェアに関する不具合を直しました #864
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
シェアに関する不具合を直しました #864
Conversation
WalkthroughTwo files were modified to improve sharing functionality: timeout and exception handling were added to Share.shareXFiles calls in the share helper with a 5-second timeout, and the share widget was updated to use Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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. Comment |
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/core/utils/helpers/share_helper.dart (1)
59-60: Broad exception catch swallows all errors silently.Catching
Exceptionwithout logging or user feedback makes debugging difficult and leaves users unaware of failures. Consider:
- Logging the exception for debugging purposes
- Optionally providing feedback to the user that sharing failed
- } on Exception { - return; + } on Exception catch (e) { + debugPrint('Share failed: $e'); + return; } finally {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/core/utils/helpers/share_helper.dart(3 hunks)lib/ui/component/app_share_widget.dart(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: iseruuuuu
Repo: iseruuuuu/food_gram_app PR: 760
File: lib/ui/screen/post_detail/post_detail_screen.dart:110-116
Timestamp: 2025-08-29T23:33:31.078Z
Learning: ユーザーiseruuuuのfood_gram_appでは、RPC実装の提案がうまく動作せず、また同時にいいねが発生する可能性は低いとの判断。
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (1)
lib/ui/component/app_share_widget.dart (1)
36-42: Use the correct computed property for displaying the first image.The change from
posts.foodImagetoposts.firstFoodImageis correct. The rawfoodImagefield stores comma-separated paths, whilefirstFoodImageis the proper extension getter that extracts the first image. This pattern is already used consistently throughout the codebase for similar display scenarios.
| try { | ||
| await Share.shareXFiles( | ||
| files, | ||
| text: text, | ||
| ).timeout(const Duration(seconds: 5)); | ||
| } on TimeoutException { | ||
| return; | ||
| } |
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.
🧩 Analysis chain
🌐 Web query:
share_plus Flutter shareXFiles timeout behavior documentation
💡 Result:
Short answer: There is no documented "timeout" parameter or built‑in timeout behavior for Share.shareXFiles / Share.share in share_plus. The plugin APIs (pub.dev) return a ShareResult when the platform share UI completes or is dismissed, but the plugin does not expose or document any configurable timeout — any perceived timeout is platform/UI‑dependent (iOS UIActivityViewController / Android chooser) or caused by app/OS behavior. [1][2]
Sources:
- share_plus package docs (API/behavior; no timeout option shown). [1]
- share_plus GitHub issues where sharing behavior is discussed (no plugin timeout feature — platform/recipient app differences are the usual cause). [2]
🌐 Web query:
Flutter Share.shareXFiles Future resolution when user dismisses sheet
💡 Result:
Short answer: await Share.shareXFiles(...) completes when the platform share API/chooser finishes or is dismissed and returns a ShareResult. Check result.status for ShareResultStatus.success, .dismissed or .unavailable to tell what happened. Behavior can vary by platform (some platforms may only return .unavailable or a raw string), and there are occasional platform-specific bugs (e.g. iOS 26 reported where the call may not return). [1][2][3]
Example:
final result = await Share.shareXFiles([XFile(path)]);
if (result.status == ShareResultStatus.success) { /* shared / }
else if (result.status == ShareResultStatus.dismissed) { / user dismissed / }
else if (result.status == ShareResultStatus.unavailable) { / no support */ }
Sources: share_plus docs (shareXFiles and ShareResult) and related issue. [1][2][3]
🌐 Web query:
appropriate timeout duration for Flutter share sheet operations best practices
💡 Result:
Short answer
- For automated tests: wait 15–30 seconds for the platform share sheet to appear (use 15s as a starting point, increase to 30s on slow devices/emulators). [1][2][3][4]
- For app code / UX: don’t block the UI waiting for a share-target response; show the share sheet and return immediately. If you must time-limit a follow-up action (e.g., awaiting a callback or a background handoff), use ~10–30 seconds and fail gracefully. Keep interactive/animation thresholds <<100 ms. [5][1]
Rationale and practical guidance
- Share sheets rely on system UI and other apps/extensions (e.g., Direct Share, activity extensions) which can be slow or vary by device/OS — that’s why tests and automation commonly use multi-second timeouts (examples in community tests use 15–20s; platform tests use “lengthy” waits). Start tests at ~15s and raise to 30s if flaky. [2][3][4]
- For user-facing code, don’t block the main thread; present the UIActivityViewController / Android Sharesheet and return control immediately. If you need to detect completion and time it out, choose conservative values (10–30s) and show a non‑blocking spinner/notice and a clear fallback or error. [1][5]
- Keep responsiveness targets separate: perceptible UI delays should be under ~100 ms; share-sheet waits are a different layer and can be much longer because they involve other processes. [5]
Concrete recommendations (copyable)
- UI tests (XCUITest / Flutter integration tests): initial waitForExistence timeout = 15s; increase to 30s for CI or older devices. [3][4]
- Integration code that awaits a share completion: prefer callbacks; if you must apply a timeout use 10–30s and show a user-facing fallback. [1][2]
- Logging & metrics: record how long the sheet takes on different devices and adjust CI/test timeouts per device class. [1][3]
Sources
- Android Sharesheet / sharing guidance (use system chooser; sharesheet behavior can vary). [1]
- Android training + examples / community test usage showing 15–30s waits. [2][3]
- iOS/XCTest / UIActivityViewController test discussions and recommended wait patterns (examples using 2–15+ second waits; Xcode test timeout controls). [4][3]
- UX responsiveness thresholds (100 ms guidance). [5]
If you want, I can: (a) propose exact timeout constants for your Flutter app/tests, (b) show example Flutter test code with those timeouts, or (c) help measure share-sheet latency on a CI matrix.
5-second timeout is too aggressive for share operations and causes silent failures.
Share.shareXFiles opens the system share sheet and waits for the user to dismiss it. According to the share_plus package documentation, the operation returns a ShareResult with status indicators (success/dismissed/unavailable), not a timeout. A 5-second timeout will likely fail during normal user interaction—users typically need 10–30 seconds to view the share sheet, select an app, and complete the action.
If a timeout is necessary, increase it to at least 10–30 seconds. Better yet, remove the timeout and check the ShareResult.status instead of silently returning, so users receive proper feedback about whether the share succeeded, was dismissed, or is unavailable.
🤖 Prompt for AI Agents
In lib/core/utils/helpers/share_helper.dart around lines 18 to 25, the code
applies a 5-second timeout to Share.shareXFiles which is too short and causes
silent failures; remove the aggressive timeout and instead await the
Share.shareXFiles call (or use a longer configurable timeout like 10–30s if you
must) and inspect the returned ShareResult.status to handle success, dismissed,
or unavailable cases (log or surface user feedback instead of silently returning
on timeout).
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.
@CodeRabbit
どうしたらいいかな?
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.
@iseruuuuu さん、いくつかの選択肢があります:
オプション1: タイムアウトを削除してShareResultを使用(推奨)
Share.shareXFilesは通常、ユーザーがシェアシートを閉じると自動的に完了するので、タイムアウトは不要です。代わりに結果を確認できます:
Future<void> sharePosts(
List<XFile> files,
String text,
) async {
final result = await Share.shareXFiles(
files,
text: text,
);
// 必要に応じて result.status をログに記録
// ShareResultStatus.success, .dismissed, .unavailable など
}オプション2: タイムアウトを30秒に延長
もし何らかの理由でタイムアウトが必要な場合は、5秒では短すぎます。ユーザーがシェアシートを見て、アプリを選択し、シェアを完了するには10〜30秒必要です:
Future<void> sharePosts(
List<XFile> files,
String text,
) async {
try {
await Share.shareXFiles(
files,
text: text,
).timeout(const Duration(seconds: 30));
} on TimeoutException {
// 必要に応じてログ記録
return;
}
}オプション3: エラーログを追加(現状維持の場合)
現在の実装を維持する場合でも、デバッグのためにログを追加することをお勧めします:
Future<void> sharePosts(
List<XFile> files,
String text,
) async {
try {
await Share.shareXFiles(
files,
text: text,
).timeout(const Duration(seconds: 30)); // 5秒→30秒に変更
} on TimeoutException {
debugPrint('Share operation timed out');
return;
}
}推奨: シェア機能のバグ修正が目的であれば、オプション1(タイムアウトを削除)またはオプション2(30秒に延長)をお勧めします。5秒のタイムアウトは短すぎて、正常な操作でも失敗する可能性があります。
どの方法が良いか教えていただければ、具体的なコードを提供します!
🧠 Learnings used
Learnt from: iseruuuuu
Repo: iseruuuuu/food_gram_app PR: 760
File: lib/ui/screen/post_detail/post_detail_screen.dart:110-116
Timestamp: 2025-08-29T23:33:31.078Z
Learning: ユーザーiseruuuuのfood_gram_appでは、RPC実装の提案がうまく動作せず、また同時にいいねが発生する可能性は低いとの判断。
| Future<void> shareOnlyPost(List<XFile> files) async { | ||
| await Share.shareXFiles(files); | ||
| try { | ||
| await Share.shareXFiles(files).timeout(const Duration(seconds: 5)); | ||
| } on TimeoutException { | ||
| return; | ||
| } | ||
| } |
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.
Same timeout concern applies here.
This has the same issue as sharePosts - the 5-second timeout may prematurely cancel legitimate user interactions with the share sheet.
🤖 Prompt for AI Agents
In lib/core/utils/helpers/share_helper.dart around lines 28 to 34, the
shareOnlyPost method applies a hard 5-second timeout to Share.shareXFiles which
can prematurely cancel legitimate user interactions with the platform share
sheet; remove the .timeout(const Duration(seconds: 5)) (or replace it with a
much longer/configurable timeout if you have a specific reason) so the share
call is allowed to complete naturally and not be cut off by the short fixed
timeout.
iseruuuuu
left a comment
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.
LGTM
Issue
概要
追加したPackage
Screenshot
RPReplay_Final1764901439.MP4
備考
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.