Let `RosterView` also be an `OrderedListView`

package.json

@@ -36,7 +36,7 @@
     "babel-cli": "^7.0.0-beta.3",
     "backbone": "1.3.3",
     "backbone.browserStorage": "0.0.3",
-    "backbone.overview": "0.0.3",
+    "backbone.overview": "jcbrand/Backbone.Overview",
     "backbone.vdomview": "jcbrand/backbone.vdomview",
     "bootstrap": "^3.3.7",
     "bourbon": "^4.3.2",

spec/controlbox.js

@@ -135,7 +135,7 @@
                 fullname: mock.pend_names[0]
             });
             test_utils.waitUntil(function () {
-                return _converse.rosterview.$el.find('.roster-group li').length;
+                return _converse.rosterview.$el.find('.roster-group li:visible').length;
             }, 700).then(function () {
                 // Checking that only one entry is created because both JID is same (Case sensitive check)
                 expect(_converse.rosterview.$el.find('li:visible').length).toBe(1);

spec/protocol.js

@@ -230,7 +230,7 @@
                     // contact in the roster.
                     return test_utils.waitUntil(function () {
                         var $header = $('a:contains("Pending contacts")');
-                        var $contacts = $header.parent().find('li');
+                        var $contacts = $header.parent().find('li:visible');
                         return $contacts.length;
                     }, 600);
                 }).then(function () {
@@ -299,7 +299,7 @@
                     // contact (but still offline).
                     return test_utils.waitUntil(function () {
                         var $header = $('a:contains("My contacts")');
-                        var $contacts = $header.parent().find('li');
+                        var $contacts = $header.parent().find('li:visible');
                         return $contacts.length;
                     }, 600);
                 }).then(function () {
@@ -557,7 +557,7 @@
                 _converse.connection._dataRecv(test_utils.createRequest(stanza));
                 return test_utils.waitUntil(function () {
                     var $header = $('a:contains("Contact requests")');
-                    var $contacts = $header.parent().find('li');
+                    var $contacts = $header.parent().find('li:visible');
                     return $contacts.length;
                 }, 600).then(function () {
                     expect(_converse.emit).toHaveBeenCalledWith('contactRequest', jasmine.any(Object));

src/converse-rosterview.js

@@ -272,7 +272,6 @@
                 }
             });
 
-
             _converse.RosterContactView = Backbone.View.extend({
                 tagName: 'li',
                 className: 'hidden',
@@ -436,18 +435,17 @@
                 }
             });
 
-
             _converse.RosterGroupView = Backbone.OrderedListView.extend({
                 tagName: 'div',
                 className: 'roster-group hidden',
                 events: {
                     "click a.group-toggle": "toggle"
                 },
-                listItems: 'model.contacts',
-                sortEvent: 'change:chat_status',
-                listSelector: '.roster-group-contacts',
 
                 ItemView: _converse.RosterContactView,
+                listItems: 'model.contacts',
+                listSelector: '.roster-group-contacts',
+                sortEvent: 'change:chat_status',
 
                 initialize () {
                     Backbone.OrderedListView.prototype.initialize.apply(this, arguments);
@@ -620,18 +618,33 @@
             });
 
 
-            _converse.RosterView = Backbone.Overview.extend({
+            _converse.RosterView = Backbone.OrderedListView.extend({
                 tagName: 'div',
                 id: 'converse-roster',
 
+                ItemView: _converse.RosterGroupView,
+                listItems: 'model',
+                listSelector: '.roster-contacts',
+                sortEvent: null, // Groups are immutable, so they don't get re-sorted
+                subviewIndex: 'name',
+
                 initialize () {
+                    Backbone.OrderedListView.prototype.initialize.apply(this, arguments);
+
                     _converse.roster.on("add", this.onContactAdded, this);
                     _converse.roster.on('change', this.onContactChange, this);
                     _converse.roster.on("destroy", this.update, this);
                     _converse.roster.on("remove", this.update, this);
-                    this.model.on("add", this.onGroupAdded, this);
+
                     this.model.on("reset", this.reset, this);
-                    _converse.on('rosterGroupsFetched', this.positionFetchedGroups, this);
+
+                    // This event gets triggered once *all* contacts (i.e. not
+                    // just this group's) have been fetched from browser
+                    // storage or the XMPP server and once they've been
+                    // assigned to their various groups.
+                    _converse.on('rosterGroupsFetched', this.sortAndPositionAllItems.bind(this));
+
+                    // _converse.on('rosterGroupsFetched', this.positionFetchedGroups, this);
                     _converse.on('rosterContactsFetched', () => {
                         _converse.roster.each((contact) => {
                             this.addRosterContact(contact, {'silent': true});
@@ -735,12 +748,6 @@
                     return this;
                 },
 
-                onGroupAdded (group) {
-                    const view = new _converse.RosterGroupView({model: group});
-                    this.add(group.get('name'), view);
-                    this.positionGroup(group);
-                },
-
                 onContactAdded (contact) {
                     this.addRosterContact(contact).update();
                     this.updateFilter();
@@ -780,41 +787,6 @@
                     return this;
                 },
 
-                positionFetchedGroups () {
-                    /* Instead of throwing an add event for each group
-                     * fetched, we wait until they're all fetched and then
-                     * we position them.
-                     * Works around the problem of positionGroup not
-                     * working when all groups besides the one being
-                     * positioned aren't already in inserted into the
-                     * roster DOM element.
-                     */
-                    this.model.sort();
-                    this.model.each(this.onGroupAdded.bind(this));
-                },
-
-                positionGroup (group) {
-                    /* Place the group's DOM element in the correct alphabetical
-                     * position amongst the other groups in the roster.
-                     *
-                     * NOTE: relies on the assumption that it will be called in
-                     * the right order of appearance of groups.
-                     */
-                    const view = this.get(group.get('name'));
-                    view.render();
-                    const list = this.roster_el,
-                          index = this.model.indexOf(view.model);
-                    if (index === 0) {
-                        list.insertAdjacentElement('afterbegin', view.el);
-                    } else if (index === (this.model.length-1)) {
-                        list.insertAdjacentElement('beforeend', view.el);
-                    } else {
-                        const neighbour_el = list.querySelector('div:nth-child('+index+')');
-                        neighbour_el.insertAdjacentElement('afterend', view.el);
-                    }
-                    return this;
-                },
-
                 getGroup (name) {
                     /* Returns the group as specified by name.
                      * Creates the group if it doesn't exist.