Collapse multiple, consecutive join/leave messages

CHANGES.md

@@ -31,6 +31,7 @@
   `fullscreen` and `mobile` respectively.
 - Fetch VCard when starting a chat with someone not in the user's roster.
 - Show status messages in an MUC room when a user's role changes.
+- In MUC chat rooms, collapse multiple, consecutive join/leave messages.
 
 ### API changes
 - New API method `_converse.disco.supports` to check whether a certain

spec/chatroom.js

@@ -416,7 +416,7 @@
                         'role': 'participant'
                     });
                 _converse.connection._dataRecv(test_utils.createRequest(presence));
-                expect($chat_content.find('div.chat-info').length).toBe(0);
+                expect($chat_content[0].querySelectorAll('div.chat-info').length).toBe(0);
 
                 /* <presence to="dummy@localhost/_converse.js-29092160"
                  *           from="coven@chat.shakespeare.lit/some1">
@@ -442,16 +442,33 @@
                 presence = $pres({
                         to: 'dummy@localhost/_converse.js-29092160',
                         from: 'coven@chat.shakespeare.lit/newguy'
-                    }).c('x', {xmlns: Strophe.NS.MUC_USER})
+                    })
+                    .c('x', {xmlns: Strophe.NS.MUC_USER})
                     .c('item', {
                         'affiliation': 'none',
                         'jid': 'newguy@localhost/_converse.js-290929789',
                         'role': 'participant'
                     });
                 _converse.connection._dataRecv(test_utils.createRequest(presence));
-                expect($chat_content.find('div.chat-info').length).toBe(2);
+                expect($chat_content[0].querySelectorAll('div.chat-info').length).toBe(2);
                 expect($chat_content.find('div.chat-info:last').html()).toBe("newguy has entered the room.");
 
+                // Add another entrant, otherwise the above message will be
+                // collapsed if "newguy" leaves immediately again
+                presence = $pres({
+                        to: 'dummy@localhost/_converse.js-29092160',
+                        from: 'coven@chat.shakespeare.lit/newgirl'
+                    })
+                    .c('x', {xmlns: Strophe.NS.MUC_USER})
+                    .c('item', {
+                        'affiliation': 'none',
+                        'jid': 'newgirl@localhost/_converse.js-213098781',
+                        'role': 'participant'
+                    });
+                _converse.connection._dataRecv(test_utils.createRequest(presence));
+                expect($chat_content[0].querySelectorAll('div.chat-info').length).toBe(3);
+                expect($chat_content.find('div.chat-info:last').html()).toBe("newgirl has entered the room.");
+
                 // Don't show duplicate join messages
                 presence = $pres({
                         to: 'dummy@localhost/_converse.js-290918392',
@@ -463,12 +480,43 @@
                         'role': 'participant'
                     });
                 _converse.connection._dataRecv(test_utils.createRequest(presence));
-                expect($chat_content.find('div.chat-info').length).toBe(2);
+                expect($chat_content[0].querySelectorAll('div.chat-info').length).toBe(3);
 
+                /*  <presence
+                 *      from='coven@chat.shakespeare.lit/thirdwitch'
+                 *      to='crone1@shakespeare.lit/desktop'
+                 *      type='unavailable'>
+                 *  <status>Disconnected: Replaced by new connection</status>
+                 *  <x xmlns='http://jabber.org/protocol/muc#user'>
+                 *      <item affiliation='member'
+                 *          jid='hag66@shakespeare.lit/pda'
+                 *          role='none'/>
+                 *  </x>
+                 *  </presence>
+                 */
                 presence = $pres({
                         to: 'dummy@localhost/_converse.js-29092160',
                         type: 'unavailable',
                         from: 'coven@chat.shakespeare.lit/newguy'
+                    })
+                    .c('status', 'Disconnected: Replaced by new connection').up()
+                    .c('x', {xmlns: Strophe.NS.MUC_USER})
+                        .c('item', {
+                            'affiliation': 'none',
+                            'jid': 'newguy@localhost/_converse.js-290929789',
+                            'role': 'none'
+                        });
+                _converse.connection._dataRecv(test_utils.createRequest(presence));
+                expect($chat_content.find('div.chat-info').length).toBe(4);
+                expect($chat_content.find('div.chat-info:last').html()).toBe(
+                    'newguy has left the room. '+
+                    '"Disconnected: Replaced by new connection"');
+
+                // When the user immediately joins again, we collapse the
+                // multiple join/leave messages.
+                presence = $pres({
+                        to: 'dummy@localhost/_converse.js-29092160',
+                        from: 'coven@chat.shakespeare.lit/newguy'
                     }).c('x', {xmlns: Strophe.NS.MUC_USER})
                     .c('item', {
                         'affiliation': 'none',
@@ -476,8 +524,69 @@
                         'role': 'participant'
                     });
                 _converse.connection._dataRecv(test_utils.createRequest(presence));
-                expect($chat_content.find('div.chat-info').length).toBe(3);
+                expect($chat_content.find('div.chat-info').length).toBe(4);
-                expect($chat_content.find('div.chat-info:last').html()).toBe("newguy has left the room");
+                var $msg_el = $chat_content.find('div.chat-info:last');
+                expect($msg_el.html()).toBe("newguy has left and re-entered the room.");
+                expect($msg_el.data('leavejoin')).toBe('"newguy"');
+
+                presence = $pres({
+                        to: 'dummy@localhost/_converse.js-29092160',
+                        type: 'unavailable',
+                        from: 'coven@chat.shakespeare.lit/newguy'
+                    })
+                    .c('x', {xmlns: Strophe.NS.MUC_USER})
+                        .c('item', {
+                            'affiliation': 'none',
+                            'jid': 'newguy@localhost/_converse.js-290929789',
+                            'role': 'none'
+                        });
+                _converse.connection._dataRecv(test_utils.createRequest(presence));
+                expect($chat_content.find('div.chat-info').length).toBe(4);
+                $msg_el = $chat_content.find('div.chat-info:last');
+                expect($msg_el.html()).toBe('newguy has left the room.');
+                expect($msg_el.data('leave')).toBe('"newguy"');
+
+                presence = $pres({
+                        to: 'dummy@localhost/_converse.js-29092160',
+                        from: 'coven@chat.shakespeare.lit/nomorenicks'
+                    })
+                    .c('x', {xmlns: Strophe.NS.MUC_USER})
+                    .c('item', {
+                        'affiliation': 'none',
+                        'jid': 'nomorenicks@localhost/_converse.js-290929789',
+                        'role': 'participant'
+                    });
+                _converse.connection._dataRecv(test_utils.createRequest(presence));
+                expect($chat_content[0].querySelectorAll('div.chat-info').length).toBe(5);
+                expect($chat_content.find('div.chat-info:last').html()).toBe("nomorenicks has entered the room.");
+
+                presence = $pres({
+                        to: 'dummy@localhost/_converse.js-290918392',
+                        type: 'unavailable',
+                        from: 'coven@chat.shakespeare.lit/nomorenicks'
+                    }).c('x', {xmlns: Strophe.NS.MUC_USER})
+                    .c('item', {
+                        'affiliation': 'none',
+                        'jid': 'nomorenicks@localhost/_converse.js-290929789',
+                        'role': 'none'
+                    });
+                _converse.connection._dataRecv(test_utils.createRequest(presence));
+                expect($chat_content[0].querySelectorAll('div.chat-info').length).toBe(5);
+                expect($chat_content.find('div.chat-info:last').html()).toBe("nomorenicks has entered and left the room.");
+
+                presence = $pres({
+                        to: 'dummy@localhost/_converse.js-29092160',
+                        from: 'coven@chat.shakespeare.lit/nomorenicks'
+                    })
+                    .c('x', {xmlns: Strophe.NS.MUC_USER})
+                    .c('item', {
+                        'affiliation': 'none',
+                        'jid': 'nomorenicks@localhost/_converse.js-290929789',
+                        'role': 'participant'
+                    });
+                _converse.connection._dataRecv(test_utils.createRequest(presence));
+                expect($chat_content[0].querySelectorAll('div.chat-info').length).toBe(5);
+                expect($chat_content.find('div.chat-info:last').html()).toBe("nomorenicks has entered the room.");
                 done();
             }));
 

src/converse-muc.js

@@ -1799,7 +1799,7 @@
                         return;
                     }
                     _.each(notification.messages, (message) => {
-                        this.$content.append(tpl_info({'message': message}));
+                        this.content.insertAdjacentHTML('beforeend', tpl_info({'message': message, 'data': ''}));
                     });
                     if (notification.reason) {
                         this.showStatusNotification(__('The reason given is: "%1$s".', notification.reason), true);
@@ -1809,30 +1809,83 @@
                     }
                 },
 
-                getJoinLeaveMessages (stanza) {
+                displayJoinNotification (stanza) {
-                    /* Parse the given stanza and return notification messages
-                     * for join/leave events.
-                     */
-                    // XXX: some mangling required to make the returned
-                    // result look like the structure returned by
-                    // parseXUserElement. Not nice...
                     const nick = Strophe.getResourceFromJid(stanza.getAttribute('from'));
                     const stat = stanza.querySelector('status');
-                    if (stanza.getAttribute('type') === 'unavailable') {
+                    const last_el = this.content.querySelector(':last-child');
-                        if (!_.isNull(stat) && stat.textContent) {
+                    if (_.includes(_.get(last_el, 'classList', []), 'chat-info') &&
-                            return [{'messages': [__(nick+' has left the room. "'+stat.textContent+'"')]}];
+                            _.get(last_el, 'dataset', {}).leave === `"${nick}"`) {
+                        last_el.outerHTML = 
+                            tpl_info({
+                                'message': __(nick+' has left and re-entered the room.'),
+                                'data': `data-leavejoin="${nick}"`
+                            });
+                    } else {
+                        let  message = __(nick+' has entered the room.');
+                        if (_.get(stat, 'textContent')) {
+                            message = message + ' "' + stat.textContent + '"';
+                        }
+                        const data = {
+                            'message': message,
+                            'data': `data-join="${nick}"`
+                        };
+                        if (_.includes(_.get(last_el, 'classList', []), 'chat-info') &&
+                            _.get(last_el, 'dataset', {}).joinleave === `"${nick}"`) {
+
+                            last_el.outerHTML = tpl_info(data);
                         } else {
-                            return [{'messages': [__(nick+' has left the room')]}];
+                            this.content.insertAdjacentHTML('beforeend', tpl_info(data));
                         }
                     }
-                    if (!this.occupantsview.model.find({'nick': nick})) {
+                    this.scrollDown();
-                        // Only show join message if we don't already have the
+                },
-                        // occupant model. Doing so avoids showing duplicate
+
-                        // join messages.
+                displayLeaveNotification (stanza) {
-                        if (!_.isNull(stat) && stat.textContent) {
+                    const nick = Strophe.getResourceFromJid(stanza.getAttribute('from'));
-                            return [{'messages': [__(nick+' has entered the room. "'+stat.textContent+'"')]}];
+                    const stat = stanza.querySelector('status');
+                    const last_el = this.content.querySelector(':last-child');
+                    if (_.includes(_.get(last_el, 'classList', []), 'chat-info') &&
+                            _.get(last_el, 'dataset', {}).join === `"${nick}"`) {
+
+                        let message = __('%1$s has entered and left the room.', nick);
+                        if (_.get(stat, 'textContent')) {
+                            message = message + ' "' + stat.textContent + '"';
+                        }
+                        last_el.outerHTML = 
+                            tpl_info({
+                                'message': message,
+                                'data': `data-joinleave="${nick}"`
+                            });
+                    } else {
+                        let  message = __('%1$s has left the room.', nick);
+                        if (_.get(stat, 'textContent')) {
+                            message = message + ' "' + stat.textContent + '"';
+                        }
+                        const data = {
+                            'message': message,
+                            'data': `data-leave="${nick}"`
+                        }
+                        if (_.includes(_.get(last_el, 'classList', []), 'chat-info') &&
+                            _.get(last_el, 'dataset', {}).leavejoin === `"${nick}"`) {
+
+                            last_el.outerHTML = tpl_info(data);
                         } else {
-                            return [{'messages': [__(nick+' has entered the room.')]}];
+                            this.content.insertAdjacentHTML('beforeend', tpl_info(data));
+                        }
+                    }
+                    this.scrollDown();
+                },
+
+                displayJoinOrLeaveNotification (stanza) {
+                    if (stanza.getAttribute('type') === 'unavailable') {
+                        this.displayLeaveNotification(stanza);
+                    } else {
+                        const nick = Strophe.getResourceFromJid(stanza.getAttribute('from'));
+                        if (!this.occupantsview.model.find({'nick': nick})) {
+                            // Only show join message if we don't already have the
+                            // occupant model. Doing so avoids showing duplicate
+                            // join messages.
+                            this.displayJoinNotification(stanza);
                         }
                     }
                 },
@@ -1848,15 +1901,16 @@
                     const elements = sizzle(`x[xmlns="${Strophe.NS.MUC_USER}"]`, stanza);
                     const is_self = stanza.querySelectorAll("status[code='110']").length;
                     const iteratee = _.partial(this.parseXUserElement.bind(this), _, stanza, is_self);
-                    let notifications = _.reject(_.map(elements, iteratee), _.isEmpty);
+                    const notifications = _.reject(_.map(elements, iteratee), _.isEmpty);
-                    if (_.isEmpty(notifications) &&
+                    if (_.isEmpty(notifications)) {
-                            _converse.muc_show_join_leave &&
+                        if (_converse.muc_show_join_leave &&
-                            stanza.nodeName === 'presence' &&
+                                stanza.nodeName === 'presence' &&
-                            this.model.get('connection_status') === converse.ROOMSTATUS.ENTERED
+                                this.model.get('connection_status') === converse.ROOMSTATUS.ENTERED) {
-                        ) {
+                            this.displayJoinOrLeaveNotification(stanza);
-                        notifications = this.getJoinLeaveMessages(stanza);
+                        }
+                    } else {
+                        _.each(notifications, this.displayNotificationsforUser.bind(this));
                     }
-                    _.each(notifications, this.displayNotificationsforUser.bind(this));
                     return stanza;
                 },
 
@@ -2003,8 +2057,12 @@
                     // For translators: the %1$s and %2$s parts will get
                     // replaced by the user and topic text respectively
                     // Example: Topic set by JC Brand to: Hello World!
-                    this.$content.append(
+                    this.content.insertAdjacentHTML(
-                        tpl_info({'message': __('Topic set by %1$s to: %2$s', sender, subject)}));
+                        'beforeend',
+                        tpl_info({
+                            'message': __('Topic set by %1$s to: %2$s', sender, subject),
+                            'data': ''
+                        }));
                     this.scrollDown();
                 },
 

src/templates/info.html

@@ -1 +1 @@
-<div class="chat-info">{{{o.message}}}</div>
+<div class="chat-info" {{{o.data}}}>{{{o.message}}}</div>