-
-
Notifications
You must be signed in to change notification settings - Fork 274
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
feature/#55 - SQLite local database, and refactoring with StatefulWidget's #62
Merged
PrimaelQuemerais
merged 7 commits into
openfoodfacts:master
from
monsieurtanuki:feature/#55
Jan 8, 2021
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
2dbb6b6
feature/#55 - SQLite local database, and refactoring with StatefulWid…
monsieurtanuki c5f5b93
feature/#55 - experimenting the NotifyListeners method in a clean mod…
monsieurtanuki ee64551
Clean separation between model and page around product query
monsieurtanuki a8d8b47
Merge branch 'master' into feature/#55
monsieurtanuki b8c6f42
Changes to match v0.3.14+1 of openfoodfacts-dart
de8de42
Merge branch 'master' into feature/#55
monsieurtanuki dd35d5c
Minor merge fix.
monsieurtanuki File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
101 changes: 0 additions & 101 deletions
101
packages/smooth_app/lib/data_models/choose_page_model.dart
This file was deleted.
Oops, something went wrong.
198 changes: 75 additions & 123 deletions
198
packages/smooth_app/lib/data_models/continuous_scan_model.dart
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,154 +1,106 @@ | ||
| import 'package:carousel_slider/carousel_controller.dart'; | ||
| import 'package:flutter/foundation.dart'; | ||
| import 'package:flutter/widgets.dart'; | ||
| import 'package:openfoodfacts/model/Product.dart'; | ||
| import 'package:qr_code_scanner/qr_code_scanner.dart'; | ||
| import 'package:smooth_app/cards/product_cards/smooth_product_card_edit.dart'; | ||
| import 'package:smooth_app/cards/product_cards/smooth_product_card_found.dart'; | ||
| import 'package:smooth_app/cards/product_cards/smooth_product_card_not_found.dart'; | ||
| import 'package:smooth_app/cards/product_cards/smooth_product_card_loading.dart'; | ||
| import 'package:smooth_app/cards/product_cards/smooth_product_card_thanks.dart'; | ||
| import 'package:smooth_app/database/full_products_database.dart'; | ||
|
|
||
| enum ScannedProductState { FOUND, NOT_FOUND, LOADING } | ||
|
|
||
| class ContinuousScanModel extends ChangeNotifier { | ||
| ContinuousScanModel({@required this.contributionMode}) { | ||
| carouselController = CarouselController(); | ||
| import 'package:smooth_app/database/barcode_product_query.dart'; | ||
| import 'package:smooth_app/database/local_database.dart'; | ||
|
|
||
| enum ScannedProductState { | ||
| FOUND, | ||
| NOT_FOUND, | ||
| LOADING, | ||
| THANKS, | ||
| } | ||
|
|
||
| scannedBarcodes = <String, ScannedProductState>{}; | ||
| cardTemplates = <String, Widget>{}; | ||
| class ContinuousScanModel { | ||
| ContinuousScanModel({@required bool contributionMode}) | ||
| : _contributionMode = contributionMode; | ||
|
|
||
| final Map<String, ScannedProductState> _states = | ||
| <String, ScannedProductState>{}; | ||
| final Map<String, Product> _products = <String, Product>{}; | ||
| final List<String> _barcodes = <String>[]; | ||
| bool _contributionMode; | ||
| QRViewController _scannerController; | ||
| String _barcodeTrustCheck; | ||
| LocalDatabase _localDatabase; | ||
|
|
||
| bool get isNotEmpty => getBarcodes().isNotEmpty; | ||
| bool get contributionMode => _contributionMode; | ||
|
|
||
| List<String> getBarcodes() => _barcodes; | ||
|
|
||
| void setBarcodeState( | ||
| final String barcode, | ||
| final ScannedProductState state, | ||
| ) { | ||
| _states[barcode] = state; | ||
| _localDatabase.dummyNotifyListeners(); | ||
| } | ||
|
|
||
| final GlobalKey scannerViewKey = GlobalKey(debugLabel: 'Barcode Scanner'); | ||
| QRViewController scannerController; | ||
| CarouselController carouselController; | ||
|
|
||
| Map<String, ScannedProductState> scannedBarcodes; | ||
| Map<String, Widget> cardTemplates; | ||
|
|
||
| List<Product> foundProducts = <Product>[]; | ||
|
|
||
| String barcodeTrustCheck; | ||
| ScannedProductState getBarcodeState(final String barcode) => _states[barcode]; | ||
|
|
||
| bool contributionMode = false; | ||
| Product getProduct(final String barcode) => _products[barcode]; | ||
|
|
||
| bool firstScan = true; | ||
| void setLocalDatabase(final LocalDatabase localDatabase) => | ||
| _localDatabase = localDatabase; | ||
|
|
||
| void setupScanner(QRViewController controller) { | ||
| scannerController = controller; | ||
| scannerController.scannedDataStream.listen((String barcode) { | ||
| onScan(barcode); | ||
| }); | ||
| _scannerController = controller; | ||
| _scannerController.scannedDataStream.listen( | ||
| (String barcode) => onScan(barcode), | ||
| ); | ||
| } | ||
|
|
||
| void onScan(String code) { | ||
| print('Barcode detected : $code'); | ||
| if (barcodeTrustCheck != code) { | ||
| barcodeTrustCheck = code; | ||
| return; | ||
| } | ||
| if (addBarcode(code)) { | ||
| _generateScannedProductsCardTemplates(); | ||
| if (!firstScan) { | ||
| carouselController.animateToPage( | ||
| cardTemplates.length - 1, | ||
| ); | ||
| } else { | ||
| firstScan = false; | ||
| List<Product> getFoundProducts() { | ||
| final List<Product> result = <Product>[]; | ||
| for (final String barcode in _barcodes) { | ||
| if (getBarcodeState(barcode) == ScannedProductState.FOUND) { | ||
| result.add(_products[barcode]); | ||
| } | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| void onScanAlt(String code, List<Offset> offsets) { | ||
| Future<void> onScan(String code) async { | ||
| print('Barcode detected : $code'); | ||
| if (addBarcode(code)) { | ||
| _generateScannedProductsCardTemplates(); | ||
| if (cardTemplates.isNotEmpty) { | ||
| carouselController.animateToPage( | ||
| cardTemplates.length - 1, | ||
| ); | ||
| } | ||
| if (_barcodeTrustCheck != code) { | ||
| _barcodeTrustCheck = code; | ||
| return; | ||
| } | ||
| _addBarcode(code); | ||
| } | ||
|
|
||
| Future<bool> _generateScannedProductsCardTemplates( | ||
| {bool switchMode = false}) async { | ||
| final FullProductsDatabase productsDatabase = FullProductsDatabase(); | ||
|
|
||
| for (final String scannedBarcode in scannedBarcodes.keys) { | ||
| switch (scannedBarcodes[scannedBarcode]) { | ||
| case ScannedProductState.FOUND: | ||
| if (switchMode) { | ||
| final Product product = await productsDatabase.getProduct( | ||
| scannedBarcode); // Acceptable thanks to offline first | ||
| setCardTemplate( | ||
| scannedBarcode, | ||
| contributionMode | ||
| ? SmoothProductCardEdit( | ||
| heroTag: product.barcode, product: product) | ||
| : SmoothProductCardFound( | ||
| heroTag: product.barcode, product: product)); | ||
| } | ||
| break; | ||
| case ScannedProductState.NOT_FOUND: | ||
| break; | ||
| case ScannedProductState.LOADING: | ||
| final bool result = | ||
| await productsDatabase.checkAndFetchProduct(scannedBarcode); | ||
| if (result) { | ||
| scannedBarcodes[scannedBarcode] = ScannedProductState.FOUND; | ||
| final Product product = | ||
| await productsDatabase.getProduct(scannedBarcode); | ||
| setCardTemplate( | ||
| scannedBarcode, | ||
| contributionMode | ||
| ? SmoothProductCardEdit( | ||
| heroTag: product.barcode, product: product) | ||
| : SmoothProductCardFound( | ||
| heroTag: product.barcode, product: product)); | ||
| foundProducts.add(product); | ||
| } else { | ||
| scannedBarcodes[scannedBarcode] = ScannedProductState.NOT_FOUND; | ||
| setCardTemplate( | ||
| scannedBarcode, | ||
| SmoothProductCardNotFound( | ||
| barcode: scannedBarcode, | ||
| callback: () { | ||
| setCardTemplate(scannedBarcode, SmoothProductCardThanks()); | ||
| }, | ||
| ), | ||
| ); | ||
| } | ||
| break; | ||
| } | ||
| Future<void> onScanAlt(String code, List<Offset> offsets) async { | ||
| print('Barcode detected : $code'); | ||
| _addBarcode(code); | ||
| } | ||
|
|
||
| void contributionModeSwitch(bool value) { | ||
| if (_contributionMode != value) { | ||
| _contributionMode = value; | ||
| _localDatabase.dummyNotifyListeners(); | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| bool addBarcode(String newBarcode) { | ||
| if (scannedBarcodes[newBarcode] == null) { | ||
| scannedBarcodes[newBarcode] = ScannedProductState.LOADING; | ||
| cardTemplates[newBarcode] = SmoothProductCardLoading(barcode: newBarcode); | ||
| notifyListeners(); | ||
| bool _addBarcode(final String barcode) { | ||
| if (getBarcodeState(barcode) == null) { | ||
| _barcodes.add(barcode); | ||
| setBarcodeState(barcode, ScannedProductState.LOADING); | ||
| _loadBarcode(barcode); | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| void setProductState(String barcode, ScannedProductState state) { | ||
| scannedBarcodes[barcode] = state; | ||
| notifyListeners(); | ||
| } | ||
|
|
||
| void setCardTemplate(String barcode, Widget cardTemplate) { | ||
| cardTemplates[barcode] = cardTemplate; | ||
| notifyListeners(); | ||
| } | ||
|
|
||
| void contributionModeSwitch(bool value) { | ||
| contributionMode = value; | ||
| _generateScannedProductsCardTemplates(switchMode: true); | ||
| notifyListeners(); | ||
| Future<void> _loadBarcode(final String barcode) async { | ||
| final Product product = await BarcodeProductQuery(barcode).getProduct(); | ||
| if (product != null) { | ||
| _localDatabase.putProduct(product); | ||
| _products[barcode] = product; | ||
| setBarcodeState(barcode, ScannedProductState.FOUND); | ||
| } else { | ||
| setBarcodeState(barcode, ScannedProductState.NOT_FOUND); | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Is there a valid reason to get rid of the
ChangeNotifier?We are using Provider as State Management and the
StatefulWidgetsshould be avoided (Now that I think about it I shouldn't have accepted the new Profile page as it doesn't follow this guideline).Provider works by separating the UI from the logic, every Widget can be stateless and the class that extends
ChangeNotifierholds all the variables, calling notifyListener() tells the Widget to rebuild.Sorry that this wasn't specified anywhere on the project, I will update the guidelines.
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 understand your point about the
ChangeNotifiervs.StatefulWidget.That being said, the code before I refactored it was really painful to read, with
Consumers here and there.My idea is to have most of the relevant data in the
Providers that are already in themain.dart- in that case we could notify each time the scan list is updated as a product list stored in the database, for instance.Interesting point, to be investigated.
Is there a link for the guidelines?
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 agree that the use of
Consumersis quite heavy but I also fear that mixingChangeNotifiersandStatefulWidgetscan become confusing. Perhaps we should determine a clear rule on whenProviderorStatefulWidgetsshould be used.Having the data at a higher levels such as
main.dartsounds good to me, I don't really see how that will reduce the use ofConsumersthough.There is actually no guidelines at the moment, only the technical document you already saw for the choice of database. I will start working on guidelines and documentation for future contributors.
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.
That's the beauty of
Providers: if I start mybuild(BuildContext context)with acontext.watch<LocalDatabase>(), every time the notify listener method is called onLocalDatabase, theWidgetis rebuilt. And I don't need anyConsumer.Don't put too much effort in the guidelines for the moment; I think we're still in an early stage, with many open questions. I guess a simple
READMEwould do the job in first approach.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.
Oh indeed that's really clever!
Should we split this PR by keeping only the database part, or do you think you can update it to support this new data structure?
I will take some note and make a small file with the essential guidelines
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 did not want to change the screens in the first place, but I had too, because of the impact of the change of database.
If you give me a couple of days I think I can do something nice with
ContinuousScanPage, in this PR. That will give us food for thought, and perhaps ideas of templates / for your guidelines.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 understand, that would be great. Once we have this page working we can transition the rest of the app to use the same principle and the document it (or probably the other way around).
I will wait for your update and we can discuss it then, let me know if you need any help.