From 3cffded2d1567aaabf8d5289b2ab753bd7910596 Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Thu, 18 May 2017 00:03:05 -0400 Subject: [PATCH] Improve course breadcrumbs LEARNER-877 --- .../xmodule/js/src/sequence/display.js | 23 +++---- .../templates/sequence-breadcrumbs.underscore | 9 --- .../_breadcrumbs.scss | 32 ++++++++++ .../test/acceptance/pages/lms/courseware.py | 63 ++++++++----------- common/test/acceptance/pages/lms/problem.py | 5 +- .../tests/lms/test_lms_courseware.py | 16 ++--- lms/djangoapps/courseware/tests/test_views.py | 4 +- lms/djangoapps/courseware/views/index.py | 7 ++- lms/static/sass/_build-lms-v1.scss | 1 + .../sass/course/courseware/_courseware.scss | 13 +++- lms/static/sass/shared/_header.scss | 21 ++++--- lms/templates/courseware/courseware.html | 38 +++++++++-- lms/templates/search/search_error.underscore | 2 +- .../course_bookmarks/course-bookmarks.html | 3 +- .../features/course_experience/__init__.py | 9 +++ .../course-updates-fragment.html | 3 +- themes/edx.org/lms/templates/header.html | 4 +- 17 files changed, 161 insertions(+), 92 deletions(-) delete mode 100644 common/static/common/templates/sequence-breadcrumbs.underscore create mode 100644 common/static/sass/edx-pattern-library-shims/_breadcrumbs.scss diff --git a/common/lib/xmodule/xmodule/js/src/sequence/display.js b/common/lib/xmodule/xmodule/js/src/sequence/display.js index aa756088a5..65588aaaa1 100644 --- a/common/lib/xmodule/xmodule/js/src/sequence/display.js +++ b/common/lib/xmodule/xmodule/js/src/sequence/display.js @@ -136,15 +136,20 @@ Sequence.prototype.updatePageTitle = function() { // update the page title to include the current section - var currentSectionTitle, + var currentUnitTitle, + newPageTitle, positionLink = this.link_for(this.position); if (positionLink && positionLink.data('page-title')) { - currentSectionTitle = positionLink.data('page-title') + ' | ' + this.base_page_title; + currentUnitTitle = positionLink.data('page-title'); + newPageTitle = currentUnitTitle + ' | ' + this.base_page_title; - if (currentSectionTitle !== document.title) { - document.title = currentSectionTitle; + if (newPageTitle !== document.title) { + document.title = newPageTitle; } + + // Update the title section of the breadcrumb + $('.nav-item-sequence').text(currentUnitTitle); } }; @@ -269,16 +274,6 @@ sequenceLinks = this.content_container.find('a.seqnav'); sequenceLinks.click(this.goto); - edx.HtmlUtils.setHtml( - this.path, - edx.HtmlUtils.template($('#sequence-breadcrumbs-tpl').text())({ - courseId: this.el.parent().data('course-id'), - blockId: this.id, - pathText: this.el.find('.nav-item.active').data('path'), - unifiedCourseView: this.path.data('unified-course-view') - }) - ); - this.sr_container.focus(); } }; diff --git a/common/static/common/templates/sequence-breadcrumbs.underscore b/common/static/common/templates/sequence-breadcrumbs.underscore deleted file mode 100644 index ec23d72839..0000000000 --- a/common/static/common/templates/sequence-breadcrumbs.underscore +++ /dev/null @@ -1,9 +0,0 @@ -<% if (unifiedCourseView) { %> - - - <%- gettext('Return to course outline') %> - <%- gettext('Outline') %> - - > -<% } %> -<%- pathText %> diff --git a/common/static/sass/edx-pattern-library-shims/_breadcrumbs.scss b/common/static/sass/edx-pattern-library-shims/_breadcrumbs.scss new file mode 100644 index 0000000000..bca74c2381 --- /dev/null +++ b/common/static/sass/edx-pattern-library-shims/_breadcrumbs.scss @@ -0,0 +1,32 @@ +// ------------------------------ +// Breadcrumb styles +// +// Mirrors styles from the Pattern Library + +.breadcrumbs { + font-size: font-size(small); + line-height: line-height(small); + + .nav-item { + @include margin-left($baseline/4); + display: inline-block; + + a, a:visited { + color: $uxpl-blue-base; + } + + a:hover { + color: $uxpl-blue-hover-active; + } + } + + .fa-angle-right { + @include margin-left($baseline/4); + display: inline-block; + color: $base-font-color; + + @include rtl { + @include transform(rotateY(180deg)); + } + } +} diff --git a/common/test/acceptance/pages/lms/courseware.py b/common/test/acceptance/pages/lms/courseware.py index 63679b2286..85bcc5ea4b 100644 --- a/common/test/acceptance/pages/lms/courseware.py +++ b/common/test/acceptance/pages/lms/courseware.py @@ -291,11 +291,6 @@ class CoursewarePage(CoursePage): attribute_value = lambda el: el.get_attribute('data-id') return self.q(css='#sequence-list .nav-item').filter(get_active).map(attribute_value).results[0] - @property - def breadcrumb(self): - """ Return the course tree breadcrumb shown above the sequential bar """ - return [part.strip() for part in self.q(css='.path .position').text[0].split('>')] - def unit_title_visible(self): """ Check if unit title is visible """ return self.q(css='.unit-title').visible @@ -365,6 +360,30 @@ class CourseNavPage(PageObject): def is_browser_on_page(self): return self.parent_page.is_browser_on_page + @property + def breadcrumb_section_title(self): + """ + Returns the section's title from the breadcrumb, or None if one is not found. + """ + label = self.q(css='.breadcrumbs .nav-item-chapter').text + return label[0].strip() if label else None + + @property + def breadcrumb_subsection_title(self): + """ + Returns the subsection's title from the breadcrumb, or None if one is not found + """ + label = self.q(css='.breadcrumbs .nav-item-section').text + return label[0].strip() if label else None + + @property + def breadcrumb_unit_title(self): + """ + Returns the unit's title from the breadcrumb, or None if one is not found + """ + label = self.q(css='.breadcrumbs .nav-item-sequence').text + return label[0].strip() if label else None + # TODO: TNL-6546: Remove method, outline no longer on courseware page @property def sections(self): @@ -531,7 +550,7 @@ class CourseNavPage(PageObject): from common.test.acceptance.pages.lms.course_home import CourseHomePage course_home_page = CourseHomePage(self.browser, self.parent_page.course_id) - self.q(css='.path a').click() + self.q(css='.nav-item-course').click() course_home_page.wait_for_page() return course_home_page @@ -540,38 +559,8 @@ class CourseNavPage(PageObject): """ Return a boolean indicating whether the user is on the section and subsection with the specified titles. - """ - # TODO: TNL-6546: Remove if/else; always use unified_course_view version (if) - if self.unified_course_view: - # breadcrumb location of form: "SECTION_TITLE > SUBSECTION_TITLE > SEQUENTIAL_TITLE" - bread_crumb_current = self.q(css='.position').text - if len(bread_crumb_current) != 1: - self.warning("Could not find the current bread crumb with section and subsection.") - return False - - return bread_crumb_current[0].strip().startswith(section_title + ' > ' + subsection_title + ' > ') - - else: - # This assumes that the currently expanded section is the one we're on - # 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='.course-navigation .chapter.is-open .group-heading').text - current_subsection_list = self.q(css='.course-navigation .chapter-content-container .menu-item.active a p').text - - if len(current_section_list) == 0: - self.warning("Could not find the current section") - return False - - elif len(current_subsection_list) == 0: - self.warning("Could not find current subsection") - return False - - else: - return ( - current_section_list[0].strip() == section_title and - current_subsection_list[0].strip().split('\n')[0] == subsection_title - ) + return self.breadcrumb_section_title == section_title and self.breadcrumb_subsection_title == subsection_title # Regular expression to remove HTML span tags from a string REMOVE_SPAN_TAG_RE = re.compile(r'(.+) <% include_special_exams = settings.FEATURES.get('ENABLE_SPECIAL_EXAMS', False) and (course.enable_proctored_exams or course.enable_timed_exams) @@ -31,7 +31,7 @@ from openedx.features.course_experience import UNIFIED_COURSE_VIEW_FLAG <%block name="header_extras"> -% for template_name in ["image-modal", "sequence-breadcrumbs"]: +% for template_name in ["image-modal"]: @@ -155,11 +155,37 @@ ${HTML(fragment.foot_html())} % endif
+
+
+ +
+
+
-
% if getattr(course, 'entrance_exam_enabled') and \ getattr(course, 'entrance_exam_minimum_score_pct') and \ entrance_exam_current_score is not UNDEFINED: diff --git a/lms/templates/search/search_error.underscore b/lms/templates/search/search_error.underscore index 12b2f36125..35990e2783 100644 --- a/lms/templates/search/search_error.underscore +++ b/lms/templates/search/search_error.underscore @@ -1 +1 @@ -<%= gettext("There was an error, try searching again.") %> +<%- gettext("There was an error, try searching again.") %> diff --git a/openedx/features/course_bookmarks/templates/course_bookmarks/course-bookmarks.html b/openedx/features/course_bookmarks/templates/course_bookmarks/course-bookmarks.html index 30a0997c60..981596f5e5 100644 --- a/openedx/features/course_bookmarks/templates/course_bookmarks/course-bookmarks.html +++ b/openedx/features/course_bookmarks/templates/course_bookmarks/course-bookmarks.html @@ -18,6 +18,7 @@ from django.template.defaultfilters import escapejs from django_comment_client.permissions import has_permission from openedx.core.djangolib.js_utils import dump_js_escaped_json, js_escaped_string from openedx.core.djangolib.markup import HTML +from openedx.features.course_experience import course_home_page_title %> <%block name="bodyclass">course @@ -43,7 +44,7 @@ ${HTML(bookmarks_fragment.foot_html())}
- Course + ${course_home_page_title(course)} ${_('My Bookmarks')} diff --git a/openedx/features/course_experience/__init__.py b/openedx/features/course_experience/__init__.py index 89aba3e6d8..795b329138 100644 --- a/openedx/features/course_experience/__init__.py +++ b/openedx/features/course_experience/__init__.py @@ -3,6 +3,8 @@ Unified course experience settings and helper methods. """ import waffle +from django.utils.translation import ugettext as _ + from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag, WaffleFlagNamespace from request_cache.middleware import RequestCache @@ -18,6 +20,13 @@ WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name='course_experience') UNIFIED_COURSE_TAB_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'unified_course_tab') +def course_home_page_title(course): # pylint: disable=unused-argument + """ + Returns the title for the course home page. + """ + return _('Course') + + def default_course_url_name(request=None): """ Returns the default course URL name for the current user. diff --git a/openedx/features/course_experience/templates/course_experience/course-updates-fragment.html b/openedx/features/course_experience/templates/course_experience/course-updates-fragment.html index c4344d5c57..d17c483087 100644 --- a/openedx/features/course_experience/templates/course_experience/course-updates-fragment.html +++ b/openedx/features/course_experience/templates/course_experience/course-updates-fragment.html @@ -7,6 +7,7 @@ from django.utils.translation import ugettext as _ from openedx.core.djangolib.markup import HTML +from openedx.features.course_experience import course_home_page_title %> <%block name="content"> @@ -18,7 +19,7 @@ from openedx.core.djangolib.markup import HTML
- Course + ${course_home_page_title(course)} ${_('Course Updates')} diff --git a/themes/edx.org/lms/templates/header.html b/themes/edx.org/lms/templates/header.html index 7f2497f15f..7fca6ce95e 100644 --- a/themes/edx.org/lms/templates/header.html +++ b/themes/edx.org/lms/templates/header.html @@ -72,7 +72,7 @@ site_status_msg = get_site_status_msg(course_id) % if user.is_authenticated(): % if not course or disable_courseware_header: % if not nav_hidden or show_program_listing: -