-
-
Notifications
You must be signed in to change notification settings - Fork 400
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: Knowledge panels for labels #5950
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo two style comments.
# Add panels for environmental Eco-Score labels | ||
if ((defined $product_ref->{ecoscore_data}) and (defined $product_ref->{ecoscore_data}{adjustments}) | ||
and (defined $product_ref->{ecoscore_data}{adjustments}{production_system}) | ||
and (defined $product_ref->{ecoscore_data}{adjustments}{production_system}{labels})) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't you have some utils function to ask is_defined($product_ref, "ecoscore_data", "adjastments", "production_system", "labels")
? It would make sense, I think !
(and it could directly return the value)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could do something like that. A related discussion: https://stackoverflow.com/questions/24949006/efficiently-get-hash-entry-only-if-it-exists-in-perl
templates/web/panels/panel.tt.html
Outdated
@@ -1,8 +1,8 @@ | |||
[% panel = panels.$panel_id %] | |||
<!-- start templates/[% template.name %] - panel_id: [% panel_id %] --> | |||
<ul data-accordion class="accordion" id="[% panel_id %]" style="margin-top:0.5rem;margin-bottom:0.5rem;"> | |||
<ul data-accordion class="accordion" id="[% panel_id | replace(':','_') %]" style="margin-top:0.5rem;margin-bottom:0.5rem;"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(very minor) HTML guys usually use "-" more than "_" as a separator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I can change that
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Added new panels for the labels used for Eco-Score (e.g. organic labels)
-> Panel data comes from the taxonomy. We may want to add a way to references translations from the po/common files instead of managing translations inside the taxonomies. That would ease the translations.
Added a way to generate strings that contain a comma separated list (e.g. a list of labels), by genering the string with trailing commas on all elements, and removing the last trailing comma. (hacky but should work)
Fixed sample Nutella brand panel to use the new title_element structure