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: refactor Eco-Score knowledge panels + accordion display on web #5841

Merged
merged 2 commits into from
Oct 7, 2021

Conversation

stephanegigandet
Copy link
Contributor

  • Small change to the knowledge panel API : the title / subtitle / icon_url properties are now in a title_element (discussed with @jasmeet0817 )
  • Improved a bit the Eco-Score panel (the panel that will be shown when we click on Eco-Score, to explain the computation). Still not complete though.
  • Changed the template to display knowledge panels on the Web to use the accordion provided by Foundation: by default we only show the title element of the panel, and users can click to see more. In the app, the behaviour will probably be different (tapping on the title will open a new page with the details).

To be added soon:

  • "expanded" property to indicate if a panel should be displayed already open, with all its content elements visible.
  • "name" properties to be displayed above panel title (as in Tim's mocks)

To test:

Panels API result: https://uk.openfoodfacts.dev/api/v2/product/3017620422003/nutella-ferrero?fields=knowledge_panels
Panels shown on the web: https://uk.openfoodfacts.dev/product/3017620422003/nutella-ferrero?panels=1

image

@stephanegigandet stephanegigandet added 📖 Knowledge Panels https://wiki.openfoodfacts.org/Knowledge_panels 🌱 Eco-Score https://world.openfoodfacts.org/eco-score-the-environmental-impact-of-food-products labels Oct 6, 2021
@stephanegigandet stephanegigandet requested a review from a team as a code owner October 6, 2021 12:50
@jasmeet0817
Copy link
Contributor

jasmeet0817 commented Oct 6, 2021

Question regarding icon_color_from_evaluation, should the BE just send the evaluation and let the FE decide whether they want to color or not, maybe the FE doesn't want to color the icon even if icon_color_from_evaluation is true, because the client may want to do something completely different according to the evaluation. Also maybe the client also wants to color the title string, not just the icon.

Then a discrepancy I saw in the sample data you shared:
Parent of a ecoscore_carbon_impact was ecoscore_carbon_impact
image

Copy link
Contributor

@jasmeet0817 jasmeet0817 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another question: What's the difference between grade and evaluation, I see some panels have grade and some have evaluation

@stephanegigandet
Copy link
Contributor Author

Question regarding icon_color_from_evaluation, should the BE just send the evaluation and let the FE decide whether they want to color or not

It's more the backend that says: this icon file can and should be colored.

For instance if it's the car icon, the image file is in fact a black SVG icon, and it should be colored.

But if it's the Nutri-Score icon, the backend will specify a different file for each grade, the image actually contains multiple colors, and we absolutely don't want the client to try to change its color.

@stephanegigandet
Copy link
Contributor Author

Another question: What's the difference between grade and evaluation, I see some panels have grade and some have evaluation

The possible values are different:

grade: a, b, c, d, e, unknown (suitable for instance for the Eco-Score or Nutri-Score panels)
evaluation: good, average, bad, neutral unknown (suitable for instance for the Carbon impact panel, or the labels panels)

@sonarcloud
Copy link

sonarcloud bot commented Oct 6, 2021

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
0.0% 0.0% Duplication

@stephanegigandet
Copy link
Contributor Author

Parent of a ecoscore_carbon_impact was ecoscore_carbon_impact

Fixed, thank you!

@jasmeet0817
Copy link
Contributor

Another question: What's the difference between grade and evaluation, I see some panels have grade and some have evaluation

The possible values are different:

grade: a, b, c, d, e, unknown (suitable for instance for the Eco-Score or Nutri-Score panels) evaluation: good, average, bad, neutral unknown (suitable for instance for the Carbon impact panel, or the labels panels)

it's just a little difficult to follow as the client of the service, since the guidelines are not clear when the client should use grade vs eval. Suggestion: If grade is something only used for scores and eval for everything else, you could rename grade to score to make the distinction more clear.

@jasmeet0817
Copy link
Contributor

btw I found that the type of panel is empty sometimes:

image

@jasmeet0817
Copy link
Contributor

Non Environment panels still have the old title/subtitle/icon
image

@stephanegigandet
Copy link
Contributor Author

you could rename grade to score to make the distinction more clear.

In fact for both the Nutri-Score and the Eco-Score, there are both a score and a grade. The score is numeric (e.g. from -40 to 15 for the Nutri-Score, and from 0 to 100 for the Eco-Score). And the scores are then mapped to a 5 letter grade.

So I don't think we should rename grade to score.

@stephanegigandet
Copy link
Contributor Author

btw I found that the type of panel is empty sometimes:

yes, to be honest, I'm not sure what we will use the type for, and which types we will eventually have. So just ignore "type" for now. I will put "info" or something for now instead of an empty type.

@jasmeet0817
Copy link
Contributor

btw I found that the type of panel is empty sometimes:

yes, to be honest, I'm not sure what we will use the type for, and which types we will eventually have. So just ignore "type" for now. I will put "info" or something for now instead of an empty type.

Yeah, maybe we don't need it. Currently the openfoodfacts-dart package expects something, so maybe just give a deafult value and perhaps we can remove it later

@jasmeet0817
Copy link
Contributor

btw I found that the type of panel is empty sometimes:

yes, to be honest, I'm not sure what we will use the type for, and which types we will eventually have. So just ignore "type" for now. I will put "info" or something for now instead of an empty type.

Yeah, maybe we don't need it. Currently the openfoodfacts-dart package expects something, so maybe just give a deafult value and perhaps we can remove it later

better yet, you can just omit the type if you don't think it's necessary, but if it's there it should have a value that the client understands.

https://github.com/openfoodfacts/openfoodfacts-dart/blob/master/lib/model/KnowledgePanel.dart#L9

@alexgarel
Copy link
Member

@stephanegigandet
Copy link
Contributor Author

I have a json parsing error in my browser for https://uk.openfoodfacts.dev/api/v2/product/3017620422003/nutella-ferrero?fields=knowledge_panels:

ah, it's my rule to remove trailing commas that is a bit too aggressive, I'll fix it.

@stephanegigandet stephanegigandet merged commit ecc8539 into main Oct 7, 2021
@stephanegigandet stephanegigandet deleted the panels branch October 7, 2021 12:24
@jasmeet0817
Copy link
Contributor

Non Environment panels still have the old title/subtitle/icon image

Ah I think this comment was missed.

@stephanegigandet
Copy link
Contributor Author

Ah I think this comment was missed.

Sorry, fix included in #5950

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 Eco-Score https://world.openfoodfacts.org/eco-score-the-environmental-impact-of-food-products 📖 Knowledge Panels https://wiki.openfoodfacts.org/Knowledge_panels
Projects
Development

Successfully merging this pull request may close these issues.

3 participants