MCKIN-12182: a11y: Ensure dialogs use proper structure (#65)

* a11y: Ensure dialogs use proper structure * fix tests * minor fix * add Icons and remove pseudo element from the css to Improve the Accessibility * bump version
openedx · Dec 6, 2019 · d8e8bbc · d8e8bbc
1 parent 7b41432
commit d8e8bbc
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 32 deletions.
diff --git a/image_explorer/image_explorer.py b/image_explorer/image_explorer.py
@@ -167,6 +167,7 @@ def student_view(self, context):
             'description_html': description,
             'hotspots': hotspots,
             'background': background,
+            'ie_uid': uuid.uuid4().hex[:15],
         }
 
         fragment = Fragment()

diff --git a/image_explorer/public/css/image_explorer.css b/image_explorer/public/css/image_explorer.css
@@ -53,6 +53,7 @@
     transition: all 0.0s linear 0s;
 }
 
+.image-explorer-wrapper button.image-explorer-hotspot > span{ display: none;}
 
 .image-explorer-hotspot.centered{ /* offset for hotspot to restrict it to its center */
     margin-left: -20.5px;
@@ -97,6 +98,8 @@ button.image-explorer-hotspot .image-explorer-hotspot-reveal .image-explorer-hot
     font-family: "fontawesome";
     font-size: 18pt;
     cursor: pointer;
+    background: none;
+    border: none;
 }
 
 .codemirror-editor-wrapper {
@@ -157,3 +160,16 @@ button.image-explorer-hotspot .image-explorer-hotspot-reveal .image-explorer-hot
 .xblock-render{
     overflow: visible !important;
 }
+
+
+/* A11y */
+.sr-only {
+    position: absolute;
+    width: 1px;
+    height: 1px;
+    padding: 0;
+    margin: -1px;
+    overflow: hidden;
+    clip: rect(0,0,0,0);
+    border: 0;
+}
diff --git a/image_explorer/public/js/image_explorer.js b/image_explorer/public/js/image_explorer.js
@@ -19,7 +19,7 @@ function ImageExplorerBlock(runtime, element) {
       var target_position_left = target.position().left;
       var hotspot_image_width = target.outerWidth();
 
-      var image_width = reveal.parents('.image-explorer-hotspot').siblings('.image-explorer-background').outerWidth();
+      var image_width = reveal.prev('.image-explorer-hotspot').siblings('.image-explorer-background').outerWidth();
 
       if (reveal_side !== 'left' && reveal_side !== 'right') {
           /* do some calculations with regards which side of the hotspot to open the feedback on */
@@ -47,17 +47,18 @@ function ImageExplorerBlock(runtime, element) {
       }
       $(this).addClass('visited');
       var target = $(eventObj.currentTarget);
-      var reveal = target.find('.image-explorer-hotspot-reveal');
+      var reveal = target.next('.image-explorer-hotspot-reveal');
 
       setRevealPosition(target, reveal);
 
       reveal.css('display', 'block');
+      reveal.focus();
       active_feedback = reveal;
       $(this).trigger('feedback:open');
       hotspot_opened_at = new Date().getTime();
       publish_event({
               event_type:'xblock.image-explorer.hotspot.opened',
-              item_id: target.data('item-id')
+              item_id: target.parent('.hotspot-container').data('item-id')
       });
     });
 
@@ -92,7 +93,7 @@ function ImageExplorerBlock(runtime, element) {
     /* close feedback action */
     function close_feedback() {
       // Close the visible feedback popup
-      var hotspot = active_feedback.closest('.image-explorer-hotspot');
+      var hotspot = active_feedback.closest('.hotspot-container');
       pause_youtube_videos(hotspot);
       hotspot.trigger('feedback:close');
       active_feedback.css('display', 'none');
@@ -118,18 +119,6 @@ function ImageExplorerBlock(runtime, element) {
       }
     });
 
-    $(document).on('keyup', function(eventObj) {
-      if (!active_feedback)
-        return;
-      var target = $(eventObj.target);
-      var close_btn = ".image-explorer-close-reveal";
-      if (target.is(close_btn) && eventObj.keyCode === 13) {
-        close_feedback();
-        eventObj.preventDefault();
-        eventObj.stopPropagation();
-      }
-    });
-
     publish_event({
         event_type:'xblock.image-explorer.loaded'
     });

diff --git a/image_explorer/templates/html/image_explorer.html b/image_explorer/templates/html/image_explorer.html
@@ -9,17 +9,24 @@ <h2>{{title}}</h2>
     <div class="image-explorer-wrapper">
         <img src="{{background.src}}" class="image-explorer-background" />
         {% for hotspot in hotspots %}
-        <button class="image-explorer-hotspot {% if hotspot_coordinates_centered %}centered{% endif %}{% if hotspot.visited %} visited {% endif %}"
-             style="position: absolute; top: {{hotspot.y}}; left: {{hotspot.x}};"
-             data-item-id="{{hotspot.item_id}}" aria-label="{{hotspot.item_id}}">
-                <div class="image-explorer-hotspot-reveal" {{hotspot.reveal_style}} data-side="{{ hotspot.feedback.side }}">
-                    <div class="image-explorer-close-reveal" tabindex="0">&#xf057;</div>
-                    <div class="image-explorer-hotspot-content-wrapper">
-                        {% if hotspot.feedback.header %}
-                            <span class="image-explorer-hotspot-reveal-header">
-                                {{hotspot.feedback.header}}
-                            </span>
-                        {% endif %}
+        <div class="hotspot-container" data-item-id="{{hotspot.item_id}}" style="position: absolute; top: {{hotspot.y}}; left: {{hotspot.x}};">
+            <button class="image-explorer-hotspot {% if hotspot_coordinates_centered %}centered{% endif %}{% if hotspot.visited %} visited {% endif %}">
+                <span id="icon-plus">
+                    <i class="material-icons">add_circle</i>
+                </span>
+                <span id="icon-check">
+                    <i class="material-icons">check_circle</i>
+                </span>
+            </button>
+            <div role="region" aria-modal="false" class="image-explorer-hotspot-reveal" {{hotspot.reveal_style}} data-side="{{ hotspot.feedback.side }}" aria-labelledby="{% if hotspot.feedback.header %}{{ie_uid}}_{{hotspot.item_id}}_header{% endif %}" aria-describedby="{{ie_uid}}_{{hotspot.item_id}}_body" tabindex="0">
+                <button class="image-explorer-close-reveal"><span class="sr-only">Close</span>&#xf057;</button>
+                <div class="image-explorer-hotspot-content-wrapper">
+                    {% if hotspot.feedback.header %}
+                        <span id="{{ie_uid}}_{{hotspot.item_id}}_header" class="image-explorer-hotspot-reveal-header">
+                            {{hotspot.feedback.header}}
+                        </span>
+                    {% endif %}
+                    <div id="{{ie_uid}}_{{hotspot.item_id}}_body">
                         {% if hotspot.feedback.body %}
                             <span class="image-explorer-hotspot-reveal-body">
                                 {{hotspot.feedback.body}}
@@ -36,7 +43,8 @@ <h2>{{title}}</h2>
                         {% endif %}
                     </div>
                 </div>
-            </button>
+            </div>
+        </div>
         {% endfor %}
     </div>
 </div>

diff --git a/setup.py b/setup.py
@@ -23,7 +23,7 @@ def package_data(pkg, root_list):
 
 setup(
     name='xblock-image-explorer',
-    version='1.1.8',
+    version='1.1.9',
     description='XBlock - Image Explorer',
     packages=['image_explorer'],
     install_requires=[

diff --git a/tests/integration/test_image_explorer.py b/tests/integration/test_image_explorer.py
@@ -13,7 +13,7 @@ def assert_page_content(self, block):
         self.assertIn("Click around the MIT Dome and see what you find!", block.description.text)
 
     def hotspots(self, block):
-        hotspots = block.find_elements_by_css_selector(".image-explorer-hotspot")
+        hotspots = block.find_elements_by_css_selector(".hotspot-container")
         for h in hotspots:
             h.content = h.find_element_by_css_selector(".image-explorer-hotspot-reveal")
             h.close_button = h.find_element_by_css_selector(".image-explorer-close-reveal")
@@ -47,7 +47,7 @@ def assert_only_hotspotA_revealed(self, block):
         self.assertIn("Below are some of the highlights:", block.hotspotA.content.text)
         self.assertIn("Once there was a police car up here", block.hotspotA.content.text)
         self.assertIn("Also there was a Fire Truck put up there", block.hotspotA.content.text)
-        self.assertIn('visited', block.hotspotA.get_attribute("class"))
+        self.assertIn('visited', block.hotspotA.find_element_by_css_selector('.image-explorer-hotspot').get_attribute("class"))
 
     def assert_only_hotspotB_revealed(self, block):
         hs = self.hotspots(block)
@@ -58,7 +58,7 @@ def assert_only_hotspotB_revealed(self, block):
 
         self.assertIn("Watch the Red Line subway go around the dome", block.hotspotB.content.text)
         self.assertTrue(block.hotspotB.find_element_by_css_selector(".image-explorer-hotspot-reveal-youtube"))
-        self.assertIn('visited', block.hotspotB.get_attribute("class"))
+        self.assertIn('visited', block.hotspotB.find_element_by_css_selector('.image-explorer-hotspot').get_attribute("class"))
 
     def test_simple_scenario(self):
         self.set_scenario_xml('<image-explorer/>')