Skip to content

E
edx-platform
Commit 5dad2afa by Brian Jacobel
Options

Squashed minor issue fixes in inline discussion view 

Make this selector more specific, was getting overrides

Fix Teams implementation of inline discussion

Fix LMS tests

Work on pagination support

Jasmine tests for this feature

Focus works when navigating between list and detail

Fix safe template violations
parent a099d600
Showing with 268 additions and 46 deletions
common/static/common/js/discussion/views/discussion_inline_view.js
...@@ -19,11 +19,28 @@ ...@@ -19,11 +19,28 @@
} }
}, },
page_re: /\?discussion_page=(\d+)/,
initialize: function(options) { initialize: function(options) {
var match;
this.$el = options.el; this.$el = options.el;
this.showByDefault = options.showByDefault || false;
this.toggleDiscussionBtn = this.$('.discussion-show'); this.toggleDiscussionBtn = this.$('.discussion-show');
this.newPostForm = this.$el.find('.new-post-article'); this.listenTo(this.model, 'change', this.render);
this.listenTo(this.newPostView, 'newPost:cancel', this.hideNewPost);
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) { loadDiscussions: function($elem, error) {
...@@ -46,7 +63,7 @@ ...@@ -46,7 +63,7 @@
}, },
renderDiscussion: function($elem, response, textStatus, discussionId) { renderDiscussion: function($elem, response, textStatus, discussionId) {
var $discussion, var discussionHtml,
user = new DiscussionUser(response.user_info), user = new DiscussionUser(response.user_info),
self = this; self = this;
...@@ -59,21 +76,21 @@ ...@@ -59,21 +76,21 @@
this.course_settings = new DiscussionCourseSettings(response.course_settings); 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, { this.discussion.reset(response.discussion_data, {
silent: false silent: false
}); });
$discussion = _.template($('#inline-discussion-template').html())({ discussionHtml = edx.HtmlUtils.template($('#inline-discussion-template').html())({
threads: response.discussion_data, threads: response.discussion_data,
read_only: this.readOnly, read_only: this.readOnly,
discussionId: discussionId discussionId: discussionId
}); });
if (this.$('section.discussion').length) { if (this.$('section.discussion').length) {
this.$('section.discussion').replaceWith($discussion); edx.HtmlUtils.setHtml(this.$el, discussionHtml);
} else { } else {
this.$el.append($discussion); edx.HtmlUtils.append(this.$el, discussionHtml);
} }
this.threadListView = new DiscussionThreadListView({ this.threadListView = new DiscussionThreadListView({
...@@ -138,6 +155,9 @@ ...@@ -138,6 +155,9 @@
// Show the thread list view // Show the thread list view
this.threadListView.$el.removeClass('is-hidden'); this.threadListView.$el.removeClass('is-hidden');
// Set focus to thread list item that was saved as active
this.threadListView.$('.is-active').focus();
}, },
toggleDiscussion: function() { toggleDiscussion: function() {
...@@ -147,9 +167,10 @@ ...@@ -147,9 +167,10 @@
this.hideDiscussion(); this.hideDiscussion();
} else { } else {
this.toggleDiscussionBtn.addClass('shown'); 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) { if (this.retrieved) {
this.$('section.discussion').slideDown(); this.$('section.discussion').slideDown();
this.$('section.discussion').removeClass('is-hidden');
this.showed = true; this.showed = true;
} else { } else {
this.loadDiscussions(this.$el, function() { this.loadDiscussions(this.$el, function() {
...@@ -165,8 +186,9 @@ ...@@ -165,8 +186,9 @@
hideDiscussion: function() { hideDiscussion: function() {
this.$('section.discussion').slideUp(); this.$('section.discussion').slideUp();
this.$('section.discussion').addClass('is-hidden');
this.toggleDiscussionBtn.removeClass('shown'); 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; this.showed = false;
}, },
...@@ -182,13 +204,15 @@ ...@@ -182,13 +204,15 @@
} else { } else {
this.newPostForm.show().focus(); this.newPostForm.show().focus();
} }
this.newPostView.$el.removeClass('is-hidden');
this.toggleDiscussionBtn.addClass('shown'); 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.$('section.discussion').slideDown();
this.showed = true; this.showed = true;
}, },
hideNewPost: function() { hideNewPost: function() {
this.newPostView.$el.addClass('is-hidden');
return this.newPostForm.slideUp(300); return this.newPostForm.slideUp(300);
} }
}); });
......
common/static/common/js/spec/discussion/view/discussion_inline_view_spec.js 0 → 100644
/* 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(
'<div class="discussion-module" data-discussion-id="test-discussion-id"' +
' data-user-create-comment="true"' +
' data-user-create-subcomment="true"' +
' data-read-only="false">' +
' <div class="discussion-module-header">' +
' <h3 class="discussion-module-title">Test Discussion</h3>' +
' <div class="inline-discussion-topic">' +
' <span class="inline-discussion-topic-title">Topic:</span> Category / Target ' +
' </div>' +
' </div>' +
' <button class="discussion-show btn btn-brand" data-discussion-id="test-discussion-id">' +
' <span class="button-text">Show Discussion</span>' +
' </button>' +
'</div>'
);
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);
});
});
});
}());
common/static/common/js/spec_helpers/discussion_spec_helper.js
/* global Content, Discussion, DiscussionCourseSettings, DiscussionUtil, DiscussionUser */ /* global _, Content, Discussion, DiscussionCourseSettings, DiscussionUtil, DiscussionUser */
(function() { (function() {
'use strict'; 'use strict';
this.DiscussionSpecHelper = (function() { this.DiscussionSpecHelper = (function() {
...@@ -51,8 +51,9 @@ ...@@ -51,8 +51,9 @@
return jasmine.createSpyObj('event', ['preventDefault', 'target']); return jasmine.createSpyObj('event', ['preventDefault', 'target']);
}; };
DiscussionSpecHelper.createTestCourseSettings = function() { DiscussionSpecHelper.createTestCourseSettings = function(options) {
return new DiscussionCourseSettings({ var context = _.extend(
{
category_map: { category_map: {
children: [['Test Topic', 'entry'], ['Other Topic', 'entry']], children: [['Test Topic', 'entry'], ['Other Topic', 'entry']],
entries: { entries: {
...@@ -66,8 +67,13 @@ ...@@ -66,8 +67,13 @@
} }
} }
}, },
is_cohorted: true is_cohorted: true,
}); allow_anonymous: false,
allow_anonymous_to_peers: false
},
options || {}
);
return new DiscussionCourseSettings(context);
}; };
DiscussionSpecHelper.createTestDiscussion = function(options) { DiscussionSpecHelper.createTestDiscussion = function(options) {
......
common/static/common/templates/discussion/inline-discussion.underscore
<section class="discussion inline-discussion" data-discussion-id="<%= discussionId %>"> <section class="discussion inline-discussion" data-discussion-id="<%- discussionId %>">
<div class="add_post_btn_container"> <div class="add_post_btn_container">
<button class="btn-link new-post-btn <%if (read_only) {%>is-hidden<%} %>"><%- gettext("Add a Post") %></button> <button class="btn-link new-post-btn <%if (read_only) {%>is-hidden<%} %>"><%- gettext("Add a Post") %></button>
</div> </div>
<article class="new-post-article"></article> <article class="new-post-article is-hidden"></article>
<section class="inline-threads"> <section class="inline-threads">
</section> </section>
...@@ -19,3 +19,4 @@ ...@@ -19,3 +19,4 @@
</div> </div>
</div> </div>
</section> </section>
common/test/acceptance/pages/lms/discussion.py
...@@ -504,7 +504,7 @@ class InlineDiscussionPage(PageObject): ...@@ -504,7 +504,7 @@ class InlineDiscussionPage(PageObject):
).fulfill() ).fulfill()
def get_num_displayed_threads(self): 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): def has_thread(self, thread_id):
"""Returns true if this page is showing the thread with the specified id.""" """Returns true if this page is showing the thread with the specified id."""
......
common/test/acceptance/tests/discussion/test_discussion.py
...@@ -1067,6 +1067,20 @@ class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMix ...@@ -1067,6 +1067,20 @@ class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMix
# Check if 'thread-wrapper' is focused after expanding thread # Check if 'thread-wrapper' is focused after expanding thread
self.assertFalse(thread_page.check_if_selector_is_focused(selector='.thread-wrapper')) 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): def test_add_a_post_is_present_if_can_create_thread_when_expanded(self):
self.discussion_page.expand_discussion() self.discussion_page.expand_discussion()
# Add a Post link is present # Add a Post link is present
......
lms/djangoapps/discussion/templates/discussion/discussion_board.html
...@@ -82,7 +82,7 @@ DiscussionBoardFactory({ ...@@ -82,7 +82,7 @@ DiscussionBoardFactory({
</aside> </aside>
<main id="main" aria-label="Content" tabindex="-1" class="discussion-column layout-col layout-col-b"> <main id="main" aria-label="Content" tabindex="-1" class="discussion-column layout-col layout-col-b">
<article class="new-post-article" style="display: none" tabindex="-1" aria-label="${_("New topic form")}"></article> <article class="new-post-article is-hidden" style="display: none" tabindex="-1" aria-label="${_("New topic form")}"></article>
<div class="forum-content"></div> <div class="forum-content"></div>
</main> </main>
</div> </div>
......
lms/djangoapps/teams/static/teams/js/spec/views/team_profile_spec.js
...@@ -101,7 +101,7 @@ define([ ...@@ -101,7 +101,7 @@ define([
it('can render itself', function() { it('can render itself', function() {
var requests = AjaxHelpers.requests(this), var requests = AjaxHelpers.requests(this),
view = createTeamProfileView(requests, {}); 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() { it('shows New Post button when user joins a team', function() {
......
lms/djangoapps/teams/static/teams/js/views/team_discussion.js
...@@ -12,7 +12,8 @@ ...@@ -12,7 +12,8 @@
render: function() { render: function() {
var discussionInlineView = new DiscussionInlineView({ var discussionInlineView = new DiscussionInlineView({
el: this.$el el: this.$el,
showByDefault: true
}); });
discussionInlineView.render(); discussionInlineView.render();
return this; return this;
......
lms/djangoapps/teams/templates/teams/teams.html
## mako ## mako
<%page expression_filter="h"/>
<%! import json %> <%! import json %>
<%! <%!
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
...@@ -55,3 +58,4 @@ from openedx.core.djangolib.js_utils import ( ...@@ -55,3 +58,4 @@ from openedx.core.djangolib.js_utils import (
</%block> </%block>
<%include file="../discussion/_underscore_templates.html" /> <%include file="../discussion/_underscore_templates.html" />
<%include file="../discussion/_thread_list_template.html" />
lms/static/sass/discussion/_discussion.scss
...@@ -323,8 +323,8 @@ body.discussion { ...@@ -323,8 +323,8 @@ body.discussion {
.add_post_btn_container { .add_post_btn_container {
@include text-align(right); @include text-align(right);
position: relative; width: flex-grid(12);
top: -25px; height: (2 * $baseline);
} }
div.add-response.post-extended-content { div.add-response.post-extended-content {
...@@ -356,8 +356,6 @@ section.discussion { ...@@ -356,8 +356,6 @@ section.discussion {
} }
.new-post-article { .new-post-article {
display: none;
.inner-wrapper { .inner-wrapper {
max-width: 1180px; max-width: 1180px;
min-width: 760px; min-width: 760px;
......
lms/static/sass/discussion/elements/_navigation.scss
...@@ -194,10 +194,14 @@ ...@@ -194,10 +194,14 @@
} }
} }
// Overrides underspecific style from courseware css: // Overrides underspecific styles 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-wrapper .forum-nav-thread-list {
.course-wrapper .course-content .forum-nav-thread-list li { @include padding-left(0);
list-style: none;
li {
margin: 0; margin: 0;
}
} }
.forum-nav-thread { .forum-nav-thread {
...@@ -229,7 +233,25 @@ ...@@ -229,7 +233,25 @@
background-color: $forum-color-hover-thread; background-color: $forum-color-hover-thread;
} }
&.is-active { .forum-nav-thread-unread-comments-count {
display: inline-block;
font-size: $forum-small-font-size;
white-space: nowrap;
}
}
.discussion:not(.inline-discussion)
&.never-read {
.forum-nav-thread-link {
@include border-left(3px solid $forum-color-never-read-post);
color: $forum-color-never-read-post;
}
}
}
.discussion:not(.inline-discussion) .forum-nav-thread {
.forum-nav-thread-link.is-active {
color: $forum-color-background; color: $forum-color-background;
background-color: $forum-color-reading-thread; background-color: $forum-color-reading-thread;
...@@ -258,20 +280,6 @@ ...@@ -258,20 +280,6 @@
color: $forum-color-background; color: $forum-color-background;
} }
} }
.forum-nav-thread-unread-comments-count {
display: inline-block;
font-size: $forum-small-font-size;
white-space: nowrap;
}
}
&.never-read {
.forum-nav-thread-link {
@include border-left(3px solid $forum-color-never-read-post);
color: $forum-color-never-read-post;
}
}
} }
%forum-nav-thread-wrapper { %forum-nav-thread-wrapper {
......
lms/static/sass/discussion/views/_inline.scss
// forums - inline discussion styling // forums - inline discussion styling
// ==================== // ====================
.inline-discussion { .discussion.inline-discussion {
padding-top: $baseline * 1.5; padding-top: $baseline;
.inline-threads { .inline-threads {
border: 1px solid $forum-color-border; border: 1px solid $forum-color-border;
...@@ -23,8 +23,20 @@ ...@@ -23,8 +23,20 @@
.forum-content { .forum-content {
padding: $baseline / 2; padding: $baseline / 2;
max-height: 500px;
overflow-y: auto; 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;
}
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment