From 29dcbfefe46329765d33d706f6de2943de6d5dd1 Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Mon, 19 Sep 2016 16:00:02 -0400 Subject: [PATCH] Progress graph a11y updates. TNL-5891 --- common/test/acceptance/pages/lms/progress.py | 36 +++ .../tests/lms/test_progress_page.py | 122 ++++++++- lms/djangoapps/courseware/features/lti.py | 9 +- lms/templates/courseware/progress.html | 66 ++--- lms/templates/courseware/progress_graph.js | 242 +++++++++++++++--- 5 files changed, 390 insertions(+), 85 deletions(-) diff --git a/common/test/acceptance/pages/lms/progress.py b/common/test/acceptance/pages/lms/progress.py index 07fbcbac20..a08559b04c 100644 --- a/common/test/acceptance/pages/lms/progress.py +++ b/common/test/acceptance/pages/lms/progress.py @@ -74,6 +74,42 @@ class ProgressPage(CoursePage): """ return text in self.q(css=".view-in-course").html[0] + def x_tick_label(self, tick_index): + """ + Returns the label for the X-axis tick index, + and a boolean indicating whether or not it is aria-hidden + """ + selector = self.q(css='#grade-detail-graph .xAxis .tickLabel')[tick_index] + tick_label = selector.find_elements_by_tag_name('span')[0] + return [tick_label.text, tick_label.get_attribute('aria-hidden')] + + def x_tick_sr_text(self, tick_index): + """ + Return an array of the sr text for a specific x-Axis tick on the + progress chart. + """ + selector = self.q(css='#grade-detail-graph .tickLabel')[tick_index] + sr_fields = selector.find_elements_by_class_name('sr') + return [field.text for field in sr_fields] + + def y_tick_label(self, tick_index): + """ + Returns the label for the Y-axis tick index, + and a boolean indicating whether or not it is aria-hidden + """ + selector = self.q(css='#grade-detail-graph .yAxis .tickLabel')[tick_index] + tick_label = selector.find_elements_by_tag_name('span')[0] + return [tick_label.text, tick_label.get_attribute('aria-hidden')] + + def graph_overall_score(self): + """ + Returns the sr-only text for overall score on the progress chart, + and the complete text for overall score (including the same sr-text). + """ + selector = self.q(css='#grade-detail-graph .overallGrade')[0] + label = selector.find_elements_by_class_name('sr')[0] + return [label.text, selector.text] + def _chapter_index(self, title): """ Return the CSS index of the chapter with `title`. diff --git a/common/test/acceptance/tests/lms/test_progress_page.py b/common/test/acceptance/tests/lms/test_progress_page.py index dfc0087c15..ae419483e7 100644 --- a/common/test/acceptance/tests/lms/test_progress_page.py +++ b/common/test/acceptance/tests/lms/test_progress_page.py @@ -4,8 +4,8 @@ End-to-end tests for the LMS that utilize the progress page. """ from contextlib import contextmanager - import ddt +from nose.plugins.attrib import attr from ..helpers import ( UniqueCourseTest, auto_auth, create_multiple_choice_problem, create_multiple_choice_xml, get_modal_alert @@ -64,6 +64,13 @@ class ProgressPageBaseTest(UniqueCourseTest): XBlockFixtureDesc('sequential', self.SUBSECTION_NAME).add_children( XBlockFixtureDesc('vertical', self.UNIT_NAME).add_children(self.problem1, self.problem2) ) + ), + XBlockFixtureDesc('chapter', "Lab Section").add_children( + XBlockFixtureDesc('sequential', "Lab Subsection").add_children( + XBlockFixtureDesc('vertical', "Lab Unit").add_children( + create_multiple_choice_problem("Lab Exercise") + ) + ) ) ).install() @@ -268,16 +275,17 @@ class SubsectionGradingPolicyTest(ProgressPageBaseTest): """ def setUp(self): super(SubsectionGradingPolicyTest, self).setUp() - self._set_policy_for_subsection("Homework") + self._set_policy_for_subsection("Homework", 0) + self._set_policy_for_subsection("Lab", 1) - def _set_policy_for_subsection(self, policy): + def _set_policy_for_subsection(self, policy, section=0): """ - Set the grading policy for the - subsection in the test. + Set the grading policy for the first subsection in the specified section. + If a section index is not provided, 0 is assumed. """ with self._logged_in_session(staff=True): self.course_outline.visit() - modal = self.course_outline.section_at(0).subsection_at(0).edit() + modal = self.course_outline.section_at(section).subsection_at(0).edit() modal.policy = policy modal.save() @@ -290,6 +298,93 @@ class SubsectionGradingPolicyTest(ProgressPageBaseTest): self.assertEqual(self._get_section_score(), section_score) self.assertTrue(self.progress_page.text_on_page(text)) + def _check_tick_text(self, index, sr_text, label, label_hidden=True): + """ + Check the label and sr text for a horizontal (X-axis) tick. + """ + self.assertEqual(sr_text, self.progress_page.x_tick_sr_text(index)) + self.assertEqual([label, 'true' if label_hidden else None], self.progress_page.x_tick_label(index)) + + def test_axis_a11y(self): + """ + Tests that the progress chart axes have appropriate a11y (screenreader) markup. + """ + with self._logged_in_session(): + self.courseware_page.visit() + # Answer the first HW problem (the unit contains 2 problems, only one will be answered correctly) + self._answer_problem_correctly() + self.courseware_page.click_next_button_on_top() + # Answer the first Lab problem (unit only contains a single problem) + self._answer_problem_correctly() + self.progress_page.visit() + + # Verify that y-Axis labels are aria-hidden + self.assertEqual(['100%', 'true'], self.progress_page.y_tick_label(0)) + self.assertEqual(['0%', 'true'], self.progress_page.y_tick_label(1)) + self.assertEqual(['Pass 50%', 'true'], self.progress_page.y_tick_label(2)) + + # Verify x-Axis labels and sr-text + self._check_tick_text(0, [u'Homework 1 - Test Subsection 1 - 50% (1/2)'], u'HW 01') + + # Homeworks 2-10 are checked in the for loop below. + + self._check_tick_text( + 10, + [u'Homework 11 Unreleased - 0% (?/?)', u'The lowest 2 Homework scores are dropped.'], + u'HW 11' + ) + + self._check_tick_text( + 11, + [u'Homework 12 Unreleased - 0% (?/?)', u'The lowest 2 Homework scores are dropped.'], + u'HW 12' + ) + + self._check_tick_text(12, [u'Homework Average = 5%'], u'HW Avg') + self._check_tick_text(13, [u'Lab 1 - Lab Subsection - 100% (1/1)'], u'Lab 01') + + # Labs 2-10 are checked in the for loop below. + + self._check_tick_text( + 23, + [u'Lab 11 Unreleased - 0% (?/?)', u'The lowest 2 Lab scores are dropped.'], + u'Lab 11' + ) + self._check_tick_text( + 24, + [u'Lab 12 Unreleased - 0% (?/?)', u'The lowest 2 Lab scores are dropped.'], + u'Lab 12' + ) + + self._check_tick_text(25, [u'Lab Average = 10%'], u'Lab Avg') + self._check_tick_text(26, [u'Midterm Exam = 0%'], u'Midterm') + self._check_tick_text(27, [u'Final Exam = 0%'], u'Final') + + self._check_tick_text( + 28, + [u'Homework = 0.75% of a possible 15.00%', u'Lab = 1.50% of a possible 15.00%'], + u'Total', + False # The label "Total" should NOT be aria-hidden + ) + + # The grading policy has 12 Homeworks and 12 Labs. Most of them are unpublished, + # with no additional information. + for i in range(1, 10): + self._check_tick_text( + i, + [u'Homework {index} Unreleased - 0% (?/?)'.format(index=i + 1)], + u'HW 0{index}'.format(index=i + 1) if i < 9 else u'HW {index}'.format(index=i + 1) + ) + self._check_tick_text( + i + 13, + [u'Lab {index} Unreleased - 0% (?/?)'.format(index=i + 1)], + u'Lab 0{index}'.format(index=i + 1) if i < 9 else u'Lab {index}'.format(index=i + 1) + ) + + # Verify the overall score. The first element in the array is the sr-only text, and the + # second is the total text (including the sr-only text). + self.assertEqual(['Overall Score', 'Overall Score\n2%'], self.progress_page.graph_overall_score()) + def test_subsection_grading_policy_on_progress_page(self): with self._logged_in_session(): self._check_scores_and_page_text([(0, 1), (0, 1)], (0, 2), "Homework 1 - Test Subsection 1 - 0% (0/2)") @@ -306,5 +401,20 @@ class SubsectionGradingPolicyTest(ProgressPageBaseTest): self.assertFalse(self.progress_page.text_on_page("Homework 1 - Test Subsection 1")) self._set_policy_for_subsection("Homework") + with self._logged_in_session(): self._check_scores_and_page_text([(1, 1), (0, 1)], (1, 2), "Homework 1 - Test Subsection 1 - 50% (1/2)") + + +@attr('a11y') +class ProgressPageA11yTest(ProgressPageBaseTest): + """ + Class to test the accessibility of the progress page. + """ + + def test_progress_page_a11y(self): + """ + Test the accessibility of the progress page. + """ + self.progress_page.visit() + self.progress_page.a11y_audit.check_for_accessibility_errors() diff --git a/lms/djangoapps/courseware/features/lti.py b/lms/djangoapps/courseware/features/lti.py index 6901eb2249..97d1cdce95 100644 --- a/lms/djangoapps/courseware/features/lti.py +++ b/lms/djangoapps/courseware/features/lti.py @@ -332,14 +332,7 @@ def check_progress(_step, text): @step('I see graph with total progress "([^"]*)"$') def see_graph(_step, progress): - selector = 'grade-detail-graph' - xpath = '//div[@id="{parent}"]//div[text()="{progress}"]'.format( - parent=selector, - progress=progress, - ) - node = world.browser.find_by_xpath(xpath) - - assert node + assert_equal(progress, world.css_find('#grade-detail-graph .overallGrade').first.text.split('\n')[1]) @step('I see in the gradebook table that "([^"]*)" is "([^"]*)"$') diff --git a/lms/templates/courseware/progress.html b/lms/templates/courseware/progress.html index dc9061cc84..8dbe8565a6 100644 --- a/lms/templates/courseware/progress.html +++ b/lms/templates/courseware/progress.html @@ -1,3 +1,4 @@ +<%page expression_filter="h"/> <%inherit file="/main.html" /> <%namespace name='static' file='/static_content.html'/> <%def name="online_help_token()"><% return "progress" %> @@ -5,6 +6,7 @@ from course_modes.models import CourseMode from certificates.models import CertificateStatuses from django.utils.translation import ugettext as _ +from openedx.core.djangolib.markup import HTML, Text from django.core.urlresolvers import reverse from django.conf import settings from django.utils.http import urlquote_plus @@ -19,16 +21,19 @@ from django.utils.http import urlquote_plus <%namespace name="progress_graph" file="/courseware/progress_graph.js"/> -<%block name="pagetitle">${_("{course_number} Progress").format(course_number=course.display_number_with_default) | h} +<%block name="pagetitle">${_("{course_number} Progress").format(course_number=course.display_number_with_default)} <%block name="js_extra"> - - - - - + + + + + @@ -40,11 +45,11 @@ from django.utils.http import urlquote_plus
% if staff_access and studio_url is not None:
- ${_("View Grading in studio")} + ${_("View Grading in studio")}
% endif

- ${_("Course Progress for Student '{username}' ({email})").format(username=student.username, email=student.email) | h} + ${_("Course Progress for Student '{username}' ({email})").format(username=student.username, email=student.email)}

%if certificate_data: @@ -55,16 +60,16 @@ from django.utils.http import urlquote_plus
<% post_url = reverse('generate_user_cert', args=[unicode(course.id)]) %>
-

${certificate_data.title | h}

-

${certificate_data.msg | h}

+

${certificate_data.title}

+

${certificate_data.msg}

%if certificate_data.cert_web_view_url: - ${_("View Certificate")} ${_("Opens in a new browser window")} + ${_("View Certificate")} ${_("Opens in a new browser window")} %elif certificate_data.cert_status == CertificateStatuses.downloadable and certificate_data.download_url: - ${_("Download Your Certificate")} ${_("Opens in a new browser window")} + ${_("Download Your Certificate")} ${_("Opens in a new browser window")} %elif certificate_data.cert_status == CertificateStatuses.requesting: - + %endif
@@ -82,30 +87,31 @@ from django.utils.http import urlquote_plus

${_("Requirements for Course Credit")}

%if credit_course_requirements['eligibility_status'] == 'not_eligible': - ${_("{student_name}, you are no longer eligible for credit in this course.").format(student_name=student.profile.name) | h} + ${_("{student_name}, you are no longer eligible for credit in this course.").format(student_name=student.profile.name)} %elif credit_course_requirements['eligibility_status'] == 'eligible': - ${_("{student_name}, you have met the requirements for credit in this course.").format(student_name=student.profile.name) | h} - ${_("{a_start}Go to your dashboard{a_end} to purchase course credit.").format( - a_start=u"".format(url=reverse('dashboard')), - a_end="" + + ${Text(_("{student_name}, you have met the requirements for credit in this course. {a_start}Go to your dashboard{a_end} to purchase course credit.")).format( + student_name=student.profile.name, + a_start=HTML("").format(url=reverse('dashboard')), + a_end=HTML("") )} %elif credit_course_requirements['eligibility_status'] == 'partial_eligible': - ${_("{student_name}, you have not yet met the requirements for credit.").format(student_name=student.profile.name) | h} + ${_("{student_name}, you have not yet met the requirements for credit.").format(student_name=student.profile.name)} %endif - + ${_("Information about course credit requirements")}
-
+
%for requirement in credit_course_requirements['requirements']:
- ${_(requirement['display_name']) | h} + ${_(requirement['display_name'])} %if requirement['namespace'] == 'grade': - ${int(requirement['criteria']['min_grade'] * 100) | h}% + ${int(requirement['criteria']['min_grade'] * 100)}% %endif
@@ -143,7 +149,7 @@ from django.utils.http import urlquote_plus %for chapter in courseware_summary: %if not chapter['display_name'] == "hidden":
-

${ chapter['display_name'] | h}

+

${ chapter['display_name']}

%for section in chapter['sections']:
@@ -153,16 +159,16 @@ from django.utils.http import urlquote_plus percentageString = "{0:.0%}".format( float(earned)/total) if earned > 0 and total > 0 else "" %>
- - ${ section.display_name | h} + + ${ section.display_name} %if total > 0 or earned > 0: - ${_("{earned} of {total} possible points").format(earned='{:.3n}'.format(float(earned)), total='{:.3n}'.format(float(total))) | h} + ${_("{earned} of {total} possible points").format(earned='{:.3n}'.format(float(earned)), total='{:.3n}'.format(float(total)))} %endif %if total > 0 or earned > 0: - ${"({0:.3n}/{1:.3n}) {2}".format( float(earned), float(total), percentageString ) | h} + ${"({0:.3n}/{1:.3n}) {2}".format( float(earned), float(total), percentageString )} %endif
%if section.due is not None: @@ -174,7 +180,7 @@ from django.utils.http import urlquote_plus
${ _("Problem Scores: ") if section.graded else _("Practice Scores: ")}
%for score in section.scores: -
${"{0:.3n}/{1:.3n}".format(float(score.earned),float(score.possible)) | h}
+
${"{0:.3n}/{1:.3n}".format(float(score.earned),float(score.possible))}
%endfor
%else: diff --git a/lms/templates/courseware/progress_graph.js b/lms/templates/courseware/progress_graph.js index 104832be91..10605eec21 100644 --- a/lms/templates/courseware/progress_graph.js +++ b/lms/templates/courseware/progress_graph.js @@ -1,24 +1,41 @@ <%page args="grade_summary, grade_cutoffs, graph_div_id, show_grade_breakdown = True, show_grade_cutoffs = True, **kwargs"/> <%! - import json - import math + import bleach + import json + import math + + from openedx.core.djangolib.js_utils import ( + dump_js_escaped_json, js_escaped_string + ) %> $(function () { function showTooltip(x, y, contents) { - $('
' + contents + '
').css( { - position: 'absolute', - display: 'none', - top: y + 5, - left: x + 15, - border: '1px solid #000', - padding: '4px 6px', - color: '#fff', - 'background-color': '#333', - opacity: 0.90 - }).appendTo("body").fadeIn(200); - } + $("#tooltip").remove(); + var $tooltip_div = $('
').css({ + position: 'absolute', + display: 'none', + top: y + 5, + left: x + 15, + border: '1px solid #000', + padding: '4px 6px', + color: '#fff', + 'background-color': '#222', + opacity: 0.90 + }); + edx.HtmlUtils.setHtml( + $tooltip_div, + edx.HtmlUtils.HTML(contents) + ); + + edx.HtmlUtils.append( + $('body'), + edx.HtmlUtils.HTML($tooltip_div) + ); + + $('#tooltip').fadeIn(200); + } /* -------------------------------- Grade detail bars -------------------------------- */ <% @@ -46,17 +63,28 @@ $(function () { 'color' : colors[colorIndex]} categoryData = categories[ section['category'] ] - + + ## Because this is Python (Mako) embedded in JavaScript, our safe linting script is + ## thoroughly confused. We should rewrite this file to remove Python/Mako. + ## safe-lint: disable=javascript-jquery-append categoryData['data'].append( [tickIndex, section['percent']] ) - ticks.append( [tickIndex, section['label'] ] ) + + ## Note that some courses had stored images in the Abbreviation. We are no longer + ## allowing the display of such images, and remove any previously stored HTML + ## to prevent ugly HTML from being shown to learners. + ## safe-lint: disable=javascript-jquery-append + ticks.append( [tickIndex, bleach.clean(section['label'], tags=[], strip=True)] ) if section['category'] in detail_tooltips: + ## safe-lint: disable=javascript-jquery-append detail_tooltips[ section['category'] ].append( section['detail'] ) else: detail_tooltips[ section['category'] ] = [ section['detail'], ] if 'mark' in section: + ## safe-lint: disable=javascript-jquery-append droppedScores.append( [tickIndex, 0.05] ) + ## safe-lint: disable=javascript-jquery-append dropped_score_tooltips.append( section['mark']['detail'] ) tickIndex += 1 @@ -64,14 +92,14 @@ $(function () { if section.get('prominent', False): tickIndex += sectionSpacer - ## ----------------------------- Grade overviewew bar ------------------------- ## + ## ----------------------------- Grade overview bar ------------------------- ## tickIndex += sectionSpacer series = categories.values() overviewBarX = tickIndex extraColorIndex = len(categories) #Keeping track of the next color to use for categories not in categories[] - if show_grade_breakdown: + if show_grade_breakdown: for section in grade_summary['grade_breakdown']: if section['percent'] > 0: if section['category'] in categories: @@ -79,7 +107,7 @@ $(function () { else: color = colors[ extraColorIndex % len(colors) ] extraColorIndex += 1 - + ## safe-lint: disable=javascript-jquery-append series.append({ 'label' : section['category'] + "-grade_breakdown", 'data' : [ [overviewBarX, section['percent']] ], @@ -103,18 +131,73 @@ $(function () { descending_grades = sorted(grade_cutoffs, key=lambda x: grade_cutoffs[x], reverse=True) for grade in descending_grades: percent = grade_cutoffs[grade] + ## safe-lint: disable=javascript-jquery-append grade_cutoff_ticks.append( [ percent, u"{0} {1:.0%}".format(grade, percent) ] ) else: grade_cutoff_ticks = [ ] %> - var series = ${ json.dumps( series ) }; - var ticks = ${ json.dumps(ticks) }; - var bottomTicks = ${ json.dumps(bottomTicks) }; - var detail_tooltips = ${ json.dumps(detail_tooltips) }; - var droppedScores = ${ json.dumps(droppedScores) }; - var grade_cutoff_ticks = ${ json.dumps(grade_cutoff_ticks) } + var series = ${ series | n, dump_js_escaped_json }; + var ticks = ${ ticks | n, dump_js_escaped_json }; + var bottomTicks = ${ bottomTicks | n, dump_js_escaped_json }; + var detail_tooltips = ${ detail_tooltips | n, dump_js_escaped_json }; + var droppedScores = ${ droppedScores | n, dump_js_escaped_json }; + var grade_cutoff_ticks = ${ grade_cutoff_ticks | n, dump_js_escaped_json } + var yAxisTooltips={}; + + /* + series looks like: + [ + { + color: "#600101", + label: "Homework", + data: [[1, 0.06666666666666667], [2, 1], [3.25, .53]] + }, + ... + ] + + detail_tooltips looks like: + { + "Dropped Scores": [0: "The lowest 1...:], + "Homework": [ + 0: "Homework 1 -- Homework -- Question Styles 7% (1/15)", + 1: "Homework 2 -- Homework -- Get Social 100% (1/1)", + 2: "Homework Average = 53%" + ], + ... + } + */ + + // loop through the series and extract the matching tick and the series label + for (var seriesIndex = 0; seriesIndex < series.length; seriesIndex++) { + for (var dataIndex = 0; dataIndex < series[seriesIndex]['data'].length; dataIndex++) { + var tickIndex = series[seriesIndex]['data'][dataIndex][0]; + // There may be more than one detail tooltip for a given tickIndex. If so, + // push the new tooltip on the existing list. + if (tickIndex in yAxisTooltips) { + yAxisTooltips[tickIndex].push(detail_tooltips[series[seriesIndex]['label']][dataIndex]); + } else { + yAxisTooltips[tickIndex] = [detail_tooltips[series[seriesIndex]['label']][dataIndex]]; + } + // If this item was a dropped score, add the tooltip message about that. + for (var droppedIndex = 0; droppedIndex < droppedScores.length; droppedIndex++) { + if (tickIndex === droppedScores[droppedIndex][0]) { + yAxisTooltips[tickIndex].push(detail_tooltips["Dropped Scores"][droppedIndex]); + } + } + } + } + + // hide the vertical axis since they are audibly lacking context + for (var i = 0; i < grade_cutoff_ticks.length; i++) { + grade_cutoff_ticks[i][1] = edx.HtmlUtils.joinHtml( + edx.HtmlUtils.HTML('') + ).text; + } + //Always be sure that one series has the xaxis set to 2, or the second xaxis labels won't show up series.push( {label: 'Dropped Scores', data: droppedScores, points: {symbol: "cross", show: true, radius: 3}, bars: {show: false}, color: "#333"} ); @@ -128,40 +211,117 @@ $(function () { markings.push({yaxis: {from: ascending_grades[i], to: ascending_grades[i+1]}, color: colors[(i-1) % colors.length]}); var options = { - series: {stack: true, - lines: {show: false, steps: false }, - bars: {show: true, barWidth: 0.8, align: 'center', lineWidth: 0, fill: .8 },}, - xaxis: {tickLength: 0, min: 0.0, max: ${tickIndex - sectionSpacer}, ticks: ticks, labelAngle: 90}, - yaxis: {ticks: grade_cutoff_ticks, min: 0.0, max: 1.0, labelWidth: 100}, - grid: { hoverable: true, clickable: true, borderWidth: 1, markings: markings }, - legend: {show: false}, + series: { + stack: true, + lines: { + show: false, + steps: false + }, + bars: { + show: true, + barWidth: 0.8, + align: 'center', + lineWidth: 0, + fill: .8 + } + }, + xaxis: { + tickLength: 0, + min: 0.0, + max: ${tickIndex - sectionSpacer | n, dump_js_escaped_json}, + ticks: function() { + for (var i = 0; i < ticks.length; i++) { + var tickLabel = edx.HtmlUtils.joinHtml( + // The very last tick will be for the total, and it usually is composed of a number of different + // grading types. To help clarify, do NOT make the label ("Total") aria-hidden in that case. + edx.HtmlUtils.HTML(i < ticks.length - 1 ? '