From 4454c819c848d8058fd2d85763161bd901a05845 Mon Sep 17 00:00:00 2001
From: Chris Rodriguez <github@chrisrodriguez.me>
Date: Wed, 30 Dec 2015 12:54:53 -0500
Subject: [PATCH] LMS: forum markup tweaks and cleanup

---
 .../discussion_thread_list_view_spec.coffee   | 16 ++--
 .../discussion/discussion-home.underscore     | 32 ++++----
 .../templates/discussion/new-post.underscore  |  4 +-
 .../discussion/thread-type.underscore         |  4 +-
 .../templates/discussion/topic.underscore     |  6 +-
 .../sass/discussion/elements/_navigation.scss | 74 +++++++++++++------
 .../discussion/views/_create-edit-post.scss   |  1 +
 .../discussion/_filter_dropdown.html          | 13 ++--
 .../discussion/_thread_list_template.html     | 17 +++--
 9 files changed, 102 insertions(+), 65 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 d88f3367dc..af89d01a6c 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,17 +6,17 @@ describe "DiscussionThreadListView", ->
         appendSetFixtures("""
         <script type="text/template" id="thread-list-template">
             <div class="forum-nav-header">
-                <a href="#" class="forum-nav-browse" aria-haspopup="true">
-                    <i class="icon fa fa-bars"></i>
-                    <span class="sr">Discussion topics; current selection is: </span>
+                <button type="button" class="forum-nav-browse" id="forum-nav-browse" aria-haspopup="true">
+                    <span class="icon fa fa-bars" aria-hidden="true"></span>
+                    <span class="sr">Discussion topics; currently listing: </span>
                     <span class="forum-nav-browse-current">All Discussions</span>
                     ▾
-                </a>
+                </button>
                 <form class="forum-nav-search">
                     <label>
-                        <span class="sr">Search</span>
-                        <input class="forum-nav-search-input" type="text" placeholder="Search all posts">
-                        <i class="icon fa fa-search"></i>
+                        <span class="sr">Search all posts</span>
+                        <input class="forum-nav-search-input" id="forum-nav-search" type="text" placeholder="Search all posts">
+                        <span class="icon fa fa-search" aria-hidden="true"></span>
                     </label>
                 </form>
             </div>
@@ -66,7 +66,7 @@ describe "DiscussionThreadListView", ->
                     </li>
                 </ul>
             </div>
-            <div class="forum-nav-thread-list-wrapper">
+            <div class="forum-nav-thread-list-wrapper" id="sort-filter-wrapper" tabindex="-1">
                 <div class="forum-nav-refine-bar">
                     <label class="forum-nav-filter-main">
                         <select class="forum-nav-filter-main-control">
diff --git a/common/static/common/templates/discussion/discussion-home.underscore b/common/static/common/templates/discussion/discussion-home.underscore
index 85c4b59220..e07b7c49a8 100644
--- a/common/static/common/templates/discussion/discussion-home.underscore
+++ b/common/static/common/templates/discussion/discussion-home.underscore
@@ -15,43 +15,43 @@
     </span>
     <table class="home-helpgrid">
       <tr class="helpgrid-row helpgrid-row-navigation">
-        <td class="row-title"><%- gettext("Find discussions") %></td>
+        <th scope="row" class="row-title"><%- gettext("Find discussions") %></td>
         <td class="row-item">
-          <i class="icon fa fa-reorder"></i>
-          <span class="row-description"><%- gettext("Focus in on specific topics") %></span>
+          <span class="icon fa fa-reorder" aria-hidden="true"></span>
+          <span class="row-description"><%- gettext("Use the Discussion Topics menu to find specific topics.") %></span>
         </td>
         <td class="row-item">
-          <i class="icon fa fa-search"></i>
-          <span class="row-description"><%- gettext("Search for specific posts") %></span>
+          <span class="icon fa fa-search" aria-hidden="true"></span>
+          <span class="row-description"><%- gettext("Search all posts") %></span>
         </td>
         <td class="row-item">
-          <i class="icon fa fa-sort"></i>
-          <span class="row-description"><%- gettext("Sort by date, vote, or comments") %></span>
+          <span class="icon fa fa-sort" aria-hidden="true"></span>
+          <span class="row-description"><%- gettext("Filter and sort topics") %></span>
         </td>
       </tr>
       <tr class="helpgrid-row helpgrid-row-participation">
-        <td class="row-title"><%- gettext("Engage with posts") %></td>
+        <th scope="row" class="row-title"><%- gettext("Engage with posts") %></td>
         <td class="row-item">
-          <i class="icon fa fa-plus"></i>
-          <span class="row-description"><%- gettext("Upvote posts and good responses") %></span>
+          <span class="icon fa fa-plus" aria-hidden="true"></span>
+          <span class="row-description"><%- gettext("Vote for good posts and responses") %></span>
         </td>
         <td class="row-item">
-          <i class="icon fa fa-flag"></i>
-          <span class="row-description"><%- gettext("Report Forum Misuse") %></span>
+          <span class="icon fa fa-flag" aria-hidden="true"></span>
+          <span class="row-description"><%- gettext("Report abuse, topics, and responses") %></span>
         </td>
         <td class="row-item">
-          <i class="icon fa fa-star"></i>
-          <span class="row-description"><%- gettext("Follow posts for updates") %></span>
+          <span class="icon fa fa-star" aria-hidden="true"></span>
+          <span class="row-description"><%- gettext("Follow or unfollow posts") %></span>
         </td>
       </tr>
       <tr class="helpgrid-row helpgrid-row-notification">
-        <td class="row-title"><%- gettext('Receive updates') %></td>
+        <th scope="row" class="row-title"><%- gettext('Receive updates') %></td>
         <td class="row-item-full" colspan="3">
           <label for="email-setting-checkbox">
             <span class="sr"><%- gettext("Toggle Notifications Setting") %></span>
             <span class="notification-checkbox">
               <input type="checkbox" id="email-setting-checkbox" class="email-setting" name="email-notification"/>
-              <i class="icon fa fa-envelope"></i>
+              <span class="icon fa fa-envelope" aria-hidden="true"></span>
             </span>
           </label>
           <span class="row-description"><%- gettext("Check this box to receive an email digest once a day notifying you about new, unread activity from posts you are following.") %></span>
diff --git a/common/static/common/templates/discussion/new-post.underscore b/common/static/common/templates/discussion/new-post.underscore
index 7f155841f7..1bdc39457f 100644
--- a/common/static/common/templates/discussion/new-post.underscore
+++ b/common/static/common/templates/discussion/new-post.underscore
@@ -30,7 +30,7 @@
     <div class="post-options">
         <label class="post-option is-enabled">
             <input type="checkbox" name="follow" class="post-option-input js-follow" checked>
-            <i class="icon fa fa-star"></i><%- gettext("follow this post") %>
+            <span class="icon fa fa-star" aria-hidden="true"></span><%- gettext("follow this post") %>
         </label>
         <% if (allow_anonymous) { %>
         <label class="post-option">
@@ -47,6 +47,6 @@
     </div>
     <div>
         <input type="submit" class="submit" value="<%- gettext('Add Post') %>">
-        <a href="#" class="cancel"><%- gettext('Cancel') %></a>
+        <button type="button" class="cancel"><%- gettext('Cancel') %></button>
     </div>
 </form>
diff --git a/common/static/common/templates/discussion/thread-type.underscore b/common/static/common/templates/discussion/thread-type.underscore
index 4ba9fa4d70..0fc347cd52 100644
--- a/common/static/common/templates/discussion/thread-type.underscore
+++ b/common/static/common/templates/discussion/thread-type.underscore
@@ -6,13 +6,13 @@
         </span><fieldset class="field-input"><legend class="sr"><%- gettext("Post type:") %></legend>
             <input aria-describedby="field_help_post_type" type="radio" name="<%= form_id %>-post-type" class="post-type-input" id="<%= form_id %>-post-type-question" value="question">
             <label for="<%= form_id %>-post-type-question" class="post-type-label">
-                <i class="icon fa fa-question"></i>
+                <span class="icon fa fa-question" aria-hidden="true"></span>
                 <% // Translators: This is a forum post type %>
                 <%- gettext("Question") %>
             </label>
             <input aria-describedby="field_help_post_type" type="radio" name="<%= form_id %>-post-type" class="post-type-input" id="<%= form_id %>-post-type-discussion" value="discussion" checked>
             <label for="<%= form_id %>-post-type-discussion" class="post-type-label">
-                <i class="icon fa fa-comments"></i>
+                <span class="icon fa fa-comments" aria-hidden="true"></span>
                 <% // Translators: This is a forum post type %>
                 <%- gettext("Discussion") %>
             </label>
diff --git a/common/static/common/templates/discussion/topic.underscore b/common/static/common/templates/discussion/topic.underscore
index b59fd8e771..c8a99f30c3 100644
--- a/common/static/common/templates/discussion/topic.underscore
+++ b/common/static/common/templates/discussion/topic.underscore
@@ -1,11 +1,11 @@
 <% // Using div here instead of label because we are using a non-native control %>
 <div class="field-label">
     <span class="field-label-text"><%- gettext("Topic Area:") %></span><div class="field-input post-topic">
-        <a href="#" class="post-topic-button">
-            <span class="sr"><%- gettext("Discussion topics; current selection is: ") %></span>
+        <button type="button" class="post-topic-button">
+            <span class="sr"><%- gettext("Discussion topics; currently listing: ") %></span>
             <span class="js-selected-topic"></span>
             <span class="drop-arrow" aria-hidden="true">▾</span>
-        </a>
+        </button>
         <div class="topic-menu-wrapper">
             <label class="topic-filter-label">
                 <span class="sr"><%- gettext("Filter topics") %></span>
diff --git a/lms/static/sass/discussion/elements/_navigation.scss b/lms/static/sass/discussion/elements/_navigation.scss
index da12615f49..4a988021d0 100644
--- a/lms/static/sass/discussion/elements/_navigation.scss
+++ b/lms/static/sass/discussion/elements/_navigation.scss
@@ -22,15 +22,25 @@
 }
 
 .forum-nav-browse {
-  @include box-sizing(border-box);
-  display: table-cell;
-  vertical-align: middle;
-  width: 50%;
-  padding: ($baseline/4);
+    @include box-sizing(border-box);
+    display: table-cell;
+    vertical-align: middle;
+    width: auto;
+    padding: 11px;
+    border: 0;
+    border-radius: 0;
+    background: transparent;
+    box-shadow: none;
 
-  &:hover, &:focus, &.is-active {
-    background-color: $gray-l5;
-  }
+    &:hover,
+    &:focus,
+    &.is-active {
+        // converted to button from a;
+        // need importants to overwrite later button styles
+        box-shadow: none !important;
+        background-image: none !important;
+        background-color: $gray-l5 !important;
+    }
 
   .icon {
     @include font-size(14);
@@ -93,6 +103,33 @@
   width: 100%;
 }
 
+.forum-nav-browse-title {
+    display: block;
+    width: 100%;
+    border: 0;
+    border-radius: 0;
+    border-bottom: 1px solid $gray-l3;
+    padding: ($baseline/2) ($baseline/2);
+    background: transparent;
+    box-shadow: none;
+    background-image: none;
+    color: $link-color;
+    text-align: left;
+
+    &:hover,
+    &:focus {
+        // switched from a to button;
+        // need to override button styles
+        background-image: none !important;
+        box-shadow: none !important;
+        background: $forum-color-active-thread !important;
+    }
+}
+
+.forum-nav-browse-title .icon {
+  @include margin-right($baseline/2);
+}
+
 .forum-nav-browse-menu {
   @include font-size(14);
   overflow-y: scroll;
@@ -100,22 +137,15 @@
 }
 
 .forum-nav-browse-submenu {
-  padding-left: $baseline;
-  list-style: none;
-}
+    list-style: none;
+    padding: 0;
 
-.forum-nav-browse-title {
-  display: block;
-  border-bottom: 1px solid $gray-l3;
-  padding: ($baseline/2) ($baseline/2);
+    li {
 
-  &:hover, &:focus {
-    background: $forum-color-active-thread;
-  }
-}
-
-.forum-nav-browse-title .icon {
-  @include margin-right($baseline/2);
+        .forum-nav-browse-title {
+            padding-left: $baseline;
+        }
+    }
 }
 
 // -------------------
diff --git a/lms/static/sass/discussion/views/_create-edit-post.scss b/lms/static/sass/discussion/views/_create-edit-post.scss
index 44e481a977..e8fc558296 100644
--- a/lms/static/sass/discussion/views/_create-edit-post.scss
+++ b/lms/static/sass/discussion/views/_create-edit-post.scss
@@ -70,6 +70,7 @@
     @extend %cont-truncated;
     z-index: 1000;
     padding: 0 $baseline 0 ($baseline*0.75);
+    width: 100%;
     height: 40px;
     font-size: 14px;
     line-height: 36px;
diff --git a/lms/templates/discussion/_filter_dropdown.html b/lms/templates/discussion/_filter_dropdown.html
index 76f58f9326..dd2245ffb3 100644
--- a/lms/templates/discussion/_filter_dropdown.html
+++ b/lms/templates/discussion/_filter_dropdown.html
@@ -19,13 +19,13 @@ import json
         data-discussion-id='${entries[entry]["id"]}'
         data-cohorted="${str(entries[entry]['is_cohorted']).lower()}"
     >
-        <a href="#" class="forum-nav-browse-title">${entry}</a>
+        <button type="button" class="forum-nav-browse-title">${entry}</button>
     </li>
 </%def>
 
 <%def name="render_category(categories, category)">
     <li class="forum-nav-browse-menu-item">
-        <a href="#" class="forum-nav-browse-title">${category}</a>
+        <button type="button" class="forum-nav-browse-title">${category}</button>
         <ul class="forum-nav-browse-submenu">
             ${render_dropdown(categories[category])}
         </ul>
@@ -37,15 +37,18 @@ import json
         <label>
             <span class="sr">${_("Filter Topics")}</span>
             <input type="text" class="forum-nav-browse-filter-input" placeholder="${_("filter topics")}">
-            <i class="icon fa fa-filter"></i>
+            <span class="icon fa fa-filter" aria-hidden="true"></span>
         </label>
     </form>
     <ul class="forum-nav-browse-menu">
         <li class="forum-nav-browse-menu-item forum-nav-browse-menu-all">
-            <a href="#" class="forum-nav-browse-title">${_("All Discussions")}</a>
+            <button type="button" class="forum-nav-browse-title">${_("All Discussions")}</button>
         </li>
         <li class="forum-nav-browse-menu-item forum-nav-browse-menu-following">
-            <a href="#" class="forum-nav-browse-title"><i class="icon fa fa-star"></i>${_("Posts I'm Following")}</a>
+            <button type="button" class="forum-nav-browse-title">
+                <span class="icon fa fa-star" aria-hidden="true"></span>
+                ${_("Posts I'm Following")}
+            </button>
         </li>
         ${render_dropdown(category_map)}
     </ul>
diff --git a/lms/templates/discussion/_thread_list_template.html b/lms/templates/discussion/_thread_list_template.html
index 0139d0b4a4..4af6dbe7c7 100644
--- a/lms/templates/discussion/_thread_list_template.html
+++ b/lms/templates/discussion/_thread_list_template.html
@@ -1,24 +1,27 @@
 <%! from django.utils.translation import ugettext as _ %>
 <script type="text/template" id="thread-list-template">
     <div class="forum-nav-header">
-        <a href="#" class="forum-nav-browse" aria-haspopup="true">
+        <button type="button" class="forum-nav-browse" id="forum-nav-browse" aria-haspopup="true">
             ## There is no whitespace between these because the front-end JS code
             ## needs to precisely compute the available width for forum-nav-
             ## browse-current in order to do truncation of topic names.
-            <i class="icon fa fa-reorder"></i><span class="sr">${_("Discussion topics; current selection is: ")}</span><span class="forum-nav-browse-current">${_("All Discussions")}</span><span class="forum-nav-browse-drop-arrow">▾</span>
-        </a>
+            <span class="icon fa fa-reorder" aria-hidden="true"></span>
+            <span class="sr">${_("Discussion topics; currently listing: ")}</span>
+            <span class="forum-nav-browse-current">${_("All Discussions")}</span>
+            <span class="forum-nav-browse-drop-arrow" aria-hidden="true">▾</span>
+        </button>
         <form class="forum-nav-search">
             <div class="forum-nav-search-ff-position-fix">
                 <label>
-                    <span class="sr">${_("Search")}</span>
-                    <input class="forum-nav-search-input" type="text" placeholder="${_("Search all posts")}">
-                    <i class="icon fa fa-search"></i>
+                    <span class="sr">${_("Search all posts")}</span>
+                    <input class="forum-nav-search-input" id="forum-nav-search" type="text" placeholder="${_("Search all posts")}">
+                    <span class="icon fa fa-search" aria-hidden="true"></span>
                 </label>
             </div>
         </form>
     </div>
     <%include file="_filter_dropdown.html" />
-    <div class="forum-nav-thread-list-wrapper">
+    <div class="forum-nav-thread-list-wrapper" id="sort-filter-wrapper" tabindex="-1">
         <div class="forum-nav-refine-bar">
             <label class="forum-nav-filter-main">
                 ## Translators: This labels a filter menu in forum navigation