Big refactor. The RosterView is no longer an overview.

This is because we can no longer assume a one to one mapping between roster
contacts and their views. Roster contacts can belong to more than one group,
each group needs to show the contact, which means we need a view for each group
the contact belongs to.

updates #83

converse.js

@@ -2724,9 +2724,7 @@
             },
 
             toggle: function (ev) {
-                if (ev && ev.preventDefault) {
-                    ev.preventDefault();
-                }
+                if (ev && ev.preventDefault) { ev.preventDefault(); }
                 this.toggleview.model.save({'collapsed': !this.toggleview.model.get('collapsed')});
                 this.$('.minimized-chats-flyout').toggle();
             },
@@ -2830,6 +2828,9 @@
 
             initialize: function () {
                 this.model.on("change", this.onChange, this);
+                this.model.on("remove", this.remove, this);
+                this.model.on("destroy", this.remove, this);
+                this.model.on("open", this.openChat, this);
             },
 
             onChange: function () {
@@ -2845,7 +2846,7 @@
             },
 
             openChat: function (ev) {
-                ev.preventDefault();
+                if (ev && ev.preventDefault) { ev.preventDefault(); }
                 return converse.chatboxviews.showChat({
                     'id': this.model.get('jid'),
                     'jid': this.model.get('jid'),
@@ -2858,7 +2859,7 @@
             },
 
             removeContact: function (ev) {
-                ev.preventDefault();
+                if (ev && ev.preventDefault) { ev.preventDefault(); }
                 var result = confirm(__("Are you sure you want to remove this contact?"));
                 if (result === true) {
                     var bare_jid = this.model.get('jid');
@@ -3250,7 +3251,7 @@
             }
         });
 
-        this.RosterView = Backbone.Overview.extend({
+        this.RosterView = Backbone.View.extend({
             tagName: 'dl',
             id: 'converse-roster',
             events: {
@@ -3294,6 +3295,7 @@
                 }
                 this.$fragment = $('<span>');
                 this.$fragment.append($(roster_markup));
+                return this;
             },
 
             update: function (item) {
@@ -3301,8 +3303,8 @@
             },
 
             reset: function () {
-                this.removeAll();
-                this.update();
+                this.$el.find('span').empty();
+                this.render().update();
                 return this;
             },
 
@@ -3396,34 +3398,34 @@
                 return $group;
             },
 
-            addCurrentContact: function (view) {
-                var item = view.model;
+            addCurrentContact: function (item) {
                 if (converse.roster_groups) {
                     if (item.get('groups').length === 0) {
-                        this.getRoster().find('.roster-group[data-group="'+HEADER_UNGROUPED+'"]').after(view.el);
+                        this.getRoster().find('.roster-group[data-group="'+HEADER_UNGROUPED+'"]')
+                            .after((new converse.RosterItemView({model: item})).render().el);
                     } else {
                         _.each(item.get('groups'), $.proxy(function (group) {
-                            this.getGroup(group).after(view.el);
+                            // We need a separate view per group
+                            this.getGroup(group).after((new converse.RosterItemView({model: item})).render().el);
                         },this));
                     }
                 } else {
-                    this.getRoster().find('.roster-group[data-group="'+HEADER_CURRENT_CONTACTS+'"]').after(view.el);
+                    this.getRoster().find('.roster-group[data-group="'+HEADER_CURRENT_CONTACTS+'"]')
+                        .after((new converse.RosterItemView({model: item})).render().el);
                 }
             },
 
             addRosterItem: function (item) {
-                var view = new converse.RosterItemView({model: item});
-                this.add(item.id, view);
-                if ((converse.show_only_online_users) && (item.get('chat_status') !== 'online')) {
-                    return this;
-                }
-                view.render()
-                if (view.$el.hasClass('current-xmpp-contact')) {
-                    this.addCurrentContact(view);
-                } else if (view.$el.hasClass('pending-xmpp-contact')) {
+                var view;
+                if (item.get('subscription') === 'both' || item.get('subscription') === 'to') {
+                    this.addCurrentContact(item);
+                } else {
+                    view = (new converse.RosterItemView({model: item})).render();
+                    if ((item.get('ask') === 'subscribe') || (item.get('subscription') === 'from')) {
                         this.getRoster().find('#pending-xmpp-contacts').after(view.el);
-                } else if (view.$el.hasClass('requesting-xmpp-contact')) {
-                    this.getRoster().find('#xmpp-contact-requests').after(view.render().el);
+                    } else if (item.get('requesting') === true) {
+                        this.getRoster().find('#xmpp-contact-requests').after(view.el);
+                    }
                 }
                 return this;
             },
@@ -3496,9 +3498,7 @@
             },
 
             sortRoster: function (chat_status) {
-                /* XXX
-                 * 1). See if the jquery detach method can be somehow used to avoid DOM reflows.
-                 * 2). Likewise see if documentFragment can be used.
+                /* XXX See if the jquery detach method can be somehow used to avoid DOM reflows.
                  */
                 var $el = this.getRoster();
                 $el.find('.roster-group').each($.proxy(function (idx, group) {

spec/chatbox.js

@@ -22,7 +22,7 @@
             });
 
             it("is created when you click on a roster item", $.proxy(function () {
-                var i, $el, click, jid, view, chatboxview;
+                var i, $el, click, jid, chatboxview;
                 // openControlBox was called earlier, so the controlbox is
                 // visible, but no other chat boxes have been created.
                 expect(this.chatboxes.length).toEqual(1);
@@ -33,12 +33,8 @@
                 for (i=0; i<online_contacts.length; i++) {
                     $el = $(online_contacts[i]);
                     jid = $el.text().replace(/ /g,'.').toLowerCase() + '@localhost';
-                    view = this.rosterview.get(jid);
-                    spyOn(view, 'openChat').andCallThrough();
-                    view.delegateEvents(); // We need to rebind all events otherwise our spy won't be called
                     $el.click();
                     chatboxview = this.chatboxviews.get(jid);
-                    expect(view.openChat).toHaveBeenCalled();
                     expect(this.chatboxes.length).toEqual(i+2);
                     expect(this.chatboxviews.trimChats).toHaveBeenCalled();
                     // Check that new chat boxes are created to the left of the
@@ -49,7 +45,7 @@
             }, converse));
 
             it("can be trimmed to conserve space", $.proxy(function () {
-                var i, $el, click, jid, key, view, chatbox, chatboxview;
+                var i, $el, click, jid, key, chatbox, chatboxview;
                 // openControlBox was called earlier, so the controlbox is
                 // visible, but no other chat boxes have been created.
                 var trimmed_chatboxes = converse.minimized_chats;
@@ -64,7 +60,6 @@
                 for (i=0; i<online_contacts.length; i++) {
                     $el = $(online_contacts[i]);
                     jid = $el.text().replace(/ /g,'.').toLowerCase() + '@localhost';
-                    view = this.rosterview.get(jid);
                     $el.click();
                     expect(this.chatboxviews.trimChats).toHaveBeenCalled();
 
@@ -96,7 +91,7 @@
 
             it("is focused if its already open and you click on its corresponding roster item", $.proxy(function () {
                 var contact_jid = mock.cur_names[2].replace(/ /g,'.').toLowerCase() + '@localhost';
-                var i, $el, click, jid, view, chatboxview, chatbox;
+                var i, $el, click, jid, chatboxview, chatbox;
                 // openControlBox was called earlier, so the controlbox is
                 // visible, but no other chat boxes have been created.
                 expect(this.chatboxes.length).toEqual(1);
@@ -105,11 +100,7 @@
                 spyOn(chatboxview, 'focus');
                 $el = this.rosterview.$el.find('a.open-chat:contains("'+chatbox.get('fullname')+'")');
                 jid = $el.text().replace(/ /g,'.').toLowerCase() + '@localhost';
-                view = this.rosterview.get(jid);
-                spyOn(view, 'openChat').andCallThrough();
-                view.delegateEvents(); // We need to rebind all events otherwise our spy won't be called
                 $el.click();
-                expect(view.openChat).toHaveBeenCalled();
                 expect(this.chatboxes.length).toEqual(2);
                 expect(chatboxview.focus).toHaveBeenCalled();
             }, converse));

tests/utils.js

@@ -80,13 +80,21 @@
         var i = 0, jid, views = [];
         for (i; i<amount; i++) {
             jid = mock.cur_names[i].replace(/ /g,'.').toLowerCase() + '@localhost';
-            views[i] = converse.rosterview.get(jid).openChat(mock.event);
+            views[i] = converse.roster.get(jid).trigger("open");
         }
         return views;
     };
 
     utils.openChatBoxFor = function (jid) {
-        return converse.rosterview.get(jid).openChat(mock.event);
+        return converse.roster.get(jid).trigger("open");
+    };
+
+    utils.removeRosterContacts = function () {
+        var model;
+        while (converse.rosterview.model.length) {
+            model = converse.rosterview.model.pop();
+            converse.rosterview.model.remove(model);
+        }
     };
 
     utils.clearBrowserStorage = function () {