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: #511 - nutrition page minor improvements #907

Merged
merged 14 commits into from
Jan 9, 2022

Conversation

monsieurtanuki
Copy link
Contributor

Impacted file:

  • nutrition_page_loaded.dart
    • explicit labels for units
    • removed "energy" line and computed "energy-kj" and "energy-kcal" accordingly
    • unit conversion (e.g. x 1000 for mg from g)
    • new small default nutrient unit repository

@monsieurtanuki monsieurtanuki merged commit 4da655d into openfoodfacts:develop Jan 9, 2022
@teolemon
Copy link
Member

teolemon commented Jan 9, 2022

  • nb: kj and kcal should not be computed from one another: one field for each
  • also, unsure why we need mg / g conversions ? we probably shouldn't try to autoupdate fields on unit change. at the end of the day, the user is responsible for his/her input

@monsieurtanuki
Copy link
Contributor Author

nb: kj and kcal should not be computed from one another: one field for each

There's no connection between kj and kcal. It's just that sometimes energy-kj was empty but energy was not and energy-unit was kj => instead of displaying an empty energy-kj, I populate it with energy.

also, unsure why we need mg / g conversions ?

If we don't do that, the values are wrong for mg and µg. Just test on a product with vitamin b12.

we probably shouldn't try to autoupdate fields on unit change.

The conversion is just at init time.
There will be another conversion the other way around when saving the nutrients: it looks like we always store the values in grams, and the unit is just there for a better display.

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

Successfully merging this pull request may close these issues.

2 participants