Some initial refactoring to support roster groups. updates #83

Made sure that the non-group usecase is still covered.

converse.js

@@ -146,13 +146,14 @@
         this.cache_otr_key = false;
         this.debug = false;
         this.default_box_height = 324; // The default height, in pixels, for the control box, chat boxes and chatrooms.
-        this.message_carbons = false;
         this.expose_rid_and_sid = false;
         this.forward_messages = false;
         this.hide_muc_server = false;
         this.i18n = locales.en;
+        this.message_carbons = false;
         this.no_trimming = false; // Set to true for phantomjs tests (where browser apparently has no width)
         this.prebind = false;
+        this.roster_groups = false;
         this.show_controlbox_by_default = false;
         this.show_only_online_users = false;
         this.show_toolbar = true;
@@ -190,10 +191,11 @@
             'fullname',
             'hide_muc_server',
             'i18n',
-            'no_trimming',
             'jid',
+            'no_trimming',
             'prebind',
             'rid',
+            'roster_groups',
             'show_controlbox_by_default',
             'show_only_online_users',
             'show_toolbar',
@@ -2862,6 +2864,7 @@
             render: function () {
                 var item = this.model,
                     ask = item.get('ask'),
+                    chat_status = item.get('chat_status'),
                     requesting  = item.get('requesting'),
                     subscription = item.get('subscription');
 
@@ -2878,7 +2881,7 @@
                         }
                     }, this);
 
-                this.$el.addClass(item.get('chat_status'));
+                this.$el.addClass(chat_status).data('status', chat_status);
 
                 if (ask === 'subscribe') {
                     this.$el.addClass('pending-xmpp-contact');
@@ -2900,7 +2903,7 @@
                     this.$el.addClass('current-xmpp-contact');
                     this.$el.html(converse.templates.roster_item(
                         _.extend(item.toJSON(), {
-                            'desc_status': STATUSES[item.get('chat_status')||'offline'],
+                            'desc_status': STATUSES[chat_status||'offline'],
                             'desc_chat': __('Click to chat with this contact'),
                             'desc_remove': __('Click to remove this contact')
                         })
@@ -3218,39 +3221,45 @@
             initialize: function () {
                 this.model.on("add", function (item) {
                     this.addRosterItemView(item).render(item);
+                    if (item.get('is_last')) {
+                        this.sortRoster().showRoster();
+                    }
                     if (!item.get('vcard_updated')) {
                         // This will update the vcard, which triggers a change
                         // request which will rerender the roster item.
                         converse.getVCard(item.get('jid'));
                     }
                 }, this);
-
                 this.model.on('change', function (item) {
                     if ((_.size(item.changed) === 1) && _.contains(_.keys(item.changed), 'sorted')) {
                         return;
                     }
                     this.updateChatBox(item).render(item);
+                    if (item.changed.chat_status) { // A changed chat status implies a new sort order
+                        this.sortRoster();
+                    }
                 }, this);
-
                 this.model.on("remove", function (item) { this.removeRosterItemView(item); }, this);
                 this.model.on("destroy", function (item) { this.removeRosterItemView(item); }, this);
                 this.model.on("reset", function () { this.removeAllRosterItemViewss(); }, this);
+                this.initRender();
+                this.model.fetch({add: true}); // Get the cached roster items from localstorage
+            },
+
 
+            initRender: function () {
                 var roster_markup = converse.templates.contacts({
-                    'label_contacts': __('My contacts')
+                    'label_contacts': this.roster_groups ? __('Ungrouped') : __('My contacts')
                 });
                 if (converse.allow_contact_requests) {
-                    roster_markup =
-                        converse.templates.requesting_contacts({
+                    roster_markup += converse.templates.requesting_contacts({
                             'label_contact_requests': __('Contact requests')
                         }) +
-                        roster_markup +
                         converse.templates.pending_contacts({
                             'label_pending_contacts': __('Pending contacts')
                         });
                 }
                 this.$el.hide().html(roster_markup);
-                this.model.fetch({add: true}); // Get the cached roster items from localstorage
             },
 
             updateChatBox: function (item, changed) {
@@ -3298,16 +3307,14 @@
                 if ($.contains(document.documentElement, view.el)) {
                     view.render();
                 } else {
-                    this.$el.find('#xmpp-contacts').after(view.render().el);
+                    // FIXME need to choose proper group
+                    this.$el.find('.roster-group').after(view.render().el);
                 }
             },
 
             render: function (item) {
-                var $my_contacts = this.$el.find('#xmpp-contacts'),
-                    $contact_requests = this.$el.find('#xmpp-contact-requests'),
-                    $pending_contacts = this.$el.find('#pending-xmpp-contacts'),
-                    sorted = false,
-                    $count, changed_presence;
+                var $contact_requests = this.$el.find('#xmpp-contact-requests'),
+                    $pending_contacts = this.$el.find('#pending-xmpp-contacts');
                 if (item) {
                     var jid = item.id,
                         view = this.get(item.id),
@@ -3332,51 +3339,81 @@
                     } else if (subscription === 'both' || subscription === 'to') {
                         this.renderRosterItem(item, view);
                     }
-                    changed_presence = item.changed.chat_status;
-                    if (changed_presence) {
-                        this.sortRoster(changed_presence);
-                        sorted = true;
-                    }
-                    if (item.get('is_last')) {
-                        if (!sorted) {
-                            this.sortRoster(item.get('chat_status'));
-                        }
-                        if (!this.$el.is(':visible')) {
-                            // Once all initial roster items have been added, we
-                            // can show the roster.
-                            this.$el.show();
-                        }
-                    }
                 }
-                // Hide the headings if there are no contacts under them
-                _.each([$my_contacts, $contact_requests, $pending_contacts], function (h) {
-                    if (h.nextUntil('dt').length) {
-                        if (!h.is(':visible')) {
-                            h.show();
-                        }
-                    }
-                    else if (h.is(':visible')) {
-                        h.hide();
-                    }
-                });
-                $count = $('#online-count');
+                this.updateCount().toggleHeadings($contact_requests, $pending_contacts);
+                converse.emit('rosterViewUpdated');
+                return this;
+            },
+
+            updateCount: function () {
+                var $count = $('#online-count');
                 $count.text('('+this.model.getNumOnlineContacts()+')');
                 if (!$count.is(':visible')) {
                     $count.show();
                 }
-                converse.emit('rosterViewUpdated');
+                return this;
+            },
+
+            toggleHeadings: function ($contact_requests, $pending_contacts) {
+                var $groups = this.$el.find('.roster-group');
+                // Hide the headings if there are no contacts under them
+                _.each([$groups, $contact_requests, $pending_contacts], function (h) {
+                    var show_or_hide = function (h) {
+                        if (h.nextUntil('dt').length) {
+                            if (!h.is(':visible')) {
+                                h.show();
+                            }
+                        }
+                        else if (h.is(':visible')) { h.hide(); }
+                    };
+                    if (h.length > 1) {
+                        $groups.each(function (idx, group) {
+                            show_or_hide($(group));
+                        });
+                    } else {
+                        show_or_hide(h);
+                    }
+                });
                 return this;
             },
 
             sortRoster: function (chat_status) {
-                var $my_contacts = this.$el.find('#xmpp-contacts');
-                $my_contacts.siblings('dd.current-xmpp-contact.'+chat_status).tsort('a', {order:'asc'});
-                $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.offline'));
-                $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.unavailable'));
-                $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.xa'));
-                $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.away'));
-                $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.dnd'));
-                $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.online'));
+                var sortFunction = function (a, b) {
+                    var a_status = a.s[0],
+                        a_name =a.s[1],
+                        b_status = b.s[0],
+                        b_name =b.s[1],
+                        comp = {
+                        'offline':      6,
+                        'unavailable':  5,
+                        'xa':           4,
+                        'away':         3,
+                        'dnd':          2,
+                        'online':       1
+                    };
+                    if (comp[a_status] === comp[b_status]) {
+                        return a_name < b_name ? -1 : (a_name > b_name ? 1 : 0);
+                    } else  {
+                        return comp[a_status] < comp[b_status] ? -1 : 1;
+                    }
+                };
+                this.$el.find('.roster-group').each(function (idx, group) {
+                    var $group = $(group);
+                    var $contacts = $group.nextUntil('dt', 'dd.current-xmpp-contact');
+                    $group.after($contacts.tsort({sortFunction: sortFunction, data: 'status'}, 'a'));
+                });
+                return this;
+            },
+
+            showRoster: function () {
+                if (!this.$el.is(':visible')) {
+                    // Once all initial roster items have been added, we
+                    // can show the roster.
+                    // TODO: It would be more efficient to use a
+                    // documentFragment and then put that in the DOM
+                    this.$el.show();
+                }
+                return this;
             }
         });
 

less/converse.less

@@ -983,7 +983,7 @@ dl.add-converse-contact {
     height: ~"calc(100% - 70px)";
 }
 
-#converse-roster .roster-group {
+#converse-roster .group-toggle {
     color: #666;
 }
 
@@ -1070,12 +1070,12 @@ dl.add-converse-contact {
     line-height: 16px;
 }
 
-#conversejs .roster-group {
+#conversejs .group-toggle {
     display: block;
     width: 100%;
 }
 
-#conversejs .roster-group:hover,
+#conversejs .group-toggle:hover,
 #conversejs dd.available-chatroom:hover,
 #conversejs #converse-roster dd:hover {
     background-color: #eee;

mockup/controlbox.html

@@ -115,8 +115,8 @@
                     </dd>
                 </dl>
                 <dl id="converse-roster" style="display: block;">
-                    <dt id="xmpp-contacts" style="display: block;">
-                        <a href="#" class="roster-group icon-opened" title="Click to hide these contacts">Colleagues</a>
+                    <dt class="roster-group" style="display: block;">
+                        <a href="#" class="group-toggle icon-opened" title="Click to hide these contacts">Colleagues</a>
                     </dt>
                     <dd class="online current-xmpp-contact">
                         <a class="open-chat" title="Click to chat with this contact" href="#">
@@ -140,8 +140,8 @@
                         <a class="remove-xmpp-contact icon-remove" title="Click to remove this contact" href="#"></a>
                     </dd>
 
-                    <dt id="xmpp-contacts" style="display: block;">
-                        <a href="#" class="roster-group icon-opened" title="Click to hide these contacts">Family</a>
+                    <dt class="roster-group" style="display: block;">
+                        <a href="#" class="group-toggle icon-opened" title="Click to hide these contacts">Family</a>
                     </dt>
                     <dd class="away current-xmpp-contact">
                         <a class="open-chat" title="Click to chat with this contact" href="#">
@@ -158,8 +158,8 @@
                         <a class="remove-xmpp-contact icon-remove" title="Click to remove this contact" href="#"></a>
                     </dd>
 
-                    <dt id="xmpp-contacts" style="display: block;">
-                        <a href="#" class="roster-group icon-opened" title="Click to hide these contacts">Friends</a>
+                    <dt class="roster-group" style="display: block;">
+                        <a href="#" class="group-toggle icon-opened" title="Click to hide these contacts">Friends</a>
                     </dt>
                     <dd class="online current-xmpp-contact">
                         <a class="open-chat" title="Click to chat with this contact" href="#">
@@ -176,8 +176,8 @@
                         <a class="remove-xmpp-contact icon-remove" title="Click to remove this contact" href="#"></a>
                     </dd>
 
-                    <dt id="xmpp-contacts" style="display: block;">
-                        <a href="#" class="roster-group icon-opened" title="Click to hide these contacts">Ungrouped</a>
+                    <dt class="roster-group" style="display: block;">
+                        <a href="#" class="group-toggle icon-opened" title="Click to hide these contacts">Ungrouped</a>
                     </dt>
                     <dd class="online current-xmpp-contact">
                         <a class="open-chat" title="Click to chat with this contact" href="#">
@@ -188,7 +188,7 @@
                     </dd>
 
                     <dt id="xmpp-contact-requests" style="display: block;">
-                        <a href="#" class="roster-group icon-opened" title="Click to hide these contacts">Contact Requests</a>
+                        <a href="#" class="group-toggle icon-opened" title="Click to hide these contacts">Contact Requests</a>
                     </dt>
                     <dd class="offline requesting-xmpp-contact">
                         <span>Bob Skinstad</span>
@@ -206,7 +206,7 @@
                     </dd>
 
                     <dt id="pending-xmpp-contacts" style="display: block;">
-                        <a href="#" class="roster-group icon-opened" title="Click to hide these contacts">Pending Contacts</a>
+                        <a href="#" class="group-toggle icon-opened" title="Click to hide these contacts">Pending Contacts</a>
                     </dt>
                     <dd class="offline pending-xmpp-contact"><span>Rassie Erasmus</span>
                         <a class="remove-xmpp-contact icon-remove" title="Click to remove this contact" href="#"></a>
@@ -302,7 +302,7 @@ $(document).ready(function () {
     }
 
     $(function() {
-        $('.roster-group').click(function(ev) {
+        $('.group-toggle').click(function(ev) {
             ev.preventDefault();
             var $el = $(ev.target);
             $el.parent().nextUntil('dt').slideToggle();

spec/chatbox.js

@@ -29,7 +29,7 @@
                 spyOn(this.chatboxviews, 'trimChats');
                 expect($("#conversejs .chatbox").length).toBe(1); // Controlbox is open
 
-                var online_contacts = this.rosterview.$el.find('dt#xmpp-contacts').siblings('dd.current-xmpp-contact').find('a.open-chat');
+                var online_contacts = this.rosterview.$el.find('dt.roster-group').siblings('dd.current-xmpp-contact').find('a.open-chat');
                 for (i=0; i<online_contacts.length; i++) {
                     $el = $(online_contacts[i]);
                     jid = $el.text().replace(/ /g,'.').toLowerCase() + '@localhost';
@@ -60,7 +60,7 @@
                 expect($("#conversejs .chatbox").length).toBe(1); // Controlbox is open
 
                 // Test that they can be trimmed
-                var online_contacts = this.rosterview.$el.find('dt#xmpp-contacts').siblings('dd.current-xmpp-contact').find('a.open-chat');
+                var online_contacts = this.rosterview.$el.find('dt.roster-group').siblings('dd.current-xmpp-contact').find('a.open-chat');
                 for (i=0; i<online_contacts.length; i++) {
                     $el = $(online_contacts[i]);
                     jid = $el.text().replace(/ /g,'.').toLowerCase() + '@localhost';

spec/controlbox.js

@@ -216,7 +216,7 @@
 
             it("do not have a heading if there aren't any", $.proxy(function () {
                 converse.rosterview.model.reset();
-                expect(this.rosterview.$el.find('dt#xmpp-contacts').css('display')).toEqual('none');
+                expect(this.rosterview.$el.find('dt.roster-group').css('display')).toEqual('none');
             }, converse));
 
             it("can be added to the roster and they will be sorted alphabetically", $.proxy(function () {
@@ -234,14 +234,14 @@
                     });
                     expect(this.rosterview.render).toHaveBeenCalled();
                     expect(converse.emit).toHaveBeenCalledWith('rosterViewUpdated');
-                    // Check that they are sorted alphabetically
-                    t = this.rosterview.$el.find('dt#xmpp-contacts').siblings('dd.current-xmpp-contact.offline').find('a.open-chat').text();
-                    expect(t).toEqual(mock.cur_names.slice(0,i+1).sort().join(''));
                 }
+                // Check that they are sorted alphabetically
+                t = this.rosterview.$el.find('dt.roster-group').siblings('dd.current-xmpp-contact.offline').find('a.open-chat').text();
+                expect(t).toEqual(mock.cur_names.slice(0,i+1).sort().join(''));
             }, converse));
 
             it("will have their own heading once they have been added", $.proxy(function () {
-                expect(this.rosterview.$el.find('dt#xmpp-contacts').css('display')).toEqual('block');
+                expect(this.rosterview.$el.find('dt.roster-group').css('display')).toEqual('block');
             }, converse));
 
             it("can change their status to online and be sorted alphabetically", $.proxy(function () {
@@ -258,7 +258,7 @@
                     expect(this.rosterview.render).toHaveBeenCalled();
                     expect(converse.emit).toHaveBeenCalledWith('rosterViewUpdated');
                     // Check that they are sorted alphabetically
-                    t = this.rosterview.$el.find('dt#xmpp-contacts').siblings('dd.current-xmpp-contact.online').find('a.open-chat').text();
+                    t = this.rosterview.$el.find('dt.roster-group').siblings('dd.current-xmpp-contact.online').find('a.open-chat').text();
                     expect(t).toEqual(mock.cur_names.slice(0,i+1).sort().join(''));
                 }
             }, converse));
@@ -277,7 +277,7 @@
                     expect(this.rosterview.render).toHaveBeenCalled();
                     expect(converse.emit).toHaveBeenCalledWith('rosterViewUpdated');
                     // Check that they are sorted alphabetically
-                    t = this.rosterview.$el.find('dt#xmpp-contacts').siblings('dd.current-xmpp-contact.dnd').find('a.open-chat').text();
+                    t = this.rosterview.$el.find('dt.roster-group').siblings('dd.current-xmpp-contact.dnd').find('a.open-chat').text();
                     expect(t).toEqual(mock.cur_names.slice(0,i+1).sort().join(''));
                 }
             }, converse));
@@ -296,7 +296,7 @@
                     expect(this.rosterview.render).toHaveBeenCalled();
                     expect(converse.emit).toHaveBeenCalledWith('rosterViewUpdated');
                     // Check that they are sorted alphabetically
-                    t = this.rosterview.$el.find('dt#xmpp-contacts').siblings('dd.current-xmpp-contact.away').find('a.open-chat').text();
+                    t = this.rosterview.$el.find('dt.roster-group').siblings('dd.current-xmpp-contact.away').find('a.open-chat').text();
                     expect(t).toEqual(mock.cur_names.slice(0,i+1).sort().join(''));
                 }
             }, converse));
@@ -315,7 +315,7 @@
                     expect(this.rosterview.render).toHaveBeenCalled();
                     expect(converse.emit).toHaveBeenCalledWith('rosterViewUpdated');
                     // Check that they are sorted alphabetically
-                    t = this.rosterview.$el.find('dt#xmpp-contacts').siblings('dd.current-xmpp-contact.xa').find('a.open-chat').text();
+                    t = this.rosterview.$el.find('dt.roster-group').siblings('dd.current-xmpp-contact.xa').find('a.open-chat').text();
                     expect(t).toEqual(mock.cur_names.slice(0,i+1).sort().join(''));
                 }
             }, converse));
@@ -334,7 +334,7 @@
                     expect(this.rosterview.render).toHaveBeenCalled();
                     expect(converse.emit).toHaveBeenCalledWith('rosterViewUpdated');
                     // Check that they are sorted alphabetically
-                    t = this.rosterview.$el.find('dt#xmpp-contacts').siblings('dd.current-xmpp-contact.unavailable').find('a.open-chat').text();
+                    t = this.rosterview.$el.find('dt.roster-group').siblings('dd.current-xmpp-contact.unavailable').find('a.open-chat').text();
                     expect(t).toEqual(mock.cur_names.slice(0, i+1).sort().join(''));
                 }
             }, converse));

src/templates/contacts.html

@@ -1 +1 @@
-<dt id="xmpp-contacts">{{label_contacts}}</dt>
+<dt class="roster-group">{{label_contacts}}</dt>