Skip to content

Conversation

AdoHal
Copy link

@AdoHal AdoHal commented Nov 23, 2021

Proposed Changes

  • Implemented barcode scanner
  • Created tests for MealItemForm screen

Related Issues (if applicable)

Please check that the PR fulfills these requirements

  • Set a 100 character limit in your editor/IDE to avoid white space diffs in the PR
  • Tests for the changes have been added (for bug fixes / features)
  • Added yourself to AUTHORS.md

@rolandgeider rolandgeider linked an issue Dec 1, 2021 that may be closed by this pull request
);

if(data['count'] != 0) {
ingredient = Ingredient.fromJson(data['results'][0]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can just do return Ingredient.fromJson(...)

String barcode;
try {
barcode = await FlutterBarcodeScanner.scanBarcode(
'#ff6666', 'Cancel', true, ScanMode.BARCODE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Cancel" is used in UI right? Better use a translated string. There are a handfull from flutter/android for such things, I'm not sure how to access them (if it's complicated, just add cancel to app_en.arb if it isn't there already)

child: Column(
children: [
IngredientTypeahead(_ingredientIdController, _ingredientController),
ElevatedButton(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to add this to ingredient typeahead? Ideally we would have a single widget where we could either type a name or scan a product. We can add a barcode scanner as a suffixIcon that opens the scanner (the search icon would go to preffixIcon). If that's not possible because the typeahead logic interferes with it, we can leave it as a separate button

@rolandgeider
Copy link
Member

oh, and add yourself to AUTHORS.md ;)

@rolandgeider
Copy link
Member

Thanks for the PR! I want to submit the app to the f-droid app store before merging more things into master but once that's done I'll merge this (a feature that's been missing for quite some time)

@rolandgeider rolandgeider merged commit fa4dfe6 into wger-project:master Jan 23, 2022
@rolandgeider
Copy link
Member

This took a big longer to merge, but it's finally done. Thanks again 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Barcode scanner
2 participants