-
-
Notifications
You must be signed in to change notification settings - Fork 287
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
feat: #1377 - reextract ingredients #1644
feat: #1377 - reextract ingredients #1644
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1644 +/- ##
==========================================
- Coverage 8.86% 8.66% -0.20%
==========================================
Files 161 164 +3
Lines 6623 6829 +206
==========================================
+ Hits 587 592 +5
- Misses 6036 6237 +201
Continue to review full report at Codecov.
|
It looks great 👍 @cli1005 |
@g123k @M123-dev @monsieurtanuki does that look good to you ? |
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.
@teolemon I've already commented this PR (or I have and have forgotten to click on the "submit review" button):
- some minor formatting comments
- not fan of "we call the server but there's no progress display or cancel button"
- not fan of "we don't care if the server call failed"
Beyond that, this is not an area of the app where I've spent a lot of time, therefore I don't fully understand the functional purpose - but that looks rather good.
packages/smooth_app/lib/cards/product_cards/knowledge_panels/knowledge_panels_builder.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/cards/product_cards/knowledge_panels/knowledge_panels_builder.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/edit_ingredients_page.dart
Outdated
Show resolved
Hide resolved
await onSubmitField(); | ||
Navigator.pop(context, 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.
Same behavior if onSubmitField
crashes or not? pop(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.
onSubmitField shows an error message to the user in a SnackBar in case of failure. I will remove the return value to prevent confusion
…om/cli1005/smooth-app into feature/1377-reextract-ingredients
|
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.
Thank you @cli1005, LGTM!
What
It should be possible to extract ingredients without taking a new photo
More details : #1377 (comment)
Screenshot
simulator-screen-recording-iphone-13-2022-04-25-at-163838_xiv1Anxx.mp4
Fixes bug(s)
Part of