From a875ba51a0119559d0b20f65f44233bfd9e4c476 Mon Sep 17 00:00:00 2001 From: ichim-david Date: Tue, 19 Feb 2013 18:16:05 +0200 Subject: [PATCH 01/15] Moved assignment of template for RosterView out of the render method to avoid calling empty and readding items everytime the render method is called --- converse.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/converse.js b/converse.js index b3943e8f4..b68c09cd6 100644 --- a/converse.js +++ b/converse.js @@ -523,7 +523,7 @@ '' + 'Avatar of {{fullname}}' + '
{{ fullname }}
' + - '' + + '' + '

' + '' + '

' + @@ -1548,6 +1548,8 @@ delete this.rosteritemviews[item.id]; this.render(); }, this); + + this.$el.html(this.template()); }, template: _.template('
Contact requests
' + @@ -1555,7 +1557,6 @@ '
Pending contacts
'), render: function () { - this.$el.empty().html(this.template()); var models = this.model.sort().models, children = $(this.el).children(), $my_contacts = this.$el.find('#xmpp-contacts').hide(), From 95fdf98ea695946321be0c1f1db5cf5e7f3bee43 Mon Sep 17 00:00:00 2001 From: ichim-david Date: Wed, 20 Feb 2013 15:36:55 +0200 Subject: [PATCH 02/15] Pass item to render method of RosterView instead of looping over each item on every render call, Inverted visibility logic for contact titles in order to avoid multiple show and hide --- converse.js | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/converse.js b/converse.js index b68c09cd6..39b7d818f 100644 --- a/converse.js +++ b/converse.js @@ -1537,16 +1537,16 @@ this.model.remove(item.id); }, this); } - this.render(); + this.render(item); }, this); this.model.on('change', function (item) { - this.render(); + this.render(item); }, this); this.model.on("remove", function (item) { delete this.rosteritemviews[item.id]; - this.render(); + this.render(item); }, this); this.$el.html(this.template()); @@ -1556,20 +1556,19 @@ '
My contacts
' + '
Pending contacts
'), - render: function () { - var models = this.model.sort().models, - children = $(this.el).children(), - $my_contacts = this.$el.find('#xmpp-contacts').hide(), - $contact_requests = this.$el.find('#xmpp-contact-requests').hide(), - $pending_contacts = this.$el.find('#pending-xmpp-contacts').hide(), + render: function (item) { + if (!item) { + return this; + } + var + $my_contacts = this.$el.find('#xmpp-contacts').show(), + $contact_requests = this.$el.find('#xmpp-contact-requests').show(), + $pending_contacts = this.$el.find('#pending-xmpp-contacts').show(), $count, num; - - for (var i=0; i 0) { - h.show(); + if (!h.nextUntil('dt').length) { + h.hide(); } }); $count = $('#online-count'); From 447c3a8d4106bcb78b06b0d17cc5b6deeed58b7d Mon Sep 17 00:00:00 2001 From: ichim-david Date: Thu, 21 Feb 2013 20:57:22 +0200 Subject: [PATCH 03/15] Set modifications of model attributes only if the item received from the rosterHandler has different options --- converse.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/converse.js b/converse.js index 39b7d818f..1730fa343 100644 --- a/converse.js +++ b/converse.js @@ -1414,7 +1414,11 @@ if (!model) { this.addRosterItem(item.jid, item.subscription, item.ask, item.name); } else { - model.set({'subscription': item.subscription, 'ask': item.ask}); + // only modify model attributes if they are different from the + // ones that were already set when the rosterItem was added + if (model.get('subscription') !== item.subscription || model.get('ask') !== item.ask) { + model.set({'subscription': item.subscription, 'ask': item.ask}); + } } } }, From 3cf671884cb049da9f920bf570d45d3d4f29994d Mon Sep 17 00:00:00 2001 From: ichim-david Date: Thu, 21 Feb 2013 21:28:40 +0200 Subject: [PATCH 04/15] Fixed the following: - Fixed function has inconsistent return points by always specifying returns instead of returning only on condition - Set room-jid as data-room-jid attribute to validate html5 - Added missing semicolon --- converse.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/converse.js b/converse.js index 1730fa343..9831a68db 100644 --- a/converse.js +++ b/converse.js @@ -186,7 +186,8 @@ var msgs = store.get(hex_sha1(this.get('own_jid')+bare_jid)) || []; if (msgs.length) { return sjcl.decrypt(hex_sha1(this.get('own_jid')), msgs[msgs.length-1]); - } + } + return undefined; }, clearMessages: function (jid) { @@ -651,7 +652,9 @@ }, room_template: _.template( '
' + - '' + + '' + '{{name}}
'), tab_template: _.template('
  • Rooms
    • '), @@ -701,7 +704,7 @@ ev.preventDefault(); var name, jid; if (ev.type === 'click') { - jid = $(ev.target).attr('room-jid'); + jid = $(ev.target).attr('data-room-jid'); } else { name = _.str.strip($(ev.target).find('input.new-chatroom-name').val()).toLowerCase(); if (name) { @@ -1115,12 +1118,13 @@ view.messageReceived(message); xmppchat.roster.addResource(partner_jid, resource); }, this)); - return; + return undefined; } else if (!view.isVisible()) { this.showChat(partner_jid); } view.messageReceived(message); xmppchat.roster.addResource(partner_jid, resource); + return true; }, initialize: function () { @@ -1394,6 +1398,7 @@ if (item) { return _.size(item.get('resources')); } + return 0; }, getNumOnlineContacts: function () { @@ -1779,7 +1784,7 @@ $(document).unbind('jarnxmpp.connected'); $(document).bind('jarnxmpp.connected', $.proxy(function (ev, connection) { - this.connection = connection + this.connection = connection; // this.connection.xmlInput = function (body) { console.log(body); }; // this.connection.xmlOutput = function (body) { console.log(body); }; this.connection.bare_jid = Strophe.getBareJidFromJid(this.connection.jid); From a256645d6da71491c5b9bb79f6bc3f98492e41c8 Mon Sep 17 00:00:00 2001 From: ichim-david Date: Thu, 21 Feb 2013 22:24:09 +0200 Subject: [PATCH 05/15] Removed rendering of rosterview since I've added check to render only with passed item and the result is the same without the rendering --- converse.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/converse.js b/converse.js index 9831a68db..3c42c9a52 100644 --- a/converse.js +++ b/converse.js @@ -1146,7 +1146,7 @@ // Add the roster xmppchat.roster = new xmppchat.RosterItems(); - xmppchat.rosterview = new xmppchat.RosterView({'model':xmppchat.roster}).render(); + xmppchat.rosterview = new xmppchat.RosterView({'model':xmppchat.roster}); xmppchat.rosterview.$el.appendTo(controlbox.contactspanel.$el); // Rebind events (necessary for click events on tabs inserted via the panels) From 66d7b515357465fb02e845970e83c034c88827e2 Mon Sep 17 00:00:00 2001 From: ichim-david Date: Thu, 21 Feb 2013 22:36:41 +0200 Subject: [PATCH 06/15] Changed the following: - Pass an option object to addRosterItem which can contains information about the object catched in the rosterHandler. In this case I am marking the last item available in the roster of the person for which this handler is triggered --- converse.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/converse.js b/converse.js index 3c42c9a52..b1bf2ebd6 100644 --- a/converse.js +++ b/converse.js @@ -1349,8 +1349,9 @@ return Backbone.Collection.prototype.get.call(this, id); }, - addRosterItem: function (jid, subscription, ask, name) { + addRosterItem: function (jid, subscription, ask, name, options) { var model = new xmppchat.RosterItem(jid, subscription, ask, name); + model.options = options || {}; this.add(model); }, @@ -1412,12 +1413,17 @@ }, rosterHandler: function (items) { - var model, item; - for (var i=0; i Date: Fri, 22 Feb 2013 14:29:49 +0200 Subject: [PATCH 07/15] Changed the following: - Show the xmppchat-roster only after all of the items have been added from first run of rosterHandler - Resort the items only when the presence has changed and not when the resource attribute has changed --- converse.js | 44 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/converse.js b/converse.js index b1bf2ebd6..5ece3ed3c 100644 --- a/converse.js +++ b/converse.js @@ -1564,6 +1564,7 @@ this.render(item); }, this); + this.$el.hide(); this.$el.html(this.template()); }, @@ -1575,11 +1576,10 @@ if (!item) { return this; } - var - $my_contacts = this.$el.find('#xmpp-contacts').show(), + var $my_contacts = this.$el.find('#xmpp-contacts').show(), $contact_requests = this.$el.find('#xmpp-contact-requests').show(), $pending_contacts = this.$el.find('#pending-xmpp-contacts').show(), - $count, num; + $count, num, presence_change; var user_id = Strophe.getNodeFromJid(item.id), view = this.rosteritemviews[item.id], ask = item.get('ask'), @@ -1593,13 +1593,37 @@ $contact_requests.after(view.render().el); $contact_requests.after($contact_requests.siblings('dd.requesting-xmpp-contact').tsort(crit)); } else if (subscription === 'both') { - $my_contacts.after(view.render().el); - $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.offline').tsort('a', crit)); - $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.unavailable').tsort('a', crit)); - $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.away').tsort('a', crit)); - $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.busy').tsort('a', crit)); - $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.online').tsort('a', crit)); - } + if (!item.options.sorted) { + // this attribute will be true only after all of the elements have been added on the page + // at this point all offline + $my_contacts.after(view.render().el); + } + else { + // just by calling render will be enough to change the icon of the existing item without + // having to reinsert it and the sort will come from the presence change + view.render(); + } + } + presence_change = view.model.changed['presence_type']; + if (presence_change) { + // resort all items only if the model has changed it's presence_type as this render + // is also triggered when the resource is changed which always comes before the presence change + // therefore we avoid resorting when the change doesn't affect the position of the item + $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.offline').tsort('a', crit)); + $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.unavailable').tsort('a', crit)); + $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.away').tsort('a', crit)); + $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.busy').tsort('a', crit)); + $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.online').tsort('a', crit)); + } + + if (item.options.isLast && !item.options.sorted) { + // this will be true after all of the roster items have been added with the default + // options where all of the items are offline and now we can show the rosterView + item.options.sorted = true; + $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.offline').tsort('a', crit)); + $my_contacts.after($my_contacts.siblings('dd.current-xmpp-contact.unavailable').tsort('a', crit)); + this.$el.show(); + } // Hide the headings if there are no contacts under them _.each([$my_contacts, $contact_requests, $pending_contacts], function (h) { if (!h.nextUntil('dt').length) { From d8d3a34737668d44466db5762ffe276c0009105f Mon Sep 17 00:00:00 2001 From: ichim-david Date: Sat, 23 Feb 2013 15:43:20 +0200 Subject: [PATCH 08/15] Fixed a bug where controlbox wasn't available on xmppchat.chatboxesview, instead pass the removal of the model to rosterview.model --- converse.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/converse.js b/converse.js index 49ba7ea3e..10793ca59 100644 --- a/converse.js +++ b/converse.js @@ -1251,7 +1251,12 @@ $(this).dialog( "close" ); xmppchat.connection.roster.remove(bare_jid, function (iq) { xmppchat.connection.roster.unauthorize(bare_jid); - xmppchat.chatboxesview.controlbox.roster.remove(bare_jid); + // TODO inspect if chatboxes ever receives controlbox + if (xmppchat.chatboxesview.controlbox) { + xmppchat.chatboxesview.controlbox.roster.remove(bare_jid); + } + // remove model from view roster + xmppchat.rosterview.model.remove(bare_jid); }); }, "Cancel": function() { From 85e9c7cecc0ba1731927ad95d0ee00abdd60d500 Mon Sep 17 00:00:00 2001 From: ichim-david Date: Sat, 23 Feb 2013 16:17:00 +0200 Subject: [PATCH 09/15] Changed the following: - Subscribe event with help of jQuery one instead of delegate, now that the rosterView is no longer emptied on every render to avoid registring several handlers of remove action, which meant you had to click 4 times for the jQuery Dialog to go away - Remove the element from the rosterView when the remove event is triggered on the model which removes just that item instead of emptying the list and appending the remaining models again --- converse.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/converse.js b/converse.js index 10793ca59..0b6b96bcb 100644 --- a/converse.js +++ b/converse.js @@ -1327,7 +1327,7 @@ } // Event handlers - this.$el.delegate('a.remove-xmpp-contact','click', function (ev) { + this.$el.find('.remove-xmpp-contact').one('click', function (ev) { ev.preventDefault(); that.removeContact(); }); @@ -1608,8 +1608,10 @@ }, this); this.model.on("remove", function (item) { + // remove element from the rosterView instance + this.rosteritemviews[item.id].$el.remove(); + delete this.rosteritemviews[item.id]; - this.render(item); }, this); this.$el.hide(); From 7daa681ac37b8fbf5ad400aa1705593b25cc26e3 Mon Sep 17 00:00:00 2001 From: ichim-david Date: Sat, 23 Feb 2013 16:49:01 +0200 Subject: [PATCH 10/15] Show dt elements on xmppchat-roster only if they have length and they are not already visible to avoid making them all the time visible when they might already be so --- converse.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/converse.js b/converse.js index 0b6b96bcb..abe296cf5 100644 --- a/converse.js +++ b/converse.js @@ -1610,7 +1610,7 @@ this.model.on("remove", function (item) { // remove element from the rosterView instance this.rosteritemviews[item.id].$el.remove(); - + delete this.rosteritemviews[item.id]; }, this); @@ -1626,9 +1626,9 @@ if (!item) { return this; } - var $my_contacts = this.$el.find('#xmpp-contacts').show(), - $contact_requests = this.$el.find('#xmpp-contact-requests').show(), - $pending_contacts = this.$el.find('#pending-xmpp-contacts').show(), + var $my_contacts = this.$el.find('#xmpp-contacts'), + $contact_requests = this.$el.find('#xmpp-contact-requests'), + $pending_contacts = this.$el.find('#pending-xmpp-contacts'), $count, num, presence_change; var user_id = Strophe.getNodeFromJid(item.id), view = this.rosteritemviews[item.id], @@ -1676,8 +1676,8 @@ } // Hide the headings if there are no contacts under them _.each([$my_contacts, $contact_requests, $pending_contacts], function (h) { - if (!h.nextUntil('dt').length) { - h.hide(); + if (h.nextUntil('dt').length && !h.is('visible')) { + h.show(); } }); $count = $('#online-count'); From 00aba6a9cd012f91590c3a359cf64698bd46e222 Mon Sep 17 00:00:00 2001 From: ichim-david Date: Sat, 23 Feb 2013 22:20:53 +0200 Subject: [PATCH 11/15] Move the event handlers on the view instead of registering them in render method of RosterItemView --- converse.js | 52 ++++++++++++++++++++++------------------------------ 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/converse.js b/converse.js index abe296cf5..7419eeab4 100644 --- a/converse.js +++ b/converse.js @@ -461,8 +461,8 @@ } else if (match[1] === "help") { msgs = [ - '/help: Show this menu', - '/clear: Remove messages' + '/help: Show this menu', + '/clear: Remove messages' ]; this.addHelpMessages(msgs); return; @@ -502,7 +502,7 @@ this.$el.data('composing', false); } else { composing = this.$el.data('composing'); - if (!composing) { + if (!composing) { if (ev.keyCode != 47) { // We don't send composing messages if the message // starts with forward-slash. @@ -934,10 +934,10 @@ initialize: function () { xmppchat.connection.muc.join( - this.model.get('jid'), - this.model.get('nick'), - $.proxy(this.onChatRoomMessage, this), - $.proxy(this.onChatRoomPresence, this), + this.model.get('jid'), + this.model.get('nick'), + $.proxy(this.onChatRoomMessage, this), + $.proxy(this.onChatRoomPresence, this), $.proxy(this.onChatRoomRoster, this)); }, @@ -981,7 +981,7 @@ } } else { if (sender === this.model.get('nick')) { - // Our own message which is already appended + // Our own message which is already appended return true; } else { $chat_content.find('div.chat-event').remove(); @@ -1228,8 +1228,16 @@ xmppchat.RosterItemView = Backbone.View.extend({ tagName: 'dd', - openChat: function () { + events: { + "click .accept-xmpp-request": "acceptRequest", + "click .decline-xmpp-request": "declineRequest", + "click .open-chat": "openChat", + "click .remove-xmpp-contact": "removeContact" + }, + + openChat: function (ev) { xmppchat.chatboxesview.openChat(this.model.get('jid')); + ev.preventDefault(); }, removeContact: function () { @@ -1266,18 +1274,20 @@ }); }, - acceptRequest: function () { + acceptRequest: function (ev) { var jid = this.model.get('jid'); xmppchat.connection.roster.authorize(jid); xmppchat.connection.roster.add(jid, this.model.get('fullname'), [], function (iq) { xmppchat.connection.roster.subscribe(jid); }); + ev.preventDefault(); }, - declineRequest: function () { + declineRequest: function (ev) { var that = this; xmppchat.connection.roster.unauthorize(this.model.get('jid')); that.trigger('decline-request', that.model); + ev.preventDefault(); }, template: _.template( @@ -1301,36 +1311,19 @@ that = this, subscription = item.get('subscription'); this.$el.addClass(item.get('presence_type')); - + if (ask === 'subscribe') { this.$el.addClass('pending-xmpp-contact'); this.$el.html(this.pending_template(item.toJSON())); } else if (ask === 'request') { this.$el.addClass('requesting-xmpp-contact'); this.$el.html(this.request_template(item.toJSON())); - this.$el.delegate('button.accept-xmpp-request', 'click', function (ev) { - ev.preventDefault(); - that.acceptRequest(); - }); - this.$el.delegate('button.decline-xmpp-request', 'click', function (ev) { - ev.preventDefault(); - that.declineRequest(); - }); xmppchat.chatboxesview.openChat('controlbox'); } else if (subscription === 'both') { this.$el.addClass('current-xmpp-contact'); this.$el.html(this.template(item.toJSON())); - this.$el.delegate('a.open-chat', 'click', function (ev) { - ev.preventDefault(); - that.openChat(); - }); } - // Event handlers - this.$el.find('.remove-xmpp-contact').one('click', function (ev) { - ev.preventDefault(); - that.removeContact(); - }); return this; }, @@ -1926,4 +1919,3 @@ return xmppchat; })); - From 19caf7c448871575910566ba4408623a0d7907eb Mon Sep 17 00:00:00 2001 From: ichim-david Date: Sun, 24 Feb 2013 10:59:57 +0200 Subject: [PATCH 12/15] Prevent default event for the removeContact link in order to avoid page reload when jQuery Dialog is openened from clicking on the delete link --- converse.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/converse.js b/converse.js index 7419eeab4..f5e076ce5 100644 --- a/converse.js +++ b/converse.js @@ -1240,7 +1240,7 @@ ev.preventDefault(); }, - removeContact: function () { + removeContact: function (ev) { var that = this; $("").dialog({ title: 'Are you sure you want to remove this contact?', @@ -1272,6 +1272,7 @@ } } }); + ev.preventDefault(); }, acceptRequest: function (ev) { @@ -1669,7 +1670,7 @@ } // Hide the headings if there are no contacts under them _.each([$my_contacts, $contact_requests, $pending_contacts], function (h) { - if (h.nextUntil('dt').length && !h.is('visible')) { + if (h.nextUntil('dt').length && !h.is(':visible')) { h.show(); } }); From 63134858d70d3cb4d6cafb7bb9d3ca2ddfcd7477 Mon Sep 17 00:00:00 2001 From: ichim-david Date: Sun, 24 Feb 2013 11:31:47 +0200 Subject: [PATCH 13/15] Properly hide or show dt that no longer have dd adjacent --- converse.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/converse.js b/converse.js index f5e076ce5..7b444609a 100644 --- a/converse.js +++ b/converse.js @@ -1670,9 +1670,12 @@ } // Hide the headings if there are no contacts under them _.each([$my_contacts, $contact_requests, $pending_contacts], function (h) { - if (h.nextUntil('dt').length && !h.is(':visible')) { + if (h.nextUntil('dt').length) { h.show(); } + else { + h.hide(); + } }); $count = $('#online-count'); $count.text(this.model.getNumOnlineContacts()); From fe0034f123413537148297ea28145e5d841ddd8f Mon Sep 17 00:00:00 2001 From: ichim-david Date: Mon, 25 Feb 2013 14:34:02 +0200 Subject: [PATCH 14/15] Check also if subscription === to in order to correctly move the accepted contact subscription from pending to current --- converse.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/converse.js b/converse.js index 7b444609a..41bc568aa 100644 --- a/converse.js +++ b/converse.js @@ -1320,7 +1320,7 @@ this.$el.addClass('requesting-xmpp-contact'); this.$el.html(this.request_template(item.toJSON())); xmppchat.chatboxesview.openChat('controlbox'); - } else if (subscription === 'both') { + } else if (subscription === 'both' || subscription === 'to') { this.$el.addClass('current-xmpp-contact'); this.$el.html(this.template(item.toJSON())); } @@ -1636,7 +1636,7 @@ } else if (ask === 'request') { $contact_requests.after(view.render().el); $contact_requests.after($contact_requests.siblings('dd.requesting-xmpp-contact').tsort(crit)); - } else if (subscription === 'both') { + } else if (subscription === 'both' || subscription === 'to') { if (!item.options.sorted) { // this attribute will be true only after all of the elements have been added on the page // at this point all offline From 732f941e205e554de7ffe6832047af8ab8b90826 Mon Sep 17 00:00:00 2001 From: ichim-david Date: Thu, 28 Feb 2013 07:48:06 +0200 Subject: [PATCH 15/15] Fixed first jasmine test for visibility of xmpp-roster --- spec/RosterSpec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/RosterSpec.js b/spec/RosterSpec.js index b26bd9cc2..dc1518df5 100644 --- a/spec/RosterSpec.js +++ b/spec/RosterSpec.js @@ -31,12 +31,12 @@ xmppchat.rosterview = new xmppchat.RosterView({'model':xmppchat.roster}); // stub xmppchat.chatboxesview = {openChat: function () {} }; - // Hack to make sure there is an element. - xmppchat.rosterview.$el = $('
    '); xmppchat.rosterview.render(); + // by default the dts are hidden from css class and only later they will be hidden + // by jQuery therefore for the first check we will see if visible instead of none it("should hide the requesting contacts heading if there aren't any", function () { - expect(xmppchat.rosterview.$el.find('dt#xmpp-contact-requests').css('display')).toEqual('none'); + expect(xmppchat.rosterview.$el.find('dt#xmpp-contact-requests').is(':visible')).toEqual(false); }); it("should be able to add requesting contacts, and they should be sorted alphabetically", function () {