Skip to content
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/#21 - scan now uses local database, for product look up and for its history #95

Merged
merged 1 commit into from
Jan 17, 2021

Conversation

monsieurtanuki
Copy link
Contributor

New file:

  • scan_page.dart: helper class to be used on top of AlternativeContinuousScanPage and ContinuousScanPage

Impacted files:

  • alternative_continuous_scan_page.dart: ContinuousScanModel is now initialized upstream by ScanPage
  • continuous_scan_model.dart: now a ChangeNotifier, uses a ProductList and database cache
  • continuous_scan_page.dart: ContinuousScanModel is now initialized upstream by ScanPage
  • contribution_page.dart: uses the new ScanPage widget instead of the 2 scan pages
  • local_database.dart: removed the dummy notify listener method
  • main.dart: uses the new ScanPage widget instead of the 2 scan pages
  • personalized_ranking_page.dart: now we use a ProductList as input
  • product_list.dart: added a getProduct method and a "scan list" identifier
  • product_page.dart: unrelated UI fixes
  • product_query_page.dart: slight refactoring
  • smooth_it_model.dart: now we use ProductList
  • smooth_product_carousel.dart: added the new "CACHED" product possibility; fixed an init bug

…or its history

New file:
* `scan_page.dart`: helper class to be used on top of `AlternativeContinuousScanPage` and `ContinuousScanPage`

Impacted files:
* `alternative_continuous_scan_page.dart`: `ContinuousScanModel` is now initialized upstream by `ScanPage`
* `continuous_scan_model.dart`: now a `ChangeNotifier`, uses a `ProductList` and database cache
* `continuous_scan_page.dart`: `ContinuousScanModel` is now initialized upstream by `ScanPage`
* `contribution_page.dart`: uses the new `ScanPage` widget instead of the 2 scan pages
* `local_database.dart`: removed the dummy notify listener method
* `main.dart`: uses the new `ScanPage` widget instead of the 2 scan pages
* `personalized_ranking_page.dart`: now we use a `ProductList` as input
* `product_list.dart`: added a `getProduct` method and a "scan list" identifier
* `product_page.dart`: unrelated UI fixes
* `product_query_page.dart`: slight refactoring
* `smooth_it_model.dart`: now we use `ProductList`
* `smooth_product_carousel.dart`: added the new "CACHED" product possibility; fixed an init bug
@M123-dev
Copy link
Member

I'll take a closer look at it later, just one thing on the side. We are still using qr_code_scanner ^0.0.13 but the newest is 0.3.0. The only big change is minSdk 20 and that the scanner delivers an object and not a string. The string can be obtained again with object.code. I think it would be good if you change this, since you're on the subject right now.

@monsieurtanuki
Copy link
Contributor Author

@M123-dev I don't know the general target for Android minSdk on this app, @stephanegigandet perhaps?

I'm a bit concerned for 2 reasons:

  • I prefer the lowest possible API level in general
  • my smartphone runs on API 21 - it's getting closer...

For the record:

  • API 19: 98.1%
  • API 20: 94.1%
  • API 21: 94.1%
  • API 22: 92.3%

Therefore I won't upgrade to qr_code_scanner ^0.3.0 in this PR, though I don't feel "furiously angry" with the concept of upgrading from API 19 to API 20.

@M123-dev
Copy link
Member

M123-dev commented Jan 16, 2021

The app currently has minSdk version 19 and yes, I agree with you, I would definitely keep the version as deep as possible. Unfortunately, I couldn't find any correct information in the changelogs why and when it was upgraded. Only in version 0.1.0 it was reduced from 24 to 21.
I have to say I am not satisfied with the scanners anyway, so I hoped that the update would fix this, has the choice already been made or is the final scanner still being searched?

Copy link
Member

@M123-dev M123-dev left a comment

Choose a reason for hiding this comment

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

Looks good

@monsieurtanuki monsieurtanuki merged commit 8379bb9 into openfoodfacts:master Jan 17, 2021
@monsieurtanuki
Copy link
Contributor Author

Thank you @M123-dev!

@PrimaelQuemerais I've just merged anyway, but I think I found something interesting about the "local provider" / "async init" questions: please have a look at scan_page.dart and its build method.

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.

2 participants