From da0c858fe0d1020f98cebd58441c65f3e1b1f710 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Mon, 15 Feb 2016 14:55:34 +0000 Subject: [PATCH] Used debounce ineffectually. Fixed now. The ChatBox.show() method was being called for every invocation, 100ms after the fact, instead of it being called only once. Fixed that and also changed to call it at the start. --- spec/chatbox.js | 159 +++++++++++++++++++---------------- spec/converse.js | 195 +++++++++++++++++++++++-------------------- src/converse-core.js | 6 +- 3 files changed, 193 insertions(+), 167 deletions(-) diff --git a/spec/chatbox.js b/spec/chatbox.js index 15ea7032f..44b8dd529 100644 --- a/spec/chatbox.js +++ b/spec/chatbox.js @@ -104,24 +104,23 @@ var $el, jid, chatboxview, chatbox; // openControlBox was called earlier, so the controlbox is // visible, but no other chat boxes have been created. - expect(this.chatboxes.length).toEqual(1); + expect(converse.chatboxes.length).toEqual(1); chatbox = test_utils.openChatBoxFor(contact_jid); - chatboxview = this.chatboxviews.get(contact_jid); + chatboxview = converse.chatboxviews.get(contact_jid); spyOn(chatboxview, 'focus'); - // Test that they can be trimmed runs(function () { converse.rosterview.update(); // XXX: Hack to make sure $roster element is attaced. - }.bind(this)); - waits(50); + }); + waits(300); // ChatBox.show() is debounced for 250ms runs(function () { - $el = this.rosterview.$el.find('a.open-chat:contains("'+chatbox.get('fullname')+'")'); + $el = converse.rosterview.$el.find('a.open-chat:contains("'+chatbox.get('fullname')+'")'); jid = $el.text().replace(/ /g,'.').toLowerCase() + '@localhost'; $el.click(); - expect(this.chatboxes.length).toEqual(2); + expect(converse.chatboxes.length).toEqual(2); expect(chatboxview.focus).toHaveBeenCalled(); - }.bind(this)); - }.bind(converse)); + }); + }); it("can be saved to, and retrieved from, browserStorage", function () { spyOn(converse, 'emit'); @@ -270,18 +269,19 @@ }.bind(converse)); it("contains a button for inserting emoticons", function () { - var contact_jid = mock.cur_names[2].replace(/ /g,'.').toLowerCase() + '@localhost'; - test_utils.openChatBoxFor(contact_jid); - var view = this.chatboxviews.get(contact_jid); - var $toolbar = view.$el.find('ul.chat-toolbar'); - var $textarea = view.$el.find('textarea.chat-textarea'); - expect($toolbar.children('li.toggle-smiley').length).toBe(1); - // Register spies - spyOn(view, 'toggleEmoticonMenu').andCallThrough(); - spyOn(view, 'insertEmoticon').andCallThrough(); - view.delegateEvents(); // We need to rebind all events otherwise our spy won't be called - + var contact_jid = mock.cur_names[2].replace(/ /g,'.').toLowerCase() + '@localhost', + view, $toolbar, $textarea; + waits(300); // ChatBox.show() is debounced for 250ms runs(function () { + test_utils.openChatBoxFor(contact_jid); + view = converse.chatboxviews.get(contact_jid); + $toolbar = view.$el.find('ul.chat-toolbar'); + $textarea = view.$el.find('textarea.chat-textarea'); + expect($toolbar.children('li.toggle-smiley').length).toBe(1); + // Register spies + spyOn(view, 'toggleEmoticonMenu').andCallThrough(); + spyOn(view, 'insertEmoticon').andCallThrough(); + view.delegateEvents(); // We need to rebind all events otherwise our spy won't be called $toolbar.children('li.toggle-smiley').click(); }); waits(250); @@ -325,7 +325,7 @@ expect(view.$el.find('.toggle-smiley ul').is(':visible')).toBeFalsy(); expect($textarea.val()).toBe(':) <3 '); }); - }.bind(converse)); + }); it("contains a button for starting an encrypted chat session", function () { // TODO: More tests can be added here... @@ -812,17 +812,20 @@ describe("An active notification", function () { it("is sent when the user opens a chat box", function () { - spyOn(converse.connection, 'send'); - var contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; - test_utils.openChatBoxFor(contact_jid); - var view = this.chatboxviews.get(contact_jid); - 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(1); - expect($stanza.children().prop('tagName')).toBe('active'); - }.bind(converse)); + waits(300); // ChatBox.show() is debounced for 250ms + runs(function () { + spyOn(converse.connection, 'send'); + var contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; + test_utils.openChatBoxFor(contact_jid); + var view = converse.chatboxviews.get(contact_jid); + 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(1); + expect($stanza.children().prop('tagName')).toBe('active'); + }); + }); it("is sent when the user maximizes a minimized a chat box", function () { var contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; @@ -843,30 +846,33 @@ describe("A composing notification", function () { it("is sent as soon as the user starts typing a message which is not a command", function () { - var contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; - test_utils.openChatBoxFor(contact_jid); - var view = this.chatboxviews.get(contact_jid); - expect(view.model.get('chat_state')).toBe('active'); - spyOn(this.connection, 'send'); - view.keyPressed({ - target: view.$el.find('textarea.chat-textarea'), - keyCode: 1 - }); - expect(view.model.get('chat_state')).toBe('composing'); - expect(this.connection.send).toHaveBeenCalled(); - var $stanza = $(this.connection.send.argsForCall[0][0].tree()); - expect($stanza.attr('to')).toBe(contact_jid); - expect($stanza.children().length).toBe(1); - expect($stanza.children().prop('tagName')).toBe('composing'); + waits(300); // ChatBox.show() is debounced for 250ms + runs(function () { + var contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; + test_utils.openChatBoxFor(contact_jid); + var view = converse.chatboxviews.get(contact_jid); + expect(view.model.get('chat_state')).toBe('active'); + spyOn(converse.connection, 'send'); + view.keyPressed({ + target: view.$el.find('textarea.chat-textarea'), + keyCode: 1 + }); + expect(view.model.get('chat_state')).toBe('composing'); + 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(1); + expect($stanza.children().prop('tagName')).toBe('composing'); - // The notification is not sent again - view.keyPressed({ - target: view.$el.find('textarea.chat-textarea'), - keyCode: 1 + // The notification is not sent again + view.keyPressed({ + target: view.$el.find('textarea.chat-textarea'), + keyCode: 1 + }); + expect(view.model.get('chat_state')).toBe('composing'); + expect(converse.emit.callCount, 1); }); - expect(view.model.get('chat_state')).toBe('composing'); - expect(converse.emit.callCount, 1); - }.bind(converse)); + }); it("will be shown if received", function () { // See XEP-0085 http://xmpp.org/extensions/xep-0085.html#definitions @@ -893,13 +899,15 @@ describe("A paused notification", function () { it("is sent if the user has stopped typing since 30 seconds", function () { - this.TIMEOUTS.PAUSED = 200; // Make the timeout shorter so that we can test - var contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; - test_utils.openChatBoxFor(contact_jid); - var view = this.chatboxviews.get(contact_jid); - spyOn(converse.connection, 'send'); - spyOn(view, 'setChatState').andCallThrough(); + var view, contact_jid; + converse.TIMEOUTS.PAUSED = 200; // Make the timeout shorter so that we can test + waits(300); // ChatBox.show() is debounced for 250ms runs(function () { + contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; + test_utils.openChatBoxFor(contact_jid); + view = converse.chatboxviews.get(contact_jid); + spyOn(converse.connection, 'send'); + spyOn(view, 'setChatState').andCallThrough(); expect(view.model.get('chat_state')).toBe('active'); view.keyPressed({ target: view.$el.find('textarea.chat-textarea'), @@ -940,7 +948,7 @@ runs(function () { expect(view.model.get('chat_state')).toBe('composing'); }); - }.bind(converse)); + }); it("will be shown if received", function () { // TODO: only show paused state if the previous state was composing @@ -1010,19 +1018,22 @@ }.bind(converse)); it("is sent if the user closes a chat box", function () { - var contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; - test_utils.openChatBoxFor(contact_jid); - var view = this.chatboxviews.get(contact_jid); - expect(view.model.get('chat_state')).toBe('active'); - spyOn(converse.connection, 'send'); - view.close(); - expect(view.model.get('chat_state')).toBe('inactive'); - 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(1); - expect($stanza.children().prop('tagName')).toBe('inactive'); - }.bind(converse)); + waits(300); // ChatBox.show() is debounced for 250ms + runs(function () { + var contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; + test_utils.openChatBoxFor(contact_jid); + var view = converse.chatboxviews.get(contact_jid); + expect(view.model.get('chat_state')).toBe('active'); + spyOn(converse.connection, 'send'); + view.close(); + expect(view.model.get('chat_state')).toBe('inactive'); + 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(1); + expect($stanza.children().prop('tagName')).toBe('inactive'); + }); + }); it("will clear any other chat status notifications if its received", function () { // See XEP-0085 http://xmpp.org/extensions/xep-0085.html#definitions diff --git a/spec/converse.js b/spec/converse.js index 9bca30006..cf5fb2097 100644 --- a/spec/converse.js +++ b/spec/converse.js @@ -98,7 +98,7 @@ it("happens when the client is idle for long enough", function () { var i = 0; // Usually initialized by registerIntervalHandler - converse.idle_seconds = 0; + converse.idle_seconds = 0; converse.auto_changed_status = false; // The relevant config options @@ -265,15 +265,15 @@ test_utils.createContacts('current'); }, converse)); - it("has a method 'get' which returns a wrapped chat box", $.proxy(function () { + it("has a method 'get' which returns a wrapped chat box", function () { // Test on chat that doesn't exist. expect(converse_api.chats.get('non-existing@jabber.org')).toBeFalsy(); + var jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; // Test on chat that's not open - var jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; var box = converse_api.chats.get(jid); expect(box instanceof Object).toBeTruthy(); - var chatboxview = this.chatboxviews.get(jid); + var chatboxview = converse.chatboxviews.get(jid); expect(chatboxview.$el.is(':visible')).toBeFalsy(); // Test for single JID @@ -281,9 +281,8 @@ box = converse_api.chats.get(jid); expect(box instanceof Object).toBeTruthy(); expect(box.get('box_id')).toBe(b64_sha1(jid)); - chatboxview = this.chatboxviews.get(jid); + chatboxview = converse.chatboxviews.get(jid); expect(chatboxview.$el.is(':visible')).toBeTruthy(); - // Test for multiple JIDs var jid2 = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@localhost'; test_utils.openChatBoxFor(jid2); @@ -291,30 +290,32 @@ expect(Array.isArray(list)).toBeTruthy(); expect(list[0].get('box_id')).toBe(b64_sha1(jid)); expect(list[1].get('box_id')).toBe(b64_sha1(jid2)); - }, converse)); - - it("has a method 'open' which opens and returns a wrapped chat box", $.proxy(function () { - // Test on chat that doesn't exist. - expect(converse_api.chats.get('non-existing@jabber.org')).toBeFalsy(); + }); + it("has a method 'open' which opens and returns a wrapped chat box", function () { var jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; - var box = converse_api.chats.open(jid); - expect(box instanceof Object).toBeTruthy(); - expect(box.get('box_id')).toBe(b64_sha1(jid)); - expect( - Object.keys(box), - ['close', 'endOTR', 'focus', 'get', 'initiateOTR', 'is_chatroom', 'maximize', 'minimize', 'open', 'set'] - ); - var chatboxview = this.chatboxviews.get(jid); - expect(chatboxview.$el.is(':visible')).toBeTruthy(); - - // Test for multiple JIDs - var jid2 = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@localhost'; - var list = converse_api.chats.open([jid, jid2]); - expect(Array.isArray(list)).toBeTruthy(); - expect(list[0].get('box_id')).toBe(b64_sha1(jid)); - expect(list[1].get('box_id')).toBe(b64_sha1(jid2)); - }, converse)); + var chatboxview; + waits('300'); // ChatBox.show() is debounced for 250ms + runs(function () { + // Test on chat that doesn't exist. + expect(converse_api.chats.get('non-existing@jabber.org')).toBeFalsy(); + var box = converse_api.chats.open(jid); + expect(box instanceof Object).toBeTruthy(); + expect(box.get('box_id')).toBe(b64_sha1(jid)); + expect( + Object.keys(box), + ['close', 'endOTR', 'focus', 'get', 'initiateOTR', 'is_chatroom', 'maximize', 'minimize', 'open', 'set'] + ); + chatboxview = converse.chatboxviews.get(jid); + expect(chatboxview.$el.is(':visible')).toBeTruthy(); + // Test for multiple JIDs + var jid2 = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@localhost'; + var list = converse_api.chats.open([jid, jid2]); + expect(Array.isArray(list)).toBeTruthy(); + expect(list[0].get('box_id')).toBe(b64_sha1(jid)); + expect(list[1].get('box_id')).toBe(b64_sha1(jid2)); + }); + }); }, converse)); describe("The \"rooms\" API", function () { @@ -326,77 +327,91 @@ }); it("has a method 'get' which returns a wrapped chat room (if it exists)", function () { - test_utils.openChatRoom('lounge', 'localhost', 'dummy'); - var jid = 'lounge@localhost'; - var room = converse_api.rooms.get(jid); - expect(room instanceof Object).toBeTruthy(); - expect(room.is_chatroom).toBeTruthy(); - var chatroomview = converse.chatboxviews.get(jid); - expect(chatroomview.$el.is(':visible')).toBeTruthy(); - chatroomview.close(); + waits('300'); // ChatBox.show() is debounced for 250ms + runs(function () { + test_utils.openChatRoom('lounge', 'localhost', 'dummy'); + var jid = 'lounge@localhost'; + var room = converse_api.rooms.get(jid); + expect(room instanceof Object).toBeTruthy(); + expect(room.is_chatroom).toBeTruthy(); + var chatroomview = converse.chatboxviews.get(jid); + expect(chatroomview.$el.is(':visible')).toBeTruthy(); + chatroomview.close(); + }); + waits('300'); // ChatBox.show() is debounced for 250ms + runs(function () { + // Test with mixed case + test_utils.openChatRoom('Leisure', 'localhost', 'dummy'); + var jid = 'Leisure@localhost'; + var room = converse_api.rooms.get(jid); + expect(room instanceof Object).toBeTruthy(); + var chatroomview = converse.chatboxviews.get(jid.toLowerCase()); + expect(chatroomview.$el.is(':visible')).toBeTruthy(); + }); + waits('300'); // ChatBox.show() is debounced for 250ms + runs(function () { + var jid = 'leisure@localhost'; + var room = converse_api.rooms.get(jid); + expect(room instanceof Object).toBeTruthy(); + var chatroomview = converse.chatboxviews.get(jid.toLowerCase()); + expect(chatroomview.$el.is(':visible')).toBeTruthy(); - // Test with mixed case - test_utils.openChatRoom('Leisure', 'localhost', 'dummy'); - jid = 'Leisure@localhost'; - room = converse_api.rooms.get(jid); - expect(room instanceof Object).toBeTruthy(); - chatroomview = converse.chatboxviews.get(jid.toLowerCase()); - expect(chatroomview.$el.is(':visible')).toBeTruthy(); + jid = 'leiSure@localhost'; + room = converse_api.rooms.get(jid); + expect(room instanceof Object).toBeTruthy(); + chatroomview = converse.chatboxviews.get(jid.toLowerCase()); + expect(chatroomview.$el.is(':visible')).toBeTruthy(); + chatroomview.close(); - jid = 'leisure@localhost'; - room = converse_api.rooms.get(jid); - expect(room instanceof Object).toBeTruthy(); - chatroomview = converse.chatboxviews.get(jid.toLowerCase()); - expect(chatroomview.$el.is(':visible')).toBeTruthy(); - - jid = 'leiSure@localhost'; - room = converse_api.rooms.get(jid); - expect(room instanceof Object).toBeTruthy(); - chatroomview = converse.chatboxviews.get(jid.toLowerCase()); - expect(chatroomview.$el.is(':visible')).toBeTruthy(); - chatroomview.close(); - - // Non-existing room - jid = 'lounge2@localhost'; - room = converse_api.rooms.get(jid); - expect(typeof room === 'undefined').toBeTruthy(); + // Non-existing room + jid = 'lounge2@localhost'; + room = converse_api.rooms.get(jid); + expect(typeof room === 'undefined').toBeTruthy(); + }); }); it("has a method 'open' which opens and returns a wrapped chat box", function () { - // Test on chat room that doesn't exist. + var chatroomview; var jid = 'lounge@localhost'; var room = converse_api.rooms.open(jid); - expect(room instanceof Object).toBeTruthy(); - expect(room.is_chatroom).toBeTruthy(); - var chatroomview = converse.chatboxviews.get(jid); - expect(chatroomview.$el.is(':visible')).toBeTruthy(); + runs(function () { + // Test on chat room that doesn't exist. + expect(room instanceof Object).toBeTruthy(); + expect(room.is_chatroom).toBeTruthy(); + chatroomview = converse.chatboxviews.get(jid); + expect(chatroomview.$el.is(':visible')).toBeTruthy(); + }); + waits('300'); // ChatBox.show() is debounced for 250ms + runs(function () { + // Test again, now that the room exists. + room = converse_api.rooms.open(jid); + expect(room instanceof Object).toBeTruthy(); + expect(room.is_chatroom).toBeTruthy(); + chatroomview = converse.chatboxviews.get(jid); + expect(chatroomview.$el.is(':visible')).toBeTruthy(); + }); + waits('300'); // ChatBox.show() is debounced for 250ms + runs(function () { + // Test with mixed case in JID + jid = 'Leisure@localhost'; + room = converse_api.rooms.open(jid); + expect(room instanceof Object).toBeTruthy(); + chatroomview = converse.chatboxviews.get(jid.toLowerCase()); + expect(chatroomview.$el.is(':visible')).toBeTruthy(); - // Test again, now that the room exists. - room = converse_api.rooms.open(jid); - expect(room instanceof Object).toBeTruthy(); - expect(room.is_chatroom).toBeTruthy(); - chatroomview = converse.chatboxviews.get(jid); - expect(chatroomview.$el.is(':visible')).toBeTruthy(); + jid = 'leisure@localhost'; + room = converse_api.rooms.open(jid); + expect(room instanceof Object).toBeTruthy(); + chatroomview = converse.chatboxviews.get(jid.toLowerCase()); + expect(chatroomview.$el.is(':visible')).toBeTruthy(); - // Test with mixed case in JID - jid = 'Leisure@localhost'; - room = converse_api.rooms.open(jid); - expect(room instanceof Object).toBeTruthy(); - chatroomview = converse.chatboxviews.get(jid.toLowerCase()); - expect(chatroomview.$el.is(':visible')).toBeTruthy(); - - jid = 'leisure@localhost'; - room = converse_api.rooms.open(jid); - expect(room instanceof Object).toBeTruthy(); - chatroomview = converse.chatboxviews.get(jid.toLowerCase()); - expect(chatroomview.$el.is(':visible')).toBeTruthy(); - - jid = 'leiSure@localhost'; - room = converse_api.rooms.open(jid); - expect(room instanceof Object).toBeTruthy(); - chatroomview = converse.chatboxviews.get(jid.toLowerCase()); - expect(chatroomview.$el.is(':visible')).toBeTruthy(); - chatroomview.close(); + jid = 'leiSure@localhost'; + room = converse_api.rooms.open(jid); + expect(room instanceof Object).toBeTruthy(); + chatroomview = converse.chatboxviews.get(jid.toLowerCase()); + expect(chatroomview.$el.is(':visible')).toBeTruthy(); + chatroomview.close(); + }); }); }); diff --git a/src/converse-core.js b/src/converse-core.js index 70a8dcaaf..7e5b9abfe 100755 --- a/src/converse-core.js +++ b/src/converse-core.js @@ -1429,7 +1429,7 @@ converse.incrementMsgCounter(); } if (!this.model.get('minimized') && !this.$el.is(':visible')) { - _.debounce(this.show.bind(this), 100)(); + this.show(); } }, @@ -1830,7 +1830,7 @@ return this; }, - show: function (focus) { + show: _.debounce(function (callback) { if (this.$el.is(':visible') && this.$el.css('opacity') === "1") { if (focus) { this.focus(); } return this; @@ -1849,7 +1849,7 @@ } }.bind(this)); return this; - }, + }, 250, true), scrollDownMessageHeight: function ($message) { if (this.$content.is(':visible')) {