From c08ee00fcd54dc0f41f036130605dc3532640f15 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Tue, 27 Oct 2020 11:57:23 +0100 Subject: [PATCH] Enforce uni-view in headless code When we're showing only one chat at a time, we want to make sure that all other chats have their `hidden` flag set to `true`. Previously this was done in chatboxviews, but given that we want to render UI based on state, this should be done in the headless part of Converse. As a result of the changes, the `beforeShowingChatView` has been removed. --- spec/bookmarks.js | 51 +++++++++++++++++++++++++++++++++-- src/converse-chatboxviews.js | 33 ----------------------- src/converse-chatview.js | 24 ++++------------- src/converse-minimize.js | 7 +---- src/headless/converse-chat.js | 12 ++++++++- 5 files changed, 66 insertions(+), 61 deletions(-) diff --git a/spec/bookmarks.js b/spec/bookmarks.js index 0a55dd6a1..b9aa515d6 100644 --- a/spec/bookmarks.js +++ b/spec/bookmarks.js @@ -1,6 +1,6 @@ /* global mock, converse */ -const Strophe = converse.env.Strophe; +const { Strophe, u, sizzle, $iq } = converse.env; describe("A chat room", function () { @@ -497,7 +497,6 @@ describe("Bookmarks", function () { ); mock.openControlBox(_converse); - const { Strophe, u, sizzle, $iq } = converse.env; const IQ_stanzas = _converse.connection.IQ_stanzas; const sent_stanza = await u.waitUntil( () => IQ_stanzas.filter(s => sizzle('items[node="storage:bookmarks"]', s).length).pop()); @@ -562,6 +561,54 @@ describe("Bookmarks", function () { done(); })); + it("can be used to open a MUC from a bookmark", mock.initConverse( + ['rosterGroupsFetched'], {'view_mode': 'fullscreen'}, async function (done, _converse) { + + const api = _converse.api; + await mock.waitUntilDiscoConfirmed( + _converse, _converse.bare_jid, + [{'category': 'pubsub', 'type': 'pep'}], + ['http://jabber.org/protocol/pubsub#publish-options'] + ); + await mock.openControlBox(_converse); + const view = await _converse.chatboxviews.get('controlbox'); + const IQ_stanzas = _converse.connection.IQ_stanzas; + const sent_stanza = await u.waitUntil( + () => IQ_stanzas.filter(s => sizzle('items[node="storage:bookmarks"]', s).length).pop()); + const stanza = $iq({'to': _converse.connection.jid, 'type':'result', 'id':sent_stanza.getAttribute('id')}) + .c('pubsub', {'xmlns': Strophe.NS.PUBSUB}) + .c('items', {'node': 'storage:bookmarks'}) + .c('item', {'id': 'current'}) + .c('storage', {'xmlns': 'storage:bookmarks'}) + .c('conference', { + 'name': 'The Play's the Thing', + 'autojoin': 'false', + 'jid': 'theplay@conference.shakespeare.lit' + }).c('nick').t('JC').up().up() + .c('conference', { + 'name': '1st Bookmark', + 'autojoin': 'false', + 'jid': 'first@conference.shakespeare.lit' + }).c('nick').t('JC'); + _converse.connection._dataRecv(mock.createRequest(stanza)); + await u.waitUntil(() => view.el.querySelectorAll('#chatrooms div.bookmarks.rooms-list .room-item').length); + expect(view.el.querySelectorAll('#chatrooms div.bookmarks.rooms-list .room-item').length).toBe(2); + view.el.querySelector('.bookmarks.rooms-list .open-room').click(); + await u.waitUntil(() => _converse.chatboxes.length === 2); + expect((await api.rooms.get('first@conference.shakespeare.lit')).get('hidden')).toBe(false); + + await u.waitUntil(() => view.el.querySelectorAll('.list-container--bookmarks .available-chatroom:not(.hidden)').length === 1); + view.el.querySelector('.list-container--bookmarks .available-chatroom:not(.hidden) .open-room').click(); + await u.waitUntil(() => _converse.chatboxes.length === 3); + expect((await api.rooms.get('first@conference.shakespeare.lit')).get('hidden')).toBe(true); + expect((await api.rooms.get('theplay@conference.shakespeare.lit')).get('hidden')).toBe(false); + + view.el.querySelector('.list-container--openrooms .open-room:first-child').click(); + await u.waitUntil(() => view.el.querySelector('.list-item.open').getAttribute('data-room-jid') === 'first@conference.shakespeare.lit'); + expect((await api.rooms.get('first@conference.shakespeare.lit')).get('hidden')).toBe(false); + expect((await api.rooms.get('theplay@conference.shakespeare.lit')).get('hidden')).toBe(true); + done(); + })); it("remembers the toggle state of the bookmarks list", mock.initConverse( ['rosterGroupsFetched'], {}, async function (done, _converse) { diff --git a/src/converse-chatboxviews.js b/src/converse-chatboxviews.js index b7133ac1c..64422e57b 100644 --- a/src/converse-chatboxviews.js +++ b/src/converse-chatboxviews.js @@ -121,38 +121,6 @@ function onChatBoxViewsInitialized () { } -function hideChat (view) { - if (view.model.get('id') === 'controlbox') { return; } - u.safeSave(view.model, {'hidden': true}); - view.hide(); -} - - -function beforeShowingChatView (view) { - if (_converse.isUniView()) { - /* We only have one chat visible at any one - * time. So before opening a chat, we make sure all other - * chats are hidden. - */ - Object.values(_converse.chatboxviews.xget(view.model.get('id'))) - .filter(v => !v.model.get('hidden')) - .forEach(hideChat); - - if (view.model.get('hidden')) { - return new Promise(resolve => { - u.safeSave( - view.model, - {'hidden': false}, { - 'success': resolve, - 'failure': resolve - } - ); - }); - } - } -} - - function calculateViewportHeightUnit () { const vh = window.innerHeight * 0.01; document.documentElement.style.setProperty('--vh', `${vh}px`); @@ -185,7 +153,6 @@ converse.plugins.add('converse-chatboxviews', { _converse.ChatBoxViews = ChatBoxViews; /************************ BEGIN Event Handlers ************************/ - api.listen.on('beforeShowingChatView', beforeShowingChatView); api.listen.on('chatBoxesInitialized', onChatBoxViewsInitialized); api.listen.on('cleanup', () => (delete _converse.chatboxviews)); api.listen.on('clearSession', () => _converse.chatboxviews.closeAllChatBoxes()); diff --git a/src/converse-chatview.js b/src/converse-chatview.js index aeace07fe..c913c7539 100644 --- a/src/converse-chatview.js +++ b/src/converse-chatview.js @@ -231,8 +231,7 @@ export const ChatBoxView = View.extend({ showControlBox () { // Used in mobile view, to navigate back to the controlbox - const view = _converse.chatboxviews.get('controlbox'); - view.show(); + _converse.chatboxviews.get('controlbox')?.show(); this.hide(); }, @@ -451,14 +450,6 @@ export const ChatBoxView = View.extend({ }); }, - shouldShowOnTextMessage () { - if (_converse.isUniView()) { - return false; - } else { - return !u.isVisible(this.el); - } - }, - /** * Given a message element, determine wether it should be * marked as a followup message to the previous element. @@ -929,19 +920,14 @@ export const ChatBoxView = View.extend({ }, show () { + if (this.model.get('hidden')) { + log.debug(`Not showing chat ${this.model.get('jid')} because it's set as hidden`); + return; + } if (u.isVisible(this.el)) { this.maybeFocus(); return; } - /** - * Triggered just before a {@link _converse.ChatBoxView} or {@link _converse.ChatRoomView} - * will be shown. - * @event _converse#beforeShowingChatView - * @type {object} - * @property { _converse.ChatBoxView | _converse.ChatRoomView } view - */ - api.trigger('beforeShowingChatView', this); - if (api.settings.get('animate')) { u.fadeIn(this.el, () => this.afterShown()); } else { diff --git a/src/converse-minimize.js b/src/converse-minimize.js index 1ad34345f..283bc5a5a 100644 --- a/src/converse-minimize.js +++ b/src/converse-minimize.js @@ -92,11 +92,6 @@ converse.plugins.add('converse-minimize', { this.__super__.isNewMessageHidden.apply(this, arguments); }, - shouldShowOnTextMessage () { - return !this.model.get('minimized') && - this.__super__.shouldShowOnTextMessage.apply(this, arguments); - }, - setChatBoxHeight (height) { if (!this.model.get('minimized')) { return this.__super__.setChatBoxHeight.call(this, height); @@ -107,7 +102,7 @@ converse.plugins.add('converse-minimize', { if (!this.model.get('minimized')) { return this.__super__.setChatBoxWidth.call(this, width); } - }, + } } }, diff --git a/src/headless/converse-chat.js b/src/headless/converse-chat.js index 3ba1e8048..f61fd977c 100644 --- a/src/headless/converse-chat.js +++ b/src/headless/converse-chat.js @@ -1136,7 +1136,17 @@ converse.plugins.add('converse-chat', { }, maybeShow (force) { - force && u.safeSave(this, {'hidden': false}); + if (force) { + if (_converse.isUniView()) { + // We only have one chat visible at any one time. + // So before opening a chat, we make sure all other chats are hidden. + const filter = c => !c.get('hidden') && + c.get('jid') !== this.get('jid') && + c.get('id') !== 'controlbox'; + _converse.chatboxes.filter(filter).forEach(c => u.safeSave(c, {'hidden': true})); + } + u.safeSave(this, {'hidden': false}); + } if (_converse.isUniView() && this.get('hidden')) { return; } else {