Small style changes, add tests

Changelog.md

@@ -16,6 +16,7 @@ Diaspora::Application.config.secret_key_base = '*************...'
 The default for including jQuery from a CDN has changed. If you want to continue to include it from a CDN, please explicitly set the `jquery_cdn` setting to `true` in diaspora.yml.
 
 ## Refactor
+* Redesign contacts page [#5153](https://github.com/diaspora/diaspora/pull/5153)
 
 ## Bug fixes
 * orca cannot see 'Add Contact' button [#5158](https://github.com/diaspora/diaspora/pull/5158)
@@ -105,7 +106,7 @@ Read more in [#4249](https://github.com/diaspora/diaspora/pull/4249) and [#4883]
 * Reorder and reword items on user settings page [#4912](https://github.com/diaspora/diaspora/pull/4912)
 * SPV: Improve padding and interaction counts [#4426](https://github.com/diaspora/diaspora/pull/4426)
 * Remove auto 'mark as read' for notifications [#4810](https://github.com/diaspora/diaspora/pull/4810)
-* Improve set read/unread in notifications dropdown [#4869](https://github.com/diaspora/diaspora/pull/4869) 
+* Improve set read/unread in notifications dropdown [#4869](https://github.com/diaspora/diaspora/pull/4869)
 * Refactor publisher: trigger events for certain actions, introduce 'disabled' state [#4932](https://github.com/diaspora/diaspora/pull/4932)
 
 ## Bug fixes

app/assets/javascripts/app/router.js

@@ -24,7 +24,7 @@ app.Router = Backbone.Router.extend({
     "people/:id": "stream",
     "u/:name": "stream"
   },
-  
+
   initialize: function() {
     // To support encoded linefeeds (%0A) we need to specify
     // our own internal router.route call with the correct regexp.

app/assets/javascripts/app/views/contacts_view.js

@@ -52,46 +52,48 @@ app.views.Contacts = Backbone.View.extend({
   addContactToAspect: function(e){
     var contact = $(e.currentTarget);
     var aspect_membership = new app.models.AspectMembership({
-      'person_id': contact.data('person_id'),
-      'aspect_id': contact.data('aspect_id')
+      'person_id': contact.attr('data-person_id'),
+      'aspect_id': contact.attr('data-aspect_id')
     });
 
-    aspect_membership.save({
+    aspect_membership.save({},{
       success: function(model,response){
         contact.attr('data-membership_id',model.id)
                .tooltip('destroy')
                .removeAttr('data-original-title')
-               .removeClass("contact_add-to_aspect").removeClass("circled-plus")
-               .addClass("contact_remove-from_aspect").addClass("circled-cross")
-               .attr('title', Diaspora.I18n.t('contacts.add_contact'))
+               .removeClass("contact_add-to-aspect").removeClass("circled-plus")
+               .addClass("contact_remove-from-aspect").addClass("circled-cross")
+               .attr('title', Diaspora.I18n.t('contacts.remove_contact'))
                .tooltip()
-               .closest('.stream_element').removeClass('not_in_aspect');
+               .closest('.stream_element').addClass('in_aspect');
       },
       error: function(model,response){
-        alert("SAVE ERROR " + JSON.stringify(model));
+        var msg = Diaspora.I18n.t('contacts.error_add', { 'name':contact.closest('.stream_element').find('.name').text() });
+        Diaspora.page.flashMessages.render({ 'success':false, 'notice':msg });
       }
     });
   },
 
   removeContactFromAspect: function(e){
     var contact = $(e.currentTarget);
     var aspect_membership = new app.models.AspectMembership({
-      'id': contact.data('membership_id')
+      'id': contact.attr('data-membership_id')
     });
 
     aspect_membership.destroy({
       success: function(model,response){
         contact.removeAttr('data-membership_id')
                .tooltip('destroy')
                .removeAttr('data-original-title')
-               .removeClass("contact_remove-from_aspect").removeClass("circled-cross")
-               .addClass("contact_add-to_aspect").addClass("circled-plus")
+               .removeClass("contact_remove-from-aspect").removeClass("circled-cross")
+               .addClass("contact_add-to-aspect").addClass("circled-plus")
                .attr('title', Diaspora.I18n.t('contacts.add_contact'))
                .tooltip()
-               .closest('.stream_element').addClass('not_in_aspect');
+               .closest('.stream_element').removeClass('in_aspect');
       },
       error: function(model,response){
-        alert("DESTROY ERROR " + JSON.stringify(model));
+        var msg = Diaspora.I18n.t('contacts.error_remove', { 'name':contact.closest('.stream_element').find('.name').text() });
+        Diaspora.page.flashMessages.render({ 'success':false, 'notice':msg });
       }
     });
   },

app/assets/javascripts/contact-list.js

@@ -1,8 +1,3 @@
-/*   Copyright (c) 2010-2011, Diaspora Inc.  This file is
- *   licensed under the Affero General Public License version 3 or later.  See
- *   the COPYRIGHT file.
- */
-
 var List = {
   runDelayedSearch: function( searchTerm ) {
     $.getJSON('/people/refresh_search',
@@ -23,27 +18,4 @@ var List = {
   startSearchDelay: function (theSearch) {
     setTimeout( "List.runDelayedSearch('" + theSearch + "')", 10000);
   }
-
 };
-
-$(document).ready(function() {
-  $('.added').bind('ajax:loading', function() {
-    var $this = $(this);
-
-    $this.addClass('disabled');
-    $this.fadeTo(200,0.4);
-  });
-
-  $('.added').bind('hover',
-    function() {
-      var $this = $(this)
-      $this.addClass("remove");
-      $this.children("img").attr("src","/images/icons/monotone_close_exit_delete.png");
-    },
-
-    function() {
-      var $this = $(this)
-      $this.removeClass("remove");
-      $this.children("img").attr("src","/images/icons/monotone_check_yes.png");
-  });
-});

app/assets/javascripts/jasmine-load-all.js

@@ -7,4 +7,5 @@
 //= require mobile
 //= require profile
 //= require people
+//= require contact-list
 //= require sinon

app/assets/stylesheets/aspects.css.scss

@@ -1,5 +1,5 @@
 .aspect_dropdown {
-  
+
   li {
     .status_indicator {
       width: 19px;

app/assets/stylesheets/contacts.css.scss

@@ -1,14 +1,14 @@
 #contacts_container {
   #people_stream.contacts {
-    .header { 
+    .header {
       border-bottom: 1px solid $border-grey;
       margin-top: 10px;
       min-height: 53px;
       #change_aspect_name { cursor: pointer; }
-      #aspect_name_form { 
+      #aspect_name_form {
         display: none;
         form { margin: 0px; }
-        input { 
+        input {
           margin-bottom: 0px;
           margin-top: 10px;
         }
@@ -17,8 +17,9 @@
       #contact_list_search {
         margin: 6px 30px 0 0;
         width: 150px;
+        &:focus { width: 250px; }
       }
-      & > a, #aspect_controls > a { 
+      & > a, #aspect_controls > a {
         text-decoration: none;
         margin-right: 25px;
       }
@@ -41,10 +42,13 @@
         color: lighten($black,75%);
         &:hover { color: $black; }
       }
-
-      &.not_in_aspect { background-color: $background-grey; }
+      &.in_aspect {
+        border-left: 3px solid $green;
+        background-color: lighten($green,35%);
+      }
+      &:not(.in_aspect) { border-left: 3px solid $white; }
     }
-    
+
     .no_contacts {
       text-align: center;
       margin-top: 50px;

app/assets/stylesheets/people.css.scss

@@ -20,9 +20,9 @@
       height: 50px;
     }
     .btn-group.aspect_membership_dropdown { margin: 12px 0; }
-    .thats_you { 
+    .thats_you {
       line-height: 50px;
-      margin-right: 20px;
+      margin-right: 10px;
     }
     .info { font-size: 11px; }
   }

app/controllers/aspects_controller.rb

@@ -87,6 +87,7 @@ def toggle_contact_visibility
       @aspect.contacts_visible = true
     end
     @aspect.save
+    render :nothing => true
   end
 
   private

app/controllers/contacts_controller.rb

@@ -34,29 +34,40 @@ def spotlight
   private
 
   def set_up_contacts
-    c = Contact.arel_table
-    @contacts = case params[:set]
-      when "only_sharing"
-        current_user.contacts.only_sharing.to_a.sort_by { |c| c.person.name }
+    type = params[:set].presence
+    type ||= "by_aspect" if params[:a_id].present?
+    type ||= "receiving"
+
+    @contacts = contacts_by_type(type)
+    @contacts_size = @contacts.length
+  end
+
+  def contacts_by_type(type)
+    contacts = case type
       when "all"
-        current_user.contacts.to_a.sort_by { |c| c.person.name }
+        [current_user.contacts]
+      when "only_sharing"
+        [current_user.contacts.only_sharing]
+      when "receiving"
+        [current_user.contacts.receiving]
+      when "by_aspect"
+        @aspect = current_user.aspects.find(params[:a_id])
+        @contacts_in_aspect = @aspect.contacts
+        @contacts_not_in_aspect = current_user.contacts.where.not(contacts: {id: @contacts_in_aspect.pluck(:id) })
+        [@contacts_in_aspect, @contacts_not_in_aspect].map {|relation|
+          relation.includes(:aspect_memberships)
+        }
       else
-        if params[:a_id]
-          @aspect = current_user.aspects.find(params[:a_id])
-          @contacts_in_aspect = @aspect.contacts.includes(:aspect_memberships, :person => :profile).to_a.sort_by { |c| c.person.name }
-          if @contacts_in_aspect.empty?
-            @contacts_not_in_aspect = current_user.contacts.includes(:aspect_memberships, :person => :profile).to_a.sort_by { |c| c.person.name }
-          else
-            @contacts_not_in_aspect = current_user.contacts.where(c[:id].not_in(@contacts_in_aspect.map(&:id))).includes(:aspect_memberships, :person => :profile).to_a.sort_by { |c| c.person.name }
-          end
-          @contacts_in_aspect + @contacts_not_in_aspect
-        else
-          current_user.contacts.receiving.to_a.sort_by { |c| c.person.name }
-        end
-    end
-    @contacts_size = @contacts.length
+        raise ArgumentError, "unknown type #{type}"
+      end
+
+    contacts.map {|relation|
+      relation.includes(:person => :profile).to_a.tap {|contacts|
+        contacts.sort_by! {|contact| contact.person.name }
+      }
+    }.inject(:+)
   end
-  
+
   def set_up_contacts_mobile
     @contacts = case params[:set]
       when "only_sharing"

app/helpers/contacts_helper.rb

@@ -4,9 +4,9 @@ def contact_aspect_dropdown(contact)
 
     if membership
       content_tag(:i, nil, :class => 'entypo circled-cross contact_remove-from-aspect',
-                  :title => t('contacts.index.remove_person_from_aspect', :person_name => contact.person_first_name, :aspect_name => @aspect.name),
+                  :title => t('contacts.index.remove_contact'),
                   'data-aspect_id' => @aspect.id,
-                  'data-person_id' => contact.person_id, 
+                  'data-person_id' => contact.person_id,
                   'data-membership_id' => membership.id )
 
     elsif @aspect.nil?
@@ -16,7 +16,7 @@ def contact_aspect_dropdown(contact)
                            :current_user => current_user }
     else
       content_tag(:i, nil, :class => 'entypo circled-plus contact_add-to-aspect',
-                  :title => t('people.person.add_contact'),
+                  :title => t('contacts.index.add_contact'),
                   'data-aspect_id' => @aspect.id,
                   'data-person_id' => contact.person_id )
     end
@@ -26,7 +26,7 @@ def start_a_conversation_link(aspect, contacts_size)
     suggested_limit = 16
     conv_opts = { class: "conversation_button", rel: "facebox"}
     conv_opts[:title] = t('.many_people_are_you_sure', suggested_limit: suggested_limit) if contacts_size > suggested_limit
-    
+
     link_to new_conversation_path(aspect_id: aspect.id, name: aspect.name), conv_opts do
       content_tag(:i, nil, :class => 'entypo mail contacts-header-icon', :title => t('contacts.index.start_a_conversation'))
     end

app/views/contacts/_aspect_listings.haml

@@ -11,7 +11,7 @@
         = t('contacts.index.my_contacts')
         .badge.badge-default.pull-right
           = my_contacts_count
-    
+
     - all_aspects.each do |aspect|
       %li.aspect{:data => {:aspect_id => aspect.id}, :class => ("active" if params["a_id"].to_i == aspect.id)}
         %a{:href => contacts_path(:a_id => aspect.id)}

app/views/contacts/_contact.html.haml

@@ -1,5 +1,5 @@
 - membership = contact.aspect_memberships.where(:aspect_id => @aspect.id).first unless @aspect.nil?
-.media.stream_element{:id => contact.person_id, :class => ("not_in_aspect" unless membership)}
+.media.stream_element{:id => contact.person_id, :class => ("in_aspect" if membership)}
   .pull-right
     = contact_aspect_dropdown(contact)
   .media-object.pull-left

app/views/contacts/_header.html.haml

@@ -10,7 +10,7 @@
         -else
           %i.entypo.lock.contacts-header-icon{:title => t('aspects.edit.aspect_list_is_not_visible')}
 
-      = link_to @aspect, method: "delete", data: { confirm: t('aspects.edit.confirm_remove_aspect') }, class: 'delete' do
+      = link_to @aspect, method: "delete", data: { confirm: t('aspects.edit.confirm_remove_aspect') }, class: 'delete', id: 'delete_aspect' do
         %i.entypo.trash.contacts-header-icon{:title => t('delete')}
     .pull-right
       = search_field_tag :contact_search, "", id: "contact_list_search", placeholder: t('contacts.index.user_search')

app/views/contacts/index.html.haml

@@ -21,10 +21,5 @@
             %h3
               = t('.no_contacts')
             %p
-              - if @aspect
-                != t('.no_contacts_message_with_aspect',
-                    :community_spotlight => link_to(t('.community_spotlight'), community_spotlight_path),
-                    :add_to_aspect_link => link_to(t('.add_to_aspect_link', :name => @aspect.name), edit_aspect_path(@aspect), :rel => "facebox"))
-              - else
-                != t('.no_contacts_message',
-                     :community_spotlight => link_to(t('.community_spotlight'), community_spotlight_path))
+              != t('.no_contacts_message',
+                   :community_spotlight => link_to(t('.community_spotlight'), community_spotlight_path))

config/locales/diaspora/en.yml

@@ -343,7 +343,8 @@ en:
       my_contacts: "My Contacts"
       all_contacts: "All Contacts"
       only_sharing_with_me: "Only sharing with me"
-      remove_person_from_aspect: "Remove %{person_name} from \"%{aspect_name}\""
+      add_contact: "Add contact"
+      remove_contact: "Remove contact"
       many_people_are_you_sure: "Are you sure you want to start a private conversation with more than %{suggested_limit} contacts? Posting to this aspect may be a better way to contact them."
       user_search: "User Search"
     spotlight:

config/locales/javascript/javascript.en.yml

@@ -43,7 +43,8 @@ en:
       aspect_list_is_visible: "Contacts in this aspect are able to see each other."
       aspect_list_is_not_visible: "Contacts in this aspect are not able to see each other."
       remove_contact: "Remove contact"
-
+      error_add: "Couldn't add <%= name %> to the aspect :("
+      error_remove: "Couldn't remove <%= name %> from the aspect :("
 
     my_activity: "My Activity"
     my_stream: "Stream"

config/routes.rb

@@ -84,7 +84,7 @@
       get :read_all
     end
   end
-  
+
 
   resources :tags, :only => [:index]
 
@@ -221,7 +221,7 @@
 
   #Statistics
   get :statistics, controller: :statistics
-  
+
   # Terms
   if AppConfig.settings.terms.enable?
     get 'terms' => 'terms#index'

features/desktop/contacts.feature

@@ -16,7 +16,7 @@ Feature: show contacts
     And I press the first "a" within ".section.contact_pictures"
     Then I should see "Alice Smith"
 
-  Scenario: see contacts of a visible aspect list 
+  Scenario: see contacts of a visible aspect list
     When I am on "bob@bob.bob"'s page
     And I add the person to my "Unicorns" aspect
     And I sign out
@@ -25,14 +25,12 @@ Feature: show contacts
     And I press the first "a" within ".section.contact_pictures"
     Then I should see "Bob Jones"
 
-  Scenario: don't see contacts of an invisible aspect list 
+  Scenario: don't see contacts of an invisible aspect list
     When I am on "bob@bob.bob"'s page
     And I add the person to my "Unicorns" aspect
     And I am on the contacts page
     And I follow "Unicorns"
-    And I follow "Manage"
-    And I press the first "a.contact_visibility_link" in the modal window
-    And I press "Done" in the modal window
+    And I press the first "a#contacts_visibility_toggle"
     And I sign out
 
     And I sign in as "alice@alice.alice"