diff --git a/common/test/acceptance/pages/lms/course_nav.py b/common/test/acceptance/pages/lms/course_nav.py index a7bde281a7..36b5171240 100644 --- a/common/test/acceptance/pages/lms/course_nav.py +++ b/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") diff --git a/common/test/acceptance/pages/lms/courseware.py b/common/test/acceptance/pages/lms/courseware.py index bb842682d8..292917f3fb 100644 --- a/common/test/acceptance/pages/lms/courseware.py +++ b/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): diff --git a/common/test/acceptance/tests/lms/test_lms.py b/common/test/acceptance/tests/lms/test_lms.py index ca0a6a260e..8eae4bbdc1 100644 --- a/common/test/acceptance/tests/lms/test_lms.py +++ b/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() diff --git a/lms/djangoapps/courseware/features/navigation.py b/lms/djangoapps/courseware/features/navigation.py index 72eb109a7a..1b3613fe07 100644 --- a/lms/djangoapps/courseware/features/navigation.py +++ b/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 diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index ddb321e4ed..43b9866782 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/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 diff --git a/lms/djangoapps/courseware/tests/test_entrance_exam.py b/lms/djangoapps/courseware/tests/test_entrance_exam.py index 636dcc1e3b..cf74d16d6a 100644 --- a/lms/djangoapps/courseware/tests/test_entrance_exam.py +++ b/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' } ] ) diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 8f8849a100..a1dbd47c24 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/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( diff --git a/lms/static/coffee/fixtures/accordion.html b/lms/static/coffee/fixtures/accordion.html index 148c245c8f..5d5d7f146b 100644 --- a/lms/static/coffee/fixtures/accordion.html +++ b/lms/static/coffee/fixtures/accordion.html @@ -2,5 +2,11 @@
close
-
- +
+ +
+ \ No newline at end of file diff --git a/lms/static/coffee/spec/navigation_spec.coffee b/lms/static/coffee/spec/navigation_spec.coffee index 162eff3f2f..4970c545eb 100644 --- a/lms/static/coffee/spec/navigation_spec.coffee +++ b/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('') + $('#accordion').append('
') 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('') + $('#accordion').append('
') 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('
') + 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() + diff --git a/lms/static/coffee/src/navigation.coffee b/lms/static/coffee/src/navigation.coffee index 06c38b781a..4316fc2eb0 100644 --- a/lms/static/coffee/src/navigation.coffee +++ b/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') + diff --git a/lms/static/sass/course/courseware/_sidebar.scss b/lms/static/sass/course/courseware/_sidebar.scss index 07a0bdf04d..80e03bc490 100644 --- a/lms/static/sass/course/courseware/_sidebar.scss +++ b/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; + + h3 { + padding: ($baseline*.75) $baseline ($baseline*.75) ($baseline*2); + } &.is-open { background: $white; - } - - &:first-child { - border-radius: 3px 0 0 0; - } - - &:last-child { - border-radius: 0 0 0 3px; - box-shadow: 0 1px 0 $white inset; + 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); } } } diff --git a/lms/templates/courseware/accordion.html b/lms/templates/courseware/accordion.html index 2d429ef11d..cca1ae504a 100644 --- a/lms/templates/courseware/accordion.html +++ b/lms/templates/courseware/accordion.html @@ -6,7 +6,7 @@ %> <%def name="make_chapter(chapter)"> -
+ +
+
+
    + % for section in chapter['sections']: +
  1. + +

    ${section['display_name']} ${', current section' if 'active' in section and section['active'] else ''}

    + <% + 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) + %> +

    ${section['format']} ${due_date}

    -     -
+ % if 'graded' in section and section['graded']: + Graded Section + % endif + + + % endfor + +
+
% for chapter in toc: ${make_chapter(chapter)} -% endfor +% endfor \ No newline at end of file