diff --git a/.travis.yml b/.travis.yml index 3dfd81e6..6c0587c4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,3 +17,5 @@ script: - python run_tests.py --with-coverage --cover-package=problem_builder notifications: email: false +addons: + firefox: "36.0" diff --git a/problem_builder/dashboard.py b/problem_builder/dashboard.py index a4494303..3c69e9db 100644 --- a/problem_builder/dashboard.py +++ b/problem_builder/dashboard.py @@ -30,6 +30,7 @@ import json import logging import operator as op +from django.template.defaultfilters import floatformat from .dashboard_visual import DashboardVisualData from .mcq import MCQBlock @@ -402,6 +403,7 @@ def student_view(self, context=None): # pylint: disable=unused-argument block['mcqs'].append({ "display_name": mcq_block.display_name_with_default, "value": value, + "accessible_value": _("Score: {score}").format(score=value) if value else _("No value yet"), "color": self.color_for_value(value) if value is not None else None, }) # If the values are numeric, display an average: @@ -412,6 +414,10 @@ def student_view(self, context=None): # pylint: disable=unused-argument if numeric_values: average_value = sum(numeric_values) / len(numeric_values) block['average'] = average_value + # average block is shown only if average value exists, so accessible text for no data is not required + block['accessible_average'] = _("Score: {score}").format( + score=floatformat(average_value) + ) block['average_label'] = self.average_labels.get(mentoring_block.url_name, _("Average")) block['has_average'] = True block['average_color'] = self.color_for_value(average_value) diff --git a/problem_builder/mentoring.py b/problem_builder/mentoring.py index 78b1b103..3b3022f6 100644 --- a/problem_builder/mentoring.py +++ b/problem_builder/mentoring.py @@ -131,6 +131,12 @@ class MentoringBlock(XBlock, StepParentMixin, StudioEditableXBlockMixin, StudioC default=_("Mentoring Questions"), scope=Scope.settings ) + feedback_label = String( + display_name=_("Feedback Header"), + help=_("Header for feedback messages"), + default=_("Feedback"), + scope=Scope.content + ) # User state attempted = Boolean( @@ -170,7 +176,7 @@ class MentoringBlock(XBlock, StepParentMixin, StudioEditableXBlockMixin, StudioC editable_fields = ( 'display_name', 'mode', 'followed_by', 'max_attempts', 'enforce_dependency', - 'display_submit', 'weight', + 'display_submit', 'feedback_label', 'weight', ) icon_class = 'problem' has_score = True diff --git a/problem_builder/public/css/dashboard.css b/problem_builder/public/css/dashboard.css index 249b5355..1e202d3c 100644 --- a/problem_builder/public/css/dashboard.css +++ b/problem_builder/public/css/dashboard.css @@ -1,6 +1,10 @@ .pb-dashboard table { max-width: 800px; + width: 700px; + table-layout: auto; border-collapse: collapse; + margin-left: auto; + margin-right: auto; margin-bottom: 15px; } @@ -9,6 +13,10 @@ font-weight: bold; } +.pb-dashboard .avg-row .desc { + font-weight: 600; +} + .pb-dashboard table td, .pb-dashboard table tbody th { border-top: 1px solid #ddd; border-bottom: 1px solid #ddd; @@ -24,7 +32,7 @@ min-width: 4em; text-align: right; padding-right: 5px; - border-right: 0.6em solid transparent; + border-right: 2em solid transparent; } .pb-dashboard table .avg-row td.desc { diff --git a/problem_builder/public/js/mentoring.js b/problem_builder/public/js/mentoring.js index 760beb32..dbbd345e 100644 --- a/problem_builder/public/js/mentoring.js +++ b/problem_builder/public/js/mentoring.js @@ -22,7 +22,8 @@ function MentoringBlock(runtime, element) { hideAllSteps: hideAllSteps, step: step, steps: steps, - publish_event: publish_event + publish_event: publish_event, + data: data }; function publish_event(data) { diff --git a/problem_builder/public/js/mentoring_standard_view.js b/problem_builder/public/js/mentoring_standard_view.js index ac45f0c2..50452c61 100644 --- a/problem_builder/public/js/mentoring_standard_view.js +++ b/problem_builder/public/js/mentoring_standard_view.js @@ -25,7 +25,7 @@ function MentoringStandardView(runtime, element, mentoring) { // Messages should only be displayed upon hitting 'submit', not on page reload mentoring.setContent(messagesDOM, results.message); if (messagesDOM.html().trim()) { - messagesDOM.prepend('
' + gettext('Feedback') + '
'); + messagesDOM.prepend('
' + mentoring.data.feedback_label + '
'); messagesDOM.show(); } diff --git a/problem_builder/templates/html/dashboard.html b/problem_builder/templates/html/dashboard.html index e938b6dd..857f9eee 100644 --- a/problem_builder/templates/html/dashboard.html +++ b/problem_builder/templates/html/dashboard.html @@ -42,30 +42,22 @@

{{display_name}}

{% for mcq in block.mcqs %} {{ mcq.display_name }} - + {% if mcq.value and show_numbers %} - {{ mcq.value }} + {% endif %} + {{ mcq.accessible_value }} {% endfor %} {% if block.has_average %} {{ block.average_label }} - + {% if show_numbers %} - {{ block.average|floatformat }} + {% endif %} + {{ block.accessible_average }} {% endif %} diff --git a/problem_builder/templates/html/dashboard_report.html b/problem_builder/templates/html/dashboard_report.html index cf29a6eb..fa1d6785 100644 --- a/problem_builder/templates/html/dashboard_report.html +++ b/problem_builder/templates/html/dashboard_report.html @@ -9,6 +9,20 @@ body { font-family: 'Open Sans', 'Helvetica Neue', Helvetica, Arial, sans-serif; } + .pb-dashboard table { + text-align: left; + } + /* screen reader class from edx-platform */ + .sr { + border: 0; + clip: rect(1px 1px 1px 1px); + height: 1px; + margin: -1px; + overflow: hidden; + padding: 0; + position: absolute; + width: 1px; + } {{css}} diff --git a/problem_builder/templates/html/mentoring.html b/problem_builder/templates/html/mentoring.html index df04e2b1..ac522305 100644 --- a/problem_builder/templates/html/mentoring.html +++ b/problem_builder/templates/html/mentoring.html @@ -1,5 +1,5 @@ {% load i18n %} -
+
{% with url=missing_dependency_url|safe %} {% blocktrans with link_start="" link_end="" %} diff --git a/problem_builder/tests/integration/test_dashboard.py b/problem_builder/tests/integration/test_dashboard.py index 856adbee..c016fc23 100644 --- a/problem_builder/tests/integration/test_dashboard.py +++ b/problem_builder/tests/integration/test_dashboard.py @@ -18,7 +18,9 @@ # "AGPLv3". If not, see . # from textwrap import dedent +from django.template.defaultfilters import floatformat from mock import Mock, patch +from selenium.common.exceptions import NoSuchElementException from xblockutils.base_test import SeleniumXBlockTest from xblockutils.resources import ResourceLoader @@ -112,6 +114,22 @@ def _install_fixture(self, dashboard_xml): self.go_to_view("student_view") self.vertical = self.load_root_xblock() + def _get_cell_contents(self, cell): + try: + visible_text = cell.find_element_by_css_selector('span:not(.sr)').text + except NoSuchElementException: + visible_text = "" + screen_reader_text = cell.find_element_by_css_selector('span.sr') + return visible_text, screen_reader_text.text + + def _assert_cell_contents(self, cell, expected_visible_text, expected_screen_reader_text): + visible_text, screen_reader_text = self._get_cell_contents(cell) + self.assertEqual(visible_text, expected_visible_text) + self.assertEqual(screen_reader_text, expected_screen_reader_text) + + def _format_sr_text(self, visible_text): + return "Score: {value}".format(value=visible_text) + def test_empty_dashboard(self): """ Test that when the student has not submitted any question answers, we still see @@ -129,8 +147,8 @@ def test_empty_dashboard(self): mcq_rows = step.find_elements_by_css_selector('tr') self.assertTrue(2 <= len(mcq_rows) <= 3) for mcq in mcq_rows: - value = mcq.find_element_by_css_selector('td:last-child') - self.assertEqual(value.text, '') + cell = mcq.find_element_by_css_selector('td:last-child') + self._assert_cell_contents(cell, '', 'No value yet') def _set_mentoring_values(self): pbs = self.browser.find_elements_by_css_selector('.mentoring') @@ -155,20 +173,23 @@ def test_dashboard(self): dashboard = self.browser.find_element_by_css_selector('.pb-dashboard') steps = dashboard.find_elements_by_css_selector('tbody') self.assertEqual(len(steps), 3) + expected_values = ('1', '2', '3', '4', 'B') for step_num, step in enumerate(steps): mcq_rows = step.find_elements_by_css_selector('tr:not(.avg-row)') self.assertTrue(2 <= len(mcq_rows) <= 3) for mcq in mcq_rows: - value = mcq.find_element_by_css_selector('td.value') - self.assertIn(value.text, ('1', '2', '3', '4', 'B')) + cell = mcq.find_element_by_css_selector('td.value') + visible_text, screen_reader_text = self._get_cell_contents(cell) + self.assertIn(visible_text, expected_values) + self.assertIn(screen_reader_text, map(self._format_sr_text, expected_values)) # Check the average: avg_row = step.find_element_by_css_selector('tr.avg-row') left_col = avg_row.find_element_by_css_selector('.desc') self.assertEqual(left_col.text, "Average") right_col = avg_row.find_element_by_css_selector('.value') expected_average = {0: "2", 1: "3", 2: "1"}[step_num] - self.assertEqual(right_col.text, expected_average) + self._assert_cell_contents(right_col, expected_average, self._format_sr_text(expected_average)) def test_dashboard_alternative(self): """ @@ -189,18 +210,25 @@ def test_dashboard_alternative(self): average_labels = ["Avg.", "Mean", "Second Quartile"] + expected_values = ('1', '2', '3', '4', 'B') + for step_num, step in enumerate(steps): mcq_rows = step.find_elements_by_css_selector('tr:not(.avg-row)') self.assertTrue(2 <= len(mcq_rows) <= 3) for mcq in mcq_rows: - value = mcq.find_element_by_css_selector('td.value') - self.assertEqual(value.text, '') + cell = mcq.find_element_by_css_selector('td.value') + visible_text, screen_reader_text = self._get_cell_contents(cell) + # this dashboard configured to not show numbers + self.assertEqual(visible_text, '') + # but screen reader content still added + self.assertIn(screen_reader_text, map(self._format_sr_text, expected_values)) # Check the average: avg_row = step.find_element_by_css_selector('tr.avg-row') left_col = avg_row.find_element_by_css_selector('.desc') self.assertEqual(left_col.text, average_labels[step_num]) right_col = avg_row.find_element_by_css_selector('.value') - self.assertEqual(right_col.text, "") + expected_average = {0: "2", 1: "3", 2: "1"}[step_num] + self._assert_cell_contents(right_col, '', self._format_sr_text(expected_average)) def test_dashboard_exclude_questions(self): """ @@ -214,6 +242,7 @@ def test_dashboard_exclude_questions(self): dashboard = self.browser.find_element_by_css_selector('.pb-dashboard') steps = dashboard.find_elements_by_css_selector('tbody') self.assertEqual(len(steps), 3) + expected_values = ('1', '2', '3', '4') lengths = [1, 2, 1] @@ -221,15 +250,17 @@ def test_dashboard_exclude_questions(self): mcq_rows = step.find_elements_by_css_selector('tr:not(.avg-row)') self.assertEqual(len(mcq_rows), lengths[step_num]) for mcq in mcq_rows: - value = mcq.find_element_by_css_selector('td.value') - self.assertIn(value.text, ('1', '2', '3', '4')) + cell = mcq.find_element_by_css_selector('td.value') + visible_text, screen_reader_text = self._get_cell_contents(cell) + self.assertIn(visible_text, expected_values) + self.assertIn(screen_reader_text, map(self._format_sr_text, expected_values)) # Check the average: avg_row = step.find_element_by_css_selector('tr.avg-row') left_col = avg_row.find_element_by_css_selector('.desc') self.assertEqual(left_col.text, "Average") right_col = avg_row.find_element_by_css_selector('.value') expected_average = {0: "1", 1: "3", 2: "1"}[step_num] - self.assertEqual(right_col.text, expected_average) + self._assert_cell_contents(right_col, expected_average, self._format_sr_text(expected_average)) def test_dashboard_malformed_exclude_questions(self): """ @@ -244,18 +275,22 @@ def test_dashboard_malformed_exclude_questions(self): steps = dashboard.find_elements_by_css_selector('tbody') self.assertEqual(len(steps), 3) + expected_values = ('1', '2', '3', '4') + lengths = [3, 2, 1] for step_num, step in enumerate(steps): mcq_rows = step.find_elements_by_css_selector('tr:not(.avg-row)') self.assertEqual(len(mcq_rows), lengths[step_num]) for mcq in mcq_rows: - value = mcq.find_element_by_css_selector('td.value') - self.assertIn(value.text, ('1', '2', '3', '4')) + cell = mcq.find_element_by_css_selector('td.value') + visible_text, screen_reader_text = self._get_cell_contents(cell) + self.assertIn(visible_text, expected_values) + self.assertIn(screen_reader_text, map(self._format_sr_text, expected_values)) # Check the average: avg_row = step.find_element_by_css_selector('tr.avg-row') left_col = avg_row.find_element_by_css_selector('.desc') self.assertEqual(left_col.text, "Average") right_col = avg_row.find_element_by_css_selector('.value') expected_average = {0: "2", 1: "3", 2: "1"}[step_num] - self.assertEqual(right_col.text, expected_average) + self._assert_cell_contents(right_col, expected_average, self._format_sr_text(expected_average)) diff --git a/run_tests.py b/run_tests.py index c4b17c38..8c7a8613 100755 --- a/run_tests.py +++ b/run_tests.py @@ -10,6 +10,8 @@ import os import sys +import logging + if __name__ == "__main__": # Use the workbench settings file: os.environ.setdefault("DJANGO_SETTINGS_MODULE", "workbench.settings") @@ -19,6 +21,8 @@ from django.conf import settings settings.INSTALLED_APPS += ("problem_builder", ) + logging.disable(logging.CRITICAL) + from django.core.management import execute_from_command_line args = sys.argv[1:] paths = [arg for arg in args if arg[0] != '-']