Skip to content

E
edx-platform
Commit 7b5be9cb by Andy Armstrong Committed by Diana Huang
Options

Address code review feedback

parent 2c000349
Showing with 43 additions and 26 deletions
common/test/acceptance/pages/lms/course_home.py
......@@ -4,6 +4,7 @@ LMS Course Home page object
from bok_choy.page_object import PageObject
from common.test.acceptance.pages.lms.bookmarks import BookmarksPage
from common.test.acceptance.pages.lms.course_page import CoursePage
from common.test.acceptance.pages.lms.courseware import CoursewarePage
......@@ -25,6 +26,12 @@ class CourseHomePage(CoursePage):
# TODO: TNL-6546: Remove the following
self.unified_course_view = False
def click_bookmarks_button(self):
""" Click on Bookmarks button """
self.q(css='.bookmarks-list-button').first.click()
bookmarks_page = BookmarksPage(self.browser, self.course_id)
bookmarks_page.visit()
class CourseOutlinePage(PageObject):
"""
......
common/test/acceptance/pages/lms/courseware.py
......@@ -7,6 +7,7 @@ from bok_choy.promise import EmptyPromise
import re
from selenium.webdriver.common.action_chains import ActionChains
from common.test.acceptance.pages.lms.bookmarks import BookmarksPage
from common.test.acceptance.pages.lms.course_page import CoursePage
......@@ -310,11 +311,12 @@ class CoursewarePage(CoursePage):
self.q(css='.bookmark-button').first.click()
EmptyPromise(lambda: self.bookmark_button_state != previous_state, "Bookmark button toggled").fulfill()
def click_bookmarks_button(self, wait_for_results=True):
# TODO: TNL-6546: Remove this helper function
def click_bookmarks_button(self):
""" Click on Bookmarks button """
self.q(css='.bookmarks-list-button').first.click()
if wait_for_results:
EmptyPromise(lambda: self.q(css='#my-bookmarks').present, "Bookmarks results present").fulfill()
bookmarks_page = BookmarksPage(self.browser, self.course_id)
bookmarks_page.visit()
class CoursewareSequentialTabPage(CoursePage):
......
common/test/acceptance/tests/lms/test_lms.py
......@@ -25,6 +25,7 @@ from common.test.acceptance.pages.common.logout import LogoutPage
from common.test.acceptance.pages.lms import BASE_URL
from common.test.acceptance.pages.lms.account_settings import AccountSettingsPage
from common.test.acceptance.pages.lms.auto_auth import AutoAuthPage
from common.test.acceptance.pages.lms.bookmarks import BookmarksPage
from common.test.acceptance.pages.lms.create_mode import ModeCreationPage
from common.test.acceptance.pages.lms.course_home import CourseHomePage
from common.test.acceptance.pages.lms.course_info import CourseInfoPage
......@@ -853,6 +854,12 @@ class HighLevelTabTest(UniqueCourseTest):
self.course_home_page.outline.go_to_section('Test Section 2', 'Test Subsection 3')
self.assertTrue(self.courseware_page.nav.is_on_section('Test Section 2', 'Test Subsection 3'))
# Verify that we can navigate to the bookmarks page
self.course_home_page.visit()
self.course_home_page.click_bookmarks_button()
bookmarks_page = BookmarksPage(self.browser, self.course_id)
self.assertTrue(bookmarks_page.is_browser_on_page())
@attr('a11y')
def test_course_home_a11y(self):
self.course_home_page.visit()
......
lms/templates/courseware/course_navigation.html
......@@ -20,7 +20,7 @@ if active_page is None and active_page_context is not UNDEFINED:
def selected(is_selected):
return "selected" if is_selected else ""
show_preview_menu = not disable_preview_menu and staff_access and active_page in ["course", "courseware", "info", "progress"]
show_preview_menu = not disable_preview_menu and staff_access and active_page in ["courseware", "info", "progress"]
cohorted_user_partition = get_cohorted_user_partition(course)
masquerade_user_name = masquerade.user_name if masquerade else None
masquerade_group_id = masquerade.group_id if masquerade else None
......
openedx/features/course_bookmarks/static/course_bookmarks/fixtures/bookmarks.html
<div class="course-view container" id="course-container">
<header class="page-header has-secondary">
<div class="page-header-main">
<nav aria-label="Discussions" class="sr-is-focusable" tabindex="-1">
<nav aria-label="My Bookmarks" class="sr-is-focusable" tabindex="-1">
<div class="has-breadcrumbs"><div class="breadcrumbs">
<span class="nav-item">
<a href="/courses/course-v1:test-course/course/">Course</a>
......
openedx/features/course_bookmarks/static/course_bookmarks/js/spec/bookmarks_list_view_spec.js
......@@ -73,7 +73,7 @@ define(['backbone',
expect(bookmarksView.$('.bookmarks-empty-detail-title').text().trim()).toBe(
'Use bookmarks to help you easily return to courseware pages. ' +
'To bookmark a page, click on "Bookmark this page" beneath the unit title.'
'To bookmark a page, click "Bookmark this page" under the page title.'
);
expect(bookmarksView.$('.paging-header').length).toBe(0);
......
openedx/features/course_bookmarks/static/course_bookmarks/templates/bookmarks-list.underscore
......@@ -46,7 +46,7 @@
</h3>
<div class="bookmarks-empty-detail">
<span class="bookmarks-empty-detail-title">
<%- gettext('Use bookmarks to help you easily return to courseware pages. To bookmark a page, click on "Bookmark this page" beneath the unit title.') %>
<%- gettext('Use bookmarks to help you easily return to courseware pages. To bookmark a page, click "Bookmark this page" under the page title.') %>
</span>
</div>
</div>
......
openedx/features/course_bookmarks/templates/course_bookmarks/course-bookmarks.html
......@@ -27,11 +27,11 @@ from openedx.core.djangolib.markup import HTML
<%include file="../courseware/course_navigation.html" args="active_page='courseware'" />
<%block name="head_extra">
${HTML(outline_fragment.head_html())}
${HTML(bookmarks_fragment.head_html())}
</%block>
<%block name="footer_extra">
${HTML(outline_fragment.foot_html())}
${HTML(bookmarks_fragment.foot_html())}
</%block>
<%block name="content">
......@@ -39,20 +39,21 @@ ${HTML(outline_fragment.foot_html())}
<header class="page-header has-secondary">
## Breadcrumb navigation
<div class="page-header-main">
<nav aria-label="Discussions" class="sr-is-focusable" tabindex="-1">
<div class="has-breadcrumbs"><div class="breadcrumbs">
<span class="nav-item">
<a href="${course_url}">Course</a>
</span>
<span class="icon fa fa-angle-right" aria-hidden="true"></span>
<span class="nav-item">My Bookmarks</span>
</div>
<nav aria-label="${_('My Bookmarks')}" class="sr-is-focusable" tabindex="-1">
<div class="has-breadcrumbs">
<div class="breadcrumbs">
<span class="nav-item">
<a href="${course_url}">Course</a>
</span>
<span class="icon fa fa-angle-right" aria-hidden="true"></span>
<span class="nav-item">${_('My Bookmarks')}</span>
</div>
</div>
</nav>
</div>
</header>
<div class="page-content">
${HTML(outline_fragment.body_html())}
${HTML(bookmarks_fragment.body_html())}
</div>
</div>
</%block>
openedx/features/course_bookmarks/views/course_bookmarks.py
......@@ -23,7 +23,7 @@ from xmodule.modulestore.django import modulestore
class CourseBookmarksView(View):
"""
The home page for a course.
View showing the user's bookmarks for a course.
"""
@method_decorator(login_required)
@method_decorator(ensure_csrf_cookie)
......@@ -31,7 +31,7 @@ class CourseBookmarksView(View):
@method_decorator(ensure_valid_course_key)
def get(self, request, course_id):
"""
Displays the home page for the specified course.
Displays the user's bookmarks for the specified course.
Arguments:
request: HTTP request
......@@ -43,14 +43,14 @@ class CourseBookmarksView(View):
course_url = reverse(course_url_name, kwargs={'course_id': unicode(course.id)})
# Render the bookmarks list as a fragment
outline_fragment = CourseBookmarksFragmentView().render_to_fragment(request, course_id=course_id)
bookmarks_fragment = CourseBookmarksFragmentView().render_to_fragment(request, course_id=course_id)
# Render the entire unified course view
# Render the course bookmarks page
context = {
'csrf': csrf(request)['csrf_token'],
'course': course,
'course_url': course_url,
'outline_fragment': outline_fragment,
'bookmarks_fragment': bookmarks_fragment,
'disable_courseware_js': True,
'uses_pattern_library': True,
}
......@@ -63,7 +63,7 @@ class CourseBookmarksFragmentView(EdxFragmentView):
"""
def render_to_fragment(self, request, course_id=None, **kwargs):
"""
Renders the course outline as a fragment.
Renders the user's course bookmarks as a fragment.
"""
course_key = CourseKey.from_string(course_id)
course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True)
......
openedx/features/course_experience/templates/course_experience/course-home.html
......@@ -40,10 +40,10 @@ ${HTML(outline_fragment.foot_html())}
<header class="page-header has-secondary">
<div class="page-header-secondary">
<div class="form-actions">
<a class="btn" href="${reverse('courseware', kwargs={'course_id': unicode(course.id.to_deprecated_string())})}">
<a class="btn action-resume-course" href="${reverse('courseware', kwargs={'course_id': unicode(course.id.to_deprecated_string())})}">
${_("Resume Course")}
</a>
<a class="btn" href="${reverse('openedx.course_bookmarks.home', args=[course.id])}">
<a class="btn action-show-bookmarks" href="${reverse('openedx.course_bookmarks.home', args=[course.id])}">
${_("Bookmarks")}
</a>
</div>
......
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