From 26bbbb967e8533de405cfcfa46809157ed1e2b3f Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Mon, 3 Aug 2015 14:18:31 -0400 Subject: [PATCH] Fix various team accessibility issues. Authors: - Peter Fogg - Daniel Friedman TNL-1953 --- .../js/components/views/paginated_view.js | 18 ++++++++++++- .../js/components/views/paging_header.js | 6 ++++- .../components/paginated-view.underscore | 2 +- .../components/paging-header.underscore | 3 +++ common/test/acceptance/pages/lms/fields.py | 5 ++-- .../test/acceptance/tests/lms/test_teams.py | 9 ++++--- .../teams/js/spec/views/teams_tab_spec.js | 6 +++++ .../teams/static/teams/js/views/edit_team.js | 12 +++------ .../teams/static/teams/js/views/team_card.js | 6 ++--- .../teams/static/teams/js/views/teams.js | 12 ++++++++- .../teams/static/teams/js/views/teams_tab.js | 25 ++++++++++++++++--- .../teams/static/teams/js/views/topic_card.js | 5 ++-- .../teams/static/teams/js/views/topics.js | 10 +++++++- .../teams/templates/edit-team.underscore | 12 +++++++++ lms/static/js/components/card/views/card.js | 5 +++- .../js/components/header/models/header.js | 3 ++- .../js/components/tabbed/views/tabbed_view.js | 8 ++++-- .../js/spec/components/header/header_spec.js | 2 +- .../components/tabbed/tabbed_view_spec.js | 2 +- lms/static/sass/shared/_fields.scss | 2 +- lms/templates/components/card/card.underscore | 14 +++++++++-- .../components/header/header.underscore | 2 +- .../components/tabbed/tab.underscore | 2 +- 23 files changed, 129 insertions(+), 42 deletions(-) diff --git a/common/static/common/js/components/views/paginated_view.js b/common/static/common/js/components/views/paginated_view.js index 405128b3b3..0507d52cbd 100644 --- 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() { diff --git a/common/static/common/js/components/views/paging_header.js b/common/static/common/js/components/views/paging_header.js index 3bed5147b1..bd5e741778 100644 --- 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; } }); diff --git a/common/static/common/templates/components/paginated-view.underscore b/common/static/common/templates/components/paginated-view.underscore index 09ea0a13b4..7a01eff6a5 100644 --- a/common/static/common/templates/components/paginated-view.underscore +++ b/common/static/common/templates/components/paginated-view.underscore @@ -1,4 +1,4 @@
-
+
diff --git a/common/static/common/templates/components/paging-header.underscore b/common/static/common/templates/components/paging-header.underscore index 6d2011a8d1..04a5d60e44 100644 --- 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)) { %> +

<%- srInfo.text %>

+<% } %>
<%= message %> diff --git a/common/test/acceptance/pages/lms/fields.py b/common/test/acceptance/pages/lms/fields.py index 754b692955..d1564efde9 100644 --- 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): diff --git a/common/test/acceptance/tests/lms/test_teams.py b/common/test/acceptance/tests/lms/test_teams.py index 7c5bacbcd9..352ccdc95e 100644 --- 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() diff --git 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 index 85456c428d..10b4530dbc 100644 --- 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(); diff --git a/lms/djangoapps/teams/static/teams/js/views/edit_team.js b/lms/djangoapps/teams/static/teams/js/views/edit_team.js index 9c7597ee3f..556cecbf38 100644 --- 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(); diff --git a/lms/djangoapps/teams/static/teams/js/views/team_card.js b/lms/djangoapps/teams/static/teams/js/views/team_card.js index 68eb780496..3578523572 100644 --- 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; diff --git a/lms/djangoapps/teams/static/teams/js/views/teams.js b/lms/djangoapps/teams/static/teams/js/views/teams.js index 8136899342..680294957d 100644 --- 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); }, diff --git a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js index fc5b427aa3..60494ce9c5 100644 --- 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": '', "sr_end": ''}, 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 diff --git a/lms/djangoapps/teams/static/teams/js/views/topic_card.js b/lms/djangoapps/teams/static/teams/js/views/topic_card.js index e280236833..c64ebcd619 100644 --- 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', diff --git a/lms/djangoapps/teams/static/teams/js/views/topics.js b/lms/djangoapps/teams/static/teams/js/views/topics.js index ab727bc9ad..7217e42a36 100644 --- 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); } }); diff --git a/lms/djangoapps/teams/static/teams/templates/edit-team.underscore b/lms/djangoapps/teams/static/teams/templates/edit-team.underscore index 59384f86d1..5fbf6bf633 100644 --- a/lms/djangoapps/teams/static/teams/templates/edit-team.underscore +++ b/lms/djangoapps/teams/static/teams/templates/edit-team.underscore @@ -1,3 +1,4 @@ +
+
diff --git a/lms/static/js/components/card/views/card.js b/lms/static/js/components/card/views/card.js index e236b6b3b1..def119d349 100644 --- 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) { diff --git a/lms/static/js/components/header/models/header.js b/lms/static/js/components/header/models/header.js index 55eb00c31d..bf1bbb835e 100644 --- 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': '' } }); diff --git a/lms/static/js/components/tabbed/views/tabbed_view.js b/lms/static/js/components/tabbed/views/tabbed_view.js index 5d5223b0ff..9258f9cd0b 100644 --- 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}); } }, diff --git a/lms/static/js/spec/components/header/header_spec.js b/lms/static/js/spec/components/header/header_spec.js index a41d4d0fb5..7df8ce8ba6 100644 --- 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('
@@ -21,7 +26,12 @@ <% if (pennant) { %> <%- pennant %> <% } %> -

<%- title %>

+

+ aria-describedby="<%= srInfo.id %>" + <% } %> + ><%- title %> +

<%- description %>

diff --git a/lms/templates/components/header/header.underscore b/lms/templates/components/header/header.underscore index 74e48ae79d..3f989b9558 100644 --- a/lms/templates/components/header/header.underscore +++ b/lms/templates/components/header/header.underscore @@ -1,7 +1,7 @@
<% if (breadcrumbs !== null && breadcrumbs.length > 0) { %> -