From f9343594bfbbd3de12f87abeb95676c8e54e155a Mon Sep 17 00:00:00 2001 From: JC Brand Date: Fri, 15 Dec 2017 16:24:30 +0000 Subject: [PATCH] Collapse multiple, consecutive join/leave messages --- CHANGES.md | 1 + spec/chatroom.js | 121 ++++++++++++++++++++++++++++++++++++++-- src/converse-muc.js | 116 ++++++++++++++++++++++++++++---------- src/templates/info.html | 2 +- 4 files changed, 204 insertions(+), 36 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 31214a1f8..62f267896 100755 --- a/CHANGES.md +++ b/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 diff --git a/spec/chatroom.js b/spec/chatroom.js index 93d919396..15cb2d9ba 100644 --- a/spec/chatroom.js +++ b/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); /* @@ -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); + /* + * Disconnected: Replaced by new connection + * + * + * + * + */ 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:last').html()).toBe("newguy has left the room"); + expect($chat_content.find('div.chat-info').length).toBe(4); + 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(); })); diff --git a/src/converse-muc.js b/src/converse-muc.js index aa7f147fe..b482c5809 100755 --- a/src/converse-muc.js +++ b/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) { - /* 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... + displayJoinNotification (stanza) { const nick = Strophe.getResourceFromJid(stanza.getAttribute('from')); const stat = stanza.querySelector('status'); - if (stanza.getAttribute('type') === 'unavailable') { - if (!_.isNull(stat) && stat.textContent) { - return [{'messages': [__(nick+' has left the room. "'+stat.textContent+'"')]}]; + const last_el = this.content.querySelector(':last-child'); + if (_.includes(_.get(last_el, 'classList', []), 'chat-info') && + _.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})) { - // Only show join message if we don't already have the - // occupant model. Doing so avoids showing duplicate - // join messages. - if (!_.isNull(stat) && stat.textContent) { - return [{'messages': [__(nick+' has entered the room. "'+stat.textContent+'"')]}]; + this.scrollDown(); + }, + + displayLeaveNotification (stanza) { + const nick = Strophe.getResourceFromJid(stanza.getAttribute('from')); + 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); - if (_.isEmpty(notifications) && - _converse.muc_show_join_leave && - stanza.nodeName === 'presence' && - this.model.get('connection_status') === converse.ROOMSTATUS.ENTERED - ) { - notifications = this.getJoinLeaveMessages(stanza); + const notifications = _.reject(_.map(elements, iteratee), _.isEmpty); + if (_.isEmpty(notifications)) { + if (_converse.muc_show_join_leave && + stanza.nodeName === 'presence' && + this.model.get('connection_status') === converse.ROOMSTATUS.ENTERED) { + this.displayJoinOrLeaveNotification(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( - tpl_info({'message': __('Topic set by %1$s to: %2$s', sender, subject)})); + this.content.insertAdjacentHTML( + 'beforeend', + tpl_info({ + 'message': __('Topic set by %1$s to: %2$s', sender, subject), + 'data': '' + })); this.scrollDown(); }, diff --git a/src/templates/info.html b/src/templates/info.html index 113033b58..42f9cfd07 100644 --- a/src/templates/info.html +++ b/src/templates/info.html @@ -1 +1 @@ -
{{{o.message}}}
+
{{{o.message}}}