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 5332d75868..10e4ead4cd 100644 --- a/common/static/common/js/discussion/views/discussion_inline_view.js +++ b/common/static/common/js/discussion/views/discussion_inline_view.js @@ -19,11 +19,28 @@ } }, + page_re: /\?discussion_page=(\d+)/, + initialize: function(options) { + var match; + this.$el = options.el; + this.showByDefault = options.showByDefault || false; this.toggleDiscussionBtn = this.$('.discussion-show'); - this.newPostForm = this.$el.find('.new-post-article'); - this.listenTo(this.newPostView, 'newPost:cancel', this.hideNewPost); + this.listenTo(this.model, 'change', this.render); + + 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) { @@ -46,7 +63,7 @@ }, renderDiscussion: function($elem, response, textStatus, discussionId) { - var $discussion, + var discussionHtml, user = new DiscussionUser(response.user_info), self = this; @@ -59,21 +76,21 @@ this.course_settings = new DiscussionCourseSettings(response.course_settings); - this.discussion = new Discussion(); + this.discussion = new Discussion(undefined, {pages: response.num_pages}); this.discussion.reset(response.discussion_data, { silent: false }); - $discussion = _.template($('#inline-discussion-template').html())({ + discussionHtml = edx.HtmlUtils.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); + edx.HtmlUtils.setHtml(this.$el, discussionHtml); } else { - this.$el.append($discussion); + edx.HtmlUtils.append(this.$el, discussionHtml); } this.threadListView = new DiscussionThreadListView({ @@ -138,6 +155,9 @@ // 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() { @@ -147,9 +167,10 @@ this.hideDiscussion(); } else { this.toggleDiscussionBtn.addClass('shown'); - this.toggleDiscussionBtn.find('.button-text').html(gettext('Hide Discussion')); + this.toggleDiscussionBtn.find('.button-text').text(gettext('Hide Discussion')); if (this.retrieved) { this.$('section.discussion').slideDown(); + this.$('section.discussion').removeClass('is-hidden'); this.showed = true; } else { this.loadDiscussions(this.$el, function() { @@ -165,8 +186,9 @@ hideDiscussion: function() { this.$('section.discussion').slideUp(); + this.$('section.discussion').addClass('is-hidden'); this.toggleDiscussionBtn.removeClass('shown'); - this.toggleDiscussionBtn.find('.button-text').html(gettext('Show Discussion')); + this.toggleDiscussionBtn.find('.button-text').text(gettext('Show Discussion')); this.showed = false; }, @@ -182,13 +204,15 @@ } else { this.newPostForm.show().focus(); } + this.newPostView.$el.removeClass('is-hidden'); this.toggleDiscussionBtn.addClass('shown'); - this.toggleDiscussionBtn.find('.button-text').html(gettext('Hide Discussion')); + this.toggleDiscussionBtn.find('.button-text').text(gettext('Hide Discussion')); this.$('section.discussion').slideDown(); this.showed = true; }, hideNewPost: function() { + this.newPostView.$el.addClass('is-hidden'); return this.newPostForm.slideUp(300); } }); 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..2f31058183 --- /dev/null +++ b/common/static/common/js/spec/discussion/view/discussion_inline_view_spec.js @@ -0,0 +1,154 @@ +/* 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 + }); + 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'); + }); + }); + + 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_helpers/discussion_spec_helper.js b/common/static/common/js/spec_helpers/discussion_spec_helper.js index 8e96ea5382..7a05b8d8ff 100644 --- a/common/static/common/js/spec_helpers/discussion_spec_helper.js +++ b/common/static/common/js/spec_helpers/discussion_spec_helper.js @@ -1,4 +1,4 @@ -/* global Content, Discussion, DiscussionCourseSettings, DiscussionUtil, DiscussionUser */ +/* global _, Content, Discussion, DiscussionCourseSettings, DiscussionUtil, DiscussionUser */ (function() { 'use strict'; this.DiscussionSpecHelper = (function() { @@ -51,23 +51,29 @@ return jasmine.createSpyObj('event', ['preventDefault', 'target']); }; - DiscussionSpecHelper.createTestCourseSettings = function() { - return new DiscussionCourseSettings({ - category_map: { - children: [['Test Topic', 'entry'], ['Other Topic', 'entry']], - entries: { - 'Test Topic': { - is_cohorted: true, - id: 'test_topic' - }, - 'Other Topic': { - is_cohorted: true, - id: 'other_topic' + DiscussionSpecHelper.createTestCourseSettings = function(options) { + var context = _.extend( + { + category_map: { + children: [['Test Topic', 'entry'], ['Other Topic', 'entry']], + entries: { + 'Test Topic': { + is_cohorted: true, + id: 'test_topic' + }, + 'Other Topic': { + is_cohorted: true, + id: 'other_topic' + } } - } + }, + is_cohorted: true, + allow_anonymous: false, + allow_anonymous_to_peers: false }, - is_cohorted: true - }); + options || {} + ); + return new DiscussionCourseSettings(context); }; DiscussionSpecHelper.createTestDiscussion = function(options) { diff --git a/common/static/common/templates/discussion/inline-discussion.underscore b/common/static/common/templates/discussion/inline-discussion.underscore index 97162b6451..b2081125f4 100644 --- a/common/static/common/templates/discussion/inline-discussion.underscore +++ b/common/static/common/templates/discussion/inline-discussion.underscore @@ -1,9 +1,9 @@ -
+
-
+
@@ -19,3 +19,4 @@
+ diff --git a/common/test/acceptance/pages/lms/discussion.py b/common/test/acceptance/pages/lms/discussion.py index 7ba2af8cf7..13223e6124 100644 --- a/common/test/acceptance/pages/lms/discussion.py +++ b/common/test/acceptance/pages/lms/discussion.py @@ -504,7 +504,7 @@ class InlineDiscussionPage(PageObject): ).fulfill() def get_num_displayed_threads(self): - return len(self._find_within(".discussion-thread")) + return len(self._find_within(".forum-nav-thread")) def has_thread(self, thread_id): """Returns true if this page is showing the thread with the specified id.""" diff --git a/common/test/acceptance/tests/discussion/test_discussion.py b/common/test/acceptance/tests/discussion/test_discussion.py index 80a0b0b188..9cba009e4c 100644 --- a/common/test/acceptance/tests/discussion/test_discussion.py +++ b/common/test/acceptance/tests/discussion/test_discussion.py @@ -1067,6 +1067,20 @@ class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMix # Check if 'thread-wrapper' is focused after expanding thread self.assertFalse(thread_page.check_if_selector_is_focused(selector='.thread-wrapper')) + @attr('a11y') + def test_inline_a11y(self): + """ + Tests Inline Discussion for accessibility issues. + """ + self.setup_multiple_inline_threads(thread_count=3) + self.discussion_page.expand_discussion() + self.discussion_page.a11y_audit.config.set_rules({ + 'ignore': [ + 'section' + ] + }) + 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 diff --git a/lms/djangoapps/discussion/templates/discussion/discussion_board.html b/lms/djangoapps/discussion/templates/discussion/discussion_board.html index 7882739cf1..a6cc5cfe1e 100644 --- a/lms/djangoapps/discussion/templates/discussion/discussion_board.html +++ b/lms/djangoapps/discussion/templates/discussion/discussion_board.html @@ -82,7 +82,7 @@ DiscussionBoardFactory({
- +
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 dd3dac878b..9661127ac7 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_discussion.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_discussion.js @@ -12,7 +12,8 @@ render: function() { var discussionInlineView = new DiscussionInlineView({ - el: this.$el + el: this.$el, + showByDefault: true }); discussionInlineView.render(); return this; 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/sass/discussion/_discussion.scss b/lms/static/sass/discussion/_discussion.scss index 13a120096b..f98180d172 100644 --- a/lms/static/sass/discussion/_discussion.scss +++ b/lms/static/sass/discussion/_discussion.scss @@ -323,8 +323,8 @@ body.discussion { .add_post_btn_container { @include text-align(right); - position: relative; - top: -25px; + width: flex-grid(12); + height: (2 * $baseline); } div.add-response.post-extended-content { @@ -356,8 +356,6 @@ section.discussion { } .new-post-article { - display: none; - .inner-wrapper { max-width: 1180px; min-width: 760px; diff --git a/lms/static/sass/discussion/elements/_navigation.scss b/lms/static/sass/discussion/elements/_navigation.scss index 2fcf7331e4..75e05c7e6a 100644 --- a/lms/static/sass/discussion/elements/_navigation.scss +++ b/lms/static/sass/discussion/elements/_navigation.scss @@ -194,10 +194,14 @@ } } -// Overrides underspecific style from courseware css: -// https://github.com/edx/edx-platform/blob/master/lms/static/sass/course/courseware/_courseware.scss#L402 -.course-wrapper .course-content .forum-nav-thread-list li { - margin: 0; +// 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; + + li { + margin: 0; + } } .forum-nav-thread { @@ -229,36 +233,6 @@ background-color: $forum-color-hover-thread; } - &.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 { display: inline-block; font-size: $forum-small-font-size; @@ -266,6 +240,8 @@ } } + .discussion:not(.inline-discussion) + &.never-read { .forum-nav-thread-link { @include border-left(3px solid $forum-color-never-read-post); @@ -274,6 +250,38 @@ } } +.discussion:not(.inline-discussion) .forum-nav-thread { + .forum-nav-thread-link.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-wrapper { display: inline-block; vertical-align: middle; diff --git a/lms/static/sass/discussion/views/_inline.scss b/lms/static/sass/discussion/views/_inline.scss index 7218d7e8f5..8c8557a73e 100644 --- a/lms/static/sass/discussion/views/_inline.scss +++ b/lms/static/sass/discussion/views/_inline.scss @@ -1,8 +1,8 @@ // forums - inline discussion styling // ==================== -.inline-discussion { - padding-top: $baseline * 1.5; +.discussion.inline-discussion { + padding-top: $baseline; .inline-threads { border: 1px solid $forum-color-border; @@ -23,8 +23,20 @@ .forum-content { padding: $baseline / 2; - max-height: 500px; 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; + } }