Add screen-reader labels across multiple parts of the UI (#5274, #5339,

#5372)
wagtail · Jun 21, 2019 · 6ec4ae0 · 6ec4ae0
1 parent 4f1a9d1
commit 6ec4ae0
Show file tree

Hide file tree

Showing 18 changed files with 152 additions and 65 deletions.
diff --git a/CHANGELOG.txt b/CHANGELOG.txt
@@ -29,7 +29,11 @@ Changelog
  * Add more contextual information for screen readers in the explorer menu’s links (Helen Chapman)
  * Added `process_child_object` and `exclude_fields` arguments to ``Page.copy()`` to make it easier for third-party apps to customise copy behavior (Karl Hobley)
  * Added `Page.with_content_json()`, allowing revision content loading behaviour to be customised on a per-model basis (Karl Hobley)
- * Improve screen-reader labels for action links in page listing (Helen Chapman, Katie Locke)
+ * Improved screen-reader labels for action links in page listing (Helen Chapman, Katie Locke)
+ * Added screen-reader labels for table headings in page listing (Helen Chapman, Katie Locke)
+ * Added screen reader labels for page privacy toggle, edit lock, status tag in page explorer & edit views (Helen Chapman, Katie Locke)
+ * Added screen-reader labels for dashboard summary cards (Helen Chapman, Katie Locke)
+ * Added screen-reader labels for privacy toggle of collections (Helen Chapman, Katie Locke)
  * Fix: ModelAdmin no longer fails when filtering over a foreign key relation (Jason Dilworth, Matt Westcott)
  * Fix: The Wagtail version number is now visible within the Settings menu (Kevin Howbrook)
  * Fix: Scaling images now rounds values to an integer so that images render without errors (Adrian Brunyate)
@@ -57,6 +61,7 @@ Changelog
  * Fix: Add labels to permission checkboxes for screen reader users (Helen Chapman, Katie Locke)
  * Fix: Page.copy() no longer copies child objects when the accesssor name is included in `exclude_fields_in_copy` (Karl Hobley)
  * Fix: Move focus to the pages explorer menu when open (Helen Chapman)
+ * Fix: Clicking the privacy toggle while the page is still loading no longer loads the wrong data in the page (Helen Chapman)
 
 
 2.5.1 (07.05.2019)

diff --git a/docs/releases/2.6.rst b/docs/releases/2.6.rst
@@ -45,6 +45,10 @@ We’ve also had a look at how controls are labeled across the UI for screen rea
 * Added a label to the modals’ “close” button for screen reader users (Helen Chapman, Katie Locke)
 * Added labels to permission checkboxes for screen reader users (Helen Chapman, Katie Locke)
 * Improve screen-reader labels for action links in page listing (Helen Chapman, Katie Locke)
+* Add screen-reader labels for table headings in page listing (Helen Chapman, Katie Locke)
+* Add screen reader labels for page privacy toggle, edit lock, status tag in page explorer & edit views (Helen Chapman, Katie Locke)
+* Add screen-reader labels for dashboard summary cards (Helen Chapman, Katie Locke)
+* Add screen-reader labels for privacy toggle of collections (Helen Chapman, Katie Locke)
 
 Again, this is still a work in progress – if you are aware of other existing accessibility issues, please do `open an issue <https://github.com/wagtail/wagtail/issues?q=is%3Aopen+is%3Aissue+label%3AAccessibility>`_ if there isn’t one already.
 
@@ -88,6 +92,7 @@ Bug fixes
  * Restore custom "Date" icon for scheduled publishing panel in Edit page’s Settings tab (Helen Chapman)
  * Added missing form media to user edit form template (Matt Westcott)
  * ``Page.copy()`` no longer copies child objects when the accesssor name is included in ``exclude_fields_in_copy`` (Karl Hobley)
+ * Clicking the privacy toggle while the page is still loading no longer loads the wrong data in the page (Helen Chapman)
 
 
 Upgrade considerations

diff --git a/wagtail/admin/site_summary.py b/wagtail/admin/site_summary.py
@@ -1,7 +1,6 @@
 from django.template.loader import render_to_string
 
-from wagtail.admin.navigation import get_explorable_root_page
-from wagtail.admin.utils import user_has_any_page_permission
+from wagtail.admin.utils import get_site_for_user, user_has_any_page_permission
 from wagtail.core import hooks
 from wagtail.core.models import Page, Site
 
@@ -27,7 +26,9 @@ class PagesSummaryItem(SummaryItem):
     template = 'wagtailadmin/home/site_summary_pages.html'
 
     def get_context(self):
-        root_page = get_explorable_root_page(self.request.user)
+        site_details = get_site_for_user(self.request.user)
+        root_page = site_details['root_page']
+        site_name = site_details['site_name']
 
         if root_page:
             page_count = Page.objects.descendant_of(root_page, inclusive=True).count()
@@ -50,6 +51,7 @@ def get_context(self):
         return {
             'root_page': root_page,
             'total_pages': page_count,
+            'site_name': site_name,
         }
 
     def is_shown(self):

diff --git a/wagtail/admin/static_src/wagtailadmin/js/privacy-switch.js b/wagtail/admin/static_src/wagtailadmin/js/privacy-switch.js
@@ -1,8 +1,8 @@
 $(function() {
     /* Interface to set permissions from the explorer / editor */
-    $('a.action-set-privacy').on('click', function() {
+    $('button.action-set-privacy').on('click', function() {
         ModalWorkflow({
-            url: this.href,
+            url: this.getAttribute('data-url'),
             onload: {
                 'set_privacy': function(modal, jsonData) {
                     $('form', modal.body).on('submit', function() {

diff --git a/wagtail/admin/templates/wagtailadmin/collections/_privacy_switch.html b/wagtail/admin/templates/wagtailadmin/collections/_privacy_switch.html
@@ -5,10 +5,14 @@
 {% if not collection.is_root %}
 <div class="privacy-indicator {% if is_public %}public{% else %}private{% endif %}">
     {% trans "Privacy" %}
-    <a href="{% url 'wagtailadmin_collections:set_privacy' collection.id %}" class="status-tag primary  action-set-privacy">
+    <button data-url="{% url 'wagtailadmin_collections:set_privacy' collection.id %}" class="status-tag primary  action-set-privacy">
         {# labels are shown/hidden in CSS according to the 'private' / 'public' class on view-permission-indicator #}
-        <span class="label-public icon icon-view">{% trans 'Public' %}</span>
-        <span class="label-private icon icon-no-view">{% trans 'Private' %}</span>
-    </a>
+        <span class="label-public icon icon-view" aria-label="{% trans 'Set collection privacy. Current status: Public' %}">
+            {% trans "Public" %}
+        </span>
+        <span class="label-private icon icon-no-view" aria-label="{% trans 'Set collection privacy. Current status: Private' %}">
+            {% trans "Private" %}
+        </span>
+    </button>
 </div>
 {% endif %}
diff --git a/wagtail/admin/templates/wagtailadmin/home/site_summary_pages.html b/wagtail/admin/templates/wagtailadmin/home/site_summary_pages.html
@@ -1,11 +1,11 @@
 {% load i18n wagtailadmin_tags %}
 
 <li class="icon icon-doc-empty-inverse">
-<a href="{% url 'wagtailadmin_explore' root_page.pk %}">
-    {% blocktrans count counter=total_pages with total_pages|intcomma as total %}
-        <span>{{ total }}</span> Page
-    {% plural %}
-        <span>{{ total }}</span> Pages
-    {% endblocktrans %}
+    <a href="{% url 'wagtailadmin_explore' root_page.pk %}">
+        {% blocktrans count counter=total_pages with total_pages|intcomma as total %}
+            <span>{{ total }}</span> Page <span class="visuallyhidden">created in {{ site_name }}</span>
+        {% plural %}
+            <span>{{ total }}</span> Pages <span class="visuallyhidden">created in {{ site_name }}</span>
+        {% endblocktrans %}
     </a>
 </li>
diff --git a/wagtail/admin/templates/wagtailadmin/pages/_lock_switch.html b/wagtail/admin/templates/wagtailadmin/pages/_lock_switch.html
@@ -9,11 +9,14 @@
 
     {% csrf_token %}
     <input type="hidden" name="next" value="{% url 'wagtailadmin_pages:edit' page.id %}" />
-    <button type="submit" class="status-tag {{ page.locked|yesno:'primary,secondary' }}{% if not page_perms.can_lock %} disabled{% endif %}">
+    <button
+        type="submit"
+        class="status-tag {{ page.locked|yesno:'primary,secondary' }}{% if not page_perms.can_lock %} disabled{% endif %}"
+        aria-label="{% if page.locked %}{% trans 'Remove editor lock. Current status: Locked' %}{% else %}{% trans 'Apply editor lock. Current status: Unlocked' %}{% endif %}">
         {% if page.locked %}
-            {% trans 'Locked' %}
+            {% trans "Locked" %}
         {% else %}
-            {% trans 'Unlocked' %}
+            {% trans "Unlocked" %}
         {% endif %}
     </button>
 </form>
diff --git a/wagtail/admin/templates/wagtailadmin/pages/_privacy_switch.html b/wagtail/admin/templates/wagtailadmin/pages/_privacy_switch.html
@@ -9,16 +9,24 @@
 <div class="privacy-indicator {% if is_public %}public{% else %}private{% endif %}">
     {% trans "Privacy" %}
     {% if page_perms.can_set_view_restrictions %}
-        <a href="{% url 'wagtailadmin_pages:set_privacy' page.id %}" class="status-tag primary  action-set-privacy">
+        <button data-url="{% url 'wagtailadmin_pages:set_privacy' page.id %}" class="status-tag primary  action-set-privacy">
             {# labels are shown/hidden in CSS according to the 'private' / 'public' class on view-permission-indicator #}
-            <span class="label-public icon icon-view">{% trans 'Public' %}</span>
-            <span class="label-private icon icon-no-view">{% trans 'Private' %}</span>
-        </a>
+            <span class="label-public icon icon-view" aria-label="{% trans 'Set page privacy. Current status: Public' %}">
+                {% trans "Public" %}
+            </span>
+            <span class="label-private icon icon-no-view" aria-label="{% trans 'Set page privacy. Current status: Private' %}">
+                {% trans "Private" %}
+            </span>
+        </button>
     {% else %}
         {% if is_public %}
-            <span class="label-public status-tag primary icon icon-view ">{% trans 'Public' %}</span>
+            <span class="label-public status-tag primary icon icon-view" aria-label="{% trans 'Page privacy. Current status: Public' %}">
+                {% trans "Public" %}
+            </span>
         {% else %}
-            <span class="label-private status-tag primary icon icon-no-view">{% trans 'Private' %}</span>
+            <span class="label-private status-tag primary icon icon-no-view" aria-label="{% trans 'Page privacy. Current status: Private' %}">
+                {% trans "Private" %}
+            </span>
         {% endif %}
     {% endif %}
 </div>

diff --git a/wagtail/admin/templates/wagtailadmin/pages/listing/_table_headers_explore.html b/wagtail/admin/templates/wagtailadmin/pages/listing/_table_headers_explore.html
@@ -11,7 +11,7 @@
 
 If either sortable or orderable is true, the following variables are also required:
 
-parent_page: The page instance currently being browsed (used to generate the correct wagtailadmin_explore urls)
+parent_page: The page instance currently being browsed (used to generate the correct wagtailadmin_explore urls and title text)
 ordering: the current sort parameter
 
 {% endcomment %}
@@ -34,30 +34,30 @@
     {% endif %}
     <th class="title">
         {% trans 'Title' as title_label %}
-        {% table_header_label label=title_label sortable=sortable sort_field='title' %}
+        {% page_table_header_label label=title_label sortable=sortable sort_field='title' parent_page_title=parent_page.title %}
     </th>
     {% if show_parent %}
         <th class="parent">
             {% trans 'Parent' as parent_label %}
-            {% table_header_label label=parent_label sortable=0 %}
+            {% page_table_header_label label=parent_label sortable=0 parent_page_title=parent_page.title %}
         </th>
     {% endif %}
      <th class="updated">
         {% trans 'Updated' as updated_label %}
-        {% table_header_label label=updated_label sortable=sortable sort_field='latest_revision_created_at' %}
+        {% page_table_header_label label=updated_label sortable=sortable sort_field='latest_revision_created_at' parent_page_title=parent_page.title %}
     </th>
     <th class="type">
         {% trans 'Type' as type_label %}
 
         {% if sortable and sortable_by_type %}
-            {% table_header_label label=type_label sortable=1 sort_field='content_type' %}
+            {% page_table_header_label label=type_label sortable=1 sort_field='content_type' parent_page_title=parent_page.title %}
         {% else %}
-            {% table_header_label label=type_label sortable=0 %}
+            {% page_table_header_label label=type_label sortable=0 parent_page_title=parent_page.title %}
         {% endif %}
     </th>
     <th class="status">
         {% trans 'Status' as status_label %}
-        {% table_header_label label=status_label sortable=sortable sort_field='live' %}
+        {% page_table_header_label label=status_label sortable=sortable sort_field='live' parent_page_title=parent_page.title %}
     </th>
     <th></th>
 </tr>
diff --git a/wagtail/admin/templates/wagtailadmin/shared/page_status_tag.html b/wagtail/admin/templates/wagtailadmin/shared/page_status_tag.html
@@ -1,5 +1,10 @@
+{% load i18n %}
 {% if page.live %}
-    <a href="{{ page.url }}" target="_blank" rel="noopener noreferrer" class="status-tag primary">{{ page.status_string }}</a>
+    <a href="{{ page.url }}" target="_blank" rel="noopener noreferrer" class="status-tag primary" title="{% trans 'Visit the live page' %}">
+        <span class="visuallyhidden">{% trans "Current page status:" %}</span> {{ page.status_string }}
+    </a>
 {% else %}
-    <span class="status-tag">{{ page.status_string }}</span>
+    <span class="status-tag">
+        <span class="visuallyhidden">{% trans "Current page status:" %}</span> {{ page.status_string }}
+    </span>
 {% endif %}
diff --git a/wagtail/admin/templatetags/wagtailadmin_tags.py b/wagtail/admin/templatetags/wagtailadmin_tags.py
@@ -9,8 +9,9 @@
 from django.template.defaultfilters import stringfilter
 from django.template.loader import render_to_string
 from django.templatetags.static import static
-from django.utils.html import format_html
+from django.utils.html import format_html, format_html_join
 from django.utils.safestring import mark_safe
+from django.utils.translation import ugettext_lazy as _
 
 from wagtail.admin.menu import admin_menu
 from wagtail.admin.navigation import get_explorable_root_page
@@ -291,7 +292,27 @@ def querystring(context, **kwargs):
 
 
 @register.simple_tag(takes_context=True)
-def table_header_label(context, label=None, sortable=True, ordering=None, sort_context_var='ordering', sort_param='ordering', sort_field=None):
+def page_table_header_label(context, label=None, parent_page_title=None, **kwargs):
+    """
+    Wraps table_header_label to add a title attribute based on the parent page title and the column label
+    """
+    if label:
+        translation_context = {'parent': parent_page_title, 'label': label}
+        ascending_title_text = _("Sort the order of child pages within '%(parent)s' by '%(label)s' in ascending order.") % translation_context
+        descending_title_text = _("Sort the order of child pages within '%(parent)s' by '%(label)s' in descending order.") % translation_context
+    else:
+        ascending_title_text = None
+        descending_title_text = None
+
+    return table_header_label(context, label=label, ascending_title_text=ascending_title_text, descending_title_text=descending_title_text, **kwargs)
+
+
+@register.simple_tag(takes_context=True)
+def table_header_label(
+    context, label=None, sortable=True, ordering=None,
+    sort_context_var='ordering', sort_param='ordering', sort_field=None,
+    ascending_title_text=None, descending_title_text=None
+):
     """
     A label to go in a table header cell, optionally with a 'sort' link that alternates between
     forward and reverse sorting
@@ -305,6 +326,9 @@ def table_header_label(context, label=None, sortable=True, ordering=None, sort_c
         For example, if sort_param='ordering' and sort_field='title', then a URL parameter of
         ordering=title indicates that the listing is ordered forwards on this column, and a URL parameter
         of ordering=-title indicated that the listing is ordered in reverse on this column
+    ascending_title_text = title attribute to use on the link when the link action will sort in ascending order
+    descending_title_text = title attribute to use on the link when the link action will sort in descending order
+
     To disable sorting on this column, set sortable=False or leave sort_field unspecified.
     """
     if not sortable or not sort_field:
@@ -317,23 +341,37 @@ def table_header_label(context, label=None, sortable=True, ordering=None, sort_c
 
     if ordering == sort_field:
         # currently ordering forwards on this column; link should change to reverse ordering
-        url = querystring(context, **{sort_param: reverse_sort_field})
-        classname = "icon icon-arrow-down-after teal"
+        attrs = {
+            'href': querystring(context, **{sort_param: reverse_sort_field}),
+            'class': "icon icon-arrow-down-after teal",
+        }
+        if descending_title_text is not None:
+            attrs['title'] = descending_title_text
 
     elif ordering == reverse_sort_field:
         # currently ordering backwards on this column; link should change to forward ordering
-        url = querystring(context, **{sort_param: sort_field})
-        classname = "icon icon-arrow-up-after teal"
+        attrs = {
+            'href': querystring(context, **{sort_param: sort_field}),
+            'class': "icon icon-arrow-up-after teal",
+        }
+        if ascending_title_text is not None:
+            attrs['title'] = ascending_title_text
 
     else:
         # not currently ordering on this column; link should change to forward ordering
-        url = querystring(context, **{sort_param: sort_field})
-        classname = "icon icon-arrow-down-after"
+        attrs = {
+            'href': querystring(context, **{sort_param: sort_field}),
+            'class': "icon icon-arrow-down-after",
+        }
+        if ascending_title_text is not None:
+            attrs['title'] = ascending_title_text
+
+    attrs_string = format_html_join(' ', '{}="{}"', attrs.items())
 
     return format_html(
         # need whitespace around label for correct positioning of arrow icon
-        '<a href="{url}" class="{classname}"> {label} </a>',
-        url=url, classname=classname, label=label
+        '<a {attrs}> {label} </a>',
+        attrs=attrs_string, label=label
     )
 
 

diff --git a/wagtail/admin/tests/test_pages_views.py b/wagtail/admin/tests/test_pages_views.py
@@ -1993,8 +1993,8 @@ def test_editor_page_shows_live_url_in_status_when_draft_edits_exist(self):
 
         response = self.client.get(reverse('wagtailadmin_pages:edit', args=(self.child_page.id, )))
 
-        link_to_draft = '<a href="/revised-slug-in-draft-only/" target="_blank" rel="noopener noreferrer" class="status-tag primary">live + draft</a>'
-        link_to_live = '<a href="/hello-world/" target="_blank" rel="noopener noreferrer" class="status-tag primary">live + draft</a>'
+        link_to_draft = '<a href="/revised-slug-in-draft-only/" target="_blank" rel="noopener noreferrer" class="status-tag primary" title="Visit the live page"><span class="visuallyhidden">Current page status:</span> live + draft</a>'
+        link_to_live = '<a href="/hello-world/" target="_blank" rel="noopener noreferrer" class="status-tag primary" title="Visit the live page"><span class="visuallyhidden">Current page status:</span> live + draft</a>'
         input_field_for_draft_slug = '<input type="text" name="slug" value="revised-slug-in-draft-only" id="id_slug" maxlength="255" required />'
         input_field_for_live_slug = '<input type="text" name="slug" value="hello-world" id="id_slug" maxlength="255" required />'
 
@@ -2016,8 +2016,8 @@ def test_editor_page_shows_custom_live_url_in_status_when_draft_edits_exist(self
 
         response = self.client.get(reverse('wagtailadmin_pages:edit', args=(self.single_event_page.id, )))
 
-        link_to_draft = '<a href="/revised-slug-in-draft-only/pointless-suffix/" target="_blank" rel="noopener noreferrer" class="status-tag primary">live + draft</a>'
-        link_to_live = '<a href="/mars-landing/pointless-suffix/" target="_blank" rel="noopener noreferrer" class="status-tag primary">live + draft</a>'
+        link_to_draft = '<a href="/revised-slug-in-draft-only/pointless-suffix/" target="_blank" rel="noopener noreferrer" class="status-tag primary" title="Visit the live page"><span class="visuallyhidden">Current page status:</span> live + draft</a>'
+        link_to_live = '<a href="/mars-landing/pointless-suffix/" target="_blank" rel="noopener noreferrer" class="status-tag primary" title="Visit the live page"><span class="visuallyhidden">Current page status:</span> live + draft</a>'
         input_field_for_draft_slug = '<input type="text" name="slug" value="revised-slug-in-draft-only" id="id_slug" maxlength="255" required />'
         input_field_for_live_slug = '<input type="text" name="slug" value="mars-landing" id="id_slug" maxlength="255" required />'