From cc29d1692953f5a82099e1f6d335ea373d2f70c1 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Wed, 3 Jan 2018 11:54:28 +0000 Subject: [PATCH 1/3] Simplify message insertion into the chat area --- src/converse-chatview.js | 48 +++++++++------------------------------- 1 file changed, 10 insertions(+), 38 deletions(-) diff --git a/src/converse-chatview.js b/src/converse-chatview.js index 33d651d0f..c566c0395 100644 --- a/src/converse-chatview.js +++ b/src/converse-chatview.js @@ -399,22 +399,6 @@ })); }, - insertMessage (attrs, insert_method) { - /* Helper method which appends a message (or prepends if the - * 2nd parameter is set to true) to the end of the chat box's - * content area. - * - * Parameters: - * (Object) attrs: An object containing the message attributes. - */ - _.flow((el) => { - insert_method(el); - return el; - }, - this.scrollDown.bind(this) - )(this.renderMessage(attrs)); - }, - getLastMessageElement () { let last_msg_el = this.content.lastElementChild; while (!_.isNull(last_msg_el) && @@ -488,37 +472,25 @@ */ const current_msg_date = moment(attrs.time) || moment, prepend_html = _.bind(this.content.insertAdjacentHTML, this.content, 'afterbegin'), - previous_msg_date = this.getLastMessageDate(current_msg_date); + previous_msg_date = this.getLastMessageDate(current_msg_date), + message_el = this.renderMessage(attrs); if (_.isNull(previous_msg_date)) { - this.insertMessage(attrs, _.bind(this.content.insertAdjacentElement, this.content, 'afterbegin')); + this.content.insertAdjacentElement('afterbegin', message_el); this.insertDayIndicator(current_msg_date, prepend_html); } else { const previous_msg_el = sizzle(`[data-isodate="${previous_msg_date}"]:last`, this.content).pop(); + previous_msg_el.insertAdjacentElement('afterend', message_el); + const day_el = this.getDayIndicatorElement(current_msg_date) - if (current_msg_date.isAfter(previous_msg_date, 'day')) { - if (_.isNull(day_el)) { - this.insertMessage( - attrs, - _.bind(previous_msg_el.insertAdjacentElement, previous_msg_el, 'afterend') - ); - this.insertDayIndicator( - current_msg_date, - _.bind(this.content.insertAdjacentHTML, previous_msg_el, 'afterend') - ); - } else { - this.insertMessage( - attrs, - _.bind(previous_msg_el.insertAdjacentElement, day_el, 'afterend') - ); - } - } else { - this.insertMessage( - attrs, - _.bind(previous_msg_el.insertAdjacentElement, previous_msg_el, 'afterend') + if (current_msg_date.isAfter(previous_msg_date, 'day') && _.isNull(day_el)) { + this.insertDayIndicator( + current_msg_date, + _.bind(this.content.insertAdjacentHTML, previous_msg_el, 'afterend') ); } } + this.scrollDown(); }, getExtraMessageTemplateAttributes () { From 46e54667c3d573654ca9fc0648326f741bab3793 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Wed, 3 Jan 2018 12:04:06 +0000 Subject: [PATCH 2/3] Simplify `insertDayIndicator` method --- spec/chatbox.js | 276 +++++++++++++++++++-------------------- spec/chatroom.js | 197 ++++++++++++++++++++++++++++ src/converse-chatview.js | 44 +++---- 3 files changed, 354 insertions(+), 163 deletions(-) diff --git a/spec/chatbox.js b/spec/chatbox.js index dbc1060a1..cfd6c33ef 100644 --- a/spec/chatbox.js +++ b/spec/chatbox.js @@ -879,144 +879,6 @@ }).then(done); })); - it("can be received out of order, and will still be displayed in the right order", - mock.initConverseWithPromises( - null, ['rosterGroupsFetched'], {}, - function (done, _converse) { - - test_utils.createContacts(_converse, 'current'); - test_utils.openControlBox(); - test_utils.openContactsPanel(_converse); - - test_utils.waitUntil(function () { - return _converse.rosterview.$el.find('.roster-group').length; - }, 300) - .then(function () { - var message, msg; - spyOn(_converse, 'log'); - spyOn(_converse.chatboxes, 'getChatBox').and.callThrough(); - _converse.filter_by_resource = true; - var sender_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; - - /* - * - * - * - * Call me but love, and I'll be new baptized; Henceforth I never will be Romeo. - * - * - * - */ - msg = $msg({'id': 'aeb213', 'to': _converse.bare_jid}) - .c('forwarded', {'xmlns': 'urn:xmpp:forward:0'}) - .c('delay', {'xmlns': 'urn:xmpp:delay', 'stamp':'2018-01-02T13:08:25Z'}).up() - .c('message', { - 'xmlns': 'jabber:client', - 'to': _converse.bare_jid, - 'from': sender_jid, - 'type': 'chat'}) - .c('body').t("message from today") - .tree(); - _converse.chatboxes.onMessage(msg); - - msg = $msg({'id': 'aeb214', 'to': _converse.bare_jid}) - .c('forwarded', {'xmlns': 'urn:xmpp:forward:0'}) - .c('delay', {'xmlns': 'urn:xmpp:delay', 'stamp':'2017-12-31T23:08:25Z'}).up() - .c('message', { - 'xmlns': 'jabber:client', - 'to': _converse.bare_jid, - 'from': sender_jid, - 'type': 'chat'}) - .c('body').t("Older message") - .tree(); - _converse.chatboxes.onMessage(msg); - - msg = $msg({'id': 'aeb215', 'to': _converse.bare_jid}) - .c('forwarded', {'xmlns': 'urn:xmpp:forward:0'}) - .c('delay', {'xmlns': 'urn:xmpp:delay', 'stamp':'2018-01-01T13:18:23Z'}).up() - .c('message', { - 'xmlns': 'jabber:client', - 'to': _converse.bare_jid, - 'from': sender_jid, - 'type': 'chat'}) - .c('body').t("Inbetween message") - .tree(); - _converse.chatboxes.onMessage(msg); - - msg = $msg({'id': 'aeb216', 'to': _converse.bare_jid}) - .c('forwarded', {'xmlns': 'urn:xmpp:forward:0'}) - .c('delay', {'xmlns': 'urn:xmpp:delay', 'stamp':'2018-01-01T13:18:23Z'}).up() - .c('message', { - 'xmlns': 'jabber:client', - 'to': _converse.bare_jid, - 'from': sender_jid, - 'type': 'chat'}) - .c('body').t("another inbetween message") - .tree(); - _converse.chatboxes.onMessage(msg); - - msg = $msg({'id': 'aeb217', 'to': _converse.bare_jid}) - .c('forwarded', {'xmlns': 'urn:xmpp:forward:0'}) - .c('delay', {'xmlns': 'urn:xmpp:delay', 'stamp':'2018-01-02T12:18:23Z'}).up() - .c('message', { - 'xmlns': 'jabber:client', - 'to': _converse.bare_jid, - 'from': sender_jid, - 'type': 'chat'}) - .c('body').t("An earlier message today") - .tree(); - _converse.chatboxes.onMessage(msg); - - msg = $msg({'id': 'aeb218', 'to': _converse.bare_jid}) - .c('forwarded', {'xmlns': 'urn:xmpp:forward:0'}) - .c('delay', {'xmlns': 'urn:xmpp:delay', 'stamp':'2018-01-02T23:28:23Z'}).up() - .c('message', { - 'xmlns': 'jabber:client', - 'to': _converse.bare_jid, - 'from': sender_jid, - 'type': 'chat'}) - .c('body').t("newer message from today") - .tree(); - _converse.chatboxes.onMessage(msg); - - var chatboxview = _converse.chatboxviews.get(sender_jid); - var $chat_content = chatboxview.$el.find('.chat-content'); - chatboxview.clearSpinner(); //cleanup - - var $time = $chat_content.find('time'); - expect($time.length).toEqual(3); - $time = $chat_content.find('time:first'); - expect($time.data('isodate')).toEqual('2017-12-31T00:00:00+00:00'); - - expect($time[0].nextElementSibling.querySelector('.chat-msg-content').textContent).toBe('Older message'); - var $el = $chat_content.find('.chat-message:first').find('.chat-msg-content') - expect($el.text()).toEqual('Older message'); - - $time = $chat_content.find('time:eq(1)'); - expect($time.data('isodate')).toEqual('2018-01-01T00:00:00+00:00'); - expect($time[0].nextElementSibling.querySelector('.chat-msg-content').textContent).toBe('Inbetween message'); - $el = $chat_content.find('.chat-message:eq(1)'); - expect($el.find('.chat-msg-content').text()).toEqual('Inbetween message'); - expect($el[0].nextElementSibling.querySelector('.chat-msg-content').textContent).toEqual('another inbetween message'); - $el = $chat_content.find('.chat-message:eq(2)'); - expect($el.find('.chat-msg-content').text()).toEqual('another inbetween message'); - - $time = $chat_content.find('time:last'); - expect($time.data('isodate')).toEqual('2018-01-02T00:00:00+00:00'); - expect($time[0].nextElementSibling.querySelector('.chat-msg-content').textContent).toBe('An earlier message today'); - $el = $chat_content.find('.chat-message:eq(3)'); - expect($el.find('.chat-msg-content').text()).toEqual('An earlier message today'); - - $el = $chat_content.find('.chat-message:eq(4)'); - expect($el.find('.chat-msg-content').text()).toEqual('message from today'); - expect($el[0].nextElementSibling.querySelector('.chat-msg-content').textContent).toEqual('newer message from today'); - done(); - }); - })); - it("is ignored if it's intended for a different resource and filter_by_resource is set to true", mock.initConverseWithPromises( null, ['rosterGroupsFetched'], {}, @@ -1071,6 +933,144 @@ })); }); + it("can be received out of order, and will still be displayed in the right order", + mock.initConverseWithPromises( + null, ['rosterGroupsFetched'], {}, + function (done, _converse) { + + test_utils.createContacts(_converse, 'current'); + test_utils.openControlBox(); + test_utils.openContactsPanel(_converse); + + test_utils.waitUntil(function () { + return _converse.rosterview.$el.find('.roster-group').length; + }, 300) + .then(function () { + var message, msg; + spyOn(_converse, 'log'); + spyOn(_converse.chatboxes, 'getChatBox').and.callThrough(); + _converse.filter_by_resource = true; + var sender_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; + + /* + * + * + * + * Call me but love, and I'll be new baptized; Henceforth I never will be Romeo. + * + * + * + */ + msg = $msg({'id': 'aeb213', 'to': _converse.bare_jid}) + .c('forwarded', {'xmlns': 'urn:xmpp:forward:0'}) + .c('delay', {'xmlns': 'urn:xmpp:delay', 'stamp':'2018-01-02T13:08:25Z'}).up() + .c('message', { + 'xmlns': 'jabber:client', + 'to': _converse.bare_jid, + 'from': sender_jid, + 'type': 'chat'}) + .c('body').t("message from today") + .tree(); + _converse.chatboxes.onMessage(msg); + + msg = $msg({'id': 'aeb214', 'to': _converse.bare_jid}) + .c('forwarded', {'xmlns': 'urn:xmpp:forward:0'}) + .c('delay', {'xmlns': 'urn:xmpp:delay', 'stamp':'2017-12-31T23:08:25Z'}).up() + .c('message', { + 'xmlns': 'jabber:client', + 'to': _converse.bare_jid, + 'from': sender_jid, + 'type': 'chat'}) + .c('body').t("Older message") + .tree(); + _converse.chatboxes.onMessage(msg); + + msg = $msg({'id': 'aeb215', 'to': _converse.bare_jid}) + .c('forwarded', {'xmlns': 'urn:xmpp:forward:0'}) + .c('delay', {'xmlns': 'urn:xmpp:delay', 'stamp':'2018-01-01T13:18:23Z'}).up() + .c('message', { + 'xmlns': 'jabber:client', + 'to': _converse.bare_jid, + 'from': sender_jid, + 'type': 'chat'}) + .c('body').t("Inbetween message") + .tree(); + _converse.chatboxes.onMessage(msg); + + msg = $msg({'id': 'aeb216', 'to': _converse.bare_jid}) + .c('forwarded', {'xmlns': 'urn:xmpp:forward:0'}) + .c('delay', {'xmlns': 'urn:xmpp:delay', 'stamp':'2018-01-01T13:18:23Z'}).up() + .c('message', { + 'xmlns': 'jabber:client', + 'to': _converse.bare_jid, + 'from': sender_jid, + 'type': 'chat'}) + .c('body').t("another inbetween message") + .tree(); + _converse.chatboxes.onMessage(msg); + + msg = $msg({'id': 'aeb217', 'to': _converse.bare_jid}) + .c('forwarded', {'xmlns': 'urn:xmpp:forward:0'}) + .c('delay', {'xmlns': 'urn:xmpp:delay', 'stamp':'2018-01-02T12:18:23Z'}).up() + .c('message', { + 'xmlns': 'jabber:client', + 'to': _converse.bare_jid, + 'from': sender_jid, + 'type': 'chat'}) + .c('body').t("An earlier message today") + .tree(); + _converse.chatboxes.onMessage(msg); + + msg = $msg({'id': 'aeb218', 'to': _converse.bare_jid}) + .c('forwarded', {'xmlns': 'urn:xmpp:forward:0'}) + .c('delay', {'xmlns': 'urn:xmpp:delay', 'stamp':'2018-01-02T23:28:23Z'}).up() + .c('message', { + 'xmlns': 'jabber:client', + 'to': _converse.bare_jid, + 'from': sender_jid, + 'type': 'chat'}) + .c('body').t("newer message from today") + .tree(); + _converse.chatboxes.onMessage(msg); + + var chatboxview = _converse.chatboxviews.get(sender_jid); + var $chat_content = chatboxview.$el.find('.chat-content'); + chatboxview.clearSpinner(); //cleanup + + var $time = $chat_content.find('time'); + expect($time.length).toEqual(3); + $time = $chat_content.find('time:first'); + expect($time.data('isodate')).toEqual('2017-12-31T00:00:00+00:00'); + + expect($time[0].nextElementSibling.querySelector('.chat-msg-content').textContent).toBe('Older message'); + var $el = $chat_content.find('.chat-message:first').find('.chat-msg-content') + expect($el.text()).toEqual('Older message'); + + $time = $chat_content.find('time:eq(1)'); + expect($time.data('isodate')).toEqual('2018-01-01T00:00:00+00:00'); + expect($time[0].nextElementSibling.querySelector('.chat-msg-content').textContent).toBe('Inbetween message'); + $el = $chat_content.find('.chat-message:eq(1)'); + expect($el.find('.chat-msg-content').text()).toEqual('Inbetween message'); + expect($el[0].nextElementSibling.querySelector('.chat-msg-content').textContent).toEqual('another inbetween message'); + $el = $chat_content.find('.chat-message:eq(2)'); + expect($el.find('.chat-msg-content').text()).toEqual('another inbetween message'); + + $time = $chat_content.find('time:last'); + expect($time.data('isodate')).toEqual('2018-01-02T00:00:00+00:00'); + expect($time[0].nextElementSibling.querySelector('.chat-msg-content').textContent).toBe('An earlier message today'); + $el = $chat_content.find('.chat-message:eq(3)'); + expect($el.find('.chat-msg-content').text()).toEqual('An earlier message today'); + + $el = $chat_content.find('.chat-message:eq(4)'); + expect($el.find('.chat-msg-content').text()).toEqual('message from today'); + expect($el[0].nextElementSibling.querySelector('.chat-msg-content').textContent).toEqual('newer message from today'); + done(); + }); + })); + it("is ignored if it's a malformed headline message", mock.initConverseWithPromises( null, ['rosterGroupsFetched'], {}, diff --git a/spec/chatroom.js b/spec/chatroom.js index 5897b9317..0e01f2127 100644 --- a/spec/chatroom.js +++ b/spec/chatroom.js @@ -591,6 +591,203 @@ done(); })); + it("shows join/leave messages when users enter or exit a room", + mock.initConverseWithPromises( + null, ['rosterGroupsFetched'], {}, + function (done, _converse) { + + test_utils.openChatRoom(_converse, "coven", 'chat.shakespeare.lit', 'some1'); + var view = _converse.chatboxviews.get('coven@chat.shakespeare.lit'); + var $chat_content = view.$el.find('.chat-content'); + + /* We don't show join/leave messages for existing occupants. We + * know about them because we receive their presences before we + * receive our own. + */ + presence = $pres({ + to: 'dummy@localhost/_converse.js-29092160', + from: 'coven@chat.shakespeare.lit/oldguy' + }).c('x', {xmlns: Strophe.NS.MUC_USER}) + .c('item', { + 'affiliation': 'none', + 'jid': 'oldguy@localhost/_converse.js-290929789', + 'role': 'participant' + }); + _converse.connection._dataRecv(test_utils.createRequest(presence)); + expect($chat_content[0].querySelectorAll('div.chat-info').length).toBe(0); + + /* + * + * + * + * + * + */ + var presence = $pres({ + to: 'dummy@localhost/_converse.js-29092160', + from: 'coven@chat.shakespeare.lit/some1' + }).c('x', {xmlns: Strophe.NS.MUC_USER}) + .c('item', { + 'affiliation': 'owner', + 'jid': 'dummy@localhost/_converse.js-29092160', + 'role': 'moderator' + }).up() + .c('status', {code: '110'}); + _converse.connection._dataRecv(test_utils.createRequest(presence)); + expect($chat_content.find('div.chat-info:first').html()).toBe("some1 has entered the room."); + + 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', + 'jid': 'newguy@localhost/_converse.js-290929789', + 'role': 'participant' + }); + _converse.connection._dataRecv(test_utils.createRequest(presence)); + 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', + from: 'coven@chat.shakespeare.lit/newguy' + }).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[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', + 'jid': 'newguy@localhost/_converse.js-290929789', + 'role': 'participant' + }); + _converse.connection._dataRecv(test_utils.createRequest(presence)); + 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(); + })); + it("shows its description in the chat heading", mock.initConverseWithPromises( null, ['rosterGroupsFetched'], {}, diff --git a/src/converse-chatview.js b/src/converse-chatview.js index c566c0395..ce3dbd292 100644 --- a/src/converse-chatview.js +++ b/src/converse-chatview.js @@ -382,21 +382,25 @@ ); }, - insertDayIndicator (date, insert_method) { - /* Inserts an indicator - * into the chat area, showing the day as given by the - * passed in date. + insertDayIndicator (next_msg_el) { + /* Inserts an indicator into the chat area, showing the + * day as given by the passed in date. * * Parameters: - * (String) date - An ISO8601 date string. - * (Function) insert_method - The method to be used to - * insert the indicator + * (HTMLElement) next_msg_el - The message element before + * which the day indicator element must be inserted. + * This element must have a "data-isodate" attribute + * which specifies its creation date. */ - const day_date = moment(date).startOf('day'); - insert_method(tpl_new_day({ - isodate: day_date.format(), - datestring: day_date.format("dddd MMM Do YYYY") - })); + const date = next_msg_el.getAttribute('data-isodate'), + day_date = moment(date).startOf('day'); + + next_msg_el.insertAdjacentHTML('beforeBegin', + tpl_new_day({ + 'isodate': day_date.format(), + 'datestring': day_date.format("dddd MMM Do YYYY") + }) + ); }, getLastMessageElement () { @@ -453,11 +457,6 @@ } }, - getDayIndicatorElement (date) { - return this.content.querySelector( - `.chat-date[data-isodate="${date.startOf('day').format()}"]`); - }, - showMessage (attrs) { /* Inserts a chat message into the content area of the chat box. * Will also insert a new day indicator if the message is on a @@ -477,17 +476,12 @@ if (_.isNull(previous_msg_date)) { this.content.insertAdjacentElement('afterbegin', message_el); - this.insertDayIndicator(current_msg_date, prepend_html); + this.insertDayIndicator(message_el); } else { const previous_msg_el = sizzle(`[data-isodate="${previous_msg_date}"]:last`, this.content).pop(); previous_msg_el.insertAdjacentElement('afterend', message_el); - - const day_el = this.getDayIndicatorElement(current_msg_date) - if (current_msg_date.isAfter(previous_msg_date, 'day') && _.isNull(day_el)) { - this.insertDayIndicator( - current_msg_date, - _.bind(this.content.insertAdjacentHTML, previous_msg_el, 'afterend') - ); + if (current_msg_date.isAfter(previous_msg_date, 'day')) { + this.insertDayIndicator(message_el); } } this.scrollDown(); From da3670d9f0d2d76bb82f4a54bd94c9898bc45802 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Wed, 3 Jan 2018 12:55:57 +0000 Subject: [PATCH 3/3] MUC Join/Leave messages now also show a new day indicator --- CHANGES.md | 1 + spec/chatroom.js | 207 ++++++++++++++------------------------- src/converse-chatview.js | 37 ++++--- src/converse-muc.js | 14 ++- 4 files changed, 112 insertions(+), 147 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 80b884b19..6325bb5ef 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -34,6 +34,7 @@ - Show status messages in an MUC room when a user's role changes. - In MUC chat rooms, collapse multiple, consecutive join/leave messages. - Performance improvements for rendering private chats, rooms and the contacts roster. +- MUC Leave/Join messages now also show a new day indicator if applicable. ### API changes - New API method `_converse.disco.supports` to check whether a certain diff --git a/spec/chatroom.js b/spec/chatroom.js index 0e01f2127..b1f510e85 100644 --- a/spec/chatroom.js +++ b/spec/chatroom.js @@ -7,6 +7,7 @@ var $msg = converse.env.$msg; var Strophe = converse.env.Strophe; var Promise = converse.env.Promise; + var moment = converse.env.moment; return describe("ChatRooms", function () { describe("The \"rooms\" API", function () { @@ -591,7 +592,7 @@ done(); })); - it("shows join/leave messages when users enter or exit a room", + it("shows a new day indicator if a join/leave message is received on a new day", mock.initConverseWithPromises( null, ['rosterGroupsFetched'], {}, function (done, _converse) { @@ -600,22 +601,6 @@ var view = _converse.chatboxviews.get('coven@chat.shakespeare.lit'); var $chat_content = view.$el.find('.chat-content'); - /* We don't show join/leave messages for existing occupants. We - * know about them because we receive their presences before we - * receive our own. - */ - presence = $pres({ - to: 'dummy@localhost/_converse.js-29092160', - from: 'coven@chat.shakespeare.lit/oldguy' - }).c('x', {xmlns: Strophe.NS.MUC_USER}) - .c('item', { - 'affiliation': 'none', - 'jid': 'oldguy@localhost/_converse.js-290929789', - 'role': 'participant' - }); - _converse.connection._dataRecv(test_utils.createRequest(presence)); - expect($chat_content[0].querySelectorAll('div.chat-info').length).toBe(0); - /* * @@ -635,83 +620,55 @@ }).up() .c('status', {code: '110'}); _converse.connection._dataRecv(test_utils.createRequest(presence)); + + var $time = $chat_content.find('time'); + expect($time.length).toEqual(1); + expect($time.attr('class')).toEqual('chat-info chat-date'); + expect($time.data('isodate')).toEqual(moment().startOf('day').format()); + expect($time.text()).toEqual(moment().startOf('day').format("dddd MMM Do YYYY")); expect($chat_content.find('div.chat-info:first').html()).toBe("some1 has entered the room."); - 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', - 'jid': 'newguy@localhost/_converse.js-290929789', - 'role': 'participant' - }); - _converse.connection._dataRecv(test_utils.createRequest(presence)); - 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."); + // XXX: Hack. We clear the chat contents instead of mocking the date + $chat_content.html(''); - // 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', - from: 'coven@chat.shakespeare.lit/newguy' - }).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[0].querySelectorAll('div.chat-info').length).toBe(3); - - /* - * Disconnected: Replaced by new connection - * - * - * - * - */ + // Test a user leaving a chat room presence = $pres({ to: 'dummy@localhost/_converse.js-29092160', type: 'unavailable', - from: 'coven@chat.shakespeare.lit/newguy' + from: 'coven@chat.shakespeare.lit/some1' }) .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', + 'jid': 'some1@localhost/_converse.js-290929789', 'role': 'none' }); _converse.connection._dataRecv(test_utils.createRequest(presence)); - expect($chat_content.find('div.chat-info').length).toBe(4); + + $time = $chat_content.find('time'); + expect($time.length).toEqual(1); + expect($time.attr('class')).toEqual('chat-info chat-date'); + expect($time.data('isodate')).toEqual(moment().startOf('day').format()); + expect($time.text()).toEqual(moment().startOf('day').format("dddd MMM Do YYYY")); + expect($chat_content.find('div.chat-info').length).toBe(1); expect($chat_content.find('div.chat-info:last').html()).toBe( - 'newguy has left the room. '+ + 'some1 has left the room. '+ '"Disconnected: Replaced by new connection"'); - // When the user immediately joins again, we collapse the - // multiple join/leave messages. + // XXX: Hack. We clear the chat contents instead of mocking the date + $chat_content.html(''); + + var stanza = Strophe.xmlHtmlNode( + ''+ + ' hello world'+ + ' '+ + '').firstChild; + _converse.connection._dataRecv(test_utils.createRequest(stanza)); + presence = $pres({ to: 'dummy@localhost/_converse.js-29092160', from: 'coven@chat.shakespeare.lit/newguy' @@ -722,70 +679,58 @@ 'role': 'participant' }); _converse.connection._dataRecv(test_utils.createRequest(presence)); - 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"'); + $time = $chat_content.find('time'); + expect($time.length).toEqual(2); + + $time = $chat_content.find('time:eq(1)'); + expect($time.attr('class')).toEqual('chat-info chat-date'); + expect($time.data('isodate')).toEqual(moment().startOf('day').format()); + expect($time.text()).toEqual(moment().startOf('day').format("dddd MMM Do YYYY")); + expect($chat_content.find('div.chat-info').length).toBe(1); + expect($chat_content.find('div.chat-info:first').html()).toBe("newguy has entered the room."); + + // XXX: Hack. We clear the chat contents instead of mocking the date + $chat_content.html(''); + + stanza = Strophe.xmlHtmlNode( + ''+ + ' hello world'+ + ' '+ + '').firstChild; + _converse.connection._dataRecv(test_utils.createRequest(stanza)); + + // Test a user leaving a chat room presence = $pres({ to: 'dummy@localhost/_converse.js-29092160', type: 'unavailable', - from: 'coven@chat.shakespeare.lit/newguy' + from: 'coven@chat.shakespeare.lit/some1' }) + .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', + 'jid': 'some1@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."); + $time = $chat_content.find('time'); + expect($time.length).toEqual(2); - 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."); + $time = $chat_content.find('time:eq(1)'); + expect($time.attr('class')).toEqual('chat-info chat-date'); + expect($time.data('isodate')).toEqual(moment().startOf('day').format()); + expect($time.text()).toEqual(moment().startOf('day').format("dddd MMM Do YYYY")); + expect($chat_content.find('div.chat-info').length).toBe(1); + expect($chat_content.find('div.chat-info:last').html()).toBe( + 'some1 has left the room. '+ + '"Disconnected: Replaced by new connection"'); done(); + return; })); it("shows its description in the chat heading", @@ -1151,7 +1096,7 @@ name = mock.chatroom_names[i]; role = mock.chatroom_roles[name].role; // See example 21 http://xmpp.org/extensions/xep-0045.html#enter-pres - jid = + jid = presence = $pres({ to:'dummy@localhost/pda', from:'lounge@localhost/'+name @@ -2060,7 +2005,7 @@ textarea.textContent = '/help This is the room subject'; $(textarea).trigger($.Event('keypress', {keyCode: 13})); expect(view.onMessageSubmitted).toHaveBeenCalled(); - const info_messages = Array.prototype.slice.call(view.el.querySelectorAll('.chat-info'), 0); + const info_messages = Array.prototype.slice.call(view.el.querySelectorAll('.chat-info:not(.chat-date)'), 0); expect(info_messages.length).toBe(17); expect(info_messages.pop().textContent).toBe('/voice: Allow muted user to post messages'); expect(info_messages.pop().textContent).toBe('/topic: Set room subject (alias for /subject)'); @@ -2302,7 +2247,7 @@ .c('status', {'code': '307'}); _converse.connection._dataRecv(test_utils.createRequest(presence)); expect( - view.el.querySelectorAll('.chat-info')[2].textContent).toBe( + view.el.querySelectorAll('.chat-info')[3].textContent).toBe( "annoyingGuy has been kicked out"); done(); }); @@ -3136,7 +3081,7 @@ done(); })); }); - + describe("The \"Rooms\" Panel", function () { it("shows the number of unread mentions received", diff --git a/src/converse-chatview.js b/src/converse-chatview.js index ce3dbd292..18ad2424a 100644 --- a/src/converse-chatview.js +++ b/src/converse-chatview.js @@ -386,21 +386,37 @@ /* Inserts an indicator into the chat area, showing the * day as given by the passed in date. * + * The indicator is only inserted if necessary. + * * Parameters: * (HTMLElement) next_msg_el - The message element before * which the day indicator element must be inserted. * This element must have a "data-isodate" attribute * which specifies its creation date. */ - const date = next_msg_el.getAttribute('data-isodate'), - day_date = moment(date).startOf('day'); + const prev_msg_el = this.getPreviousMessageElement(next_msg_el), + prev_msg_date = _.isNull(prev_msg_el) ? null : prev_msg_el.getAttribute('data-isodate'), + next_msg_date = next_msg_el.getAttribute('data-isodate'); - next_msg_el.insertAdjacentHTML('beforeBegin', - tpl_new_day({ - 'isodate': day_date.format(), - 'datestring': day_date.format("dddd MMM Do YYYY") - }) - ); + if (_.isNull(prev_msg_date) || moment(next_msg_date).isAfter(prev_msg_date, 'day')) { + const day_date = moment(next_msg_date).startOf('day'); + next_msg_el.insertAdjacentHTML('beforeBegin', + tpl_new_day({ + 'isodate': day_date.format(), + 'datestring': day_date.format("dddd MMM Do YYYY") + }) + ); + } + }, + + getPreviousMessageElement (el) { + let prev_msg_el = el.previousSibling; + while (!_.isNull(prev_msg_el) && + !u.hasClass(prev_msg_el, 'message') && + !u.hasClass(prev_msg_el, 'chat-info')) { + prev_msg_el = prev_msg_el.previousSibling + } + return prev_msg_el; }, getLastMessageElement () { @@ -476,14 +492,11 @@ if (_.isNull(previous_msg_date)) { this.content.insertAdjacentElement('afterbegin', message_el); - this.insertDayIndicator(message_el); } else { const previous_msg_el = sizzle(`[data-isodate="${previous_msg_date}"]:last`, this.content).pop(); previous_msg_el.insertAdjacentElement('afterend', message_el); - if (current_msg_date.isAfter(previous_msg_date, 'day')) { - this.insertDayIndicator(message_el); - } } + this.insertDayIndicator(message_el); this.scrollDown(); }, diff --git a/src/converse-muc.js b/src/converse-muc.js index ad6b465ea..27dd127c5 100644 --- a/src/converse-muc.js +++ b/src/converse-muc.js @@ -1837,9 +1837,10 @@ const nick = Strophe.getResourceFromJid(stanza.getAttribute('from')); const stat = stanza.querySelector('status'); const last_el = this.content.lastElementChild; + if (_.includes(_.get(last_el, 'classList', []), 'chat-info') && _.get(last_el, 'dataset', {}).leave === `"${nick}"`) { - last_el.outerHTML = + last_el.outerHTML = tpl_info({ 'data': `data-leavejoin="${nick}"`, 'isodate': moment().format(), @@ -1860,7 +1861,9 @@ last_el.outerHTML = tpl_info(data); } else { - this.content.insertAdjacentHTML('beforeend', tpl_info(data)); + const el = u.stringToElement(tpl_info(data)); + this.content.insertAdjacentElement('beforeend', el); + this.insertDayIndicator(el); } } this.scrollDown(); @@ -1877,7 +1880,7 @@ if (_.get(stat, 'textContent')) { message = message + ' "' + stat.textContent + '"'; } - last_el.outerHTML = + last_el.outerHTML = tpl_info({ 'data': `data-joinleave="${nick}"`, 'isodate': moment().format(), @@ -1890,6 +1893,7 @@ } const data = { 'message': message, + 'isodate': moment().format(), 'data': `data-leave="${nick}"` } if (_.includes(_.get(last_el, 'classList', []), 'chat-info') && @@ -1897,7 +1901,9 @@ last_el.outerHTML = tpl_info(data); } else { - this.content.insertAdjacentHTML('beforeend', tpl_info(data)); + const el = u.stringToElement(tpl_info(data)); + this.content.insertAdjacentElement('beforeend', el); + this.insertDayIndicator(el); } } this.scrollDown();