Skip to content

E
edx-platform
Commit 89ba90a6 by Greg Price
Options

Re-design forum thread list 

Use larger and lighter title font, decrease padding, and use lighter
solid backgrounds instead of gradients. Also, refactor SASS code to
conform better to our new style, including using more specific class
names.

Originally reviewed in #4072
parent 1a0d8827
Showing with 293 additions and 301 deletions
common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee
......@@ -4,34 +4,41 @@ describe "DiscussionThreadListView", ->
DiscussionSpecHelper.setUpGlobals()
setFixtures """
<script type="text/template" id="thread-list-item-template">
<a href="<%- id %>" data-id="<%- id %>">
<span class="title"><%- title %></span>
<span class="comments-count">
<%
var fmt;
var data = {
'span_sr_open': '<span class="sr">',
'span_close': '</span>',
'unread_comments_count': unread_comments_count,
'comments_count': comments_count
};
if (unread_comments_count > 0) {
fmt = '%(comments_count)s %(span_sr_open)scomments (%(unread_comments_count)s unread comments)%(span_close)s';
} else {
fmt = '%(comments_count)s %(span_sr_open)scomments %(span_close)s';
}
print(interpolate(fmt, data, true));
%>
</span>
<span class="votes-count">+<%=
interpolate(
'%(votes_up_count)s%(span_sr_open)s votes %(span_close)s',
{'span_sr_open': '<span class="sr">', 'span_close': '</span>', 'votes_up_count': votes['up_count']},
true
)
%></span>
</a>
<li data-id="<%- id %>" class="forum-nav-thread<% if (typeof(read) != "undefined" && !read) { %> is-unread<% } %>">
<a href="#" class="forum-nav-thread-link">
<div class="forum-nav-thread-wrapper-1">
<span class="forum-nav-thread-title"><%- title %></span>
</div><div class="forum-nav-thread-wrapper-2">
<% if (endorsed) { %>
<span class="forum-nav-thread-endorsed"><i class="icon icon-ok"></i><span class="sr">Endorsed response</span></span>
<% } %>
<span class="forum-nav-thread-votes-count">+<%=
interpolate(
'%(votes_up_count)s%(span_sr_open)s votes %(span_close)s',
{'span_sr_open': '<span class="sr">', 'span_close': '</span>', 'votes_up_count': votes['up_count']},
true
)
%></span>
<span class="forum-nav-thread-comments-count <% if (unread_comments_count > 0) { %>is-unread<% } %>">
<%
var fmt;
var data = {
'span_sr_open': '<span class="sr">',
'span_close': '</span>',
'unread_comments_count': unread_comments_count,
'comments_count': comments_count
};
if (unread_comments_count > 0) {
fmt = '%(comments_count)s %(span_sr_open)scomments (%(unread_comments_count)s unread comments)%(span_close)s';
} else {
fmt = '%(comments_count)s %(span_sr_open)scomments %(span_close)s';
}
print(interpolate(fmt, data, true));
%>
</span>
</div>
</a>
</li>
</script>
<script type="text/template" id="thread-list-template">
<div class="browse-search">
......@@ -53,9 +60,7 @@ describe "DiscussionThreadListView", ->
</ul>
</div>
<div class="search-alerts"></div>
<div class="post-list-wrapper">
<ul class="post-list"></ul>
</div>
<ul class="forum-nav-thread-list"></ul>
</script>
<script aria-hidden="true" type="text/template" id="search-alert-template">
<div class="search-alert" id="search-alert-<%- cid %>">
......@@ -71,9 +76,27 @@ describe "DiscussionThreadListView", ->
<div class="sidebar"></div>
"""
@threads = [
{id: "1", title: "Thread1", body: "dummy body", votes: {up_count: '20'}, unread_comments_count:0, comments_count:1, created_at: '2013-04-03T20:08:39Z',},
{id: "2", title: "Thread2", body: "dummy body", votes: {up_count: '42'}, unread_comments_count:0, comments_count:2, created_at: '2013-04-03T20:07:39Z',},
{id: "3", title: "Thread3", body: "dummy body", votes: {up_count: '12'}, unread_comments_count:0, comments_count:3, created_at: '2013-04-03T20:06:39Z',},
makeThreadWithProps({
id: "1",
title: "Thread1",
votes: {up_count: '20'},
comments_count: 1,
created_at: '2013-04-03T20:08:39Z',
}),
makeThreadWithProps({
id: "2",
title: "Thread2",
votes: {up_count: '42'},
comments_count: 2,
created_at: '2013-04-03T20:07:39Z',
}),
makeThreadWithProps({
id: "3",
title: "Thread3",
votes: {up_count: '12'},
comments_count: 3,
created_at: '2013-04-03T20:06:39Z',
}),
]
spyOn($, "ajax")
......@@ -82,6 +105,21 @@ describe "DiscussionThreadListView", ->
@view = new DiscussionThreadListView({collection: @discussion, el: $(".sidebar")})
@view.render()
makeThreadWithProps = (props) ->
# Minimal set of properties necessary for rendering
thread = {
id: "dummy_id",
pinned: false,
endorsed: false,
votes: {up_count: '0'},
unread_comments_count: 0,
comments_count: 0,
}
$.extend(thread, props)
renderSingleThreadWithProps = (props) ->
makeView(new Discussion([new Thread(makeThreadWithProps(props))])).render()
makeView = (discussion) ->
return new DiscussionThreadListView(
el: $(".sidebar"),
......@@ -89,10 +127,10 @@ describe "DiscussionThreadListView", ->
)
checkThreadsOrdering = (view, sort_order, type) ->
expect(view.$el.find(".post-list .list-item").children().length).toEqual(3)
expect(view.$el.find(".post-list .list-item:nth-child(1) .title").text()).toEqual(sort_order[0])
expect(view.$el.find(".post-list .list-item:nth-child(2) .title").text()).toEqual(sort_order[1])
expect(view.$el.find(".post-list .list-item:nth-child(3) .title").text()).toEqual(sort_order[2])
expect(view.$el.find(".forum-nav-thread").children().length).toEqual(3)
expect(view.$el.find(".forum-nav-thread:nth-child(1) .forum-nav-thread-title").text()).toEqual(sort_order[0])
expect(view.$el.find(".forum-nav-thread:nth-child(2) .forum-nav-thread-title").text()).toEqual(sort_order[1])
expect(view.$el.find(".forum-nav-thread:nth-child(3) .forum-nav-thread-title").text()).toEqual(sort_order[2])
expect(view.$el.find(".sort-bar a.active").text()).toEqual(type)
describe "thread rendering should be correct", ->
......@@ -269,3 +307,12 @@ describe "DiscussionThreadListView", ->
@view.searchForUser("dummy")
expect($.ajax).toHaveBeenCalled()
expect(@view.addSearchAlert).not.toHaveBeenCalled()
describe "endorsed renders correctly", ->
it "when absent", ->
renderSingleThreadWithProps({})
expect($(".forum-nav-thread-endorsed").length).toEqual(0)
it "when present", ->
renderSingleThreadWithProps({endorsed: true})
expect($(".forum-nav-thread-endorsed").length).toEqual(1)
common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee
......@@ -9,8 +9,8 @@ if Backbone?
"click .sort-bar a": "sortThreads"
"click .browse-topic-drop-menu": "filterTopic"
"click .browse-topic-drop-search-input": "ignoreClick"
"click .post-list .list-item a": "threadSelected"
"click .post-list .more-pages a": "loadMorePages"
"click .forum-nav-thread-link": "threadSelected"
"click .forum-nav-load-more-link": "loadMorePages"
"change .cohort-options": "chooseCohort"
'keyup .browse-topic-drop-search-input': DiscussionFilter.filterDrop
......@@ -68,8 +68,8 @@ if Backbone?
@clearSearchAlerts()
thread_id = thread.get('id')
content = @renderThread(thread)
current_el = @$("a[data-id=#{thread_id}]")
active = current_el.hasClass("active")
current_el = @$(".forum-nav-thread[data-id=#{thread_id}]")
active = current_el.has(".forum-nav-thread-link.is-active").length != 0
current_el.replaceWith(content)
if active
@setActiveThread(thread_id)
......@@ -111,8 +111,7 @@ if Backbone?
sidebarHeight = Math.min(sidebarHeight + 1, discussionBody.outerHeight())
sidebar.css 'height', sidebarHeight
postListWrapper = @$('.post-list-wrapper')
postListWrapper.css('height', (sidebarHeight - @sidebar_header_height - 4) + 'px')
@$('.forum-nav-thread-list').css('height', (sidebarHeight - @sidebar_header_height - 4) + 'px')
# Because we want the behavior that when the body is clicked the menu is
......@@ -138,30 +137,32 @@ if Backbone?
@
renderThreads: =>
@$(".post-list").html("")
@$(".forum-nav-thread-list").html("")
rendered = $("<div></div>")
for thread in @displayedCollection.models
content = @renderThread(thread)
rendered.append content
content.wrap("<li class='list-item' data-id='\"#{thread.get('id')}\"' />")
@$(".post-list").html(rendered.html())
@$(".forum-nav-thread-list").html(rendered.html())
@renderMorePages()
@updateSidebar()
@trigger "threads:rendered"
renderMorePages: ->
if @displayedCollection.hasMorePages()
@$(".post-list").append("<li class='more-pages'><a href='#'>" + gettext("Load more") + "</a></li>")
@$(".forum-nav-thread-list").append("<li class='forum-nav-load-more'><a href='#' class='forum-nav-load-more-link'>" + gettext("Load more") + "</a></li>")
loadMorePages: (event) ->
getLoadingContent: (srText) ->
return '<div class="forum-nav-loading" tabindex="0"><span class="icon-spinner icon-spin"/><span class="sr" role="alert">' + srText + '</span></div>'
loadMorePages: (event) =>
if event
event.preventDefault()
@$(".more-pages").html('<div class="loading-animation" tabindex=0><span class="sr" role="alert">' + gettext('Loading more threads') + '</span></div>')
@$(".more-pages").addClass("loading")
loadingDiv = @$(".more-pages .loading-animation")
DiscussionUtil.makeFocusTrap(loadingDiv)
loadingDiv.focus()
loadMoreElem = @$(".forum-nav-load-more")
loadMoreElem.html(@getLoadingContent(gettext("Loading more threads")))
loadingElem = loadMoreElem.find(".forum-nav-loading")
DiscussionUtil.makeFocusTrap(loadingElem)
loadingElem.focus()
options = {}
switch @mode
when 'search'
......@@ -184,12 +185,12 @@ if Backbone?
if lastThread
# Pagination; focus the first thread after what was previously the last thread
@once("threads:rendered", ->
$(".post-list li:has(a[data-id='#{lastThread}']) + li a").focus()
$(".forum-nav-thread[data-id='#{lastThread}'] + .forum-nav-thread .forum-nav-thread-link").focus()
)
else
# Totally refreshing the list (e.g. from clicking a sort button); focus the first thread
@once("threads:rendered", ->
$(".post-list a").first()?.focus()
$(".forum-nav-thread-link").first()?.focus()
)
error = =>
......@@ -200,15 +201,9 @@ if Backbone?
renderThread: (thread) =>
content = $(_.template($("#thread-list-item-template").html())(thread.toJSON()))
if thread.get('subscribed')
content.addClass("followed")
if thread.get('endorsed')
content.addClass("resolved")
if thread.get('read')
content.addClass("read")
unreadCount = thread.get('unread_comments_count')
if unreadCount > 0
content.find('.comments-count').addClass("unread").attr(
content.find('.forum-nav-thread-comments-count').attr(
"data-tooltip",
interpolate(
ngettext('%(unread_count)s new comment', '%(unread_count)s new comments', unreadCount),
......@@ -216,25 +211,14 @@ if Backbone?
true
)
)
@highlight(content)
highlight: (el) ->
el.html(el.html().replace(/&lt;mark&gt;/g, "<mark>").replace(/&lt;\/mark&gt;/g, "</mark>"))
renderThreadListItem: (thread) =>
view = new ThreadListItemView(model: thread)
view.on "thread:selected", @threadSelected
view.on "thread:removed", @threadRemoved
view.render()
@$(".post-list").append(view.el)
content
threadSelected: (e) =>
# Use .attr('data-id') rather than .data('id') because .data does type
# coercion. Usually, this is fine, but when Mongo gives an object id with
# no letters, it casts it to a Number.
thread_id = $(e.target).closest("a").attr("data-id")
thread_id = $(e.target).closest(".forum-nav-thread").attr("data-id")
@setActiveThread(thread_id)
@trigger("thread:selected", thread_id) # This triggers a callback in the DiscussionRouter which calls the line above...
false
......@@ -243,8 +227,8 @@ if Backbone?
@trigger("thread:removed", thread_id)
setActiveThread: (thread_id) ->
@$(".post-list a[data-id!='#{thread_id}']").removeClass("active")
@$(".post-list a[data-id='#{thread_id}']").addClass("active")
@$(".forum-nav-thread[data-id!='#{thread_id}'] .forum-nav-thread-link").removeClass("is-active")
@$(".forum-nav-thread[data-id='#{thread_id}'] .forum-nav-thread-link").addClass("is-active")
showSearch: ->
@$(".browse").removeClass('is-dropped')
......@@ -257,7 +241,7 @@ if Backbone?
goHome: ->
@template = _.template($("#discussion-home").html())
$(".discussion-column").html(@template)
$(".post-list a").removeClass("active")
$(".forum-nav-thread-list a").removeClass("is-active")
$("input.email-setting").bind "click", @updateEmailNotifications
url = DiscussionUtil.urlFor("notifications_status",window.user.get("id"))
DiscussionUtil.safeAjax
......@@ -448,7 +432,7 @@ if Backbone?
dataType: 'json'
$loading: $
loadingCallback: =>
@$(".post-list").html('<li class="loading"><div class="loading-animation"><span class="sr">' + gettext('Loading thread list') + '</span></div></li>')
@$(".forum-nav-thread-list").html("<li class='forum-nav-load-more'>" + @getLoadingContent(gettext("Loading thread list")) + "</li>")
loadedCallback: =>
if callback
callback.apply @, [value]
......
lms/static/sass/application-extend2.scss.mako
......@@ -48,8 +48,12 @@
@import 'views/shoppingcart';
// applications
@import 'discussion/discussion';
@import 'discussion/discussion-developer';
@import "discussion/utilities/variables";
@import 'discussion/discussion'; // Process old file after definitions but before everything else
@import "discussion/elements/navigation";
@import 'discussion/utilities/developer';
@import 'discussion/utilities/shame';
@import 'news';
// temp - shame and developer
......
lms/static/sass/discussion/_discussion.scss
......@@ -953,205 +953,6 @@ body.discussion {
}
}
.post-list-wrapper {
overflow-y: scroll;
overflow-x: hidden;
//border-right: 1px solid transparent;
}
.post-list {
background-color: #ddd;
.loading {
padding: ($baseline*.75) 0;
background: $gray-l6;
.loading-animation {
background-image: url(../images/spinner-on-grey.gif);
}
}
.more-pages a {
background: #eee;
font-size: 12px;
line-height: 33px;
text-align: center;
&:hover, &:focus {
background-image: none;
background-color: #e6e6e6;
}
}
a {
display: block;
position: relative;
float: left;
clear: both;
width: 100%;
padding: 0 ($baseline/2) 0 18px;
margin-bottom: 1px;
margin-right: -1px;
@include linear-gradient(top, rgba(255, 255, 255, .7), rgba(255, 255, 255, 0));
background-color: $white;
@include clearfix;
&:hover, &:focus {
@include linear-gradient(top, rgba(255, 255, 255, .7), rgba(255, 255, 255, 0));
background-color: #eee;
}
&.staff-post.staff-response {
.staff-post-icon {
top: 5px;
}
.staff-response-icon {
top: 18px;
}
}
.staff-post-icon,
.staff-response-icon {
position: absolute;
top: 11px;
left: 3px;
width: 13px;
height: 13px;
background: url(../images/staff-icons.png) no-repeat;
}
.staff-post-icon {
left: 2px;
background-position: 0 0;
}
.staff-response-icon {
background-position: -13px 0;
}
.title {
display: block;
float: left;
width: 70%;
margin: ($baseline/2) 0 ($baseline/2);
font-size: 13px;
font-weight: 700;
line-height: 1.4;
color: $dark-gray;
}
&.read {
background: #f2f2f2;
.title {
font-weight: 400;
color: #737373;
}
}
&.resolved:before {
content: '';
position: absolute;
top: 12px;
right: 75px;
width: 9px;
height: 8px;
background: url(../images/sidebar-resolved-icons.png) no-repeat;
}
&.followed:after {
content: '';
position: absolute;
top: 0;
right: 0;
width: 12px;
height: 12px;
background: url(../images/following-flag.png) no-repeat;
}
&.active {
@include linear-gradient(top, #96e0fd, #61c7fc);
border-color: #4697c1;
box-shadow: 0 1px 0 #4697c1, 0 -1px 0 #4697c1;
.title {
color: #333;
}
.staff-post-icon {
background-position: 0 -13px;
}
.staff-response-icon {
background-position: -13px -13px;
}
.comments-count {
@include linear-gradient(top, #3994c7, #4da7d3);
color: $white;
&:after {
background-position: 0 0;
}
}
&.followed:after {
background-position: 0 -12px;
}
&.resolved:before {
background-position: 0 -8px;
}
}
}
.votes-count,
.comments-count {
display: block;
float: right;
margin-top: 8px;
border-radius: 2px;
width: 36px;
height: 20px;
color: #767676;
text-align: center;
font-size: 11px;
line-height: 20px;
}
.comments-count {
@include linear-gradient(top, #d4d4d4, #dfdfdf);
position: relative;
margin-left: ($baseline/4);
margin-right:($baseline/4);
width: 28px;
font-weight: 700;
&:after {
content: '';
display: block;
position: absolute;
top: 20px;
right: 3px;
width: 5px;
height: 5px;
background: url(../images/comment-icon-bottoms.png) no-repeat;
background-position: 0 -5px;
}
&.unread {
@include linear-gradient(top, #84d7fe, #60a8d6);
color: #333;
&:after {
color: #99e0fe;
background-position: 0 0px;
}
}
}
}
.bottom-post-status {
padding: 30px;
font-size: 20px;
......
lms/static/sass/discussion/elements/_navigation.scss 0 → 100644
// -----------
// Thread list
// -----------
.forum-nav-thread-list {
overflow-y: scroll;
}
.forum-nav-thread {
border-bottom: 1px solid $gray-l3;
}
.forum-nav-thread-link {
@include clearfix();
}
%forum-nav-thread-wrapper {
display: inline-block;
vertical-align: middle;
}
.forum-nav-thread-wrapper-1 {
@extend %forum-nav-thread-wrapper;
width: 70%;
}
.forum-nav-thread-wrapper-2 {
@extend %forum-nav-thread-wrapper;
width: 30%;
text-align: right;
}
.forum-nav-thread-title {
@extend %t-title7;
display: block;
}
%forum-nav-thread-wrapper-2-content {
@include font-size(11);
display: inline-block;
margin-right: ($baseline/4);
text-align: center;
color: $black;
&:last-child {
margin-right: 0;
}
}
.forum-nav-thread-endorsed {
@extend %forum-nav-thread-wrapper-2-content;
color: $green-d1;
}
.forum-nav-thread-votes-count {
@extend %forum-nav-thread-wrapper-2-content;
}
.forum-nav-thread-comments-count {
@extend %forum-nav-thread-wrapper-2-content;
@extend %t-weight4;
position: relative;
margin-left: ($baseline/4);
margin-bottom: ($baseline/4); // Because tail is position: absolute
border-radius: 2px;
padding: ($baseline/10) ($baseline/5);
min-width: 2em; // Fit most comment counts but allow expansion if necessary
background-color: $gray-l3;
// Speech bubble tail
&:after {
content: '';
display: block;
position: absolute;
bottom: (-$baseline/4);
right: ($baseline/4);
width: 0;
height: 0;
border-style: solid;
border-width: 0 ($baseline/4) ($baseline/4) 0;
border-color: transparent $gray-l3 transparent transparent;
}
&.is-unread {
background-color: $white;
&:after {
border-right-color: $white
}
}
}
.forum-nav-thread.is-unread .forum-nav-thread-comments-count {
background-color: $blue;
color: $white;
&:after {
border-right-color: $blue;
}
}
%forum-nav-load-more-content {
text-align: center;
}
.forum-nav-load-more-link {
@extend %forum-nav-load-more-content;
color: $link-color;
}
.forum-nav-loading {
@extend %forum-nav-load-more-content;
}
lms/static/sass/discussion/utilities/_shame.scss 0 → 100644
// The sidebar class does a lot of things that we don't want in the thread list;
// the following rules contain styling that is necessary and would otherwise
// reside in elements/_navigation.scss if the sidebar styling did not make the
// !important directive necessary.
.forum-nav-thread {
background-color: $gray-l5 !important;
&.is-unread {
background-color: $white !important;
}
}
.forum-nav-thread-link {
padding: ($baseline/4) ($baseline/2) !important;
&.is-active, &:hover, &:focus {
background-color: $forum-color-active-thread !important;
}
}
.forum-nav-load-more {
border-bottom: 1px solid $gray-l3 !important;
background-color: $gray-l5 !important;
}
.forum-nav-load-more-link {
&:hover, &:focus {
color: $link-color !important;
background-color: $forum-color-active-thread !important;
}
}
.forum-nav-load-more-link, .forum-nav-loading {
padding: $baseline 0 !important;
}
lms/static/sass/discussion/utilities/_variables.scss 0 → 100644
$forum-color-active-thread: tint($blue, 85%);
lms/templates/discussion/_thread_list_template.html
......@@ -45,8 +45,5 @@
%endif
</div>
<div class="search-alerts"></div>
<div class="post-list-wrapper">
<ul class="post-list">
</ul>
</div>
<ul class="forum-nav-thread-list"></ul>
</script>
lms/templates/discussion/_underscore_templates.html
......@@ -205,9 +205,29 @@
</script>
<script aria-hidden="true" type="text/template" id="thread-list-item-template">
<a href="${'<%- id %>'}" data-id="${'<%- id %>'}">
<span class="title">${"<%- title %>"}</span>
<li data-id="${'<%- id %>'}" class="forum-nav-thread${'<% if (typeof(read) != "undefined" && !read) { %> is-unread<% } %>'}">
<a href="#" class="forum-nav-thread-link">
<div class="forum-nav-thread-wrapper-1">
<span class="forum-nav-thread-title">${"<%- title %>"}</span>
</div><div class="forum-nav-thread-wrapper-2">
${"<% if (endorsed) { %>"}
## Translators: This is a label for a forum thread with a response that was endorsed by the course staff
<span class="forum-nav-thread-endorsed"><i class="icon icon-ok"></i><span class="sr">${_("Endorsed response")}</span></span>
${"<% } %>"}
<%
js_block = u"""
interpolate(
'{}',
{{'span_sr_open': '<span class=\"sr\">', 'span_close': '</span>', 'votes_up_count': votes['up_count']}},
true
)
""".format(
## Translators: 'votes_up_count' is a numerical placeholder for a specific discussion thread; 'span_*' placeholders refer to HTML markup. Please translate the word 'votes'.
escapejs( _('%(votes_up_count)s%(span_sr_open)s votes %(span_close)s'))
)
%>
<span class="forum-nav-thread-votes-count">+${'<%='}${js_block}${'%>'}</span>
<%
js_block = u"""
var fmt;
var data = {{
......@@ -229,24 +249,14 @@
markup_none_unread=escapejs(_('%(comments_count)s %(span_sr_open)scomments %(span_close)s'))
)
%>
<span class="comments-count">
<span class="forum-nav-thread-comments-count ${'<% if (unread_comments_count > 0) { %>is-unread<% } %>'}">
${'<%'}${js_block}${'%>'}
</span>
<%
js_block = u"""
interpolate(
'{}',
{{'span_sr_open': '<span class=\"sr\">', 'span_close': '</span>', 'votes_up_count': votes['up_count']}},
true
)
""".format(
## Translators: 'votes_up_count' is a numerical placeholder for a specific discussion thread; 'span_*' placeholders refer to HTML markup. Please translate the word 'votes'.
escapejs( _('%(votes_up_count)s%(span_sr_open)s votes %(span_close)s'))
)
%>
<span class="votes-count">+${'<%='}${js_block}${'%>'}</span>
</div>
</a>
</li>
</script>
<script aria-hidden="true" type="text/template" id="discussion-home">
<div class="discussion-article blank-slate">
<section class="home-header">
......
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