Move search box to header. Add search results to breadcrumbs when you search

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');

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

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"

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):

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

lms/djangoapps/discussion/static/discussion/js/spec/views/discussion_search_view_spec.js

+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('<div class="search-container"></div>');
+                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();
+                });
+            });
+        });
+    }
+);

lms/djangoapps/discussion/static/discussion/js/views/discussion_search_view.js

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

lms/djangoapps/discussion/static/discussion/templates/search.underscore

+<label class="field-label sr-only" for="search" id="search-hint"><%- gettext("Search all posts") %></label>
+<input
+    class="field-input input-text search-input"
+    type="search"
+    name="search"
+    id="search"
+    placeholder="<%- gettext("Search all posts") %>"
+/>
+<button class="btn-brand btn-small search-btn" type="button"><%- gettext("Search") %></button>

lms/djangoapps/discussion/templates/discussion/discussion_board.html

@@ -55,16 +55,20 @@ DiscussionBoardFactory({
          data-flag-moderator="${flag_moderator}"
          data-user-cohort-id="${user_cohort}">
     <header class="page-header has-secondary">
+        ## Breadcrumb navigation
         <div class="page-header-main">
             <div class="sr-is-focusable" tabindex="-1"></div>
             <div class="has-breadcrumbs"></div>
         </div>
         <div class="page-header-secondary">
+            ## Add Post button
             % if has_permission(user, 'create_thread', course.id):
             <div class="form-actions">
                 <button class="btn btn-small new-post-btn">${_("Add a Post")}</button>
             </div>
             % endif
+            ## Search box
+            <div class="forum-search"></div>
         </div>
     </header>
     <div class="page-content">

lms/static/lms/js/spec/main.js

@@ -664,6 +664,7 @@
     var testFiles = [
         'discussion/js/spec/discussion_board_factory_spec.js',
         'discussion/js/spec/discussion_profile_page_factory_spec.js',
+        'discussion/js/spec/views/discussion_search_view_spec.js',
         'discussion/js/spec/views/discussion_user_profile_view_spec.js',
         'lms/js/spec/preview/preview_factory_spec.js',
         'js/spec/api_admin/catalog_preview_spec.js',

lms/static/sass/discussion/_build.scss

@@ -36,5 +36,6 @@ $static-path: '../..' !default;
 @import 'views/thread';
 @import 'views/create-edit-post';
 @import 'views/response';
+@import 'views/search';
 @import 'utilities/developer';
 @import 'utilities/shame';

lms/static/sass/discussion/elements/_navigation.scss

@@ -7,7 +7,30 @@
 }
 
 // ------
-// Header
+// Discussion Forums Page Header
+// ------
+.discussion-board > .page-header {
+    $searchBoxPadding: rem($baseline / 2 + 2);
+    $searchBoxHeight: (rem($baseline) + ($searchBoxPadding * 2));
+
+    div {
+        display: inline-block;
+        vertical-align: middle;
+    }
+
+    .page-header-main {
+        line-height: $searchBoxHeight;
+    }
+
+    .page-header-secondary > .form-actions > button {
+        // Overrides base size set in lms/static/sass/shared-v2/_layouts.scss
+        // Done to match size of UXPL's search box. This is bad, I know.
+        height: $searchBoxHeight !important;
+    }
+}
+
+// ------
+// Topic Listing Header
 // ------
 .forum-nav-header {
   box-sizing: border-box;
@@ -20,10 +43,8 @@
 
 .forum-nav-browse {
     box-sizing: border-box;
-    // TODO: don't use table-cell for layout purposes as it switches the screenreader mode
-    display: table-cell;
+    width: 100%;
     vertical-align: middle;
-    width: auto;
     padding: 11px;
     border: 0;
     border-radius: 0;
@@ -54,28 +75,6 @@
   @include margin-left($baseline/4);
 }
 
-.forum-nav-search {
-  box-sizing: border-box;
-  // TODO: don't use table-cell for layout purposes as it switches the screenreader mode
-  display: table-cell;
-  position: relative;
-  vertical-align: middle;
-  width: 50%;
-  padding: ($baseline/4);
-}
-
-.forum-nav-search .icon {
-  font-size: $forum-small-font-size;
-  position: absolute;
-  margin-top: -6px;
-  top: 50%;
-  @include right($baseline/4 + 1px + $baseline / 4); // Wrapper padding + border + input padding
-}
-
-.forum-nav-search-input {
-  width: 100%;
-}
-
 // -----------
 // Browse menu
 // -----------

lms/static/sass/discussion/utilities/_shame.scss

@@ -7,28 +7,6 @@
   color: $black !important;
 }
 
-// Override global label rules
-.forum-nav-search label {
-  margin-bottom: 0;
-}
-
-// Override global input rules
-.forum-nav-search-input {
-  @include padding-left($baseline/4 !important);
-  @include padding-right($baseline/2 + 12px !important); // Leave room for icon
-  box-shadow: none !important;
-  border: 1px solid $forum-color-border !important;
-  border-radius: $forum-border-radius !important;
-  height: auto !important;
-  font-size: $forum-small-font-size !important;
-}
-
-// Firefox does not compute the correct containing box for absolute positioning
-// of .forum-nav-search .icon, so there's an extra div to make it happy
-.forum-nav-search-ff-position-fix {
-  position: relative;
-}
-
 // Temporary breadcrumbs
 .has-breadcrumbs {
   .breadcrumbs {

lms/static/sass/discussion/views/_search.scss

+.forum-search {
+    display: inline-block;
+    margin-left: $baseline;
+
+    .search-input {
+        width: input-width(short);
+    }
+}

lms/templates/discussion/_thread_list_template.html

@@ -11,15 +11,6 @@
             <span class="forum-nav-browse-current">${_("All Discussions")}</span>
             <span class="forum-nav-browse-drop-arrow" aria-hidden="true">▾</span>
         </button>
-        <form class="forum-nav-search">
-            <div class="forum-nav-search-ff-position-fix">
-                <label>
-                    <span class="sr">${_("Search all posts")}</span>
-                    <input class="forum-nav-search-input" id="forum-nav-search" type="text" placeholder="${_("Search all posts")}">
-                    <span class="icon fa fa-search" aria-hidden="true"></span>
-                </label>
-            </div>
-        </form>
     </div>
     <%include file="_filter_dropdown.html" />
     <div class="forum-nav-thread-list-wrapper" id="sort-filter-wrapper" tabindex="-1">

lms/templates/ux/reference/course-skeleton.html

@@ -48,16 +48,15 @@
                 ##  - update the Pattern Library's markup to match
                 <div class="page-header-search">
                     <form class="search-form" role="search">
-                        <div class="search-box">
-                            <input class="search-field" type="text" value=""
-                                   aria-label="Search all the things" placeholder="Search all the things">
-                            <button type="button" class="btn action action-clear" aria-label="Clear search">
-                                <span class="fa fa-times-circle" aria-hidden="true"></span>
-                            </button>
-                        </div>
-                        <button type="submit" class="btn-brand action action-search" aria-label="Search items">
-                            <span class="fa fa-search" aria-hidden="true"></span>
-                        </button>
+                        <label class="field-label sr-only" for="search" id="search-hint">Search all the things</label>
+                        <input
+                            class="field-input input-text search-input"
+                            type="search"
+                            name="search"
+                            id="search"
+                            placeholder="Search all the things"
+                        />
+                        <button class="btn-brand btn-small search-btn" type="button">Search</button>
                     </form>
                 </div>
             </div>