From df35415ec1f0efd3cd12909547722017176cc286 Mon Sep 17 00:00:00 2001 From: Brian Jacobel Date: Mon, 15 Aug 2016 11:44:07 -0400 Subject: [PATCH] Remove forum-nav-header and wire breadcrumbs to replace it TNL-5186 --- .../views/discussion_thread_list_view.js | 71 +------------------ .../view/discussion_thread_list_view_spec.js | 6 -- .../discussion/discussion-home.underscore | 2 +- .../test/acceptance/pages/lms/discussion.py | 5 +- .../tests/discussion/test_discussion.py | 1 - .../discussion/js/discussion_board_factory.js | 2 +- .../js/views/discussion_fake_breadcrumbs.js | 3 +- .../templates/fake-breadcrumbs.underscore | 4 +- .../sass/discussion/elements/_navigation.scss | 47 +----------- .../sass/discussion/utilities/_shame.scss | 9 ++- .../discussion/_thread_list_template.html | 11 --- 11 files changed, 16 insertions(+), 145 deletions(-) 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 f5db60c1c8..4d96e2eb97 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 @@ -91,7 +91,6 @@ } DiscussionThreadListView.prototype.events = { - 'click .forum-nav-browse': 'toggleBrowseMenu', 'keypress .forum-nav-browse-filter-input': function(event) { return DiscussionUtil.ignoreEnterKey(event); }, @@ -193,15 +192,9 @@ TODO fix this entire chain of events */ - DiscussionThreadListView.prototype.addAndSelectThread = function(thread) { - var commentableId, menuItem, + var commentableId = thread.get('commentable_id'), self = this; - commentableId = thread.get('commentable_id'); - menuItem = this.$('.forum-nav-browse-menu-item[data-discussion-id]').filter(function() { - return $(this).data('discussion-id') === commentableId; - }); - this.setCurrentTopicDisplay(this.getPathText(menuItem)); return this.retrieveDiscussion(commentableId, function() { return self.trigger('thread:created', thread.get('id')); }); @@ -438,7 +431,6 @@ DiscussionThreadListView.prototype.showBrowseMenu = function() { if (!this.isBrowseMenuVisible()) { - this.$('.forum-nav-browse').addClass('is-active'); this.$('.forum-nav-browse-menu-wrapper').show(); this.$('.forum-nav-thread-list-wrapper').hide(); $('.forum-nav-browse-filter-input').focus(); @@ -449,7 +441,6 @@ DiscussionThreadListView.prototype.hideBrowseMenu = function() { if (this.isBrowseMenuVisible()) { - this.$('.forum-nav-browse').removeClass('is-active'); this.$('.forum-nav-browse-menu-wrapper').hide(); this.$('.forum-nav-thread-list-wrapper').show(); $('body').unbind('click', this.hideBrowseMenu); @@ -524,64 +515,6 @@ } }; - DiscussionThreadListView.prototype.setCurrentTopicDisplay = function(text) { - return this.$('.forum-nav-browse-current').text(this.fitName(text)); - }; - - DiscussionThreadListView.prototype.getNameWidth = function(name) { - var $test, width; - $test = $('
'); - $test.css({ - 'font-size': this.$('.forum-nav-browse-current').css('font-size'), - opacity: 0, - position: 'absolute', - left: -1000, - top: -1000 - }); - $('body').append($test); - $test.text(name); - width = $test.width(); - $test.remove(); - return width; - }; - - DiscussionThreadListView.prototype.fitName = function(name) { - var partialName, path, prefix, rawName, width, x; - this.maxNameWidth = this.$('.forum-nav-browse').width() - - this.$('.forum-nav-browse .icon').outerWidth(true) - - this.$('.forum-nav-browse-drop-arrow').outerWidth(true); - width = this.getNameWidth(name); - if (width < this.maxNameWidth) { - return name; - } - path = (function() { - var _i, _len, _ref, _results; - _ref = name.split('/'); - _results = []; - for (_i = 0, _len = _ref.length; _i < _len; _i++) { - x = _ref[_i]; - _results.push(x.replace(/^\s+|\s+$/g, '')); - } - return _results; - }()); - prefix = ''; - while (path.length > 1) { - prefix = gettext('…') + '/'; - path.shift(); - partialName = prefix + path.join('/'); - if (this.getNameWidth(partialName) < this.maxNameWidth) { - return partialName; - } - } - rawName = path[0]; - name = prefix + rawName; - while (this.getNameWidth(name) > this.maxNameWidth) { - rawName = rawName.slice(0, rawName.length - 1); - name = prefix + rawName + gettext('…'); - } - return name; - }; - DiscussionThreadListView.prototype.selectTopicHandler = function(event) { event.preventDefault(); return this.selectTopic($(event.target)); @@ -592,7 +525,6 @@ this.hideBrowseMenu(); $item = $target.closest('.forum-nav-browse-menu-item'); - this.setCurrentTopicDisplay(this.getPathText($item)); this.trigger('topic:selected', this.getBreadcrumbText($item)); if ($item.hasClass('forum-nav-browse-menu-all')) { @@ -666,7 +598,6 @@ 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); 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 14bd89d1da..7ccf8018fa 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 @@ -618,12 +618,6 @@ describe('selecting an item', function() { var testSelectionRequest; - it('should change the button text', function() { - setupAjax(); - $('.forum-nav-browse-menu-following .forum-nav-browse-title').click(); - return expect($('.forum-nav-browse-current').text()).toEqual("Posts I'm Following"); - }); - it('should show/hide the cohort selector', function() { var self = this; DiscussionSpecHelper.makeModerator(); diff --git a/common/static/common/templates/discussion/discussion-home.underscore b/common/static/common/templates/discussion/discussion-home.underscore index 3bd825a747..3982228650 100644 --- a/common/static/common/templates/discussion/discussion-home.underscore +++ b/common/static/common/templates/discussion/discussion-home.underscore @@ -18,7 +18,7 @@ <%- gettext("Find discussions") %> - <%- gettext("Use the Discussion Topics menu to find specific topics.") %> + <%- gettext("Use the All Topics menu to find specific topics.") %> diff --git a/common/test/acceptance/pages/lms/discussion.py b/common/test/acceptance/pages/lms/discussion.py index 3b5fbd9b18..14acc9c26c 100644 --- a/common/test/acceptance/pages/lms/discussion.py +++ b/common/test/acceptance/pages/lms/discussion.py @@ -378,7 +378,7 @@ class DiscussionSortPreferencePage(CoursePage): def show_all_discussions(self): """ Show the list of all discussions. """ - self.q(css=".forum-nav-browse").click() + self.q(css=".all-topics").click() def get_selected_sort_preference(self): """ @@ -423,7 +423,7 @@ class DiscussionTabSingleThreadPage(CoursePage): def show_all_discussions(self): """ Show the list of all discussions. """ - self.q(css=".forum-nav-browse").click() + self.q(css=".all-topics").click() def close_open_thread(self): with self.thread_page.secondary_action_menu_open(".thread-main-wrapper"): @@ -443,6 +443,7 @@ class DiscussionTabSingleThreadPage(CoursePage): Click specific thread on the list. """ thread_selector = "li[data-id='{}']".format(thread_id) + self.show_all_discussions() self.q(css=thread_selector).first.click() EmptyPromise( lambda: self._thread_is_rendered_successfully(thread_id), diff --git a/common/test/acceptance/tests/discussion/test_discussion.py b/common/test/acceptance/tests/discussion/test_discussion.py index 78e17c2503..8927dc12bb 100644 --- a/common/test/acceptance/tests/discussion/test_discussion.py +++ b/common/test/acceptance/tests/discussion/test_discussion.py @@ -374,7 +374,6 @@ class DiscussionTabMultipleThreadTest(BaseDiscussionTestCase): self.thread_ids[1] ) self.thread_page_1.visit() - self.thread_page_1.show_all_discussions() def setup_multiple_threads(self, thread_count): threads = [] 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 9cdc4ea7aa..bea9bd56dc 100644 --- a/lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js +++ b/lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js @@ -78,7 +78,7 @@ searchBox.clearSearch(); this.model.set('contents', []); router.navigate('', {trigger: true}); - router.nav.selectTopic($('.forum-nav-browse-menu-all')); + router.nav.toggleBrowseMenu(event); } } }).render(); diff --git a/lms/djangoapps/discussion/static/discussion/js/views/discussion_fake_breadcrumbs.js b/lms/djangoapps/discussion/static/discussion/js/views/discussion_fake_breadcrumbs.js index 7c9b3311db..fd48c9454c 100644 --- a/lms/djangoapps/discussion/static/discussion/js/views/discussion_fake_breadcrumbs.js +++ b/lms/djangoapps/discussion/static/discussion/js/views/discussion_fake_breadcrumbs.js @@ -11,10 +11,11 @@ define([ 'backbone', + 'gettext', 'edx-ui-toolkit/js/utils/html-utils', 'text!discussion/templates/fake-breadcrumbs.underscore' ], - function(Backbone, HtmlUtils, breadcrumbsTemplate) { + function(Backbone, gettext, HtmlUtils, breadcrumbsTemplate) { var DiscussionFakeBreadcrumbs = Backbone.View.extend({ initialize: function() { this.template = HtmlUtils.template(breadcrumbsTemplate); diff --git a/lms/djangoapps/discussion/static/discussion/templates/fake-breadcrumbs.underscore b/lms/djangoapps/discussion/static/discussion/templates/fake-breadcrumbs.underscore index e65d2f29db..557473bd04 100644 --- a/lms/djangoapps/discussion/static/discussion/templates/fake-breadcrumbs.underscore +++ b/lms/djangoapps/discussion/static/discussion/templates/fake-breadcrumbs.underscore @@ -1,6 +1,8 @@
- All Topics + + <%- gettext('All Topics') %> + <% contents.forEach(function(content) { %> diff --git a/lms/static/sass/discussion/elements/_navigation.scss b/lms/static/sass/discussion/elements/_navigation.scss index 191bca49ba..356ee81ddb 100644 --- a/lms/static/sass/discussion/elements/_navigation.scss +++ b/lms/static/sass/discussion/elements/_navigation.scss @@ -29,52 +29,6 @@ } } -// ------ -// Topic Listing Header -// ------ -.forum-nav-header { - box-sizing: border-box; - // TODO: don't use table for layout purposes as it switches the screenreader mode - display: table; - border-bottom: 1px solid $forum-color-border; - width: 100%; - background-color: $gray-l3; -} - -.forum-nav-browse { - box-sizing: border-box; - width: 100%; - vertical-align: middle; - padding: 11px; - border: 0; - border-radius: 0; - background: transparent; - box-shadow: none; - - &:hover, - &:focus, - &.is-active { - // converted to button from a; - // need importants to overwrite later button styles - box-shadow: none !important; - background-image: none !important; - background-color: $gray-l5 !important; - } - - .icon { - @include margin-right($baseline/4); - font-size: $forum-base-font-size; - } -} - -.forum-nav-browse-current { - font-size: $forum-small-font-size; -} - -.forum-nav-browse-drop-arrow { - @include margin-left($baseline/4); -} - // ----------- // Browse menu // ----------- @@ -157,6 +111,7 @@ padding: ($baseline/4) ($baseline/2); color: $black; text-align: right; + @include border-radius($forum-border-radius, $forum-border-radius, 0, 0); } .forum-nav-filter-main { diff --git a/lms/static/sass/discussion/utilities/_shame.scss b/lms/static/sass/discussion/utilities/_shame.scss index 9dff5abf08..43250bdbd9 100644 --- a/lms/static/sass/discussion/utilities/_shame.scss +++ b/lms/static/sass/discussion/utilities/_shame.scss @@ -2,15 +2,14 @@ // navigation - header // ------------------- -// Override global a rules -.forum-nav-browse { - color: $black !important; -} - // Temporary breadcrumbs .has-breadcrumbs { .breadcrumbs { margin: 5px 0 0 0; + + .all-topics .fa { + @include margin-right(10px); + } } } diff --git a/lms/templates/discussion/_thread_list_template.html b/lms/templates/discussion/_thread_list_template.html index 0094e70546..d0d73f7ce4 100644 --- a/lms/templates/discussion/_thread_list_template.html +++ b/lms/templates/discussion/_thread_list_template.html @@ -1,17 +1,6 @@ <%page expression_filter="h"/> <%! from django.utils.translation import ugettext as _ %>