diff --git a/common/static/common/js/discussion/views/discussion_thread_list_view.js b/common/static/common/js/discussion/views/discussion_thread_list_view.js index c2764e86e1..6375c7aeb0 100644 --- a/common/static/common/js/discussion/views/discussion_thread_list_view.js +++ b/common/static/common/js/discussion/views/discussion_thread_list_view.js @@ -98,8 +98,6 @@ 'keyup .forum-nav-browse-filter-input': 'filterTopics', 'click .forum-nav-browse-menu-wrapper': 'ignoreClick', 'click .forum-nav-browse-title': 'selectTopicHandler', - 'keydown .forum-nav-search-input': 'performSearch', - 'click .fa-search': 'performSearch', 'change .forum-nav-sort-control': 'sortThreads', 'click .forum-nav-thread-link': 'threadSelected', 'click .forum-nav-load-more-link': 'loadMorePages', @@ -591,7 +589,6 @@ DiscussionThreadListView.prototype.selectTopic = function($target) { var allItems, discussionIds, $item; this.hideBrowseMenu(); - this.clearSearch(); $item = $target.closest('.forum-nav-browse-menu-item'); this.setCurrentTopicDisplay(this.getPathText($item)); @@ -666,22 +663,15 @@ return this.retrieveFirstPage(event); }; - DiscussionThreadListView.prototype.performSearch = function(event) { - /* - event.which 13 represent the Enter button - */ - - var text; - if (event.which === 13 || event.type === 'click') { - event.preventDefault(); - this.hideBrowseMenu(); - this.setCurrentTopicDisplay(gettext('Search Results')); - text = this.$('.forum-nav-search-input').val(); - this.searchFor(text); - } + DiscussionThreadListView.prototype.performSearch = function($searchInput) { + this.hideBrowseMenu(); + this.setCurrentTopicDisplay(gettext('Search Results')); + // trigger this event so the breadcrumbs can update as well + this.trigger('search:initiated'); + this.searchFor($searchInput.val(), $searchInput); }; - DiscussionThreadListView.prototype.searchFor = function(text) { + DiscussionThreadListView.prototype.searchFor = function(text, $searchInput) { var url, self = this; this.clearSearchAlerts(); this.clearFilters(); @@ -696,7 +686,7 @@ */ return DiscussionUtil.safeAjax({ - $elem: this.$('.forum-nav-search-input'), + $elem: $searchInput, data: { text: text }, @@ -790,12 +780,6 @@ }); }; - DiscussionThreadListView.prototype.clearSearch = function() { - this.$('.forum-nav-search-input').val(''); - this.current_search = ''; - return this.clearSearchAlerts(); - }; - DiscussionThreadListView.prototype.clearFilters = function() { this.$('.forum-nav-filter-main-control').val('all'); return this.$('.forum-nav-filter-cohort-control').val('all'); diff --git a/common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js b/common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js index d55a902d80..6deea3d84f 100644 --- a/common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js +++ b/common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js @@ -374,24 +374,6 @@ }); }); - describe('Search events', function() { - it('perform search when enter pressed inside search textfield', function() { - setupAjax(); - spyOn(this.view, 'searchFor'); - this.view.$el.find('.forum-nav-search-input').trigger($.Event('keydown', { - which: 13 - })); - return expect(this.view.searchFor).toHaveBeenCalled(); - }); - - it('perform search when search icon is clicked', function() { - setupAjax(); - spyOn(this.view, 'searchFor'); - this.view.$el.find('.fa-search').click(); - return expect(this.view.searchFor).toHaveBeenCalled(); - }); - }); - describe('username search', function() { var setAjaxResults; @@ -582,14 +564,6 @@ return expectBrowseMenuVisible(false); }); - it('should hide when a search is executed', function() { - setupAjax(); - $('.forum-nav-search-input').trigger($.Event('keydown', { - which: 13 - })); - return expectBrowseMenuVisible(false); - }); - it('should hide when a category is clicked', function() { $('.forum-nav-browse-title')[0].click(); return expectBrowseMenuVisible(false); @@ -636,13 +610,6 @@ describe('selecting an item', function() { var testSelectionRequest; - it('should clear the search box', function() { - setupAjax(); - $('.forum-nav-search-input').val('foobar'); - $('.forum-nav-browse-menu-following .forum-nav-browse-title').click(); - return expect($('.forum-nav-search-input').val()).toEqual(''); - }); - it('should change the button text', function() { setupAjax(); $('.forum-nav-browse-menu-following .forum-nav-browse-title').click(); diff --git a/common/test/acceptance/pages/lms/discussion.py b/common/test/acceptance/pages/lms/discussion.py index 39abb9984c..3b5fbd9b18 100644 --- a/common/test/acceptance/pages/lms/discussion.py +++ b/common/test/acceptance/pages/lms/discussion.py @@ -678,7 +678,7 @@ class DiscussionTabHomePage(CoursePage, DiscussionPageMixin): return self.q(css=".discussion-body section.home-header").present def perform_search(self, text="dummy"): - self.q(css=".forum-nav-search-input").fill(text + chr(10)) + self.q(css=".search-input").fill(text + chr(10)) EmptyPromise( self.is_ajax_finished, "waiting for server to return result" diff --git a/common/test/acceptance/tests/discussion/test_discussion.py b/common/test/acceptance/tests/discussion/test_discussion.py index 14742745a6..78e17c2503 100644 --- a/common/test/acceptance/tests/discussion/test_discussion.py +++ b/common/test/acceptance/tests/discussion/test_discussion.py @@ -267,6 +267,14 @@ class DiscussionNavigationTest(BaseDiscussionTestCase): self.thread_page.q(css=".breadcrumbs .nav-item")[0].click() self.assertEqual(len(self.thread_page.q(css=".breadcrumbs .nav-item")), 1) + def test_breadcrumbs_clear_search(self): + self.thread_page.q(css=".search-input").fill("search text") + self.thread_page.q(css=".search-btn").click() + + # Verify that clicking the first breadcrumb clears your search + self.thread_page.q(css=".breadcrumbs .nav-item")[0].click() + self.assertEqual(self.thread_page.q(css=".search-input").text[0], "") + @attr(shard=2) class DiscussionTabSingleThreadTest(BaseDiscussionTestCase, DiscussionResponsePaginationTestMixin): diff --git a/lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js b/lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js index 103bd130e4..9cdc4ea7aa 100644 --- a/lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js +++ b/lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js @@ -7,9 +7,10 @@ 'backbone', 'discussion/js/discussion_router', 'discussion/js/views/discussion_fake_breadcrumbs', + 'discussion/js/views/discussion_search_view', 'common/js/discussion/views/new_post_view' ], - function($, Backbone, DiscussionRouter, DiscussionFakeBreadcrumbs, NewPostView) { + function($, Backbone, DiscussionRouter, DiscussionFakeBreadcrumbs, DiscussionSearchView, NewPostView) { return function(options) { var userInfo = options.user_info, sortPreference = options.sort_preference, @@ -22,7 +23,9 @@ newPostView, router, breadcrumbs, - BreadcrumbsModel; + BreadcrumbsModel, + searchBox, + routerEvents; // TODO: Perhaps eliminate usage of global variables when possible window.DiscussionUtil.loadRoles(options.roles); @@ -53,6 +56,13 @@ }); router.start(); + // Initialize and render search box + searchBox = new DiscussionSearchView({ + el: $('.forum-search'), + threadListView: router.nav + }).render(); + + // Initialize and render breadcrumbs BreadcrumbsModel = Backbone.Model.extend({ defaults: { contents: [] @@ -65,6 +75,7 @@ events: { 'click .all-topics': function(event) { event.preventDefault(); + searchBox.clearSearch(); this.model.set('contents', []); router.navigate('', {trigger: true}); router.nav.selectTopic($('.forum-nav-browse-menu-all')); @@ -72,9 +83,23 @@ } }).render(); - // Add new breadcrumbs when the user selects topics - router.nav.on('topic:selected', function(topic) { - breadcrumbs.model.set('contents', topic); + routerEvents = { + // Add new breadcrumbs and clear search box when the user selects topics + 'topic:selected': function(topic) { + breadcrumbs.model.set('contents', topic); + }, + // Clear search box when a thread is selected + 'thread:selected': function() { + searchBox.clearSearch(); + }, + // Add 'Search Results' to breadcrumbs when user searches + 'search:initiated': function() { + breadcrumbs.model.set('contents', ['Search Results']); + } + }; + + Object.keys(routerEvents).forEach(function(key) { + router.nav.on(key, routerEvents[key]); }); }; }); diff --git a/lms/djangoapps/discussion/static/discussion/js/spec/views/discussion_search_view_spec.js b/lms/djangoapps/discussion/static/discussion/js/spec/views/discussion_search_view_spec.js new file mode 100644 index 0000000000..f7c05f4daf --- /dev/null +++ b/lms/djangoapps/discussion/static/discussion/js/spec/views/discussion_search_view_spec.js @@ -0,0 +1,36 @@ +define([ + 'jquery', + 'edx-ui-toolkit/js/utils/constants', + 'discussion/js/views/discussion_search_view' +], + function($, constants, DiscussionSearchView) { + 'use strict'; + + describe('DiscussionSearchView', function() { + var view; + beforeEach(function() { + setFixtures('
'); + view = new DiscussionSearchView({ + el: $('.search-container'), + threadListView: { + performSearch: jasmine.createSpy() + } + }).render(); + }); + + describe('Search events', function() { + it('perform search when enter pressed inside search textfield', function() { + view.$el.find('.search-input').trigger($.Event('keydown', { + which: constants.keyCodes.enter + })); + expect(view.threadListView.performSearch).toHaveBeenCalled(); + }); + + it('perform search when search icon is clicked', function() { + view.$el.find('.search-btn').click(); + expect(view.threadListView.performSearch).toHaveBeenCalled(); + }); + }); + }); + } +); diff --git a/lms/djangoapps/discussion/static/discussion/js/views/discussion_search_view.js b/lms/djangoapps/discussion/static/discussion/js/views/discussion_search_view.js new file mode 100644 index 0000000000..9d1dd00620 --- /dev/null +++ b/lms/djangoapps/discussion/static/discussion/js/views/discussion_search_view.js @@ -0,0 +1,50 @@ +(function(define) { + 'use strict'; + + define([ + 'underscore', + 'backbone', + 'edx-ui-toolkit/js/utils/html-utils', + 'edx-ui-toolkit/js/utils/constants', + 'text!discussion/templates/search.underscore' + ], + function(_, Backbone, HtmlUtils, constants, searchTemplate) { + /* + * TODO: Much of the actual search functionality still takes place in discussion_thread_list_view.js + * Because of how it's structured there, extracting it is a massive task. Significant refactoring is needed + * in order to clean up that file and make it possible to break its logic into files like this one. + */ + var searchView = Backbone.View.extend({ + events: { + 'keydown .search-input': 'performSearch', + 'click .search-btn': 'performSearch', + 'topic:selected': 'clearSearch' + }, + initialize: function(options) { + _.extend(this, _.pick(options, 'threadListView')); + + this.template = HtmlUtils.template(searchTemplate); + this.threadListView = options.threadListView; + + this.listenTo(this.model, 'change', this.render); + this.render(); + }, + render: function() { + HtmlUtils.setHtml(this.$el, this.template()); + return this; + }, + performSearch: function(event) { + if (event.which === constants.keyCodes.enter || event.type === 'click') { + event.preventDefault(); + this.threadListView.performSearch($('.search-input', this.$el)); + } + }, + clearSearch: function() { + this.$('.search-input').val(''); + this.threadListView.clearSearchAlerts(); + } + }); + + return searchView; + }); +}).call(this, define || RequireJS.define); diff --git a/lms/djangoapps/discussion/static/discussion/templates/search.underscore b/lms/djangoapps/discussion/static/discussion/templates/search.underscore new file mode 100644 index 0000000000..3f289ce2a5 --- /dev/null +++ b/lms/djangoapps/discussion/static/discussion/templates/search.underscore @@ -0,0 +1,9 @@ + +" +/> + diff --git a/lms/djangoapps/discussion/templates/discussion/discussion_board.html b/lms/djangoapps/discussion/templates/discussion/discussion_board.html index b77b843ed8..98e7aae04f 100644 --- a/lms/djangoapps/discussion/templates/discussion/discussion_board.html +++ b/lms/djangoapps/discussion/templates/discussion/discussion_board.html @@ -55,16 +55,20 @@ DiscussionBoardFactory({ data-flag-moderator="${flag_moderator}" data-user-cohort-id="${user_cohort}">