From 551cea937903730f19ed7ff296f03a0bcba0f62b Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 10 Jun 2014 11:09:10 -0400 Subject: [PATCH] Add labels to forum thread list A thread will be labeled if it is pinned, if the user is following it, or if it was authored by a staff member or community TA. This also slightly changes the color used for community TA labels. Originally reviewed in #4072 --- .../discussion_thread_list_view_spec.coffee | 57 ++++++++++++++++++- .../coffee/src/discussion/content.coffee | 5 +- .../static/coffee/src/discussion/main.coffee | 2 + .../static/coffee/src/discussion/utils.coffee | 4 +- lms/static/sass/discussion/_discussion.scss | 6 +- .../sass/discussion/elements/_navigation.scss | 43 ++++++++++++++ .../sass/discussion/utilities/_shame.scss | 20 +++++++ .../sass/discussion/utilities/_variables.scss | 4 ++ .../discussion/_underscore_templates.html | 30 ++++++++++ 9 files changed, 163 insertions(+), 8 deletions(-) diff --git a/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee b/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee index 577893caa0..48c941cb13 100644 --- a/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee +++ b/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee @@ -8,6 +8,24 @@ describe "DiscussionThreadListView", ->
<%- title %> + <% + var labels = ""; + if (pinned) { + labels += '
  • Pinned
    • '; + } + if (typeof(subscribed) != "undefined" && subscribed) { + labels += '
  • Following
    • '; + } + if (staff_authored) { + labels += '
  • By: Staff
    • '; + } + if (community_ta_authored) { + labels += '
  • By: Community TA
    • '; + } + if (labels != "") { + print('
      ' + labels + '
    '); + } + %>
    <% if (endorsed) { %> Endorsed response @@ -135,7 +153,7 @@ describe "DiscussionThreadListView", -> describe "thread rendering should be correct", -> checkRender = (threads, type, sort_order) -> - discussion = new Discussion(threads, {pages: 1, sort: type}) + discussion = new Discussion(_.map(threads, (thread) -> new Thread(thread)), {pages: 1, sort: type}) view = makeView(discussion) view.render() checkThreadsOrdering(view, sort_order, type) @@ -151,7 +169,7 @@ describe "DiscussionThreadListView", -> describe "Sort click should be correct", -> changeSorting = (threads, selected_type, new_type, sort_order) -> - discussion = new Discussion(threads, {pages: 1, sort: selected_type}) + discussion = new Discussion(_.map(threads, (thread) -> new Thread(thread)), {pages: 1, sort: selected_type}) view = makeView(discussion) view.render() expect(view.$el.find(".sort-bar a.active").text()).toEqual(selected_type) @@ -316,3 +334,38 @@ describe "DiscussionThreadListView", -> it "when present", -> renderSingleThreadWithProps({endorsed: true}) expect($(".forum-nav-thread-endorsed").length).toEqual(1) + + describe "post labels render correctly", -> + beforeEach -> + @moderatorId = "42" + @administratorId = "43" + @communityTaId = "44" + DiscussionUtil.loadRoles({ + "Moderator": [parseInt(@moderatorId)], + "Administrator": [parseInt(@administratorId)], + "Community TA": [parseInt(@communityTaId)], + }) + + it "for pinned", -> + renderSingleThreadWithProps({pinned: true}) + expect($(".forum-nav-thread-label-pinned").length).toEqual(1) + + it "for following", -> + renderSingleThreadWithProps({subscribed: true}) + expect($(".forum-nav-thread-label-following").length).toEqual(1) + + it "for moderator", -> + renderSingleThreadWithProps({user_id: @moderatorId}) + expect($(".forum-nav-thread-label-staff").length).toEqual(1) + + it "for administrator", -> + renderSingleThreadWithProps({user_id: @administratorId}) + expect($(".forum-nav-thread-label-staff").length).toEqual(1) + + it "for community TA", -> + renderSingleThreadWithProps({user_id: @communityTaId}) + expect($(".forum-nav-thread-label-community-ta").length).toEqual(1) + + it "when none should be present", -> + renderSingleThreadWithProps({}) + expect($(".forum-nav-thread-labels").length).toEqual(0) diff --git a/common/static/coffee/src/discussion/content.coffee b/common/static/coffee/src/discussion/content.coffee index 7090063f6a..ebb7a9b299 100644 --- a/common/static/coffee/src/discussion/content.coffee +++ b/common/static/coffee/src/discussion/content.coffee @@ -53,9 +53,12 @@ if Backbone? initialize: -> Content.addContent @id, @ + userId = @get('user_id') + @set('staff_authored', DiscussionUtil.isStaff(userId)) + @set('community_ta_authored', DiscussionUtil.isTA(userId)) if Content.getInfo(@id) @updateInfo(Content.getInfo(@id)) - @set 'user_url', DiscussionUtil.urlFor('user_profile', @get('user_id')) + @set 'user_url', DiscussionUtil.urlFor('user_profile', userId) @resetComments(@get('children')) remove: -> diff --git a/common/static/coffee/src/discussion/main.coffee b/common/static/coffee/src/discussion/main.coffee index 29e97161d3..67c15d1e34 100644 --- a/common/static/coffee/src/discussion/main.coffee +++ b/common/static/coffee/src/discussion/main.coffee @@ -20,6 +20,8 @@ if Backbone? Backbone.history.start({pushState: true, root: "/courses/#{$$course_id}/discussion/forum/"}) DiscussionProfileApp = start: (elem) -> + # Roles are not included in user profile page, but they are not used for anything + DiscussionUtil.loadRoles({"Moderator": [], "Administrator": [], "Community TA": []}) element = $(elem) window.$$course_id = element.data("course-id") threads = element.data("threads") diff --git a/common/static/coffee/src/discussion/utils.coffee b/common/static/coffee/src/discussion/utils.coffee index 8242779c8a..09ab0a9343 100644 --- a/common/static/coffee/src/discussion/utils.coffee +++ b/common/static/coffee/src/discussion/utils.coffee @@ -32,12 +32,12 @@ class @DiscussionUtil @loadFlagModerator($("#discussion-container").data("flag-moderator")) @isStaff: (user_id) -> - user_id ?= @user.id + user_id ?= @user?.id staff = _.union(@roleIds['Moderator'], @roleIds['Administrator']) _.include(staff, parseInt(user_id)) @isTA: (user_id) -> - user_id ?= @user.id + user_id ?= @user?.id ta = _.union(@roleIds['Community TA']) _.include(ta, parseInt(user_id)) diff --git a/lms/static/sass/discussion/_discussion.scss b/lms/static/sass/discussion/_discussion.scss index d401df9a5f..f07f15b0a2 100644 --- a/lms/static/sass/discussion/_discussion.scss +++ b/lms/static/sass/discussion/_discussion.scss @@ -1241,7 +1241,7 @@ body.discussion { &.community-ta{ padding-top: 38px; - border-color: #449944; + border-color: $forum-color-community-ta; } .staff-banner { @@ -1269,7 +1269,7 @@ body.discussion { padding: 1px 5px; @include box-sizing(border-box); border-radius: 2px 2px 0 0; - background: #449944; + background: $forum-color-community-ta; font-size: 9px; font-weight: 700; color: $white; @@ -1473,7 +1473,7 @@ body.discussion { margin-left: ($baseline/10); padding: 0 ($baseline/5); border-radius: 2px; - background: #449944; + background: $forum-color-community-ta; font-size: 9px; font-weight: 700; font-style: normal; diff --git a/lms/static/sass/discussion/elements/_navigation.scss b/lms/static/sass/discussion/elements/_navigation.scss index b8ac14061e..7a0b94e2e8 100644 --- a/lms/static/sass/discussion/elements/_navigation.scss +++ b/lms/static/sass/discussion/elements/_navigation.scss @@ -34,6 +34,49 @@ display: block; } +%forum-nav-thread-label { + @extend %t-weight4; + @include font-size(9); + display: inline; + border: 1px solid; + border-radius: 3px; + text-transform: uppercase; + white-space: nowrap; + + &:last-child { + margin-right: 0; + } + + .icon { + margin-right: ($baseline/5); + } + +} + +.forum-nav-thread-label-pinned { + @extend %forum-nav-thread-label; + border-color: $forum-color-pinned; + color: $forum-color-pinned; +} + +.forum-nav-thread-label-following { + @extend %forum-nav-thread-label; + border-color: $forum-color-following; + color: $forum-color-following; +} + +.forum-nav-thread-label-staff { + @extend %forum-nav-thread-label; + border-color: $forum-color-staff; + color: $forum-color-staff; +} + +.forum-nav-thread-label-community-ta { + @extend %forum-nav-thread-label; + border-color: $forum-color-community-ta; + color: $forum-color-community-ta; +} + %forum-nav-thread-wrapper-2-content { @include font-size(11); display: inline-block; diff --git a/lms/static/sass/discussion/utilities/_shame.scss b/lms/static/sass/discussion/utilities/_shame.scss index eaa2a6edea..991cec5df0 100644 --- a/lms/static/sass/discussion/utilities/_shame.scss +++ b/lms/static/sass/discussion/utilities/_shame.scss @@ -19,6 +19,11 @@ } } +li[class*=forum-nav-thread-label-] { + margin-top: ($baseline/4) !important; + padding: 1px 6px !important; +} + .forum-nav-load-more { border-bottom: 1px solid $gray-l3 !important; background-color: $gray-l5 !important; @@ -34,3 +39,18 @@ .forum-nav-load-more-link, .forum-nav-loading { padding: $baseline 0 !important; } + +// The following rules would be unnecessary but for broadly scoped rules defined +// elsewhere in our CSS. + +li[class*=forum-nav-thread-label-] { + // Override global span rules + span { + color: inherit; + } + + // Override clearfix stuff in .sidebar ul li rules + &:before, &:after { + display: none !important; + } +} diff --git a/lms/static/sass/discussion/utilities/_variables.scss b/lms/static/sass/discussion/utilities/_variables.scss index bfc9bf8677..4009d74d3b 100644 --- a/lms/static/sass/discussion/utilities/_variables.scss +++ b/lms/static/sass/discussion/utilities/_variables.scss @@ -1 +1,5 @@ $forum-color-active-thread: tint($blue, 85%); +$forum-color-pinned: $pink; +$forum-color-following: $blue; +$forum-color-staff: $blue; +$forum-color-community-ta: $green-d1; diff --git a/lms/templates/discussion/_underscore_templates.html b/lms/templates/discussion/_underscore_templates.html index 185a9ec640..f7b9bbd57a 100644 --- a/lms/templates/discussion/_underscore_templates.html +++ b/lms/templates/discussion/_underscore_templates.html @@ -209,6 +209,36 @@
    ${"<%- title %>"} + <% + js_block = u""" + var labels = ""; + if (pinned) {{ + labels += '
  • {pinned_text}
    • '; + }} + if (typeof(subscribed) != "undefined" && subscribed) {{ + labels += '
  • {following_text}
    • '; + }} + if (staff_authored) {{ + labels += '
  • {staff_text}
    • '; + }} + if (community_ta_authored) {{ + labels += '
  • {community_ta_text}
    • '; + }} + if (labels != "") {{ + print('
      ' + labels + '
    '); + }} + """.format( + ## Translators: This is a label for a forum thread that has been pinned + pinned_text=escapejs(_("Pinned")), + ## Translators: This is a label for a forum thread that the user is subscribed to + following_text=escapejs(_("Following")), + ## Translators: This is a label for a forum thread that was authored by a member of the course staff + staff_text=escapejs(_("By: Staff")), + ## Translators: This is a label for a forum thread that was authored by a community TA + community_ta_text=escapejs(_("By: Community TA")) + ) + %> + ${"<%"}${js_block}${"%>"}
    ${"<% if (endorsed) { %>"} ## Translators: This is a label for a forum thread with a response that was endorsed by the course staff