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

feat: 5430 - "producer provided" icon for nutrients and 4 product fields #5777

Merged
merged 17 commits into from
Nov 11, 2024

Conversation

monsieurtanuki
Copy link
Contributor

What

  • Adds an icon for each field or nutrient that was provided by the producer.
  • So far, the impacted field list is product name, brands, quantity and serving size.
  • We can add the icon to other fields, but even on the website it doesn't seem to be systematic (e.g. category for 7300400481588 is "producer provided" but that doesn't show on the website)

Screenshots

3168930156987 16811207
Screenshot_1730485277 Screenshot_1730484206
Screenshot_1730483661 Screenshot_1730484189

Fixes bug(s)

Impacted files

  • add_basic_details_page.dart: display "producer provided" icon for name, brands and quantity
  • nutrition_page_loaded.dart: display "producer provided" icon for serving size and each individual nutrient
  • product_query.dart: set the icon to display when a product field value is "producer provided"
  • smooth_autocomplete_text_field.dart: added parameter suffixIcon
  • smooth_text_form_field.dart: added parameter suffixIcon

Impacted files:
* `add_basic_details_page.dart`: display "producer provided" icon for name, brands and quantity
* `nutrition_page_loaded.dart`: display "producer provided" icon for serving size and each individual nutrient
* `product_query.dart`: set the icon to display when a product field value is "producer provided"
* `smooth_autocomplete_text_field.dart`: added parameter `suffixIcon`
* `smooth_text_form_field.dart`: added parameter `suffixIcon`
@codecov-commenter
Copy link

codecov-commenter commented Nov 1, 2024

Codecov Report

Attention: Patch coverage is 9.09091% with 60 lines in your changes missing coverage. Please review.

Project coverage is 7.02%. Comparing base (4d9c7fc) to head (624f833).
Report is 464 commits behind head on develop.

Files with missing lines Patch % Lines
..._app/lib/pages/product/add_basic_details_page.dart 0.00% 25 Missing ⚠️
...h_app/lib/pages/product/nutrition_page_loaded.dart 0.00% 24 Missing ⚠️
...smooth_app/lib/pages/product/owner_field_info.dart 11.11% 8 Missing ⚠️
...ib/generic_lib/widgets/smooth_text_form_field.dart 71.42% 2 Missing ⚠️
...ib/pages/input/smooth_autocomplete_text_field.dart 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           develop   #5777      +/-   ##
==========================================
- Coverage     9.54%   7.02%   -2.53%     
==========================================
  Files          325     415      +90     
  Lines        16411   22752    +6341     
==========================================
+ Hits          1567    1599      +32     
- Misses       14844   21153    +6309     

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

Copy link
Member

@teolemon teolemon left a comment

Choose a reason for hiding this comment

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

We should probably make the icon clickable with a possibility to understand what it's about and possibly to complain or email/file a ticket on Nutri-Patrol if they believe there's a data mistake.

@monsieurtanuki
Copy link
Contributor Author

We should probably make the icon clickable with a possibility to understand what it's about and possibly to complain or email/file a ticket on Nutri-Patrol if they believe there's a data mistake.

@teolemon Makes almost sense.
In the current PR I added a suffix icon to the text field - this UI option is supported by flutter.
Displaying a separate icon would break a little the display, why not.
But that would look preposterous for the nutrition facts page: an additional icon button on each already crowded nutrient line that always says the same thing.

What could be done instead from the current PR is adding an expandable line (e.g. a ListTile) with the same icon and a "Producer provided - what does it mean" text + link.
And we can display this line only once per page and only for pages that do have at least one producer provided data.
What do you think of that?

@teolemon
Copy link
Member

teolemon commented Nov 2, 2024

sounds reasonable

New file:
* `owner_field_info.dart`: Standard info tile about "owner fields".

Impacted files:
* `add_basic_details_page.dart`: now displaying `OwnerFieldInfo` if relevant; minor refactoring
* `nutrition_page_loaded.dart`: now displaying `OwnerFieldInfo` if relevant; minor refactoring
* `product_query.dart`: moved field to new file `owner_field_info.dart`
@monsieurtanuki
Copy link
Contributor Author

@teolemon Just added a standard info tile about "owner fields" when relevant. Should that be clickable? Is the text explicit enough?

3168930156987 16811207
Screenshot_1730661553 Screenshot_1730661586
Screenshot_1730661533 Screenshot_1730661579

@teolemon
Copy link
Member

teolemon commented Nov 4, 2024

cc @stephanegigandet

@monsieurtanuki
Copy link
Contributor Author

Can we have the same mechanism for Producer provided photos ? do we have the necessary info ?

Doesn't look like we have that kind of information in owner fields: https://fr.openfoodfacts.org/api/v3/product/7300400481588?fields=owner_fields
Any product you're thinking of?

Should we have a link to mailto:producers@openfoodfacts.org or a nutripatrol report link https://nutripatrol.openfoodfacts.org/flag/product?barcode=3017620422003&source=web&flavor=off ?

With the current solution we don't have too much space. A click on the info tile could open a dialog or even a page with more explanations. And a button to that email and another to that report link.

Copy link
Collaborator

@g123k g123k left a comment

Choose a reason for hiding this comment

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

Generally speaking, seems OK for me.
Could you add some Semantics for screen readers?

@monsieurtanuki
Copy link
Contributor Author

Generally speaking, seems OK for me. Could you add some Semantics for screen readers?

Sure, I'll do that the next time I code for this PR.

Copy link
Member

@teolemon teolemon left a comment

Choose a reason for hiding this comment

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

@monsieurtanuki thanks for this addition

@monsieurtanuki
Copy link
Contributor Author

Thank you @teolemon for your review!

I just have to add a Semantics and we're OK.

Impacted files:
* add_basic_details_page.dart: added semantics
* app_en.arb: 2 new labels for "owner field info"
* nutrition_page_loaded.dart: added semantics
* owner_field_info.dart: translated labels
@monsieurtanuki monsieurtanuki merged commit 45b7e7b into openfoodfacts:develop Nov 11, 2024
6 checks passed
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.

Add support for showing producer provided and protected fields
4 participants