From 187783beaf7f08eb6eb505b6c588e5d43269770d Mon Sep 17 00:00:00 2001 From: Brian Jacobel Date: Tue, 29 Nov 2016 14:39:45 -0500 Subject: [PATCH] Requested UI changes - borderless, all posts is no longer highlighted Fix read only support for team discussions Fix error messages to use "posts" not "threads" More small UI tweaks, padding and alignment Add a11y tests for all inline discussion states Improve discussion error messages --- common/static/common/js/discussion/utils.js | 10 +- .../views/discussion_content_view.js | 114 +++++++++--------- .../views/discussion_inline_view.js | 6 +- .../views/discussion_thread_list_view.js | 7 +- .../views/discussion_thread_view.js | 12 +- .../discussion/views/response_comment_view.js | 4 +- .../common/js/spec/discussion/utils_spec.js | 2 +- .../view/discussion_thread_list_view_spec.js | 2 +- .../acceptance/tests/discussion/helpers.py | 1 + .../tests/discussion/test_discussion.py | 36 +++--- .../static/teams/js/views/team_discussion.js | 6 +- .../static/teams/js/views/team_profile.js | 2 +- .../sass/discussion/_discussion-v1.scss | 1 - lms/static/sass/discussion/_discussion.scss | 7 +- .../sass/discussion/elements/_navigation.scss | 17 ++- .../discussion/utilities/_variables-v1.scss | 2 +- .../discussion/utilities/_variables-v2.scss | 2 +- lms/static/sass/discussion/views/_inline.scss | 16 ++- lms/static/sass/discussion/views/_thread.scss | 5 + 19 files changed, 138 insertions(+), 114 deletions(-) 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 index 8183e46a96..5e43308c2a 100644 --- a/common/static/common/js/discussion/views/discussion_inline_view.js +++ b/common/static/common/js/discussion/views/discussion_inline_view.js @@ -26,6 +26,7 @@ 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); @@ -144,6 +145,7 @@ 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'); }, @@ -179,8 +181,8 @@ this.loadDiscussions(this.$el, function() { self.hideDiscussion(); DiscussionUtil.discussionAlert( - gettext('Sorry'), - gettext('We had some trouble loading the discussion. Please try again.') + gettext('Error'), + gettext('This discussion could not be loaded. Refresh the page to try again.') ); }); } 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 dbc44250d3..243bc57b74 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 @@ -309,7 +309,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 to try again.') ); }; return this.collection.retrieveAnotherPage(this.mode, options, { @@ -478,7 +479,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 +516,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..b66c1d70a5 100644 --- a/common/static/common/js/discussion/views/discussion_thread_view.js +++ b/common/static/common/js/discussion/views/discussion_thread_view.js @@ -261,18 +261,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 to 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 to try again.') ); } } 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..f6660b1ad3 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 to 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_thread_list_view_spec.js b/common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js index a6ec3cb8fd..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 @@ -550,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() { diff --git a/common/test/acceptance/tests/discussion/helpers.py b/common/test/acceptance/tests/discussion/helpers.py index 84f5ff0d7a..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 diff --git a/common/test/acceptance/tests/discussion/test_discussion.py b/common/test/acceptance/tests/discussion/test_discussion.py index 4e98377082..9d0199df2e 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({ @@ -1039,7 +1027,9 @@ class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMix """ Tests Inline Discussion for accessibility issues. """ - self.setup_multiple_inline_threads(thread_count=3) + self.setup_multiple_threads(thread_count=3) + + # First test the a11y of the expanded list of threads self.discussion_page.expand_discussion() self.discussion_page.a11y_audit.config.set_rules({ 'ignore': [ @@ -1048,6 +1038,14 @@ class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMix }) self.discussion_page.a11y_audit.check_for_accessibility_errors() + # 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() # Add a Post link is present @@ -1363,7 +1361,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): @@ -1385,7 +1383,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): @@ -1397,7 +1395,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/lms/djangoapps/teams/static/teams/js/views/team_discussion.js b/lms/djangoapps/teams/static/teams/js/views/team_discussion.js index 9661127ac7..55f92caf7e 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_discussion.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_discussion.js @@ -6,14 +6,16 @@ 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 discussionInlineView = new DiscussionInlineView({ el: this.$el, - showByDefault: true + showByDefault: true, + readOnly: this.readOnly }); 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/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 69f6494e1d..36b9922966 100644 --- a/lms/static/sass/discussion/_discussion.scss +++ b/lms/static/sass/discussion/_discussion.scss @@ -296,9 +296,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,6 +316,7 @@ body.discussion { .discussion-module-header { @include float(left); width: flex-grid(7); + margin-bottom: ($baseline * 0.75); } .add_post_btn_container { @@ -399,6 +397,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; } // ==================== diff --git a/lms/static/sass/discussion/elements/_navigation.scss b/lms/static/sass/discussion/elements/_navigation.scss index b3e509062c..30c263dc17 100644 --- a/lms/static/sass/discussion/elements/_navigation.scss +++ b/lms/static/sass/discussion/elements/_navigation.scss @@ -126,7 +126,6 @@ .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; @@ -135,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%; @@ -174,7 +175,7 @@ // Thread list // ----------- .forum-nav-thread-list { - @include padding-left(0); + padding-left: 0 !important; // should *not* be RTLed, see below for explanation margin: 0; overflow-y: scroll; list-style: none; @@ -182,6 +183,10 @@ .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 { @@ -216,6 +221,9 @@ .forum-nav-thread-link { @include border-left(3px solid transparent); + @include rtl { + flex-direction: row-reverse; + } display: flex; padding: $baseline / 2; transition: none; @@ -290,6 +298,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; diff --git a/lms/static/sass/discussion/utilities/_variables-v1.scss b/lms/static/sass/discussion/utilities/_variables-v1.scss index 13b89db4fc..f1e100a744 100644 --- a/lms/static/sass/discussion/utilities/_variables-v1.scss +++ b/lms/static/sass/discussion/utilities/_variables-v1.scss @@ -24,7 +24,7 @@ $forum-color-read-post: $forum-color-primary !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-navigation-bar: $forum-color-primary !default; +$forum-color-navigation-bar: #f6f6f6 !default; // post images $post-image-dimension: ($baseline*3) !default; // image size + margin diff --git a/lms/static/sass/discussion/utilities/_variables-v2.scss b/lms/static/sass/discussion/utilities/_variables-v2.scss index 07420b80f0..f6987334d1 100644 --- a/lms/static/sass/discussion/utilities/_variables-v2.scss +++ b/lms/static/sass/discussion/utilities/_variables-v2.scss @@ -24,7 +24,7 @@ $forum-color-read-post: palette(grayscale, 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-primary !default; +$forum-color-navigation-bar: palette(grayscale, x-back) !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 index f6c0dd17b5..3f86adebc3 100644 --- a/lms/static/sass/discussion/views/_inline.scss +++ b/lms/static/sass/discussion/views/_inline.scss @@ -2,8 +2,6 @@ // ==================== .discussion.inline-discussion { - padding-top: $baseline; - .inline-threads { border: 1px solid $forum-color-border; border-radius: $forum-border-radius; @@ -14,10 +12,18 @@ border-radius: $forum-border-radius; .forum-nav-bar { - background-color: $forum-color-navigation-bar; + color: $forum-color-navigation-bar; + padding: ($baseline / 2) $baseline; + position: relative; - .btn-link { - color: $forum-color-active-text; + .all-posts-btn { + color: $forum-color-primary; + + .icon { + position: absolute; + @include left(7px); + top: 17px; + } } } diff --git a/lms/static/sass/discussion/views/_thread.scss b/lms/static/sass/discussion/views/_thread.scss index 4ba192200c..7320629205 100644 --- a/lms/static/sass/discussion/views/_thread.scss +++ b/lms/static/sass/discussion/views/_thread.scss @@ -27,8 +27,13 @@ } .post-body { + @include float(left); width: flex-grid(10,12); } + + .post-context { + @include float(left); + } } .posted-details {