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

fix: 5570 - no nutriscore/ecoscore for non-food products #5629

Merged
merged 3 commits into from
Sep 27, 2024

Conversation

monsieurtanuki
Copy link
Contributor

What

  • Removed the "tap to answer 3 questions" button for non-food products.
  • Regarding the product card, not much needed to be done, as we displayed the ecoscore and nutriscore widgets if available, and there are not related attributes for non-food products. Minor padding fix, though.

Screenshots

Sorry about the screenshot sizes, I happened to be working on a tablet emulator.

"tap to answer" button on top removed that button
Screenshot_1727359546 Screenshot_1727359556

Fake product names for real OxF examples: only the food product has ecoscore and nutriscore
Screenshot_1727360274

Fixes bug(s)

Impacted files

  • product_incomplete_card.dart: minor padding fix
  • smooth_product_card_found.dart: no display of "incomplete product button" for non-food products

Impacted files:
* `product_incomplete_card.dart`: minor padding fix
* `smooth_product_card_found.dart`: no display of "incomplete product button" for non-food products
@monsieurtanuki
Copy link
Contributor Author

Because scanner_shared depends on openfoodfacts_flutter_lints from git which requires SDK version ^3.5.3, version solving failed.

@g123k I guess merging #5613 is getting urgent, or we cannot PR anything anymore.

@codecov-commenter
Copy link

codecov-commenter commented Sep 26, 2024

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 6.89%. Comparing base (4d9c7fc) to head (9b5427d).
Report is 332 commits behind head on develop.

Files with missing lines Patch % Lines
...cards/product_cards/smooth_product_card_found.dart 0.00% 3 Missing ⚠️
...app/lib/pages/product/product_incomplete_card.dart 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           develop   #5629      +/-   ##
==========================================
- Coverage     9.54%   6.89%   -2.66%     
==========================================
  Files          325     402      +77     
  Lines        16411   21263    +4852     
==========================================
- Hits          1567    1466     -101     
- Misses       14844   19797    +4953     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@monsieurtanuki
Copy link
Contributor Author

Because scanner_shared depends on openfoodfacts_flutter_lints from git which requires SDK version ^3.5.3, version solving failed.

@g123k I guess merging #5613 is getting urgent, or we cannot PR anything anymore.

Thank you @g123k for your quick action!

@@ -21,6 +21,10 @@ class ProductIncompleteCard extends StatelessWidget {
final bool isLoggedInMandatory;

static bool isProductIncomplete(final Product product) {
if (product.productType != null &&
Copy link
Member

Choose a reason for hiding this comment

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

the name doesn't completely reflect what's actually going on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the name doesn't completely reflect what's actually going on

You're right. The thing is that we display the "hey your product is incomplete" message only when the product is incomplete, for food products, therefore isProductIncomplete made sense.
I've just added an "if not food, consider as complete" test.
I can rename the method to isIncompleteFoodProduct: would that be OK with you?

@monsieurtanuki monsieurtanuki merged commit 50a64f8 into openfoodfacts:develop Sep 27, 2024
6 checks passed
@monsieurtanuki
Copy link
Contributor Author

Thank you @teolemon for your review!

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

Successfully merging this pull request may close these issues.

Disable Eco-Score and Nutri-Score display in Search results / ListView if project != off
3 participants