From b4d53aaa94ba170cad4e2d723bbb77ec13c44b48 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Fri, 9 Jan 2015 09:02:35 +0100 Subject: [PATCH] Some refactoring of chat states work. updates #292 - Don't add a timeout for the GONE state. - Change state to GONE when the user closes the chat box. - Change the state to inactive when the user minimizes the chat box. - Change the state to active when the users maximizes the chat box. - Add more tests for chat states. --- converse.js | 95 ++++++++++++--------- spec/chatbox.js | 217 +++++++++++++++++++++++++++++++----------------- tests/utils.js | 4 +- 3 files changed, 201 insertions(+), 115 deletions(-) diff --git a/converse.js b/converse.js index 0f92c2cc3..4c43c7a55 100644 --- a/converse.js +++ b/converse.js @@ -208,7 +208,7 @@ var PAUSED = 'paused'; var GONE = 'gone'; this.TIMEOUTS = { // Set as module attr so that we can override in tests. - 'PAUSED': 30000, + 'PAUSED': 20000, 'INACTIVE': 90000, 'GONE': 510000 }; @@ -722,7 +722,9 @@ this.messages.browserStorage = new Backbone.BrowserStorage[converse.storage]( b64_sha1('converse.messages'+this.get('jid')+converse.bare_jid)); this.save({ - 'chat_state': ACTIVE, + // The chat_state will be set to ACTIVE once the chat box is opened + // and we listen for change:chat_state, so shouldn't set it to ACTIVE here. + 'chat_state': undefined, 'box_id' : b64_sha1(this.get('jid')), 'height': height, 'minimized': this.get('minimized') || false, @@ -973,6 +975,8 @@ 'click .close-chatbox-button': 'close', 'click .toggle-chatbox-button': 'minimize', 'keypress textarea.chat-textarea': 'keyPressed', + 'focus textarea.chat-textarea': 'chatBoxFocused', + 'blur textarea.chat-textarea': 'chatBoxBlurred', 'click .toggle-smiley': 'toggleEmoticonMenu', 'click .toggle-smiley ul li': 'insertEmoticon', 'click .toggle-clear': 'clearMessages', @@ -1146,8 +1150,7 @@ }, sendMessageStanza: function (text) { - /* - * Sends the actual XML stanza to the XMPP server. + /* Sends the actual XML stanza to the XMPP server. */ // TODO: Look in ChatPartners to see what resources we have for the recipient. // if we have one resource, we sent to only that resources, if we have multiple @@ -1207,9 +1210,9 @@ }, sendChatState: function () { - /* XEP-0085 Chat State Notifications. - * Sends a message with the status of the user in this chat session + /* Sends a message with the status of the user in this chat session * as taken from the 'chat_state' attribute of the chat box. + * See XEP-0085 Chat State Notifications. */ converse.connection.send( $msg({'to':this.model.get('jid'), 'type': 'chat'}) @@ -1217,33 +1220,40 @@ ); }, - escalateChatState: function () { - /* XEP-0085 Chat State Notifications. - * This method gets called asynchronously via setTimeout. It escalates the - * chat state and depending on the current state can set a new timeout. + setChatState: function (state, no_save) { + /* Mutator for setting the chat state of this chat session. + * Handles clearing of any chat state notification timeouts and + * setting new ones if necessary. + * Timeouts are set when the state being set is COMPOSING or PAUSED. + * After the timeout, COMPOSING will become PAUSED and PAUSED will become INACTIVE. + * See XEP-0085 Chat State Notifications. + * + * Parameters: + * (string) state - The chat state (consts ACTIVE, COMPOSING, PAUSED, INACTIVE, GONE) + * (no_save) no_save - Just do the cleanup or setup but don't actually save the state. */ - delete this.chat_state_timeout; - if (this.model.get('chat_state') == COMPOSING) { - this.model.set('chat_state', PAUSED); - // From now on, if no activity in 2 mins, we'll set the - // state to - this.chat_state_timeout = setTimeout($.proxy(this.escalateChatState, this), converse.TIMEOUTS.INACTIVE); - } else if (this.model.get('chat_state') == PAUSED) { - this.model.set('chat_state', INACTIVE); - // From now on, if no activity in 10 mins, we'll set the - // state to - this.chat_state_timeout = setTimeout($.proxy(this.escalateChatState, this), converse.TIMEOUTS.GONE); - } else if (this.model.get('chat_state') == INACTIVE) { - this.model.set('chat_state', GONE); + if (_.contains([ACTIVE, INACTIVE, GONE], state)) { + if (typeof this.chat_state_timeout !== 'undefined') { + clearTimeout(this.chat_state_timeout); + delete this.chat_state_timeout; + } + } else if (state === COMPOSING) { + this.chat_state_timeout = setTimeout( + $.proxy(this.setChatState, this), converse.TIMEOUTS.PAUSED, PAUSED); + } else if (state === PAUSED) { + this.chat_state_timeout = setTimeout( + $.proxy(this.setChatState, this), converse.TIMEOUTS.INACTIVE, INACTIVE); } + if (!no_save && this.model.get('chat_state') != state) { + this.model.set('chat_state', state); + } + return this; }, keyPressed: function (ev) { + /* Event handler for when a key is pressed in a chat box textarea. + */ var $textarea = $(ev.target), message; - if (typeof this.chat_state_timeout !== 'undefined') { - clearTimeout(this.chat_state_timeout); - delete this.chat_state_timeout; // XXX: Necessary? - } if (ev.keyCode == KEY.ENTER) { ev.preventDefault(); message = $textarea.val(); @@ -1256,19 +1266,24 @@ } converse.emit('messageSend', message); } - this.model.set('chat_state', ACTIVE); - } else if (!this.model.get('chatroom')) { - // chat state data is currently only for single user chat - // Concerning group chat: http://xmpp.org/extensions/xep-0085.html#bizrules-groupchat - this.chat_state_timeout = setTimeout($.proxy(this.escalateChatState, this), converse.TIMEOUTS.PAUSED); - if (this.model.get('chat_state') != COMPOSING && ev.keyCode != KEY.FORWARD_SLASH) { - // Set chat state to composing if keyCode is not a forward-slash - // (which would imply an internal command and not a message). - this.model.set('chat_state', COMPOSING); - } + this.setChatState(ACTIVE); + } else if (!this.model.get('chatroom')) { // chat state data is currently only for single user chat + // Set chat state to composing if keyCode is not a forward-slash + // (which would imply an internal command and not a message). + this.setChatState(COMPOSING, ev.keyCode==KEY.FORWARD_SLASH); } }, + chatBoxFocused: function (ev) { + ev.preventDefault(); + this.setChatState(ACTIVE); + }, + + chatBoxBlurred: function (ev) { + ev.preventDefault(); + this.setChatState(INACTIVE); + }, + onDragResizeStart: function (ev) { if (!converse.allow_dragresize) { return true; } // Record element attributes for mouseMove(). @@ -1446,6 +1461,7 @@ } else { this.model.trigger('hide'); } + this.setChatState(GONE); converse.emit('chatBoxClosed', this); return this; }, @@ -1454,7 +1470,7 @@ // Restores a minimized chat box this.$el.insertAfter(converse.chatboxviews.get("controlbox").$el).show('fast', $.proxy(function () { converse.refreshWebkit(); - this.focus(); + this.setChatState(ACTIVE).focus(); converse.emit('chatBoxMaximized', this); }, this)); }, @@ -1462,7 +1478,7 @@ minimize: function (ev) { if (ev && ev.preventDefault) { ev.preventDefault(); } // Minimizes a chat box - this.model.minimize(); + this.setChatState(INACTIVE).model.minimize(); this.$el.hide('fast', converse.refreshwebkit); converse.emit('chatBoxMinimized', this); }, @@ -1591,6 +1607,7 @@ this.model.save(); this.initDragResize(); } + this.setChatState(ACTIVE); return this; }, diff --git a/spec/chatbox.js b/spec/chatbox.js index 5b1f6a146..5742acd88 100644 --- a/spec/chatbox.js +++ b/spec/chatbox.js @@ -408,43 +408,6 @@ runs(function () {}); }); - it("can indicate a chat state notification", $.proxy(function () { - // See XEP-0085 http://xmpp.org/extensions/xep-0085.html#definitions - spyOn(converse, 'emit'); - var sender_jid = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@localhost'; - - // state - var msg = $msg({ - from: sender_jid, - to: this.connection.jid, - type: 'chat', - id: (new Date()).getTime() - }).c('body').c('composing', {'xmlns': Strophe.NS.CHATSTATES}).tree(); - - this.chatboxes.onMessage(msg); - expect(converse.emit).toHaveBeenCalledWith('message', msg); - var chatboxview = this.chatboxviews.get(sender_jid); - expect(chatboxview).toBeDefined(); - // Check that the notification appears inside the chatbox in the DOM - var $events = chatboxview.$el.find('.chat-event'); - expect($events.length).toBe(1); - expect($events.text()).toEqual(mock.cur_names[1].split(' ')[0] + ' is typing'); - - // state - msg = $msg({ - from: sender_jid, - to: this.connection.jid, - type: 'chat', - id: (new Date()).getTime() - }).c('body').c('paused', {'xmlns': Strophe.NS.CHATSTATES}).tree(); - - this.chatboxes.onMessage(msg); - expect(converse.emit).toHaveBeenCalledWith('message', msg); - $events = chatboxview.$el.find('.chat-event'); - expect($events.length).toBe(1); - expect($events.text()).toEqual(mock.cur_names[1].split(' ')[0] + ' has stopped typing'); - }, converse)); - it("can be received which will open a chatbox and be displayed inside it", $.proxy(function () { spyOn(converse, 'emit'); var message = 'This is a received message'; @@ -552,7 +515,7 @@ expect(trimmed_chatboxes.keys().length).toBe(0); }, converse)); }, converse)); - + it("will indicate when it has a time difference of more than a day between it and its predecessor", $.proxy(function () { spyOn(converse, 'emit'); var contact_name = mock.cur_names[1]; @@ -731,7 +694,38 @@ describe("A Chat Status Notification", $.proxy(function () { - describe("A composing notifciation", $.proxy(function () { + describe("An active notification", $.proxy(function () { + it("is sent when the user opens a chat box", $.proxy(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'); + }, converse)); + + it("is sent when the user maximizes a minimized a chat box", $.proxy(function () { + var contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; + test_utils.openChatBoxFor(contact_jid); + var view = this.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(1); + expect($stanza.children().prop('tagName')).toBe('active'); + }, converse)); + }, converse)); + + describe("A composing notification", $.proxy(function () { it("is sent as soon as the user starts typing a message which is not a command", $.proxy(function () { var contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; test_utils.openChatBoxFor(contact_jid); @@ -748,7 +742,7 @@ 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'), @@ -757,6 +751,28 @@ expect(view.model.get('chat_state')).toBe('composing'); expect(converse.emit.callCount, 1); }, converse)); + + it("will be shown if received", $.proxy(function () { + // See XEP-0085 http://xmpp.org/extensions/xep-0085.html#definitions + spyOn(converse, 'emit'); + var sender_jid = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@localhost'; + + // state + var msg = $msg({ + from: sender_jid, + to: this.connection.jid, + type: 'chat', + id: (new Date()).getTime() + }).c('body').c('composing', {'xmlns': Strophe.NS.CHATSTATES}).tree(); + this.chatboxes.onMessage(msg); + expect(converse.emit).toHaveBeenCalledWith('message', msg); + var chatboxview = this.chatboxviews.get(sender_jid); + expect(chatboxview).toBeDefined(); + // Check that the notification appears inside the chatbox in the DOM + var $events = chatboxview.$el.find('.chat-event'); + expect($events.length).toBe(1); + expect($events.text()).toEqual(mock.cur_names[1].split(' ')[0] + ' is typing'); + }, converse)); }, converse)); describe("A paused notification", $.proxy(function () { @@ -784,12 +800,32 @@ expect($stanza.children().prop('tagName')).toBe('paused'); }); }, converse)); + + it("will be shown if received", $.proxy(function () { + // TODO: only show paused state if the previous state was composing + // See XEP-0085 http://xmpp.org/extensions/xep-0085.html#definitions + spyOn(converse, 'emit'); + var sender_jid = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@localhost'; + // state + msg = $msg({ + from: sender_jid, + to: this.connection.jid, + type: 'chat', + id: (new Date()).getTime() + }).c('body').c('paused', {'xmlns': Strophe.NS.CHATSTATES}).tree(); + this.chatboxes.onMessage(msg); + expect(converse.emit).toHaveBeenCalledWith('message', msg); + var chatboxview = this.chatboxviews.get(sender_jid); + $events = chatboxview.$el.find('.chat-event'); + expect($events.length).toBe(1); + expect($events.text()).toEqual(mock.cur_names[1].split(' ')[0] + ' has stopped typing'); + }, converse)); }, converse)); describe("An inactive notifciation", $.proxy(function () { it("is sent if the user has stopped typing since 2 minutes", $.proxy(function () { // Make the timeouts shorter so that we can test - this.TIMEOUTS.PAUSED = 200; + this.TIMEOUTS.PAUSED = 200; this.TIMEOUTS.INACTIVE = 200; var contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; test_utils.openChatBoxFor(contact_jid); @@ -817,46 +853,79 @@ expect($stanza.children().prop('tagName')).toBe('inactive'); }); }, converse)); - }, converse)); - describe("An gone notifciation", $.proxy(function () { - it("is sent if the user has stopped typing since 10 minutes", $.proxy(function () { - // Make the timeouts shorter so that we can test - this.TIMEOUTS.PAUSED = 200; - this.TIMEOUTS.INACTIVE = 200; - this.TIMEOUTS.GONE = 200; + it("is sent when the user a minimizes a chat box", $.proxy(function () { var contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost'; test_utils.openChatBoxFor(contact_jid); var view = this.chatboxviews.get(contact_jid); - runs(function () { - expect(view.model.get('chat_state')).toBe('active'); - view.keyPressed({ - target: view.$el.find('textarea.chat-textarea'), - keyCode: 1 - }); - expect(view.model.get('chat_state')).toBe('composing'); - }); - waits(250); - runs(function () { - expect(view.model.get('chat_state')).toBe('paused'); - }); - waits(250); - runs(function () { - expect(view.model.get('chat_state')).toBe('inactive'); - spyOn(converse.connection, 'send'); - }); - waits(250); - runs(function () { - expect(view.model.get('chat_state')).toBe('gone'); - 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('gone'); - }); + spyOn(converse.connection, 'send'); + view.minimize(); + 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'); }, converse)); + + it("will be shown if received", $.proxy(function () { + // TODO: only show paused state if the previous state was composing + // See XEP-0085 http://xmpp.org/extensions/xep-0085.html#definitions + spyOn(converse, 'emit'); + var sender_jid = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@localhost'; + // state + msg = $msg({ + from: sender_jid, + to: this.connection.jid, + type: 'chat', + id: (new Date()).getTime() + }).c('body').c('paused', {'xmlns': Strophe.NS.CHATSTATES}).tree(); + this.chatboxes.onMessage(msg); + expect(converse.emit).toHaveBeenCalledWith('message', msg); + var chatboxview = this.chatboxviews.get(sender_jid); + $events = chatboxview.$el.find('.chat-event'); + expect($events.length).toBe(1); + expect($events.text()).toEqual(mock.cur_names[1].split(' ')[0] + ' has stopped typing'); + }, converse)); + }, converse)); + describe("An gone notifciation", $.proxy(function () { + it("is sent if the user closes a chat box", $.proxy(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('gone'); + 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('gone'); + }, converse)); + + xit("will be shown if received", $.proxy(function () { + // TODO: only show paused state if the previous state was composing + // See XEP-0085 http://xmpp.org/extensions/xep-0085.html#definitions + spyOn(converse, 'emit'); + var sender_jid = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@localhost'; + // state + msg = $msg({ + from: sender_jid, + to: this.connection.jid, + type: 'chat', + id: (new Date()).getTime() + }).c('body').c('gone', {'xmlns': Strophe.NS.CHATSTATES}).tree(); + this.chatboxes.onMessage(msg); + expect(converse.emit).toHaveBeenCalledWith('message', msg); + var chatboxview = this.chatboxviews.get(sender_jid); + $events = chatboxview.$el.find('.chat-event'); + expect($events.length).toBe(1); + expect($events.text()).toEqual(mock.cur_names[1].split(' ')[0] + ' has stopped typing'); + }, converse)); + }, converse)); }, converse)); }, converse)); diff --git a/tests/utils.js b/tests/utils.js index cf9736637..98576072c 100644 --- a/tests/utils.js +++ b/tests/utils.js @@ -12,14 +12,14 @@ utils.createRequest = function (iq) { iq = typeof iq.tree == "function" ? iq.tree() : iq; var req = new Strophe.Request(iq, function() {}); - req.getResponse = function() { + req.getResponse = function() { var env = new Strophe.Builder('env', {type: 'mock'}).tree(); env.appendChild(iq); return env; }; return req; }; - + utils.closeAllChatBoxes = function () { var i, chatbox; for (i=converse.chatboxes.models.length-1; i>-1; i--) {