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

Add the product main language to the details page (write) #4771

Open
monsieurtanuki opened this issue Nov 6, 2023 · 38 comments
Open

Add the product main language to the details page (write) #4771

monsieurtanuki opened this issue Nov 6, 2023 · 38 comments
Assignees
Labels
✨ enhancement New feature or request ⭐ top issue Top issue.

Comments

@monsieurtanuki
Copy link
Contributor

Problem

Today it's not possible in the app to see or set the product main language (LANGUAGE / lang).

Proposed solution

Add the product main language to the details page (write).

Additional context

May explain #4672

@monsieurtanuki monsieurtanuki added ✨ enhancement New feature or request good first issue Good for newcomers labels Nov 6, 2023
@Azad99-9
Copy link

Azad99-9 commented Nov 8, 2023

@monsieurtanuki i would like to work on this issue, please assign it to me.

I have gone through #4672 as well, it would be more helpful if you could brief me a little bit more about this issue.

@monsieurtanuki
Copy link
Contributor Author

Hi @Azad99-9!
Let's focus on the current issue.
After all I think the "main language" should be added to the "other details" page (cf. "website" field).
Please post a screenshot here before any PR, just to check if you are in the right direction.

@Azad99-9
Copy link

Azad99-9 commented Nov 9, 2023

main_language_bug

@monsieurtanuki
Shall i add the main language option in this page after the website field.

@monsieurtanuki
Copy link
Contributor Author

@monsieurtanuki Shall i add the main language option in this page after the website field.

@Azad99-9 Yes, something like that.

@teolemon teolemon changed the title Add the product main language Add the product main language to the details page (write) Nov 13, 2023
@Azad99-9
Copy link

main_language_done
@monsieurtanuki added the product specific language selector in the other details section and this is what it looks like.
any followups?

@monsieurtanuki
Copy link
Contributor Author

@Azad99-9 Please consider that:

  • The language can be null and I don't see how it would look with your UI
  • The label is definitely incorrect - it should rather be "Main product language"

@Azad99-9
Copy link

@monsieurtanuki just need a little more clarity, All user interface elements within the specific product range should be displayed in the main language of the product ?

@monsieurtanuki
Copy link
Contributor Author

@Azad99-9 Not really sure what you mean but:

  • there is one single field in a product, called lang, a potentially null language, that contains what we call the product "main language", and this is what the issue is about - making it visible and editable
  • beyond that, there are localized fields that are Map<Language, String>, but this is not what the issue is about

@Azad99-9
Copy link

Yeah that was comprehensive, go it.👍

@Azad99-9
Copy link

Azad99-9 commented Nov 18, 2023

@Azad99-9 Please consider that:

  • The language can be null and I don't see how it would look with your UI
  • The label is definitely incorrect - it should rather be "Main product language"

@monsieurtanuki

  1. I've implemented a solution to address null values by setting English as the default language whenever the value is missing or null in the field product.lang.
  2. Additionally, I've introduced a new field named "product_main_language_title" across all "arb" files, which corresponds to the string "Main product language".

Shall i raise the pull request?

@monsieurtanuki
Copy link
Contributor Author

I've implemented a solution to address null values by setting English as the default language whenever the value is missing or null in the field product.lang.

English is NOT the default. null is the default. Please imagine a solution in the language selector where the language can be null, perhaps with an additional label like "not specified".

Additionally, I've introduced a new field named "product_main_language_title" across all "arb" files

Please just edit the file that you change, in this case probably ONLY app_en.arb.

which corresponds to the string "Main product language".

That's good.

@Azad99-9
Copy link

@monsieurtanuki When a new product is being created, the language selector is automatically setting the 'product main language' field to English by default. This setup indicates that the product main language field is always assigned a value and technically cannot be null, as the language selector widget lacks an option corresponding to null.

Would it be advisable to modify the language selector widget to include an additional option, such as 'not-specified,' allowing the 'product main language' field to accommodate null values, as previously suggested?.

@monsieurtanuki
Copy link
Contributor Author

When a new product is being created, the language selector is automatically setting the 'product main language' field to English by default.

Only if that's the way you coded it. lang can be null and we have to deal with this case. Actually it's probably populated by default with the lc parameter of the first saveProduct.

This setup indicates that the product main language field is always assigned a value and technically cannot be null.

Yes it technically can: String? lang.

Would it be advisable to modify the language selector widget to include an additional option, such as 'not-specified,' allowing the 'product main language' field to accommodate null values, as previously suggested?.

That's exactly what I suggested in my previous comment: "Please imagine a solution in the language selector where the language can be null, perhaps with an additional label like "not specified"."

@Azad99-9
Copy link

@monsieurtanuki may i know what is the difference between "product query language" and product.lang.

@monsieurtanuki
Copy link
Contributor Author

@Azad99-9 In this issue it's "product MAIN language" and lang.
For instance, you can query in Dutch a product in Belgium whose main language is French.

@abdullah-khudher
Copy link
Contributor

abdullah-khudher commented Dec 22, 2023

Is there any update on this issue @Azad99-9?

@Azad99-9
Copy link

Kindly unassign me this issue.

@abdullah-khudher
Copy link
Contributor

abdullah-khudher commented Dec 24, 2023

@monsieurtanuki There is a product main language dropdown on the details page to choose the language when trying to add a new product.

Do you mean in some other place?

WhatsApp Image 2023-12-25 at 12 38 53 AM

WhatsApp Image 2023-12-25 at 12 42 15 AM

@monsieurtanuki
Copy link
Contributor Author

@abdullah-khudher Again, there are localized fields. For each of them, there is a language selector. For instance, the name of the product is X in English and Y in French. Like in your screenshots.

AND, there's a field that contains the product MAIN language. That's what the issue is about.

@M123-dev
Copy link
Member

As far as I understood it (Correct me if I'm wrong): Say there is a product sold in Germany

[Example Brand] Soyamilch

We can have its name in English:

[Example brand] Soy milk

But we can specify the main language of a product, which in this case would be German, since that's what's on the product.

That's what the lang field in the product is for

@monsieurtanuki
Copy link
Contributor Author

@M123-dev That's correct!

@monsieurtanuki
Copy link
Contributor Author

@abdullah-khudher Still working on it?

@panicoli0
Copy link

Can I take that issue?

@monsieurtanuki
Copy link
Contributor Author

Hi @panicoli0!
You can see in that thread what needs to be done regarding UI and code.
Don't hesitate to ask additional questions before sending a PR.

@panicoli0
Copy link

Hey @monsieurtanuki
I think that I got it, let me make it more visual just for clarification purposes.
So I created this mockup:
image

The idea is to have the capability to define the product's main language right?

@monsieurtanuki
Copy link
Contributor Author

The idea is to have the capability to define the product's main language right?

@panicoli0 Indeed it is.
But I suggest you code in the "additional details" page (where "website" is) because:

  • two language selectors in the same page is source of confusion for users and developers
  • the "main language" field is rather a minor field and there's no reason to put it in the "basic details" page

@teolemon
Copy link
Member

The proper wording is "Main product language"

@panicoli0
Copy link

Agreed!
This is how is going to look:
image

Feel free to share your thoughts

@monsieurtanuki
Copy link
Contributor Author

@panicoli0 Not a big fan of the "Main product language" looking like a chapter title but that's the right page, that will do for the moment.

Please note that this is the only language selector case where the language may be null:

  • you'll have to deal with that
  • without much impact to the rest of the app

@panicoli0
Copy link

panicoli0 commented Mar 18, 2024

I just spotted that product.lang is not editable and is always coming as default with a value of ENGLISH.
I tried to save the value the same as we did for the website without success.

/// Returns a [Product] with the values from the text fields.
  Product? _getMinimalistProduct() {
    // Product()
    //   ..barcode = _product.barcode
    //   ..website = _websiteController.text;

    Product? result;
    Product getBasicProduct() => Product()
      ..barcode = _product.barcode
      ..lang = _multilingualHelper.getCurrentLanguage()
      ..website = _websiteController.text;

    if (_websiteController.isDifferentFromInitialValue) {
      result ??= getBasicProduct();
      result.website = _websiteController.text;
    }
    _multilangualHelperControllerWithHistory.text =
        _multilingualHelper.getCurrentLanguage().toString();
    if (_multilangualHelperControllerWithHistory.isDifferentFromInitialValue) {
      result ??= getBasicProduct();
      result.lang = _multilingualHelper.getCurrentLanguage();
    }
    return result;
  }

something from outside is resetting the value to ENGLISH.
Any clue about it?

@monsieurtanuki
Copy link
Contributor Author

Hi @panicoli0!
There's a lot of tap dancing in your code, and that's not a good sign.
Please don't call the language selector _multilingualHelper because in the rest of the app it means different labels for different languages. What about _mainLanguageHelper?
Your code does not change the main language on the server because you haven't coded it. You need to include lang in UpToDateProvider (not 100% sure about the name) so that it's copied to the product changes sent to the server.
There may be another side-effect to fix after that, but we'll see that later.

@panicoli0
Copy link

panicoli0 commented Mar 18, 2024

Hey @monsieurtanuki
Agree with your suggestion I will use _mainLanguageHelper
Just for testing purposes, I tried changing the website field, saving and loading and it works!
The problem is with the _product.lang. I don't know why is not being saved the same as the website (Despite that website(string) and lang(OpenFoodFactsLanguage).

here is the code that I'm using to store the product.lang:
image

Any chance to have a quick call between us? I can share the branch with you.

@M123-dev
Copy link
Member

M123-dev commented Mar 19, 2024

I just checked it inside the SDK. It looks like the lang field is read only, or that we can't set it manually. See openfoodfacts/openfoodfacts-dart#894

Do you have an other idea how to set it monsieurtanuki, otherwise we would have to ask Stéphane.


Unrelated: @panicoli0 I'd recommend posting your email in a harder to read format like name [at] test dot com so that scrapers are having a hard time putting you on some email lists.

@monsieurtanuki
Copy link
Contributor Author

Hi @panicoli0! Unfortunately I cannot dedicate that much time to that issue. In addition to that, as @M123-dev noted, for some products we cannot change the main language.
Maybe it's not a "good first issue" after all :(

@panicoli0
Copy link

hey @monsieurtanuki and @M123-dev
what about to add a new field, like 'main_product_language' ?

@monsieurtanuki monsieurtanuki removed the good first issue Good for newcomers label Apr 1, 2024
@M123-dev
Copy link
Member

M123-dev commented Jun 7, 2024

Yes @panicoli0 but that would have to be done in the server.

@stephanegigandet is there a way to set the main product language, the lang field

@stephanegigandet
Copy link
Contributor

@M123-dev the "lang" field is not read only on the server side

@monsieurtanuki
Copy link
Contributor Author

I would have a look at it but I have issues with higher priorities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request ⭐ top issue Top issue.
Projects
Status: 💬 To discuss and validate
Development

No branches or pull requests

7 participants