From 2e30874180291c4cdd099caf3784cd7db82fcfbe Mon Sep 17 00:00:00 2001
From: Greg Price <gprice@edx.org>
Date: Tue, 22 Jul 2014 16:35:39 -0400
Subject: [PATCH] Add post type indicators to forum sidebar nav

The previously existing checkmark for threads with endorsed responses is
removed, as it is mostly redundant.
---
 .../discussion_thread_list_view_spec.coffee   | 42 ++++++++++++++-----
 .../view/discussion_view_spec_helper.coffee   |  1 +
 .../views/discussion_content_view.coffee      | 10 -----
 .../views/discussion_thread_view.coffee       |  2 +-
 .../views/thread_response_show_view.coffee    | 12 ++++++
 .../views/thread_response_view.coffee         |  1 +
 .../sass/discussion/elements/_navigation.scss | 30 +++++++++----
 .../sass/discussion/utilities/_variables.scss |  1 +
 .../discussion/_underscore_templates.html     | 29 ++++++++++---
 9 files changed, 94 insertions(+), 34 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 813c6b36b8..2c8a0a1afe 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
@@ -6,7 +6,23 @@ describe "DiscussionThreadListView", ->
         <script type="text/template" id="thread-list-item-template">
           <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">
+              <div class="forum-nav-thread-wrapper-0">
+                <%
+                var icon_class, sr_text;
+                if (thread_type == "discussion") {
+                    icon_class = "icon-comments";
+                    sr_text = "discussion";
+                } else if (endorsed) {
+                    icon_class = "icon-ok";
+                    sr_text = "answered question";
+                } else {
+                    icon_class = "icon-question";
+                    sr_text = "unanswered question";
+                }
+                %>
+                <span class="sr"><%= sr_text %></span>
+                <i class="icon <%= icon_class %>"></i>
+              </div><div class="forum-nav-thread-wrapper-1">
                 <span class="forum-nav-thread-title"><%- title %></span>
                 <%
                 var labels = "";
@@ -27,9 +43,6 @@ describe "DiscussionThreadListView", ->
                 }
                 %>
               </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',
@@ -372,14 +385,21 @@ describe "DiscussionThreadListView", ->
             expect($.ajax).toHaveBeenCalled()
             expect(@view.addSearchAlert).not.toHaveBeenCalled()
 
-    describe "endorsed renders correctly", ->
-      it "when absent", ->
-        renderSingleThreadWithProps({})
-        expect($(".forum-nav-thread-endorsed").length).toEqual(0)
+    describe "post type renders correctly", ->
+      it "for discussion", ->
+        renderSingleThreadWithProps({thread_type: "discussion"})
+        expect($(".forum-nav-thread-wrapper-0 .icon")).toHaveClass("icon-comments")
+        expect($(".forum-nav-thread-wrapper-0 .sr")).toHaveText("discussion")
 
-      it "when present", ->
-        renderSingleThreadWithProps({endorsed: true})
-        expect($(".forum-nav-thread-endorsed").length).toEqual(1)
+      it "for answered question", ->
+        renderSingleThreadWithProps({thread_type: "question", endorsed: true})
+        expect($(".forum-nav-thread-wrapper-0 .icon")).toHaveClass("icon-ok")
+        expect($(".forum-nav-thread-wrapper-0 .sr")).toHaveText("answered question")
+
+      it "for unanswered question", ->
+        renderSingleThreadWithProps({thread_type: "question", endorsed: false})
+        expect($(".forum-nav-thread-wrapper-0 .icon")).toHaveClass("icon-question")
+        expect($(".forum-nav-thread-wrapper-0 .sr")).toHaveText("unanswered question")
 
     describe "post labels render correctly", ->
       beforeEach ->
diff --git a/common/static/coffee/spec/discussion/view/discussion_view_spec_helper.coffee b/common/static/coffee/spec/discussion/view/discussion_view_spec_helper.coffee
index bfe659587f..cc37048171 100644
--- a/common/static/coffee/spec/discussion/view/discussion_view_spec_helper.coffee
+++ b/common/static/coffee/spec/discussion/view/discussion_view_spec_helper.coffee
@@ -3,6 +3,7 @@ class @DiscussionViewSpecHelper
         # Minimal set of properties necessary for rendering
         thread = {
           id: "dummy_id",
+          thread_type: "discussion",
           pinned: false,
           endorsed: false,
           votes: {up_count: '0'},
diff --git a/common/static/coffee/src/discussion/views/discussion_content_view.coffee b/common/static/coffee/src/discussion/views/discussion_content_view.coffee
index 00939e95f6..09e07564c8 100644
--- a/common/static/coffee/src/discussion/views/discussion_content_view.coffee
+++ b/common/static/coffee/src/discussion/views/discussion_content_view.coffee
@@ -8,16 +8,6 @@ if Backbone?
         (event) -> DiscussionUtil.activateOnSpace(event, @toggleFlagAbuse)
   
     attrRenderer:
-      endorsed: (endorsed) ->
-        if endorsed
-          @$(".action-endorse").show().addClass("is-endorsed")
-        else
-          if @model.get('ability')?.can_endorse
-            @$(".action-endorse").show()
-          else
-            @$(".action-endorse").hide()
-          @$(".action-endorse").removeClass("is-endorsed")
-
       closed: (closed) ->
         return if not @$(".action-openclose").length
         return if not @$(".post-status-closed").length
diff --git a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee
index 280ddd0555..80675b118f 100644
--- a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee
+++ b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee
@@ -216,7 +216,7 @@ if Backbone?
       @model.comment()
 
     endorseThread: (endorsed) =>
-      is_endorsed = @$el.find(".is-endorsed").length
+      is_endorsed = @$el.find(".is-endorsed").length > 0
       @model.set 'endorsed', is_endorsed
 
     submitComment: (event) ->
diff --git a/common/static/coffee/src/discussion/views/thread_response_show_view.coffee b/common/static/coffee/src/discussion/views/thread_response_show_view.coffee
index c4e8cec734..8435cb4171 100644
--- a/common/static/coffee/src/discussion/views/thread_response_show_view.coffee
+++ b/common/static/coffee/src/discussion/views/thread_response_show_view.coffee
@@ -12,6 +12,18 @@ if Backbone?
         "keydown .discussion-flag-abuse":
           (event) -> DiscussionUtil.activateOnSpace(event, @toggleFlagAbuse)
 
+    attrRenderer: $.extend({}, DiscussionContentView.prototype.attrRenderer, {
+      endorsed: (endorsed) ->
+        if endorsed
+          @$(".action-endorse").show().addClass("is-endorsed")
+        else
+          if @model.get('ability')?.can_endorse
+            @$(".action-endorse").show()
+          else
+            @$(".action-endorse").hide()
+          @$(".action-endorse").removeClass("is-endorsed")
+    })
+
     $: (selector) ->
         @$el.find(selector)
 
diff --git a/common/static/coffee/src/discussion/views/thread_response_view.coffee b/common/static/coffee/src/discussion/views/thread_response_view.coffee
index 8dc157df9b..fa548df00b 100644
--- a/common/static/coffee/src/discussion/views/thread_response_view.coffee
+++ b/common/static/coffee/src/discussion/views/thread_response_view.coffee
@@ -155,6 +155,7 @@ if Backbone?
       @showView = new ThreadResponseShowView(model: @model)
       @showView.bind "response:_delete", @_delete
       @showView.bind "response:edit", @edit
+      @showView.on "comment:endorse", => @trigger("comment:endorse")
 
     renderShowView: () ->
       @renderSubView(@showView)
diff --git a/lms/static/sass/discussion/elements/_navigation.scss b/lms/static/sass/discussion/elements/_navigation.scss
index 9adf6e6fe6..d7b8a81674 100644
--- a/lms/static/sass/discussion/elements/_navigation.scss
+++ b/lms/static/sass/discussion/elements/_navigation.scss
@@ -179,14 +179,35 @@
   vertical-align: middle;
 }
 
+.forum-nav-thread-wrapper-0 {
+  @extend %forum-nav-thread-wrapper;
+  width: 7%;
+
+  .icon {
+    @include font-size(14);
+  }
+
+  .icon-comments {
+    color: $gray-l2;
+  }
+
+  .icon-ok {
+    color: $forum-color-marked-answer;
+  }
+
+  .icon-question {
+    color: $pink;
+  }
+}
+
 .forum-nav-thread-wrapper-1 {
   @extend %forum-nav-thread-wrapper;
-  width: 70%;
+  width: 80%;
 }
 
 .forum-nav-thread-wrapper-2 {
   @extend %forum-nav-thread-wrapper;
-  width: 30%;
+  width: 13%;
   text-align: right;
 }
 
@@ -252,11 +273,6 @@
   }
 }
 
-.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;
 }
diff --git a/lms/static/sass/discussion/utilities/_variables.scss b/lms/static/sass/discussion/utilities/_variables.scss
index 4009d74d3b..3c5f1a8b23 100644
--- a/lms/static/sass/discussion/utilities/_variables.scss
+++ b/lms/static/sass/discussion/utilities/_variables.scss
@@ -3,3 +3,4 @@ $forum-color-pinned: $pink;
 $forum-color-following: $blue;
 $forum-color-staff: $blue;
 $forum-color-community-ta: $green-d1;
+$forum-color-marked-answer: $green-d1;
diff --git a/lms/templates/discussion/_underscore_templates.html b/lms/templates/discussion/_underscore_templates.html
index c801a6180b..86439bede5 100644
--- a/lms/templates/discussion/_underscore_templates.html
+++ b/lms/templates/discussion/_underscore_templates.html
@@ -264,7 +264,30 @@
 <script aria-hidden="true" type="text/template" id="thread-list-item-template">
   <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">
+      <div class="forum-nav-thread-wrapper-0">
+        ${u"""<%
+        var icon_class, sr_text;
+        if (thread_type == "discussion") {{
+            icon_class = "icon-comments";
+            sr_text = "{discussion}";
+        }} else if (endorsed) {{
+            icon_class = "icon-ok";
+            sr_text = "{answered_question}";
+        }} else {{
+            icon_class = "icon-question";
+            sr_text = "{unanswered_question}";
+        }}
+        %>""".format(
+            ## Translators: This is a label for a Discussion forum thread
+            discussion=escapejs(_("discussion")),
+            ## Translators: This is a label for a Question forum thread with a marked answer
+            answered_question=escapejs(_("answered question")),
+            ## Translators: This is a label for a Question forum thread without a marked answer
+            unanswered_question=escapejs(_("unanswered question"))
+        )}
+        <span class="sr">${"<%= sr_text %>"}</span>
+        <i class="icon ${"<%= icon_class %>"}"></i>
+      </div><div class="forum-nav-thread-wrapper-1">
         <span class="forum-nav-thread-title">${"<%- title %>"}</span>
         <%
         js_block = u"""
@@ -297,10 +320,6 @@
          %>
         ${"<%"}${js_block}${"%>"}
       </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(