diff --git a/common/lib/xmodule/xmodule/js/src/discussion/display.coffee b/common/lib/xmodule/xmodule/js/src/discussion/display.coffee index a350cf9866..2369b0f92d 100644 --- a/common/lib/xmodule/xmodule/js/src/discussion/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/discussion/display.coffee @@ -1,4 +1,4 @@ class @InlineDiscussion extends XModule.Descriptor constructor: (element) -> @el = $(element).find('.discussion-module') - @view = new DiscussionModuleView(el: @el) + @view = new DiscussionInlineView(el: @el) diff --git a/common/static/common/js/discussion/discussion_module_view.js b/common/static/common/js/discussion/discussion_module_view.js deleted file mode 100644 index 36ed2a4859..0000000000 --- a/common/static/common/js/discussion/discussion_module_view.js +++ /dev/null @@ -1,265 +0,0 @@ -/* globals Discussion, DiscussionUtil, DiscussionUser, DiscussionCourseSettings, DiscussionThreadView, Content, -NewPostView */ -(function() { - 'use strict'; - var __hasProp = {}.hasOwnProperty, - __extends = function(child, parent) { - for (var key in parent) { - if (__hasProp.call(parent, key)) { - child[key] = parent[key]; - } - } - function ctor() { - this.constructor = child; - } - - ctor.prototype = parent.prototype; - child.prototype = new ctor(); - child.__super__ = parent.prototype; - return child; - }; - - if (typeof Backbone !== 'undefined' && Backbone !== null) { - this.DiscussionModuleView = (function(_super) { - __extends(DiscussionModuleView, _super); - - function DiscussionModuleView() { - var self = this; - this.navigateToPage = function() { - return DiscussionModuleView.prototype.navigateToPage.apply(self, arguments); - }; - this.renderPagination = function() { - return DiscussionModuleView.prototype.renderPagination.apply(self, arguments); - }; - this.addThread = function() { - return DiscussionModuleView.prototype.addThread.apply(self, arguments); - }; - this.renderDiscussion = function() { - return DiscussionModuleView.prototype.renderDiscussion.apply(self, arguments); - }; - this.loadPage = function() { - return DiscussionModuleView.prototype.loadPage.apply(self, arguments); - }; - this.toggleDiscussion = function() { - return DiscussionModuleView.prototype.toggleDiscussion.apply(self, arguments); - }; - this.hideDiscussion = function() { - return DiscussionModuleView.prototype.hideDiscussion.apply(self, arguments); - }; - this.hideNewPost = function() { - return DiscussionModuleView.prototype.hideNewPost.apply(self, arguments); - }; - this.toggleNewPost = function() { - return DiscussionModuleView.prototype.toggleNewPost.apply(self, arguments); - }; - return DiscussionModuleView.__super__.constructor.apply(this, arguments); - } - - DiscussionModuleView.prototype.events = { - 'click .discussion-show': 'toggleDiscussion', - 'keydown .discussion-show': function(event) { - return DiscussionUtil.activateOnSpace(event, this.toggleDiscussion); - }, - 'click .new-post-btn': 'toggleNewPost', - 'keydown .new-post-btn': function(event) { - return DiscussionUtil.activateOnSpace(event, this.toggleNewPost); - }, - 'click .discussion-paginator a': 'navigateToPage' - }; - - DiscussionModuleView.prototype.page_re = /\?discussion_page=(\d+)/; - - DiscussionModuleView.prototype.initialize = function(options) { - var match; - this.toggleDiscussionBtn = this.$('.discussion-show'); - match = this.page_re.exec(window.location.href); - this.context = options.context || 'course'; - this.readOnly = $('.discussion-module').data('read-only'); - if (match) { - this.page = parseInt(match[1]); - } else { - this.page = 1; - } - }; - - DiscussionModuleView.prototype.toggleNewPost = function(event) { - event.preventDefault(); - if (!this.newPostForm) { - this.toggleDiscussion(); - this.isWaitingOnNewPost = true; - return; - } - if (this.showed) { - this.newPostForm.slideDown(300); - } else { - this.newPostForm.show().focus(); - } - this.toggleDiscussionBtn.addClass('shown'); - this.toggleDiscussionBtn.find('.button-text').html(gettext('Hide Discussion')); - this.$('section.discussion').slideDown(); - this.showed = true; - }; - - DiscussionModuleView.prototype.hideNewPost = function() { - return this.newPostForm.slideUp(300); - }; - - DiscussionModuleView.prototype.hideDiscussion = function() { - this.$('section.discussion').slideUp(); - this.toggleDiscussionBtn.removeClass('shown'); - this.toggleDiscussionBtn.find('.button-text').html(gettext('Show Discussion')); - this.showed = false; - }; - - DiscussionModuleView.prototype.toggleDiscussion = function() { - var $elem, - self = this; - if (this.showed) { - return this.hideDiscussion(); - } else { - this.toggleDiscussionBtn.addClass('shown'); - this.toggleDiscussionBtn.find('.button-text').html(gettext('Hide Discussion')); - if (this.retrieved) { - this.$('section.discussion').slideDown(); - this.showed = true; - } else { - $elem = this.toggleDiscussionBtn; - return this.loadPage($elem, function() { - self.hideDiscussion(); - return DiscussionUtil.discussionAlert( - gettext('Sorry'), - gettext('We had some trouble loading the discussion. Please try again.') - ); - }); - } - } - }; - - DiscussionModuleView.prototype.loadPage = function($elem, error) { - var discussionId, url, - self = this; - discussionId = this.$el.data('discussion-id'); - url = DiscussionUtil.urlFor('retrieve_discussion', discussionId) + ('?page=' + this.page); - return DiscussionUtil.safeAjax({ - $elem: $elem, - $loading: $elem, - takeFocus: true, - url: url, - type: 'GET', - dataType: 'json', - success: function(response, textStatus) { - return self.renderDiscussion($elem, response, textStatus, discussionId); - }, - error: error - }); - }; - - DiscussionModuleView.prototype.renderDiscussion = function($elem, response, textStatus, discussionId) { - var $discussion, user, - self = this; - $elem.focus(); - user = new DiscussionUser(response.user_info); - window.user = user; - DiscussionUtil.setUser(user); - Content.loadContentInfos(response.annotated_content_info); - DiscussionUtil.loadRoles(response.roles); - this.course_settings = new DiscussionCourseSettings(response.course_settings); - this.discussion = new Discussion(); - this.discussion.reset(response.discussion_data, { - silent: false - }); - $discussion = _.template($('#inline-discussion-template').html())({ - 'threads': response.discussion_data, - read_only: this.readOnly, - 'discussionId': discussionId - }); - if (this.$('section.discussion').length) { - this.$('section.discussion').replaceWith($discussion); - } else { - this.$el.append($discussion); - } - this.newPostForm = this.$el.find('.new-post-article'); - this.threadviews = this.discussion.map(function(thread) { - var view; - view = new DiscussionThreadView({ - el: self.$('article#thread_' + thread.id), - model: thread, - mode: 'inline', - context: self.context, - course_settings: self.course_settings, - topicId: discussionId - }); - thread.on('thread:thread_type_updated', function() { - view.rerender(); - return view.expand(); - }); - return view; - }); - _.each(this.threadviews, function(dtv) { - return dtv.render(); - }); - DiscussionUtil.bulkUpdateContentInfo(window.$$annotated_content_info); - this.newPostView = new NewPostView({ - el: this.newPostForm, - collection: this.discussion, - course_settings: this.course_settings, - topicId: discussionId, - is_commentable_cohorted: response.is_commentable_cohorted - }); - this.newPostView.render(); - this.listenTo(this.newPostView, 'newPost:cancel', this.hideNewPost); - this.discussion.on('add', this.addThread); - this.retrieved = true; - this.showed = true; - this.renderPagination(response.num_pages); - if (this.isWaitingOnNewPost) { - return this.newPostForm.show().focus(); - } - }; - - DiscussionModuleView.prototype.addThread = function(thread) { - var article, threadView; - article = $("
"); - this.$('section.discussion > .threads').prepend(article); - threadView = new DiscussionThreadView({ - el: article, - model: thread, - mode: 'inline', - context: this.context, - course_settings: this.course_settings, - topicId: this.$el.data('discussion-id') - }); - threadView.render(); - return this.threadviews.unshift(threadView); - }; - - DiscussionModuleView.prototype.renderPagination = function(numPages) { - var pageUrl, pagination, params; - pageUrl = function(number) { - return '?discussion_page=' + number; - }; - params = DiscussionUtil.getPaginationParams(this.page, numPages, pageUrl); - pagination = _.template($('#pagination-template').html())(params); - return this.$('section.discussion-pagination').html(pagination); - }; - - DiscussionModuleView.prototype.navigateToPage = function(event) { - var currPage, - self = this; - event.preventDefault(); - window.history.pushState({}, window.document.title, event.target.href); - currPage = this.page; - this.page = $(event.target).data('page-number'); - return this.loadPage($(event.target), function() { - self.page = currPage; - DiscussionUtil.discussionAlert( - gettext('Sorry'), - gettext('We had some trouble loading the threads you requested. Please try again.') - ); - }); - }; - - return DiscussionModuleView; - })(Backbone.View); - } -}).call(window); diff --git a/common/static/common/js/discussion/utils.js b/common/static/common/js/discussion/utils.js index 44755e98d7..83ff8bcd67 100644 --- a/common/static/common/js/discussion/utils.js +++ b/common/static/common/js/discussion/utils.js @@ -184,10 +184,8 @@ if (!params.error) { params.error = function() { self.discussionAlert( - gettext('Sorry'), - gettext( - 'We had some trouble processing your request. Please ensure you have copied any ' + - 'unsaved work and then reload the page.') + gettext('Error'), + gettext('Your request could not be processed. Refresh the page and try again.') ); }; } @@ -223,7 +221,7 @@ self = this; if (errorMsg) { safeAjaxParams.error = function() { - return self.discussionAlert(gettext('Sorry'), errorMsg); + return self.discussionAlert(gettext('Error'), errorMsg); }; } undo = _.pick(model.attributes, _.keys(updates)); @@ -276,7 +274,7 @@ } } } else { - $errorItem = makeErrorElem('We had some trouble processing your request. Please try again.', 0); + $errorItem = makeErrorElem('Your request could not be processed. Refresh the page and try again.', 0); // eslint-disable-line max-len edx.HtmlUtils.append(errorsField, $errorItem); } diff --git a/common/static/common/js/discussion/views/discussion_content_view.js b/common/static/common/js/discussion/views/discussion_content_view.js index 19e965e143..3b00c3e257 100644 --- a/common/static/common/js/discussion/views/discussion_content_view.js +++ b/common/static/common/js/discussion/views/discussion_content_view.js @@ -1,4 +1,4 @@ -/* globals DiscussionContentView, DiscussionUtil */ +/* globals _, Backbone, DiscussionContentView, DiscussionUtil */ (function() { 'use strict'; var __hasProp = {}.hasOwnProperty, @@ -148,22 +148,24 @@ return _results; }; - DiscussionContentView.prototype.makeWmdEditor = function(cls_identifier) { + DiscussionContentView.prototype.makeWmdEditor = function(classIdentifier) { if (!this.$el.find('.wmd-panel').length) { - return DiscussionUtil.makeWmdEditor(this.$el, $.proxy(this.$, this), cls_identifier); + return DiscussionUtil.makeWmdEditor(this.$el, $.proxy(this.$, this), classIdentifier); + } else { + return null; } }; - DiscussionContentView.prototype.getWmdEditor = function(cls_identifier) { - return DiscussionUtil.getWmdEditor(this.$el, $.proxy(this.$, this), cls_identifier); + DiscussionContentView.prototype.getWmdEditor = function(classIdentifier) { + return DiscussionUtil.getWmdEditor(this.$el, $.proxy(this.$, this), classIdentifier); }; - DiscussionContentView.prototype.getWmdContent = function(cls_identifier) { - return DiscussionUtil.getWmdContent(this.$el, $.proxy(this.$, this), cls_identifier); + DiscussionContentView.prototype.getWmdContent = function(classIdentifier) { + return DiscussionUtil.getWmdContent(this.$el, $.proxy(this.$, this), classIdentifier); }; - DiscussionContentView.prototype.setWmdContent = function(cls_identifier, text) { - return DiscussionUtil.setWmdContent(this.$el, $.proxy(this.$, this), cls_identifier, text); + DiscussionContentView.prototype.setWmdContent = function(classIdentifier, text) { + return DiscussionUtil.setWmdContent(this.$el, $.proxy(this.$, this), classIdentifier, text); }; DiscussionContentView.prototype.initialize = function() { @@ -171,7 +173,7 @@ this.model.bind('change', this.renderPartialAttrs, this); return this.listenTo(this.model, 'change:endorsed', function() { if (self.model instanceof Comment) { - return self.trigger('comment:endorse'); + self.trigger('comment:endorse'); } }); }; @@ -330,7 +332,7 @@ DiscussionContentShowView.prototype.handleSecondaryActionEscape = function(event) { if (event.keyCode === 27) { this.toggleSecondaryActions(event); - return this.$('.action-more').focus(); + this.$('.action-more').focus(); } }; @@ -338,23 +340,23 @@ var self = this; return setTimeout(function() { if (self.secondaryActionsExpanded && self.$('.actions-dropdown :focus').length === 0) { - return self.toggleSecondaryActions(event); + self.toggleSecondaryActions(event); } }, 10); }; DiscussionContentShowView.prototype.toggleFollow = function(event) { - var is_subscribing, msg, url; + var isSubscribing, msg, url; event.preventDefault(); - is_subscribing = !this.model.get('subscribed'); - url = this.model.urlFor(is_subscribing ? 'follow' : 'unfollow'); - if (is_subscribing) { - msg = gettext('We had some trouble subscribing you to this thread. Please try again.'); + isSubscribing = !this.model.get('subscribed'); + url = this.model.urlFor(isSubscribing ? 'follow' : 'unfollow'); + if (isSubscribing) { + msg = gettext('You could not be subscribed to this post. Refresh the page and try again.'); } else { - msg = gettext('We had some trouble unsubscribing you from this thread. Please try again.'); + msg = gettext('You could not be unsubscribed from this post. Refresh the page and try again.'); } return DiscussionUtil.updateWithUndo(this.model, { - 'subscribed': is_subscribing + subscribed: isSubscribing }, { url: url, type: 'POST', @@ -363,30 +365,30 @@ }; DiscussionContentShowView.prototype.toggleEndorse = function(event) { - var beforeFunc, is_endorsing, msg, updates, url, + var isEndorsing, msg, updates, url, self = this; event.preventDefault(); - is_endorsing = !this.model.get('endorsed'); + isEndorsing = !this.model.get('endorsed'); url = this.model.urlFor('endorse'); updates = { - endorsed: is_endorsing, - endorsement: is_endorsing ? { + endorsed: isEndorsing, + endorsement: isEndorsing ? { username: DiscussionUtil.getUser().get('username'), user_id: DiscussionUtil.getUser().id, time: new Date().toISOString() } : null }; if (this.model.get('thread').get('thread_type') === 'question') { - if (is_endorsing) { - msg = gettext('We had some trouble marking this response as an answer. Please try again.'); + if (isEndorsing) { + msg = gettext('This response could not be marked as an answer. Refresh the page and try again.'); // eslint-disable-line max-len } else { - msg = gettext('We had some trouble removing this response as an answer. Please try again.'); + msg = gettext('This response could not be unmarked as an answer. Refresh the page and try again.'); // eslint-disable-line max-len } } else { - if (is_endorsing) { - msg = gettext('We had some trouble marking this response endorsed. Please try again.'); + if (isEndorsing) { + msg = gettext('This response could not be marked as endorsed. Refresh the page and try again.'); } else { - msg = gettext('We had some trouble removing this endorsement. Please try again.'); + msg = gettext('This response could not be unendorsed. Refresh the page and try again.'); } } return DiscussionUtil.updateWithUndo( @@ -395,7 +397,7 @@ { url: url, type: 'POST', - data: {endorsed: is_endorsing}, + data: {endorsed: isEndorsing}, $elem: $(event.currentTarget) }, msg, @@ -404,22 +406,22 @@ }; DiscussionContentShowView.prototype.toggleVote = function(event) { - var is_voting, updates, url, user, + var isVoting, updates, url, user, self = this; event.preventDefault(); user = DiscussionUtil.getUser(); - is_voting = !user.voted(this.model); - url = this.model.urlFor(is_voting ? 'upvote' : 'unvote'); + isVoting = !user.voted(this.model); + url = this.model.urlFor(isVoting ? 'upvote' : 'unvote'); updates = { - upvoted_ids: (is_voting ? _.union : _.difference)(user.get('upvoted_ids'), [this.model.id]) + upvoted_ids: (isVoting ? _.union : _.difference)(user.get('upvoted_ids'), [this.model.id]) }; if (!$(event.target.closest('.actions-item')).hasClass('is-disabled')) { return DiscussionUtil.updateWithUndo(user, updates, { url: url, type: 'POST', $elem: $(event.currentTarget) - }, gettext('We had some trouble saving your vote. Please try again.')).done(function() { - if (is_voting) { + }, gettext('This vote could not be processed. Refresh the page and try again.')).done(function() { + if (isVoting) { return self.model.vote(); } else { return self.model.unvote(); @@ -429,17 +431,17 @@ }; DiscussionContentShowView.prototype.togglePin = function(event) { - var is_pinning, msg, url; + var isPinning, msg, url; event.preventDefault(); - is_pinning = !this.model.get('pinned'); - url = this.model.urlFor(is_pinning ? 'pinThread' : 'unPinThread'); - if (is_pinning) { - msg = gettext('We had some trouble pinning this thread. Please try again.'); + isPinning = !this.model.get('pinned'); + url = this.model.urlFor(isPinning ? 'pinThread' : 'unPinThread'); + if (isPinning) { + msg = gettext('This post could not be pinned. Refresh the page and try again.'); } else { - msg = gettext('We had some trouble unpinning this thread. Please try again.'); + msg = gettext('This post could not be unpinned. Refresh the page and try again.'); } return DiscussionUtil.updateWithUndo(this.model, { - pinned: is_pinning + pinned: isPinning }, { url: url, type: 'POST', @@ -448,18 +450,18 @@ }; DiscussionContentShowView.prototype.toggleReport = function(event) { - var is_flagging, msg, updates, url; + var isFlagging, msg, updates, url; event.preventDefault(); if (this.model.isFlagged()) { - is_flagging = false; - msg = gettext('We had some trouble removing your flag on this post. Please try again.'); + isFlagging = false; + msg = gettext('This post could not be flagged for abuse. Refresh the page and try again.'); } else { - is_flagging = true; - msg = gettext('We had some trouble reporting this post. Please try again.'); + isFlagging = true; + msg = gettext('This post could not be unflagged for abuse. Refresh the page and try again.'); } - url = this.model.urlFor(is_flagging ? 'flagAbuse' : 'unFlagAbuse'); + url = this.model.urlFor(isFlagging ? 'flagAbuse' : 'unFlagAbuse'); updates = { - abuse_flaggers: (is_flagging ? _.union : _.difference)( + abuse_flaggers: (isFlagging ? _.union : _.difference)( this.model.get('abuse_flaggers'), [DiscussionUtil.getUser().id] ) }; @@ -471,16 +473,16 @@ }; DiscussionContentShowView.prototype.toggleClose = function(event) { - var is_closing, msg, updates; + var isClosing, msg, updates; event.preventDefault(); - is_closing = !this.model.get('closed'); - if (is_closing) { - msg = gettext('We had some trouble closing this thread. Please try again.'); + isClosing = !this.model.get('closed'); + if (isClosing) { + msg = gettext('This post could not be closed. Refresh the page and try again.'); } else { - msg = gettext('We had some trouble reopening this thread. Please try again.'); + msg = gettext('This post could not be reopened. Refresh the page and try again.'); } updates = { - closed: is_closing + closed: isClosing }; return DiscussionUtil.updateWithUndo(this.model, updates, { url: this.model.urlFor('close'), diff --git a/common/static/common/js/discussion/views/discussion_inline_view.js b/common/static/common/js/discussion/views/discussion_inline_view.js new file mode 100644 index 0000000000..7a44a6571a --- /dev/null +++ b/common/static/common/js/discussion/views/discussion_inline_view.js @@ -0,0 +1,238 @@ +/* globals + _, Backbone, Content, Discussion, DiscussionUtil, DiscussionUser, DiscussionCourseSettings, + DiscussionThreadListView, DiscussionThreadView, NewPostView + */ + +(function() { + 'use strict'; + + this.DiscussionInlineView = Backbone.View.extend({ + events: { + 'click .discussion-show': 'toggleDiscussion', + 'keydown .discussion-show': function(event) { + return DiscussionUtil.activateOnSpace(event, this.toggleDiscussion); + }, + 'click .new-post-btn': 'toggleNewPost', + 'click .all-posts-btn': 'navigateToAllPosts', + keydown: 'handleKeydown', + 'keydown .new-post-btn': function(event) { + return DiscussionUtil.activateOnSpace(event, this.toggleNewPost); + } + }, + + page_re: /\?discussion_page=(\d+)/, + + initialize: function(options) { + var match; + + this.$el = options.el; + this.readOnly = options.readOnly; + this.showByDefault = options.showByDefault || false; + this.toggleDiscussionBtn = this.$('.discussion-show'); + this.listenTo(this.model, 'change', this.render); + this.escKey = 27; + + match = this.page_re.exec(window.location.href); + if (match) { + this.page = parseInt(match[1], 10); + } else { + this.page = 1; + } + + // By default the view is displayed in a hidden state. If you want it to be shown by default (e.g. in Teams) + // pass showByDefault as an option. This code will open it on initialization. + if (this.showByDefault) { + this.toggleDiscussion(); + } + }, + + loadDiscussions: function($elem, error) { + var discussionId = this.$el.data('discussion-id'), + url = DiscussionUtil.urlFor('retrieve_discussion', discussionId) + ('?page=' + this.page), + self = this; + + DiscussionUtil.safeAjax({ + $elem: this.$el, + $loading: this.$el, + takeFocus: true, + url: url, + type: 'GET', + dataType: 'json', + success: function(response, textStatus) { + self.renderDiscussion(self.$el, response, textStatus, discussionId); + }, + error: error + }); + }, + + renderDiscussion: function($elem, response, textStatus, discussionId) { + var discussionHtml, + user = new DiscussionUser(response.user_info), + self = this; + + $elem.focus(); + + window.user = user; + DiscussionUtil.setUser(user); + Content.loadContentInfos(response.annotated_content_info); + DiscussionUtil.loadRoles(response.roles); + + this.course_settings = new DiscussionCourseSettings(response.course_settings); + + this.discussion = new Discussion(undefined, {pages: response.num_pages}); + this.discussion.reset(response.discussion_data, { + silent: false + }); + + discussionHtml = edx.HtmlUtils.template($('#inline-discussion-template').html())({ + threads: response.discussion_data, + read_only: this.readOnly, + discussionId: discussionId + }); + + if (this.$('section.discussion').length) { + edx.HtmlUtils.setHtml(this.$el, discussionHtml); + this.$('section.discussion').replaceWith(edx.HtmlUtils.ensureHtml(discussionHtml).toString()); + } else { + edx.HtmlUtils.append(this.$el, discussionHtml); + } + + this.threadListView = new DiscussionThreadListView({ + el: this.$('.inline-threads'), + collection: self.discussion, + courseSettings: self.course_settings, + hideRefineBar: true // TODO: re-enable the search/filter bar when it works correctly + }); + + this.threadListView.render(); + + this.threadListView.on('thread:selected', _.bind(this.navigateToThread, this)); + + DiscussionUtil.bulkUpdateContentInfo(window.$$annotated_content_info); + + this.newPostForm = this.$el.find('.new-post-article'); + + this.newPostView = new NewPostView({ + el: this.newPostForm, + collection: this.discussion, + course_settings: this.course_settings, + topicId: discussionId, + is_commentable_cohorted: response.is_commentable_cohorted + }); + + this.newPostView.render(); + + this.listenTo(this.newPostView, 'newPost:createPost', this.onNewPostCreated); + this.listenTo(this.newPostView, 'newPost:cancel', this.hideNewPost); + this.discussion.on('add', this.addThread); + + this.retrieved = true; + this.showed = true; + + if (this.isWaitingOnNewPost) { + this.newPostForm.removeClass('is-hidden').focus(); + } + + // Hide the thread view initially + this.$('.inline-thread').addClass('is-hidden'); + }, + + navigateToThread: function(threadId) { + var thread = this.discussion.get(threadId); + this.threadView = new DiscussionThreadView({ + el: this.$('.forum-content'), + model: thread, + mode: 'inline', + course_settings: this.course_settings + }); + this.threadView.render(); + this.listenTo(this.threadView.showView, 'thread:_delete', this.navigateToAllPosts); + this.threadListView.$el.addClass('is-hidden'); + this.$('.inline-thread').removeClass('is-hidden'); + }, + + navigateToAllPosts: function() { + // Hide the inline thread section + this.$('.inline-thread').addClass('is-hidden'); + + // Delete the thread view + this.threadView.$el.empty().off(); + this.threadView.stopListening(); + this.threadView = null; + + // Show the thread list view + this.threadListView.$el.removeClass('is-hidden'); + + // Set focus to thread list item that was saved as active + this.threadListView.$('.is-active').focus(); + }, + + toggleDiscussion: function() { + var self = this; + + if (this.showed) { + this.hideDiscussion(); + } else { + this.toggleDiscussionBtn.addClass('shown'); + this.toggleDiscussionBtn.find('.button-text').text(gettext('Hide Discussion')); + if (this.retrieved) { + this.$('section.discussion').removeClass('is-hidden'); + this.showed = true; + } else { + this.loadDiscussions(this.$el, function() { + self.hideDiscussion(); + DiscussionUtil.discussionAlert( + gettext('Error'), + gettext('This discussion could not be loaded. Refresh the page and try again.') + ); + }); + } + } + }, + + hideDiscussion: function() { + this.$('section.discussion').addClass('is-hidden'); + this.toggleDiscussionBtn.removeClass('shown'); + this.toggleDiscussionBtn.find('.button-text').text(gettext('Show Discussion')); + this.showed = false; + }, + + toggleNewPost: function(event) { + event.preventDefault(); + if (!this.newPostForm) { + this.toggleDiscussion(); + this.isWaitingOnNewPost = true; + return; + } + if (this.showed) { + this.$('section.discussion').find('.inline-discussion-thread-container').addClass('is-hidden'); + this.$('section.discussion').find('.add_post_btn_container').addClass('is-hidden'); + this.newPostForm.removeClass('is-hidden'); + } + this.newPostView.$el.removeClass('is-hidden'); + this.toggleDiscussionBtn.addClass('shown'); + this.toggleDiscussionBtn.find('.button-text').text(gettext('Hide Discussion')); + this.showed = true; + }, + + onNewPostCreated: function() { + this.navigateToAllPosts(); + this.hideNewPost(); + }, + + hideNewPost: function() { + this.$('section.discussion').find('.inline-discussion-thread-container').removeClass('is-hidden'); + this.$('section.discussion').find('.add_post_btn_container') + .removeClass('is-hidden') + .focus(); + this.newPostForm.addClass('is-hidden'); + }, + + handleKeydown: function(event) { + var keyCode = event.keyCode; + if (keyCode === this.escKey) { + this.$('section.discussion').find('.cancel').trigger('click'); + } + } + }); +}).call(window); 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 c6a8c379d0..ef9cf793a2 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,6 +91,8 @@ DiscussionThreadListView.prototype.initialize = function(options) { var self = this; this.courseSettings = options.courseSettings; + this.hideRefineBar = options.hideRefineBar; + this.supportsActiveThread = options.supportsActiveThread; this.displayedCollection = new Discussion(this.collection.models, { pages: this.collection.pages }); @@ -107,7 +109,7 @@ this.boardName = null; this.current_search = ''; this.mode = 'all'; - this.showThreadPreview = options.showThreadPreview; + this.showThreadPreview = true; this.searchAlertCollection = new Backbone.Collection([], { model: Backbone.Model }); @@ -164,7 +166,7 @@ active = $currentElement.has('.forum-nav-thread-link.is-active').length !== 0; $currentElement.replaceWith($content); this.showMetadataAccordingToSort(); - if (active) { + if (this.supportsActiveThread && active) { this.setActiveThread(threadId); } }; @@ -220,6 +222,9 @@ } this.showMetadataAccordingToSort(); this.renderMorePages(); + if (this.hideRefineBar) { + this.$('.forum-nav-refine-bar').addClass('is-hidden'); + } this.trigger('threads:rendered'); }; @@ -309,7 +314,8 @@ error = function() { self.renderThreads(); DiscussionUtil.discussionAlert( - gettext('Sorry'), gettext('We had some trouble loading more threads. Please try again.') + gettext('Error'), + gettext('Additional posts could not be loaded. Refresh the page and try again.') ); }; return this.collection.retrieveAnotherPage(this.mode, options, { @@ -346,7 +352,9 @@ DiscussionThreadListView.prototype.threadSelected = function(e) { var threadId; threadId = $(e.target).closest('.forum-nav-thread').attr('data-id'); - this.setActiveThread(threadId); + if (this.supportsActiveThread) { + this.setActiveThread(threadId); + } this.trigger('thread:selected', threadId); return false; }; @@ -478,7 +486,7 @@ element, edx.HtmlUtils.joinHtml( edx.HtmlUtils.HTML("
  • "), - self.getLoadingContent(gettext('Loading thread list')), + self.getLoadingContent(gettext('Loading posts list')), edx.HtmlUtils.HTML('
  • ') ) ); @@ -515,7 +523,7 @@ ); self.addSearchAlert(message); } else if (response.discussion_data.length === 0) { - self.addSearchAlert(gettext('No threads matched your query.')); + self.addSearchAlert(gettext('No posts matched your query.')); } self.displayedCollection.reset(self.collection.models); if (text) { diff --git a/common/static/common/js/discussion/views/discussion_thread_view.js b/common/static/common/js/discussion/views/discussion_thread_view.js index cbd14298cd..5544e8fb52 100644 --- a/common/static/common/js/discussion/views/discussion_thread_view.js +++ b/common/static/common/js/discussion/views/discussion_thread_view.js @@ -152,14 +152,7 @@ }); }); } - if (this.mode === 'tab') { - setTimeout(function() { - return self.loadInitialResponses(); - }, 100); - return this.$('.post-tools').hide(); - } else { - return this.collapse(); - } + this.loadInitialResponses(); }; DiscussionThreadView.prototype.attrRenderer = $.extend({}, DiscussionContentView.prototype.attrRenderer, { @@ -221,9 +214,7 @@ }; DiscussionThreadView.prototype.loadResponses = function(responseLimit, $elem, firstLoad) { - var takeFocus, - self = this; - takeFocus = this.mode === 'tab' ? false : true; + var self = this; this.responsesRequest = DiscussionUtil.safeAjax({ url: DiscussionUtil.urlFor( 'retrieve_single_thread', this.model.get('commentable_id'), this.model.id @@ -234,7 +225,7 @@ }, $elem: $elem, $loading: $elem, - takeFocus: takeFocus, + takeFocus: false, complete: function() { self.responsesRequest = null; }, @@ -253,7 +244,6 @@ ); self.trigger('thread:responses:rendered'); self.loadedResponses = true; - return self.$el.find('.discussion-article[data-id="' + self.model.id + '"]').focus(); }, error: function(xhr, textStatus) { if (textStatus === 'abort') { @@ -261,18 +251,18 @@ } if (xhr.status === 404) { DiscussionUtil.discussionAlert( - gettext('Sorry'), - gettext('The thread you selected has been deleted. Please select another thread.') + gettext('Error'), + gettext('The post you selected has been deleted.') ); } else if (firstLoad) { DiscussionUtil.discussionAlert( - gettext('Sorry'), - gettext('We had some trouble loading responses. Please reload the page.') + gettext('Error'), + gettext('Responses could not be loaded. Refresh the page and try again.') ); } else { DiscussionUtil.discussionAlert( - gettext('Sorry'), - gettext('We had some trouble loading more responses. Please try again.') + gettext('Error'), + gettext('Additional responses could not be loaded. Refresh the page and try again.') ); } } 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 a6e4d74f72..0ade9f0e5c 100644 --- a/common/static/common/js/discussion/views/new_post_view.js +++ b/common/static/common/js/discussion/views/new_post_view.js @@ -1,4 +1,4 @@ -/* globals DiscussionTopicMenuView, DiscussionUtil, Thread */ +/* globals _, Backbone, DiscussionTopicMenuView, DiscussionUtil, Thread */ (function() { 'use strict'; var __hasProp = {}.hasOwnProperty, @@ -81,14 +81,14 @@ }; NewPostView.prototype.getCohortOptions = function() { - var user_cohort_id; + var userCohortId; if (this.course_settings.get('is_cohorted') && DiscussionUtil.isPrivilegedUser()) { - user_cohort_id = $('#discussion-container').data('user-cohort-id'); + userCohortId = $('#discussion-container').data('user-cohort-id'); return _.map(this.course_settings.get('cohorts'), function(cohort) { return { value: cohort.id, text: cohort.name, - selected: cohort.id === user_cohort_id + selected: cohort.id === userCohortId }; }); } else { @@ -100,6 +100,7 @@ 'submit .forum-new-post-form': 'createPost', 'change .post-option-input': 'postOptionChange', 'click .cancel': 'cancel', + 'click .add-post-cancel': 'cancel', 'reset .forum-new-post-form': 'updateStyles' }; @@ -125,15 +126,15 @@ }; NewPostView.prototype.createPost = function(event) { - var anonymous, anonymous_to_peers, body, follow, group, thread_type, title, topicId, url, + var anonymous, anonymousToPeers, body, follow, group, threadType, title, topicId, url, self = this; event.preventDefault(); - thread_type = this.$('.post-type-input:checked').val(); + threadType = this.$('.post-type-input:checked').val(); title = this.$('.js-post-title').val(); body = this.$('.js-post-body').find('.wmd-input').val(); group = this.$('.js-group-select option:selected').attr('value'); anonymous = false || this.$('.js-anon').is(':checked'); - anonymous_to_peers = false || this.$('.js-anon-peers').is(':checked'); + anonymousToPeers = false || this.$('.js-anon-peers').is(':checked'); follow = false || this.$('.js-follow').is(':checked'); topicId = this.isTabMode() ? this.topicView.getCurrentTopicId() : this.topicId; url = DiscussionUtil.urlFor('create_thread', topicId); @@ -144,11 +145,11 @@ type: 'POST', dataType: 'json', data: { - thread_type: thread_type, + thread_type: threadType, title: title, body: body, anonymous: anonymous, - anonymous_to_peers: anonymous_to_peers, + anonymous_to_peers: anonymousToPeers, auto_subscribe: follow, group_id: group }, @@ -156,20 +157,29 @@ success: function(response) { var thread; thread = new Thread(response.content); - self.$el.hide(); + self.$el.addClass('is-hidden'); self.resetForm(); + self.trigger('newPost:createPost'); return self.collection.add(thread); } }); }; + NewPostView.prototype.formModified = function() { + var postBodyHasContent = this.$('.js-post-body').find('.wmd-input').val() !== '', + titleHasContent = this.$('.js-post-title').val() !== ''; + return postBodyHasContent || titleHasContent; + }; + NewPostView.prototype.cancel = function(event) { event.preventDefault(); - if (!confirm(gettext('Your post will be discarded.'))) { - return; + if (this.formModified()) { + if (!confirm(gettext('Your post will be discarded.'))) { // eslint-disable-line no-alert + return; + } } this.trigger('newPost:cancel'); - return this.resetForm(); + this.resetForm(); }; NewPostView.prototype.resetForm = function() { @@ -177,7 +187,7 @@ DiscussionUtil.clearFormErrors(this.$('.post-errors')); this.$('.wmd-preview p').html(''); if (this.isTabMode()) { - return this.topicView.setTopic(this.$('button.topic-title').first()); + this.topicView.setTopic(this.$('button.topic-title').first()); } }; diff --git a/common/static/common/js/discussion/views/response_comment_view.js b/common/static/common/js/discussion/views/response_comment_view.js index 7e7afd63e4..ad870fd877 100644 --- a/common/static/common/js/discussion/views/response_comment_view.js +++ b/common/static/common/js/discussion/views/response_comment_view.js @@ -114,8 +114,8 @@ }, error: function() { return DiscussionUtil.discussionAlert( - gettext('Sorry'), - gettext('We had some trouble deleting this comment. Please try again.') + gettext('Error'), + gettext('This comment could not be deleted. Refresh the page and try again.') ); } }); diff --git a/common/static/common/js/spec/discussion/utils_spec.js b/common/static/common/js/spec/discussion/utils_spec.js index a8f91d2612..2652b5d793 100644 --- a/common/static/common/js/spec/discussion/utils_spec.js +++ b/common/static/common/js/spec/discussion/utils_spec.js @@ -29,7 +29,7 @@ }); spyOn(DiscussionUtil, 'discussionAlert'); DiscussionUtil.safeAjax.calls.mostRecent().args[0].error(); - expect(DiscussionUtil.discussionAlert).toHaveBeenCalledWith('Sorry', 'error message'); + expect(DiscussionUtil.discussionAlert).toHaveBeenCalledWith('Error', 'error message'); deferred.reject(); return expect(model.attributes).toEqual({ hello: false, diff --git a/common/static/common/js/spec/discussion/view/discussion_inline_view_spec.js b/common/static/common/js/spec/discussion/view/discussion_inline_view_spec.js new file mode 100644 index 0000000000..77aba3f7e8 --- /dev/null +++ b/common/static/common/js/spec/discussion/view/discussion_inline_view_spec.js @@ -0,0 +1,190 @@ +/* globals + _, Discussion, DiscussionCourseSettings, DiscussionViewSpecHelper, DiscussionSpecHelper, + DiscussionInlineView, DiscussionUtil, DiscussionThreadShowView, Thread + */ +(function() { + 'use strict'; + describe('DiscussionInlineView', function() { + var createTestView, showDiscussion, setNextAjaxResult, + TEST_THREAD_TITLE = 'Test thread title'; + + beforeEach(function() { + DiscussionSpecHelper.setUpGlobals(); + setFixtures( + '
    ' + + '
    ' + + '

    Test Discussion

    ' + + '
    ' + + ' Topic: Category / Target ' + + '
    ' + + '
    ' + + ' ' + + '
    ' + ); + DiscussionSpecHelper.setUnderscoreFixtures(); + this.ajaxSpy = spyOn($, 'ajax'); + + // Don't attempt to render markdown + spyOn(DiscussionUtil, 'makeWmdEditor'); + spyOn(DiscussionThreadShowView.prototype, 'convertMath'); + }); + + createTestView = function() { + var testView = new DiscussionInlineView({ + el: $('.discussion-module') + }); + testView.render(); + return testView; + }; + + showDiscussion = function(test, testView) { + setNextAjaxResult(test, { + user_info: DiscussionSpecHelper.getTestUserInfo(), + roles: DiscussionSpecHelper.getTestRoleInfo(), + course_settings: DiscussionSpecHelper.createTestCourseSettings().attributes, + discussion_data: DiscussionViewSpecHelper.makeThreadWithProps({ + commentable_id: 'test-topic', + title: TEST_THREAD_TITLE + }), + page: 1, + num_pages: 1, + content: { + endorsed_responses: [], + non_endorsed_responses: [], + children: [] + } + }); + testView.$('.discussion-show').click(); + }; + + setNextAjaxResult = function(test, result) { + test.ajaxSpy.and.callFake(function(params) { + var deferred = $.Deferred(); + deferred.resolve(); + params.success(result); + return deferred; + }); + }; + + describe('inline discussion', function() { + it('is shown after "Show Discussion" is clicked', function() { + var testView = createTestView(this), + showButton = testView.$('.discussion-show'); + showDiscussion(this, testView); + + // Verify that the discussion is now shown + expect(showButton).toHaveClass('shown'); + expect(showButton.text().trim()).toEqual('Hide Discussion'); + expect(testView.$('.inline-discussion:visible')).not.toHaveClass('is-hidden'); + }); + + it('is hidden after "Hide Discussion" is clicked', function() { + var testView = createTestView(this), + showButton = testView.$('.discussion-show'); + showDiscussion(this, testView); + + // Hide the discussion by clicking the toggle button again + testView.$('.discussion-show').click(); + + // Verify that the discussion is now hidden + expect(showButton).not.toHaveClass('shown'); + expect(showButton.text().trim()).toEqual('Show Discussion'); + expect(testView.$('.inline-discussion:visible')).toHaveClass('is-hidden'); + }); + }); + + describe('new post form', function() { + it('should not be visible when the discussion is first shown', function() { + var testView = createTestView(this); + showDiscussion(this, testView); + expect(testView.$('.new-post-article')).toHaveClass('is-hidden'); + }); + + it('should be shown when the "Add a Post" button is clicked', function() { + var testView = createTestView(this); + showDiscussion(this, testView); + testView.$('.new-post-btn').click(); + expect(testView.$('.new-post-article')).not.toHaveClass('is-hidden'); + }); + + it('should be hidden when the "Cancel" button is clicked', function() { + var testView = createTestView(this); + showDiscussion(this, testView); + testView.$('.new-post-btn').click(); + testView.$('.forum-new-post-form .cancel').click(); + expect(testView.$('.new-post-article')).toHaveClass('is-hidden'); + }); + + it('should be hidden when the "Close" button is clicked', function() { + var testView = createTestView(this); + showDiscussion(this, testView); + testView.$('.new-post-btn').click(); + testView.$('.forum-new-post-form .add-post-cancel').click(); + expect(testView.$('.new-post-article')).toHaveClass('is-hidden'); + }); + + it('should return to the thread listing after adding a post', function() { + var testView = createTestView(this); + showDiscussion(this, testView); + + // Navigate to an individual thread + testView.$('.forum-nav-thread-link').click(); + + // Click "Add a Post", fill in the form, and submit it + testView.$('.new-post-btn').click(); + testView.$('.js-post-title').text('Test title'); + testView.$('.wmd-input').text('Test body'); + setNextAjaxResult(this, { + hello: 'world' + }); + testView.$('.forum-new-post-form .submit').click(); + + // Verify that the list of threads is shown + expect(testView.$('.inline-threads')).not.toHaveClass('is-hidden'); + + // Verify that the individual thread is no longer shown + expect(testView.$('.group-visibility-label').length).toBe(0); + }); + }); + + describe('thread listing', function() { + it('builds a view that lists the threads', function() { + var testView = createTestView(this); + showDiscussion(this, testView); + expect(testView.$('.forum-nav-thread-title').text()).toBe(TEST_THREAD_TITLE); + }); + }); + + describe('thread post drill down', function() { + it('can drill down to a thread', function() { + var testView = createTestView(this); + showDiscussion(this, testView); + testView.$('.forum-nav-thread-link').click(); + + // Verify that the list of threads is hidden + expect(testView.$('.inline-threads')).toHaveClass('is-hidden'); + + // Verify that the individual thread is shown + expect(testView.$('.group-visibility-label').text().trim()).toBe('This post is visible to everyone.'); + }); + + it('can go back to the list of threads', function() { + var testView = createTestView(this); + showDiscussion(this, testView); + testView.$('.forum-nav-thread-link').click(); + testView.$('.all-posts-btn').click(); + + // Verify that the list of threads is shown + expect(testView.$('.inline-threads')).not.toHaveClass('is-hidden'); + + // Verify that the individual thread is no longer shown + expect(testView.$('.group-visibility-label').length).toBe(0); + }); + }); + }); +}()); 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 8a270fa22e..50f5048ab8 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 @@ -188,12 +188,11 @@ renderSingleThreadWithProps = function(props) { return makeView(new Discussion([new Thread(DiscussionViewSpecHelper.makeThreadWithProps(props))])).render(); }; - makeView = function(discussion, options) { - var opts = options || {}; + makeView = function(discussion) { return new DiscussionThreadListView({ el: $('#fixture-element'), collection: discussion, - showThreadPreview: opts.showThreadPreview || true, + showThreadPreview: true, courseSettings: new DiscussionCourseSettings({ is_cohorted: true }) @@ -551,7 +550,7 @@ it('does not add a search alert when no alternate term was searched', function() { testCorrection(this.view, null); expect(this.view.addSearchAlert.calls.count()).toEqual(1); - return expect(this.view.addSearchAlert.calls.mostRecent().args[0]).toMatch(/no threads matched/i); + return expect(this.view.addSearchAlert.calls.mostRecent().args[0]).toMatch(/no posts matched/i); }); it('clears search alerts when a new search is performed', function() { @@ -676,10 +675,8 @@ it('should not be shown when showThreadPreview is false', function() { var view, discussion = new Discussion([]), - options = { - showThreadPreview: false - }; - view = makeView(discussion, options); + showThreadPreview = false; + view = makeView(discussion, showThreadPreview); view.render(); expect(view.$el.find('.thread-preview-body').length).toEqual(0); }); diff --git a/common/static/common/js/spec/discussion/view/discussion_thread_view_spec.js b/common/static/common/js/spec/discussion/view/discussion_thread_view_spec.js index 3a35cfdbc5..5e99c20913 100644 --- a/common/static/common/js/spec/discussion/view/discussion_thread_view_spec.js +++ b/common/static/common/js/spec/discussion/view/discussion_thread_view_spec.js @@ -118,7 +118,7 @@ }; describe('closed and open Threads', function() { var checkCommentForm, checkVoteDisplay, createDiscussionThreadView; - createDiscussionThreadView = function(originallyClosed, mode) { + createDiscussionThreadView = function(originallyClosed) { var discussion, thread, threadData, view; threadData = DiscussionViewSpecHelper.makeThreadWithProps({ closed: originallyClosed @@ -128,63 +128,58 @@ view = new DiscussionThreadView({ model: thread, el: $('#fixture-element'), - mode: mode, course_settings: DiscussionSpecHelper.createTestCourseSettings() }); renderWithTestResponses(view, 1); - if (mode === 'inline') { - view.expand(); - } spyOn(DiscussionUtil, 'updateWithUndo').and.callFake(function(model, updates) { return model.set(updates); }); return view; }; - checkCommentForm = function(originallyClosed, mode) { + checkCommentForm = function(originallyClosed) { var view; - view = createDiscussionThreadView(originallyClosed, mode); + view = createDiscussionThreadView(originallyClosed); expect(view.$('.comment-form').closest('li').is(':visible')).toBe(!originallyClosed); expect(view.$('.discussion-reply-new').is(':visible')).toBe(!originallyClosed); view.$('.action-close').click(); expect(view.$('.comment-form').closest('li').is(':visible')).toBe(originallyClosed); return expect(view.$('.discussion-reply-new').is(':visible')).toBe(originallyClosed); }; - checkVoteDisplay = function(originallyClosed, mode) { + checkVoteDisplay = function(originallyClosed) { var view; - view = createDiscussionThreadView(originallyClosed, mode); + view = createDiscussionThreadView(originallyClosed); expect(view.$('.thread-main-wrapper .action-vote').is(':visible')).toBe(!originallyClosed); expect(view.$('.thread-main-wrapper .display-vote').is(':visible')).toBe(originallyClosed); view.$('.action-close').click(); expect(view.$('.action-vote').is(':visible')).toBe(originallyClosed); return expect(view.$('.display-vote').is(':visible')).toBe(!originallyClosed); }; - return _.each(['tab', 'inline'], function(mode) { + return function() { it( - 'Test that in ' + mode + ' mode when a closed thread is opened the comment form is displayed', - function() { return checkCommentForm(true, mode); } + 'Test that when a closed thread is opened the comment form is displayed', + function() { return checkCommentForm(true); } ); it( - 'Test that in ' + mode + ' mode when a open thread is closed the comment form is hidden', - function() { return checkCommentForm(false, mode); } + 'Test that when a open thread is closed the comment form is hidden', + function() { return checkCommentForm(false); } ); it( - 'Test that in ' + mode + ' mode when a closed thread is opened the vote button is displayed and ' + + 'Test that when a closed thread is opened the vote button is displayed and ' + 'vote count is hidden', - function() { return checkVoteDisplay(true, mode); } + function() { return checkVoteDisplay(true); } ); it( - 'Test that in ' + mode + ' mode when a open thread is closed the vote button is hidden and ' + + 'Test that when a open thread is closed the vote button is hidden and ' + 'vote count is displayed', - function() { return checkVoteDisplay(false, mode); } + function() { return checkVoteDisplay(false); } ); - }); + }; }); - describe('tab mode', function() { + describe('thread responses', function() { beforeEach(function() { this.view = new DiscussionThreadView({ model: this.thread, el: $('#fixture-element'), - mode: 'tab', course_settings: DiscussionSpecHelper.createTestCourseSettings() }); }); @@ -276,119 +271,6 @@ }); }); }); - describe('inline mode', function() { - beforeEach(function() { - this.view = new DiscussionThreadView({ - model: this.thread, - el: $('#fixture-element'), - mode: 'inline', - course_settings: DiscussionSpecHelper.createTestCourseSettings() - }); - }); - describe('render', function() { - it('shows content that should be visible when collapsed', function() { - this.view.render(); - return assertExpandedContentVisible(this.view, false); - }); - it('does not render any responses by default', function() { - this.view.render(); - expect($.ajax).not.toHaveBeenCalled(); - return expect(this.view.$el.find('.responses li').length).toEqual(0); - }); - }); - describe('focus', function() { - it('sends focus to the conversation when opened', function(done) { - var self; - DiscussionViewSpecHelper.setNextResponseContent({ - resp_total: 0, - children: [] - }); - this.view.render(); - this.view.expand(); - self = this; - return jasmine.waitUntil(function() { - var article; - article = self.view.$el.find('.discussion-article'); - return article[0] === article[0].ownerDocument.activeElement; - }).then(function() { - return done(); - }); - }); - }); - describe('expand/collapse', function() { - it('shows/hides appropriate content', function() { - DiscussionViewSpecHelper.setNextResponseContent({ - resp_total: 0, - children: [] - }); - this.view.render(); - this.view.expand(); - assertExpandedContentVisible(this.view, true); - this.view.collapse(); - return assertExpandedContentVisible(this.view, false); - }); - it('switches between the abbreviated and full body', function() { - var expectedAbbreviation, longBody; - DiscussionViewSpecHelper.setNextResponseContent({ - resp_total: 0, - children: [] - }); - longBody = new Array(100).join('test '); - expectedAbbreviation = DiscussionUtil.abbreviateString(longBody, 140); - this.thread.set('body', longBody); - this.view.render(); - expect($('.post-body').text()).toEqual(expectedAbbreviation); - expect(DiscussionThreadShowView.prototype.convertMath).toHaveBeenCalled(); - DiscussionThreadShowView.prototype.convertMath.calls.reset(); - this.view.expand(); - expect($('.post-body').text()).toEqual(longBody); - expect(DiscussionThreadShowView.prototype.convertMath).toHaveBeenCalled(); - DiscussionThreadShowView.prototype.convertMath.calls.reset(); - this.view.collapse(); - expect($('.post-body').text()).toEqual(expectedAbbreviation); - return expect(DiscussionThreadShowView.prototype.convertMath).toHaveBeenCalled(); - }); - it('strips script tags appropriately', function() { - var longMaliciousBody, maliciousAbbreviation; - DiscussionViewSpecHelper.setNextResponseContent({ - resp_total: 0, - children: [] - }); - longMaliciousBody = new Array(100).join( - "\n" - ); - this.thread.set('body', longMaliciousBody); - maliciousAbbreviation = DiscussionUtil.abbreviateString(this.thread.get('body'), 140); - this.view.render(); - expect($('.post-body').html()).not.toEqual(maliciousAbbreviation); - expect($('.post-body').text()).toEqual(maliciousAbbreviation); - expect($('.post-body').html()).not.toContain(' -
    - +
    +
    +
    -
    + -
    - <% _.each(threads, function(thread) { %> -
    -
    - <% }); %> -
    +
    +
    +
    -
    -
    +
    +
    + +
    +
    +
    +
    +
    + diff --git a/common/static/common/templates/discussion/new-post.underscore b/common/static/common/templates/discussion/new-post.underscore index 2289dcec62..2ee1f7557c 100644 --- a/common/static/common/templates/discussion/new-post.underscore +++ b/common/static/common/templates/discussion/new-post.underscore @@ -1,4 +1,11 @@
    + <% if (mode === 'inline') { %> +

    <%- gettext("Add a Post") %>

    + + <% } %>
    <% if (cohort_options) { %> diff --git a/common/static/common/templates/discussion/thread-response-show.underscore b/common/static/common/templates/discussion/thread-response-show.underscore index fc227aca2f..29cbd233d8 100644 --- a/common/static/common/templates/discussion/thread-response-show.underscore +++ b/common/static/common/templates/discussion/thread-response-show.underscore @@ -1,4 +1,4 @@ -
    +
    <%= author_display %>

    diff --git a/common/static/common/templates/discussion/thread-show.underscore b/common/static/common/templates/discussion/thread-show.underscore index 8cb3469866..5055f5be29 100644 --- a/common/static/common/templates/discussion/thread-show.underscore +++ b/common/static/common/templates/discussion/thread-show.underscore @@ -1,7 +1,23 @@

    -
    +
    + <% if (!readOnly) { %> +
    + <%= + _.template( + $('#forum-actions').html())( + { + contentId: cid, + contentType: 'post', + primaryActions: ['vote', 'follow'], + secondaryActions: ['pin', 'edit', 'delete', 'report', 'close'], + readOnly: readOnly + } + ) + %> +
    + <% } %>
    -

    <%- title %>

    +

    <%- title %>

    <% var timeAgoHtml = interpolate( @@ -32,27 +48,11 @@

    - <% if (!readOnly) { %> -
    - <%= - _.template( - $('#forum-actions').html())( - { - contentId: cid, - contentType: 'post', - primaryActions: ['vote', 'follow'], - secondaryActions: ['pin', 'edit', 'delete', 'report', 'close'], - readOnly: readOnly - } - ) - %> -
    - <% } %>
    <%- body %>
    - <% if (mode == "tab" && obj.courseware_url) { %> + <% if (mode === "tab" && obj.courseware_url) { %> <% var courseware_title_linked = interpolate( '%(courseware_title)s', diff --git a/common/static/common/templates/discussion/thread.underscore b/common/static/common/templates/discussion/thread.underscore index e2881e01fe..23520e68d9 100644 --- a/common/static/common/templates/discussion/thread.underscore +++ b/common/static/common/templates/discussion/thread.underscore @@ -9,7 +9,7 @@
    <% if (!readOnly) { %>
    -
    @@ -26,14 +26,10 @@
      - +
      <% } %>
      -
      - - -
      diff --git a/common/test/acceptance/fixtures/discussion.py b/common/test/acceptance/fixtures/discussion.py index 0607625fb0..4b1b0e70e5 100644 --- a/common/test/acceptance/fixtures/discussion.py +++ b/common/test/acceptance/fixtures/discussion.py @@ -51,6 +51,7 @@ class Thread(ContentFactory): group_id = None pinned = False read = False + context = "course" class Comment(ContentFactory): diff --git a/common/test/acceptance/pages/lms/discussion.py b/common/test/acceptance/pages/lms/discussion.py index 7ba2af8cf7..80134635ed 100644 --- a/common/test/acceptance/pages/lms/discussion.py +++ b/common/test/acceptance/pages/lms/discussion.py @@ -15,6 +15,54 @@ class DiscussionPageMixin(object): def is_ajax_finished(self): return self.browser.execute_script("return jQuery.active") == 0 + def find_visible_element(self, selector): + """ + Finds a single visible element with the specified selector. + """ + full_selector = selector + if self.root_selector: + full_selector = self.root_selector + " " + full_selector + elements = self.q(css=full_selector) + return next((element for element in elements if element.is_displayed()), None) + + @property + def new_post_button(self): + """ + Returns the new post button if visible, else it returns None. + """ + return self.find_visible_element(".new-post-btn") + + @property + def new_post_form(self): + """ + Returns the new post form if visible, else it returns None. + """ + return self.find_visible_element(".forum-new-post-form") + + def click_new_post_button(self): + """ + Clicks the 'New Post' button. + """ + self.wait_for( + lambda: self.new_post_button, + description="Waiting for new post button" + ) + self.new_post_button.click() + self.wait_for( + lambda: self.new_post_form, + description="Waiting for new post form" + ) + + def click_cancel_new_post(self): + """ + Clicks the 'Cancel' button from the new post form. + """ + self.click_element(".cancel") + self.wait_for( + lambda: not self.new_post_form, + "Waiting for new post form to close" + ) + class DiscussionThreadPage(PageObject, DiscussionPageMixin): url = None @@ -470,12 +518,15 @@ class DiscussionTabSingleThreadPage(CoursePage): return len(self.q(css=".forum-nav-thread").results) == thread_count -class InlineDiscussionPage(PageObject): +class InlineDiscussionPage(PageObject, DiscussionPageMixin): + """ + Acceptance tests for inline discussions. + """ url = None def __init__(self, browser, discussion_id): super(InlineDiscussionPage, self).__init__(browser) - self._discussion_selector = ( + self.root_selector = ( ".discussion-module[data-discussion-id='{discussion_id}'] ".format( discussion_id=discussion_id ) @@ -486,11 +537,11 @@ class InlineDiscussionPage(PageObject): Returns a query corresponding to the given CSS selector within the scope of this discussion page """ - return self.q(css=self._discussion_selector + " " + selector) + return self.q(css=self.root_selector + " " + selector) def is_browser_on_page(self): self.wait_for_ajax() - return self.q(css=self._discussion_selector).present + return self.q(css=self.root_selector).present def is_discussion_expanded(self): return self._find_within(".discussion").present @@ -504,60 +555,43 @@ class InlineDiscussionPage(PageObject): ).fulfill() def get_num_displayed_threads(self): - return len(self._find_within(".discussion-thread")) - - def has_thread(self, thread_id): - """Returns true if this page is showing the thread with the specified id.""" - return self._find_within('.discussion-thread#thread_{}'.format(thread_id)).present + return len(self._find_within(".forum-nav-thread")) def element_exists(self, selector): - return self.q(css=self._discussion_selector + " " + selector).present - - def is_new_post_opened(self): - return self._find_within(".new-post-article").visible + return self.q(css=self.root_selector + " " + selector).present def click_element(self, selector): self.wait_for_element_presence( - "{discussion} {selector}".format(discussion=self._discussion_selector, selector=selector), + "{discussion} {selector}".format(discussion=self.root_selector, selector=selector), "{selector} is visible".format(selector=selector) ) self._find_within(selector).click() - def click_cancel_new_post(self): - self.click_element(".cancel") - EmptyPromise( - lambda: not self.is_new_post_opened(), - "New post closed" - ).fulfill() - - def click_new_post_button(self): - self.click_element(".new-post-btn") - EmptyPromise( - self.is_new_post_opened, - "New post opened" - ).fulfill() - @wait_for_js def _is_element_visible(self, selector): query = self._find_within(selector) return query.present and query.visible + def show_thread(self, thread_id): + """ + Clicks the link for the specified thread to show the detailed view. + """ + thread_selector = ".forum-nav-thread[data-id='{thread_id}'] .forum-nav-thread-link".format(thread_id=thread_id) + self._find_within(thread_selector).first.click() + self.thread_page = InlineDiscussionThreadPage(self.browser, thread_id) # pylint: disable=attribute-defined-outside-init + self.thread_page.wait_for_page() + class InlineDiscussionThreadPage(DiscussionThreadPage): + """ + Page object to manipulate an individual thread view in an inline discussion. + """ def __init__(self, browser, thread_id): super(InlineDiscussionThreadPage, self).__init__( browser, - "body.courseware .discussion-module #thread_{thread_id}".format(thread_id=thread_id) + ".discussion-module .discussion-article[data-id='{thread_id}']".format(thread_id=thread_id) ) - def expand(self): - """Clicks the link to expand the thread""" - self._find_within(".forum-thread-expand").first.click() - EmptyPromise( - lambda: bool(self.get_response_total_text()), - "Thread expanded" - ).fulfill() - def is_thread_anonymous(self): return not self.q(css=".posted-details > .username").present @@ -584,9 +618,9 @@ class DiscussionUserProfilePage(CoursePage): return ( self.q(css='.discussion-user-threads[data-course-id="{}"]'.format(self.course_id)).present and - self.q(css='.user-profile .learner-profile-link').present + self.q(css='.user-name').present and - self.q(css='.user-profile .learner-profile-link').text[0] == self.username + self.q(css='.user-name').text[0] == self.username ) @wait_for_js @@ -594,85 +628,16 @@ class DiscussionUserProfilePage(CoursePage): return self.browser.execute_script("return $('html, body').offset().top") == 0 def get_shown_thread_ids(self): - elems = self.q(css="article.discussion-thread") - return [elem.get_attribute("id")[7:] for elem in elems] - - def get_current_page(self): - def check_func(): - try: - current_page = int(self.q(css="nav.discussion-paginator li.current-page").text[0]) - except: - return False, None - return True, current_page - - return Promise( - check_func, 'discussion-paginator current page has text', timeout=5, - ).fulfill() - - def _check_pager(self, text, page_number=None): - """ - returns True if 'text' matches the text in any of the pagination elements. If - page_number is provided, only return True if the element points to that result - page. - """ - elems = self.q(css=self.PAGING_SELECTOR).filter(lambda elem: elem.text == text) - if page_number: - elems = elems.filter(lambda elem: int(elem.get_attribute('data-page-number')) == page_number) - return elems.present - - def get_clickable_pages(self): - return sorted([ - int(elem.get_attribute('data-page-number')) - for elem in self.q(css=self.PAGING_SELECTOR) - if str(elem.text).isdigit() - ]) - - def is_prev_button_shown(self, page_number=None): - return self._check_pager(self.TEXT_PREV, page_number) - - def is_next_button_shown(self, page_number=None): - return self._check_pager(self.TEXT_NEXT, page_number) - - def _click_pager_with_text(self, text, page_number): - """ - click the first pagination element with whose text is `text` and ensure - the resulting page number matches `page_number`. - """ - targets = [elem for elem in self.q(css=self.PAGING_SELECTOR) if elem.text == text] - targets[0].click() - EmptyPromise( - lambda: self.get_current_page() == page_number, - "navigated to desired page" - ).fulfill() - - def click_prev_page(self): - self._click_pager_with_text(self.TEXT_PREV, self.get_current_page() - 1) - EmptyPromise( - self.is_window_on_top, - "Window is on top" - ).fulfill() - - def click_next_page(self): - self._click_pager_with_text(self.TEXT_NEXT, self.get_current_page() + 1) - EmptyPromise( - self.is_window_on_top, - "Window is on top" - ).fulfill() - - def click_on_page(self, page_number): - self._click_pager_with_text(unicode(page_number), page_number) - EmptyPromise( - self.is_window_on_top, - "Window is on top" - ).fulfill() + elems = self.q(css="li.forum-nav-thread") + return [elem.get_attribute("data-id") for elem in elems] def click_on_sidebar_username(self): self.wait_for_page() - self.q(css='.learner-profile-link').first.click() + self.q(css='.user-name').first.click() def get_user_roles(self): """Get user roles""" - return self.q(css='.sidebar-user-roles').text[0] + return self.q(css='.user-roles').text[0] class DiscussionTabHomePage(CoursePage, DiscussionPageMixin): @@ -682,6 +647,7 @@ class DiscussionTabHomePage(CoursePage, DiscussionPageMixin): def __init__(self, browser, course_id): super(DiscussionTabHomePage, self).__init__(browser, course_id) self.url_path = "discussion/forum/" + self.root_selector = None def is_browser_on_page(self): return self.q(css=".discussion-body section.home-header").present @@ -733,18 +699,6 @@ class DiscussionTabHomePage(CoursePage, DiscussionPageMixin): "waiting for dismissed alerts to disappear" ).fulfill() - def click_new_post_button(self): - """ - Clicks the 'New Post' button. - """ - self.new_post_button.click() - EmptyPromise( - lambda: ( - self.new_post_form - ), - "New post action succeeded" - ).fulfill() - def click_element(self, selector): """ Clicks the element specified by selector @@ -752,22 +706,6 @@ class DiscussionTabHomePage(CoursePage, DiscussionPageMixin): element = self.q(css=selector) return element.click() - @property - def new_post_button(self): - """ - Returns the new post button. - """ - elements = self.q(css=".new-post-btn") - return elements.first if elements.visible and len(elements) == 1 else None - - @property - def new_post_form(self): - """ - Returns the new post form. - """ - elements = self.q(css=".forum-new-post-form") - return elements[0] if elements.visible and len(elements) == 1 else None - def set_new_post_editor_value(self, new_body): """ Set the Discussions new post editor (wmd) with the content in new_body diff --git a/common/test/acceptance/tests/discussion/helpers.py b/common/test/acceptance/tests/discussion/helpers.py index 0da35d8b21..aa2e5fbb9c 100644 --- a/common/test/acceptance/tests/discussion/helpers.py +++ b/common/test/acceptance/tests/discussion/helpers.py @@ -12,6 +12,7 @@ from common.test.acceptance.fixtures.discussion import ( Thread, Response, ForumsConfigMixin, + MultipleThreadFixture, ) from common.test.acceptance.pages.lms.discussion import DiscussionTabSingleThreadPage from common.test.acceptance.tests.helpers import UniqueCourseTest @@ -37,6 +38,22 @@ class BaseDiscussionMixin(object): self.setup_thread_page(thread_id) return thread_id + def setup_multiple_threads(self, thread_count, **thread_kwargs): + """ + Set up multiple threads on the page by passing 'thread_count'. + """ + self.thread_ids = [] # pylint: disable=attribute-defined-outside-init + threads = [] # pylint: disable=attribute-defined-outside-init + for i in range(thread_count): + thread_id = "test_thread_{}_{}".format(i, uuid4().hex) + thread_body = "Dummy long text body." * 50 + threads.append( + Thread(id=thread_id, commentable_id=self.discussion_id, body=thread_body, **thread_kwargs), + ) + self.thread_ids.append(thread_id) + thread_fixture = MultipleThreadFixture(threads) + thread_fixture.push() + class CohortTestMixin(object): """ diff --git a/common/test/acceptance/tests/discussion/test_cohorts.py b/common/test/acceptance/tests/discussion/test_cohorts.py index dbdbac8237..fa765eb176 100644 --- a/common/test/acceptance/tests/discussion/test_cohorts.py +++ b/common/test/acceptance/tests/discussion/test_cohorts.py @@ -129,8 +129,8 @@ class InlineDiscussionTest(UniqueCourseTest): discussion_page = InlineDiscussionPage(self.browser, self.discussion_id) discussion_page.expand_discussion() self.assertEqual(discussion_page.get_num_displayed_threads(), 1) - self.thread_page = InlineDiscussionThreadPage(self.browser, thread_id) # pylint: disable=attribute-defined-outside-init - self.thread_page.expand() + discussion_page.show_thread(thread_id) + self.thread_page = discussion_page.thread_page # pylint: disable=attribute-defined-outside-init def refresh_thread_page(self, thread_id): self.browser.refresh() diff --git a/common/test/acceptance/tests/discussion/test_discussion.py b/common/test/acceptance/tests/discussion/test_discussion.py index 80a0b0b188..296d54fc0c 100644 --- a/common/test/acceptance/tests/discussion/test_discussion.py +++ b/common/test/acceptance/tests/discussion/test_discussion.py @@ -33,7 +33,8 @@ from common.test.acceptance.fixtures.discussion import ( Response, Comment, SearchResult, - MultipleThreadFixture) + MultipleThreadFixture, +) from common.test.acceptance.tests.discussion.helpers import BaseDiscussionMixin from common.test.acceptance.tests.helpers import skip_if_browser @@ -416,7 +417,7 @@ class DiscussionTabSingleThreadTest(BaseDiscussionTestCase, DiscussionResponsePa self.assertFalse(self.thread_page.is_comment_deletable("comment1")) -class DiscussionTabMultipleThreadTest(BaseDiscussionTestCase): +class DiscussionTabMultipleThreadTest(BaseDiscussionTestCase, BaseDiscussionMixin): """ Tests for the discussion page with multiple threads """ @@ -441,19 +442,6 @@ class DiscussionTabMultipleThreadTest(BaseDiscussionTestCase): ) self.thread_page_1.visit() - @attr(shard=2) - def setup_multiple_threads(self, thread_count): - threads = [] - for i in range(thread_count): - thread_id = "test_thread_{}_{}".format(i, uuid4().hex) - thread_body = "Dummy Long text body." * 50 - threads.append( - Thread(id=thread_id, commentable_id=self.discussion_id, body=thread_body), - ) - self.thread_ids.append(thread_id) - view = MultipleThreadFixture(threads) - view.push() - @attr('a11y') def test_page_accessibility(self): self.thread_page_1.a11y_audit.config.set_rules({ @@ -1031,41 +1019,32 @@ class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMix def setup_thread_page(self, thread_id): self.discussion_page.expand_discussion() - self.assertEqual(self.discussion_page.get_num_displayed_threads(), 1) - self.thread_page = InlineDiscussionThreadPage(self.browser, thread_id) # pylint: disable=attribute-defined-outside-init - self.thread_page.expand() + self.discussion_page.show_thread(thread_id) + self.thread_page = self.discussion_page.thread_page # pylint: disable=attribute-defined-outside-init - def setup_multiple_inline_threads(self, thread_count): + @attr('a11y') + def test_inline_a11y(self): """ - Set up multiple treads on the page by passing 'thread_count' + Tests Inline Discussion for accessibility issues. """ - threads = [] - for i in range(thread_count): - thread_id = "test_thread_{}_{}".format(i, uuid4().hex) - threads.append( - Thread(id=thread_id, commentable_id=self.discussion_id), - ) - self.thread_ids.append(thread_id) - thread_fixture = MultipleThreadFixture(threads) - thread_fixture.add_response( - Response(id="response1"), - [Comment(id="comment1", user_id="other"), Comment(id="comment2", user_id=self.user_id)], - threads[0] - ) - thread_fixture.push() + self.setup_multiple_threads(thread_count=3) - def test_page_while_expanding_inline_discussion(self): - """ - Tests for the Inline Discussion page with multiple treads. Page should not focus 'thread-wrapper' - after loading responses. - """ - self.setup_multiple_inline_threads(thread_count=3) + # First test the a11y of the expanded list of threads self.discussion_page.expand_discussion() - thread_page = InlineDiscussionThreadPage(self.browser, self.thread_ids[0]) - thread_page.expand() + self.discussion_page.a11y_audit.config.set_rules({ + 'ignore': [ + 'section' + ] + }) + self.discussion_page.a11y_audit.check_for_accessibility_errors() - # Check if 'thread-wrapper' is focused after expanding thread - self.assertFalse(thread_page.check_if_selector_is_focused(selector='.thread-wrapper')) + # Now show the first thread and test the a11y again + self.discussion_page.show_thread(self.thread_ids[0]) + self.discussion_page.a11y_audit.check_for_accessibility_errors() + + # Finally show the new post form and test its a11y + self.discussion_page.click_new_post_button() + self.discussion_page.a11y_audit.check_for_accessibility_errors() def test_add_a_post_is_present_if_can_create_thread_when_expanded(self): self.discussion_page.expand_discussion() @@ -1083,7 +1062,7 @@ class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMix thread = Thread(id=uuid4().hex, anonymous_to_peers=True, commentable_id=self.discussion_id) thread_fixture = SingleThreadViewFixture(thread) thread_fixture.push() - self.setup_thread_page(thread.get("id")) + self.setup_thread_page(thread.get("id")) # pylint: disable=no-member self.assertEqual(self.thread_page.is_thread_anonymous(), not is_staff) def test_anonymous_to_peers_threads_as_staff(self): @@ -1116,41 +1095,66 @@ class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMix Response(id="response1"), [Comment(id="comment1", user_id="other"), Comment(id="comment2", user_id=self.user_id)]) thread_fixture.push() - self.setup_thread_page(thread.get("id")) + self.setup_thread_page(thread.get("id")) # pylint: disable=no-member self.assertFalse(self.thread_page.has_add_response_button()) self.assertFalse(self.thread_page.is_element_visible("action-more")) def test_dual_discussion_xblock(self): """ Scenario: Two discussion xblocks in one unit shouldn't override their actions - Given that I'm on courseware page where there are two inline discussion - When I click on one discussion xblock new post button - Then it should add new post form of that xblock in DOM - And I should be shown new post form of that xblock - And I shouldn't be shown second discussion xblock new post form - And I click on second discussion xblock new post button - Then it should add new post form of second xblock in DOM - And I should be shown second discussion new post form - And I shouldn't be shown first discussion xblock new post form - And I have two new post form in the DOM - When I click back on first xblock new post button - And I should be shown new post form of that xblock - And I shouldn't be shown second discussion xblock new post form + Given that I'm on a courseware page where there are two inline discussion + When I click on the first discussion block's new post button + Then I should be shown only the new post form for the first block + When I click on the second discussion block's new post button + Then I should be shown both new post forms + When I cancel the first form + Then I should be shown only the new post form for the second block + When I cancel the second form + And I click on the first discussion block's new post button + Then I should be shown only the new post form for the first block + When I cancel the first form + Then I should be shown none of the forms """ self.discussion_page.wait_for_page() self.additional_discussion_page.wait_for_page() + + # Expand the first discussion, click to add a post self.discussion_page.expand_discussion() self.discussion_page.click_new_post_button() - with self.discussion_page.handle_alert(): - self.discussion_page.click_cancel_new_post() + + # Verify that only the first discussion's form is shown + self.assertIsNotNone(self.discussion_page.new_post_form) + self.assertIsNone(self.additional_discussion_page.new_post_form) + + # Expand the second discussion, click to add a post self.additional_discussion_page.expand_discussion() self.additional_discussion_page.click_new_post_button() - self.assertFalse(self.discussion_page._is_element_visible(".new-post-article")) - with self.additional_discussion_page.handle_alert(): - self.additional_discussion_page.click_cancel_new_post() - self.discussion_page.expand_discussion() + + # Verify that both discussion's forms are shown + self.assertIsNotNone(self.discussion_page.new_post_form) + self.assertIsNotNone(self.additional_discussion_page.new_post_form) + + # Cancel the first form + self.discussion_page.click_cancel_new_post() + + # Verify that only the second discussion's form is shown + self.assertIsNone(self.discussion_page.new_post_form) + self.assertIsNotNone(self.additional_discussion_page.new_post_form) + + # Cancel the second form and click to show the first one + self.additional_discussion_page.click_cancel_new_post() self.discussion_page.click_new_post_button() - self.assertFalse(self.additional_discussion_page._is_element_visible(".new-post-article")) + + # Verify that only the first discussion's form is shown + self.assertIsNotNone(self.discussion_page.new_post_form) + self.assertIsNone(self.additional_discussion_page.new_post_form) + + # Cancel the first form + self.discussion_page.click_cancel_new_post() + + # Verify that neither discussion's forms are shwon + self.assertIsNone(self.discussion_page.new_post_form) + self.assertIsNone(self.additional_discussion_page.new_post_form) @attr(shard=2) @@ -1188,11 +1192,18 @@ class DiscussionUserProfileTest(UniqueCourseTest): roles_str = ','.join(roles) return AutoAuthPage(self.browser, course_id=self.course_id, roles=roles_str, **user_info).visit().get_user_id() - def check_pages(self, num_threads): - # set up the stub server to return the desired amount of thread results - threads = [Thread(id=uuid4().hex) for _ in range(num_threads)] - UserProfileViewFixture(threads).push() - # navigate to default view (page 1) + def test_redirects_to_learner_profile(self): + """ + Scenario: Verify that learner-profile link is present on forum discussions page and we can navigate to it. + + Given that I am on discussion forum user's profile page. + And I can see a username on the page + When I click on my username. + Then I will be navigated to Learner Profile page. + And I can my username on Learner Profile page + """ + learner_profile_page = LearnerProfilePage(self.browser, self.PROFILED_USERNAME) + page = DiscussionUserProfilePage( self.browser, self.course_id, @@ -1200,90 +1211,6 @@ class DiscussionUserProfileTest(UniqueCourseTest): self.PROFILED_USERNAME ) page.visit() - - current_page = 1 - total_pages = max(num_threads - 1, 1) / self.PAGE_SIZE + 1 - all_pages = range(1, total_pages + 1) - return page - - def _check_page(): - # ensure the page being displayed as "current" is the expected one - self.assertEqual(page.get_current_page(), current_page) - # ensure the expected threads are being shown in the right order - threads_expected = threads[(current_page - 1) * self.PAGE_SIZE:current_page * self.PAGE_SIZE] - self.assertEqual(page.get_shown_thread_ids(), [t["id"] for t in threads_expected]) - # ensure the clickable page numbers are the expected ones - self.assertEqual(page.get_clickable_pages(), [ - p for p in all_pages - if p != current_page - and p - 2 <= current_page <= p + 2 - or (current_page > 2 and p == 1) - or (current_page < total_pages and p == total_pages) - ]) - # ensure the previous button is shown, but only if it should be. - # when it is shown, make sure it works. - if current_page > 1: - self.assertTrue(page.is_prev_button_shown(current_page - 1)) - page.click_prev_page() - self.assertEqual(page.get_current_page(), current_page - 1) - page.click_next_page() - self.assertEqual(page.get_current_page(), current_page) - else: - self.assertFalse(page.is_prev_button_shown()) - # ensure the next button is shown, but only if it should be. - if current_page < total_pages: - self.assertTrue(page.is_next_button_shown(current_page + 1)) - else: - self.assertFalse(page.is_next_button_shown()) - - # click all the way up through each page - for __ in range(current_page, total_pages): - _check_page() - if current_page < total_pages: - page.click_on_page(current_page + 1) - current_page += 1 - - # click all the way back down - for __ in range(current_page, 0, -1): - _check_page() - if current_page > 1: - page.click_on_page(current_page - 1) - current_page -= 1 - - def test_0_threads(self): - self.check_pages(0) - - def test_1_thread(self): - self.check_pages(1) - - def test_20_threads(self): - self.check_pages(20) - - def test_21_threads(self): - self.check_pages(21) - - def test_151_threads(self): - self.check_pages(151) - - def test_pagination_window_reposition(self): - page = self.check_pages(50) - page.click_next_page() - page.wait_for_ajax() - self.assertTrue(page.is_window_on_top()) - - def test_redirects_to_learner_profile(self): - """ - Scenario: Verify that learner-profile link is present on forum discussions page and we can navigate to it. - - Given that I am on discussion forum user's profile page. - And I can see a username on left sidebar - When I click on my username. - Then I will be navigated to Learner Profile page. - And I can my username on Learner Profile page - """ - learner_profile_page = LearnerProfilePage(self.browser, self.PROFILED_USERNAME) - - page = self.check_pages(1) page.click_on_sidebar_username() learner_profile_page.wait_for_page() @@ -1301,7 +1228,13 @@ class DiscussionUserProfileTest(UniqueCourseTest): ) # Visit the page and verify the roles are listed correctly. - page = self.check_pages(1) + page = DiscussionUserProfilePage( + self.browser, + self.course_id, + self.profiled_user_id, + self.PROFILED_USERNAME + ) + page.visit() student_roles = page.get_user_roles() self.assertEqual(student_roles, ', '.join(expected_student_roles)) @@ -1321,7 +1254,13 @@ class DiscussionUserProfileTest(UniqueCourseTest): # Visit the user profile in course discussion page of Course-B. Make sure the # roles are listed correctly. - page = self.check_pages(1) + page = DiscussionUserProfilePage( + self.browser, + self.course_id, + self.profiled_user_id, + self.PROFILED_USERNAME + ) + page.visit() self.assertEqual(page.get_user_roles(), u'Student') @@ -1357,7 +1296,7 @@ class DiscussionSearchAlertTest(UniqueCourseTest): def test_no_rewrite(self): self.setup_corrected_text(None) self.page.perform_search() - self.check_search_alert_messages(["no threads"]) + self.check_search_alert_messages(["no posts"]) @attr(shard=2) def test_rewrite_dismiss(self): @@ -1379,7 +1318,7 @@ class DiscussionSearchAlertTest(UniqueCourseTest): self.setup_corrected_text(None) self.page.perform_search() - self.check_search_alert_messages(["no threads"]) + self.check_search_alert_messages(["no posts"]) @attr(shard=2) def test_rewrite_and_user(self): @@ -1391,7 +1330,7 @@ class DiscussionSearchAlertTest(UniqueCourseTest): def test_user_only(self): self.setup_corrected_text(None) self.page.perform_search(self.SEARCHED_USERNAME) - self.check_search_alert_messages(["no threads", self.SEARCHED_USERNAME]) + self.check_search_alert_messages(["no posts", self.SEARCHED_USERNAME]) # make sure clicking the link leads to the user profile page UserProfileViewFixture([]).push() self.page.get_search_alert_links().first.click() diff --git a/common/test/acceptance/tests/lms/test_teams.py b/common/test/acceptance/tests/lms/test_teams.py index 089da322c7..0f2b63e56b 100644 --- a/common/test/acceptance/tests/lms/test_teams.py +++ b/common/test/acceptance/tests/lms/test_teams.py @@ -1689,7 +1689,8 @@ class TeamPageTest(TeamsTabBase): thread = Thread( id="test_thread_{}".format(uuid4().hex), commentable_id=self.teams[0]['discussion_topic_id'], - body="Dummy text body." + body="Dummy text body.", + context="standalone", ) thread_fixture = MultipleThreadFixture([thread]) thread_fixture.push() @@ -1718,14 +1719,15 @@ class TeamPageTest(TeamsTabBase): thread = self.setup_thread() self.team_page.visit() self.assertEqual(self.team_page.discussion_id, self.teams[0]['discussion_topic_id']) - discussion = self.team_page.discussion_page - discussion.wait_for_page() - self.assertTrue(discussion.is_discussion_expanded()) - self.assertEqual(discussion.get_num_displayed_threads(), 1) - self.assertTrue(discussion.has_thread(thread['id'])) + discussion_page = self.team_page.discussion_page + discussion_page.wait_for_page() + self.assertTrue(discussion_page.is_discussion_expanded()) + self.assertEqual(discussion_page.get_num_displayed_threads(), 1) + discussion_page.show_thread(thread['id']) + thread_page = discussion_page.thread_page assertion = self.assertTrue if should_have_permission else self.assertFalse - assertion(discussion.q(css='.post-header-actions').present) - assertion(discussion.q(css='.add-response').present) + assertion(thread_page.q(css='.post-header-actions').present) + assertion(thread_page.q(css='.add-response').present) def test_discussion_on_my_team_page(self): """ diff --git a/lms/djangoapps/discussion/static/discussion/js/discussion_profile_page_factory.js b/lms/djangoapps/discussion/static/discussion/js/discussion_profile_page_factory.js index 537418a102..469a1a35bd 100644 --- a/lms/djangoapps/discussion/static/discussion/js/discussion_profile_page_factory.js +++ b/lms/djangoapps/discussion/static/discussion/js/discussion_profile_page_factory.js @@ -4,17 +4,28 @@ define( [ 'jquery', + 'backbone', + 'common/js/discussion/content', + 'common/js/discussion/discussion', 'common/js/discussion/utils', 'common/js/discussion/models/discussion_user', + 'common/js/discussion/models/discussion_course_settings', 'discussion/js/views/discussion_user_profile_view' ], - function($, DiscussionUtil, DiscussionUser, DiscussionUserProfileView) { + function($, Backbone, Content, Discussion, DiscussionUtil, DiscussionUser, DiscussionCourseSettings, + DiscussionUserProfileView) { return function(options) { - var $element = options.$el, - threads = options.threads, + var threads = options.threads, + contentInfo = options.contentInfo, userInfo = options.userInfo, + user = new DiscussionUser(userInfo), page = options.page, - numPages = options.numPages; + numPages = options.numPages, + sortPreference = options.sortPreference, + discussionUserProfileView, + discussion, + courseSettings; + // Roles are not included in user profile page, but they are not used for anything DiscussionUtil.loadRoles({ Moderator: [], @@ -22,16 +33,25 @@ 'Community TA': [] }); - // TODO: remove global variable usage + DiscussionUtil.loadRoles(options.roles); window.$$course_id = options.courseId; - window.user = new DiscussionUser(userInfo); + window.courseName = options.course_name; + DiscussionUtil.setUser(user); + window.user = user; + Content.loadContentInfos(contentInfo); - new DiscussionUserProfileView({ // eslint-disable-line no-new - el: $element, - collection: threads, + // Create a discussion model + discussion = new Discussion(threads, {pages: numPages, sort: sortPreference}); + courseSettings = new DiscussionCourseSettings(options.courseSettings); + + discussionUserProfileView = new DiscussionUserProfileView({ + el: $('.discussion-user-threads'), + discussion: discussion, page: page, - numPages: numPages + numPages: numPages, + courseSettings: courseSettings }); + discussionUserProfileView.render(); }; }); }).call(this, define || RequireJS.define); diff --git a/lms/djangoapps/discussion/static/discussion/js/spec/discussion_profile_page_factory_spec.js b/lms/djangoapps/discussion/static/discussion/js/spec/discussion_profile_page_factory_spec.js index 1a1c3d4770..302512de0c 100644 --- a/lms/djangoapps/discussion/static/discussion/js/spec/discussion_profile_page_factory_spec.js +++ b/lms/djangoapps/discussion/static/discussion/js/spec/discussion_profile_page_factory_spec.js @@ -1,3 +1,5 @@ +/* globals Discussion */ + define( [ 'underscore', @@ -15,10 +17,12 @@ define( DiscussionProfilePageFactory(_.extend( { courseId: testCourseId, - $el: $('.discussion-user-threads'), - user_info: DiscussionSpecHelper.getTestUserInfo(), roles: DiscussionSpecHelper.getTestRoleInfo(), - sort_preference: null, + courseSettings: DiscussionSpecHelper.createTestCourseSettings().attributes, + el: $('.discussion-user-threads'), + discussion: new Discussion(), + userInfo: DiscussionSpecHelper.getTestUserInfo(), + sortPreference: null, threads: [], page: 1, numPages: 5 @@ -34,7 +38,7 @@ define( it('can render itself', function() { initializeDiscussionProfilePageFactory(); - expect($('.discussion-user-threads').text()).toContain('Active Threads'); + expect($('.discussion-user-threads').text()).toContain('Show'); }); }); } diff --git a/lms/djangoapps/discussion/static/discussion/js/spec/views/discussion_user_profile_view_spec.js b/lms/djangoapps/discussion/static/discussion/js/spec/views/discussion_user_profile_view_spec.js index 2709da2d94..531f807935 100644 --- a/lms/djangoapps/discussion/static/discussion/js/spec/views/discussion_user_profile_view_spec.js +++ b/lms/djangoapps/discussion/static/discussion/js/spec/views/discussion_user_profile_view_spec.js @@ -1,278 +1,42 @@ -define( - [ - 'underscore', - 'jquery', - 'URI', - 'common/js/discussion/utils', - 'common/js/discussion/views/discussion_thread_profile_view', - 'common/js/spec_helpers/discussion_spec_helper', - 'discussion/js/views/discussion_user_profile_view' - ], - function(_, $, URI, DiscussionUtil, DiscussionThreadProfileView, DiscussionSpecHelper, DiscussionUserProfileView) { - 'use strict'; +/* globals Discussion, DiscussionCourseSettings */ +define([ + 'underscore', + 'jquery', + 'URI', + 'common/js/discussion/utils', + 'common/js/discussion/views/discussion_thread_profile_view', + 'common/js/discussion/discussion', + 'common/js/spec_helpers/discussion_spec_helper', + 'discussion/js/views/discussion_user_profile_view' +], +function(_, $, URI, DiscussionUtil, DiscussionThreadProfileView, Discussion, +DiscussionSpecHelper, DiscussionUserProfileView) { + 'use strict'; + describe('DiscussionUserProfileView', function() { + var createDiscussionUserProfileView = function() { + var discussion = DiscussionSpecHelper.createTestDiscussion({}), + courseSettings = DiscussionSpecHelper.createTestCourseSettings(); - describe('DiscussionUserProfileView', function() { - var makeThreads, makeView; - beforeEach(function() { - DiscussionSpecHelper.setUpGlobals(); - DiscussionSpecHelper.setUnderscoreFixtures(); - return spyOn(DiscussionThreadProfileView.prototype, 'render'); + setFixtures(''); + DiscussionSpecHelper.setUnderscoreFixtures(); + + return new DiscussionUserProfileView({ + el: $('.discussion-user-profile-board'), + discussion: discussion, + courseSettings: courseSettings, + sortPreference: null }); + }; - makeThreads = function(numThreads) { - return _.map(_.range(numThreads), function(i) { - return { - id: i.toString(), - body: 'dummy body' - }; - }); - }; - - makeView = function(threads, page, numPages) { - return new DiscussionUserProfileView({ - collection: threads, - page: page, - numPages: numPages - }); - }; - - describe('thread rendering should be correct', function() { - var checkRender; - checkRender = function(numThreads) { - var threads, view; - threads = makeThreads(numThreads); - view = makeView(threads, 1, 1); - expect(view.$('.discussion').children().length).toEqual(numThreads); - return _.each(threads, function(thread) { - return expect(view.$('#thread_' + thread.id).length).toEqual(1); - }); - }; - - it('with no threads', function() { - return checkRender(0); - }); - - it('with one thread', function() { - return checkRender(1); - }); - - it('with several threads', function() { - return checkRender(5); - }); - }); - - describe('pagination rendering should be correct', function() { - var checkRender = function(params) { - var getPageNumber, paginator, view; - view = makeView([], params.page, params.numPages); - paginator = view.$('.discussion-paginator'); - expect(paginator.find('.current-page').text()).toEqual(params.page.toString()); - expect(paginator.find('.first-page').length).toBe(params.first ? 1 : 0); - expect(paginator.find('.previous-page').length).toBe(params.previous ? 1 : 0); - expect(paginator.find('.previous-ellipses').length).toBe(params.leftdots ? 1 : 0); - expect(paginator.find('.next-page').length).toBe(params.next ? 1 : 0); - expect(paginator.find('.next-ellipses').length).toBe(params.rightdots ? 1 : 0); - expect(paginator.find('.last-page').length).toBe(params.last ? 1 : 0); - getPageNumber = function(element) { - return parseInt($(element).text(), 10); - }; - expect(_.map(paginator.find('.lower-page a'), getPageNumber)).toEqual(params.lowPages); - return expect(_.map(paginator.find('.higher-page a'), getPageNumber)).toEqual(params.highPages); - }; - - it('for one page', function() { - return checkRender({ - page: 1, - numPages: 1, - previous: null, - first: null, - leftdots: false, - lowPages: [], - highPages: [], - rightdots: false, - last: null, - next: null - }); - }); - - it('for first page of three (max with no last)', function() { - return checkRender({ - page: 1, - numPages: 3, - previous: null, - first: null, - leftdots: false, - lowPages: [], - highPages: [2, 3], - rightdots: false, - last: null, - next: 2 - }); - }); - - it('for first page of four (has last but no dots)', function() { - return checkRender({ - page: 1, - numPages: 4, - previous: null, - first: null, - leftdots: false, - lowPages: [], - highPages: [2, 3], - rightdots: false, - last: 4, - next: 2 - }); - }); - - it('for first page of five (has dots)', function() { - return checkRender({ - page: 1, - numPages: 5, - previous: null, - first: null, - leftdots: false, - lowPages: [], - highPages: [2, 3], - rightdots: true, - last: 5, - next: 2 - }); - }); - - it('for last page of three (max with no first)', function() { - return checkRender({ - page: 3, - numPages: 3, - previous: 2, - first: null, - leftdots: false, - lowPages: [1, 2], - highPages: [], - rightdots: false, - last: null, - next: null - }); - }); - - it('for last page of four (has first but no dots)', function() { - return checkRender({ - page: 4, - numPages: 4, - previous: 3, - first: 1, - leftdots: false, - lowPages: [2, 3], - highPages: [], - rightdots: false, - last: null, - next: null - }); - }); - - it('for last page of five (has dots)', function() { - return checkRender({ - page: 5, - numPages: 5, - previous: 4, - first: 1, - leftdots: true, - lowPages: [3, 4], - highPages: [], - rightdots: false, - last: null, - next: null - }); - }); - - it('for middle page of five (max with no first/last)', function() { - return checkRender({ - page: 3, - numPages: 5, - previous: 2, - first: null, - leftdots: false, - lowPages: [1, 2], - highPages: [4, 5], - rightdots: false, - last: null, - next: 4 - }); - }); - - it('for middle page of seven (has first/last but no dots)', function() { - return checkRender({ - page: 4, - numPages: 7, - previous: 3, - first: 1, - leftdots: false, - lowPages: [2, 3], - highPages: [5, 6], - rightdots: false, - last: 7, - next: 5 - }); - }); - - it('for middle page of nine (has dots)', function() { - return checkRender({ - page: 5, - numPages: 9, - previous: 4, - first: 1, - leftdots: true, - lowPages: [3, 4], - highPages: [6, 7], - rightdots: true, - last: 9, - next: 6 - }); - }); - }); - - describe('pagination interaction', function() { - beforeEach(function() { - var deferred; - this.view = makeView(makeThreads(3), 1, 2); - deferred = $.Deferred(); - return spyOn($, 'ajax').and.returnValue(deferred); - }); - - it('causes updated rendering', function() { - $.ajax.and.callFake(function(params) { - params.success({ - discussion_data: [ - { - id: 'on_page_42', - body: 'dummy body' - } - ], - page: 42, - num_pages: 99 - }); - return { - always: function() { - } - }; - }); - this.view.$('.discussion-pagination a').first().click(); - expect(this.view.$('.current-page').text()).toEqual('42'); - return expect(this.view.$('.last-page').text()).toEqual('99'); - }); - - it('handles AJAX errors', function() { - spyOn(DiscussionUtil, 'discussionAlert'); - $.ajax.and.callFake(function(params) { - params.error(); - return { - always: function() { - } - }; - }); - this.view.$('.discussion-pagination a').first().click(); - return expect(DiscussionUtil.discussionAlert).toHaveBeenCalled(); - }); + describe('thread list in user profile page', function() { + it('should render', function() { + var discussionUserProfileView = createDiscussionUserProfileView(), + threadListView; + discussionUserProfileView.render(); + threadListView = discussionUserProfileView.discussionThreadListView; + threadListView.render(); + expect(threadListView.$('.forum-nav-thread-list').length).toBe(1); }); }); }); +}); diff --git a/lms/djangoapps/discussion/static/discussion/js/views/discussion_board_view.js b/lms/djangoapps/discussion/static/discussion/js/views/discussion_board_view.js index c6bd9c2677..075fcdf3f9 100644 --- a/lms/djangoapps/discussion/static/discussion/js/views/discussion_board_view.js +++ b/lms/djangoapps/discussion/static/discussion/js/views/discussion_board_view.js @@ -32,7 +32,6 @@ initialize: function(options) { this.courseSettings = options.courseSettings; - this.showThreadPreview = true; this.sidebar_padding = 10; this.current_search = ''; this.mode = 'all'; @@ -47,7 +46,7 @@ collection: this.discussion, el: this.$('.discussion-thread-list-container'), courseSettings: this.courseSettings, - showThreadPreview: this.showThreadPreview + supportsActiveThread: true }).render(); this.searchView = new DiscussionSearchView({ el: this.$('.forum-search') diff --git a/lms/djangoapps/discussion/static/discussion/js/views/discussion_user_profile_view.js b/lms/djangoapps/discussion/static/discussion/js/views/discussion_user_profile_view.js index 12d062e7ac..7c3e199ecd 100644 --- a/lms/djangoapps/discussion/static/discussion/js/views/discussion_user_profile_view.js +++ b/lms/djangoapps/discussion/static/discussion/js/views/discussion_user_profile_view.js @@ -1,3 +1,4 @@ +/* globals DiscussionThreadView */ (function(define) { 'use strict'; @@ -13,77 +14,67 @@ 'common/js/discussion/utils', 'common/js/discussion/views/discussion_thread_profile_view', 'text!discussion/templates/user-profile.underscore', - 'text!common/templates/discussion/pagination.underscore' + 'common/js/discussion/views/discussion_thread_list_view' ], function(_, $, Backbone, gettext, URI, HtmlUtils, ViewUtils, Discussion, DiscussionUtil, - DiscussionThreadProfileView, userProfileTemplate, paginationTemplate) { + DiscussionThreadProfileView, userProfileTemplate, DiscussionThreadListView) { var DiscussionUserProfileView = Backbone.View.extend({ events: { - 'click .discussion-paginator a': 'changePage' + 'click .all-posts-btn': 'navigateToAllThreads' }, initialize: function(options) { - Backbone.View.prototype.initialize.call(this); - this.page = options.page; - this.numPages = options.numPages; - this.discussion = new Discussion(); - this.discussion.on('reset', _.bind(this.render, this)); - this.discussion.reset(this.collection, {silent: false}); + this.courseSettings = options.courseSettings; + this.discussion = options.discussion; + this.mode = 'all'; + this.listenTo(this.model, 'change', this.render); }, render: function() { - var self = this, - baseUri = URI(window.location).removeSearch('page'), - pageUrlFunc, - paginationParams; - HtmlUtils.setHtml(this.$el, HtmlUtils.template(userProfileTemplate)({ - threads: self.discussion.models - })); - this.discussion.map(function(thread) { - var view = new DiscussionThreadProfileView({ - el: self.$('article#thread_' + thread.id), - model: thread - }); - view.render(); - return view; - }); - pageUrlFunc = function(page) { - return baseUri.clone().addSearch('page', page).toString(); - }; - paginationParams = DiscussionUtil.getPaginationParams(this.page, this.numPages, pageUrlFunc); - HtmlUtils.setHtml( - this.$el.find('.discussion-pagination'), - HtmlUtils.template(paginationTemplate)(paginationParams) + HtmlUtils.setHtml(this.$el, + HtmlUtils.template(userProfileTemplate)({}) ); + + this.discussionThreadListView = new DiscussionThreadListView({ + collection: this.discussion, + el: this.$('.inline-threads'), + courseSettings: this.courseSettings, + hideRefineBar: true // TODO: re-enable the search/filter bar when it works correctly + }).render(); + + this.discussionThreadListView.on('thread:selected', _.bind(this.navigateToThread, this)); + return this; }, - changePage: function(event) { - var self = this, - url; - event.preventDefault(); - url = $(event.target).attr('href'); - DiscussionUtil.safeAjax({ - $elem: this.$el, - $loading: $(event.target), - takeFocus: true, - url: url, - type: 'GET', - dataType: 'json', - success: function(response) { - self.page = response.page; - self.numPages = response.num_pages; - self.discussion.reset(response.discussion_data, {silent: false}); - history.pushState({}, '', url); - ViewUtils.setScrollTop(0); - }, - error: function() { - DiscussionUtil.discussionAlert( - gettext('Sorry'), - gettext('We had some trouble loading the page you requested. Please try again.') - ); - } + navigateToThread: function(threadId) { + var thread = this.discussion.get(threadId); + this.threadView = new DiscussionThreadView({ + el: this.$('.forum-content'), + model: thread, + mode: 'inline', + course_settings: this.courseSettings }); + this.threadView.render(); + this.listenTo(this.threadView.showView, 'thread:_delete', this.navigateToAllThreads); + this.discussionThreadListView.$el.addClass('is-hidden'); + this.$('.inline-thread').removeClass('is-hidden'); + }, + + navigateToAllThreads: function() { + // Hide the inline thread section + this.$('.inline-thread').addClass('is-hidden'); + + // Delete the thread view + this.threadView.$el.empty().off(); + this.threadView.stopListening(); + this.threadView = null; + + // Show the thread list view + this.discussionThreadListView.$el.removeClass('is-hidden'); + + // Set focus to thread list item that was saved as active + this.discussionThreadListView.$('.is-active').focus(); } }); diff --git a/lms/djangoapps/discussion/static/discussion/templates/search.underscore b/lms/djangoapps/discussion/static/discussion/templates/search.underscore index 3f289ce2a5..d1372529f3 100644 --- a/lms/djangoapps/discussion/static/discussion/templates/search.underscore +++ b/lms/djangoapps/discussion/static/discussion/templates/search.underscore @@ -6,4 +6,4 @@ id="search" placeholder="<%- gettext("Search all posts") %>" /> - + diff --git a/lms/djangoapps/discussion/static/discussion/templates/user-profile.underscore b/lms/djangoapps/discussion/static/discussion/templates/user-profile.underscore index 71d726711f..073e7bee08 100644 --- a/lms/djangoapps/discussion/static/discussion/templates/user-profile.underscore +++ b/lms/djangoapps/discussion/static/discussion/templates/user-profile.underscore @@ -1,7 +1,11 @@ -

      <%- gettext("Active Threads") %>

      -
      - <% _.each(threads, function(thread) { %> -
      - <% }); %> -
      -
      +
      + diff --git a/lms/djangoapps/discussion/templates/discussion/discussion_board.html b/lms/djangoapps/discussion/templates/discussion/discussion_board.html index 7882739cf1..03f07ca09d 100644 --- a/lms/djangoapps/discussion/templates/discussion/discussion_board.html +++ b/lms/djangoapps/discussion/templates/discussion/discussion_board.html @@ -67,7 +67,7 @@ DiscussionBoardFactory({ ## Add Post button % if has_permission(user, 'create_thread', course.id):
      - +
      % endif ## Search box @@ -82,7 +82,7 @@ DiscussionBoardFactory({
      - +
      diff --git a/lms/djangoapps/discussion/templates/discussion/discussion_profile_page.html b/lms/djangoapps/discussion/templates/discussion/discussion_profile_page.html index 1d2f4025f2..a727028d38 100644 --- a/lms/djangoapps/discussion/templates/discussion/discussion_profile_page.html +++ b/lms/djangoapps/discussion/templates/discussion/discussion_profile_page.html @@ -5,15 +5,18 @@ <%page expression_filter="h"/> <%inherit file="../main.html" /> <%namespace name='static' file='../static_content.html'/> +<%def name="online_help_token()"><% return "discussions" %> <%! -from django.utils.translation import ugettext as _ +import json +from django.utils.translation import ugettext as _, ungettext from django.template.defaultfilters import escapejs -from openedx.core.djangolib.js_utils import ( - dump_js_escaped_json, js_escaped_string -) +from django.core.urlresolvers import reverse + +from django_comment_client.permissions import has_permission +from openedx.core.djangolib.js_utils import dump_js_escaped_json, js_escaped_string %> -<%block name="bodyclass">discussion-user-profile +<%block name="bodyclass">discussion discussion-user-profile <%block name="pagetitle">${_("Discussion - {course_number}").format(course_number=course.display_number_with_default)} <%block name="headextra"> @@ -22,57 +25,70 @@ from openedx.core.djangolib.js_utils import ( <%block name="js_extra"> <%include file="_js_body_dependencies.html" /> - <%static:require_module module_name="discussion/js/discussion_profile_page_factory" class_name="DiscussionProfilePageFactory"> - <% profile_page_context = { - 'courseId': unicode(course.id), - 'courseName': course.display_name_with_default, - 'userInfo': user_info, - 'threads': threads, - 'page': page, - 'numPages': num_pages, + 'courseSettings': ${course_settings | n, dump_js_escaped_json}, + 'courseId': '${unicode(course.id) | n, js_escaped_string}', + 'courseName': '${course.display_name_with_default | n, js_escaped_string}', + 'contentInfo': ${annotated_content_info | n, dump_js_escaped_json}, + 'userInfo': ${user_info | n, dump_js_escaped_json}, + 'roles': ${roles | n, dump_js_escaped_json}, + 'threads': ${threads | n, dump_js_escaped_json}, + 'page': ${page | n, dump_js_escaped_json}, + 'sortPreference': '${sort_preference | n, js_escaped_string}', + 'numPages': ${num_pages | n, dump_js_escaped_json} } - %> + DiscussionProfilePageFactory(_.extend( { - $el: $('.discussion-user-threads') + el: $('.discussion-user-profile-board') }, - ${profile_page_context | n, dump_js_escaped_json} + profile_page_context )); <%include file="../courseware/course_navigation.html" args="active_page='discussion'" /> -
      +<%block name="content"> + + <%include file="_underscore_templates.html" /> +<%include file="_thread_list_template.html" /> diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index 20b14a2530..5ed2bce1c5 100644 --- a/lms/djangoapps/discussion/tests/test_views.py +++ b/lms/djangoapps/discussion/tests/test_views.py @@ -1081,9 +1081,7 @@ class InlineDiscussionTestCase(ForumsEnableMixin, ModuleStoreTestCase): """Verifies that the response contains the appropriate courseware_url and courseware_title""" self.assertEqual(response.status_code, 200) response_data = json.loads(response.content) - expected_courseware_url = '/courses/TestX/101/Test_Course/jump_to/i4x://TestX/101/discussion/Discussion1' expected_courseware_title = 'Chapter / Discussion1' - self.assertEqual(response_data['discussion_data'][0]['courseware_url'], expected_courseware_url) self.assertEqual(response_data["discussion_data"][0]["courseware_title"], expected_courseware_title) def test_courseware_data(self, mock_request): @@ -1155,8 +1153,8 @@ class UserProfileTestCase(ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase) html = response.content self.assertRegexpMatches(html, r'data-page="1"') self.assertRegexpMatches(html, r'data-num-pages="1"') - self.assertRegexpMatches(html, r'1 discussion started') - self.assertRegexpMatches(html, r'2 comments') + self.assertRegexpMatches(html, r'1 discussion started') + self.assertRegexpMatches(html, r'2 comments') self.assertRegexpMatches(html, r''id': '{}''.format(self.TEST_THREAD_ID)) self.assertRegexpMatches(html, r''title': '{}''.format(self.TEST_THREAD_TEXT)) self.assertRegexpMatches(html, r''body': '{}''.format(self.TEST_THREAD_TEXT)) @@ -1181,15 +1179,9 @@ class UserProfileTestCase(ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase) def test_html(self, mock_request): self.check_html(mock_request) - def test_html_p2(self, mock_request): - self.check_html(mock_request, page="2") - def test_ajax(self, mock_request): self.check_ajax(mock_request) - def test_ajax_p2(self, mock_request): - self.check_ajax(mock_request, page="2") - def test_404_non_enrolled_user(self, __): """ Test that when student try to visit un-enrolled students' discussion profile, diff --git a/lms/djangoapps/discussion/views.py b/lms/djangoapps/discussion/views.py index c32959169c..b70c72b113 100644 --- a/lms/djangoapps/discussion/views.py +++ b/lms/djangoapps/discussion/views.py @@ -408,8 +408,10 @@ def user_profile(request, course_key, user_id): nr_transaction = newrelic.agent.current_transaction() - #TODO: Allow sorting? + user = cc.User.from_django_user(request.user) + user_info = user.to_dict() course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True) + course_settings = make_course_settings(course, request.user) try: # If user is not enrolled in the course, do not proceed. @@ -442,6 +444,8 @@ def user_profile(request, course_key, user_id): is_staff = has_permission(request.user, 'openclose_thread', course.id) threads = [utils.prepare_content(thread, course_key, is_staff) for thread in threads] + with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): + add_courseware_context(threads, course, request.user) if request.is_ajax(): return utils.JsonResponse({ 'discussion_data': threads, @@ -454,6 +458,9 @@ def user_profile(request, course_key, user_id): course_id=course.id ).order_by("name").values_list("name", flat=True).distinct() + with newrelic.agent.FunctionTrace(nr_transaction, "get_cohort_info"): + user_cohort_id = get_cohort_id(request.user, course_key) + context = { 'course': course, 'user': request.user, @@ -462,9 +469,20 @@ def user_profile(request, course_key, user_id): 'profiled_user': profiled_user.to_dict(), 'threads': threads, 'user_info': user_info, + 'roles': utils.get_role_ids(course_key), + 'can_create_comment': has_permission(request.user, "create_comment", course.id), + 'can_create_subcomment': has_permission(request.user, "create_sub_comment", course.id), + 'can_create_thread': has_permission(request.user, "create_thread", course.id), + 'flag_moderator': bool( + has_permission(request.user, 'openclose_thread', course.id) or + has_access(request.user, 'staff', course) + ), + 'user_cohort': user_cohort_id, 'annotated_content_info': annotated_content_info, 'page': query_params['page'], 'num_pages': query_params['num_pages'], + 'sort_preference': user.default_sort_key, + 'course_settings': course_settings, 'learner_profile_page_url': reverse('learner_profile', kwargs={'username': django_user.username}), 'disable_courseware_js': True, 'uses_pattern_library': True, diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/team_profile_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/team_profile_spec.js index 2021c3d36e..83fa3768f7 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/team_profile_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/team_profile_spec.js @@ -101,7 +101,7 @@ define([ it('can render itself', function() { var requests = AjaxHelpers.requests(this), view = createTeamProfileView(requests, {}); - expect(view.$('.discussion-thread').length).toEqual(3); + expect(view.$('.forum-nav-thread').length).toEqual(3); }); it('shows New Post button when user joins a team', function() { diff --git a/lms/djangoapps/teams/static/teams/js/views/team_discussion.js b/lms/djangoapps/teams/static/teams/js/views/team_discussion.js index 2d4d336523..55f92caf7e 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_discussion.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_discussion.js @@ -3,21 +3,21 @@ */ (function(define) { 'use strict'; - define(['backbone', 'underscore', 'gettext', 'common/js/discussion/discussion_module_view'], - function(Backbone, _, gettext, DiscussionModuleView) { + define(['backbone', 'underscore', 'gettext', 'common/js/discussion/views/discussion_inline_view'], + function(Backbone, _, gettext, DiscussionInlineView) { var TeamDiscussionView = Backbone.View.extend({ - initialize: function() { + initialize: function(options) { window.$$course_id = this.$el.data('course-id'); + this.readOnly = options.readOnly; }, render: function() { - var discussionModuleView = new DiscussionModuleView({ + var discussionInlineView = new DiscussionInlineView({ el: this.$el, - readOnly: this.$el.data('read-only') === true, - context: 'standalone' + showByDefault: true, + readOnly: this.readOnly }); - discussionModuleView.render(); - discussionModuleView.loadPage(this.$el); + discussionInlineView.render(); return this; } }); diff --git a/lms/djangoapps/teams/static/teams/js/views/team_profile.js b/lms/djangoapps/teams/static/teams/js/views/team_profile.js index d65c2cf1f9..ac605791ac 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_profile.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_profile.js @@ -56,7 +56,7 @@ ); this.discussionView = new TeamDiscussionView({ el: this.$('.discussion-module'), - readOnly: this.$('.discussion-module').data('read-only') === true + readOnly: !isMember }); this.discussionView.render(); diff --git a/lms/djangoapps/teams/templates/teams/teams.html b/lms/djangoapps/teams/templates/teams/teams.html index fa58f64b76..9482ea8b8d 100644 --- a/lms/djangoapps/teams/templates/teams/teams.html +++ b/lms/djangoapps/teams/templates/teams/teams.html @@ -1,4 +1,7 @@ ## mako + +<%page expression_filter="h"/> + <%! import json %> <%! from django.utils.translation import ugettext as _ @@ -55,3 +58,4 @@ from openedx.core.djangolib.js_utils import ( <%include file="../discussion/_underscore_templates.html" /> +<%include file="../discussion/_thread_list_template.html" /> diff --git a/lms/static/lms/js/build.js b/lms/static/lms/js/build.js index 228773ff67..2a1b44b360 100644 --- a/lms/static/lms/js/build.js +++ b/lms/static/lms/js/build.js @@ -79,7 +79,7 @@ 'logger': 'empty:', 'utility': 'empty:', 'URI': 'empty:', - 'common/js/discussion/discussion_module_view': 'empty:', + 'common/js/discussion/views/discussion_inline_view': 'empty:', 'modernizr': 'empty', // Don't bundle UI Toolkit helpers as they are loaded into the "edx" namespace diff --git a/lms/static/lms/js/spec/main.js b/lms/static/lms/js/spec/main.js index b8b220c0e1..b012565709 100644 --- a/lms/static/lms/js/spec/main.js +++ b/lms/static/lms/js/spec/main.js @@ -642,7 +642,7 @@ ], exports: 'ThreadResponseView' }, - 'common/js/discussion/discussion_module_view': { + 'common/js/discussion/views/discussion_inline_view': { deps: [ 'jquery', 'underscore', @@ -666,7 +666,7 @@ 'common/js/discussion/views/thread_response_show_view', 'common/js/discussion/views/thread_response_view' ], - exports: 'DiscussionModuleView' + exports: 'DiscussionInlineView' }, 'common/js/spec_helpers/discussion_spec_helper': { deps: [ diff --git a/lms/static/sass/discussion/_build.scss b/lms/static/sass/discussion/_build.scss index a5ed2f4d2c..299d6f3c84 100644 --- a/lms/static/sass/discussion/_build.scss +++ b/lms/static/sass/discussion/_build.scss @@ -37,5 +37,6 @@ $static-path: '../..' !default; @import 'views/create-edit-post'; @import 'views/response'; @import 'views/search'; +@import 'views/inline'; @import 'utilities/developer'; @import 'utilities/shame'; diff --git a/lms/static/sass/discussion/_discussion-v1.scss b/lms/static/sass/discussion/_discussion-v1.scss index 9059f02718..956e777932 100644 --- a/lms/static/sass/discussion/_discussion-v1.scss +++ b/lms/static/sass/discussion/_discussion-v1.scss @@ -54,7 +54,6 @@ .discussion-module { .discussion { clear: both; - padding-top: ($baseline/2); } .btn { diff --git a/lms/static/sass/discussion/_discussion.scss b/lms/static/sass/discussion/_discussion.scss index a9b7486131..2835cd7514 100644 --- a/lms/static/sass/discussion/_discussion.scss +++ b/lms/static/sass/discussion/_discussion.scss @@ -2,7 +2,7 @@ // ==================== // NOTE: this file is deprecated, and we should not continue to add to this file. Use other partials as appropriate. -body.discussion { +.discussion-body { .edit-post-form { @include clearfix(); @@ -175,18 +175,17 @@ body.discussion { } } -.container .discussion-body { +.discussion-body { @include clearfix(); border: none; background: transparent; box-shadow: none; - line-height: 1.4; .bottom-post-status { padding: 30px; font-size: $forum-x-large-font-size; font-weight: 700; - color: $gray-l3; + color: $forum-color-copy-light; text-align: center; } @@ -196,18 +195,10 @@ body.discussion { a { word-wrap: break-word; } - - p + p { - margin-top: $baseline; - } - } - - .responses li header { - margin-bottom: $baseline; } blockquote { - background: $gray-l5; + background: $forum-color-background-light; border-radius: $forum-border-radius; padding: ($baseline/4) ($baseline/2); font-size: $forum-base-font-size; @@ -252,7 +243,6 @@ body.discussion { .discussion-reply-new { @include clearfix(); @include transition(opacity .2s linear 0s); - padding: 0 ($baseline/2); h4 { font-size: $forum-large-font-size; @@ -296,9 +286,6 @@ body.discussion { @extend .discussion-body; display: block; position: relative; - margin: $baseline 0; - padding: $baseline; - border: 1px solid $forum-color-border !important; border-radius: $forum-border-radius; header { @@ -319,18 +306,13 @@ body.discussion { .discussion-module-header { @include float(left); width: flex-grid(7); + margin-bottom: ($baseline * 0.75); } .add_post_btn_container { @include text-align(right); - position: relative; - top: -45px; - } - - .discussion { - &.inline-discussion { - padding-top: $baseline * 3; - } + width: flex-grid(12); + height: (2 * $baseline); } div.add-response.post-extended-content { @@ -362,7 +344,6 @@ section.discussion { } .new-post-article { - display: none; .inner-wrapper { max-width: 1180px; @@ -377,6 +358,7 @@ section.discussion { color: $gray-d3; font-weight: 700; } + } .edit-post-form { @@ -405,6 +387,9 @@ section.discussion { .xblock-student_view-discussion { @extend %ui-print-excluded; + // Overrides overspecific courseware CSS from: + // https://github.com/edx/edx-platform/blob/master/lms/static/sass/course/courseware/_courseware.scss#L499 + padding-top: 15px !important; } // ==================== @@ -441,6 +426,8 @@ section.discussion-pagination { .response-count { @include float(right); + color: $forum-color-response-count; + font-size: $forum-base-font-size; } .response-pagination { diff --git a/lms/static/sass/discussion/_layouts.scss b/lms/static/sass/discussion/_layouts.scss index 8253da41b0..9b55da8aac 100644 --- a/lms/static/sass/discussion/_layouts.scss +++ b/lms/static/sass/discussion/_layouts.scss @@ -1,31 +1,29 @@ // Layouts for discussion pages +@import '../course/base/extends'; -.user-profile { - background-color: $sidebar-color; +.discussion-user-profile-board { - .user-profile { - padding: $baseline; - min-height: 500px; + .discussion-profile-title { + margin-bottom: $baseline / 5; + font-size: $forum-x-large-font-size; } - .sidebar-username { - font-weight: 700; - font-size: $forum-large-font-size; + .discussion-profile-count { + margin-top: $baseline / 4; } - .sidebar-user-roles { - margin-top: $baseline/2; + .discussion-profile-info { + @include margin-right($baseline); + } + + .user-name { + @include margin-right($baseline); + font-size: $forum-x-large-font-size; + } + + .user-roles { + font-size: $forum-small-font-size; font-style: italic; - font-size: $forum-base-font-size; - } - - .sidebar-threads-count { - margin-top: $baseline/2; - } - - .sidebar-threads-count span, - .sidebar-comments-count span { - font-weight: 700; } } diff --git a/lms/static/sass/discussion/_mixins.scss b/lms/static/sass/discussion/_mixins.scss index cc8861bf59..f86c6c7366 100644 --- a/lms/static/sass/discussion/_mixins.scss +++ b/lms/static/sass/discussion/_mixins.scss @@ -43,16 +43,16 @@ @mixin discussion-wmd-preview-container { @include border-radius(0, 0, $forum-border-radius, $forum-border-radius); box-sizing: border-box; - border: 1px solid $gray-l1; + border: 1px solid $forum-color-border; border-top: none; width: 100%; - background: $gray-l4; + background: $forum-color-background-light; box-shadow: 0 1px 3px $shadow-l1 inset; } @mixin discussion-new-post-wmd-preview-container { @include discussion-wmd-preview-container; - border-color: $gray-d3; + border-color: $forum-color-border; box-shadow: 0 1px 3px $shadow-d1 inset; } @@ -67,7 +67,7 @@ @mixin discussion-wmd-preview { padding: ($baseline/2) $baseline; width: auto; - color: $gray-d3; + background-color: $forum-color-background-light; ol, ul { // Fix up the RTL-only _reset.scss, but only in specific places @include padding-left($baseline*2); diff --git a/lms/static/sass/discussion/elements/_editor.scss b/lms/static/sass/discussion/elements/_editor.scss index 51b6f005e7..29550a0a29 100644 --- a/lms/static/sass/discussion/elements/_editor.scss +++ b/lms/static/sass/discussion/elements/_editor.scss @@ -51,20 +51,6 @@ width: 100%; } - .wmd-input { - @include border-radius($forum-border-radius, $forum-border-radius, 0, 0); - width: 100%; - height: 150px; - font-style: normal; - font-size: $forum-base-font-size; - font-family: Monaco, 'Lucida Console', monospace; - line-height: 1.6em; - - &::-webkit-input-placeholder { - color: #888; - } - } - .wmd-button-row { @include transition(all .2s ease-out 0s); position: relative; diff --git a/lms/static/sass/discussion/elements/_labels.scss b/lms/static/sass/discussion/elements/_labels.scss index 8ea2f38922..8d0936549c 100644 --- a/lms/static/sass/discussion/elements/_labels.scss +++ b/lms/static/sass/discussion/elements/_labels.scss @@ -1,10 +1,10 @@ // discussion - elements - labels // ==================== -body.discussion, .discussion-module { +.discussion { .post-label { @include margin($baseline/4, $baseline/2, 0, 0); - @extend %t-weight4; + @extend %t-light; font-size: $forum-small-font-size; display: inline; white-space: nowrap; diff --git a/lms/static/sass/discussion/elements/_navigation.scss b/lms/static/sass/discussion/elements/_navigation.scss index 4c017df08e..f4974e5ed3 100644 --- a/lms/static/sass/discussion/elements/_navigation.scss +++ b/lms/static/sass/discussion/elements/_navigation.scss @@ -122,10 +122,10 @@ // ------------------- // Sort and filter bar // ------------------- + .forum-nav-refine-bar { @include clearfix(); @include border-radius($forum-border-radius, $forum-border-radius, 0, 0); - @include text-align(right); font-size: $forum-small-font-size; border-bottom: 1px solid $forum-color-border; background-color: $gray-l5; @@ -134,16 +134,18 @@ } .forum-nav-filter-main { + @include text-align(left); + @include float(left); box-sizing: border-box; display: inline-block; width: 50%; - @include text-align(left); } .forum-nav-filter-cohort, .forum-nav-sort { + @include text-align(right); + @include float(right); box-sizing: border-box; display: inline-block; - @include text-align(right); @media (min-width: $bp-screen-md) { width: 50%; @@ -173,14 +175,19 @@ // Thread list // ----------- .forum-nav-thread-list { - @include padding-left(0); + padding-left: 0 !important; // should *not* be RTLed, see below for explanation + min-height: 60px; // @TODO: Remove this when we have a real empty state for the discussion thread list margin: 0; - overflow-y: scroll; + overflow-y: auto; list-style: none; border-radius: 0 0 3px 3px; .forum-nav-thread-labels { margin: 5px 0 0; + // Overrides overspecific courseware CSS from: + // https://github.com/edx/edx-platform/blob/master/lms/static/sass/course/courseware/_courseware.scss#L470 + // note this should *not* be RTLed, as the rule it overrides is not RTLed + padding-left: 0 !important; } .thread-preview-body { @@ -190,6 +197,22 @@ overflow: hidden; white-space: nowrap; text-overflow: ellipsis; + + @include rtl { + // This is counterintuitive, but when showing a preview of the first part of RTL text, using direction: rtl + // will actually show the _last_ part of that text. + direction: ltr; + } + } +} + +// Overrides underspecific styles from courseware css +.course-wrapper .course-content .forum-nav-thread-list-wrapper .forum-nav-thread-list { + @include padding-left(0); + list-style: none; + + .forum-nav-thread { + margin: 0; } } @@ -223,33 +246,7 @@ } &.is-active { - color: $forum-color-background; background-color: $forum-color-reading-thread; - - .forum-nav-thread-labels > li { - border-color: $forum-color-background; - color: $forum-color-background; - } - - .forum-nav-thread-votes-count { - color: $forum-color-background; - } - - .forum-nav-thread-comments-count { - color: $base-font-color; - - &:after { - @include border-right-color($forum-color-border); - } - } - - span.icon { - color: $forum-color-background; - } - - .thread-preview-body { - color: $forum-color-background; - } } .forum-nav-thread-unread-comments-count { @@ -275,6 +272,7 @@ .forum-nav-thread-wrapper-0 { @extend %forum-nav-thread-wrapper; @include margin-right($baseline/5); + align-self: flex-start; .icon { font-size: $forum-base-font-size; @@ -293,13 +291,16 @@ .forum-nav-thread-wrapper-1 { @extend %forum-nav-thread-wrapper; margin: 0 ($baseline / 4); - width: 80%; + // 125 is the width we need to save for the "X new" comments indicator - and we want to clip the preview + // at the same length whether there are unread comments for this story or not. + max-width: calc(100% - 125px); flex-grow: 1; // This column should consume all the available space } .forum-nav-thread-wrapper-2 { @extend %forum-nav-thread-wrapper; @include text-align(right); + min-width: 90px; white-space: nowrap; } @@ -370,7 +371,9 @@ &:hover, &:focus { color: $forum-color-active-text; - background-color: $forum-color-active-thread; + // !important overrides the one set here: + // https://github.com/edx/edx-platform/blob/master/lms/static/sass/elements/_controls.scss#L472 + background-color: $forum-color-active-thread !important; } } diff --git a/lms/static/sass/discussion/inline-discussion-rtl.scss b/lms/static/sass/discussion/inline-discussion-rtl.scss index c8b887ab0b..55cf452543 100644 --- a/lms/static/sass/discussion/inline-discussion-rtl.scss +++ b/lms/static/sass/discussion/inline-discussion-rtl.scss @@ -9,3 +9,4 @@ // app - discussion @import 'build-discussion'; +@import 'views/inline'; diff --git a/lms/static/sass/discussion/inline-discussion.scss b/lms/static/sass/discussion/inline-discussion.scss index a8f048d9d7..38a048250a 100644 --- a/lms/static/sass/discussion/inline-discussion.scss +++ b/lms/static/sass/discussion/inline-discussion.scss @@ -9,3 +9,4 @@ // app - discussion @import 'build-discussion'; +@import 'views/inline'; diff --git a/lms/static/sass/discussion/utilities/_developer.scss b/lms/static/sass/discussion/utilities/_developer.scss index a45a4b0aa0..831c21a556 100644 --- a/lms/static/sass/discussion/utilities/_developer.scss +++ b/lms/static/sass/discussion/utilities/_developer.scss @@ -17,66 +17,64 @@ // provisional styling for "search alerts" (messages boxes that appear in the sidebar below the search // input field with notices pertaining to the search result). // -------------------- -body.discussion { - .forum-nav { +.forum-nav { - // wrapper for multiple alerts - .search-alerts { + // wrapper for multiple alerts + .search-alerts { + } + + // a single alert, which can be independently displayed / dismissed + .search-alert { + @include transition(none); + padding: ($baseline/2) 11px ($baseline/2) 18px; + background-color: $black; + } + + .search-alert-content, .search-alert-controls { + display: inline-block; + vertical-align: middle; + } + + // alert content + .search-alert-content { + width: 70%; + + // alert copy + .message { + font-size: $forum-small-font-size; + color: $white; + + em { + @extend %t-weight5; + font-style: italic; + } } - // a single alert, which can be independently displayed / dismissed - .search-alert { + // links to jump to users/content in alerts + .link-jump { @include transition(none); - padding: ($baseline/2) 11px ($baseline/2) 18px; - background-color: $black; + @extend %t-weight5; } + } - .search-alert-content, .search-alert-controls { - display: inline-block; - vertical-align: middle; - } + // alert controls + .search-alert-controls { + @include text-align(right); + width: 28%; - // alert content - .search-alert-content { - width: 70%; + .control { + @include transition(none); + @extend %t-weight5; + padding: ($baseline/4) ($baseline/2); + color: $white; + font-size: $forum-base-font-size; - // alert copy - .message { - font-size: $forum-small-font-size; + // reseting poorly globally scoped hover/focus state for this control + &:hover, &:focus { color: $white; - - em { - @extend %t-weight5; - font-style: italic; - } - } - - // links to jump to users/content in alerts - .link-jump { - @include transition(none); - @extend %t-weight5; - } - } - - // alert controls - .search-alert-controls { - @include text-align(right); - width: 28%; - - .control { - @include transition(none); - @extend %t-weight5; - padding: ($baseline/4) ($baseline/2); - color: $white; - font-size: $forum-base-font-size; - - // reseting poorly globally scoped hover/focus state for this control - &:hover, &:focus { - color: $white; - text-decoration: none; - } + text-decoration: none; } } } diff --git a/lms/static/sass/discussion/utilities/_shame.scss b/lms/static/sass/discussion/utilities/_shame.scss index d04a574fe7..253cef89b7 100644 --- a/lms/static/sass/discussion/utilities/_shame.scss +++ b/lms/static/sass/discussion/utilities/_shame.scss @@ -118,7 +118,34 @@ li[class*=forum-nav-thread-label-] { // ------- .discussion-module { - .wrapper-post-header .post-title { - margin-bottom: 0 !important; // overrides "#seq_content h1" styling + .post-header { + margin-bottom: 0 !important; // overrides default header styling + padding-bottom: 0 !important; // overrides default header styling + + .posted-details { + margin: ($baseline/5) 0 !important; // overrides courseware p styling + } + + .post-labels { + font-size: $forum-base-font-size; // overrides default heading styling + } + + .post-title { + margin-bottom: 0 !important; // overrides "#seq_content h1" styling + } + } +} + +// overrides courseware styling to keep views consistent everywhere +.discussion-article { + .response-header { + line-height: 1 !important; + font-size: $forum-base-font-size !important; + margin-bottom: 0 !important; + padding-bottom: 0 !important; + } + + p { + margin-bottom: 0 !important; } } diff --git a/lms/static/sass/discussion/utilities/_variables-v1.scss b/lms/static/sass/discussion/utilities/_variables-v1.scss index 0e57bb85a1..c513267663 100644 --- a/lms/static/sass/discussion/utilities/_variables-v1.scss +++ b/lms/static/sass/discussion/utilities/_variables-v1.scss @@ -1,26 +1,34 @@ // discussion - utilities - variables // ==================== -// color variables +// base color variables +$forum-color-primary: rgb(0, 117, 180) !default; +$forum-color-copy-light: rgb(65, 65, 65) !default; +$forum-color-background-light: rgb(245, 245, 245) !default; + +// contextual color variables $forum-color-background: $white; -$forum-color-active-thread: $blue !default; -$forum-color-hover: $action-primary-bg !default; +$forum-color-active-thread: $forum-color-primary !default; +$forum-color-hover: rgb(6, 86, 131) !default; $forum-color-active-text: $white !default; -$forum-color-pinned: $pink !default; -$forum-color-reported: $pink !default; +$forum-color-pinned: rgb(152, 44, 98) !default; +$forum-color-reported: rgb(152, 44, 98) !default; $forum-color-closed: $black !default; -$forum-color-following: $blue !default; -$forum-color-staff: $blue !default; -$forum-color-community-ta: $green-d1 !default; -$forum-color-marked-answer: $green-d1 !default; -$forum-color-border: $gray-l3 !default; -$forum-color-error: $red !default; -$forum-color-hover-thread: #f6f6f6 !default; -$forum-color-reading-thread: $gray-d3 !default; -$forum-color-read-post: $blue !default; -$forum-color-never-read-post: $gray-d3 !default; -$forum-color-editor-preview-label: $gray-d2 !default; -$forum-color-response-count: $gray-d2 !default; +$forum-color-following: $forum-color-primary !default; +$forum-color-staff: $forum-color-primary !default; +$forum-color-community-ta: rgb(0, 129, 0) !default; +$forum-color-marked-answer: rgb(0, 129, 0) !default; +$forum-color-border: rgb(217, 217, 217) !default; +$forum-color-error: rgb(203, 7, 18) !default; +$forum-color-hover-thread: $forum-color-background-light !default; +$forum-color-reading-thread: $forum-color-background-light !default; +$forum-color-read-post: $forum-color-copy-light !default; +$forum-color-never-read-post: $forum-color-primary !default; +$forum-color-editor-preview-label: $forum-color-copy-light !default; +$forum-color-response-count: $forum-color-copy-light !default; +$forum-color-navigation-bar: $forum-color-background-light !default; +$forum-color-count: $forum-color-copy-light !default; +$forum-color-background-label: rgb(65, 65, 65) !default; // post images $post-image-dimension: ($baseline*3) !default; // image size + margin @@ -37,5 +45,5 @@ $forum-small-font-size: 12px; $forum-border-radius: 3px; // btn colors -$uxpl-primary-blue: $blue !default; +$uxpl-primary-blue: rgb(0, 117, 180) !default; $btn-default-background-color: $white; diff --git a/lms/static/sass/discussion/utilities/_variables-v2.scss b/lms/static/sass/discussion/utilities/_variables-v2.scss index 6dfdb0b900..b33d1f211a 100644 --- a/lms/static/sass/discussion/utilities/_variables-v2.scss +++ b/lms/static/sass/discussion/utilities/_variables-v2.scss @@ -1,7 +1,12 @@ // discussion - utilities - variables // ==================== -// color variables +// base color variables +$forum-color-primary: palette(primary, base) !default; +$forum-color-copy-light: palette(grayscale, base) !default; +$forum-color-background-light: palette(grayscale, x-back) !default; + +// contextual color variables $forum-color-background: $lms-container-background-color !default; $forum-color-active-thread: $lms-active-color !default; $forum-color-hover: palette(primary, dark) !default; @@ -9,18 +14,21 @@ $forum-color-active-text: $lms-container-background-color !default; $forum-color-pinned: palette(secondary, dark) !default; $forum-color-reported: palette(secondary, dark) !default; $forum-color-closed: $black !default; -$forum-color-following: palette(primary, base) !default; -$forum-color-staff: palette(primary, base) !default; +$forum-color-following: $forum-color-primary !default; +$forum-color-staff: $forum-color-primary !default; $forum-color-community-ta: palette(success, text) !default; $forum-color-marked-answer: palette(success, text) !default; $forum-color-border: palette(grayscale, back) !default; $forum-color-error: palette(error, accent) !default; -$forum-color-hover-thread: palette(grayscale, x-back) !default; -$forum-color-reading-thread: palette(primary, base) !default; +$forum-color-hover-thread: $forum-color-background-light !default; +$forum-color-reading-thread: $forum-color-background-light !default; $forum-color-read-post: palette(grayscale, base) !default; -$forum-color-never-read-post: palette(primary, base) !default; +$forum-color-never-read-post: $forum-color-primary !default; $forum-color-editor-preview-label: palette(grayscale, base) !default; $forum-color-response-count: palette(grayscale, base) !default; +$forum-color-navigation-bar: $forum-color-background-light !default; +$forum-color-count: palette(grayscale, base) !default; +$forum-color-background-label: palette(grayscale, base) !default; // post images $post-image-dimension: ($baseline*3) !default; // image size + margin diff --git a/lms/static/sass/discussion/views/_inline.scss b/lms/static/sass/discussion/views/_inline.scss new file mode 100644 index 0000000000..f629b2cefa --- /dev/null +++ b/lms/static/sass/discussion/views/_inline.scss @@ -0,0 +1,67 @@ +// forums - inline discussion styling +// ==================== + +.discussion.inline-discussion { + .inline-threads { + border: 1px solid $forum-color-border; + border-radius: $forum-border-radius; + } + + .inline-thread { + border: 1px solid $forum-color-border; + border-radius: $forum-border-radius; + + .forum-nav-bar { + color: $forum-color-navigation-bar; + padding: ($baseline / 2) $baseline; + position: relative; + + .all-posts-btn { + color: $forum-color-primary; + + .icon { + @include margin-left(-15px); + } + } + } + + .forum-content { + padding: $baseline / 2; + overflow-y: auto; + } + } + + .wmd-preview-container { + @include discussion-new-post-wmd-preview-container; + margin-bottom: $baseline; + } + + .wmd-preview-label { + @include discussion-wmd-preview-label; + } + + .wmd-preview { + @include discussion-wmd-preview; + } + + .new-post-article { + position: relative; + border: 1px solid $forum-color-border; + + .add-post-cancel { + @include right($baseline / 2); + top: $baseline / 2; + position: absolute; + color: $uxpl-primary-blue; + + &:hover, + &:focus { + border-color: transparent; + box-shadow: none; + background-color: transparent; + background-image: none; + } + } + } + +} diff --git a/lms/static/sass/discussion/views/_response.scss b/lms/static/sass/discussion/views/_response.scss index ee465cd8c1..bf9c7dd587 100644 --- a/lms/static/sass/discussion/views/_response.scss +++ b/lms/static/sass/discussion/views/_response.scss @@ -42,8 +42,7 @@ } // +base - single response element -.container .discussion-response { - +.discussion-response { .response-header-content { // CASE: larger username for responses diff --git a/lms/static/sass/discussion/views/_thread.scss b/lms/static/sass/discussion/views/_thread.scss index 4ba192200c..c8843a7c03 100644 --- a/lms/static/sass/discussion/views/_thread.scss +++ b/lms/static/sass/discussion/views/_thread.scss @@ -13,37 +13,32 @@ .discussion-post { padding: 0 ($baseline/2); - .wrapper-post-header { - padding-bottom: 0; - } - - .post-header-content { - display: inline-block; - width: flex-grid(9,12); - } - .post-header-actions { @include float(right); } +} - .post-body { - width: flex-grid(10,12); +// post article +.discussion-article { + .posted-details { + @extend %t-copy-sub2; + @extend %t-light; + margin: ($baseline/5) 0; + color: $forum-color-copy-light; + + .username { + @extend %t-strong; + display: inline; + } + + .timeago, .top-post-status { + color: inherit; + } } } -.posted-details { - @extend %t-copy-sub2; - margin: ($baseline/5) 0; - color: $gray-d1; - - .username { - @extend %t-strong; - display: inline; - } - - .timeago, .top-post-status { - color: inherit; - } +.thread-responses-wrapper { + padding: 0 ($baseline/2); } // response layout @@ -62,6 +57,11 @@ position: absolute; top: $baseline; } + + // response body + .response-body { + @extend %t-copy-sub1; + } } // comments layout @@ -74,7 +74,7 @@ width: flex-grid(10,12); p + p { - margin-top: 12px; + margin-top: ($baseline/2); } } @@ -94,58 +94,56 @@ } // +thread - elements - shared styles -body.discussion { +.discussion-post, .discussion-response, .discussion-comment { + @include clearfix(); - .discussion-post, .discussion-response, .discussion-comment { - @include clearfix(); + // thread - images + .author-image { + @include margin-right($baseline/2); + display: inline-block; + vertical-align: top; - // thread - images - .author-image { - @include margin-right($baseline/2); - display: inline-block; - vertical-align: top; - - // STATE: No profile image - &:empty { - display: none; - } - - // CASE: post image - &.level-post { - height: $post-image-dimension; - width: $post-image-dimension; - } - - // CASE: response image - &.level-response { - height: $response-image-dimension; - width: $response-image-dimension; - } - - // CASE: comment image - &.level-comment { - height: $comment-image-dimension; - width: $comment-image-dimension; - } - - img { - border-radius: $forum-border-radius; - } + // STATE: No profile image + &:empty { + display: none; } - } - .discussion-response .response-body { - @include padding-right($baseline); //ensures content doesn't overlap on post or response actions. + // CASE: post image + &.level-post { + height: $post-image-dimension; + width: $post-image-dimension; + } + + // CASE: response image + &.level-response { + height: $response-image-dimension; + width: $response-image-dimension; + } + + // CASE: comment image + &.level-comment { + height: $comment-image-dimension; + width: $comment-image-dimension; + } + + img { + border-radius: $forum-border-radius; + } } } +.discussion-response .response-body { + @include padding(($baseline/2), $baseline, 0, 0); //ensures content doesn't overlap on post or response actions. + margin-bottom: 0.2em; + font-size: $forum-base-font-size; +} + // +post - individual element styling -// NOTE: discussion-article is used for inline discussion modules. -.discussion-post, -.discussion-article { +.discussion-post { @include clearfix(); .post-header-content { + max-width: calc(100% - 100px); // post title .post-title { @@ -157,24 +155,19 @@ body.discussion { // post body .post-body { @extend %t-copy-sub1; - // clear: both; //TO-DO: confirm that removing this is ok for all cases of discussion posts. + padding: ($baseline/2) 0; } // post context .post-context { @extend %t-copy-sub2; - margin-top: $baseline; - color: $gray-d1; + @extend %t-light; + color: $forum-color-copy-light; // CASE: no courseware context or cohort visibility rules &:empty { display: none; } - - // post visibility - cohorts - .group-visibility-label { - margin-top: ($baseline/4); - } } } @@ -188,10 +181,6 @@ body.discussion { margin-bottom: 0; } - .thread-main-wrapper, .thread-responses-wrapper { - padding: $baseline; - } - .discussion-article { @include transition(all .2s linear 0s); border: 1px solid $forum-color-border; @@ -239,11 +228,6 @@ body.discussion { font-size: $forum-large-font-size; } } - - .response-body { - margin-bottom: 0.2em; - font-size: $forum-base-font-size; - } } .discussion-reply-new { @@ -285,13 +269,6 @@ body.discussion { } } -// Custom styling for the list of user threads -.discussion-user-threads { - .discussion-post { - padding: $baseline/2; - } -} - .thread-wrapper, .forum-new-post-form { img { diff --git a/lms/templates/discussion/_discussion_inline.html b/lms/templates/discussion/_discussion_inline.html index 2d9aca2fab..cb6b9706d5 100644 --- a/lms/templates/discussion/_discussion_inline.html +++ b/lms/templates/discussion/_discussion_inline.html @@ -1,6 +1,7 @@ <%page expression_filter="h"/> <%include file="_underscore_templates.html" /> +<%include file="_thread_list_template.html" /> <%! from django.utils.translation import ugettext as _ @@ -16,18 +17,15 @@ from openedx.core.djangolib.js_utils import js_escaped_string

      ${_(display_name)}

      ${_("Topic:")} ${discussion_category} / ${discussion_target}
      - diff --git a/lms/templates/discussion/_js_body_dependencies.html b/lms/templates/discussion/_js_body_dependencies.html index d4be582f9b..d10b6918b6 100644 --- a/lms/templates/discussion/_js_body_dependencies.html +++ b/lms/templates/discussion/_js_body_dependencies.html @@ -17,7 +17,7 @@ from openedx.core.djangolib.js_utils import js_escaped_string discussion_classes = [ ['Discussion', 'common/js/discussion/discussion'], ['Content', 'common/js/discussion/content'], - ['DiscussionModuleView', 'common/js/discussion/discussion_module_view'], + ['DiscussionInlineView', 'common/js/discussion/views/discussion_inline_view'], ['DiscussionThreadView', 'common/js/discussion/views/discussion_thread_view'], ['DiscussionThreadListView', 'common/js/discussion/views/discussion_thread_list_view'], ['DiscussionThreadProfileView', 'common/js/discussion/views/discussion_thread_profile_view'], diff --git a/lms/templates/discussion/_thread_list_template.html b/lms/templates/discussion/_thread_list_template.html index 20a2d64170..3afffaaee3 100644 --- a/lms/templates/discussion/_thread_list_template.html +++ b/lms/templates/discussion/_thread_list_template.html @@ -1,7 +1,7 @@ <%page expression_filter="h"/> <%! from django.utils.translation import ugettext as _ %>