-
-
Notifications
You must be signed in to change notification settings - Fork 263
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
fix: "Stores" field to not show loading spinner (#4669) #4678
fix: "Stores" field to not show loading spinner (#4669) #4678
Conversation
Loading spinner was showing, even if there were no loading or suggestions available. Fixes openfoodfacts#4669
Codecov Report
@@ Coverage Diff @@
## develop #4678 +/- ##
=======================================
Coverage 9.91% 9.91%
=======================================
Files 310 310
Lines 15798 15795 -3
=======================================
Hits 1566 1566
+ Misses 14232 14229 -3
... and 10 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@@ -79,6 +79,7 @@ class _SimpleInputTextFieldState extends State<SimpleInputTextField> { | |||
if (_manager == null || search.length < widget.minLengthForSuggestions) { | |||
_suggestions[search] = _SearchResults.empty(); | |||
} else { | |||
_setLoading(true); |
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.
What about _setLoading(false);
in the catch
?
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.
I would say no, since there is a _setLoading(false)
call, that will be always called after try-catch block.
But there is a bug here. If exception is thrown, _suggestions[search]
will be null, and it will not disable loading and also crash the app on the last return. Will fix it.
Also fixes crash and properly disables loading, when try-catch block throws exception.
58a7e52
to
825a333
Compare
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.
Hi @WildOrangutan!
I haven't tested it but that looks good to me, thank you!
What
"simple input text field " was showing loading spinner, even if there were no autocomplete suggestions available.
Any input field, without autocomplete enabled, will now not show loading spinner.
Screenshot
untitled.webm
Fixes bug(s)
Fixes #4669