From 3909d1498348a90ec98c9099502ff7dbe8bd191c Mon Sep 17 00:00:00 2001
From: Daniel Pan <freeplant@gmail.com>
Date: Sun, 17 Apr 2016 10:23:28 +0800
Subject: [PATCH] Improve group popovers

---
 media/css/seahub.css                          | 18 ++++++++-----
 seahub/templates/js/templates.html            | 26 +++++++++----------
 seahub/templates/libraries.html               | 10 +++----
 static/scripts/app/views/group-discussions.js |  2 ++
 static/scripts/app/views/group-settings.js    |  1 +
 static/scripts/app/views/widgets/popover.js   |  1 +
 6 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/media/css/seahub.css b/media/css/seahub.css
index 543f882d17..e852fcb3d9 100644
--- a/media/css/seahub.css
+++ b/media/css/seahub.css
@@ -1190,12 +1190,23 @@ textarea:-moz-placeholder {/* for FF */
     margin:4px 0 0;
 }
 .popover-con {
-    padding:0 10px;
     overflow:auto;
 }
+a.popover-item {
+    color:#444;
+    font-weight:normal;
+    text-decoration:none;
+    display:block;
+    padding-left:10px;
+    padding-right:10px;
+    line-height:31px;
+    cursor:pointer;
+    margin:5px 0;
+}
 /**** user-item ****/ /* e.g. group member in 'group members' panel */
 .user-item {
     margin:5px 0;
+    padding-left:10px;
 }
 .user-item .txt {
     margin-left:40px;
@@ -3371,11 +3382,6 @@ img.thumbnail {
 .group-setting-list {
     border-bottom:1px solid #e3e3e5;
 }
-.group-setting-item {
-    line-height:31px;
-    cursor:pointer;
-    margin:5px 0;
-}
 #add-group-members-form .submit {
     margin:0 0 0 5px;
 }
diff --git a/seahub/templates/js/templates.html b/seahub/templates/js/templates.html
index 53ae3601a3..138fbce2c2 100644
--- a/seahub/templates/js/templates.html
+++ b/seahub/templates/js/templates.html
@@ -1004,37 +1004,37 @@
 <script type="text/template" id="group-settings-content-tmpl">
 <% if (is_owner) { %>
     <ul class="group-setting-list">
-        <li class="group-setting-item" data-op="rename">{% trans "Rename" %}</li>
-        <li class="group-setting-item" data-op="transfer">{% trans "Transfer" %}</li>
+        <li><a href="#" class="group-setting-item popover-item" data-op="rename">{% trans "Rename" %}</a></li>
+        <li><a href="#" class="group-setting-item popover-item" data-op="transfer">{% trans "Transfer" %}</a></li>
         <% if (!wiki_enabled) { %>
-        <li class="group-setting-item" data-op="add-wiki">{% trans "Add Wiki" %}</li>
+        <li><a href="#" class="group-setting-item popover-item" data-op="add-wiki">{% trans "Add Wiki" %}</a></li>
         <% } else { %>
-        <li class="group-setting-item" data-op="remove-wiki">{% trans "Remove Wiki" %}</li>
+        <li><a href="#" class="group-setting-item popover-item" data-op="remove-wiki">{% trans "Remove Wiki" %}</a></li>
         <% } %>
     </ul>
     <ul class="group-setting-list">
-        <li class="group-setting-item" data-op="import-members">{% trans "Import Members" %}</li>
-        <li class="group-setting-item" data-op="manage-members">{% trans "Manage Members" %}</li>
+        <li><a href="#" class="group-setting-item popover-item" data-op="import-members">{% trans "Import Members" %}</a></li>
+        <li><a href="#" class="group-setting-item popover-item" data-op="manage-members">{% trans "Manage Members" %}</a></li>
     </ul>
     <ul class="last-group-setting-list">
-        <li class="group-setting-item" data-op="dismiss">{% trans "Dismiss" %}</li>
+        <li><a href="#" class="group-setting-item popover-item" data-op="dismiss">{% trans "Dismiss" %}</a></li>
     </ul>
 <% } else if (is_admin) { %>
     <ul class="group-setting-list">
-        <li class="group-setting-item" data-op="rename">{% trans "Rename" %}</li>
+        <li><a href="#" class="group-setting-item popover-item" data-op="rename">{% trans "Rename" %}</a></li>
         <% if (!wiki_enabled) { %>
-        <li class="group-setting-item" data-op="add-wiki">{% trans "Add Wiki" %}</li>
+        <li><a href="#" class="group-setting-item popover-item" data-op="add-wiki">{% trans "Add Wiki" %}</a></li>
         <% } else { %>
-        <li class="group-setting-item" data-op="remove-wiki">{% trans "Remove Wiki" %}</li>
+        <li><a href="#" class="group-setting-item popover-item" data-op="remove-wiki">{% trans "Remove Wiki" %}</a></li>
         <% } %>
     </ul>
     <ul class="last-group-setting-list">
-        <li class="group-setting-item" data-op="import-members">{% trans "Import Members" %}</li>
-        <li class="group-setting-item" data-op="manage-members">{% trans "Manage Members" %}</li>
+        <li><a href="#" class="group-setting-item popover-item" data-op="import-members">{% trans "Import Members" %}</a></li>
+        <li><a href="#" class="group-setting-item popover-item" data-op="manage-members">{% trans "Manage Members" %}</a></li>
     </ul>
 <% } else { %>
     <ul class="last-group-setting-list">
-        <li class="group-setting-item" data-op="leave">{% trans "Leave group" %}</li>
+        <li><a href="#" class="group-setting-item popover-item" data-op="leave">{% trans "Leave group" %}</a></li>
     </ul>
 <% } %>
 </script>
diff --git a/seahub/templates/libraries.html b/seahub/templates/libraries.html
index 27613e7ed9..acc2bf18a3 100644
--- a/seahub/templates/libraries.html
+++ b/seahub/templates/libraries.html
@@ -116,7 +116,7 @@
 <script type="text/template" id="group-settings-tmpl">
     <div class="outer-caret up-outer-caret"><div class="inner-caret"></div></div>
     <div class="popover-hd ovhd">
-        <span class="popover-close close sf2-icon-x1 op-icon fright" title="{% trans "Close" %}"></span>
+        <a href="#" title="{% trans "Close" %}" aria-label="{% trans "Close" %}" class="popover-close close sf2-icon-x1 op-icon fright"></a>
         <h3 class="popover-title">{% trans "Settings" %}</h3>
     </div>
     <div class="popover-con">
@@ -126,11 +126,11 @@
 <script type="text/template" id="group-members-tmpl">
     <div class="outer-caret up-outer-caret"><div class="inner-caret"></div></div>
     <div class="popover-hd ovhd">
-        <span class="popover-close close sf2-icon-x1 op-icon fright" title="{% trans "Close" %}"></span>
+        <a href="#" title="{% trans "Close" %}" aria-label="{% trans "Close" %}" class="popover-close close sf2-icon-x1 op-icon fright"></a>
         <h3 class="popover-title">{% trans "Members" %}</h3>
     </div>
     <div class="popover-con">
-        <span class="loading-icon loading-tip"></span>
+        <div class="loading-icon loading-tip"></div>
         <ul id="group-member-list" class="hide"></ul>
         <p class="error hide"></p>
     </div>
@@ -139,11 +139,11 @@
 <script type="text/template" id="group-discussions-tmpl">
     <div class="outer-caret up-outer-caret"><div class="inner-caret"></div></div>
     <div class="popover-hd ovhd">
-        <span class="popover-close close sf2-icon-x1 op-icon fright" title="{% trans "Close" %}"></span>
+        <a href="#" title="{% trans "Close" %}" aria-label="{% trans "Close" %}" class="popover-close close sf2-icon-x1 op-icon fright"></a>
         <h3 class="popover-title">{% trans "Discussions" %}</h3>
     </div>
     <div class="popover-con">
-        <span class="loading-icon loading-tip"></span>
+        <div class="loading-icon loading-tip"></div>
         <p class="load-more-discussion hide js-load-more">{% trans "More..." %}</p>
         <ul id="group-discussion-list" class="hide"></ul>
         <p class="no-discussion-tip hide">{% trans "No discussion in this group yet." %}</p>
diff --git a/static/scripts/app/views/group-discussions.js b/static/scripts/app/views/group-discussions.js
index 99e9d28e71..d0738297b7 100644
--- a/static/scripts/app/views/group-discussions.js
+++ b/static/scripts/app/views/group-discussions.js
@@ -116,6 +116,7 @@ define([
             });
 
             $("#group").append(this.$el);
+            this.$(".msg-input").focus();
             app.router.navigate('group/' + this.groupView.group.id + '/discussions/');
         },
 
@@ -133,6 +134,7 @@ define([
         hide: function() {
             PopoverView.prototype.hide.call(this);
             app.router.navigate('group/' + this.groupView.group.id + '/');
+            return false;
         },
 
         replyTo: function(to_user) {
diff --git a/static/scripts/app/views/group-settings.js b/static/scripts/app/views/group-settings.js
index 75b570b3aa..7a1a67b6e8 100644
--- a/static/scripts/app/views/group-settings.js
+++ b/static/scripts/app/views/group-settings.js
@@ -92,6 +92,7 @@ define([
                     break;
             }
             this.hide();
+            return false;
         },
 
         rename: function() {
diff --git a/static/scripts/app/views/widgets/popover.js b/static/scripts/app/views/widgets/popover.js
index c46bca8084..cd84c8c662 100644
--- a/static/scripts/app/views/widgets/popover.js
+++ b/static/scripts/app/views/widgets/popover.js
@@ -60,6 +60,7 @@ define([
         hide: function() {
             app.ui.currentPopover = null;
             this.$el.detach();
+            return false;
         },
 
         show: function() {