Skip to content

E
edx-platform
Commit b32d2205 by Ahsan Ulhaq Committed by Chris Rodriguez
Options

Course navigation menu accessibility issue 

Following have been done
1- Change the structure of the course navigation Menu HTML
2- Add some basic styling
3- Add basic JS for html rendering
4- Update tests according to new structure

AC-76
parent 28fddda4
Showing with 176 additions and 147 deletions
common/test/acceptance/pages/lms/course_nav.py
......@@ -82,7 +82,7 @@ class CourseNavPage(PageObject):
# Click the section to ensure it's open (no harm in clicking twice if it's already open)
# Add one to convert from list index to CSS index
section_css = 'nav>div.chapter:nth-of-type({0})>h3>a'.format(sec_index + 1)
section_css = 'nav>div.chapter:nth-of-type({0})'.format(sec_index + 1)
self.q(css=section_css).first.click()
# Get the subsection by index
......@@ -94,7 +94,7 @@ class CourseNavPage(PageObject):
return
# Convert list indices (start at zero) to CSS indices (start at 1)
subsection_css = "nav>div.chapter:nth-of-type({0})>ul>li:nth-of-type({1})>a".format(
subsection_css = "nav>div.chapter-content-container:nth-of-type({0})>div>ol>li:nth-of-type({1})>a".format(
sec_index + 1, subsec_index + 1
)
......@@ -130,7 +130,7 @@ class CourseNavPage(PageObject):
"""
Return a list of all section titles on the page.
"""
chapter_css = 'nav > div.chapter > h3 > a'
chapter_css = 'nav > button.chapter > h3'
return self.q(css=chapter_css).map(lambda el: el.text.strip()).results
def _subsection_titles(self, section_index):
......@@ -140,7 +140,9 @@ class CourseNavPage(PageObject):
"""
# Retrieve the subsection title for the section
# Add one to the list index to get the CSS index, which starts at one
subsection_css = 'nav>div.chapter:nth-of-type({0})>ul>li>a>p:nth-of-type(1)'.format(section_index)
subsection_css = 'nav>.chapter-content-container:nth-of-type({0})>div>ol>li>a>p:nth-of-type(1)'.format(
section_index
)
# If the element is visible, we can get its text directly
# Otherwise, we need to get the HTML
......@@ -171,8 +173,8 @@ class CourseNavPage(PageObject):
That's true right after we click the section/subsection, but not true in general
(the user could go to a section, then expand another tab).
"""
current_section_list = self.q(css='nav>div.chapter.is-open>h3>a').text
current_subsection_list = self.q(css='nav>div.chapter.is-open li.active>a>p').text
current_section_list = self.q(css='nav>button.chapter.is-open>h3').text
current_subsection_list = self.q(css='nav div.chapter-content-container ol li.active>a>p').text
if len(current_section_list) == 0:
self.warning("Could not find the current section")
......
common/test/acceptance/pages/lms/courseware.py
......@@ -14,7 +14,7 @@ class CoursewarePage(CoursePage):
url_path = "courseware/"
xblock_component_selector = '.vert .xblock'
section_selector = '.chapter'
subsection_selector = '.chapter ul li'
subsection_selector = '.chapter-content-container ol li'
def is_browser_on_page(self):
return self.q(css='body.courseware').present
......@@ -102,7 +102,7 @@ class CoursewarePage(CoursePage):
"""
return the url of the active subsection in the left nav
"""
return self.q(css='.chapter ul li.active a').attrs('href')[0]
return self.q(css='.chapter-content-container ol li.active a').attrs('href')[0]
@property
def can_start_proctored_exam(self):
......
common/test/acceptance/tests/lms/test_lms.py
......@@ -1121,7 +1121,7 @@ class EntranceExamTest(UniqueCourseTest):
When I view the courseware that has an entrance exam
Then there should be an "Entrance Exam" chapter.'
"""
entrance_exam_link_selector = 'div#accordion nav div h3 a'
entrance_exam_link_selector = 'div#accordion nav button h3'
# visit courseware page and make sure there is not entrance exam chapter.
self.courseware_page.visit()
self.courseware_page.wait_for_page()
......
lms/djangoapps/courseware/features/navigation.py
......@@ -92,7 +92,7 @@ def when_i_navigate_to_a_section(step):
world.disable_jquery_animations()
# Open the 2nd section
world.css_click(css_selector='div.chapter', index=1)
world.css_click(css_selector='button.chapter', index=1)
subsection_css = 'a[href*="Test_Subsection_2/"]'
# Click on the subsection to see the content
......
lms/djangoapps/courseware/module_render.py
......@@ -74,6 +74,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.x_module import XModuleDescriptor
from xmodule.mixin import wrap_with_license
from util.json_request import JsonResponse
from util.model_utils import slugify
from util.sandboxing import can_execute_unsafe_code, get_python_lib_zip
from util import milestones_helpers
from verify_student.services import ReverificationService
......@@ -165,6 +166,7 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_
for chapter in chapters:
# Only show required content, if there is required content
# chapter.hide_from_toc is read-only (boo)
display_id = slugify(chapter.display_name_with_default)
local_hide_from_toc = False
if required_content:
if unicode(chapter.location) not in required_content:
......@@ -246,6 +248,7 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_
sections.append(section_context)
toc_chapters.append({
'display_name': chapter.display_name_with_default,
'display_id': display_id,
'url_name': chapter.url_name,
'sections': sections,
'active': chapter.url_name == active_chapter
......
lms/djangoapps/courseware/tests/test_entrance_exam.py
......@@ -155,7 +155,8 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase):
}
],
'url_name': u'Entrance_Exam_Section_-_Chapter_1',
'display_name': u'Entrance Exam Section - Chapter 1'
'display_name': u'Entrance Exam Section - Chapter 1',
'display_id': u'Entrance-Exam-Section---Chapter-1',
}
]
)
......@@ -182,19 +183,22 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase):
}
],
'url_name': u'Overview',
'display_name': u'Overview'
'display_name': u'Overview',
'display_id': u'Overview'
},
{
'active': False,
'sections': [],
'url_name': u'Week_1',
'display_name': u'Week 1'
'display_name': u'Week 1',
'display_id': u'Week-1'
},
{
'active': False,
'sections': [],
'url_name': u'Instructor',
'display_name': u'Instructor'
'display_name': u'Instructor',
'display_id': u'Instructor'
},
{
'active': True,
......@@ -209,7 +213,8 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase):
}
],
'url_name': u'Entrance_Exam_Section_-_Chapter_1',
'display_name': u'Entrance Exam Section - Chapter 1'
'display_name': u'Entrance Exam Section - Chapter 1',
'display_id': u'Entrance-Exam-Section---Chapter-1'
}
]
)
......
lms/djangoapps/courseware/tests/test_module_render.py
......@@ -642,11 +642,11 @@ class TestTOC(ModuleStoreTestCase):
'format': '', 'due': None, 'active': False},
{'url_name': 'video_4f66f493ac8f', 'display_name': 'Video', 'graded': True,
'format': '', 'due': None, 'active': False}],
'url_name': 'Overview', 'display_name': u'Overview'},
'url_name': 'Overview', 'display_name': u'Overview', 'display_id': u'Overview'},
{'active': False, 'sections':
[{'url_name': 'toyvideo', 'display_name': 'toyvideo', 'graded': True,
'format': '', 'due': None, 'active': False}],
'url_name': 'secret:magic', 'display_name': 'secret:magic'}])
'url_name': 'secret:magic', 'display_name': 'secret:magic', 'display_id': 'secretmagic'}])
course = self.store.get_course(self.toy_course.id, depth=2)
with check_mongo_calls(toc_finds):
......@@ -682,11 +682,11 @@ class TestTOC(ModuleStoreTestCase):
'format': '', 'due': None, 'active': False},
{'url_name': 'video_4f66f493ac8f', 'display_name': 'Video', 'graded': True,
'format': '', 'due': None, 'active': False}],
'url_name': 'Overview', 'display_name': u'Overview'},
'url_name': 'Overview', 'display_name': u'Overview', 'display_id': u'Overview'},
{'active': False, 'sections':
[{'url_name': 'toyvideo', 'display_name': 'toyvideo', 'graded': True,
'format': '', 'due': None, 'active': False}],
'url_name': 'secret:magic', 'display_name': 'secret:magic'}])
'url_name': 'secret:magic', 'display_name': 'secret:magic', 'display_id': 'secretmagic'}])
with check_mongo_calls(toc_finds):
actual = render.toc_for_course(
......
lms/static/coffee/fixtures/accordion.html
......@@ -2,5 +2,11 @@
<header id="open_close_accordion">
<a href="#">close</a>
</header>
<div id="accordion"></div>
</div>
<div id="accordion">
<button class="chapter">
<h3>
Introduction Chapter
</h3>
</button>
</div>
</div>
\ No newline at end of file
lms/static/coffee/spec/navigation_spec.coffee
......@@ -8,7 +8,7 @@ describe 'Navigation', ->
describe 'when there is an active section', ->
beforeEach ->
spyOn $.fn, 'accordion'
$('#accordion').append('<ul><li></li></ul><ul><li class="active"></li></ul>')
$('#accordion').append('<div><div><ol><li></li></ol></div></div><div><div><ol><li class="active"></li></ol></div></div>')
new Navigation
it 'activate the accordion with correct active section', ->
......@@ -21,7 +21,7 @@ describe 'Navigation', ->
describe 'when there is no active section', ->
beforeEach ->
spyOn $.fn, 'accordion'
$('#accordion').append('<ul><li></li></ul><ul><li></li></ul>')
$('#accordion').append('<div><div><ol><li></li></ol></div></div><div><div><ol><li></li></ol></div></div>')
new Navigation
it 'activate the accordian with no section as active', ->
......@@ -37,6 +37,9 @@ describe 'Navigation', ->
it 'bind the navigation toggle', ->
expect($('#open_close_accordion a')).toHandleWith 'click', @navigation.toggle
it 'bind the setChapter', ->
expect($('#accordion .chapter')).toHandleWith 'click', @navigation.setChapter
describe 'when the #accordion does not exists', ->
beforeEach ->
$('#accordion').remove()
......@@ -70,3 +73,25 @@ describe 'Navigation', ->
expect(Logger.log).toHaveBeenCalledWith 'accordion',
newheader: 'new'
oldheader: 'old'
describe 'setChapter', ->
beforeEach ->
$('#accordion').append('<div><div><ol><li class="active"><a href="#"></a></li></ol></div></div>')
new Navigation
it 'Chapter opened', ->
e = jQuery.Event('click')
$('#accordion .chapter').trigger(e)
expect($('.chapter')).toHaveClass('is-open')
it 'content active on chapter opened', ->
e = jQuery.Event('click')
$('#accordion .chapter').trigger(e)
expect($('.chapter').next('div').children('div')).toHaveClass('ui-accordion-content-active')
expect($('.ui-accordion-content-active')).toHaveAttr('aria-hidden', 'false')
it 'focus move to first child on chapter opened', ->
spyOn($.fn, 'focus')
e = jQuery.Event('click')
$('#accordion .chapter').trigger(e)
expect($('.ui-accordion-content-active li:first-child a').focus).toHaveBeenCalled()
lms/static/coffee/src/navigation.coffee
......@@ -2,7 +2,7 @@ class @Navigation
constructor: ->
if $('#accordion').length
# First look for an active section
active = $('#accordion ul:has(li.active)').index('#accordion ul')
active = $('#accordion div div ol:has(li.active)').index('#accordion div div ol')
# if we didn't find one, look for an active chapter
if active < 0
active = $('#accordion h3.active').index('#accordion h3')
......@@ -15,9 +15,15 @@ class @Navigation
autoHeight: false
heightStyle: 'content'
$('#accordion .ui-state-active').closest('.chapter').addClass('is-open')
$('#accordion .ui-state-active').parent().next('div').children('div').addClass('ui-accordion-content-active')
$('#accordion .ui-state-active').parent().next('div').show()
$('#accordion .ui-state-active').parent().attr('aria-expanded' , 'true').attr('aria-pressed' , 'true')
$('#accordion div').filter(':not(.chapter-content-container)').addClass("ui-accordion-content ui-helper-reset ui-widget-content ui-corner-bottom").filter(":not(.ui-accordion-content-active)").hide()
$('.ui-accordion-content div').attr('aria-hidden', 'false')
$('.ui-accordion-content-active div').attr('aria-hidden', 'true')
$('#open_close_accordion a').click @toggle
$('#accordion').show()
$('#accordion a').click @setChapter
$('#accordion .chapter').click @setChapter
log: (event, ui) ->
Logger.log 'accordion',
......@@ -28,6 +34,14 @@ class @Navigation
$('.course-wrapper').toggleClass('closed')
setChapter: ->
$('#accordion .is-open').removeClass('is-open')
$(this).closest('.chapter').addClass('is-open')
\ No newline at end of file
$('#accordion .is-open').removeClass('is-open').attr('aria-expanded' , 'false').attr('aria-pressed' , 'false').find('h3 span').removeClass('ui-icon-triangle-1-s').addClass('ui-icon-triangle-1-e')
$(this).closest('.chapter').addClass('is-open').attr('aria-expanded' , 'true').attr('aria-pressed' , 'true').find('h3 span').removeClass('ui-icon-triangle-1-e').addClass('ui-icon-triangle-1-s')
$('.ui-accordion-content-active').attr('aria-hidden', 'true')
$('.ui-accordion-content-active').parent().hide()
$('#accordion .ui-accordion-content-active').removeClass('ui-accordion-content-active')
$(this).closest('.chapter').next('div').children('div').addClass('ui-accordion-content-active')
$('.ui-accordion-content-active').parent().show()
$('.ui-accordion-content-active').show()
$('.ui-accordion-content-active li:first-child a').focus()
$('.ui-accordion-content-active').attr('aria-hidden', 'false')
lms/static/sass/course/courseware/_sidebar.scss
......@@ -17,43 +17,36 @@
}
div#accordion {
@extend %t-copy-sub1;
width: auto;
font-size: 14px;
h3 {
border-radius: 0;
margin: 0;
@extend %t-title6;
@include padding-left($baseline);
overflow: visible;
font-size: 16px;
margin: 0;
border-radius: 0;
box-shadow: none;
color: $link-color;
&:first-child {
border: none;
}
&:hover, &:focus {
color: #666;
}
&.ui-state-hover, &.ui-state-focus {
a {
color: #666;
}
}
&.ui-accordion-header {
@extend %t-regular;
border-bottom: none;
color: $black;
a {
@include padding-left($baseline);
border-radius: 0;
box-shadow: none;
@include padding-left(19px);
color: $link-color;
}
&.ui-state-active {
@extend .active;
border-bottom: none;
color: $base-font-color;
&:hover, &:focus {
background: none;
......@@ -61,9 +54,9 @@
}
span.ui-icon {
@include left(0);
opacity: 0.3;
@include left($baseline);
background-image: url("/static/images/ui-icons_222222_256x240.png"); // jQuery UI sprite
opacity: 0.3;
&.ui-icon-triangle-1-e {
......@@ -82,25 +75,24 @@
}
.chapter {
width: 100% !important;
@include box-sizing(border-box);
padding: 11px 14px;
@include linear-gradient(top, $sidebar-chapter-bg-top, $sidebar-chapter-bg-bottom);
background-color: $sidebar-chapter-bg;
box-shadow: 0 1px 0 $white inset, 0 -1px 0 $shadow-l1 inset;
@include transition(background-color .1s linear 0s);
width: 100% !important;
padding: 0;
border: 0;
border-radius: 0;
box-shadow: 0 1px 0 $white inset, 0 -1px 0 $shadow-l1 inset;
background-color: $sidebar-chapter-bg;
color: $link-color;
&.is-open {
background: $white;
}
&:first-child {
border-radius: 3px 0 0 0;
h3 {
padding: ($baseline*.75) $baseline ($baseline*.75) ($baseline*2);
}
&:last-child {
border-radius: 0 0 0 3px;
box-shadow: 0 1px 0 $white inset;
&.is-open {
background: $white;
box-shadow: none;
}
&:hover, &:focus {
......@@ -108,41 +100,47 @@
}
}
ul.ui-accordion-content {
background: transparent;
div.chapter-content-container{
display: none;
padding: 0 14px ($baseline/2) 14px;
border-bottom: 1px solid $shadow-l1;
background: $white;
}
div.ui-accordion-content {
margin: 0;
padding: 0 ($baseline*.75);
border: none;
border-radius: 0;
margin: 0;
padding: 9px 0 9px 9px;
overflow: auto;
width: 100%;
background: transparent;
li {
margin: ($baseline/5) 0;
// margin-bottom: ($baseline/5);
border-bottom: 0;
border-radius: 0;
margin-bottom: ($baseline/5);
a {
background: transparent;
border-radius: 4px;
display: block;
@include padding( ($baseline/4), ($baseline*1.5), ($baseline/4), ($baseline/2));
@extend %t-strong;
@include padding(($baseline/4) ($baseline/2));
position: relative;
display: block;
border-radius: ($baseline/5);
background: transparent;
text-decoration: none;
p {
font-weight: bold;
font-family: $sans-serif;
@extend %t-action3;
@extend %t-strong;
margin-bottom: 0;
line-height: 1.3;
font-family: $sans-serif;
&.subtitle {
@extend %t-copy-sub2;
@extend %t-weight2;
display: inline-block;
width: 90%;
color: $gray-d1;
@extend %t-action3;
@extend %t-regular;
display: block;
margin: 0;
color: $gray-d1;
&:empty {
display: none;
......@@ -164,7 +162,6 @@
}
&:hover, &:focus {
background: $shadow-l1;
> a p {
color: $gray-d3;
......@@ -182,19 +179,19 @@
}
&.active {
@extend %t-weight5;
@extend %t-strong;
&:after {
@extend %t-regular;
@include transition(none);
@include right($baseline);
content: '›';
position: absolute;
top: 50%;
right: 20px;
margin-top: -13px;
font-size: 30px;
font-weight: normal;
color: #333;
color: $gray-d3;
opacity: 0;
@include transition(none);
}
> a {
......@@ -208,24 +205,27 @@
}
p {
color: #333;
color: $gray-d3;
}
}
span.subtitle {
@extend %t-weight2;
@extend %t-regular;
}
}
&.graded {
> a {
.icon {
vertical-align: middle;
}
> a {
> img {
@include right($baseline/4);
position: absolute;
bottom: ($baseline/4);
margin: auto;
}
}
&.active > a {
background: linear-gradient(to bottom, #e6e6e6, #d6d6d6);
background: linear-gradient(to bottom, $gray-l4, #d6d6d6);
}
}
}
......
lms/templates/courseware/accordion.html
......@@ -6,7 +6,7 @@
%>
<%def name="make_chapter(chapter)">
<div class="chapter">
<button class="chapter" id="${chapter['display_id']}-parent" aria-controls="${chapter['display_id']}-child" aria-expanded="false" aria-pressed="false">
<%
if chapter.get('active'):
aria_label = _('{chapter}, current chapter').format(chapter=chapter['display_name'])
......@@ -16,63 +16,36 @@
active_class = ''
%>
<h3 ${active_class} aria-label="${aria_label}">
<a href="#">
${chapter['display_name']}
</a>
</h3>
</button>
<div class="chapter-content-container">
<div id="${chapter['display_id']}-child" role="region" aria-labelledby="${chapter['display_id']}-parent" aria-hidden="false">
<ol>
% for section in chapter['sections']:
<li class="${'active' if 'active' in section and section['active'] else ''} ${'graded' if 'graded' in section and section['graded'] else ''}">
<a href="${reverse('courseware_section', args=[course_id, chapter['url_name'], section['url_name']])}">
<p>${section['display_name']} ${'<span class="sr">, current section</span>' if 'active' in section and section['active'] else ''}</p>
<%
if section.get('due') is None:
due_date = ''
else:
formatted_string = get_time_display(section['due'], due_date_display_format, coerce_tz=settings.TIME_ZONE_DISPLAYED_FOR_DEADLINES)
due_date = '' if len(formatted_string)==0 else _('due {date}').format(date=formatted_string)
%>
<p class="subtitle">${section['format']} ${due_date}</p>
<ul>
% for section in chapter['sections']:
<li class="${'active' if 'active' in section and section['active'] else ''} ${'graded' if 'graded' in section and section['graded'] else ''}">
<a href="${reverse('courseware_section', args=[course_id, chapter['url_name'], section['url_name']])}">
<p>${section['display_name']} ${'<span class="sr">, current section</span>' if 'active' in section and section['active'] else ''}</p>
<%
if section.get('due') is None:
due_date = ''
else:
formatted_string = get_time_display(section['due'], due_date_display_format, coerce_tz=settings.TIME_ZONE_DISPLAYED_FOR_DEADLINES)
due_date = '' if len(formatted_string)==0 else _('due {date}').format(date=formatted_string)
%>
## There is behavior differences between
## rending of sections which have proctoring/timed examinations
## and those that do not.
##
## Proctoring exposes a exam status message field as well as
## a status icon
% if section['format'] or due_date or 'proctoring' in section:
<p class="subtitle">
% if 'proctoring' in section:
## Display the proctored exam status icon and status message
<i class="fa ${section['proctoring'].get('suggested_icon', 'fa-lock')} ${section['proctoring'].get('status', 'eligible')}"></i>&nbsp;
<span class="subtitle-name">${section['proctoring'].get('short_description', '')}
</span>
## completed proctored exam statuses should not show the due date
## since the exam has already been submitted by the user
% if not section['proctoring'].get('in_completed_state', False):
<span class="subtitle-name">${due_date}</span>
% endif
% else:
## non-proctored section, we just show the exam format and the due date
## this is the standard case in edx-platform
<span class="subtitle-name">${section['format']} ${due_date}
</span>
% endif
</p>
% endif
% if 'graded' in section and section['graded']:
## sections that are graded should indicate this through an icon
<i class="icon fa fa-pencil-square-o" aria-hidden="true" data-tooltip="${_("This section is graded.")}"></i>
% endif
</a>
</li>
% endfor
</ul>
</div>
% if 'graded' in section and section['graded']:
<img src="/static/images/graded.png" alt="Graded Section">
% endif
</a>
</li>
% endfor
</ol>
</div>
</div>
</%def>
% for chapter in toc:
${make_chapter(chapter)}
% endfor
% endfor
\ No newline at end of file
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment