Accessible Sequence navigation

MA-2190

common/lib/xmodule/xmodule/css/sequence/display.scss

@@ -30,6 +30,23 @@ $link-color: rgb(26, 161, 222) !default;
   }
 }
 
+%ui-clear-button  {
+  background-color: rgba(0,0,0,0);
+  background-image: none;
+  background-position: center 14px;
+  background-repeat: no-repeat;
+  border: none;
+  border-radius: 0;
+
+  background-clip: border-box;
+  box-shadow: none;
+  box-sizing: content-box;
+  font-family: inherit;
+  font-size: inherit;
+  font-weight: inherit;
+}
+
+
 // ====================
 
 .sequence-nav {
@@ -90,7 +107,7 @@ $link-color: rgb(26, 161, 222) !default;
     padding: 0 ($baseline/2);
     width: 100%;
 
-    a {
+    button {
       @extend .block-link;
     }
 
@@ -98,15 +115,14 @@ $link-color: rgb(26, 161, 222) !default;
       display: table-cell;
       min-width: 20px;
 
-      a {
+      button {
         @extend %ui-fake-link;
+        @extend %ui-clear-button;
         @include transition(none);
         width: 100%;
         height: 42px;
         margin: 0;
-        background-position: center 14px;
-        background-repeat: no-repeat;
-        border: 1px solid transparent;
+
         display: block;
         padding: 0;
         position: relative;
@@ -197,7 +213,6 @@ $link-color: rgb(26, 161, 222) !default;
 
         .sequence-tooltip {
           @extend %ui-depth2;
-          display: none;
           margin-top: ($baseline/5);
           background: #333;
           color: $white;
@@ -209,7 +224,6 @@ $link-color: rgb(26, 161, 222) !default;
           top: 48px;
           text-shadow: 0 -1px 0 $black;
           @include text-align(left);
-          @include transition(all .1s $ease-in-out-quart 0s);
           white-space: pre;
           pointer-events: none;
 
@@ -233,17 +247,11 @@ $link-color: rgb(26, 161, 222) !default;
             width: 10px;
           }
         }
-
-        &:hover, &:focus {
-          .sequence-tooltip {
-            display: block;
-          }
-        }
       }
     }
   }
 
-  body.touch-based-device & ol li a:hover p {
+  body.touch-based-device & ol li button:hover p {
     display: none;
   }
 }

common/lib/xmodule/xmodule/js/fixtures/sequence.html

@@ -4,8 +4,8 @@
     </ol>
 
     <ul class="sequence-nav-buttons">
-      <li class="prev"><a href="#">Previous</a></li>
-      <li class="next"><a href="#">Next</a></li>
+      <li class="prev"><button>Previous</button></li>
+      <li class="next"><button>Next</button></li>
     </ul>
   </nav>
 
@@ -13,8 +13,8 @@
 
   <nav class="sequence-bottom">
     <ul class="sequence-nav-buttons">
-      <li class="prev"><a href="#">Previous</a></li>
-      <li class="next"><a href="#">Next</a></li>
+      <li class="prev"><button>Previous</button></li>
+      <li class="next"><button>Next</button></li>
     </ul>
   </nav>
 </div>

common/lib/xmodule/xmodule/js/src/sequence/display.coffee

@@ -19,9 +19,17 @@ class @Sequence
     $(selector, @el)
 
   bind: ->
-    @$('#sequence-list a').click @goto
+    @$('#sequence-list .nav-item').click @goto
     @el.on 'bookmark:add', @addBookmarkIconToActiveNavItem
     @el.on 'bookmark:remove', @removeBookmarkIconFromActiveNavItem
+    @$('#sequence-list .nav-item').on('focus mouseenter', @displayTabTooltip)
+    @$('#sequence-list .nav-item').on('blur mouseleave', @hideTabTooltip)
+
+  displayTabTooltip: (event) =>
+    $(event.currentTarget).find('.sequence-tooltip').removeClass('sr')
+
+  hideTabTooltip: (event) =>
+    $(event.currentTarget).find('.sequence-tooltip').addClass('sr')
 
   initProgress: ->
     @progressTable = {}  # "#problem_#{id}" -> progress
@@ -136,7 +144,6 @@ class @Sequence
       @el.find('.path').text(@el.find('.nav-item.active').data('path'))
 
       @sr_container.focus();
-      # @$("a.active").blur()
 
   goto: (event) =>
     event.preventDefault()
@@ -189,7 +196,7 @@ class @Sequence
       @render new_position
 
   link_for: (position) ->
-    @$("#sequence-list a[data-element=#{position}]")
+    @$("#sequence-list .nav-item[data-element=#{position}]")
 
   mark_visited: (position) ->
     # Don't overwrite class attribute to avoid changing Progress class

common/test/acceptance/pages/lms/course_nav.py

@@ -57,7 +57,7 @@ class CourseNavPage(PageObject):
         Example return value:
             ['Chemical Bonds Video', 'Practice Problems', 'Homework']
         """
-        seq_css = 'ol#sequence-list>li>a>.sequence-tooltip'
+        seq_css = 'ol#sequence-list>li>.nav-item>.sequence-tooltip'
         return self.q(css=seq_css).map(self._clean_seq_titles).results
 
     def go_to_section(self, section_title, subsection_title):
@@ -124,7 +124,7 @@ class CourseNavPage(PageObject):
 
             # Click on the sequence item at the correct index
             # Convert the list index (starts at 0) to a CSS index (starts at 1)
-            seq_css = "ol#sequence-list>li:nth-of-type({0})>a".format(seq_index + 1)
+            seq_css = "ol#sequence-list>li:nth-of-type({0})>.nav-item".format(seq_index + 1)
             self.q(css=seq_css).first.click()
             # Click triggers an ajax event
             self.wait_for_ajax()

common/test/acceptance/pages/lms/courseware.py

@@ -233,7 +233,7 @@ class CoursewarePage(CoursePage):
         """ Returns the usage id of active sequence item """
         get_active = lambda el: 'active' in el.get_attribute('class')
         attribute_value = lambda el: el.get_attribute('data-id')
-        return self.q(css='#sequence-list a').filter(get_active).map(attribute_value).results[0]
+        return self.q(css='#sequence-list .nav-item').filter(get_active).map(attribute_value).results[0]
 
     @property
     def breadcrumb(self):

common/test/acceptance/tests/lms/test_lms_courseware.py

@@ -3,7 +3,6 @@
 End-to-end tests for the LMS.
 """
 from nose.plugins.attrib import attr
-from unittest import skip
 
 from ..helpers import UniqueCourseTest
 from ...pages.studio.auto_auth import AutoAuthPage
@@ -533,7 +532,6 @@ class CoursewareMultipleVerticalsTest(UniqueCourseTest):
         ).visit()
         self.assertIn('html 2 dummy body', html2_page.get_selected_tab_content())
 
-    @skip('Fix a11y test errors.')
     @attr('a11y')
     def test_courseware_a11y(self):
         """

lms/djangoapps/courseware/features/navigation.py

@@ -109,7 +109,7 @@ def when_i_navigate_to_a_subsection(step):
 
 @step(u'I navigate to an item in a sequence')
 def when_i_navigate_to_an_item_in_a_sequence(step):
-    sequence_css = 'a[data-element="2"]'
+    sequence_css = '.nav-item[data-element="2"]'
     world.css_click(sequence_css)
 
 

lms/djangoapps/courseware/tests/test_split_module.py

@@ -123,7 +123,7 @@ class SplitTestBase(SharedModuleStoreTestCase):
         content = resp.content
 
         # Assert we see the proper icon in the top display
-        self.assertIn('<a class="{} inactive progress-0 nav-item"'.format(self.ICON_CLASSES[user_tag]), content)
+        self.assertIn('<button class="{} inactive progress-0 nav-item"'.format(self.ICON_CLASSES[user_tag]), content)
         # And proper tooltips
         for tooltip in self.TOOLTIPS[user_tag]:
             self.assertIn(tooltip, content)

lms/templates/seq_module.html

@@ -14,19 +14,16 @@
         ## implementation note: will need to figure out how to handle combining detail
         ## statuses of multiple modules in js.
         <li>
-        <a class="seq_${item['type']} inactive progress-${item['progress_status']} nav-item"
+        <button class="seq_${item['type']} inactive progress-${item['progress_status']} nav-item"
            data-id="${item['id']}"
            data-element="${idx+1}"
-           href="javascript:void(0);"
            data-page-title="${item['page_title']|h}"
            data-path="${item['path']|h}"
-           aria-controls="seq_contents_${idx}"
-           id="tab_${idx}"
-           tabindex="0">
+           id="tab_${idx}">
             <i class="icon fa seq_${item['type']}" aria-hidden="true"></i>
             <i class="fa fa-fw fa-bookmark bookmark-icon ${"is-hidden" if not item['bookmarked'] else "bookmarked"}" aria-hidden="true"></i>
-            <div class="sequence-tooltip"><span class="sr">${item['type']}&nbsp;</span>${item['title']}<span class="sr bookmark-icon-sr">&nbsp;${_("Bookmarked") if item['bookmarked'] else ""}</span></div>
-          </a>
+            <div class="sequence-tooltip sr"><span class="sr">${item['type']}&nbsp;</span>${item['title']}<span class="sr bookmark-icon-sr">&nbsp;${_("Bookmarked") if item['bookmarked'] else ""}</span></div>
+          </button>
         </li>
         % endfor
       </ol>