From 46469569224476c304cd02cd8384c51f235da189 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Thu, 11 Mar 2021 12:33:50 +0100 Subject: [PATCH] Don't render hidden chats --- CHANGES.md | 3 ++ spec/chatbox.js | 22 ++++----- spec/controlbox.js | 3 +- spec/minchats.js | 17 +++---- spec/muc-mentions.js | 8 ++-- spec/muclist.js | 1 + spec/rai.js | 12 ++--- spec/roster.js | 10 ++--- src/plugins/chatboxviews/templates/chats.js | 15 +++++-- src/plugins/minimize/index.js | 12 ++--- src/plugins/minimize/utils.js | 49 ++++++++++----------- src/shared/chat/baseview.js | 8 +++- 12 files changed, 88 insertions(+), 72 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 2eba6b572..6cba3a21c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -32,6 +32,9 @@ Removed events: * `bookmarkViewsInitialized` * `rosterGroupsFetched` +The `chatBoxMaximized` and `chatBoxMinimized` events now have the `model` as +payload and not the `view` since it might not be exist at that time. + ## 7.0.5 (Unreleased) - #2377: The @converse/headless NPM package is missing the dist directory, causing import errors diff --git a/spec/chatbox.js b/spec/chatbox.js index 7db1f881f..2060c00d5 100644 --- a/spec/chatbox.js +++ b/spec/chatbox.js @@ -119,6 +119,7 @@ describe("Chatboxes", function () { await new Promise(resolve => _converse.api.listen.once('chatBoxViewInitialized', resolve)); await u.waitUntil(() => message_promise); expect(_converse.chatboxviews.keys().length).toBe(2); + expect(_converse.chatboxviews.keys().pop()).toBe(sender_jid); done(); })); @@ -198,6 +199,7 @@ describe("Chatboxes", function () { mock.initConverse(['chatBoxesFetched'], {}, async function (done, _converse) { await mock.waitForRoster(_converse, 'current'); + await mock.openControlBox(_converse); const contact_jid = mock.cur_names[7].replace(/ /g,'.').toLowerCase() + '@montague.lit'; const rosterview = document.querySelector('converse-roster'); await u.waitUntil(() => rosterview.querySelectorAll('.roster-group').length); @@ -372,8 +374,8 @@ describe("Chatboxes", function () { u.waitUntil(() => rosterview.querySelectorAll('.roster-group').length); spyOn(_converse.connection, 'send'); await mock.openChatBoxFor(_converse, contact_jid); - const view = _converse.chatboxviews.get(contact_jid); - expect(view.model.get('chat_state')).toBe('active'); + const model = _converse.chatboxes.get(contact_jid); + expect(model.get('chat_state')).toBe('active'); expect(_converse.connection.send).toHaveBeenCalled(); const stanza = _converse.connection.send.calls.argsFor(0)[0].tree(); expect(stanza.getAttribute('to')).toBe(contact_jid); @@ -394,12 +396,12 @@ describe("Chatboxes", function () { const rosterview = document.querySelector('converse-roster'); await u.waitUntil(() => rosterview.querySelectorAll('.roster-group').length); await mock.openChatBoxFor(_converse, contact_jid); - const view = _converse.chatboxviews.get(contact_jid); - _converse.minimize.minimize(view.model); - expect(view.model.get('chat_state')).toBe('inactive'); + const model = _converse.chatboxes.get(contact_jid); + _converse.minimize.minimize(model); + expect(model.get('chat_state')).toBe('inactive'); spyOn(_converse.connection, 'send'); - _converse.minimize.maximize(view.model); - await u.waitUntil(() => view.model.get('chat_state') === 'active', 1000); + _converse.minimize.maximize(model); + await u.waitUntil(() => model.get('chat_state') === 'active', 1000); expect(_converse.connection.send).toHaveBeenCalled(); const calls = _.filter(_converse.connection.send.calls.all(), function (call) { return call.args[0] instanceof Strophe.Builder; @@ -427,7 +429,7 @@ describe("Chatboxes", function () { const rosterview = document.querySelector('converse-roster'); await u.waitUntil(() => rosterview.querySelectorAll('.roster-group').length); await mock.openChatBoxFor(_converse, contact_jid); - var view = _converse.chatboxviews.get(contact_jid); + const view = _converse.chatboxviews.get(contact_jid); expect(view.model.get('chat_state')).toBe('active'); spyOn(_converse.connection, 'send'); spyOn(_converse.api, "trigger").and.callThrough(); @@ -468,7 +470,7 @@ describe("Chatboxes", function () { const rosterview = document.querySelector('converse-roster'); await u.waitUntil(() => rosterview.querySelectorAll('.roster-group').length); await mock.openChatBoxFor(_converse, contact_jid); - var view = _converse.chatboxviews.get(contact_jid); + const view = _converse.chatboxviews.get(contact_jid); expect(view.model.get('chat_state')).toBe('active'); spyOn(_converse.connection, 'send'); spyOn(_converse.api, "trigger").and.callThrough(); @@ -1162,7 +1164,7 @@ describe("Chatboxes", function () { _converse.handleMessageStanza(msgFactory()); await u.waitUntil(() => chatbox.messages.length > 1); expect(select_msgs_indicator().textContent).toBe('2'); - _converse.minimize.minimize(view.model); + _converse.minimize.maximize(view.model); u.waitUntil(() => typeof select_msgs_indicator() === 'undefined'); done(); })); diff --git a/spec/controlbox.js b/spec/controlbox.js index 812051543..f19a80776 100644 --- a/spec/controlbox.js +++ b/spec/controlbox.js @@ -227,7 +227,7 @@ describe("The 'Add Contact' widget", function () { mock.initConverse([], {'autocomplete_add_contact': false}, async function (done, _converse) { await mock.waitForRoster(_converse, 'all', 0); - mock.openControlBox(_converse); + await mock.openControlBox(_converse); const cbview = _converse.chatboxviews.get('controlbox'); cbview.querySelector('.add-contact').click() const modal = _converse.api.modal.get('add-contact-modal'); @@ -274,6 +274,7 @@ describe("The 'Add Contact' widget", function () { const XMLHttpRequestBackup = window.XMLHttpRequest; window.XMLHttpRequest = MockXHR; + await mock.openControlBox(_converse); const cbview = _converse.chatboxviews.get('controlbox'); cbview.querySelector('.add-contact').click() const modal = _converse.api.modal.get('add-contact-modal'); diff --git a/spec/minchats.js b/spec/minchats.js index 279b916c0..672cac4bb 100644 --- a/spec/minchats.js +++ b/spec/minchats.js @@ -70,7 +70,7 @@ describe("A chat message", function () { }); -describe("A Groupcaht", function () { +describe("A Groupchat", function () { it("can be minimized by clicking a DOM element with class 'toggle-chatbox-button'", mock.initConverse(['chatBoxesFetched'], {}, async function (done, _converse) { @@ -117,7 +117,7 @@ describe("A Chatbox", function () { expect(_converse.api.trigger.calls.count(), 2); expect(u.isVisible(chatview)).toBeFalsy(); expect(chatview.model.get('minimized')).toBeTruthy(); - chatview.querySelector('.toggle-chatbox-button').click(); + document.querySelector('converse-minimized-chat').click(); await u.waitUntil(() => _converse.chatboxviews.keys().length); const minimized_chats = document.querySelector("converse-minimized-chat") @@ -135,8 +135,7 @@ describe("A Chatbox", function () { expect(u.isVisible(minimized_chats.firstElementChild)).toBe(false); await _converse.api.chats.create(sender_jid, {'minimized': true}); await u.waitUntil(() => _converse.chatboxes.length > 1); - const view = _converse.chatboxviews.get(sender_jid); - expect(u.isVisible(view)).toBeFalsy(); + expect(_converse.chatboxviews.get(sender_jid)).toBe(undefined); expect(u.isVisible(minimized_chats.firstElementChild)).toBe(true); expect(minimized_chats.firstElementChild.querySelectorAll('converse-minimized-chat').length).toBe(1); expect(_converse.chatboxes.filter('minimized').length).toBe(1); @@ -149,7 +148,6 @@ describe("A Chatbox", function () { await mock.waitForRoster(_converse, 'current'); await mock.openControlBox(_converse); - let jid, chatboxview; // openControlBox was called earlier, so the controlbox is // visible, but no other chat boxes have been created. expect(_converse.chatboxes.length).toEqual(1); @@ -170,12 +168,11 @@ describe("A Chatbox", function () { for (i=0; i _converse.chatboxviews.keys().length); - spyOn(_converse.minimize, 'maximize').and.callThrough(); + await u.waitUntil(() => _converse.chatboxviews.keys().length === 1); const minimized_chats = document.querySelector("converse-minimized-chats") minimized_chats.querySelector("a.restore-chat").click(); expect(_converse.minimize.trimChats.calls.count()).toBe(17); diff --git a/spec/muc-mentions.js b/spec/muc-mentions.js index 43dc28b83..64d1a50c8 100644 --- a/spec/muc-mentions.js +++ b/spec/muc-mentions.js @@ -25,10 +25,10 @@ describe("MUC Mention Notfications", function () { await mock.receiveOwnMUCPresence(_converse, muc_jid, nick); await muc_creation_promise; - const view = api.chatviews.get(muc_jid); - await u.waitUntil(() => (view.model.session.get('connection_status') === converse.ROOMSTATUS.ENTERED)); - expect(view.model.get('hidden')).toBe(true); - await u.waitUntil(() => view.model.session.get('connection_status') === converse.ROOMSTATUS.DISCONNECTED); + const model = _converse.chatboxes.get(muc_jid); + await u.waitUntil(() => (model.session.get('connection_status') === converse.ROOMSTATUS.ENTERED)); + expect(model.get('hidden')).toBe(true); + await u.waitUntil(() => model.session.get('connection_status') === converse.ROOMSTATUS.DISCONNECTED); const room_el = await u.waitUntil(() => document.querySelector("converse-rooms-list .available-chatroom")); expect(Array.from(room_el.classList).includes('unread-msgs')).toBeFalsy(); diff --git a/spec/muclist.js b/spec/muclist.js index 4a88fca68..3dcbacda5 100644 --- a/spec/muclist.js +++ b/spec/muclist.js @@ -267,6 +267,7 @@ describe("A groupchat shown in the groupchats list", function () { await mock.openChatRoom(_converse, 'lounge', 'conference.shakespeare.lit', 'JC'); expect(_converse.chatboxes.length).toBe(2); + await mock.openControlBox(_converse); const controlbox = _converse.chatboxviews.get('controlbox'); const lview = controlbox.querySelector('converse-rooms-list'); await u.waitUntil(() => lview.querySelectorAll(".open-room").length); diff --git a/spec/rai.js b/spec/rai.js index 07711fc40..25951f0ba 100644 --- a/spec/rai.js +++ b/spec/rai.js @@ -133,9 +133,9 @@ describe("XEP-0437 Room Activity Indicators", function () { await mock.receiveOwnMUCPresence(_converse, muc_jid, nick); await muc_creation_promise; - const view = api.chatviews.get(muc_jid); - await u.waitUntil(() => (view.model.session.get('connection_status') === converse.ROOMSTATUS.ENTERED)); - expect(view.model.get('hidden')).toBe(true); + const model = _converse.chatboxes.get(muc_jid); + await u.waitUntil(() => (model.session.get('connection_status') === converse.ROOMSTATUS.ENTERED)); + expect(model.get('hidden')).toBe(true); const getSentPresences = () => sent_stanzas.filter(s => s.nodeName === 'presence'); @@ -156,8 +156,8 @@ describe("XEP-0437 Room Activity Indicators", function () { `` ); - await u.waitUntil(() => view.model.session.get('connection_status') === converse.ROOMSTATUS.DISCONNECTED); - expect(view.model.get('has_activity')).toBe(false); + await u.waitUntil(() => model.session.get('connection_status') === converse.ROOMSTATUS.DISCONNECTED); + expect(model.get('has_activity')).toBe(false); const room_el = await u.waitUntil(() => document.querySelector("converse-rooms-list .available-chatroom")); expect(Array.from(room_el.classList).includes('unread-msgs')).toBeFalsy(); @@ -171,7 +171,7 @@ describe("XEP-0437 Room Activity Indicators", function () { `); _converse.connection._dataRecv(mock.createRequest(activity_stanza)); - await u.waitUntil(() => view.model.get('has_activity')); + await u.waitUntil(() => model.get('has_activity')); expect(Array.from(room_el.classList).includes('unread-msgs')).toBeTruthy(); done(); })); diff --git a/spec/roster.js b/spec/roster.js index 44bcbdef0..412dd1c18 100644 --- a/spec/roster.js +++ b/spec/roster.js @@ -322,7 +322,7 @@ describe("The Contacts Roster", function () { _converse.roster.get(jid).presence.set('show', 'online'); jid = mock.cur_names[4].replace(/ /g,'.').toLowerCase() + '@montague.lit'; _converse.roster.get(jid).presence.set('show', 'dnd'); - mock.openControlBox(_converse); + await mock.openControlBox(_converse); const rosterview = document.querySelector('converse-roster'); const button = rosterview.querySelector('span[data-type="state"]'); button.click(); @@ -488,7 +488,7 @@ describe("The Contacts Roster", function () { await mock.waitForRoster(_converse, 'current', 0); const groups = ['Colleagues', 'friends']; - mock.openControlBox(_converse); + await mock.openControlBox(_converse); for (let i=0; i { const { chatboxes, CONTROLBOX_TYPE, CHATROOMS_TYPE, HEADLINES_TYPE } = _converse; const view_mode = api.settings.get('view_mode'); @@ -9,7 +16,7 @@ export default () => { const logged_out = !connection?.connected || !connection?.authenticated || connection?.disconnecting; return html` ${view_mode === 'overlayed' ? html`` : ''} - ${repeat(chatboxes, m => m.get('jid'), m => { + ${repeat(chatboxes.filter(shouldShowChat), m => m.get('jid'), m => { if (m.get('type') === CONTROLBOX_TYPE) { return html` ${view_mode === 'overlayed' ? html`` : ''} @@ -20,15 +27,15 @@ export default () => { `; } else if (m.get('type') === CHATROOMS_TYPE) { return html` - + `; } else if (m.get('type') === HEADLINES_TYPE) { return html` - + `; } else { return html` - + `; } })} diff --git a/src/plugins/minimize/index.js b/src/plugins/minimize/index.js index 705056dd9..1ba16ee02 100644 --- a/src/plugins/minimize/index.js +++ b/src/plugins/minimize/index.js @@ -118,17 +118,17 @@ converse.plugins.add('converse-minimize', { _converse.minimize.minimize = minimize; _converse.minimize.maximize = maximize; + function onChatInitialized (model) { + model.on( 'change:minimized', () => onMinimizedChanged(model)); + } + /************************ BEGIN Event Handlers ************************/ api.listen.on('chatBoxViewInitialized', view => _converse.minimize.trimChats(view)); api.listen.on('chatRoomViewInitialized', view => _converse.minimize.trimChats(view)); api.listen.on('chatBoxMaximized', view => _converse.minimize.trimChats(view)); api.listen.on('controlBoxOpened', view => _converse.minimize.trimChats(view)); - api.listen.on('chatBoxViewInitialized', v => v.listenTo(v.model, 'change:minimized', () => onMinimizedChanged(v))); - - api.listen.on('chatRoomViewInitialized', view => { - view.listenTo(view.model, 'change:minimized', () => onMinimizedChanged(view)); - view.model.get('minimized') && view.hide(); - }); + api.listen.on('chatBoxInitialized', onChatInitialized); + api.listen.on('chatRoomInitialized', onChatInitialized); api.listen.on('getHeadingButtons', (view, buttons) => { if (view.model.get('type') === _converse.CHATROOMS_TYPE) { diff --git a/src/plugins/minimize/utils.js b/src/plugins/minimize/utils.js index 7856a087c..7e306ae4d 100644 --- a/src/plugins/minimize/utils.js +++ b/src/plugins/minimize/utils.js @@ -145,13 +145,23 @@ export function maximize (ev, chatbox) { }); } -export function minimize (ev, chatbox) { +export function minimize (ev, model) { if (ev?.preventDefault) { ev.preventDefault(); } else { - chatbox = ev; + model = ev; } - u.safeSave(chatbox, { + // save the scroll position to restore it on maximize + const view = _converse.chatboxviews.get(model.get('jid')); + const content = view.querySelector('.chat-content__messages'); + const scroll = content.scrollTop; + if (model.collection && model.collection.browserStorage) { + model.save({ scroll }); + } else { + model.set({ scroll }); + } + model.setChatState(_converse.INACTIVE); + u.safeSave(model, { 'hidden': true, 'minimized': true, 'time_minimized': new Date().toISOString() @@ -165,20 +175,18 @@ export function minimize (ev, chatbox) { * Will trigger {@link _converse#chatBoxMaximized} * @returns {_converse.ChatBoxView|_converse.ChatRoomView} */ -function onMaximized (view) { - if (!view.model.isScrolledUp()) { - view.model.clearUnreadMsgCounter(); +function onMaximized (model) { + if (!model.isScrolledUp()) { + model.clearUnreadMsgCounter(); } - view.model.setChatState(_converse.ACTIVE); - view.show(); + model.setChatState(_converse.ACTIVE); /** * Triggered when a previously minimized chat gets maximized * @event _converse#chatBoxMaximized * @type { _converse.ChatBoxView } * @example _converse.api.listen.on('chatBoxMaximized', view => { ... }); */ - api.trigger('chatBoxMaximized', view); - return view; + api.trigger('chatBoxMaximized', model); } /** @@ -188,29 +196,20 @@ function onMaximized (view) { * Will trigger {@link _converse#chatBoxMinimized} * @returns {_converse.ChatBoxView|_converse.ChatRoomView} */ -function onMinimized (view) { - // save the scroll position to restore it on maximize - const content = view.querySelector('.chat-content__messages'); - if (view.model.collection && view.model.collection.browserStorage) { - view.model.save({ 'scroll': content.scrollTop }); - } else { - view.model.set({ 'scroll': content.scrollTop }); - } - view.model.setChatState(_converse.INACTIVE); +function onMinimized (model) { /** * Triggered when a previously maximized chat gets Minimized * @event _converse#chatBoxMinimized * @type { _converse.ChatBoxView } * @example _converse.api.listen.on('chatBoxMinimized', view => { ... }); */ - api.trigger('chatBoxMinimized', view); - return view; + api.trigger('chatBoxMinimized', model); } -export function onMinimizedChanged (view) { - if (view.model.get('minimized')) { - onMinimized(view); +export function onMinimizedChanged (model) { + if (model.get('minimized')) { + onMinimized(model); } else { - onMaximized(view); + onMaximized(model); } } diff --git a/src/shared/chat/baseview.js b/src/shared/chat/baseview.js index d1f3f9a50..940c8ded6 100644 --- a/src/shared/chat/baseview.js +++ b/src/shared/chat/baseview.js @@ -13,6 +13,12 @@ export default class BaseChatView extends ElementView { this.debouncedScrollDown = debounce(this.scrollDown, 100); } + disconnectedCallback () { + super.disconnectedCallback(); + const jid = this.getAttribute('jid'); + _converse.chatboxviews.remove(jid, this); + } + hideNewMessagesIndicator () { const new_msgs_indicator = this.querySelector('.new-msgs-indicator'); if (new_msgs_indicator !== null) { @@ -195,7 +201,7 @@ export default class BaseChatView extends ElementView { 'scrollTop': null }); } - this.querySelector('.chat-content__messages').scrollDown(); + this.querySelector('.chat-content__messages')?.scrollDown(); this.onScrolledDown(); }