From 6ac4f2601ddd6e4b93018536862368a8083860ef Mon Sep 17 00:00:00 2001 From: JC Brand Date: Fri, 12 Aug 2016 20:38:39 +0000 Subject: [PATCH] Fixes #677 Chatbox does not open after close Problem was a race condition between hide and show methods. Solution was to not hide the chat box during the initialize method. --- docs/CHANGES.md | 3 +++ spec/chatbox.js | 11 ++++------- spec/converse.js | 5 ++--- src/converse-api.js | 4 +--- src/converse-chatview.js | 2 +- src/converse-core.js | 2 +- 6 files changed, 12 insertions(+), 15 deletions(-) diff --git a/docs/CHANGES.md b/docs/CHANGES.md index 3a1c636c0..78c312c79 100755 --- a/docs/CHANGES.md +++ b/docs/CHANGES.md @@ -4,6 +4,9 @@ - #632 Offline and Logout states do not properly update once users start chatting. [chrisuehlinger, jcband] - #674 Polish translation updated to the current master. [ser] +- #677 Chatbox does not open after close. [jcbrand] +- The behavior of `converse.chats.get` has changed. If the chat box is not + already open, then `undefined` will be returned. [jcbrand] - Typing (i.e. chat state) notifications are now also sent out from MUC rooms. [jcbrand] - `ChatRoomView.onChatRoomMessageSubmitted` has been renamed to `onMessageSubmitted`, to make it the same as the method on `ChatBoxView`. [jcbrand] diff --git a/spec/chatbox.js b/spec/chatbox.js index cc8528efa..8ee81604f 100644 --- a/spec/chatbox.js +++ b/spec/chatbox.js @@ -1041,16 +1041,13 @@ // state var msg = $msg({ from: sender_jid, - to: this.connection.jid, + to: converse.connection.jid, type: 'chat', id: (new Date()).getTime() - }).c('body').c('composing', {'xmlns': Strophe.NS.CHATSTATES}).tree(); - this.chatboxes.onMessage(msg); + }).c('composing', {'xmlns': Strophe.NS.CHATSTATES}).tree(); + converse.chatboxes.onMessage(msg); expect(converse.emit).toHaveBeenCalledWith('message', msg); - var chatboxview = this.chatboxviews.get(sender_jid); - expect(chatboxview).toBeDefined(); - expect(chatboxview.$el.is(':visible')).toBeFalsy(); // The chat box is not visible - }.bind(converse)); + }); describe("An active notification", function () { it("is sent when the user opens a chat box", function () { diff --git a/spec/converse.js b/spec/converse.js index 843ec33d9..07091f650 100644 --- a/spec/converse.js +++ b/spec/converse.js @@ -267,10 +267,9 @@ // Test on chat that's not open var box = converse_api.chats.get(jid); - expect(box instanceof Object).toBeTruthy(); - var chatboxview = converse.chatboxviews.get(jid); - expect(chatboxview.$el.is(':visible')).toBeFalsy(); + expect(typeof box === 'undefined').toBeTruthy(); + var chatboxview = converse.chatboxviews.get(jid); // Test for single JID test_utils.openChatBoxFor(jid); box = converse_api.chats.get(jid); diff --git a/src/converse-api.js b/src/converse-api.js index a53ac4608..db47b1cec 100644 --- a/src/converse-api.js +++ b/src/converse-api.js @@ -117,12 +117,10 @@ return null; } else if (typeof jids === "string") { chatbox = converse.wrappedChatBox(converse.chatboxes.getChatBox(jids, true)); - chatbox.open(); return chatbox; } return _.map(jids, function (jid) { chatbox = converse.wrappedChatBox(converse.chatboxes.getChatBox(jid, true)); - chatbox.open(); return chatbox; }); }, @@ -138,7 +136,7 @@ }); return result; } else if (typeof jids === "string") { - return converse.wrappedChatBox(converse.chatboxes.getChatBox(jids, true)); + return converse.wrappedChatBox(converse.chatboxes.getChatBox(jids)); } return _.map(jids, _.partial( diff --git a/src/converse-chatview.js b/src/converse-chatview.js index 9fb16c201..9ffccf56f 100644 --- a/src/converse-chatview.js +++ b/src/converse-chatview.js @@ -83,7 +83,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().hide(); + this.render().fetchMessages().insertIntoDOM().afterShown(); // 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)); diff --git a/src/converse-core.js b/src/converse-core.js index 472427fc6..f632457c7 100755 --- a/src/converse-core.js +++ b/src/converse-core.js @@ -1367,6 +1367,7 @@ contact_jid = from_bare_jid; resource = from_resource; } + converse.emit('message', message); // Get chat box, but only create a new one when the message has a body. chatbox = this.getChatBox(contact_jid, $message.find('body').length > 0); if (!chatbox) { @@ -1376,7 +1377,6 @@ return true; // We already have this message stored. } chatbox.createMessage($message, $delay, message); - converse.emit('message', message); return true; },