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

Accessility: set a title attribute on Main menu item for better experience on screen readers #10160

Merged
merged 2 commits into from Jun 2, 2016

Conversation

tyrylu
Copy link
Contributor

@tyrylu tyrylu commented May 18, 2016

This pull request fixesan accessibility issue in the main menu with items having only an icon. The title attribute is not enough to provide full accessibility, for example NVDA does not show it when navigating using arrow keys or in the elements list. So this change adds an aria-label to the icon when it exists ant therefore the label is not displayed as the link's child.

…ccessibility label, so it an aria-label to the icon so screen reader element lists and such have something to display.
@@ -3,7 +3,7 @@

{% macro menuItemLabel(label, icon) %}
{% if icon is defined and icon and icon starts with 'icon-' %}
<span class="{{ icon|striptags }}"></span>
<span class="{{ icon|striptags }}" aria-label="{{ label|translate }}"></span>
Copy link
Member

Choose a reason for hiding this comment

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

Do you mind changing it to {{ label|translate|e('html_attr) }} for better escaping? Cheers

@mattab mattab added the Needs Review PRs that need a code review label May 27, 2016
@mattab mattab added this to the 2.16.2 milestone May 27, 2016
@mattab mattab added the c: Accessibility When something is not usable for a certain group (eg missing contrast) or devices (eg smartphones). label May 27, 2016
@sgiehl sgiehl merged commit f38c12a into matomo-org:master Jun 2, 2016
@mattab mattab changed the title Main menu accessibility fix Accessility: set a title attribute on Main menu item for better experience on screen readers Aug 3, 2016
@mattab
Copy link
Member

mattab commented Aug 3, 2016

Thanks @tyrylu for the pull request, and improving accessibility in Piwik 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Accessibility When something is not usable for a certain group (eg missing contrast) or devices (eg smartphones). Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants