From 34a4a70ae21cbe08da7f26f03bca48f39e28e552 Mon Sep 17 00:00:00 2001
From: JC Brand <jc@opkode.com>
Date: Fri, 30 Sep 2022 16:10:35 +0200
Subject: [PATCH] Move bookmarks into a modal

---
 CHANGES.md                                    |   2 +
 docs/source/configuration.rst                 |  16 ---
 .../{form.js => components/bookmark-form.js}  |   0
 .../{ => components}/bookmarks-list.js        |  17 +--
 .../{ => components}/templates/form.js        |   0
 .../{ => components}/templates/item.js        |   6 +-
 .../components/templates/list.js              |  35 ++++++
 src/plugins/bookmark-views/index.js           |   6 +-
 src/plugins/bookmark-views/mixins.js          |   1 -
 .../{modal.js => modals/bookmark-form.js}     |   2 +-
 .../bookmark-views/modals/bookmark-list.js    |  18 +++
 .../bookmark-views/styles/bookmarks.scss      |   6 +
 src/plugins/bookmark-views/templates/list.js  |  32 ------
 .../bookmark-views/tests/bookmarks-list.js    | 108 ++++++------------
 src/plugins/bookmark-views/tests/bookmarks.js |  37 ------
 src/plugins/bookmark-views/utils.js           |   1 -
 .../controlbox/templates/controlbox.js        |   5 +-
 src/plugins/roomslist/templates/roomslist.js  |  25 ++--
 src/plugins/roomslist/view.js                 |   3 +-
 src/shared/styles/forms.scss                  |  16 +--
 20 files changed, 140 insertions(+), 196 deletions(-)
 rename src/plugins/bookmark-views/{form.js => components/bookmark-form.js} (100%)
 rename src/plugins/bookmark-views/{ => components}/bookmarks-list.js (78%)
 rename src/plugins/bookmark-views/{ => components}/templates/form.js (100%)
 rename src/plugins/bookmark-views/{ => components}/templates/item.js (69%)
 create mode 100644 src/plugins/bookmark-views/components/templates/list.js
 rename src/plugins/bookmark-views/{modal.js => modals/bookmark-form.js} (93%)
 create mode 100644 src/plugins/bookmark-views/modals/bookmark-list.js
 delete mode 100644 src/plugins/bookmark-views/templates/list.js

diff --git a/CHANGES.md b/CHANGES.md
index 894b06644..f817d7ea1 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -12,6 +12,7 @@
 - Update `nick` attribute on ChatRoom when user nickname changes
 - Restrict editing of MUC messages to ones with the same XEP-0421 occupant ID
 - Remove unfurls for links removed in a subsequent message correction
+- Bookmarks now appear in a modal and the `hide_open_bookmarks` config setting has been removed.
 - #1004: Stop using fonts to render icons and use SVG instead
 - #2797: Weird unicode characters rendering outside of line-height
 - #2870: Fix for multiple URLs to be linkified when sent together in chat and adds a test for this.
@@ -22,6 +23,7 @@
 - #3007: Fix links becoming text when a message is edited
 - #3018: Fix MUC icons not functioning.
 
+
 ## 9.1.1 (2022-05-05)
 
 - GIFs don't render inside unfurls and cause a TypeError
diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst
index afe7901d2..d0b469d2e 100644
--- a/docs/source/configuration.rst
+++ b/docs/source/configuration.rst
@@ -962,22 +962,6 @@ hide_offline_users
 
 If set to ``true``, then offline users aren't shown in the roster.
 
-hide_open_bookmarks
--------------------
-
-* Default:  ``false`` (``true`` when the `view_mode`_ is set to ``fullscreen``).
-
-This setting applies to the ``converse-bookmarks`` plugin and specfically the
-list of bookmarks shown in the ``Rooms`` tab of the control box.
-
-By default all bookmarks are shown in that list, if this setting is set to
-``true``, then only bookmarks for rooms not currently open (i.e. that the
-current user hasn't joined), are shown.
-
-Makes sense to set this to ``true`` when also using the non-core
-``converse-roomslist`` plugin, which shows a list of currently open (i.e.
-"joined") rooms.
-
 .. _`i18n`:
 
 i18n
diff --git a/src/plugins/bookmark-views/form.js b/src/plugins/bookmark-views/components/bookmark-form.js
similarity index 100%
rename from src/plugins/bookmark-views/form.js
rename to src/plugins/bookmark-views/components/bookmark-form.js
diff --git a/src/plugins/bookmark-views/bookmarks-list.js b/src/plugins/bookmark-views/components/bookmarks-list.js
similarity index 78%
rename from src/plugins/bookmark-views/bookmarks-list.js
rename to src/plugins/bookmark-views/components/bookmarks-list.js
index 68b493eea..87bcbb44d 100644
--- a/src/plugins/bookmark-views/bookmarks-list.js
+++ b/src/plugins/bookmark-views/components/bookmarks-list.js
@@ -1,8 +1,12 @@
+import debounce from "lodash-es/debounce";
 import tpl_bookmarks_list from './templates/list.js';
+import tpl_spinner from "templates/spinner.js";
 import { CustomElement } from 'shared/components/element.js';
 import { _converse, api } from '@converse/headless/core.js';
 import { initStorage } from '@converse/headless/utils/storage.js';
 
+import '../styles/bookmarks.scss';
+
 
 export default class BookmarksView extends CustomElement {
 
@@ -10,6 +14,8 @@ export default class BookmarksView extends CustomElement {
         await api.waitUntil('bookmarksInitialized');
         const { bookmarks, chatboxes } = _converse;
 
+        this.liveFilter = debounce((ev) => this.model.set({'filter_text': ev.target.value}), 100);
+
         this.listenTo(bookmarks, 'add', () => this.requestUpdate());
         this.listenTo(bookmarks, 'remove', () => this.requestUpdate());
 
@@ -29,15 +35,12 @@ export default class BookmarksView extends CustomElement {
     }
 
     render () {
-        return _converse.bookmarks && this.model ? tpl_bookmarks_list(this) : '';
+        return _converse.bookmarks && this.model ? tpl_bookmarks_list(this) : tpl_spinner();
     }
 
-    toggleBookmarksList (ev) {
-        ev?.preventDefault?.();
-        const { CLOSED, OPENED } = _converse;
-        this.model.save({
-            'toggle-state': this.model.get('toggle-state') === CLOSED ? OPENED : CLOSED
-        });
+    clearFilter (ev) {
+        ev?.stopPropagation?.();
+        this.model.set('filter_text', '');
     }
 }
 
diff --git a/src/plugins/bookmark-views/templates/form.js b/src/plugins/bookmark-views/components/templates/form.js
similarity index 100%
rename from src/plugins/bookmark-views/templates/form.js
rename to src/plugins/bookmark-views/components/templates/form.js
diff --git a/src/plugins/bookmark-views/templates/item.js b/src/plugins/bookmark-views/components/templates/item.js
similarity index 69%
rename from src/plugins/bookmark-views/templates/item.js
rename to src/plugins/bookmark-views/components/templates/item.js
index 4a1380ac7..35cb566ff 100644
--- a/src/plugins/bookmark-views/templates/item.js
+++ b/src/plugins/bookmark-views/components/templates/item.js
@@ -1,15 +1,13 @@
 import { __ } from 'i18n';
-import { _converse, api } from '@converse/headless/core.js';
 import { html } from "lit";
-import { openRoomViaEvent, removeBookmarkViaEvent } from '../utils.js';
+import { openRoomViaEvent, removeBookmarkViaEvent } from '../../utils.js';
 
 export default (bm) => {
     const jid = bm.get('jid');
-    const is_hidden = !!(api.settings.get('hide_open_bookmarks') && _converse.chatboxes.get(jid));
     const info_remove_bookmark = __('Unbookmark this groupchat');
     const open_title = __('Click to open this groupchat');
     return html`
-        <div class="list-item controlbox-padded room-item available-chatroom d-flex flex-row ${ is_hidden ? 'hidden' : ''}" data-room-jid="${jid}">
+        <div class="list-item room-item available-chatroom d-flex flex-row" data-room-jid="${jid}">
             <a class="list-item-link open-room w-100" data-room-jid="${jid}"
                 title="${open_title}"
                 @click=${openRoomViaEvent}>${bm.getDisplayName()}</a>
diff --git a/src/plugins/bookmark-views/components/templates/list.js b/src/plugins/bookmark-views/components/templates/list.js
new file mode 100644
index 000000000..94e42dbb7
--- /dev/null
+++ b/src/plugins/bookmark-views/components/templates/list.js
@@ -0,0 +1,35 @@
+import bookmark_item from './item.js';
+import { __ } from 'i18n';
+import { _converse } from '@converse/headless/core.js';
+import { html } from "lit";
+
+const filterBookmark = (b, text) => b.get('name')?.includes(text) || b.get('jid')?.includes(text);
+
+export default (el) => {
+    const i18n_placeholder = __('Filter');
+    const filter_text = el.model.get('filter_text');
+    const { bookmarks } = _converse;
+    const shown_bookmarks = filter_text ? bookmarks.filter(b => filterBookmark(b, filter_text)) : bookmarks;
+
+    return html`
+        <form class="converse-form bookmarks-filter">
+            <div class="btn-group w-100">
+                <input
+                    .value=${filter_text ?? ''}
+                    @keydown="${ev => el.liveFilter(ev)}"
+                    class="form-control"
+                    placeholder="${i18n_placeholder}"/>
+
+                <converse-icon size="1em" class="fa fa-times clear-input ${ !filter_text ? 'hidden' : '' }"
+                    @click=${el.clearFilter}>
+                </converse-icon>
+            </div>
+        </form>
+
+        <div class="list-container list-container--bookmarks">
+            <div class="items-list bookmarks rooms-list">
+                ${ shown_bookmarks.map(bm => bookmark_item(bm)) }
+            </div>
+        </div>
+    `;
+}
diff --git a/src/plugins/bookmark-views/index.js b/src/plugins/bookmark-views/index.js
index 4a6291233..b731c9f24 100644
--- a/src/plugins/bookmark-views/index.js
+++ b/src/plugins/bookmark-views/index.js
@@ -3,9 +3,11 @@
  * @copyright 2022, the Converse.js contributors
  * @license Mozilla Public License (MPLv2)
  */
+import './modals/bookmark-list.js';
+import './modals/bookmark-form.js';
 import '@converse/headless/plugins/muc/index.js';
-import BookmarkForm from './form.js';
-import BookmarksView from './bookmarks-list.js';
+import BookmarkForm from './components/bookmark-form.js';
+import BookmarksView from './components/bookmarks-list.js';
 import { _converse, api, converse } from '@converse/headless/core';
 import { bookmarkableChatRoomView } from './mixins.js';
 import { getHeadingButtons, removeBookmarkViaEvent, addBookmarkViaEvent } from './utils.js';
diff --git a/src/plugins/bookmark-views/mixins.js b/src/plugins/bookmark-views/mixins.js
index e95640057..693b98b0c 100644
--- a/src/plugins/bookmark-views/mixins.js
+++ b/src/plugins/bookmark-views/mixins.js
@@ -1,4 +1,3 @@
-import './modal.js';
 import { _converse, api, converse } from '@converse/headless/core';
 
 const { u } = converse.env;
diff --git a/src/plugins/bookmark-views/modal.js b/src/plugins/bookmark-views/modals/bookmark-form.js
similarity index 93%
rename from src/plugins/bookmark-views/modal.js
rename to src/plugins/bookmark-views/modals/bookmark-form.js
index 29008b835..85052284d 100644
--- a/src/plugins/bookmark-views/modal.js
+++ b/src/plugins/bookmark-views/modals/bookmark-form.js
@@ -1,4 +1,4 @@
-import './form.js';
+import '../components/bookmark-form.js';
 import BaseModal from "plugins/modal/modal.js";
 import { html } from "lit";
 import { __ } from 'i18n';
diff --git a/src/plugins/bookmark-views/modals/bookmark-list.js b/src/plugins/bookmark-views/modals/bookmark-list.js
new file mode 100644
index 000000000..ad72a63c7
--- /dev/null
+++ b/src/plugins/bookmark-views/modals/bookmark-list.js
@@ -0,0 +1,18 @@
+import '../components/bookmarks-list.js';
+import BaseModal from "plugins/modal/modal.js";
+import { html } from "lit";
+import { __ } from 'i18n';
+import { api } from "@converse/headless/core";
+
+export default class BookmarkListModal extends BaseModal {
+
+    renderModal () { // eslint-disable-line class-methods-use-this
+        return html`<converse-bookmarks></converse-bookmarks>`;
+    }
+
+    getModalTitle () { // eslint-disable-line class-methods-use-this
+        return __('Bookmarks');
+    }
+}
+
+api.elements.define('converse-bookmark-list-modal', BookmarkListModal);
diff --git a/src/plugins/bookmark-views/styles/bookmarks.scss b/src/plugins/bookmark-views/styles/bookmarks.scss
index 5842969f9..bf701f3cd 100644
--- a/src/plugins/bookmark-views/styles/bookmarks.scss
+++ b/src/plugins/bookmark-views/styles/bookmarks.scss
@@ -24,3 +24,9 @@
         }
     }
 }
+
+converse-bookmarks {
+    .list-item-link {
+        padding: 0 1em;
+    }
+}
diff --git a/src/plugins/bookmark-views/templates/list.js b/src/plugins/bookmark-views/templates/list.js
deleted file mode 100644
index f8b37afd8..000000000
--- a/src/plugins/bookmark-views/templates/list.js
+++ /dev/null
@@ -1,32 +0,0 @@
-import bookmark_item from './item.js';
-import { __ } from 'i18n';
-import { _converse } from '@converse/headless/core.js';
-import { html } from "lit";
-import { until } from 'lit/directives/until.js';
-
-const list = (el, bookmarks) => {
-    const desc_bookmarks = __('Click to toggle the bookmarks list');
-    const label_bookmarks = __('Bookmarks');
-    const toggle_state = el.model.get('toggle-state');
-    return html`
-        <div class="list-container list-container--bookmarks ${ bookmarks.length ? 'fade-in' : 'hidden' }">
-            <a class="list-toggle bookmarks-toggle controlbox-padded"
-               title="${desc_bookmarks}"
-               @click=${() => el.toggleBookmarksList()}>
-
-               <converse-icon
-                   class="fa ${(toggle_state === _converse.OPENED) ? 'fa-caret-down' : 'fa-caret-right' }"
-                   size="1em"
-                   color="var(--muc-color)">
-                </converse-icon> ${label_bookmarks}</a>
-            <div class="items-list bookmarks rooms-list ${ (toggle_state === _converse.OPENED) ? 'fade-in' : 'hidden fade-out' }">
-            ${ _converse.bookmarks.map(bm => bookmark_item(bm)) }
-            </div>
-        </div>
-    `;
-}
-
-export default (el) => {
-    const bookmarks = _converse.bookmarks.getUnopenedBookmarks();
-    return until(bookmarks.then((bookmarks) => list(el, bookmarks)), '');
-}
diff --git a/src/plugins/bookmark-views/tests/bookmarks-list.js b/src/plugins/bookmark-views/tests/bookmarks-list.js
index 480f3a26d..afa3a5f31 100644
--- a/src/plugins/bookmark-views/tests/bookmarks-list.js
+++ b/src/plugins/bookmark-views/tests/bookmarks-list.js
@@ -2,7 +2,7 @@
 
 const { Strophe, u, sizzle, $iq } = converse.env;
 
-describe("The bookmarks list", function () {
+describe("The bookmarks list modal", function () {
 
     it("shows a list of bookmarks", mock.initConverse(
             ['chatBoxesFetched'], {},
@@ -16,6 +16,9 @@ describe("The bookmarks list", function () {
         );
         mock.openControlBox(_converse);
 
+        const controlbox = _converse.chatboxviews.get('controlbox');
+        controlbox.querySelector('.show-bookmark-list-modal').click();
+
         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());
@@ -59,9 +62,10 @@ describe("The bookmarks list", function () {
                             }).c('nick').t('JC').up().up();
         _converse.connection._dataRecv(mock.createRequest(stanza));
 
-        await u.waitUntil(() => document.querySelectorAll('#chatrooms div.bookmarks.rooms-list .room-item').length);
-        expect(document.querySelectorAll('#chatrooms div.bookmarks.rooms-list .room-item').length).toBe(5);
-        let els = document.querySelectorAll('#chatrooms div.bookmarks.rooms-list .room-item a.list-item-link');
+        const modal = _converse.api.modal.get('converse-bookmark-list-modal');
+        await u.waitUntil(() => modal.querySelectorAll('.bookmarks.rooms-list .room-item').length);
+        expect(modal.querySelectorAll('.bookmarks.rooms-list .room-item').length).toBe(5);
+        let els = modal.querySelectorAll('.bookmarks.rooms-list .room-item a.list-item-link');
         expect(els[0].textContent).toBe("1st Bookmark");
         expect(els[1].textContent).toBe("Another room");
         expect(els[2].textContent).toBe("Bookmark with a very very long name that will be shortened");
@@ -69,10 +73,10 @@ describe("The bookmarks list", function () {
         expect(els[4].textContent).toBe("The Play's the Thing");
 
         spyOn(_converse.api, 'confirm').and.callFake(() => Promise.resolve(true));
-        document.querySelector('#chatrooms .bookmarks.rooms-list .room-item:nth-child(2) a:nth-child(2)').click();
+        modal.querySelector('.bookmarks.rooms-list .room-item:nth-child(2) a:nth-child(2)').click();
         expect(_converse.api.confirm).toHaveBeenCalled();
-        await u.waitUntil(() => document.querySelectorAll('#chatrooms div.bookmarks.rooms-list .room-item').length === 4)
-        els = document.querySelectorAll('#chatrooms div.bookmarks.rooms-list .room-item a.list-item-link');
+        await u.waitUntil(() => modal.querySelectorAll('.bookmarks.rooms-list .room-item').length === 4)
+        els = modal.querySelectorAll('.bookmarks.rooms-list .room-item a.list-item-link');
         expect(els[0].textContent).toBe("1st Bookmark");
         expect(els[1].textContent).toBe("Bookmark with a very very long name that will be shortened");
         expect(els[2].textContent).toBe("noname@conference.shakespeare.lit");
@@ -80,17 +84,22 @@ describe("The bookmarks list", function () {
     }));
 
     it("can be used to open a MUC from a bookmark", mock.initConverse(
-            [], {'view_mode': 'fullscreen'}, async function (_converse) {
+            ['chatBoxesFetched'], {'view_mode': 'fullscreen'},
+            async function (_converse) {
 
         const api = _converse.api;
+
+        await mock.waitForRoster(_converse, 'current', 0);
         await mock.waitUntilDiscoConfirmed(
             _converse, _converse.bare_jid,
             [{'category': 'pubsub', 'type': 'pep'}],
             ['http://jabber.org/protocol/pubsub#publish-options']
         );
-        await mock.waitForRoster(_converse, 'current', 0);
-        await mock.openControlBox(_converse);
-        const view = await _converse.chatboxviews.get('controlbox');
+        mock.openControlBox(_converse);
+
+        const controlbox = await _converse.chatboxviews.get('controlbox');
+        controlbox.querySelector('.show-bookmark-list-modal').click();
+
         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());
@@ -110,79 +119,26 @@ describe("The bookmarks list", function () {
                                 'jid': 'first@conference.shakespeare.lit'
                             }).c('nick').t('JC');
         _converse.connection._dataRecv(mock.createRequest(stanza));
-        await u.waitUntil(() => view.querySelectorAll('#chatrooms div.bookmarks.rooms-list .room-item').length);
-        expect(view.querySelectorAll('#chatrooms div.bookmarks.rooms-list .room-item').length).toBe(2);
-        view.querySelector('.bookmarks.rooms-list .open-room').click();
+
+        const modal = api.modal.get('converse-bookmark-list-modal');
+        await u.waitUntil(() => u.isVisible(modal), 1000);
+
+        await u.waitUntil(() => modal.querySelectorAll('.bookmarks.rooms-list .room-item').length);
+        expect(modal.querySelectorAll('.bookmarks.rooms-list .room-item').length).toBe(2);
+        modal.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.querySelectorAll('.list-container--bookmarks .available-chatroom:not(.hidden)').length === 1);
-        view.querySelector('.list-container--bookmarks .available-chatroom:not(.hidden) .open-room').click();
+        await u.waitUntil(() => modal.querySelectorAll('.list-container--bookmarks .available-chatroom').length);
+        modal.querySelector('.list-container--bookmarks .available-chatroom:last-child .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.querySelector('.list-container--openrooms .open-room:first-child').click();
-        await u.waitUntil(() => view.querySelector('.list-item.open').getAttribute('data-room-jid') === 'first@conference.shakespeare.lit');
+        controlbox.querySelector('.list-container--openrooms .open-room:first-child').click();
+        await u.waitUntil(() => controlbox.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);
     }));
-
-    it("remembers the toggle state of the bookmarks list", mock.initConverse(
-            [], {}, async function (_converse) {
-
-        await mock.waitForRoster(_converse, 'current', 0);
-        await mock.openControlBox(_converse);
-        await mock.waitUntilDiscoConfirmed(
-            _converse, _converse.bare_jid,
-            [{'category': 'pubsub', 'type': 'pep'}],
-            ['http://jabber.org/protocol/pubsub#publish-options']
-        );
-
-        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('iq items[node="storage:bookmarks"]', s).length).pop());
-
-        expect(Strophe.serialize(sent_stanza)).toBe(
-            `<iq from="romeo@montague.lit/orchard" id="${sent_stanza.getAttribute('id')}" type="get" xmlns="jabber:client">`+
-            '<pubsub xmlns="http://jabber.org/protocol/pubsub">'+
-                '<items node="storage:bookmarks"/>'+
-            '</pubsub>'+
-            '</iq>'
-        );
-        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'});
-        _converse.connection._dataRecv(mock.createRequest(stanza));
-        await _converse.api.waitUntil('bookmarksInitialized');
-
-        _converse.bookmarks.create({
-            'jid': 'theplay@conference.shakespeare.lit',
-            'autojoin': false,
-            'name':  'The Play',
-            'nick': ''
-        });
-        const chats_el = document.querySelector('converse-chats');
-        const selector = '#chatrooms .bookmarks.rooms-list .room-item';
-        await u.waitUntil(() => sizzle(selector, chats_el).filter(u.isVisible).length);
-        expect(u.hasClass('collapsed', sizzle('#chatrooms .bookmarks.rooms-list', chats_el).pop())).toBeFalsy();
-        expect(sizzle(selector, chats_el).filter(u.isVisible).length).toBe(1);
-
-        const bookmarks_el = chats_el.querySelector('converse-bookmarks');
-        expect(bookmarks_el.model.get('toggle-state')).toBe(_converse.OPENED);
-
-        sizzle('#chatrooms .bookmarks-toggle', chats_el).pop().click();
-
-        await u.waitUntil(() => u.hasClass('hidden', sizzle('#chatrooms .bookmarks.rooms-list', chats_el).pop()));
-        expect(bookmarks_el.model.get('toggle-state')).toBe(_converse.CLOSED);
-
-        sizzle('#chatrooms .bookmarks-toggle', chats_el).pop().click();
-
-        await u.waitUntil(() => !u.hasClass('hidden', sizzle('#chatrooms .bookmarks.rooms-list', chats_el).pop()));
-        expect(sizzle(selector, chats_el).filter(u.isVisible).length).toBe(1);
-        expect(bookmarks_el.model.get('toggle-state')).toBe(_converse.OPENED);
-    }));
 });
diff --git a/src/plugins/bookmark-views/tests/bookmarks.js b/src/plugins/bookmark-views/tests/bookmarks.js
index 04eef062c..86ea34c76 100644
--- a/src/plugins/bookmark-views/tests/bookmarks.js
+++ b/src/plugins/bookmark-views/tests/bookmarks.js
@@ -492,40 +492,3 @@ describe("Bookmarks", function () {
         expect(_converse.bookmarks.get('another@conference.shakespeare.lit').get('autojoin')).toBe(false);
     }));
 });
-
-describe("When hide_open_bookmarks is true and a bookmarked room is opened", function () {
-
-    it("can be closed", mock.initConverse(
-            [], { hide_open_bookmarks: true }, async function (_converse) {
-
-        await mock.waitForRoster(_converse, 'current', 0);
-        await mock.openControlBox(_converse);
-        await mock.waitUntilBookmarksReturned(_converse);
-
-        // Check that it's there
-        const jid = 'room@conference.example.org';
-        _converse.bookmarks.create({
-            'jid': jid,
-            'autojoin': false,
-            'name':  'The Play',
-            'nick': ' Othello'
-        });
-        expect(_converse.bookmarks.length).toBe(1);
-
-        const u = converse.env.utils;
-        const bookmarks_el = document.querySelector('converse-bookmarks');
-        await u.waitUntil(() => bookmarks_el.querySelectorAll(".open-room").length, 500);
-        const room_els = bookmarks_el.querySelectorAll(".open-room");
-        expect(room_els.length).toBe(1);
-
-        const bookmark = bookmarks_el.querySelector(".open-room");
-        bookmark.click();
-        await u.waitUntil(() => _converse.chatboxviews.get(jid));
-
-        expect(u.hasClass('hidden', bookmarks_el.querySelector(".available-chatroom"))).toBeTruthy();
-        // Check that it reappears once the room is closed
-        const view = _converse.chatboxviews.get(jid);
-        view.close();
-        await u.waitUntil(() => !u.hasClass('hidden', bookmarks_el.querySelector(".available-chatroom")));
-    }));
-});
diff --git a/src/plugins/bookmark-views/utils.js b/src/plugins/bookmark-views/utils.js
index eebae4197..cf4e20301 100644
--- a/src/plugins/bookmark-views/utils.js
+++ b/src/plugins/bookmark-views/utils.js
@@ -1,4 +1,3 @@
-import './modal.js';
 import invokeMap from 'lodash-es/invokeMap';
 import { Model } from '@converse/skeletor/src/model.js';
 import { __ } from 'i18n';
diff --git a/src/plugins/controlbox/templates/controlbox.js b/src/plugins/controlbox/templates/controlbox.js
index 9f6a1007d..7ecfcc66c 100644
--- a/src/plugins/controlbox/templates/controlbox.js
+++ b/src/plugins/controlbox/templates/controlbox.js
@@ -39,10 +39,7 @@ export default (el) => {
                         ? html`
                             <converse-user-profile></converse-user-profile>
                             <converse-headlines-feeds-list class="controlbox-section"></converse-headlines-feeds-list>
-                            <div id="chatrooms" class="controlbox-section">
-                                <converse-rooms-list></converse-rooms-list>
-                                <converse-bookmarks></converse-bookmarks>
-                            </div>
+                            <div id="chatrooms" class="controlbox-section"><converse-rooms-list></converse-rooms-list></div>
                             ${ api.settings.get("authentication") === _converse.ANONYMOUS ? '' :
                                 html`<div id="converse-roster" class="controlbox-section"><converse-roster></converse-roster></div>`
                             }`
diff --git a/src/plugins/roomslist/templates/roomslist.js b/src/plugins/roomslist/templates/roomslist.js
index 5b41da2b6..c481fe18d 100644
--- a/src/plugins/roomslist/templates/roomslist.js
+++ b/src/plugins/roomslist/templates/roomslist.js
@@ -76,9 +76,26 @@ export default (o) => {
     const i18n_heading_chatrooms = __('Groupchats');
     const i18n_title_list_rooms = __('Query for groupchats');
     const i18n_title_new_room = __('Add a new groupchat');
+    const i18n_show_bookmarks = __('Show bookmarked groupchats');
     return html`
         <div class="d-flex controlbox-padded">
-            <span class="w-100 controlbox-heading controlbox-heading--groupchats">${i18n_heading_chatrooms}</span>
+            <span class="w-100 controlbox-heading controlbox-heading--groupchats">
+                <a class="list-toggle open-rooms-toggle" title="${i18n_desc_rooms}" @click=${o.toggleRoomsList}>
+                    <converse-icon
+                        class="fa ${ (o.toggle_state === _converse.OPENED) ? 'fa-caret-down' : 'fa-caret-right' }"
+                        size="1em"
+                        color="var(--muc-color)"></converse-icon>
+                    ${i18n_heading_chatrooms}
+                </a>
+            </span>
+
+            <a class="controlbox-heading__btn show-bookmark-list-modal"
+                @click=${(ev) => api.modal.show('converse-bookmark-list-modal', { 'model': o.model }, ev)}
+                title="${i18n_show_bookmarks}"
+                data-toggle="modal">
+                    <converse-icon class="fa fa-bookmark right" size="1em"></converse-icon>
+            </a>
+
             <a class="controlbox-heading__btn show-list-muc-modal"
                 @click=${(ev) => api.modal.show('converse-muc-list-modal', { 'model': o.model }, ev)}
                 title="${i18n_title_list_rooms}" data-toggle="modal" data-target="#muc-list-modal">
@@ -92,12 +109,6 @@ export default (o) => {
         </div>
 
         <div class="list-container list-container--openrooms ${ o.rooms.length ? '' : 'hidden' }">
-            <a class="list-toggle open-rooms-toggle controlbox-padded" title="${i18n_desc_rooms}" @click=${o.toggleRoomsList}>
-            <converse-icon
-                class="fa ${ (o.toggle_state === _converse.OPENED) ? 'fa-caret-down' : 'fa-caret-right' }"
-                size="1em"
-                color="var(--muc-color)">
-            </converse-icon> ${__('Open Groupchats')}</a>
             <div class="items-list rooms-list open-rooms-list ${ o.collapsed && 'collapsed' }">
                 ${ o.rooms.map(room => room_item(Object.assign({room}, o))) }
             </div>
diff --git a/src/plugins/roomslist/view.js b/src/plugins/roomslist/view.js
index 9e040bdee..934cf35ee 100644
--- a/src/plugins/roomslist/view.js
+++ b/src/plugins/roomslist/view.js
@@ -92,7 +92,8 @@ export class RoomsList extends CustomElement {
 
     toggleRoomsList (ev) {
         ev?.preventDefault?.();
-        const icon_el = ev.target.matches('.fa') ? ev.target : ev.target.querySelector('.fa');
+        const target = ev.currentTarget;
+        const icon_el = target.matches('.fa') ? target : target.querySelector('.fa');
         if (icon_el.classList.contains("fa-caret-down")) {
             u.slideIn(this.querySelector('.open-rooms-list')).then(() => {
                 this.model.save({'toggle-state': _converse.CLOSED});
diff --git a/src/shared/styles/forms.scss b/src/shared/styles/forms.scss
index 1a1857649..bdafd05cc 100644
--- a/src/shared/styles/forms.scss
+++ b/src/shared/styles/forms.scss
@@ -50,13 +50,15 @@
             margin-top: 0.5em;
         }
 
-        .clear-input {
-            margin-top: 0.5em;
-            margin-bottom : 0.5em;
-            position: absolute;
-            right: 0.2em;
-            cursor: pointer;
-            font-size: var(--font-size);
+        .btn-group {
+            .clear-input {
+                margin-top: 0.5em;
+                margin-bottom : 0.5em;
+                position: absolute;
+                right: 0.2em;
+                cursor: pointer;
+                font-size: var(--font-size);
+            }
         }
 
         &#converse-register,