From 531ebf335cc2508b81bb057615922e089ee85d37 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Mon, 6 Apr 2020 14:27:44 +0200 Subject: [PATCH] Properly test and fix show/hide of MUC topic --- CHANGES.md | 2 +- sass/_chatbox.scss | 4 +- spec/bookmarks.js | 7 +- spec/muc.js | 142 ++++++++++++++++++++++----------- spec/xss.js | 3 +- src/components/dropdown.js | 1 - src/converse-bookmark-views.js | 4 +- src/converse-minimize.js | 4 +- src/converse-muc-views.js | 19 +++-- src/headless/converse-core.js | 23 +++++- 10 files changed, 138 insertions(+), 71 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index b2ab25d10..ab803ec9c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -18,6 +18,7 @@ Soon we'll deprecate the latter, so prepare now. - #1826: A user can now add himself as a contact - #1839: Headline messages are shown in controlbox - #1896: Don't send receipts for messages fetched from the archive +- #1937: Editing a message removes the mentions highlight - Allow ignoring of bootstrap modules at build using environment variable. For xample: `export BOOTSTRAP_IGNORE_MODULES="Modal,Dropdown" && make dist` - Bugfix. Handle stanza that clears the MUC subject - New config option [modtools_disable_assign](https://conversejs.org/docs/html/configuration.html#modtools-disable-assign) @@ -26,7 +27,6 @@ Soon we'll deprecate the latter, so prepare now. - Start using [lit-html](https://lit-html.polymer-project.org/) instead of lodash for templating. - [muc_fetch_members](https://conversejs.org/docs/html/configuration.html#muc-fetch-members) now also accepts an array of affiliations to fetch. - Remove the configuration setting `muc_show_join_leave_status`. The optional status message is no longer shown at all. -- #1937: Editing a message removes the mentions highlight ## 6.0.0 (2020-01-09) diff --git a/sass/_chatbox.scss b/sass/_chatbox.scss index 0945020d1..0f7e4188e 100644 --- a/sass/_chatbox.scss +++ b/sass/_chatbox.scss @@ -57,10 +57,10 @@ font-size: var(--font-size-small); margin: 0; overflow: hidden; - padding: 0.5rem 1rem 1rem 1rem; + padding: 0.5rem 1rem 0.5rem 1rem; text-overflow: ellipsis; - white-space: nowrap; width: 100%; + max-height: 5em; } .chatbox-title { diff --git a/spec/bookmarks.js b/spec/bookmarks.js index 15082552a..4c05bcd16 100644 --- a/spec/bookmarks.js +++ b/spec/bookmarks.js @@ -37,7 +37,7 @@ spyOn(view, 'renderBookmarkForm').and.callThrough(); spyOn(view, 'closeForm').and.callThrough(); await u.waitUntil(() => view.el.querySelector('.toggle-bookmark') !== null); - let toggle = view.el.querySelector('.toggle-bookmark'); + const toggle = view.el.querySelector('.toggle-bookmark'); expect(toggle.title).toBe('Bookmark this groupchat'); toggle.click(); expect(view.renderBookmarkForm).toHaveBeenCalled(); @@ -130,10 +130,9 @@ }); _converse.connection._dataRecv(test_utils.createRequest(stanza)); await u.waitUntil(() => view.model.get('bookmarked')); - toggle = await u.waitUntil(() => view.el.querySelector('.toggle-bookmark')); expect(view.model.get('bookmarked')).toBeTruthy(); - expect(toggle.title).toBe('Unbookmark this groupchat'); - expect(u.hasClass('on-button', toggle), true); + await u.waitUntil(() => view.el.querySelector('.toggle-bookmark')?.title === 'Unbookmark this groupchat'); + expect(u.hasClass('on-button', view.el.querySelector('.toggle-bookmark')), true); // We ignore this IQ stanza... (unless it's an error stanza), so // nothing to test for here. done(); diff --git a/spec/muc.js b/spec/muc.js index 06c7e4dc8..82babb88e 100644 --- a/spec/muc.js +++ b/spec/muc.js @@ -501,6 +501,100 @@ }); }); + describe("the topic", function () { + + it("is shown the header", + mock.initConverse( + ['rosterGroupsFetched'], {}, + async function (done, _converse) { + + await test_utils.openAndEnterChatRoom(_converse, 'jdev@conference.jabber.org', 'jc'); + const text = 'Jabber/XMPP Development | RFCs and Extensions: https://xmpp.org/ | Protocol and XSF discussions: xsf@muc.xmpp.org'; + let stanza = u.toStanza(` + + ${text} + + + `); + _converse.connection._dataRecv(test_utils.createRequest(stanza)); + const view = _converse.chatboxviews.get('jdev@conference.jabber.org'); + await new Promise(resolve => view.model.once('change:subject', resolve)); + + expect(sizzle('.chat-event:last', view.el).pop().textContent.trim()).toBe('Topic set by ralphm'); + const head_desc = await u.waitUntil(() => view.el.querySelector('.chat-head__desc')); + expect(head_desc?.textContent.trim()).toBe(text); + + stanza = u.toStanza( + ` + This is a message subject + This is a message + `); + _converse.connection._dataRecv(test_utils.createRequest(stanza)); + await new Promise(resolve => view.once('messageInserted', resolve)); + expect(sizzle('.chat-msg__subject', view.el).length).toBe(1); + expect(sizzle('.chat-msg__subject', view.el).pop().textContent.trim()).toBe('This is a message subject'); + expect(sizzle('.chat-msg__text').length).toBe(1); + expect(sizzle('.chat-msg__text').pop().textContent.trim()).toBe('This is a message'); + expect(view.el.querySelector('.chat-head__desc').textContent.trim()).toBe(text); + + // Removes current topic + stanza = u.toStanza( + ` + + `); + _converse.connection._dataRecv(test_utils.createRequest(stanza)); + await new Promise(resolve => view.model.once('change:subject', resolve)); + await u.waitUntil(() => view.el.querySelector('.chat-head__desc') === null); + expect(view.el.querySelector('.chat-info:last-child').textContent.trim()).toBe("Topic cleared by ralphm"); + done(); + })); + + it("can be toggled by the user", + mock.initConverse( + ['rosterGroupsFetched'], {}, + async function (done, _converse) { + + await test_utils.openAndEnterChatRoom(_converse, 'jdev@conference.jabber.org', 'jc'); + const text = 'Jabber/XMPP Development | RFCs and Extensions: https://xmpp.org/ | Protocol and XSF discussions: xsf@muc.xmpp.org'; + let stanza = u.toStanza(` + + ${text} + + + `); + _converse.connection._dataRecv(test_utils.createRequest(stanza)); + const view = _converse.chatboxviews.get('jdev@conference.jabber.org'); + await new Promise(resolve => view.model.once('change:subject', resolve)); + + expect(sizzle('.chat-event:last', view.el).pop().textContent.trim()).toBe('Topic set by ralphm'); + const head_desc = await u.waitUntil(() => view.el.querySelector('.chat-head__desc')); + expect(head_desc?.textContent.trim()).toBe(text); + + stanza = u.toStanza( + ` + This is a message subject + This is a message + `); + _converse.connection._dataRecv(test_utils.createRequest(stanza)); + await new Promise(resolve => view.once('messageInserted', resolve)); + expect(sizzle('.chat-msg__subject', view.el).length).toBe(1); + expect(sizzle('.chat-msg__subject', view.el).pop().textContent.trim()).toBe('This is a message subject'); + expect(sizzle('.chat-msg__text').length).toBe(1); + expect(sizzle('.chat-msg__text').pop().textContent.trim()).toBe('This is a message'); + const topic_el = view.el.querySelector('.chat-head__desc'); + expect(topic_el.textContent.trim()).toBe(text); + expect(u.isVisible(topic_el)).toBe(true); + + const toggle = view.el.querySelector('.hide-topic'); + expect(toggle.textContent).toBe('Hide topic'); + toggle.click(); + await u.waitUntil(() => !u.isVisible(topic_el)); + expect(view.el.querySelector('.hide-topic').textContent).toBe('Show topic'); + done(); + })); + }); + + it("clears cached messages when it gets closed and clear_messages_on_reconnection is true", mock.initConverse( ['rosterGroupsFetched'], {'clear_messages_on_reconnection': true}, @@ -1930,52 +2024,6 @@ }, 500); })); - it("shows the room topic in the header", - mock.initConverse( - ['rosterGroupsFetched'], {}, - async function (done, _converse) { - - await test_utils.openAndEnterChatRoom(_converse, 'jdev@conference.jabber.org', 'jc'); - const text = 'Jabber/XMPP Development | RFCs and Extensions: https://xmpp.org/ | Protocol and XSF discussions: xsf@muc.xmpp.org'; - let stanza = u.toStanza(` - - ${text} - - - `); - _converse.connection._dataRecv(test_utils.createRequest(stanza)); - const view = _converse.chatboxviews.get('jdev@conference.jabber.org'); - await new Promise(resolve => view.model.once('change:subject', resolve)); - - expect(sizzle('.chat-event:last', view.el).pop().textContent.trim()).toBe('Topic set by ralphm'); - const head_desc = await u.waitUntil(() => view.el.querySelector('.chat-head__desc')); - expect(head_desc?.textContent.trim()).toBe(text); - - stanza = u.toStanza( - ` - This is a message subject - This is a message - `); - _converse.connection._dataRecv(test_utils.createRequest(stanza)); - await new Promise(resolve => view.once('messageInserted', resolve)); - expect(sizzle('.chat-msg__subject', view.el).length).toBe(1); - expect(sizzle('.chat-msg__subject', view.el).pop().textContent.trim()).toBe('This is a message subject'); - expect(sizzle('.chat-msg__text').length).toBe(1); - expect(sizzle('.chat-msg__text').pop().textContent.trim()).toBe('This is a message'); - expect(view.el.querySelector('.chat-head__desc').textContent.trim()).toBe(text); - - // Removes current topic - stanza = u.toStanza( - ` - - `); - _converse.connection._dataRecv(test_utils.createRequest(stanza)); - await new Promise(resolve => view.model.once('change:subject', resolve)); - await u.waitUntil(() => view.el.querySelector('.chat-head__desc') === null); - expect(view.el.querySelector('.chat-info:last-child').textContent.trim()).toBe("Topic cleared by ralphm"); - done(); - })); - it("reconnects when no-acceptable error is returned when sending a message", mock.initConverse( ['rosterGroupsFetched'], {}, @@ -2438,7 +2486,7 @@ expect(view.model.features.get('temporary')).toBe(true); expect(view.model.features.get('unmoderated')).toBe(true); expect(view.model.features.get('unsecured')).toBe(false); - expect(view.el.querySelector('.chatbox-title__text').textContent.trim()).toBe('New room name'); + await u.waitUntil(() => view.el.querySelector('.chatbox-title__text')?.textContent.trim() === 'New room name'); done(); })); diff --git a/spec/xss.js b/spec/xss.js index f45f272eb..7c729cdcf 100644 --- a/spec/xss.js +++ b/spec/xss.js @@ -236,8 +236,7 @@ 'author': 'ralphm' }}); expect(sizzle('.chat-event:last').pop().textContent.trim()).toBe('Topic set by ralphm'); - const desc = await u.waitUntil(() => view.el.querySelector('.chat-head__desc')); - expect(desc.textContent.trim()).toBe(subject); + await u.waitUntil(() => view.el.querySelector('.chat-head__desc')?.textContent.trim() === subject); done(); })); }); diff --git a/src/components/dropdown.js b/src/components/dropdown.js index dfde03dd3..9bab07485 100644 --- a/src/components/dropdown.js +++ b/src/components/dropdown.js @@ -8,7 +8,6 @@ import converse from "@converse/headless/converse-core"; const u = converse.env.utils; - export class Dropdown extends CustomElement { static get properties () { diff --git a/src/converse-bookmark-views.js b/src/converse-bookmark-views.js index 3ddaee80f..ce9f66daf 100644 --- a/src/converse-bookmark-views.js +++ b/src/converse-bookmark-views.js @@ -36,9 +36,9 @@ converse.plugins.add('converse-bookmark-views', { // plugin architecture they will replace existing methods on the // relevant objects or classes. ChatRoomView: { - async getHeadingButtons () { + getHeadingButtons () { const { _converse } = this.__super__; - const buttons = await this.__super__.getHeadingButtons.call(this); + const buttons = this.__super__.getHeadingButtons.apply(this, arguments); if (_converse.allow_bookmarks) { const supported = _converse.checkBookmarksSupport(); const bookmarked = this.model.get('bookmarked'); diff --git a/src/converse-minimize.js b/src/converse-minimize.js index e4e5c91ab..14595f5c9 100644 --- a/src/converse-minimize.js +++ b/src/converse-minimize.js @@ -139,9 +139,9 @@ converse.plugins.add('converse-minimize', { return result; }, - async getHeadingButtons () { + getHeadingButtons () { const { _converse } = this.__super__; - const buttons = await this.__super__.getHeadingButtons.call(this); + const buttons = this.__super__.getHeadingButtons.apply(this, arguments); const data = { 'a_class': 'toggle-chatbox-button', 'handler': ev => this.minimize(ev), diff --git a/src/converse-muc-views.js b/src/converse-muc-views.js index 666fe4893..5fb0da35c 100644 --- a/src/converse-muc-views.js +++ b/src/converse-muc-views.js @@ -8,7 +8,7 @@ import "converse-modal"; import "@converse/headless/utils/muc"; import { Model } from 'skeletor.js/src/model.js'; import { View } from 'skeletor.js/src/view.js'; -import { head, isString, isUndefined } from "lodash"; +import { debounce, head, isString, isUndefined } from "lodash"; import { BootstrapModal } from "./converse-modal.js"; import { render } from "lit-html"; import { __ } from '@converse/headless/i18n'; @@ -703,7 +703,10 @@ converse.plugins.add('converse-muc-views', { this.listenTo(this.model.notifications, 'change', this.renderNotifications); this.listenTo(this.model.session, 'change:connection_status', this.onConnectionStatusChanged); - this.listenTo(this.model, 'change', this.renderHeading); + const user_settings = _converse.api.user.settings.getModel(); + this.listenTo(user_settings, 'change:mucs_with_hidden_subject', this.renderHeading); + + this.listenTo(this.model, 'change', debounce(() => this.renderHeading(), 250)); this.listenTo(this.model, 'change:hidden_occupants', this.updateOccupantsToggle); this.listenTo(this.model, 'change:subject', this.setChatRoomSubject); this.listenTo(this.model, 'configurationNeeded', this.getAndRenderConfigurationForm); @@ -1171,7 +1174,7 @@ converse.plugins.add('converse-muc-views', { } }, - async getHeadingButtons () { + getHeadingButtons (subject_hidden) { const buttons = [{ 'i18n_text': __('Details'), 'i18n_title': __('Show more information about this groupchat'), @@ -1227,9 +1230,6 @@ converse.plugins.add('converse-muc-views', { }); } - const muc_jid = this.model.get('jid'); - const jids = await api.user.settings.get('mucs_with_hidden_subject', []) - const subject_hidden = jids.includes(muc_jid); const subject = this.model.get('subject'); if (subject && subject.text) { buttons.push({ @@ -1238,7 +1238,7 @@ converse.plugins.add('converse-muc-views', { __('Show the topic message in the heading') : __('Hide the topic in the heading'), 'handler': ev => this.toggleTopic(ev), - 'a_class': '', + 'a_class': 'hide-topic', 'icon_class': 'fa-minus-square', 'name': 'toggle-topic' }); @@ -1268,12 +1268,15 @@ converse.plugins.add('converse-muc-views', { * @method _converse.ChatRoomView#generateHeadingTemplate */ async generateHeadingTemplate () { - const heading_btns = await this.getHeadingButtons(); + const jids = await api.user.settings.get('mucs_with_hidden_subject', []) + const subject_hidden = jids.includes(this.model.get('jid')); + const heading_btns = this.getHeadingButtons(subject_hidden); const standalone_btns = heading_btns.filter(b => b.standalone); const dropdown_btns = heading_btns.filter(b => !b.standalone); return tpl_chatroom_head( Object.assign(this.model.toJSON(), { _converse, + subject_hidden, 'dropdown_btns': dropdown_btns.map(b => this.getHeadingDropdownItem(b)), 'standalone_btns': standalone_btns.map(b => this.getHeadingStandaloneButton(b)), 'title': this.model.getDisplayName(), diff --git a/src/headless/converse-core.js b/src/headless/converse-core.js index 861afedf6..3c0180f6a 100644 --- a/src/headless/converse-core.js +++ b/src/headless/converse-core.js @@ -529,6 +529,16 @@ const api = _converse.api = { * @memberOf _converse.api.user */ settings: { + /** + * Returns the user settings model. Useful when you want to listen for change events. + * @method _converse.api.user.settings.getModel + * @returns {Model} + * @example _converse.api.user.settings.getModel + */ + getModel () { + return user_settings; + }, + /** * Get the value of a particular user setting. * @method _converse.api.user.settings.get @@ -558,8 +568,13 @@ const api = _converse.api = { */ async set (key, val) { await initUserSettings(); - const o = isObject(key) ? key : {key: val}; - return user_settings.save(o, {'promise': true}); + if (isObject(key)) { + return user_settings.save(key, {'promise': true}); + } else { + const o = {}; + o[key] = val; + return user_settings.save(o, {'promise': true}); + } } } }, @@ -1086,6 +1101,10 @@ function clearSession () { _converse.session.destroy(); delete _converse.session; } + if (_converse.shouldClearCache()) { + const model = _converse.api.user.settings.getModel(); + model.clear(); + } /** * Synchronouse event triggered once the user session has been cleared, * for example when the user has logged out or when Converse has