Merge pull request #29 from open-craft/presentation-updates

Presentation updates, part 2

.travis.yml

@@ -17,3 +17,5 @@ script:
     - python run_tests.py --with-coverage --cover-package=problem_builder
 notifications:
   email: false
+addons:
+  firefox: "36.0"

problem_builder/dashboard.py

@@ -30,6 +30,7 @@ import ast
 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 @@ class DashboardBlock(StudioEditableXBlockMixin, XBlock):
                 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 @@ class DashboardBlock(StudioEditableXBlockMixin, XBlock):
             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)

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

problem_builder/public/css/dashboard.css

 .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 {

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) {

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('<div class="title1">' + gettext('Feedback') + '</div>');
+            messagesDOM.prepend('<div class="title1">' + mentoring.data.feedback_label + '</div>');
             messagesDOM.show();
         }
 

problem_builder/templates/html/dashboard.html

@@ -42,30 +42,22 @@
           {% for mcq in block.mcqs %}
             <tr>
               <th class="desc">{{ mcq.display_name }}</th>
-              <td class="value"
-                  {% if mcq.color %} style="border-right-color: {{mcq.color}};"{% endif %}
-                  {% if not show_numbers %}
-                    {% if mcq.value %} aria-label="Score: {{mcq.value}}" {% else %} aria-label="{% trans 'No value yet' %}" {%endif%}
-                  {% endif %}
-              >
+              <td class="value" {% if mcq.color %} style="border-right-color: {{mcq.color}};"{% endif %}>
                 {% if mcq.value and show_numbers %}
-                  {{ mcq.value }}
+                  <span aria-hidden="true">{{ mcq.value }}</span>
                 {% endif %}
+                <span class="sr">{{ mcq.accessible_value }}</span>
               </td>
             </tr>
           {% endfor %}
           {% if block.has_average %}
            <tr class="avg-row">
               <th class="desc">{{ block.average_label }}</th>
-              <td class="value"
-                  {% if block.average_color %} style="border-right-color: {{block.average_color}};"{% endif %}
-                  {% if not show_numbers %}
-                    {% if block.average %} aria-label="Score: {{block.average|floatformat}}" {% else %} aria-label="{% trans 'No value yet' %}" {%endif%}
-                  {% endif %}
-              >
+              <td class="value" {% if block.average_color %} style="border-right-color: {{block.average_color}};"{% endif %}>
                 {% if show_numbers %}
-                  {{ block.average|floatformat }}
+                  <span aria-hidden="true">{{ block.average|floatformat }}</span>
                 {% endif %}
+                <span class="sr">{{ block.accessible_average }}</span>
               </td>
             </tr>
           {% endif %}

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}}
     </style>
   </head>

problem_builder/templates/html/mentoring.html

 {% load i18n %}
-<div class="mentoring themed-xblock" data-mode="{{ self.mode }}" data-step="{{ self.step }}">
+<div class="mentoring themed-xblock" data-mode="{{ self.mode }}" data-step="{{ self.step }}" data-feedback_label="{{ self.feedback_label}}">
   <div class="missing-dependency warning" data-missing="{{ self.has_missing_dependency }}">
     {% with url=missing_dependency_url|safe %}
     {% blocktrans with link_start="<a href='"|add:url|add:"'>" link_end="</a>" %}

problem_builder/tests/integration/test_dashboard.py

@@ -18,7 +18,9 @@
 # "AGPLv3".  If not, see <http://www.gnu.org/licenses/>.
 #
 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 @@ class TestDashboardBlock(SeleniumXBlockTest):
         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 @@ class TestDashboardBlock(SeleniumXBlockTest):
             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 @@ class TestDashboardBlock(SeleniumXBlockTest):
         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 @@ class TestDashboardBlock(SeleniumXBlockTest):
 
         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 @@ class TestDashboardBlock(SeleniumXBlockTest):
         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 @@ class TestDashboardBlock(SeleniumXBlockTest):
             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 @@ class TestDashboardBlock(SeleniumXBlockTest):
         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))

run_tests.py

@@ -10,6 +10,8 @@ because the workbench SDK's settings file is not inside any python module.
 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 @@ if __name__ == "__main__":
     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] != '-']