-
-
Notifications
You must be signed in to change notification settings - Fork 275
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: #1395 - show newly added product in carousel #1520
fix: #1395 - show newly added product in carousel #1520
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1520 +/- ##
==========================================
- Coverage 8.90% 8.89% -0.02%
==========================================
Files 161 161
Lines 6569 6580 +11
==========================================
Hits 585 585
- Misses 5984 5995 +11
Continue to review full report at Codecov.
|
Heyy the code looks good, but there is one flaw in the user flow left. We restrict going back over the back button in the AppBar, but going back via gesture or system back button is still possible. If we need to do some logic before going back we can always do this with |
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 @cli1005!
I have no strong opinions regarding the functional part of this PR.
I have minor comments about the code itself; please have a look at them.
return true; | ||
} | ||
|
||
Future<void> updateContinuousScanModel( |
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.
Unless I missed something, please make it private.
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.
Unless I missed something, please make it private.
✅
@@ -35,12 +35,14 @@ class _AddNewProductPageState extends State<AddNewProductPage> { | |||
final Map<ImageField, List<File>> _uploadedImages = | |||
<ImageField, List<File>>{}; | |||
|
|||
bool isProductLoaded = false; |
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.
Unless I missed something, please make it private.
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.
Unless I missed something, please make it private.
✅
@@ -33,11 +33,12 @@ class _SmoothProductCarouselState extends State<SmoothProductCarousel> { | |||
bool _returnToSearchCard = false; | |||
|
|||
int get _searchCardAdjustment => widget.containSearchCard ? 1 : 0; | |||
late ContinuousScanModel model; |
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.
Unless I missed something please make it private.
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.
Unless I missed something please make it private.
✅
@@ -105,6 +105,13 @@ class ContinuousScanModel with ChangeNotifier { | |||
_addBarcode(code); | |||
} | |||
|
|||
Future<bool> onCreate(String? barcode) async { |
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.
onCreate
is a bit vague and misleading (sounds like a constructor or a lifecycle method). What about onProductCreation
?
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.
✅
Showing the back button after product has been loaded might mislead user that they could end the creation without creating any product since they have not clicked the button Finish 🤔 |
What about showing a Snackbar directly after every picture is uploaded |
And we might change the messages to something like : "Upload your first photo to create a new product", @teolemon, maybe a review of the adding product flow is necessary, for me the following flow is more logical, personal opinion 🤔
⬇️
⬇️
⬇️
|
The minimum would be front, ingredients, nutrition. Behind the scenes, we can create the product as soon as we want (1st image sounds good) |
I merge this pr, more features à add concerning add product flow will be handled in #881 or issues related |
What
Screenshot
Fixes bug(s)
Part of