Add error message and documentation when a SnackBar is off screen
#102073
Conversation
flutter-dashboard
bot
added
d: api docs
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
This is a really helpful addition to the API - thank you! Just a few suggestions and nits
examples/api/lib/material/scaffold/scaffold_messenger_state.show_snack_bar.1.dart
Outdated
Show resolved
Hide resolved
examples/api/lib/material/scaffold/scaffold_messenger_state.show_snack_bar.1.dart
Outdated
Show resolved
Hide resolved
examples/api/test/material/scaffold/scaffold_messenger_state.show_snack_bar.1_test.dart
Outdated
Show resolved
Hide resolved
bleroux
force-pushed
the
floating_snackbar_troubleshooting
branch
from
c596435 to
6ded7c3
Compare
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
Thanks for the updates! The example and the example's tests look great. It looks like there are more cases though that can cause this issue that we should be catching, see notes below. :)
Thanks!
There was a problem hiding this comment.
This if statement already exists above on line 1092, can this just be included in that block?
There was a problem hiding this comment.
Perhaps not, since the position of a floating snackbar can be affected by other Scaffold widgets too, like persistent footer buttons and bottom navigation bars, can you account for these cases too?
| if (floatingActionButtonRect.size != Size.zero && isSnackBarFloating) { | |
| if ( isSnackBarFloating) { |
Maybe? If there is no floating action button, how are the other cases handled?
There was a problem hiding this comment.
@Piinks
Very interesting! Large persistent footer buttons and large bottom navigation bars might happen too. I updated the PR accordingly.
About the if statement location, the check has to be done after calling positionChild otherwise the error is thrown before position is calculated and the SnackBar position will differ in debug mode (I first put the message in the existing if statement and noticed that).
There was a problem hiding this comment.
This if statement already exists above on line 1092, can this just be included in that block?
bleroux
force-pushed
the
floating_snackbar_troubleshooting
branch
2 times, most recently
from
0fce53d to
34be384
Compare
There was a problem hiding this comment.
I am not sure I follow this comment, why do we have to wait until after calling positionChild? We have the information we need to assert prior to the call right?
There was a problem hiding this comment.
Hey @Piinks!
If the assert is done before calling positionChild, the error is thrown and the Snackbar is positionned at (0,0).
Here is a screenshot :
If the assert is done after positionChild, the error is thrown and the SnackBar is not visible :
As we are throwing an error saying the SnackBar is not visible, It might feels strange for the user to see the Snackbar in this case. Maybe my comment could be reworded?
There was a problem hiding this comment.
Yes it does seem to be a contradiction - why would it throw in the case of the first image?
There was a problem hiding this comment.
I pushed an update where I tried to make the comment clearer 😅
There was a problem hiding this comment.
Unfortunately the commit history was squashed so I cannot see what specifically changed.
why would it throw in the case of the first image?
I am still wondering this. :)
bleroux
force-pushed
the
floating_snackbar_troubleshooting
branch
3 times, most recently
from
de39b02 to
c14f96e
Compare
There was a problem hiding this comment.
the error message states that the SnackBar is off screen.
Why would this error message be presented if it is on screen?
There was a problem hiding this comment.
the error message states that the SnackBar is off screen.
Why would this error message be presented if it is on screen?
This comment is about what would happen if we assert earlier, in the following if statement
https://github.com/flutter/flutter/blob/c14f96ed81c096fdde6d4bdb8d77c5ccbaac06e4/packages/flutter/lib/src/material/scaffold.dart#L1107
If we do the assert in this if statement, the error message will rightly state that the SnackBar is off screen, but the SnackBar will be wrongly visible at (0,0). The reason is that, before calling positionChild, the SnackBar has a default position of (0,0).
If the assert is done before calling positionChild, the final position will not be updated and the SnackBar will be wrongly visible (in debug mode).
There was a problem hiding this comment.
Thank you for explaining! I had a hard time wrapping my head around this one. :)
bleroux
force-pushed
the
floating_snackbar_troubleshooting
branch
from
c14f96e to
0c745a8
Compare
3 participants
Description
Scaffold.performLayoutto throw a Flutter error when a SnackBar is off screen.Scaffold.showSnackBartroubleshooting comments.Related Issue
Fixes #84263
Tests
Add a test to material/snack_bar_test.dart
Add a test for the new example
Pre-launch Checklist
///).