Updates #729. Minimized chat boxes don't stay hidden

Bug got introduced during fix for #677

Eventually had to do a significant refactoring, to more consistently use the
`hidden` class instead of `display: None`. (relevant for #695)

css/converse.css

@@ -1527,7 +1527,6 @@
       left: 0; }
 
 #conversejs #controlbox {
-  display: none;
   margin-right: 1em; }
   @media screen and (max-width: 480px) {
     #conversejs #controlbox {
@@ -2119,7 +2118,6 @@
   border-top-left-radius: 4px;
   border-top-right-radius: 4px;
   color: white;
-  display: none;
   float: right;
   font-weight: bold;
   height: 100%;

sass/_controlbox.scss

@@ -1,6 +1,5 @@
 #conversejs {
     #controlbox {
-        display: none;
         margin-right: 2*$chat-gutter;
         @media screen and (max-width: $mobile-portrait-length) {
             margin: 0;

sass/_minimized_chats.scss

@@ -3,7 +3,6 @@
         border-top-left-radius: $chatbox-border-radius;
         border-top-right-radius: $chatbox-border-radius;
         color: $inverse-link-color;
-        display: none;
         float: right;
         font-weight: bold;
         height: 100%;

spec/chatbox.js

@@ -847,50 +847,51 @@
                     test_utils.createContacts(converse, 'current');
                     test_utils.openControlBox();
                     test_utils.openContactsPanel(converse);
-
                     var contact_name = mock.cur_names[0];
                     var contact_jid = contact_name.replace(/ /g,'.').toLowerCase() + '@localhost';
-                    spyOn(converse, 'emit').andCallThrough();
-                    test_utils.openChatBoxFor(converse, contact_jid);
-                    var chatview = converse.chatboxviews.get(contact_jid);
-                    expect(chatview.$el.is(':visible')).toBeTruthy();
-                    expect(chatview.model.get('minimized')).toBeFalsy();
-                    chatview.$el.find('.toggle-chatbox-button').click();
-                    expect(chatview.model.get('minimized')).toBeTruthy();
-                    var message = 'This message is sent to a minimized chatbox';
-                    var sender_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost';
-                    var msg = $msg({
-                        from: sender_jid,
-                        to: converse.connection.jid,
-                        type: 'chat',
-                        id: (new Date()).getTime()
-                    }).c('body').t(message).up()
-                    .c('active', {'xmlns': 'http://jabber.org/protocol/chatstates'}).tree();
-                    converse.chatboxes.onMessage(msg);
-                    expect(converse.emit).toHaveBeenCalledWith('message', msg);
-                    var trimmed_chatboxes = converse.minimized_chats;
-                    var trimmedview = trimmed_chatboxes.get(contact_jid);
-                    var $count = trimmedview.$el.find('.chat-head-message-count');
-                    expect(chatview.$el.is(':visible')).toBeFalsy();
-                    expect(trimmedview.model.get('minimized')).toBeTruthy();
-                    expect($count.is(':visible')).toBeTruthy();
-                    expect($count.html()).toBe('1');
-                    converse.chatboxes.onMessage(
-                        $msg({
-                            from: mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost',
+                    runs(function () {
+                        spyOn(converse, 'emit').andCallThrough();
+                        test_utils.openChatBoxFor(converse, contact_jid);
+                        var chatview = converse.chatboxviews.get(contact_jid);
+                        expect(chatview.$el.is(':visible')).toBeTruthy();
+                        expect(chatview.model.get('minimized')).toBeFalsy();
+                        chatview.$el.find('.toggle-chatbox-button').click();
+                        expect(chatview.model.get('minimized')).toBeTruthy();
+                        var message = 'This message is sent to a minimized chatbox';
+                        var sender_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost';
+                        var msg = $msg({
+                            from: sender_jid,
                             to: converse.connection.jid,
                             type: 'chat',
                             id: (new Date()).getTime()
-                        }).c('body').t('This message is also sent to a minimized chatbox').up()
-                        .c('active', {'xmlns': 'http://jabber.org/protocol/chatstates'}).tree()
-                    );
-                    expect(chatview.$el.is(':visible')).toBeFalsy();
-                    expect(trimmedview.model.get('minimized')).toBeTruthy();
-                    $count = trimmedview.$el.find('.chat-head-message-count');
-                    expect($count.is(':visible')).toBeTruthy();
-                    expect($count.html()).toBe('2');
-                    trimmedview.$el.find('.restore-chat').click();
-                    expect(trimmed_chatboxes.keys().length).toBe(0);
+                        }).c('body').t(message).up()
+                        .c('active', {'xmlns': 'http://jabber.org/protocol/chatstates'}).tree();
+                        converse.chatboxes.onMessage(msg);
+                        expect(converse.emit).toHaveBeenCalledWith('message', msg);
+                        var trimmed_chatboxes = converse.minimized_chats;
+                        var trimmedview = trimmed_chatboxes.get(contact_jid);
+                        var $count = trimmedview.$el.find('.chat-head-message-count');
+                        expect(chatview.$el.is(':visible')).toBeFalsy();
+                        expect(trimmedview.model.get('minimized')).toBeTruthy();
+                        expect($count.is(':visible')).toBeTruthy();
+                        expect($count.html()).toBe('1');
+                        converse.chatboxes.onMessage(
+                            $msg({
+                                from: mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost',
+                                to: converse.connection.jid,
+                                type: 'chat',
+                                id: (new Date()).getTime()
+                            }).c('body').t('This message is also sent to a minimized chatbox').up()
+                            .c('active', {'xmlns': 'http://jabber.org/protocol/chatstates'}).tree()
+                        );
+                        expect(chatview.$el.is(':visible')).toBeFalsy();
+                        expect(trimmedview.model.get('minimized')).toBeTruthy();
+                        $count = trimmedview.$el.find('.chat-head-message-count');
+                        expect($count.is(':visible')).toBeTruthy();
+                        expect($count.html()).toBe('2');
+                        trimmedview.$el.find('.restore-chat').click();
+                        expect(trimmed_chatboxes.keys().length).toBe(0);
+                    });
                 }));
 
                 it("will indicate when it has a time difference of more than a day between it and its predecessor", mock.initConverse(function (converse) {
@@ -1198,22 +1199,27 @@
                         test_utils.createContacts(converse, 'current');
                         test_utils.openControlBox();
                         test_utils.openContactsPanel(converse);
-
                         var contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost';
-                        test_utils.openChatBoxFor(converse, contact_jid);
-                        var view = converse.chatboxviews.get(contact_jid);
-                        view.minimize();
-                        expect(view.model.get('chat_state')).toBe('inactive');
-                        spyOn(converse.connection, 'send');
-                        view.maximize();
-                        expect(view.model.get('chat_state')).toBe('active');
-                        expect(converse.connection.send).toHaveBeenCalled();
-                        var $stanza = $(converse.connection.send.argsForCall[0][0].tree());
-                        expect($stanza.attr('to')).toBe(contact_jid);
-                        expect($stanza.children().length).toBe(3);
-                        expect($stanza.children().get(0).tagName).toBe('active');
-                        expect($stanza.children().get(1).tagName).toBe('no-store');
-                        expect($stanza.children().get(2).tagName).toBe('no-permanent-store');
+
+                        runs(function () {
+                            test_utils.openChatBoxFor(converse, contact_jid);
+                        });
+                        waits(300); // ChatBox.show() is debounced for 250ms
+                        runs(function () {
+                            var view = converse.chatboxviews.get(contact_jid);
+                            view.model.minimize();
+                            expect(view.model.get('chat_state')).toBe('inactive');
+                            spyOn(converse.connection, 'send');
+                            view.model.maximize();
+                            expect(view.model.get('chat_state')).toBe('active');
+                            expect(converse.connection.send).toHaveBeenCalled();
+                            var $stanza = $(converse.connection.send.argsForCall[0][0].tree());
+                            expect($stanza.attr('to')).toBe(contact_jid);
+                            expect($stanza.children().length).toBe(3);
+                            expect($stanza.children().get(0).tagName).toBe('active');
+                            expect($stanza.children().get(1).tagName).toBe('no-store');
+                            expect($stanza.children().get(2).tagName).toBe('no-permanent-store');
+                        });
                     }));
                 });
 

spec/chatroom.js

@@ -656,18 +656,17 @@
                 runs(function () {
                     view.$el.find('.toggle-chatbox-button').click();
                 });
-                waits(50);
+                waits(350);
                 runs(function () {
                     expect(view.minimize).toHaveBeenCalled();
                     expect(converse.emit).toHaveBeenCalledWith('chatBoxMinimized', jasmine.any(Object));
-                    expect(converse.emit.callCount, 2);
                     expect(view.$el.is(':visible')).toBeFalsy();
                     expect(view.model.get('minimized')).toBeTruthy();
                     expect(view.minimize).toHaveBeenCalled();
                     var trimmedview = trimmed_chatboxes.get(view.model.get('id'));
                     trimmedview.$("a.restore-chat").click();
                 });
-                waits(250);
+                waits(350);
                 runs(function () {
                     expect(view.maximize).toHaveBeenCalled();
                     expect(converse.emit).toHaveBeenCalledWith('chatBoxMaximized', jasmine.any(Object));

spec/controlbox.js

@@ -904,34 +904,41 @@
             });
 
             it("can be added to the roster and they will be sorted alphabetically", mock.initConverse(function (converse) {
-                test_utils.createContacts(converse, 'requesting').openControlBox();
-                test_utils.openContactsPanel(converse);
-                converse.rosterview.model.reset(); // We want to manually create users so that we can spy
                 var i, children;
                 var names = [];
-                spyOn(converse, 'emit');
-                spyOn(converse.rosterview, 'update').andCallThrough();
-                spyOn(converse.controlboxtoggle, 'showControlBox').andCallThrough();
                 var addName = function (idx, item) {
                     if (!$(item).hasClass('request-actions')) {
                         names.push($(item).text().replace(/^\s+|\s+$/g, ''));
                     }
                 };
-                for (i=0; i<mock.req_names.length; i++) {
-                    converse.roster.create({
-                        jid: mock.req_names[i].replace(/ /g,'.').toLowerCase() + '@localhost',
-                        subscription: 'none',
-                        ask: null,
-                        requesting: true,
-                        fullname: mock.req_names[i]
-                    });
+                runs(function () {
+                    test_utils.openContactsPanel(converse);
+                });
+                waits(250);
+                runs(function () {
+                    spyOn(converse, 'emit');
+                    spyOn(converse.rosterview, 'update').andCallThrough();
+                    spyOn(converse.controlboxtoggle, 'showControlBox').andCallThrough();
+                    for (i=0; i<mock.req_names.length; i++) {
+                        converse.roster.create({
+                            jid: mock.req_names[i].replace(/ /g,'.').toLowerCase() + '@localhost',
+                            subscription: 'none',
+                            ask: null,
+                            requesting: true,
+                            fullname: mock.req_names[i]
+                        });
+                    }
+                });
+                waits(250);
+                runs(function () {
                     expect(converse.rosterview.update).toHaveBeenCalled();
-                }
-                // Check that they are sorted alphabetically
-                children = converse.rosterview.get('Contact requests').$el.siblings('dd.requesting-xmpp-contact').children('span');
-                names = [];
-                children.each(addName);
-                expect(names.join('')).toEqual(mock.req_names.slice(0,i+1).sort().join(''));
+                    // Check that they are sorted alphabetically
+                    children = converse.rosterview.get('Contact requests').$el.siblings('dd.requesting-xmpp-contact').children('span');
+                    names = [];
+                    children.each(addName);
+                    expect(names.join('')).toEqual(mock.req_names.slice(0,mock.req_names.length+1).sort().join(''));
+                });
+
             }));
 
             it("do not have a header if there aren't any", mock.initConverse(function (converse) {
@@ -948,7 +955,7 @@
                         fullname: name
                     });
                 });
-                waits(50);
+                waits(350);
                 runs(function () {
                     expect(converse.rosterview.get('Contact requests').$el.is(':visible')).toEqual(true);
                     converse.rosterview.$el.find(".req-contact-name:contains('"+name+"')")

src/converse-api.js

@@ -112,11 +112,15 @@
                     converse.log("chats.open: You need to provide at least one JID", "error");
                     return null;
                 } else if (typeof jids === "string") {
-                    chatbox = converse.wrappedChatBox(converse.chatboxes.getChatBox(jids, true));
+                    chatbox = converse.wrappedChatBox(
+                        converse.chatboxes.getChatBox(jids, true).trigger('show')
+                    );
                     return chatbox;
                 }
                 return _.map(jids, function (jid) {
-                    chatbox = converse.wrappedChatBox(converse.chatboxes.getChatBox(jid, true));
+                    chatbox = converse.wrappedChatBox(
+                        converse.chatboxes.getChatBox(jid, true).trigger('show')
+                    );
                     return chatbox;
                 });
             },

src/converse-chatview.js

@@ -91,7 +91,7 @@
             converse.ChatBoxView = Backbone.View.extend({
                 length: 200,
                 tagName: 'div',
-                className: 'chatbox',
+                className: 'chatbox hidden',
                 is_chatroom: false,  // Leaky abstraction from MUC
 
                 events: {
@@ -115,7 +115,7 @@
                     this.model.on('change:status', this.onStatusChanged, this);
                     this.model.on('showHelpMessages', this.showHelpMessages, this);
                     this.model.on('sendMessage', this.sendMessage, this);
-                    this.render().fetchMessages().insertIntoDOM().afterShown();
+                    this.render().fetchMessages().insertIntoDOM();
                     // XXX: adding the event below to the events map above doesn't work.
                     // The code that gets executed because of that looks like this:
                     //      this.$el.on('scroll', '.chat-content', this.markScrolled.bind(this));
@@ -730,7 +730,7 @@
                 },
 
                 hide: function () {
-                    this.$el.hide();
+                    this.el.classList.add('hidden');
                     utils.refreshWebkit();
                     return this;
                 },
@@ -754,7 +754,7 @@
                         if (focus) { this.focus(); }
                         return;
                     }
-                    this.$el.fadeIn(this.afterShown.bind(this));
+                    utils.fadeIn(this.el, this.afterShown.bind(this));
                 },
 
                 show: function (focus) {

src/converse-controlbox.js

@@ -359,24 +359,23 @@
                 },
 
                 hide: function (callback) {
-                    this.$el.hide('fast', function () {
-                        utils.refreshWebkit();
-                        converse.emit('chatBoxClosed', this);
-                        if (!converse.connection.connected) {
-                            converse.controlboxtoggle.render();
+                    this.$el.addClass('hidden');
+                    utils.refreshWebkit();
+                    converse.emit('chatBoxClosed', this);
+                    if (!converse.connection.connected) {
+                        converse.controlboxtoggle.render();
+                    }
+                    converse.controlboxtoggle.show(function () {
+                        if (typeof callback === "function") {
+                            callback();
                         }
-                        converse.controlboxtoggle.show(function () {
-                            if (typeof callback === "function") {
-                                callback();
-                            }
-                        });
                     });
                     return this;
                 },
 
                 onControlBoxToggleHidden: function () {
                     var that = this;
-                    this.$el.show('fast', function () {
+                    utils.fadeIn(this.el, function () {
                         converse.controlboxtoggle.updateOnlineCount();
                         utils.refreshWebkit();
                         converse.emit('controlBoxOpened', that);
@@ -732,7 +731,7 @@
 
             converse.ControlBoxToggle = Backbone.View.extend({
                 tagName: 'a',
-                className: 'toggle-controlbox',
+                className: 'toggle-controlbox hidden',
                 id: 'toggle-controlbox',
                 events: {
                     'click': 'onClick'
@@ -742,7 +741,7 @@
                 },
 
                 initialize: function () {
-                    $('#conversejs').prepend(this.render());
+                    converse.chatboxviews.$el.prepend(this.render());
                     this.updateOnlineCount();
                     converse.on('initialized', function () {
                         converse.roster.on("add", this.updateOnlineCount, this);
@@ -761,7 +760,7 @@
                         converse.templates.controlbox_toggle({
                             'label_toggle': __('Toggle chat')
                         })
-                    ).hide();
+                    );
                 },
 
                 updateOnlineCount: _.debounce(function () {
@@ -776,11 +775,12 @@
                 }, converse.animate ? 100 : 0),
 
                 hide: function (callback) {
-                    this.$el.fadeOut('fast', callback);
+                    this.el.classList.add('hidden');
+                    callback();
                 },
 
                 show: function (callback) {
-                    this.$el.show('fast', callback);
+                    utils.fadeIn(this.el, callback);
                 },
 
                 showControlBox: function () {

src/converse-minimize.js

@@ -145,24 +145,11 @@
                     }
                 },
 
-                onMaximized: function () {
-                    converse.chatboxviews.trimChats(this);
-                    utils.refreshWebkit();
-                    this.$content.scrollTop(this.model.get('scroll'));
-                    this.setChatState(converse.ACTIVE).focus();
-                    this.scrollDown();
-                    converse.emit('chatBoxMaximized', this);
-                },
-
-                onMinimized: function () {
-                    utils.refreshWebkit();
-                    converse.emit('chatBoxMinimized', this);
-                },
-
                 maximize: function () {
                     // Restores a minimized chat box
-                    this.$el.insertAfter(converse.chatboxviews.get("controlbox").$el)
-                        .show('fast', this.onMaximized.bind(this));
+                    this.$el.insertAfter(converse.chatboxviews.get("controlbox").$el);
+                    this.show();
+                    converse.emit('chatBoxMaximized', this);
                     return this;
                 },
 
@@ -171,7 +158,8 @@
                     // save the scroll position to restore it on maximize
                     this.model.save({'scroll': this.$content.scrollTop()});
                     this.setChatState(converse.INACTIVE).model.minimize();
-                    this.$el.hide('fast', this.onMinimized.bind(this));
+                    this.hide();
+                    converse.emit('chatBoxMinimized', this);
                 },
             },
 
@@ -270,7 +258,7 @@
                             // conditions.
                             view = this.get(oldest_chat.get('id'));
                             if (view) {
-                                view.$el.hide();
+                                view.hide();
                             }
                             oldest_chat.minimize();
                         }

src/converse-muc.js

@@ -300,7 +300,7 @@
                  */
                 length: 300,
                 tagName: 'div',
-                className: 'chatbox chatroom',
+                className: 'chatbox chatroom hidden',
                 is_chatroom: true,
                 events: {
                     'click .close-chatbox-button': 'close',
@@ -327,8 +327,7 @@
                     var id = b64_sha1('converse.occupants'+converse.bare_jid+this.model.get('id')+this.model.get('nick'));
                     this.occupantsview.model.browserStorage = new Backbone.BrowserStorage.session(id);
                     this.occupantsview.chatroomview = this;
-
-                    this.render().$el.hide();
+                    this.render();
                     this.occupantsview.model.fetch({add:true});
                     var nick = this.model.get('nick');
                     if (!nick) {

src/utils.js

@@ -208,6 +208,32 @@
                 locale = utils.isLocaleAvailable(window.navigator.systemLanguage, library_check);
             }
             return locale || 'en';
+
+        },
+
+        fadeIn: function (el, callback) {
+            if ($.fx.off) {
+                el.classList.remove('hidden');
+                callback();
+                return;
+            }
+            el.style.opacity = 0;
+            el.classList.remove('hidden');
+            var last = +new Date();
+            var tick = function() {
+                el.style.opacity = +el.style.opacity + (new Date() - last) / 100;
+                last = +new Date();
+                if (+el.style.opacity < 1) {
+                    if (!_.isUndefined(window.requestAnimationFrame)) {
+                        window.requestAnimationFrame(tick);
+                    } else {
+                        window.setTimeout(tick, 16);
+                    }
+                } else {
+                    callback();
+                }
+            };
+            tick();
         },
 
         isOTRMessage: function (message) {

tests/utils.js

@@ -50,12 +50,13 @@
     };
 
     utils.openControlBox = function () {
-        var toggle = $(".toggle-controlbox");
+        var $toggle = $(".toggle-controlbox");
         if (!$("#controlbox").is(':visible')) {
-            if (!toggle.is(':visible')) {
-                toggle.show(toggle.click);
+            if (!$toggle.is(':visible')) {
+                $toggle[0].classList.remove('hidden');
+                $toggle.click();
             } else {
-                toggle.click();
+                $toggle.click();
             }
         }
         return this;
@@ -74,6 +75,7 @@
     };
 
     utils.openContactsPanel = function (converse) {
+        this.openControlBox(converse);
         var cbview = converse.chatboxviews.get('controlbox');
         var $tabs = cbview.$el.find('#controlbox-tabs');
         $tabs.find('li').first().find('a').click();