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

Packagings weights not recorded if using a comma (6,5) or a unit (e.g. 6.9 g) is included (mobile) #3663

Closed
stephanegigandet opened this issue Jan 31, 2023 · 10 comments · Fixed by #3676
Assignees
Labels
🐛 bug Something isn't working ✏️ Editing - Packaging input Related to the structured input of food packaging.

Comments

@stephanegigandet
Copy link
Contributor

If a packaging weight is entered with a unit (e.g. "9.6 g"), then the value is not recorded at all.

@stephanegigandet
Copy link
Contributor Author

I will also change the product edit form on the web, and the API, to accept strings with units: openfoodfacts/openfoodfacts-server#8054

@stephanegigandet stephanegigandet changed the title Packagings weights not recorded if unit (e.g. g) is included (mobile) Packagings weights not recorded if unit (e.g. 6.9 g) is included (mobile) Jan 31, 2023
@teolemon teolemon added the ✏️ Editing - Packaging input Related to the structured input of food packaging. label Jan 31, 2023
@monsieurtanuki
Copy link
Contributor

@stephanegigandet There are pending issues in Smoothie on the packaging screen as we presented numeric fields as text fields, and did not fully check if they were really numeric (we just replaced with null if not a number).

That's currently the case for

  • int? numberOfUnits - Number of units of this component contained in the product.
  • double? weightMeasured - Weight in grams as measured by a user of one unit of the empty component.

If I understand you, that means we'll switch to the following types, am I right?

  • int? numberOfUnits - no change here
  • String? weightMeasured - now it's a String

@monsieurtanuki
Copy link
Contributor

@stephanegigandet Please ping me when it's done on the server side (I've just checked, it's not).

@monsieurtanuki monsieurtanuki self-assigned this Jan 31, 2023
@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki on the server side, both number_of_units and weight_measured will still be numbers, but if the WRITE API is passed a string, the API will convert them to a number (and send back a warning and an indication of what the value was converted to). For the READ API, both will be numbers.

I'm thinking the easiest would be for the app to send back a string. That way you don't have to deal with conversions (e.g. numbers with a dot or comma, all types of units like kg, mg etc.).

Corresponding PR server side: openfoodfacts/openfoodfacts-server#8056 (in review, not deployed yet)

@monsieurtanuki
Copy link
Contributor

@stephanegigandet Correct me if I'm wrong: I would write '1kg' and I would read later 1000?
Maybe a bit disturbing for the user. And in Smoothie we have no way to manage/display the returned warnings.

Honestly I would prefer a clean warning for the user at submit time: a red hint saying "Error: please enter a value in grams". And anyway we have to deal with dot/comma in nutrition facts too.

What about adding a g/mg/µg button, at least on the UI? Here I'm talking about Smoothie, that would compute locally the value in grams from a numeric text field and the unit button. And as you guys won't get/store the unit, we would display the "best" unit.

I don't know...

@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki The API will convert '1kg' to 1000.

If you prefer to do the conversions client side, no problem at all. But we need to make sure that values like '1.4', '1,4', '1.4g', "1,4 g' etc. are converted to 1.4. And maybe show a warning if it wasn't possible to convert it.

There's no need to do all the conversions for mg and kg etc. We do tell users to enter the weight in grams (the title of the field is "Weight of an empty unit (g)".

The UI is already very complex for packagings, so I don't think it's a good idea to show a unit button.

@monsieurtanuki
Copy link
Contributor

@stephanegigandet OK for not adding a unit button to the packaging screen.
As I said before, we should deal with incorrect numeric fields directly on the screen at submit / field focus time, not in the background tasks.
In Smoothie at least, we would be better off with a numeric keyboard, cf. #3507. We ask for grams, period.

@stephanegigandet stephanegigandet changed the title Packagings weights not recorded if unit (e.g. 6.9 g) is included (mobile) Packagings weights not recorded if using a comma (6,5) or a unit (e.g. 6.9 g) is included (mobile) Feb 6, 2023
@stephanegigandet
Copy link
Contributor Author

Hi @monsieurtanuki , can we fix this particular issue quickly, either with a numeric keyboard, or by converting the string to a valid number? (removing the g unit, and changing comma to dot). Some contributors spend a lot of time to enter weights, and their entries are discarded. I just witnessed someone do it yesterday: she used commas (6,5) when entering weights, and it wasn't saved.

@monsieurtanuki
Copy link
Contributor

Hi @stephanegigandet!
I'll fix it today with an ugly code: same alphanumeric keyboard, just keeping numeric and comma/dots.
As a temporary quick fix.

@stephanegigandet
Copy link
Contributor Author

Hi @stephanegigandet! I'll fix it today with an ugly code: same alphanumeric keyboard, just keeping numeric and comma/dots. As a temporary quick fix.

Great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working ✏️ Editing - Packaging input Related to the structured input of food packaging.
Development

Successfully merging a pull request may close this issue.

3 participants