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: add back nutrition facts table title in knowledge panel #6867

Merged
merged 2 commits into from
Jun 8, 2022

Conversation

stephanegigandet
Copy link
Contributor

The openfoodfacts-dart package expects a non null title for the table elements in knowledge panels. This adds back the table title that was removed in #6839

Hot fix deployed in production as it was breaking scanning and searching in Smoothie.

Related:
openfoodfacts/smooth-app#2203
openfoodfacts/openfoodfacts-dart#477

@stephanegigandet stephanegigandet requested a review from a team as a code owner June 7, 2022 14:38
@github-actions github-actions bot added 📖 Knowledge Panels https://wiki.openfoodfacts.org/Knowledge_panels Template::Toolkit The templating toolkit used by product opener. The starting point for HTML/JS/CSS fixes. labels Jun 7, 2022
Co-authored-by: Pierre Slamich <pierre@openfoodfacts.org>
@sonarcloud
Copy link

sonarcloud bot commented Jun 7, 2022

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

@VaiTon
Copy link
Member

VaiTon commented Jun 7, 2022

Maybe adding a TODO "remove later" would make us remember to change it?

@stephanegigandet
Copy link
Contributor Author

Maybe adding a TODO "remove later" would make us remember to change it?

@VaiTon : What would need to be changed? It's a good thing to have table titles. They don't have to be displayed by the client, but they are useful for screen readers.

On the web, we use it to set an aria-label:

<table[% IF element.table_id %] id="[% table_element.table_id %]"[% END %] aria-label="[% table_element.title %]">

@VaiTon
Copy link
Member

VaiTon commented Jun 8, 2022

I thought we moved them here

If it's still useful, let's keep it!

@stephanegigandet stephanegigandet merged commit 2085f4f into main Jun 8, 2022
@stephanegigandet stephanegigandet deleted the add-back-nutrition-table-title branch June 8, 2022 15:43
@stephanegigandet
Copy link
Contributor Author

I thought we moved them here

Right, we added the title elsewhere, but we still need a table title, even if it's shown only for screen readers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📖 Knowledge Panels https://wiki.openfoodfacts.org/Knowledge_panels Template::Toolkit The templating toolkit used by product opener. The starting point for HTML/JS/CSS fixes.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants