openfoodfacts / smooth-app

🤳🥫 The new Open Food Facts mobile application for Android and iOS, crafted with Flutter and Dart
https://world.openfoodfacts.org/open-food-facts-mobile-app?utm_source=off&utf_medium=web&utm_campaign=github-repo
Apache License 2.0
861 stars 286 forks source link

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

Closed monsieurtanuki closed 2 weeks ago

monsieurtanuki commented 3 weeks ago

What

Screenshots

3168930156987 16811207
Screenshot_1730485277 Screenshot_1730484206
Screenshot_1730483661 Screenshot_1730484189

Fixes bug(s)

Impacted files

codecov-commenter commented 3 weeks ago

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 :warning:
...h_app/lib/pages/product/nutrition_page_loaded.dart 0.00% 24 Missing :warning:
...smooth_app/lib/pages/product/owner_field_info.dart 11.11% 8 Missing :warning:
...ib/generic_lib/widgets/smooth_text_form_field.dart 71.42% 2 Missing :warning:
...ib/pages/input/smooth_autocomplete_text_field.dart 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

monsieurtanuki commented 3 weeks ago

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 commented 3 weeks ago

sounds reasonable

monsieurtanuki commented 3 weeks ago

@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 commented 3 weeks ago

cc @stephanegigandet

monsieurtanuki commented 3 weeks ago

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.

monsieurtanuki commented 2 weeks ago

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.

monsieurtanuki commented 2 weeks ago

Thank you @teolemon for your review!

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