Merge pull request #112 from open-craft/mtyaka/a11y-interaction-issues

a11y Interaction Improvements

.travis.yml

@@ -12,7 +12,7 @@ install:
     - "pip install selenium==2.53.0"
     - "pip uninstall -y xblock-drag-and-drop-v2"
     - "python setup.py sdist"
-    - "pip install dist/xblock-drag-and-drop-v2-2.0.13.tar.gz"
+    - "pip install dist/xblock-drag-and-drop-v2-2.0.14.tar.gz"
 script:
     - pep8 drag_and_drop_v2 tests --max-line-length=120
     - pylint drag_and_drop_v2

Changelog.md

+Version 2.0.14 (2017-01-17)
+---------------------------
+
+* Various accessibility improvements (PRs #110, #111, #112)
+
+Version 2.0.13 (2017-01-02)
+---------------------------
+
+* i18n improvements (PR #113)
+
 Version 2.0.12 (2016-11-08)
 ---------------------------
 

drag_and_drop_v2/templates/html/drag_and_drop.html

 {% load i18n %}
-<section class="themed-xblock xblock--drag-and-drop">
-    <i class="fa fa-spin fa-spinner initial-load-spinner"></i>{% trans "Loading drag and drop problem." %}
-</section>
+<div class="themed-xblock xblock--drag-and-drop">
+  <span class="fa fa-spin fa-spinner initial-load-spinner" aria-hidden="true"></span>
+  {% trans "Loading drag and drop problem." %}
+</div>

drag_and_drop_v2/templates/html/drag_and_drop_edit.html

@@ -4,10 +4,12 @@
 <div class="xblock--drag-and-drop--editor editor-with-buttons">
     {{ js_templates|safe }}
 
-    <section class="drag-builder">
-        <div class="tab feedback-tab">
-
-            <section class="tab-content">
+    <div class="drag-builder">
+        <section class="tab feedback-tab">
+            <header class="tab-header">
+                <h3>{% trans "Basic Settings" %}</h3>
+            </header>
+            <div class="tab-content">
                 <form class="feedback-form">
                     <label class="h4">
                         <span>{% trans fields.display_name.display_name %}</span>
@@ -80,14 +82,14 @@
                         <textarea class="final-feedback">{{ self.data.feedback.finish }}</textarea>
                     </label>
                 </form>
-            </section>
-        </div>
+            </div>
+        </section>
 
-        <div class="tab zones-tab hidden">
+        <section class="tab zones-tab hidden">
             <header class="tab-header">
                 <h3>{% trans "Zones" %}</h3>
             </header>
-            <section class="tab-content">
+            <div class="tab-content">
                 <form class="target-image-form">
                     <label class="h4" for="background-url-{{id_suffix}}">
                         <span>{% trans "Background URL" %}</span>
@@ -114,8 +116,8 @@
                         {% endblocktrans %}
                     </div>
                 </form>
-            </section>
-            <section class="tab-content">
+            </div>
+            <div class="tab-content">
                 <form class="display-labels-form">
                     <h4>{% trans "Zone labels" %}</h4>
                     <label class="checkbox-label">
@@ -130,8 +132,8 @@
                         <span>{% trans "Display zone borders on the image" %}</span>
                     </label>
                 </form>
-            </section>
-            <section class="tab-content">
+            </div>
+            <div class="tab-content">
                 <h4>{% trans "Zone definitions" %}</h4>
                 <div class="zone-editor">
                     <div class="controls">
@@ -147,14 +149,14 @@
                         </div>
                     </div>
                 </div>
-            </section>
-        </div>
+            </div>
+        </section>
 
-        <div class="tab items-tab hidden">
+        <section class="tab items-tab hidden">
             <header class="tab-header">
                 <h3>{% trans "Items" %}</h3>
             </header>
-            <section class="tab-content">
+            <div class="tab-content">
                 <form class="item-styles-form">
                     <label class="h4">
                         <span>{% trans fields.item_background_color.display_name %}</span>
@@ -184,18 +186,18 @@
                       {% trans fields.max_items_per_zone.help %}
                     </div>
                 </form>
-            </section>
-            <section class="tab-content">
+            </div>
+            <div class="tab-content">
                 <h4>{% trans "Item definitions" %}</h4>
                 <form class="items-form"></form>
                 <button class="btn add-item add-element">
                     <span class="icon add" aria-hidden="true"></span>
                     {% trans "Add an item" %}
                 </button>
-            </section>
-        </div>
+            </div>
+        </section>
 
-    </section>
+    </div>
 
     <div class="xblock-actions">
       <ul class="action-buttons">

drag_and_drop_v2/translations/en/LC_MESSAGES/text.po

@@ -595,6 +595,31 @@ msgid_plural "{possible} points possible (ungraded)"
 msgstr[0] ""
 msgstr[1] ""
 
+#: drag_and_drop_v2/public/js/drag_and_drop.js:374
+#: drag_and_drop_v2/public/js/drag_and_drop.js:839
+msgid "Hints:"
+msgstr ""
+
+#: drag_and_drop_v2/public/js/drag_and_drop.js:224
+msgid ", dropzone"
+msgstr ""
+
+#: drag_and_drop_v2/public/js/drag_and_drop.js:479
+msgid "Drag and Drop Problem"
+msgstr ""
+
+#: drag_and_drop_v2/public/js/drag_and_drop.js:509
+msgid "Drop Targets"
+msgstr ""
+
+#: drag_and_drop_v2/public/js/drag_and_drop.js:517
+msgid "Actions"
+msgstr ""
+
+#: drag_and_drop_v2/public/js/drag_and_drop.js:12
+msgid "Submitting"
+msgstr ""
+
 #: utils.py:44
 msgid "Your highest score is {score}"
 msgstr ""

drag_and_drop_v2/translations/eo/LC_MESSAGES/text.po

@@ -694,6 +694,31 @@ msgstr[1] "{possible} pöïnts pössïßlé (üngrädéd) Ⱡ'σяєм ιρѕυ
 msgid "Close"
 msgstr "Çlösé Ⱡ'σяєм ιρѕ#"
 
+#: drag_and_drop_v2/public/js/drag_and_drop.js:374
+#: drag_and_drop_v2/public/js/drag_and_drop.js:839
+msgid "Hints:"
+msgstr "Hïnts: Ⱡ'σяєм ιρѕυ#"
+
+#: drag_and_drop_v2/public/js/drag_and_drop.js:224
+msgid ", dropzone"
+msgstr ", dröpzöné Ⱡ'σяєм ιρѕυм ∂σłσ#"
+
+#: drag_and_drop_v2/public/js/drag_and_drop.js:479
+msgid "Drag and Drop Problem"
+msgstr "Dräg änd Dröp Prößlém Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, #"
+
+#: drag_and_drop_v2/public/js/drag_and_drop.js:509
+msgid "Drop Targets"
+msgstr "Dröp Tärgéts Ⱡ'σяєм ιρѕυм ∂σłσя ѕ#"
+
+#: drag_and_drop_v2/public/js/drag_and_drop.js:517
+msgid "Actions"
+msgstr "Àçtïöns Ⱡ'σяєм ιρѕυм #"
+
+#: drag_and_drop_v2/public/js/drag_and_drop.js:12
+msgid "Submitting"
+msgstr "Süßmïttïng Ⱡ'σяєм ιρѕυм ∂σłσ#"
+
 #: utils.py:44
 msgid "Your highest score is {score}"
 msgstr "Ýöür hïghést sçöré ïs {score} Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕ#"

setup.py

@@ -23,7 +23,7 @@ def package_data(pkg, root_list):
 
 setup(
     name='xblock-drag-and-drop-v2',
-    version='2.0.13',
+    version='2.0.14',
     description='XBlock - Drag-and-Drop v2',
     packages=['drag_and_drop_v2'],
     install_requires=[

tests/integration/test_base.py

@@ -48,7 +48,7 @@ ItemDefinition = namedtuple(  # pylint: disable=invalid-name
 
 
 class BaseIntegrationTest(SeleniumBaseTest):
-    default_css_selector = 'section.themed-xblock.xblock--drag-and-drop'
+    default_css_selector = '.themed-xblock.xblock--drag-and-drop'
     module_name = __name__
 
     _additional_escapes = {
@@ -239,22 +239,32 @@ class DefaultDataTestMixin(object):
 class InteractionTestBase(object):
     POPUP_ERROR_CLASS = "popup-incorrect"
 
-    @classmethod
-    def _get_items_with_zone(cls, items_map):
+    def setUp(self):
+        super(InteractionTestBase, self).setUp()
+
+        scenario_xml = self._get_scenario_xml()
+        self._add_scenario(self.PAGE_ID, self.PAGE_TITLE, scenario_xml)
+        self._page = self.go_to_page(self.PAGE_TITLE)
+        # Resize window so that the entire drag container is visible.
+        # Selenium has issues when dragging to an area that is off screen.
+        self.browser.set_window_size(1024, 1024)
+
+    @staticmethod
+    def _get_items_with_zone(items_map):
         return {
             item_key: definition for item_key, definition in items_map.items()
             if definition.zone_ids != []
         }
 
-    @classmethod
-    def _get_items_without_zone(cls, items_map):
+    @staticmethod
+    def _get_items_without_zone(items_map):
         return {
             item_key: definition for item_key, definition in items_map.items()
             if definition.zone_ids == []
         }
 
-    @classmethod
-    def _get_items_by_zone(cls, items_map):
+    @staticmethod
+    def _get_items_by_zone(items_map):
         zone_ids = set([definition.zone_ids[0] for _, definition in items_map.items() if definition.zone_ids])
         return {
             zone_id: {item_key: definition for item_key, definition in items_map.items()
@@ -262,15 +272,17 @@ class InteractionTestBase(object):
             for zone_id in zone_ids
         }
 
-    def setUp(self):
-        super(InteractionTestBase, self).setUp()
-
-        scenario_xml = self._get_scenario_xml()
-        self._add_scenario(self.PAGE_ID, self.PAGE_TITLE, scenario_xml)
-        self._page = self.go_to_page(self.PAGE_TITLE)
-        # Resize window so that the entire drag container is visible.
-        # Selenium has issues when dragging to an area that is off screen.
-        self.browser.set_window_size(1024, 800)
+    @staticmethod
+    def _get_incorrect_zone_for_item(item, zones):
+        """Returns the first zone that is not correct for this item."""
+        zone_id = None
+        zone_title = None
+        for z_id, z_title in zones:
+            if z_id not in item.zone_ids:
+                zone_id = z_id
+                zone_title = z_title
+                break
+        return [zone_id, zone_title]
 
     def _get_item_by_value(self, item_value):
         return self._page.find_elements_by_xpath(".//div[@data-value='{item_id}']".format(item_id=item_value))[0]
@@ -312,7 +324,7 @@ class InteractionTestBase(object):
         both the HTML attribute and the DOM property are set to false.
         We work around that selenium bug by using JavaScript to get the correct value of 'draggable'.
         """
-        script = "return $('div.option[data-value={}]').prop('draggable')".format(item_value)
+        script = "return $('.option[data-value={}]').prop('draggable')".format(item_value)
         return self.browser.execute_script(script)
 
     def assertDraggable(self, item_value):
@@ -370,7 +382,7 @@ class InteractionTestBase(object):
         item.send_keys("")
         item.send_keys(action_key)
         # Focus is on first *zone* now
-        self.assert_grabbed_item(item)
+        self.assert_item_grabbed(item)
         # Get desired zone and figure out how many times we have to press Tab to focus the zone.
         if zone_id is None:  # moving back to the bank
             zone = self._get_item_bank()
@@ -387,9 +399,12 @@ class InteractionTestBase(object):
             ActionChains(self.browser).send_keys(Keys.TAB).perform()
         zone.send_keys(action_key)
 
-    def assert_grabbed_item(self, item):
+    def assert_item_grabbed(self, item):
         self.assertEqual(item.get_attribute('aria-grabbed'), 'true')
 
+    def assert_item_not_grabbed(self, item):
+        self.assertEqual(item.get_attribute('aria-grabbed'), 'false')
+
     def assert_placed_item(self, item_value, zone_title, assessment_mode=False):
         item = self._get_placed_item_by_value(item_value)
         self.wait_until_visible(item)
@@ -474,3 +489,10 @@ class InteractionTestBase(object):
 
     def assert_button_enabled(self, submit_button, enabled=True):
         self.assertEqual(submit_button.is_enabled(), enabled)
+
+    def assert_reader_feedback_messages(self, expected_message_lines):
+        expected_paragraphs = ['<p>{}</p>'.format(l) for l in expected_message_lines]
+        expected_html = ''.join(expected_paragraphs)
+        feedback_area = self._page.find_element_by_css_selector('.reader-feedback-area')
+        actual_html = feedback_area.get_attribute('innerHTML')
+        self.assertEqual(actual_html, expected_html)

tests/integration/test_events.py

@@ -76,7 +76,7 @@ class EventsFiredTest(DefaultDataTestMixin, ParameterizedTestsMixin, BaseEventsT
     @data(*enumerate(scenarios))  # pylint: disable=star-args
     @unpack
     def test_event(self, index, event):
-        self.parameterized_item_positive_feedback_on_good_move(self.items_map)
+        self.parameterized_item_positive_feedback_on_good_move_standard(self.items_map)
         dummy, name, published_data = self.publish.call_args_list[index][0]
         self.assertEqual(name, event['name'])
         self.assertEqual(published_data, event['data'])

tests/integration/test_interaction_assessment.py

@@ -81,14 +81,12 @@ class AssessmentInteractionTest(
     """
     @data(*ITEM_DRAG_KEYBOARD_KEYS)
     def test_item_no_feedback_on_good_move(self, action_key):
-        self.parameterized_item_positive_feedback_on_good_move(
-            self.items_map, action_key=action_key, assessment_mode=True
-        )
+        self.parameterized_item_positive_feedback_on_good_move_assessment(self.items_map, action_key=action_key)
 
     @data(*ITEM_DRAG_KEYBOARD_KEYS)
     def test_item_no_feedback_on_bad_move(self, action_key):
-        self.parameterized_item_negative_feedback_on_bad_move(
-            self.items_map, self.all_zones, action_key=action_key, assessment_mode=True
+        self.parameterized_item_negative_feedback_on_bad_move_assessment(
+            self.items_map, self.all_zones, action_key=action_key
         )
 
     @data(*ITEM_DRAG_KEYBOARD_KEYS)
@@ -250,6 +248,18 @@ class AssessmentInteractionTest(
         """
         Test updating overall feedback after submitting solution in assessment mode
         """
+        def check_feedback(overall_feedback_lines, per_item_feedback_lines=None):
+            # Check that the feedback is correctly displayed in the overall feedback area.
+            expected_overall_feedback = "\n".join(["FEEDBACK"] + overall_feedback_lines)
+            self.assertEqual(self._get_feedback().text, expected_overall_feedback)
+
+            # Check that the SR.readText function was passed correct feedback messages.
+            sr_feedback_lines = overall_feedback_lines
+            if per_item_feedback_lines:
+                sr_feedback_lines += ["Some of your answers were not correct.", "Hints:"]
+                sr_feedback_lines += per_item_feedback_lines
+            self.assert_reader_feedback_messages(sr_feedback_lines)
+
         # used keyboard mode to avoid bug/feature with selenium "selecting" everything instead of dragging an element
         self.place_item(0, TOP_ZONE_ID, Keys.RETURN)
 
@@ -261,29 +271,25 @@ class AssessmentInteractionTest(
         expected_grade = 2.0 / 5.0
 
         feedback_lines = [
-            "FEEDBACK",
             FeedbackMessages.correctly_placed(1),
             FeedbackMessages.not_placed(3),
             START_FEEDBACK,
             FeedbackMessages.GRADE_FEEDBACK_TPL.format(score=expected_grade)
         ]
-        expected_feedback = "\n".join(feedback_lines)
-        self.assertEqual(self._get_feedback().text, expected_feedback)
+        check_feedback(feedback_lines)
 
         # Place the item into incorrect zone. The score does not change.
         self.place_item(1, BOTTOM_ZONE_ID, Keys.RETURN)
         self.click_submit()
 
         feedback_lines = [
-            "FEEDBACK",
             FeedbackMessages.correctly_placed(1),
             FeedbackMessages.misplaced_returned(1),
             FeedbackMessages.not_placed(2),
             START_FEEDBACK,
             FeedbackMessages.GRADE_FEEDBACK_TPL.format(score=expected_grade)
         ]
-        expected_feedback = "\n".join(feedback_lines)
-        self.assertEqual(self._get_feedback().text, expected_feedback)
+        check_feedback(feedback_lines, ["No, this item does not belong here. Try again."])
 
         # reach final attempt
         for _ in xrange(self.MAX_ATTEMPTS-3):
@@ -299,13 +305,11 @@ class AssessmentInteractionTest(
         expected_grade = 1.0
 
         feedback_lines = [
-            "FEEDBACK",
             FeedbackMessages.correctly_placed(4),
             FINISH_FEEDBACK,
             FeedbackMessages.FINAL_ATTEMPT_TPL.format(score=expected_grade)
         ]
-        expected_feedback = "\n".join(feedback_lines)
-        self.assertEqual(self._get_feedback().text, expected_feedback)
+        check_feedback(feedback_lines)
 
     def test_per_item_feedback_multiple_misplaced(self):
         self.place_item(0, MIDDLE_ZONE_ID, Keys.RETURN)

tests/integration/test_render.py

@@ -194,7 +194,7 @@ class TestDragAndDropRender(BaseIntegrationTest):
                 '#-Zone_{}'.format(zone_number), **zone_box_percentages
             )
             zone_name = zone.find_element_by_css_selector('p.zone-name')
-            self.assertEqual(zone_name.text, 'Zone {}'.format(zone_number))
+            self.assertEqual(zone_name.text, 'Zone {}\n, dropzone'.format(zone_number))
             zone_description = zone.find_element_by_css_selector('p.zone-description')
             self.assertEqual(zone_description.text, 'This describes zone {}'.format(zone_number))
             # Zone description should only be visible to screen readers:
@@ -204,12 +204,10 @@ class TestDragAndDropRender(BaseIntegrationTest):
         self.load_scenario()
 
         popup = self._get_popup()
-        popup_wrapper = self._get_popup_wrapper()
         popup_content = self._get_popup_content()
         self.assertFalse(popup.is_displayed())
         self.assertIn('popup', popup.get_attribute('class'))
         self.assertEqual(popup_content.text, "")
-        self.assertEqual(popup_wrapper.get_attribute('aria-live'), 'polite')
 
     @data(None, Keys.RETURN)
     def test_go_to_beginning_button(self, action_key):
@@ -253,9 +251,7 @@ class TestDragAndDropRender(BaseIntegrationTest):
     def test_feedback(self):
         self.load_scenario()
 
-        feedback = self._get_feedback()
         feedback_message = self._get_feedback_message()
-        self.assertEqual(feedback.get_attribute('aria-live'), 'polite')
         self.assertEqual(feedback_message.text, START_FEEDBACK)
 
     def test_background_image(self):

tests/integration/test_title_and_question.py

@@ -26,10 +26,10 @@ class TestDragAndDropTitleAndProblem(BaseIntegrationTest):
         self.addCleanup(scenarios.remove_scenario, const_page_id)
 
         page = self.go_to_page(const_page_name)
-        is_problem_header_visible = len(page.find_elements_by_css_selector('section.problem > h4')) > 0
+        is_problem_header_visible = len(page.find_elements_by_css_selector('.problem > h4')) > 0
         self.assertEqual(is_problem_header_visible, show_problem_header)
 
-        problem = page.find_element_by_css_selector('section.problem > p')
+        problem = page.find_element_by_css_selector('.problem > p')
         self.assertEqual(self.get_element_html(problem), problem_text)
 
     @unpack

tests/unit/test_basics.py

@@ -45,7 +45,7 @@ class BasicTests(TestCaseMixin, unittest.TestCase):
     def test_template_contents(self):
         context = {}
         student_fragment = self.block.runtime.render(self.block, 'student_view', context)
-        self.assertIn('<section class="themed-xblock xblock--drag-and-drop">', student_fragment.content)
+        self.assertIn('<div class="themed-xblock xblock--drag-and-drop">', student_fragment.content)
         self.assertIn('Loading drag and drop problem.', student_fragment.content)
 
     def test_get_configuration(self):