From 2a984c60b565bbb0bfa7a1632a3e0eb0795721a4 Mon Sep 17 00:00:00 2001 From: Brian Jacobel Date: Thu, 8 Sep 2016 16:19:39 -0400 Subject: [PATCH] Replace discussion topic selector with a native dropdown TNL-5165 --- .../views/discussion_topic_menu_view.js | 187 ++++-------------- .../js/discussion/views/new_post_view.js | 2 +- .../view/discussion_thread_edit_view_spec.js | 4 +- .../view/discussion_topic_menu_view_spec.js | 91 +-------- .../discussion/view/new_post_view_spec.js | 52 ++--- .../new-post-menu-category.underscore | 7 +- .../discussion/new-post-menu-entry.underscore | 4 +- .../templates/discussion/topic.underscore | 24 +-- lms/static/sass/discussion/_mixins.scss | 1 - .../sass/discussion/utilities/_shame.scss | 21 +- .../discussion/views/_create-edit-post.scss | 89 +-------- 11 files changed, 102 insertions(+), 380 deletions(-) diff --git a/common/static/common/js/discussion/views/discussion_topic_menu_view.js b/common/static/common/js/discussion/views/discussion_topic_menu_view.js index 7079972541..30be5945cc 100644 --- a/common/static/common/js/discussion/views/discussion_topic_menu_view.js +++ b/common/static/common/js/discussion/views/discussion_topic_menu_view.js @@ -1,110 +1,74 @@ +/* globals Backbone, _ */ + (function() { 'use strict'; if (Backbone) { this.DiscussionTopicMenuView = Backbone.View.extend({ events: { - 'click .post-topic-button': 'toggleTopicDropdown', - 'click .topic-menu-wrapper': 'handleTopicEvent', - 'click .topic-filter-label': 'ignoreClick', - 'keyup .topic-filter-input': 'filterDrop' + 'change .post-topic': 'handleTopicEvent' }, attributes: { - 'class': 'post-field' + class: 'post-field' }, initialize: function(options) { this.course_settings = options.course_settings; this.currentTopicId = options.topicId; - this.maxNameWidth = 26; _.bindAll(this, - 'toggleTopicDropdown', 'handleTopicEvent', 'hideTopicDropdown', 'ignoreClick' + 'handleTopicEvent' ); return this; }, - /** - * When the menu is expanded, a click on the body element (outside of the menu) or on a menu element - * should close the menu except when the target is the search field. To accomplish this, we have to ignore - * clicks on the search field by stopping the propagation of the event. - */ - ignoreClick: function(event) { - event.stopPropagation(); - return this; - }, - render: function() { - var context = _.clone(this.course_settings.attributes); + var $general, + context = _.clone(this.course_settings.attributes); + context.topics_html = this.renderCategoryMap(this.course_settings.get('category_map')); - this.$el.html(_.template($('#topic-template').html())(context)); - this.dropdownButton = this.$('.post-topic-button'); - this.topicMenu = this.$('.topic-menu-wrapper'); - this.selectedTopic = this.$('.js-selected-topic'); - this.hideTopicDropdown(); + edx.HtmlUtils.setHtml(this.$el, edx.HtmlUtils.template($('#topic-template').html())(context)); + + $general = this.$('.post-topic option:contains(General)'); + if (this.getCurrentTopicId()) { - this.setTopic(this.$('.topic-title').filter( - '[data-discussion-id="' + this.getCurrentTopicId() + '"]') - ); + this.setTopic(this.$('.post-topic option').filter( + '[data-discussion-id="' + this.getCurrentTopicId() + '"]' + )); + } else if ($general) { + this.setTopic($general); } else { - this.setTopic(this.$('button.topic-title').first()); + this.setTopic(this.$('.post-topic option').first()); } return this.$el; }, renderCategoryMap: function(map) { - var category_template = _.template($('#new-post-menu-category-template').html()), - entry_template = _.template($('#new-post-menu-entry-template').html()); + var categoryTemplate = edx.HtmlUtils.template($('#new-post-menu-category-template').html()), + entryTemplate = edx.HtmlUtils.template($('#new-post-menu-entry-template').html()), + mappedCategorySnippets = _.map(map.children, function(name) { + var entry, + html = ''; + if (_.has(map.entries, name)) { + entry = map.entries[name]; + html = entryTemplate({ + text: name, + id: entry.id, + is_cohorted: entry.is_cohorted + }); + } else { // subcategory + html = categoryTemplate({ + text: name, + entries: this.renderCategoryMap(map.subcategories[name]) + }); + } + return html; + }, this); - return _.map(map.children, function(name) { - var html = '', entry; - if (_.has(map.entries, name)) { - entry = map.entries[name]; - html = entry_template({ - text: name, - id: entry.id, - is_cohorted: entry.is_cohorted - }); - } else { // subcategory - html = category_template({ - text: name, - entries: this.renderCategoryMap(map.subcategories[name]) - }); - } - return html; - }, this).join(''); - }, - - toggleTopicDropdown: function(event) { - event.preventDefault(); - event.stopPropagation(); - if (this.menuOpen) { - this.hideTopicDropdown(); - } else { - this.showTopicDropdown(); - } - return this; - }, - - showTopicDropdown: function() { - this.menuOpen = true; - this.dropdownButton.addClass('dropped'); - this.topicMenu.show(); - $(document.body).on('click.topicMenu', this.hideTopicDropdown); - return this; - }, - - hideTopicDropdown: function() { - this.menuOpen = false; - this.dropdownButton.removeClass('dropped'); - this.topicMenu.hide(); - $(document.body).off('click.topicMenu'); - return this; + return edx.HtmlUtils.joinHtml.apply(null, mappedCategorySnippets); }, handleTopicEvent: function(event) { - event.preventDefault(); - event.stopPropagation(); - this.setTopic($(event.target)); + this.setTopic($('option:selected', event.target)); return this; }, @@ -112,9 +76,8 @@ if ($target.data('discussion-id')) { this.topicText = this.getFullTopicName($target); this.currentTopicId = $target.data('discussion-id'); - this.setSelectedTopicName(this.topicText); + $target.prop('selected', true); this.trigger('thread:topic_change', $target); - this.hideTopicDropdown(); } return this; }, @@ -123,9 +86,6 @@ return this.currentTopicId; }, - setSelectedTopicName: function(text) { - return this.selectedTopic.text(this.fitName(text)); - }, /** * Return full name for the `topicElement` if it is passed. * Otherwise, full name for the current topic will be returned. @@ -136,74 +96,13 @@ var name; if (topicElement) { name = topicElement.html(); - _.each(topicElement.parents('.topic-submenu'), function(item) { - name = $(item).siblings('.topic-title').text() + ' / ' + name; + _.each(topicElement.parents('optgroup'), function(item) { + name = $(item).attr('label') + ' / ' + name; }); return name; } else { return this.topicText; } - }, - - // @TODO move into utils.coffee - fitName: function(name) { - var ellipsisText = gettext('…'), - partialName, path, rawName, concatedLength; - - if (name.length < this.maxNameWidth) { - return name; - } else { - path = _.map(name.split('/'), function(item) { - return item.replace(/^\s+|\s+$/g, ''); - }); - while (path.length > 1) { - path.shift(); - partialName = ellipsisText + '/' + path.join('/'); - if (partialName.length < this.maxNameWidth) { - return partialName; - } - } - rawName = path[0]; - name = ellipsisText + ' / ' + rawName; - if (name.length > this.maxNameWidth) { - concatedLength = ellipsisText.length + ' / '.length + ellipsisText.length; - rawName = rawName.slice(0, this.maxNameWidth - concatedLength); - name = ellipsisText + ' / ' + rawName + ' ' + ellipsisText; - } - } - return name; - }, - - // TODO: this helper class duplicates functionality in DiscussionThreadListView.filterTopics - // for use with a very similar category dropdown in the New Post form. The two menus' implementations - // should be merged into a single reusable view. - filterDrop: function(e) { - var $drop, $items, query; - $drop = $(e.target).parents('.topic-menu-wrapper'); - query = $(e.target).val(); - $items = $drop.find('.topic-menu-item'); - - if (query.length === 0) { - $items.removeClass('hidden'); - return; - } - - $items.addClass('hidden'); - $items.each(function(_index, item) { - var path, pathText, pathTitles; - path = $(item).parents('.topic-menu-item').andSelf(); - pathTitles = path.children('.topic-title').map(function(_, elem) { - return $(elem).text(); - }).get(); - pathText = pathTitles.join(' / ').toLowerCase(); - if (query.split(' ').every(function(term) { - return pathText.search(term.toLowerCase()) !== -1; - })) { - $(item).removeClass('hidden'); - $(item).find('.topic-menu-item').removeClass('hidden'); - $(item).parents('.topic-menu-item').removeClass('hidden'); - } - }); } }); } diff --git a/common/static/common/js/discussion/views/new_post_view.js b/common/static/common/js/discussion/views/new_post_view.js index f06e5d6cac..a6e4d74f72 100644 --- a/common/static/common/js/discussion/views/new_post_view.js +++ b/common/static/common/js/discussion/views/new_post_view.js @@ -55,7 +55,7 @@ }); this.$el.html(_.template($('#new-post-template').html())(context)); threadTypeTemplate = _.template($('#thread-type-template').html()); - if ($('.js-group-select').is(':disabled')) { + if ($('.js-group-select').prop('disabled')) { $('.group-selector-wrapper').addClass('disabled'); } this.addField(threadTypeTemplate({ diff --git a/common/static/common/js/spec/discussion/view/discussion_thread_edit_view_spec.js b/common/static/common/js/spec/discussion/view/discussion_thread_edit_view_spec.js index 534e1d4150..c431ae1b2f 100644 --- a/common/static/common/js/spec/discussion/view/discussion_thread_edit_view_spec.js +++ b/common/static/common/js/spec/discussion/view/discussion_thread_edit_view_spec.js @@ -47,10 +47,10 @@ } }; }); - view.$el.find('.topic-title').filter(function(idx, el) { return $(el).data('discussionId') === newTopicId; - }).click(); // set new topic + }).prop('selected', true); // set new topic + view.$('.post-topic').trigger('change'); view.$('.edit-post-title').val('changed thread title'); // set new title if (discussionMode !== 'inline') { view.$("label[for$='post-type-discussion']").click(); // set new thread type diff --git a/common/static/common/js/spec/discussion/view/discussion_topic_menu_view_spec.js b/common/static/common/js/spec/discussion/view/discussion_topic_menu_view_spec.js index 0ffc3b750b..2e6ced3019 100644 --- a/common/static/common/js/spec/discussion/view/discussion_topic_menu_view_spec.js +++ b/common/static/common/js/spec/discussion/view/discussion_topic_menu_view_spec.js @@ -1,4 +1,4 @@ -/* globals DiscussionTopicMenuView, DiscussionSpecHelper, DiscussionCourseSettings */ +/* globals DiscussionTopicMenuView, DiscussionSpecHelper, DiscussionCourseSettings, _ */ (function() { 'use strict'; describe('DiscussionTopicMenuView', function() { @@ -10,21 +10,6 @@ }, options); this.view = new DiscussionTopicMenuView(options); this.view.render().appendTo('#fixture-element'); - this.defaultTextWidth = this.completeText.length; - }; - - this.openMenu = function() { - var menuWrapper = this.view.$('.topic-menu-wrapper'); - expect(menuWrapper).toBeHidden(); - this.view.$el.find('.post-topic-button').first().click(); - expect(menuWrapper).toBeVisible(); - }; - - this.closeMenu = function() { - var menuWrapper = this.view.$('.topic-menu-wrapper'); - expect(menuWrapper).toBeVisible(); - this.view.$el.find('.post-topic-button').first().click(); - expect(menuWrapper).toBeHidden(); }; DiscussionSpecHelper.setUpGlobals(); @@ -89,89 +74,25 @@ }, 'is_cohorted': true }); - this.parentCategoryText = 'Basic Question Types'; - this.selectedOptionText = 'Selection From Options'; - this.completeText = this.parentCategoryText + ' / ' + this.selectedOptionText; }); - it('completely show parent category and sub-category', function() { - var dropdownText; + it('defaults to first subtopic', function() { this.createTopicView(); - this.view.maxNameWidth = this.defaultTextWidth + 1; - this.view.$el.find('.topic-menu-entry').first().click(); - dropdownText = this.view.$el.find('.js-selected-topic').text(); - expect(this.completeText).toEqual(dropdownText); - }); - - it('truncation happens with specific title lengths', function() { - var dropdownText; - this.createTopicView(); - this.view.$el.find('.topic-menu-entry')[2].click(); - dropdownText = this.view.$el.find('.js-selected-topic').text(); - expect(dropdownText).toEqual('…/Very long category name'); - - this.view.$el.find('.topic-menu-entry')[5].click(); - dropdownText = this.view.$el.find('.js-selected-topic').text(); - expect(dropdownText).toEqual('… / What Are Your Goals f …'); - }); - - it('truncation happens with longer title lengths', function() { - var dropdownText; - this.createTopicView(); - this.view.$el.find('.topic-menu-entry')[3].click(); - dropdownText = this.view.$el.find('.js-selected-topic').text(); - expect(dropdownText).toEqual('… / Very very very very l …'); + expect(this.view.$el.find('option.topic-title:selected').text()).toEqual('Selection From Options'); }); it('titles are escaped before display', function() { - var dropdownText; this.createTopicView(); - this.view.$el.find('.topic-menu-entry')[4].click(); - dropdownText = this.view.$el.find('.js-selected-topic').text(); - expect(dropdownText).toContain('em>'); - }); - - it('broken span doesn\'t occur', function() { - var dropdownText; - this.createTopicView(); - this.view.maxNameWidth = this.selectedOptionText.length + 100; - this.view.$el.find('button.topic-title').first().click(); - dropdownText = this.view.$el.find('.js-selected-topic').text(); - expect(dropdownText.indexOf('/ span>')).toEqual(-1); + $(this.view.$el.find('option.topic-title')[4]).prop('selected', true); + expect(this.view.$el.find('option.topic-title:selected').text()).toContain(''); }); it('appropriate topic is selected if `topicId` is passed', function() { - var completeText = this.parentCategoryText + ' / Numerical Input', - dropdownText; this.createTopicView({ topicId: 'c49f0dfb8fc94c9c8d9999cc95190c56' }); - this.view.maxNameWidth = this.defaultTextWidth + 1; this.view.render(); - dropdownText = this.view.$el.find('.js-selected-topic').text(); - expect(completeText).toEqual(dropdownText); - }); - - it("defaults to the first topic if you don't click one", function() { - this.createTopicView(); - expect( - this.view.$el.find('.js-selected-topic').text() - ).toMatch( - this.view.$el.find('.topic-menu-entry')[0].innerHTML - ); - }); - - it('click outside of the dropdown close it', function() { - this.createTopicView(); - this.openMenu(); - $(document.body).click(); - expect(this.view.$('.topic-menu-wrapper')).toBeHidden(); - }); - - it('can toggle the menu', function() { - this.createTopicView(); - this.openMenu(); - this.closeMenu(); + expect(this.view.$el.find('option.topic-title:selected').text()).toEqual('Numerical Input'); }); }); }).call(this); diff --git a/common/static/common/js/spec/discussion/view/new_post_view_spec.js b/common/static/common/js/spec/discussion/view/new_post_view_spec.js index 0582fbcc61..1708fd081d 100644 --- a/common/static/common/js/spec/discussion/view/new_post_view_spec.js +++ b/common/static/common/js/spec/discussion/view/new_post_view_spec.js @@ -34,29 +34,33 @@ describe('cohort selector', function() { beforeEach(function() { this.course_settings = new DiscussionCourseSettings({ - 'category_map': { - 'children': ['Topic', 'General'], - 'entries': { - 'Topic': { - 'is_cohorted': true, - 'id': 'topic' + category_map: { + children: ['Topic', 'General', 'Not Cohorted'], + entries: { + Topic: { + is_cohorted: true, + id: 'topic' }, - 'General': { - 'is_cohorted': false, - 'id': 'general' + General: { + is_cohorted: true, + id: 'general' + }, + 'Not Cohorted': { + is_cohorted: false, + id: 'not-cohorted' } } }, - 'allow_anonymous': false, - 'allow_anonymous_to_peers': false, - 'is_cohorted': true, - 'cohorts': [ + allow_anonymous: false, + allow_anonymous_to_peers: false, + is_cohorted: true, + cohorts: [ { - 'id': 1, - 'name': 'Cohort1' + id: 1, + name: 'Cohort1' }, { - 'id': 2, - 'name': 'Cohort2' + id: 2, + name: 'Cohort2' } ] }); @@ -71,20 +75,24 @@ it('is not visible to students', function() { return checkVisibility(this.view, false, false, true); }); - it('allows TAs to see the cohort selector', function() { + it('allows TAs to see the cohort selector when the topic is cohorted', function() { DiscussionSpecHelper.makeTA(); return checkVisibility(this.view, true, false, true); }); - it('allows moderators to see the cohort selector', function() { + it('allows moderators to see the cohort selector when the topic is cohorted', function() { DiscussionSpecHelper.makeModerator(); return checkVisibility(this.view, true, false, true); }); it('only enables the cohort selector when applicable', function() { DiscussionSpecHelper.makeModerator(); checkVisibility(this.view, true, false, true); - $('.topic-menu-entry:contains(General)').click(); + + $('option:contains(Not Cohorted)').prop('selected', true); + $('.post-topic').trigger('change'); checkVisibility(this.view, true, true, false); - $('.topic-menu-entry:contains(Topic)').click(); + + $('option:contains(Topic)').prop('selected', true); + $('.post-topic').trigger('change'); return checkVisibility(this.view, true, false, false); }); it('allows the user to make a cohort selection', function() { @@ -218,7 +226,7 @@ expect(view.$('.js-anon')).not.toBeChecked(); expect(view.$('.js-anon-peers')).not.toBeChecked(); if (mode === 'tab') { - return expect(view.$('.js-selected-topic').text()).toEqual('General'); + return expect(view.$('.post-topic option:selected').text()).toEqual('General'); } }; return _.each(['tab', 'inline'], function(mode) { diff --git a/common/static/common/templates/discussion/new-post-menu-category.underscore b/common/static/common/templates/discussion/new-post-menu-category.underscore index f10d07638f..432bc2b952 100644 --- a/common/static/common/templates/discussion/new-post-menu-category.underscore +++ b/common/static/common/templates/discussion/new-post-menu-category.underscore @@ -1,4 +1,3 @@ - + + <%= edx.HtmlUtils.ensureHtml(entries) %> + diff --git a/common/static/common/templates/discussion/new-post-menu-entry.underscore b/common/static/common/templates/discussion/new-post-menu-entry.underscore index 7e55735ed1..d3f300f94c 100644 --- a/common/static/common/templates/discussion/new-post-menu-entry.underscore +++ b/common/static/common/templates/discussion/new-post-menu-entry.underscore @@ -1,3 +1 @@ - + diff --git a/common/static/common/templates/discussion/topic.underscore b/common/static/common/templates/discussion/topic.underscore index c8a99f30c3..c8dfa897b2 100644 --- a/common/static/common/templates/discussion/topic.underscore +++ b/common/static/common/templates/discussion/topic.underscore @@ -1,19 +1,9 @@ -<% // Using div here instead of label because we are using a non-native control %> -
- <%- gettext("Topic Area:") %>
- -
- - -
-
-
+ <%- gettext("Add your post to a relevant topic to help others find it.") %> diff --git a/lms/static/sass/discussion/_mixins.scss b/lms/static/sass/discussion/_mixins.scss index b3abaade1f..55c97ac7ae 100644 --- a/lms/static/sass/discussion/_mixins.scss +++ b/lms/static/sass/discussion/_mixins.scss @@ -35,7 +35,6 @@ width: 100%; height: 125px; background: $forum-color-background; - box-shadow: 0 1px 3px rgba(0, 0, 0, 0.15) inset; font-size: $forum-base-font-size; font-family: $sans-serif; line-height: 1.6; diff --git a/lms/static/sass/discussion/utilities/_shame.scss b/lms/static/sass/discussion/utilities/_shame.scss index 6c9aa65d24..5e2dfdc802 100644 --- a/lms/static/sass/discussion/utilities/_shame.scss +++ b/lms/static/sass/discussion/utilities/_shame.scss @@ -96,28 +96,9 @@ li[class*=forum-nav-thread-label-] { text-shadow: none; } - .post-type, .topic-filter-label { + .post-type { margin-bottom: 0; } - - // Override global ul rules - .topic-menu { - @include padding-left(0); - } - - .topic-menu, .topic-submenu { - margin-top: 0; - margin-bottom: 0; - } - - // Override global span rules - .post-topic-button .drop-arrow { - line-height: 36px; - } - - .topic-title { - line-height: 14px; - } } // ------- diff --git a/lms/static/sass/discussion/views/_create-edit-post.scss b/lms/static/sass/discussion/views/_create-edit-post.scss index 1dbd1e7b8e..4022c9231d 100644 --- a/lms/static/sass/discussion/views/_create-edit-post.scss +++ b/lms/static/sass/discussion/views/_create-edit-post.scss @@ -29,8 +29,8 @@ display: inline-block; width: 100%; vertical-align: top; - border-width: 0; &:not([type="text"]) { + border-width: 0; padding: 0; } } @@ -77,22 +77,6 @@ // UI: inputs .forum-new-post-form, .edit-post-form { - .post-topic-button { - @include white-button; - @extend %cont-truncated; - z-index: 1000; - padding: 0 $baseline 0 ($baseline*0.75); - width: 100%; - height: 40px; - font-size: $forum-base-font-size; - line-height: 36px; - - .drop-arrow { - @include float(right); - color: #999; - } - } - .post-type-input { @extend %text-sr; } @@ -108,6 +92,7 @@ height: 40px; text-align: center; color: $gray-d3; + border: 1px solid $forum-color-border; font-weight: 600; line-height: 36px; @include float(left); @@ -122,6 +107,7 @@ } .post-type-input:checked + .post-type-label { + border: none; background-color: $forum-color-active-thread; color: $forum-color-active-text; background-image: none; @@ -138,13 +124,17 @@ border-radius: $forum-border-radius; padding: 0 $baseline/2; height: 40px; - box-shadow: 0 1px 3px rgba(0, 0, 0, 0.15) inset; color: #333; font-weight: 700; font-size: $forum-large-font-size; font-family: 'Open Sans', sans-serif; } + select.field-input { + border: 1px solid $forum-color-border !important; // !important required to override -webkit-appearance + height: 40px; + } + .post-option { box-sizing: border-box; display: inline-block; @@ -193,66 +183,3 @@ list-style: none; } } - -// ==================== - -// UI: topic menu - -// TO-DO: refactor to use _navigation.scss as general topic selector -.forum-new-post-form .post-topic , -.edit-post-form .post-topic { - position: relative; - - .topic-menu-wrapper { - @extend %ui-depth4; - @include left(0); - box-sizing: border-box; - position: absolute; - top: 40px; - width: 100%; - background: $forum-color-background; - box-shadow: 0 2px 1px $shadow; - } - - .topic-filter-label { - border-bottom: 1px solid $forum-color-border; - } - - .topic-filter-input { - box-sizing: border-box; - border: none; - border-bottom: 1px solid $forum-color-border; - padding: 0 15px; - width: 100%; - height: 30px; - color: #333; - font-size: $forum-small-font-size; - line-height: 16px; - } - - .topic-menu { - @include margin-left($baseline/2); - overflow-y: scroll; - max-height: 400px; - list-style: none; - } - - .topic-submenu { - @include margin-left($baseline); - list-style: none; - } - - .topic-title { - display: block; - padding: ($baseline/4) ($baseline/2); - font-size: $forum-base-font-size; - } - - button.topic-title { - @include transition(none); - - &:hover, &:focus { - background-color: $gray-l4; - } - } -}