Fix various team accessibility issues.

Authors: - Peter Fogg - Daniel Friedman TNL-1953
2015-08-03 14:18:31 -04:00
parent 00eb18a1f9
commit 26bbbb967e
23 changed files with 129 additions and 42 deletions
--- a/common/static/common/js/components/views/paginated_view.js
+++ b/common/static/common/js/components/views/paginated_view.js
@@ -1,3 +1,19 @@
+/**
+ * A base class for a view which renders and paginates a collection,
+ * along with a header and footer displaying controls for
+ * pagination.
+ *
+ * Subclasses should define a `type` property which will be used to
+ * create class names for the different subcomponents, as well as an
+ * `itemViewClass` which will be used to display each individual
+ * element of the collection.
+ *
+ * If provided, the `srInfo` property will be used to provide
+ * information for screen readers on each item. The `srInfo.text`
+ * property will be shown in the header, and the `srInfo.id` property
+ * will be used to connect each card's title with the header text via
+ * the ARIA describedby attribute.
+ */
 ;(function(define) {
    'use strict';
    define([
@@ -24,7 +40,7 @@
            },

            createHeaderView: function() {
-                return new PagingHeader({collection: this.options.collection});
+                return new PagingHeader({collection: this.options.collection, srInfo: this.srInfo});
            },

            createFooterView: function() {
--- a/common/static/common/js/components/views/paging_header.js
+++ b/common/static/common/js/components/views/paging_header.js
@@ -8,6 +8,7 @@
    ], function (Backbone, _, gettext, headerTemplate) {
        var PagingHeader = Backbone.View.extend({
            initialize: function (options) {
+                this.srInfo = options.srInfo;
                this.collections = options.collection;
                this.collection.bind('add', _.bind(this.render, this));
                this.collection.bind('remove', _.bind(this.render, this));
@@ -28,7 +29,10 @@
                        context, true
                    );
                }
-                this.$el.html(_.template(headerTemplate, {message: message}));
+                this.$el.html(_.template(headerTemplate, {
+                    message: message,
+                    srInfo: this.srInfo
+                }));
                return this;
            }
        });
--- a/common/static/common/templates/components/paginated-view.underscore
+++ b/common/static/common/templates/components/paginated-view.underscore
@@ -1,4 +1,4 @@
 <div class="sr-is-focusable sr-<%= type %>-view" tabindex="-1"></div>
 <div class="<%= type %>-paging-header"></div>
-<div class="<%= type %>-list"></div>
+<ul class="<%= type %>-list"></ul>
 <div class="<%= type %>-paging-footer"></div>
--- a/common/static/common/templates/components/paging-header.underscore
+++ b/common/static/common/templates/components/paging-header.underscore
@@ -1,3 +1,6 @@
+<% if (!_.isUndefined(srInfo)) { %>
+  <h2 class="sr" id="<%= srInfo.id %>"><%- srInfo.text %></h2>
+<% } %>
 <div class="search-tools">
    <span class="search-count">
        <%= message %>
--- a/common/test/acceptance/pages/lms/fields.py
+++ b/common/test/acceptance/pages/lms/fields.py
@@ -145,7 +145,7 @@ class FieldsMixin(object):

        return self.value_for_text_field(field_id)

-    def value_for_text_field(self, field_id, value=None):
+    def value_for_text_field(self, field_id, value=None, press_enter=True):
        """
        Get or set the value of a text field.
        """
@@ -159,7 +159,8 @@ class FieldsMixin(object):
            current_value = query.attrs('value')[0]
            query.results[0].send_keys(u'\ue003' * len(current_value))  # Delete existing value.
            query.results[0].send_keys(value)  # Input new value
-            query.results[0].send_keys(u'\ue007')  # Press Enter
+            if press_enter:
+                query.results[0].send_keys(u'\ue007')  # Press Enter
        return query.attrs('value')[0]

    def value_for_textarea_field(self, field_id, value=None):
--- a/common/test/acceptance/tests/lms/test_teams.py
+++ b/common/test/acceptance/tests/lms/test_teams.py
@@ -180,12 +180,12 @@ class TeamsTabTest(TeamsTabBase):
        self.verify_teams_present(True)

    @ddt.data(
-        ('browse', 'div.topics-list'),
+        ('browse', '.topics-list'),
        # TODO: find a reliable way to match the "My Teams" tab
        # ('my-teams', 'div.teams-list'),
        ('teams/{topic_id}/{team_id}', 'div.discussion-module'),
        ('topics/{topic_id}/create-team', 'div.create-team-instructions'),
-        ('topics/{topic_id}', 'div.teams-list'),
+        ('topics/{topic_id}', '.teams-list'),
        ('not-a-real-route', 'div.warning')
    )
    @ddt.unpack
@@ -612,7 +612,7 @@ class CreateTeamTest(TeamsTabBase):

    def fill_create_form(self):
        """Fill the create team form fields with appropriate values."""
-        self.create_team_page.value_for_text_field(field_id='name', value=self.team_name)
+        self.create_team_page.value_for_text_field(field_id='name', value=self.team_name, press_enter=False)
        self.create_team_page.value_for_textarea_field(
            field_id='description',
            value='The Avengers are a fictional team of superheroes.'
@@ -691,7 +691,8 @@ class CreateTeamTest(TeamsTabBase):
                  'transform themselves through cutting-edge technologies, innovative pedagogy, and '
                  'rigorous courses. More than 70 schools, nonprofits, corporations, and international'
                  'organizations offer or plan to offer courses on the edX website. As of 22 October 2014,'
-                  'edX has more than 4 million users taking more than 500 courses online.'
+                  'edX has more than 4 million users taking more than 500 courses online.',
+            press_enter=False
        )
        self.create_team_page.submit_form()

--- a/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js
+++ b/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js
@@ -72,6 +72,12 @@ define([
                expectFocus(teamsTabView.$('.warning'));
            });

+            it('does not interfere with anchor links to #content', function () {
+                var teamsTabView = createTeamsTabView();
+                teamsTabView.router.navigate('#content', {trigger: true});
+                expect(teamsTabView.$('.warning')).toHaveClass('is-hidden');
+            });
+
            it('displays and focuses an error message when trying to navigate to a nonexistent topic', function () {
                var requests = AjaxHelpers.requests(this),
                    teamsTabView = createTeamsTabView();
--- a/lms/djangoapps/teams/static/teams/js/views/edit_team.js
+++ b/lms/djangoapps/teams/static/teams/js/views/edit_team.js
@@ -15,6 +15,7 @@

                events: {
                    'click .action-primary': 'createTeam',
+                    'submit form': 'createTeam',
                    'click .action-cancel': 'goBackToTopic'
                },

@@ -48,13 +49,6 @@
                        helpMessage: gettext('A short description of the team to help other learners understand the goals or direction of the team (maximum 300 characters).')
                    });

-                    this.optionalDescriptionField = new FieldViews.ReadonlyFieldView({
-                        model: this.teamModel,
-                        title: gettext('Optional Characteristics'),
-                        valueAttribute: 'optional_description',
-                        helpMessage: gettext('Help other learners decide whether to join your team by specifying some characteristics for your team. Choose carefully, because fewer people might be interested in joining your team if it seems too restrictive.')
-                    });
-
                    this.teamLanguageField = new FieldViews.DropdownFieldView({
                        model: this.teamModel,
                        title: gettext('Language'),
@@ -82,7 +76,6 @@
                    this.$el.html(_.template(editTeamTemplate)({primaryButtonTitle: this.primaryButtonTitle}));
                    this.set(this.teamNameField, '.team-required-fields');
                    this.set(this.teamDescriptionField, '.team-required-fields');
-                    this.set(this.optionalDescriptionField, '.team-optional-fields');
                    this.set(this.teamLanguageField, '.team-optional-fields');
                    this.set(this.teamCountryField, '.team-optional-fields');
                    return this;
@@ -97,7 +90,8 @@
                    }
                },

-                createTeam: function () {
+                createTeam: function (event) {
+                    event.preventDefault();
                    var view = this,
                        teamLanguage = this.teamLanguageField.fieldValue(),
                        teamCountry = this.teamCountryField.fieldValue();
--- a/lms/djangoapps/teams/static/teams/js/views/team_card.js
+++ b/lms/djangoapps/teams/static/teams/js/views/team_card.js
@@ -93,10 +93,8 @@
                    true
                );
            },
-            action: function (event) {
-                var url = 'teams/' + this.teamModel().get('topic_id') + '/' + this.teamModel().get('id');
-                event.preventDefault();
-                this.router.navigate(url, {trigger: true});
+            actionUrl: function () {
+                return '#teams/' + this.teamModel().get('topic_id') + '/' + this.teamModel().get('id');
            }
        });
        return TeamCardView;
--- a/lms/djangoapps/teams/static/teams/js/views/teams.js
+++ b/lms/djangoapps/teams/static/teams/js/views/teams.js
@@ -9,6 +9,15 @@
        var TeamsView = PaginatedView.extend({
            type: 'teams',

+            events: {
+                'click button.action': '' // entry point for team creation
+            },
+
+            srInfo: {
+                id: "heading-browse-teams",
+                text: gettext('All teams')
+            },
+
            initialize: function (options) {
                this.topic = options.topic;
                this.teamMemberships = options.teamMemberships;
@@ -18,7 +27,8 @@
                    topic: options.topic,
                    maxTeamSize: options.maxTeamSize,
                    countries: this.selectorOptionsArrayToHashWithBlank(options.teamParams.countries),
-                    languages: this.selectorOptionsArrayToHashWithBlank(options.teamParams.languages)
+                    languages: this.selectorOptionsArrayToHashWithBlank(options.teamParams.languages),
+                    srInfo: this.srInfo
                });
                PaginatedView.prototype.initialize.call(this);
            },
--- a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js
+++ b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js
@@ -22,6 +22,13 @@
                  TopicModel, TopicCollection, TeamModel, TeamCollection, TeamMembershipCollection,
                  TopicsView, TeamProfileView, MyTeamsView, TopicTeamsView, TeamEditView,
                  teamsTemplate) {
+            var TeamsHeaderModel = HeaderModel.extend({
+                initialize: function (attributes) {
+                    _.extend(this.defaults, {nav_aria_label: gettext('teams')});
+                    HeaderModel.prototype.initialize.call(this);
+                }
+            });
+
            var ViewWithHeader = Backbone.View.extend({
                initialize: function (options) {
                    this.header = options.header;
@@ -57,6 +64,12 @@
                    router = this.router = new Backbone.Router();
                    _.each([
                        [':default', _.bind(this.routeNotFound, this)],
+                        ['content', _.bind(function () {
+                            // The backbone router unfortunately usurps the
+                            // default behavior of in-page-links.  This hack
+                            // prevents the screen reader in-page-link from
+                            // being picked up by the backbone router.
+                        }, this)],
                        ['topics/:topic_id(/)', _.bind(this.browseTopic, this)],
                        ['topics/:topic_id/create-team(/)', _.bind(this.newTeam, this)],
                        ['teams/:topic_id/:team_id(/)', _.bind(this.browseTeam, this)],
@@ -102,7 +115,7 @@

                    this.mainView = this.tabbedView = new ViewWithHeader({
                        header: new HeaderView({
-                            model: new HeaderModel({
+                            model: new TeamsHeaderModel({
                                description: gettext("See all teams in your course, organized by topic. Join a team to collaborate with other learners who are interested in the same topic as you are."),
                                title: gettext("Teams")
                            })
@@ -113,7 +126,11 @@
                                url: 'my-teams',
                                view: this.myTeamsView
                            }, {
-                                title: gettext('Browse'),
+                                title: interpolate(
+                                    // Translators: sr_start and sr_end surround text meant only for screen readers.  The whole string will be shown to users as "Browse teams" if they are using a screenreader, and "Browse" otherwise.
+                                    gettext("Browse %(sr_start)s teams %(sr_end)s"),
+                                    {"sr_start": '<span class="sr">', "sr_end": '</span>'}, true
+                                ),
                                url: 'browse',
                                view: this.topicsView
                            }],
@@ -165,7 +182,7 @@
                    this.getTeamsView(topicID).done(function (teamsView) {
                        self.mainView = new ViewWithHeader({
                            header: new HeaderView({
-                                model: new HeaderModel({
+                                model: new TeamsHeaderModel({
                                    description: gettext("Create a new team if you can't find existing teams to join, or if you would like to learn with friends you know."),
                                    title: gettext("Create a New Team"),
                                    breadcrumbs: [
@@ -277,7 +294,7 @@
                        });
                    }
                    headerView = new HeaderView({
-                        model: new HeaderModel({
+                        model: new TeamsHeaderModel({
                            description: subject.get('description'),
                            title: subject.get('name'),
                            breadcrumbs: breadcrumbs
--- a/lms/djangoapps/teams/static/teams/js/views/topic_card.js
+++ b/lms/djangoapps/teams/static/teams/js/views/topic_card.js
@@ -30,9 +30,8 @@
                    CardView.prototype.initialize.apply(this, arguments);
                },

-                action: function (event) {
-                    event.preventDefault();
-                    this.router.navigate('topics/' + this.model.get('id'), {trigger: true});
+                actionUrl: function () {
+                    return '#topics/' + this.model.get('id');
                },

                configuration: 'square_card',
--- a/lms/djangoapps/teams/static/teams/js/views/topics.js
+++ b/lms/djangoapps/teams/static/teams/js/views/topics.js
@@ -7,8 +7,16 @@
        var TopicsView = PaginatedView.extend({
            type: 'topics',

+            srInfo: {
+                id: "heading-browse-topics",
+                text: gettext("All topics")
+            },
+
            initialize: function (options) {
-                this.itemViewClass = TopicCardView.extend({router: options.router});
+                this.itemViewClass = TopicCardView.extend({
+                    router: options.router,
+                    srInfo: this.srInfo
+                });
                PaginatedView.prototype.initialize.call(this);
            }
        });
--- a/lms/djangoapps/teams/static/teams/templates/edit-team.underscore
+++ b/lms/djangoapps/teams/static/teams/templates/edit-team.underscore
@@ -1,3 +1,4 @@
+<form>
 <div class="create-team wrapper-msg is-incontext urgency-low warning is-hidden" tabindex="-1">
    <div class="msg">
        <div class="msg-content">
@@ -20,6 +21,16 @@
    </div>

    <div class="team-optional-fields">
+        <fieldset>
+            <div class="u-field u-field-optional_description">
+                <legend aria-describedby="optional-characteristics-help">
+                    <p class="u-field-title"><%- gettext('Optional Characteristics') %></p>
+                </legend>
+                <span id="optional-characteristics-help" class="u-field-message">
+                    <p class="u-field-message-help"><%- gettext('Help other learners decide whether to join your team by specifying some characteristics for your team. Choose carefully, because fewer people might be interested in joining your team if it seems too restrictive.') %></p>
+                </span>
+            </div>
+        </fieldset>
    </div>
 </div>

@@ -45,3 +56,4 @@
        %>
    </button>
 </div>
+</form>
--- a/lms/static/js/components/card/views/card.js
+++ b/lms/static/js/components/card/views/card.js
@@ -23,6 +23,8 @@
            'text!templates/components/card/card.underscore'],
        function ($, _, Backbone, cardTemplate) {
            var CardView = Backbone.View.extend({
+                tagName: 'li',
+
                events: {
                    'click .action' : 'action'
                },
@@ -82,7 +84,8 @@
                        action_class: this.callIfFunction(this.actionClass),
                        action_url: this.callIfFunction(this.actionUrl),
                        action_content: this.callIfFunction(this.actionContent),
-                        configuration: this.callIfFunction(this.configuration)
+                        configuration: this.callIfFunction(this.configuration),
+                        srInfo: this.srInfo
                    }));
                    var detailsEl = this.$el.find('.card-meta');
                    _.each(this.callIfFunction(this.details), function (detail) {
--- a/lms/static/js/components/header/models/header.js
+++ b/lms/static/js/components/header/models/header.js
@@ -8,7 +8,8 @@ define(['backbone'], function (Backbone) {
        defaults: {
            'title': '',
            'description': '',
-            'breadcrumbs': null
+            'breadcrumbs': null,
+            'nav_aria_label': ''
        }
    });

--- a/lms/static/js/components/tabbed/views/tabbed_view.js
+++ b/lms/static/js/components/tabbed/views/tabbed_view.js
@@ -48,7 +48,11 @@
                           }));
                           self.$('.page-content-nav').append(tabEl);
                       });
-                       if(Backbone.history.getHash() === "") {
+                       // Re-display the default (first) tab if the
+                       // current route does not belong to one of the
+                       // tabs.  Otherwise continue displaying the tab
+                       // corresponding to the current URL.
+                       if (!(Backbone.history.getHash() in this.urlMap)) {
                           this.setActiveTab(0);
                       }
                       return this;
@@ -70,7 +74,7 @@
                       view.setElement(this.$('.page-content-main')).render();
                       this.$('.sr-is-focusable.sr-tab').focus();
                       if (this.router) {
-                           this.router.navigate(tab.url, {replace: true, trigger: true});
+                           this.router.navigate(tab.url, {replace: true});
                       }
                   },

--- a/lms/static/js/spec/components/header/header_spec.js
+++ b/lms/static/js/spec/components/header/header_spec.js
@@ -13,7 +13,7 @@

                   var testBreadcrumbs = function (breadcrumbs) {
                       model.set('breadcrumbs', breadcrumbs);
-                       expect(view.$el.html()).toContain('<nav class="breadcrumbs">');
+                       expect(view.$('nav.breadcrumbs').length).toBe(1);
                       _.each(view.$('.nav-item'), function (el, index) {
                           expect($(el).attr('href')).toEqual(breadcrumbs[index].url);
                           expect($(el).text()).toEqual(breadcrumbs[index].title);
--- a/lms/static/js/spec/components/tabbed/tabbed_view_spec.js
+++ b/lms/static/js/spec/components/tabbed/tabbed_view_spec.js
@@ -101,7 +101,7 @@
                           view.$('.nav-item[data-index=1]').click();
                           expect(Backbone.history.navigate).toHaveBeenCalledWith(
                               'test 2',
-                               {replace: true, trigger: true}
+                               {replace: true}
                           );
                       });

--- a/lms/static/sass/shared/_fields.scss
+++ b/lms/static/sass/shared/_fields.scss
@@ -118,7 +118,7 @@

    .u-field-message-help,
    .u-field-message-notification {
-        color: $gray-l1;
+        color: $gray;
    }
 }

--- a/lms/templates/components/card/card.underscore
+++ b/lms/templates/components/card/card.underscore
@@ -4,7 +4,12 @@
        <% if (pennant) { %>
            <small class="card-type"><%- pennant %></small>
        <% } %>
-        <h3 class="card-title"><%- title %></h3>
+        <h3 class="card-title"
+            <% if (!_.isUndefined(srInfo)) { %>
+                aria-describedby="<%= srInfo.id %>"
+            <% } %>
+            ><%- title %>
+        </h3>
        <p class="card-description"><%- description %></p>
    </div>
 </div>
@@ -21,7 +26,12 @@
        <% if (pennant) { %>
            <small class="card-type"><%- pennant %></small>
        <% } %>
-        <h3 class="card-title"><%- title %></h3>
+        <h3 class="card-title"
+            <% if (!_.isUndefined(srInfo)) { %>
+                aria-describedby="<%= srInfo.id %>"
+            <% } %>
+            ><%- title %>
+        </h3>
        <p class="card-description"><%- description %></p>
    </div>
    <div class="card-actions">
--- a/lms/templates/components/header/header.underscore
+++ b/lms/templates/components/header/header.underscore
@@ -1,7 +1,7 @@
 <header class="page-header has-secondary">
    <div class="page-header-main">
        <% if (breadcrumbs !== null && breadcrumbs.length > 0) { %>
-            <nav class="breadcrumbs">
+            <nav class="breadcrumbs" aria-label="<%- nav_aria_label %>">
            <% _.each(breadcrumbs, function (breadcrumb) { %>
                <a class="nav-item" href="<%= breadcrumb.url %>"><%- breadcrumb.title %></a>
                <span class="icon fa-angle-right" aria-hidden="true"></span>
--- a/lms/templates/components/tabbed/tab.underscore
+++ b/lms/templates/components/tabbed/tab.underscore
@@ -1 +1 @@
-<a class="nav-item" href="" data-url="<%= url %>" data-index="<%= index %>" role="tab" aria-selected="false"><%- title %></a>
+<a class="nav-item" href="" data-url="<%= url %>" data-index="<%= index %>" role="tab" aria-selected="false"><%= title %></a>