-
Notifications
You must be signed in to change notification settings - Fork 17
Move SnackBars to top to avoid blocking bottom navigation #413
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
- Display SnackBars at the top of the screen using floating behavior - Prevent SnackBars from overlapping the bottom navigation bar - Allow uninterrupted navigation while notifications are visible - Centralize SnackBar configuration via a shared helper - Ensure consistent notification behavior across the app
WalkthroughCentralizes SnackBar presentation by adding a new Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 9
🤖 Fix all issues with AI agents
In `@lib/features/chat/widgets/encrypted_file_message.dart`:
- Around line 496-514: Replace the hardcoded SnackBar messages in the error
handling of open file logic by using localized strings from S.of(context)!;
update the two calls to SnackBarHelper.showTopSnackBar in
encrypted_file_message.dart to use S.of(context)!.couldNotOpenFile (with
result.message interpolated) and S.of(context)!.errorOpeningFile (with the error
interpolated) instead of the literal strings, and add the corresponding
keys/values to the ARB/localization files so translations include placeholders
for the interpolated message/error; keep the same duration/backgroundColor and
preserve the checks for mounted.
In `@lib/features/chat/widgets/message_input.dart`:
- Around line 131-135: Replace the raw exception text shown in
SnackBarHelper.showTopSnackBar inside message_input.dart with a localized
user-facing string (use S.of(context).<appropriateKey> like errorUploadingFile
or uploadError) and move the exception details to a log/diagnostic call (e.g.,
debugPrint or your logger) so the UI only displays the localized message while
the actual exception $e is logged for troubleshooting; update the
SnackBarHelper.showTopSnackBar call and add a nearby log statement (referencing
SnackBarHelper.showTopSnackBar and the surrounding upload/error handling code)
to implement this change.
In `@lib/features/disputes/widgets/dispute_input_section.dart`:
- Around line 130-144: Replace the hardcoded SnackBar strings with localized
strings from S.of(context)!: update the two calls to
SnackBarHelper.showTopSnackBar in the success and catch blocks to use
S.of(context)!.<yourKey>(...) (e.g., messageSent and failedToSendMessage) that
accept the message or error as parameters, or use S.of(context)!.<yourKey> with
string interpolation if you added keys without params; keep the same call sites
(SnackBarHelper.showTopSnackBar, inside the mounted checks) and add new
localization keys (or parameterized keys) to the arb/localization files to
provide "Message sent: {message}" and "Failed to send message: {error}"
translations.
In `@lib/features/disputes/widgets/dispute_message_bubble.dart`:
- Line 5: Replace the nullable localization access and hardcoded fallback with
the non-nullable localized string: locate where AppLocalizations.of(context)? or
similar is used to read messageCopiedToClipboard in dispute_message_bubble.dart
and change it to use the non-nullable accessor (e.g.,
AppLocalizations.of(context)!.messageCopiedToClipboard) and remove the "??
'...'" fallback so the code uses the ARB-provided string directly.
In `@lib/features/order/screens/add_lightning_invoice_screen.dart`:
- Around line 83-86: Replace the hardcoded error string passed to
SnackBarHelper.showTopSnackBar with a localized message from S.of(context)!,
e.g. use a new key like cancelOrderFailed (or similar) combined with the
exception text; update the call in add_lightning_invoice_screen.dart where
SnackBarHelper.showTopSnackBar(...) is invoked to use
S.of(context)!.cancelOrderFailed and concatenate or format the exception, and
add the corresponding cancelOrderFailed entry to the ARB localization files (and
regenerate localization artifacts) so the message is available at runtime.
- Around line 67-70: Replace the hardcoded error string passed to
SnackBarHelper.showTopSnackBar with a localized message: use S.of(context)! to
fetch a new localization key (e.g., failedUpdateInvoice) and include the
exception text via string interpolation or formatting; add the corresponding
key/value to the ARB localization files and regenerate localization classes so
S.of(context)!.failedUpdateInvoice can be used in the
SnackBarHelper.showTopSnackBar call in add_lightning_invoice_screen.dart.
In `@lib/shared/utils/snack_bar_helper.dart`:
- Around line 18-33: The current SnackBar positioning uses hardcoded pixels
(top: 96 and bottomMargin = mediaQuery.size.height - 96 - 70) which breaks on
devices with notches or different AppBar heights; change the layout to compute
the top offset using MediaQuery.of(context).padding.top for the status bar and
accept an optional appBarHeight parameter (or default to kToolbarHeight) to
compute margin.top = mediaQuery.padding.top + (appBarHeight ?? kToolbarHeight) +
16, replace bottomMargin calculation to mediaQuery.size.height - margin.top -
estimatedSnackBarHeight (or use a Positioned overlay for precise control), and
call ScaffoldMessenger.of(context).clearSnackBars() before
ScaffoldMessenger.of(context).showSnackBar(...) so multiple notifications don’t
stack; adjust references to bottomMargin and top margin in the SnackBar margin
block accordingly.
In `@lib/shared/widgets/clickable_text_widget.dart`:
- Around line 38-44: Replace the hardcoded SnackBar message in _handleTap by
using the localization helper S.of(context)! instead of the literal string:
build the message with the appropriate localization key (e.g.,
S.of(context)!.copiedToClipboard) combined with widget.leftText and
widget.clickableText, and pass that to SnackBarHelper.showTopSnackBar; also add
the import for l10n.dart at the top and ensure the corresponding ARB key (e.g.,
"copiedToClipboard": "{prefix} {value} copied to clipboard" or a suitable key
name) exists and is generated so S.of(context)! can be used.
In `@lib/shared/widgets/exchange_rate_widget.dart`:
- Line 6: Import the generated localization file and replace the hardcoded
SnackBar string with the localized key: add import
'package:mostro_mobile/generated/l10n.dart' at the top of
lib/shared/widgets/exchange_rate_widget.dart, update the SnackBar message
creation in the widget (where it currently uses "Refreshing exchange rate...")
to use S.of(context)!.refreshingExchangeRate, and add the key
refreshingExchangeRate: "Refreshing exchange rate..." to the three ARB files
(en, es, it) so the localized string is available.
🧹 Nitpick comments (3)
lib/features/auth/screens/register_screen.dart (1)
12-12: Defer SnackBar side effect to a post-frame callback.
In screens, show SnackBars after the current frame to keep build/side-effects separated.♻️ Suggested change
- SnackBarHelper.showTopSnackBar( - context, - state.error, - ); + WidgetsBinding.instance.addPostFrameCallback((_) { + SnackBarHelper.showTopSnackBar( + context, + state.error, + ); + });Based on learnings, prefer post-frame callbacks for SnackBars in screen widgets.
Also applies to: 42-46
lib/features/order/screens/add_order_screen.dart (1)
395-399: Defer SnackBars to a post-frame callback in this screen.
This keeps UI side effects out of synchronous flow per screen guidelines.♻️ Suggested pattern (apply to all three SnackBarHelper calls)
- SnackBarHelper.showTopSnackBar( - context, - S.of(context)!.pleaseSelectCurrency, - ); + WidgetsBinding.instance.addPostFrameCallback((_) { + if (!mounted) return; + SnackBarHelper.showTopSnackBar( + context, + S.of(context)!.pleaseSelectCurrency, + ); + });As per coding guidelines, use post-frame callbacks for SnackBars in screens.
Also applies to: 407-410, 430-434
lib/features/trades/screens/trade_detail_screen.dart (1)
726-744: Defer dispute SnackBars to a post-frame callback.
Keeps side effects out of synchronous UI flow.♻️ Suggested pattern
- if (success && context.mounted) { - SnackBarHelper.showTopSnackBar( - context, - S.of(context)!.disputeCreatedSuccessfully, - backgroundColor: AppTheme.mostroGreen, - ); - } else if (context.mounted) { - SnackBarHelper.showTopSnackBar( - context, - S.of(context)!.disputeCreationFailed, - backgroundColor: AppTheme.red1, - ); - } + WidgetsBinding.instance.addPostFrameCallback((_) { + if (!context.mounted) return; + if (success) { + SnackBarHelper.showTopSnackBar( + context, + S.of(context)!.disputeCreatedSuccessfully, + backgroundColor: AppTheme.mostroGreen, + ); + } else { + SnackBarHelper.showTopSnackBar( + context, + S.of(context)!.disputeCreationFailed, + backgroundColor: AppTheme.red1, + ); + } + });As per coding guidelines, use post-frame callbacks for SnackBars in screens.
- Remove hardcoded pixel values flagged in code review - Use system-provided metrics for layout calculations - Prevent multiple SnackBars from stacking
|
@coderabbitai |
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: 1
🤖 Fix all issues with AI agents
In `@lib/shared/utils/snack_bar_helper.dart`:
- Around line 60-82: The computed bottomMargin can be negative on compact
screens; update the calculation that defines bottomMargin (currently
screenHeight - topMargin - estimatedSnackBarHeight) to clamp it to a minimum of
0.0 (e.g., use a max operation) before passing it into the SnackBar margin in
messenger.showSnackBar so EdgeInsets.only(bottom: bottomMargin) never receives a
negative value; adjust the variable near topMargin/estimatedSnackBarHeight and
ensure the clamped bottomMargin is used in the SnackBar constructor.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2841 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 1149 🏁 Script executed: Length of output: 2966 🏁 Script executed: Length of output: 43 🏁 Script executed: Length of output: 2129 🏁 Script executed: Length of output: 3052 🏁 Script executed: Length of output: 2277 🏁 Script executed: Length of output: 161 🏁 Script executed: Length of output: 46
The issue documents all 7 files with hardcoded SnackBar messages that need localization:
Each entry includes the specific line numbers, current hardcoded strings, and suggested localization keys to add to your existing ARB files (I noticed you already have The issue is assigned to you and includes an implementation checklist. You're absolutely right that these are pre-existing strings—your PR |
|
@grunch la mayoría de los comentarios de coderabbit eran sobre mensajes harcodeados que salían en la snackbar y ya estaban desde antes en la rama main, aquí no fue agregado ningún mensaje nuevo. Como el objetivo de este PR es solo mover la snackbar hacia arriba y ya tiene 23 archivos cambiados, le dije a coderabbit que creara un issue con todos esos mensajes, para resolverlos en otro PR, no en este. |
grunch
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.
great job!
fix #397
This PR touches 23 files (22 updates + 1 new), but the changes are minimal and repetitive. Each file was updated to use the new shared SnackBar helper, ensuring consistent top-positioned SnackBars and preventing navigation blocking.
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.