diff --git a/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee b/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee index 48c941cb13..26160ff8f2 100644 --- a/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee +++ b/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee @@ -69,13 +69,14 @@ describe "DiscussionThreadListView", -> -
- Sort by: - +
+ + +
@@ -149,7 +150,7 @@ describe "DiscussionThreadListView", -> expect(view.$el.find(".forum-nav-thread:nth-child(1) .forum-nav-thread-title").text()).toEqual(sort_order[0]) expect(view.$el.find(".forum-nav-thread:nth-child(2) .forum-nav-thread-title").text()).toEqual(sort_order[1]) expect(view.$el.find(".forum-nav-thread:nth-child(3) .forum-nav-thread-title").text()).toEqual(sort_order[2]) - expect(view.$el.find(".sort-bar a.active").text()).toEqual(type) + expect(view.$el.find(".forum-nav-sort-control").val()).toEqual(type) describe "thread rendering should be correct", -> checkRender = (threads, type, sort_order) -> @@ -157,6 +158,8 @@ describe "DiscussionThreadListView", -> view = makeView(discussion) view.render() checkThreadsOrdering(view, sort_order, type) + expect(view.$el.find(".forum-nav-thread-comments-count:visible").length).toEqual(if type == "votes" then 0 else 3) + expect(view.$el.find(".forum-nav-thread-votes-count:visible").length).toEqual(if type == "votes" then 3 else 0) it "with sort preference date", -> checkRender(@threads, "date", [ "Thread1", "Thread2", "Thread3"]) @@ -167,12 +170,13 @@ describe "DiscussionThreadListView", -> it "with sort preference comments", -> checkRender(@threads, "comments", [ "Thread3", "Thread2", "Thread1"]) - describe "Sort click should be correct", -> + describe "Sort change should be correct", -> changeSorting = (threads, selected_type, new_type, sort_order) -> discussion = new Discussion(_.map(threads, (thread) -> new Thread(thread)), {pages: 1, sort: selected_type}) view = makeView(discussion) view.render() - expect(view.$el.find(".sort-bar a.active").text()).toEqual(selected_type) + sortControl = view.$el.find(".forum-nav-sort-control") + expect(sortControl.val()).toEqual(selected_type) sorted_threads = [] if new_type == 'date' sorted_threads = [threads[0], threads[1], threads[2]] @@ -186,9 +190,8 @@ describe "DiscussionThreadListView", -> ) {always: ->} ) - view.$el.find(".sort-bar a[data-sort='"+new_type+"']").click() + sortControl.val(new_type).change() expect($.ajax).toHaveBeenCalled() - expect(view.sortBy).toEqual(new_type) checkThreadsOrdering(view, sort_order, new_type) it "with sort preference date", -> diff --git a/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee index 5e27aa8f3d..88f5b99067 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee @@ -6,18 +6,17 @@ if Backbone? "click .browse": "toggleTopicDrop" "keydown .post-search-field": "performSearch" "focus .post-search-field": "showSearch" - "click .sort-bar a": "sortThreads" + "change .forum-nav-sort-control": "sortThreads" "click .browse-topic-drop-menu": "filterTopic" "click .browse-topic-drop-search-input": "ignoreClick" "click .forum-nav-thread-link": "threadSelected" "click .forum-nav-load-more-link": "loadMorePages" - "change .cohort-options": "chooseCohort" + "change .forum-nav-filter-cohort-control": "chooseCohort" 'keyup .browse-topic-drop-search-input': DiscussionFilter.filterDrop initialize: -> @displayedCollection = new Discussion(@collection.models, pages: @collection.pages) @collection.on "change", @reloadDisplayedCollection - @sortBy = "date" @discussionIds="" @collection.on "reset", (discussion) => board = $(".current-board").html() @@ -30,7 +29,6 @@ if Backbone? # @filterTopic($.Event("filter", {'target': target[0]})) @collection.on "add", @addAndSelectThread @sidebar_padding = 10 - @sidebar_header_height = 87 @boardName @template = _.template($("#thread-list-template").html()) @current_search = "" @@ -71,6 +69,7 @@ if Backbone? current_el = @$(".forum-nav-thread[data-id=#{thread_id}]") active = current_el.has(".forum-nav-thread-link.is-active").length != 0 current_el.replaceWith(content) + @showMetadataAccordingToSort() if active @setActiveThread(thread_id) @@ -88,7 +87,7 @@ if Backbone? scrollTop = $(window).scrollTop(); windowHeight = $(window).height(); - discussionBody = $(".discussion-article") + discussionBody = $(".discussion-column") discussionsBodyTop = if discussionBody[0] then discussionBody.offset().top discussionsBodyBottom = discussionsBodyTop + discussionBody.outerHeight() @@ -111,7 +110,9 @@ if Backbone? sidebarHeight = Math.min(sidebarHeight + 1, discussionBody.outerHeight()) sidebar.css 'height', sidebarHeight - @$('.forum-nav-thread-list').css('height', (sidebarHeight - @sidebar_header_height - 4) + 'px') + browseSearchHeight = @$(".browse-search").outerHeight() + refineBarHeight = @$(".forum-nav-refine-bar").outerHeight() + @$('.forum-nav-thread-list').css('height', (sidebarHeight - browseSearchHeight - refineBarHeight - 2) + 'px') # Because we want the behavior that when the body is clicked the menu is @@ -123,6 +124,7 @@ if Backbone? render: -> @timer = 0 @$el.html(@template()) + @$(".forum-nav-sort-control").val(@collection.sort_preference) $(window).bind "load", @updateSidebar $(window).bind "scroll", @updateSidebar @@ -131,9 +133,6 @@ if Backbone? @displayedCollection.on "reset", @renderThreads @displayedCollection.on "thread:remove", @renderThreads @renderThreads() - sort_element = @$('.sort-bar a[data-sort="' + this.collection.sort_preference + '"]') - sort_element.attr('aria-checked',true) - sort_element.addClass('active') @ renderThreads: => @@ -144,10 +143,24 @@ if Backbone? rendered.append content @$(".forum-nav-thread-list").html(rendered.html()) + @showMetadataAccordingToSort() + @renderMorePages() @updateSidebar() @trigger "threads:rendered" + showMetadataAccordingToSort: () => + # Ensure that threads display metadata appropriate for the current sort + voteCounts = @$(".forum-nav-thread-votes-count") + commentCounts = @$(".forum-nav-thread-comments-count") + voteCounts.hide() + commentCounts.hide() + switch @$(".forum-nav-sort-control").val() + when "date", "comments" + commentCounts.show() + when "votes" + voteCounts.show() + renderMorePages: -> if @displayedCollection.hasMorePages() @$(".forum-nav-thread-list").append("
  • " + gettext("Load more") + "
    • ") @@ -197,17 +210,17 @@ if Backbone? @renderThreads() DiscussionUtil.discussionAlert(gettext("Sorry"), gettext("We had some trouble loading more threads. Please try again.")) - @collection.retrieveAnotherPage(@mode, options, {sort_key: @sortBy}, error) + @collection.retrieveAnotherPage(@mode, options, {sort_key: @$(".forum-nav-sort-control").val()}, error) renderThread: (thread) => content = $(_.template($("#thread-list-item-template").html())(thread.toJSON())) - unreadCount = thread.get('unread_comments_count') + unreadCount = thread.get('unread_comments_count') + (if thread.get("read") then 0 else 1) if unreadCount > 0 content.find('.forum-nav-thread-comments-count').attr( "data-tooltip", interpolate( ngettext('%(unread_count)s new comment', '%(unread_count)s new comments', unreadCount), - {unread_count: thread.get('unread_comments_count')}, + {unread_count: unreadCount}, true ) ) @@ -340,26 +353,26 @@ if Backbone? if discussionId == "#all" @discussionIds = "" @$(".post-search-field").val("") - @$('.cohort').show() + @$('.forum-nav-filter-cohort').show() @retrieveAllThreads() else if discussionId == "#flagged" @discussionIds = "" @$(".post-search-field").val("") - @$('.cohort').hide() + @$('.forum-nav-filter-cohort').hide() @retrieveFlaggedThreads() else if discussionId == "#following" @retrieveFollowed(event) - @$('.cohort').hide() + @$('.forum-nav-filter-cohort').hide() else discussionIds = _.map item.find(".board-name[data-discussion_id]"), (board) -> $(board).data("discussion_id").id if $(event.target).attr('cohorted') == "True" - @retrieveDiscussions(discussionIds, "function(){$('.cohort').show();}") + @retrieveDiscussions(discussionIds, "function(){$('.forum-nav-filter-cohort').show();}") else - @retrieveDiscussions(discussionIds, "function(){$('.cohort').hide();}") + @retrieveDiscussions(discussionIds, "function(){$('.forum-nav-filter-cohort').hide();}") chooseCohort: (event) -> - @group_id = @$('.cohort-options :selected').val() + @group_id = @$('.forum-nav-filter-cohort-control :selected').val() @collection.current_page = 0 @collection.reset() @loadMorePages(event) @@ -400,14 +413,8 @@ if Backbone? @loadMorePages(event) sortThreads: (event) -> - activeSort = @$(".sort-bar a.active") - activeSort.removeClass("active") - activeSort.attr("aria-checked", "false") - newSort = $(event.target) - newSort.addClass("active") - newSort.attr("aria-checked", "true") - @sortBy = newSort.data("sort") - @displayedCollection.setSortComparator(@sortBy) + @displayedCollection.setSortComparator(@$(".forum-nav-sort-control").val()) + @retrieveFirstPage(event) performSearch: (event) -> diff --git a/common/test/acceptance/pages/lms/discussion.py b/common/test/acceptance/pages/lms/discussion.py index 431784716c..cd812a1dbd 100644 --- a/common/test/acceptance/pages/lms/discussion.py +++ b/common/test/acceptance/pages/lms/discussion.py @@ -179,19 +179,20 @@ class DiscussionSortPreferencePage(CoursePage): """ Return true if the browser is on the right page else false. """ - return self.q(css="body.discussion .sort-bar").present + return self.q(css="body.discussion .forum-nav-sort-control").present - def get_selected_sort_preference_text(self): + def get_selected_sort_preference(self): """ Return the text of option that is selected for sorting. """ - return self.q(css="body.discussion .sort-bar a.active").text[0].lower() + options = self.q(css="body.discussion .forum-nav-sort-control option") + return options.filter(lambda el: el.is_selected())[0].get_attribute("value") def change_sort_preference(self, sort_by): """ Change the option of sorting by clicking on new option. """ - self.q(css="body.discussion .sort-bar a[data-sort='{0}']".format(sort_by)).click() + self.q(css="body.discussion .forum-nav-sort-control option[value='{0}']".format(sort_by)).click() def refresh_page(self): """ diff --git a/common/test/acceptance/tests/test_discussion.py b/common/test/acceptance/tests/test_discussion.py index d7e7be8c29..a71f0f9b31 100644 --- a/common/test/acceptance/tests/test_discussion.py +++ b/common/test/acceptance/tests/test_discussion.py @@ -509,7 +509,7 @@ class DiscussionSortPreferenceTest(UniqueCourseTest): """ Test to check the default sorting preference of user. (Default = date ) """ - selected_sort = self.sort_page.get_selected_sort_preference_text() + selected_sort = self.sort_page.get_selected_sort_preference() self.assertEqual(selected_sort, "date") def test_change_sort_preference(self): @@ -520,7 +520,7 @@ class DiscussionSortPreferenceTest(UniqueCourseTest): for sort_type in ["votes", "comments", "date"]: self.assertNotEqual(selected_sort, sort_type) self.sort_page.change_sort_preference(sort_type) - selected_sort = self.sort_page.get_selected_sort_preference_text() + selected_sort = self.sort_page.get_selected_sort_preference() self.assertEqual(selected_sort, sort_type) def test_last_preference_saved(self): @@ -531,8 +531,8 @@ class DiscussionSortPreferenceTest(UniqueCourseTest): for sort_type in ["votes", "comments", "date"]: self.assertNotEqual(selected_sort, sort_type) self.sort_page.change_sort_preference(sort_type) - selected_sort = self.sort_page.get_selected_sort_preference_text() + selected_sort = self.sort_page.get_selected_sort_preference() self.assertEqual(selected_sort, sort_type) self.sort_page.refresh_page() - selected_sort = self.sort_page.get_selected_sort_preference_text() + selected_sort = self.sort_page.get_selected_sort_preference() self.assertEqual(selected_sort, sort_type) diff --git a/lms/static/sass/discussion/_discussion.scss b/lms/static/sass/discussion/_discussion.scss index f07f15b0a2..0be8886ac2 100644 --- a/lms/static/sass/discussion/_discussion.scss +++ b/lms/static/sass/discussion/_discussion.scss @@ -888,71 +888,6 @@ body.discussion { } } - .sort-bar { - height: auto; - min-height: 27px; - border-bottom: 1px solid #a3a3a3; - @include linear-gradient(top, rgba(255, 255, 255, .3), rgba(255, 255, 255, 0)); - background-color: #aeaeae; - box-shadow: 0 1px 0 rgba(255, 255, 255, .2) inset; - - span, - a { - font-size: 9px; - font-weight: bold; - line-height: 25px; - color: #333; - text-transform: uppercase; - text-shadow: 0 1px 0 rgba(255, 255, 255, .4); - } - - .sort-label { - display: block; - float: left; - margin: 0 $baseline/2; - } - - li { - float: left; - margin: 4px 4px 0 0; - } - - a { - display: block; - height: 18px; - padding: 0 9px; - border-radius: 19px; - color: #333; - line-height: 17px; - - &:hover, &:focus { - @include linear-gradient(top, rgba(255, 255, 255, .4), rgba(255, 255, 255, .2)); - color: #333; - } - - &.active { - @include linear-gradient(top, rgba(0, 0, 0, .3), rgba(0, 0, 0, 0)); - background-color: #999; - color: $white; - text-shadow: 0 -1px 0 rgba(0, 0, 0, .3); - box-shadow: 0 1px 0 rgba(255, 255, 255, 0.2), 0 1px 1px rgba(0, 0, 0, .2) inset; - } - - } - .group-filter-label { - width: 40px; - margin-left: $baseline/2; - } - - .group-filter-select { - margin: 5px 0px 5px 5px; - width: 80px; - font-size:10px; - background: transparent; - border-color: #ccc; - } - } - .bottom-post-status { padding: 30px; font-size: 20px; diff --git a/lms/static/sass/discussion/elements/_navigation.scss b/lms/static/sass/discussion/elements/_navigation.scss index 7a0b94e2e8..2a1a26df84 100644 --- a/lms/static/sass/discussion/elements/_navigation.scss +++ b/lms/static/sass/discussion/elements/_navigation.scss @@ -1,3 +1,34 @@ +// ------------------- +// Sort and filter bar +// ------------------- +.forum-nav-refine-bar { + @include clearfix(); + @include font-size(11); + border-bottom: 1px solid $gray-l3; + background-color: $gray-l5; + padding: ($baseline/4) ($baseline/2); + color: $black; +} + +%forum-nav-select { + border: none; + max-width: 100%; + background-color: transparent; + font: inherit; +} + +.forum-nav-filter-cohort-control { + @extend %forum-nav-select; +} + +.forum-nav-sort { + float: right; +} + +.forum-nav-sort-control { + @extend %forum-nav-select; +} + // ----------- // Thread list // ----------- diff --git a/lms/static/sass/discussion/utilities/_shame.scss b/lms/static/sass/discussion/utilities/_shame.scss index 991cec5df0..0f069889fa 100644 --- a/lms/static/sass/discussion/utilities/_shame.scss +++ b/lms/static/sass/discussion/utilities/_shame.scss @@ -1,3 +1,16 @@ +// -------------------------------- +// navigation - sort and filter bar +// -------------------------------- + +// Override global span rules +.forum-nav-sort-label { + color: inherit; +} + +// -------------------------------- +// navigation - thread list +// -------------------------------- + // The sidebar class does a lot of things that we don't want in the thread list; // the following rules contain styling that is necessary and would otherwise // reside in elements/_navigation.scss if the sidebar styling did not make the diff --git a/lms/templates/discussion/_thread_list_template.html b/lms/templates/discussion/_thread_list_template.html index 0ea9964642..e1c95c8197 100644 --- a/lms/templates/discussion/_thread_list_template.html +++ b/lms/templates/discussion/_thread_list_template.html @@ -26,23 +26,27 @@
    -
    - ${_("Sort by:")} - - +
    %if is_course_cohorted and is_moderator: - ${_("Show:")} - + %for c in cohorts: - + %endfor %endif + + + +
    diff --git a/lms/templates/discussion/_underscore_templates.html b/lms/templates/discussion/_underscore_templates.html index f7b9bbd57a..d6dc58ffca 100644 --- a/lms/templates/discussion/_underscore_templates.html +++ b/lms/templates/discussion/_underscore_templates.html @@ -260,11 +260,12 @@ <% js_block = u""" var fmt; + // Counts in data do not include the post itself, but the UI should var data = {{ 'span_sr_open': '', 'span_close': '', - 'unread_comments_count': unread_comments_count, - 'comments_count': comments_count + 'unread_comments_count': unread_comments_count + (read ? 0 : 1), + 'comments_count': comments_count + 1 }}; if (unread_comments_count > 0) {{ fmt = '{markup_with_unread}';