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

fix: Update logo of the label V-label #8247

Closed
wants to merge 1 commit into from

Conversation

thestarsahil
Copy link

2 logos for the vegetarian label :

Végétarien → French
Vegetarian → English

What

Screenshot

Related issue(s) and discussion

  • Fixes #[ISSUE NUMBER]

@thestarsahil thestarsahil requested a review from a team as a code owner March 24, 2023 16:01
@github-advanced-security
Copy link

You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

@stephanegigandet stephanegigandet changed the title Update logo of the label V-label fix: Update logo of the label V-label Mar 27, 2023
@alexgarel
Copy link
Member

Hi @thestarsahil thank you for your PR.

Did you tested it ?

I don't think it will work, as stated in the issue:

they have to be in phase with the name of the label in labels taxonomy + the size (see other logos in this folder)

By the way, if after suceeding you want to add a "how to add a label image" in the docs/ folder, you are more than welcome :-)

@thestarsahil
Copy link
Author

@alexgarel yes i tested its working on and Sure, I'll add a guide on how to add a label image to the docs folder after successfully completing the task. Thank you for the suggestion!

@thestarsahil
Copy link
Author

@alexgarel Can you please review this pull request?

2 logos for the vegetarian label :

Végétarien → French
Vegetarian → English
@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@benbenben2
Copy link
Collaborator

Hi @thestarsahil.

Few comments from my side, I hope it will help.

You added the images in "html/images/lang/hr/" where "hr" is for Croatian.
The images would be more suitable in "en" (English) and "fr" (French), respectively (but see last comment below).
Don't forget to rename the images according to the convention (name-of-the-label.[width]x90.png)
And it seems that you did not change the taxonomies/labels.txt file at all to point to the path of the image.

Here are some references that may guide you:

wiki doc:
https://wiki.openfoodfacts.org/Global_labels_taxonomy
https://wiki.openfoodfacts.org/Global_labels_taxonomy_logos

other doc:
https://support.openfoodfacts.org/help/en-gb/15-improving-open-food-facts-in-my-language-country/55-i-would-like-to-add-a-new-logo-for-labels

Here is an example how-to-add-labels:
https://github.com/openfoodfacts/openfoodfacts-server/pull/7920/files
#7957

Remark that there is already a logo for european-vegetarian-union: https://world.openfoodfacts.dev/label/european-vegetarian-union
It is maybe not needed to add more. Ping @alexgarel and @stephanegigandet for advice there.

@benbenben2
Copy link
Collaborator

Meanwhile, I just realized that your PR @thestarsahil is related to this issue: Fixes #8214.

So, we can ignore my last remark :)

You will have to (replace <> by appropriate number and file extension everywhere):

Curiously, for French, it is now showing the English version: https://fr.openfoodfacts.dev/label/european-vegetarian-union-vegetarian
But the file is there: html/images/lang/fr/labels/european-vegetarian-union-vegetarian.90.90.svg
But the file is named in English.
I believe that the file should be named like the French label is named in the taxonomy. That would be:
html/images/lang/en/labels/union-vegetarienne-europeenne.<>.90.<>

That is how others labels seems to be done, see for example with PDO and AOP:
https://world.openfoodfacts.dev/label/pdo
https://fr.openfoodfacts.dev/label/aop

You can test it during your dev.

@teolemon teolemon added the labels label Jun 8, 2023
@alexgarel
Copy link
Member

@thestarsahil do you think you will have time to continue this PR ?

@thestarsahil
Copy link
Author

@alexgarel yes I can

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.

4 participants