Improve course breadcrumbs

LEARNER-877

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();
             }
         };

common/static/common/templates/sequence-breadcrumbs.underscore

@@ -1,9 +0,0 @@
-<% if (unifiedCourseView) { %>
-    <a href="<%- '/courses/' + courseId + '/course/#' + blockId %>">
-        <span class="fa fa-arrow-circle-prev icon" aria-hidden="true" aria-describedby="outline-description"></span>
-        <span class="sr-only" id="outline-description"><%- gettext('Return to course outline') %></span>
-        <b><%- gettext('Outline') %></b>
-    </a>
-    <span> > </span>
-<% } %>
-<span class="position"><%- pathText %></span>

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));
+    }
+  }
+}

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'</span>(.+)<span')

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

@@ -149,7 +149,10 @@ class ProblemPage(PageObject):
         """
         Click the Show Answer button.
         """
-        self.q(css='.problem .show').click()
+        css = '.problem .show'
+        # First make sure that the button visible and can be clicked on.
+        self.scroll_to_element(css)
+        self.q(css=css).click()
         self.wait_for_ajax()
 
     def is_hint_notification_visible(self):

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

@@ -77,10 +77,6 @@ class CoursewareTest(UniqueCourseTest):
         self.problem_page = ProblemPage(self.browser)  # pylint: disable=attribute-defined-outside-init
         self.assertEqual(self.problem_page.problem_name, 'Test Problem 1')
 
-    def _create_breadcrumb(self, index):
-        """ Create breadcrumb """
-        return ['Test Section {}'.format(index), 'Test Subsection {}'.format(index), 'Test Problem {}'.format(index)]
-
     def test_courseware(self):
         """
         Test courseware if recent visited subsection become unpublished.
@@ -118,11 +114,15 @@ class CoursewareTest(UniqueCourseTest):
         """
         xblocks = self.course_fix.get_nested_xblocks(category="problem")
         for index in range(1, len(xblocks) + 1):
+            test_section_title = 'Test Section {}'.format(index)
+            test_subsection_title = 'Test Subsection {}'.format(index)
+            test_unit_title = 'Test Problem {}'.format(index)
             self.course_home_page.visit()
-            self.course_home_page.outline.go_to_section('Test Section {}'.format(index), 'Test Subsection {}'.format(index))
-            courseware_page_breadcrumb = self.courseware_page.breadcrumb
-            expected_breadcrumb = self._create_breadcrumb(index)  # pylint: disable=no-member
-            self.assertEqual(courseware_page_breadcrumb, expected_breadcrumb)
+            self.course_home_page.outline.go_to_section(test_section_title, test_subsection_title)
+            course_nav = self.courseware_page.nav
+            self.assertEqual(course_nav.breadcrumb_section_title, test_section_title)
+            self.assertEqual(course_nav.breadcrumb_subsection_title, test_subsection_title)
+            self.assertEqual(course_nav.breadcrumb_unit_title, test_unit_title)
 
 
 @attr(shard=9)

lms/djangoapps/courseware/tests/test_views.py

@@ -210,8 +210,8 @@ class IndexQueryTestCase(ModuleStoreTestCase):
     NUM_PROBLEMS = 20
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 10, 143),
-        (ModuleStoreEnum.Type.split, 4, 143),
+        (ModuleStoreEnum.Type.mongo, 10, 144),
+        (ModuleStoreEnum.Type.split, 4, 144),
     )
     @ddt.unpack
     def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count):

lms/djangoapps/courseware/views/index.py

@@ -33,7 +33,7 @@ from openedx.core.djangoapps.user_api.preferences.api import get_user_preference
 from openedx.core.djangoapps.crawlers.models import CrawlersConfig
 from openedx.core.djangoapps.monitoring_utils import set_custom_metrics_for_course_key
 from openedx.features.enterprise_support.api import data_sharing_consent_required
-from openedx.features.course_experience import UNIFIED_COURSE_VIEW_FLAG
+from openedx.features.course_experience import UNIFIED_COURSE_VIEW_FLAG, default_course_url_name
 from request_cache.middleware import RequestCache
 from shoppingcart.models import CourseRegistrationCode
 from student.views import is_course_blocked
@@ -324,9 +324,14 @@ class CoursewareIndex(View):
         Also returns the table of contents for the courseware.
         """
         request = RequestCache.get_current_request()
+        course_url_name = default_course_url_name(request)
+        course_url = reverse(course_url_name, kwargs={'course_id': unicode(self.course.id)})
         courseware_context = {
             'csrf': csrf(self.request)['csrf_token'],
             'course': self.course,
+            'course_url': course_url,
+            'chapter': self.chapter,
+            'section': self.section,
             'init': '',
             'fragment': Fragment(),
             'staff_access': self.is_staff,

lms/static/sass/_build-lms-v1.scss

@@ -14,6 +14,7 @@
 
 // Pattern Library shims
 @import 'edx-pattern-library-shims/base/variables';
+@import 'edx-pattern-library-shims/breadcrumbs';
 @import 'edx-pattern-library-shims/buttons';
 
 // base - elements

lms/static/sass/course/courseware/_courseware.scss

@@ -107,10 +107,21 @@ html.video-fullscreen {
     display: none;
   }
 
+  main {
+    padding: $baseline;
+  }
+
+  .course-content {
+    padding: 0;
+  }
+
+  .courseware-results-wrapper {
+    padding: ($baseline*2) 3%; // percent allows for smaller padding on mobile
+  }
+
   .course-content,
   .courseware-results-wrapper {
     @extend .content;
-    padding: ($baseline*2) 3%; // percent allows for smaller padding on mobile
     line-height: 1.6;
 
     .xblock {

lms/static/sass/shared/_header.scss

@@ -894,13 +894,18 @@
 }
 
 .doc-link {
-    @include float(right);
-    @include margin(($baseline*0.75), ($baseline*0.75), ($baseline*0.75), ($baseline*0.75));
-    font-size: 14px;
-    font-weight: bold;
-    color: $base-font-color;
+  @include float(right);
+  @include margin(($baseline*0.75), ($baseline*0.75), ($baseline*0.75), ($baseline*0.75));
+  font-size: 14px;
+  font-weight: bold;
+  color: $base-font-color;
 
-    &:visited {
-      color: $base-font-color;
-    }
+  &:visited {
+    color: $base-font-color;
+  }
+}
+
+.page-header {
+  padding: $baseline;
+  border-bottom: 1px solid $border-color-2;
 }

lms/templates/courseware/courseware.html

@@ -12,7 +12,7 @@ from django.utils.translation import ugettext as _
 from edxnotes.helpers import is_feature_enabled as is_edxnotes_enabled
 from openedx.core.djangolib.js_utils import js_escaped_string
 from openedx.core.djangolib.markup import HTML
-from openedx.features.course_experience import UNIFIED_COURSE_VIEW_FLAG
+from openedx.features.course_experience import course_home_page_title, UNIFIED_COURSE_VIEW_FLAG
 %>
 <%
   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"]:
 <script type="text/template" id="${template_name}-tpl">
     <%static:include path="common/templates/${template_name}.underscore" />
 </script>
@@ -155,11 +155,37 @@ ${HTML(fragment.foot_html())}
     </div>
 % endif
     <section class="course-content" id="course-content">
+        <header class="page-header has-secondary">
+            <div class="page-header-main">
+                <nav aria-label="${_('Course')}" class="sr-is-focusable" tabindex="-1">
+                    <div class="has-breadcrumbs">
+                        <div class="breadcrumbs">
+                            % if waffle.flag_is_active(request, UNIFIED_COURSE_VIEW_FLAG):
+                                <span class="nav-item nav-item-course">
+                                    <a href="${course_url}">${course_home_page_title(course)}</a>
+                                </span>
+                                <span class="icon fa fa-angle-right" aria-hidden="true"></span>
+                            % endif
+                            % if chapter:
+                                <span class="nav-item nav-item-chapter">
+                                    <a href="${course_url}#${unicode(chapter.location)}">${chapter.display_name_with_default}</a>
+                                </span>
+                                <span class="icon fa fa-angle-right" aria-hidden="true"></span>
+                            % endif
+                            % if section:
+                                <span class="nav-item nav-item-section">
+                                    <a href="${course_url}#${unicode(section.location)}">${section.display_name_with_default}</a>
+                                </span>
+                                <span class="icon fa fa-angle-right" aria-hidden="true"></span>
+                            % endif
+                            <span class="nav-item nav-item-sequence">${sequence_title}</span>
+                        </div>
+                    </div>
+                </nav>
+            </div>
+        </header>
+
         <main id="main" tabindex="-1" aria-label="Content">
-            <div
-              class="path"
-              data-unified-course-view="${'true' if waffle.flag_is_active(request, UNIFIED_COURSE_VIEW_FLAG) else 'false'}"
-            ></div>
             % if getattr(course, 'entrance_exam_enabled') and \
                getattr(course, 'entrance_exam_minimum_score_pct') and \
                entrance_exam_current_score is not UNDEFINED:

lms/templates/search/search_error.underscore

@@ -1 +1 @@
-<%= gettext("There was an error, try searching again.") %>
+<%- gettext("There was an error, try searching again.") %>

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</%block>
@@ -43,7 +44,7 @@ ${HTML(bookmarks_fragment.foot_html())}
                 <div class="has-breadcrumbs">
                     <div class="breadcrumbs">
                         <span class="nav-item">
-                            <a href="${course_url}">Course</a>
+                            <a href="${course_url}">${course_home_page_title(course)}</a>
                         </span>
                         <span class="icon fa fa-angle-right" aria-hidden="true"></span>
                         <span class="nav-item">${_('My Bookmarks')}</span>

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.

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
                 <div class="has-breadcrumbs">
                     <div class="breadcrumbs">
                         <span class="nav-item">
-                            <a href="${course_url}">Course</a>
+                            <a href="${course_url}">${course_home_page_title(course)}</a>
                         </span>
                         <span class="icon fa fa-angle-right" aria-hidden="true"></span>
                         <span class="nav-item">${_('Course Updates')}</span>

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:
-          <nav aria-label="Main" class="nav-main">
+          <nav aria-label="${_('Main')}" class="nav-main">
             <ul class="left list-inline nav-global authenticated">
               % if not nav_hidden:
                 <%block name="navigation_global_links_authenticated">
@@ -121,7 +121,7 @@ site_status_msg = get_site_status_msg(course_id)
       % endif
 
     % else:
-      <nav aria-label="Account" class="nav-account-management">
+      <nav aria-label="${_('Account')}" class="nav-account-management">
         <div class="right nav-courseware list-inline">
           <div class="item nav-courseware-01">
             % if not settings.FEATURES['DISABLE_LOGIN_BUTTON']: