Sort chatroom occupants alphabetically and according to role

CHANGES.md

@@ -47,6 +47,7 @@
 - Consolidate error and validation reporting on the registration form.
 - Don't close the emojis panel after inserting an emoji.
 - Focus the message textarea when the emojis panel is opened or closed.
+- MUC chatroom occupants are now sorted alphabetically and according to their roles.
 
 ### Technical changes
 - Converse.js now includes a [Virtual DOM](https://github.com/snabbdom/snabbdom)

css/converse.css

@@ -2605,7 +2605,8 @@
             padding: .5em; }
         #converse-embedded-chat .chatroom .box-flyout .chatroom-body .occupants ul,
         #conversejs .chatroom .box-flyout .chatroom-body .occupants ul {
-          padding: 0.3em 0;
+          padding: 0.5em 0 0 0;
+          margin-bottom: 0.5em;
           overflow-x: hidden;
           overflow-y: auto;
           list-style: none; }

css/inverse.css

@@ -2767,7 +2767,8 @@ body {
             padding: .5em; }
         #converse-embedded-chat .chatroom .box-flyout .chatroom-body .occupants ul,
         #conversejs .chatroom .box-flyout .chatroom-body .occupants ul {
-          padding: 0.3em 0;
+          padding: 0.5em 0 0 0;
+          margin-bottom: 0.5em;
           overflow-x: hidden;
           overflow-y: auto;
           list-style: none; }

sass/_chatrooms.scss

@@ -131,7 +131,8 @@
                         }
                     }
                     ul {
-                        padding: 0.3em 0;
+                        padding: 0.5em 0 0 0;
+                        margin-bottom: 0.5em;
                         overflow-x: hidden;
                         overflow-y: auto;
                         list-style: none;

spec/chatroom.js

@@ -838,12 +838,13 @@
                 test_utils.openAndEnterChatRoom(_converse, 'lounge', 'localhost', 'dummy').then(function() {
                     var name;
                     var view = _converse.chatboxviews.get('lounge@localhost'),
-                        $occupants = view.$('.occupant-list');
-                    var presence, role;
+                        occupants = view.el.querySelector('.occupant-list');
+                    var presence, role, jid, model;
                     for (var i=0; i<mock.chatroom_names.length; i++) {
                         name = mock.chatroom_names[i];
                         role = mock.chatroom_roles[name].role;
                         // See example 21 http://xmpp.org/extensions/xep-0045.html#enter-pres
+                        jid = 
                         presence = $pres({
                                 to:'dummy@localhost/pda',
                                 from:'lounge@localhost/'+name
@@ -855,9 +856,11 @@
                         }).up()
                         .c('status').attrs({code:'110'}).nodeTree;
                         _converse.connection._dataRecv(test_utils.createRequest(presence));
-                        expect($occupants.find('li').length).toBe(2+i);
-                        expect($($occupants.find('li')[i+1]).text()).toBe(mock.chatroom_names[i]);
-                        expect($($occupants.find('li')[i+1]).hasClass('moderator')).toBe(role === "moderator");
+                        expect(occupants.querySelectorAll('li').length).toBe(2+i);
+                        model = view.occupantsview.model.where({'nick': name})[0];
+                        var index = view.occupantsview.model.indexOf(model);
+                        expect(occupants.querySelectorAll('li')[index].textContent).toBe(mock.chatroom_names[i]);
+                        expect($(occupants.querySelectorAll('li')[index]).hasClass('moderator')).toBe(role === "moderator");
                     }
 
                     // Test users leaving the room
@@ -877,7 +880,7 @@
                             role: 'none'
                         }).nodeTree;
                         _converse.connection._dataRecv(test_utils.createRequest(presence));
-                        expect($occupants.find('li').length).toBe(i+1);
+                        expect(occupants.querySelectorAll('li').length).toBe(i+1);
                     }
                     done();
                 });
@@ -907,14 +910,14 @@
 
                     _converse.connection._dataRecv(test_utils.createRequest(presence));
                     var view = _converse.chatboxviews.get('lounge@localhost');
-                    var occupant = view.$el.find('.occupant-list').find('li');
-                    expect(occupant.length).toBe(2);
-                    expect($(occupant).last().text()).toBe("&lt;img src=&quot;x&quot; onerror=&quot;alert(123)&quot;/&gt;");
+                    var occupants = view.el.querySelector('.occupant-list').querySelectorAll('li');
+                    expect(occupants.length).toBe(2);
+                    expect($(occupants).first().text()).toBe("&lt;img src=&quot;x&quot; onerror=&quot;alert(123)&quot;/&gt;");
                     done();
                 });
             }));
 
-            it("indicates moderators by means of a special css class and tooltip",
+            it("indicates moderators and visitors by means of a special css class and tooltip",
                 mock.initConverseWithPromises(
                     null, ['rosterGroupsFetched'], {},
                     function (done, _converse) {
@@ -934,12 +937,33 @@
                     .c('status').attrs({code:'110'}).nodeTree;
 
                     _converse.connection._dataRecv(test_utils.createRequest(presence));
-                    var occupant = view.$el.find('.occupant-list').find('li');
-                    expect(occupant.length).toBe(2);
-                    expect($(occupant).first().text()).toBe("dummy");
-                    expect($(occupant).last().text()).toBe("moderatorman");
-                    expect($(occupant).last().attr('class').indexOf('moderator')).not.toBe(-1);
-                    expect($(occupant).last().attr('title')).toBe(contact_jid + ' This user is a moderator. Click to mention moderatorman in your message.');
+                    var occupants = view.el.querySelector('.occupant-list').querySelectorAll('li');
+                    expect(occupants.length).toBe(2);
+                    expect($(occupants).first().text()).toBe("moderatorman");
+                    expect($(occupants).last().text()).toBe("dummy");
+                    expect($(occupants).first().attr('class').indexOf('moderator')).not.toBe(-1);
+                    expect($(occupants).first().attr('title')).toBe(
+                        contact_jid + ' This user is a moderator. Click to mention moderatorman in your message.'
+                    );
+
+                    contact_jid = mock.cur_names[3].replace(/ /g,'.').toLowerCase() + '@localhost';
+                    presence = $pres({
+                        to:'dummy@localhost/pda',
+                        from:'lounge@localhost/visitorwoman'
+                    }).c('x').attrs({xmlns:'http://jabber.org/protocol/muc#user'})
+                    .c('item').attrs({
+                        jid: contact_jid,
+                        role: 'visitor',
+                    }).up()
+                    .c('status').attrs({code:'110'}).nodeTree;
+                    _converse.connection._dataRecv(test_utils.createRequest(presence));
+
+                    occupants = view.el.querySelector('.occupant-list').querySelectorAll('li');
+                    expect($(occupants).last().text()).toBe("visitorwoman");
+                    expect($(occupants).last().attr('class').indexOf('visitor')).not.toBe(-1);
+                    expect($(occupants).last().attr('title')).toBe(
+                        contact_jid + ' This user can NOT send messages in this room. Click to mention visitorwoman in your message.'
+                    );
                     done();
                 });
             }));

src/converse-muc.js

@@ -70,6 +70,13 @@
     const ROOMS_PANEL_ID = 'chatrooms';
     const CHATROOMS_TYPE = 'chatroom';
 
+    const MUC_ROLE_WEIGHTS = {
+        'moderator':    1,
+        'participant':  2,
+        'visitor':      3,
+        'none':         4,
+    };
+
     const { Strophe, Backbone, Promise, $iq, $build, $msg, $pres, b64_sha1, sizzle, _, moment } = converse.env;
 
     // Add Strophe Namespaces
@@ -502,10 +509,6 @@
                     const model = new _converse.ChatRoomOccupants();
                     model.chatroomview = this;
                     this.occupantsview = new _converse.ChatRoomOccupantsView({'model': model});
-                    const id = b64_sha1(`converse.occupants${_converse.bare_jid}${this.model.get('jid')}`);
-                    this.occupantsview.model.browserStorage = new Backbone.BrowserStorage.session(id);
-                    this.occupantsview.render();
-                    this.occupantsview.model.fetch({add:true});
                     this.occupantsview.model.on('change:role', this.informOfOccupantsRoleChange, this);
                     return this;
                 },
@@ -2096,16 +2099,16 @@
                 }
             });
 
-            _converse.ChatRoomOccupantView = Backbone.View.extend({
+            _converse.ChatRoomOccupantView = Backbone.VDOMView.extend({
                 tagName: 'li',
                 initialize () {
                     this.model.on('change', this.render, this);
                     this.model.on('destroy', this.destroy, this);
                 },
 
-                render () {
+                toHTML () {
                     const show = this.model.get('show') || 'online';
-                    const new_el = tpl_occupant(
+                    return tpl_occupant(
                         _.extend(
                             { 'jid': '',
                               'show': show,
@@ -2114,19 +2117,8 @@
                               'desc_moderator': __('This user is a moderator.'),
                               'desc_occupant': __('This user can send messages in this room.'),
                               'desc_visitor': __('This user can NOT send messages in this room.')
-                            }, this.model.toJSON()
-                        )
+                            }, this.model.toJSON())
                     );
-                    const $parents = this.$el.parents();
-                    if ($parents.length) {
-                        this.$el.replaceWith(new_el);
-                        this.setElement($parents.first().children(`#${this.model.get('id')}`), true);
-                        this.delegateEvents();
-                    } else {
-                        this.$el.replaceWith(new_el);
-                        this.setElement(new_el, true);
-                    }
-                    return this;
                 },
 
                 destroy () {
@@ -2135,7 +2127,19 @@
             });
 
             _converse.ChatRoomOccupants = Backbone.Collection.extend({
-                model: _converse.ChatRoomOccupant
+                model: _converse.ChatRoomOccupant,
+
+                comparator (occupant1, occupant2) {
+                    const role1 = occupant1.get('role') || 'none';
+                    const role2 = occupant2.get('role') || 'none';
+                    if (MUC_ROLE_WEIGHTS[role1] === MUC_ROLE_WEIGHTS[role2]) {
+                        const nick1 = occupant1.get('nick').toLowerCase();
+                        const nick2 = occupant2.get('nick').toLowerCase();
+                        return nick1 < nick2 ? -1 : (nick1 > nick2? 1 : 0);
+                    } else  {
+                        return MUC_ROLE_WEIGHTS[role1] < MUC_ROLE_WEIGHTS[role2] ? -1 : 1;
+                    }
+                },
             });
 
             _converse.ChatRoomOccupantsView = Backbone.Overview.extend({
@@ -2144,6 +2148,11 @@
 
                 initialize () {
                     this.model.on("add", this.onOccupantAdded, this);
+                    this.model.on("change:role", (occupant) => {
+                        this.model.sort();
+                        this.positionOccupant(occupant);
+                    });
+
                     this.chatroomview = this.model.chatroomview;
                     this.chatroomview.model.on('change:open', this.renderInviteWidget, this);
                     this.chatroomview.model.on('change:affiliation', this.renderInviteWidget, this);
@@ -2160,6 +2169,17 @@
                     this.chatroomview.model.on('change:temporary', this.onFeatureChanged, this);
                     this.chatroomview.model.on('change:unmoderated', this.onFeatureChanged, this);
                     this.chatroomview.model.on('change:unsecured', this.onFeatureChanged, this);
+
+                    const id = b64_sha1(`converse.occupants${_converse.bare_jid}${this.chatroomview.model.get('jid')}`);
+                    this.model.browserStorage = new Backbone.BrowserStorage.session(id);
+                    this.render();
+                    this.model.fetch({
+                        'add': true,
+                        'silent': true,
+                        'success': () => {
+                            this.model.each(this.onOccupantAdded.bind(this));
+                        }
+                    });
                 },
 
                 render () {
@@ -2269,6 +2289,36 @@
                         `height: calc(100% - ${el.offsetHeight}px - 5em);`;
                 },
 
+                positionOccupant (occupant) {
+                    /* Positions an occupant correctly in the list of
+                     * occupants.
+                     *
+                     * IMPORTANT: there's an important implicit assumption being
+                     * made here. And that is that initially this method gets called
+                     * for each occupant in the right positional order.
+                     *
+                     * In other words, it gets called for the 0th, then the
+                     * 1st, then the 2nd, 3rd and so on.
+                     *
+                     * That's why we call it in the "success" handler after
+                     * fetching the occupants, so that we know we have ALL of
+                     * them and that they're sorted.
+                     */
+                    const view = this.get(occupant.get('id'));
+                    view.render();
+                    const list = this.el.querySelector('.occupant-list');
+                    const 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 = list.querySelector('li:nth-child('+index+')');
+                        neighbour.insertAdjacentElement('afterend', view.el);
+                    }
+                    return view;
+                },
+
                 onOccupantAdded (item) {
                     let view = this.get(item.get('id'));
                     if (!view) {
@@ -2277,11 +2327,10 @@
                             new _converse.ChatRoomOccupantView({model: item})
                         );
                     } else {
-                        delete view.model; // Remove ref to old model to help garbage collection
                         view.model = item;
                         view.initialize();
                     }
-                    this.$('.occupant-list').append(view.render().$el);
+                    this.positionOccupant(item);
                 },
 
                 parsePresence (pres) {