Commit 9a526d41 authored by JC Brand's avatar JC Brand

Refactoring of the XEP-0085 Chat State Notifications code

* Distinguish between CSN messages and other types of messages
* Properly clear previous notifications
* Better handling of notifications from multiple users (in MUC)
* Rename methods to make clarify intent
parent e961fb51
...@@ -7354,6 +7354,9 @@ body.reset { ...@@ -7354,6 +7354,9 @@ body.reset {
#converse-embedded-chat .chatbox .chat-body .chat-info.badge, #converse-embedded-chat .chatbox .chat-body .chat-info.badge,
#conversejs .chatbox .chat-body .chat-info.badge { #conversejs .chatbox .chat-body .chat-info.badge {
color: white; } color: white; }
#converse-embedded-chat .chatbox .chat-body .chat-info.chat-state-notification,
#conversejs .chatbox .chat-body .chat-info.chat-state-notification {
font-style: italic; }
#converse-embedded-chat .chatbox .chat-body .chat-info.chat-event, #converse-embedded-chat .chatbox .chat-body .chat-info.chat-event,
#conversejs .chatbox .chat-body .chat-info.chat-event { #conversejs .chatbox .chat-body .chat-info.chat-event {
clear: left; clear: left;
......
...@@ -7407,6 +7407,9 @@ body { ...@@ -7407,6 +7407,9 @@ body {
#converse-embedded-chat .chatbox .chat-body .chat-info.badge, #converse-embedded-chat .chatbox .chat-body .chat-info.badge,
#conversejs .chatbox .chat-body .chat-info.badge { #conversejs .chatbox .chat-body .chat-info.badge {
color: white; } color: white; }
#converse-embedded-chat .chatbox .chat-body .chat-info.chat-state-notification,
#conversejs .chatbox .chat-body .chat-info.chat-state-notification {
font-style: italic; }
#converse-embedded-chat .chatbox .chat-body .chat-info.chat-event, #converse-embedded-chat .chatbox .chat-body .chat-info.chat-event,
#conversejs .chatbox .chat-body .chat-info.chat-event { #conversejs .chatbox .chat-body .chat-info.chat-event {
clear: left; clear: left;
......
...@@ -182,6 +182,9 @@ ...@@ -182,6 +182,9 @@
&.badge { &.badge {
color: $chat-head-text-color; color: $chat-head-text-color;
} }
&.chat-state-notification {
font-style: italic;
}
&.chat-event { &.chat-event {
clear: left; clear: left;
font-style: italic; font-style: italic;
......
...@@ -1634,10 +1634,10 @@ ...@@ -1634,10 +1634,10 @@
describe("A Chat Status Notification", function () { describe("A Chat Status Notification", function () {
it("does not open automatically if a chat state notification is received", it("does not open a new chatbox",
mock.initConverseWithPromises( mock.initConverseWithPromises(
null, ['rosterGroupsFetched'], {}, null, ['rosterGroupsFetched'], {},
function (done, _converse) { function (done, _converse) {
test_utils.createContacts(_converse, 'current'); test_utils.createContacts(_converse, 'current');
test_utils.openControlBox(); test_utils.openControlBox();
...@@ -1659,9 +1659,9 @@ ...@@ -1659,9 +1659,9 @@
describe("An active notification", function () { describe("An active notification", function () {
it("is sent when the user opens a chat box", it("is sent when the user opens a chat box",
mock.initConverseWithPromises( mock.initConverseWithPromises(
null, ['rosterGroupsFetched'], {}, null, ['rosterGroupsFetched'], {},
function (done, _converse) { function (done, _converse) {
test_utils.createContacts(_converse, 'current'); test_utils.createContacts(_converse, 'current');
test_utils.openControlBox(); test_utils.openControlBox();
...@@ -1787,8 +1787,21 @@ ...@@ -1787,8 +1787,21 @@
var chatboxview = _converse.chatboxviews.get(sender_jid); var chatboxview = _converse.chatboxviews.get(sender_jid);
expect(chatboxview).toBeDefined(); expect(chatboxview).toBeDefined();
// Check that the notification appears inside the chatbox in the DOM // Check that the notification appears inside the chatbox in the DOM
var $events = $(chatboxview.el).find('.chat-event'); var events = chatboxview.el.querySelectorAll('.chat-state-notification');
expect($events.text()).toEqual(mock.cur_names[1] + ' is typing'); expect(events.length).toBe(1);
expect(events[0].textContent).toEqual(mock.cur_names[1] + ' is typing');
// Check that it doesn't appear twice
msg = $msg({
from: sender_jid,
to: _converse.connection.jid,
type: 'chat',
id: (new Date()).getTime()
}).c('body').c('composing', {'xmlns': Strophe.NS.CHATSTATES}).tree();
_converse.chatboxes.onMessage(msg);
events = chatboxview.el.querySelectorAll('.chat-state-notification');
expect(events.length).toBe(1);
expect(events[0].textContent).toEqual(mock.cur_names[1] + ' is typing');
done(); done();
})); }));
...@@ -1835,7 +1848,7 @@ ...@@ -1835,7 +1848,7 @@
expect(msg_obj.get('sender')).toEqual('me'); expect(msg_obj.get('sender')).toEqual('me');
expect(msg_obj.get('delayed')).toEqual(false); expect(msg_obj.get('delayed')).toEqual(false);
var $chat_content = $(chatboxview.el).find('.chat-content'); var $chat_content = $(chatboxview.el).find('.chat-content');
var status_text = $chat_content.find('.chat-info.chat-event').text(); var status_text = $chat_content.find('.chat-info.chat-state-notification').text();
expect(status_text).toBe('Typing from another device'); expect(status_text).toBe('Typing from another device');
done(); done();
}); });
...@@ -1845,9 +1858,9 @@ ...@@ -1845,9 +1858,9 @@
describe("A paused notification", function () { describe("A paused notification", function () {
it("is sent if the user has stopped typing since 30 seconds", it("is sent if the user has stopped typing since 30 seconds",
mock.initConverseWithPromises( mock.initConverseWithPromises(
null, ['rosterGroupsFetched'], {}, null, ['rosterGroupsFetched'], {},
function (done, _converse) { function (done, _converse) {
var view, contact_jid; var view, contact_jid;
test_utils.createContacts(_converse, 'current'); test_utils.createContacts(_converse, 'current');
...@@ -1931,7 +1944,7 @@ ...@@ -1931,7 +1944,7 @@
_converse.chatboxes.onMessage(msg); _converse.chatboxes.onMessage(msg);
expect(_converse.emit).toHaveBeenCalledWith('message', jasmine.any(Object)); expect(_converse.emit).toHaveBeenCalledWith('message', jasmine.any(Object));
var chatboxview = _converse.chatboxviews.get(sender_jid); var chatboxview = _converse.chatboxviews.get(sender_jid);
var $events = $(chatboxview.el).find('.chat-event'); var $events = $(chatboxview.el).find('.chat-info.chat-state-notification');
expect($events.text()).toEqual(mock.cur_names[1] + ' has stopped typing'); expect($events.text()).toEqual(mock.cur_names[1] + ' has stopped typing');
done(); done();
}); });
...@@ -1980,7 +1993,7 @@ ...@@ -1980,7 +1993,7 @@
expect(msg_obj.get('sender')).toEqual('me'); expect(msg_obj.get('sender')).toEqual('me');
expect(msg_obj.get('delayed')).toEqual(false); expect(msg_obj.get('delayed')).toEqual(false);
var $chat_content = $(chatboxview.el).find('.chat-content'); var $chat_content = $(chatboxview.el).find('.chat-content');
var status_text = $chat_content.find('.chat-info.chat-event').text(); var status_text = $chat_content.find('.chat-info.chat-state-notification').text();
expect(status_text).toBe('Stopped typing on the other device'); expect(status_text).toBe('Stopped typing on the other device');
done(); done();
}); });
...@@ -2106,7 +2119,7 @@ ...@@ -2106,7 +2119,7 @@
}); });
})); }));
it("will clear any other chat status notifications if its received", it("will clear any other chat status notifications",
mock.initConverseWithPromises( mock.initConverseWithPromises(
null, ['rosterGroupsFetched'], {}, null, ['rosterGroupsFetched'], {},
function (done, _converse) { function (done, _converse) {
...@@ -2119,10 +2132,20 @@ ...@@ -2119,10 +2132,20 @@
var sender_jid = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@localhost'; var sender_jid = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@localhost';
test_utils.openChatBoxFor(_converse, sender_jid); test_utils.openChatBoxFor(_converse, sender_jid);
var view = _converse.chatboxviews.get(sender_jid); var view = _converse.chatboxviews.get(sender_jid);
expect($(view.el).find('.chat-event').length).toBe(0); expect(view.el.querySelectorAll('.chat-event').length).toBe(0);
view.showStatusNotification(sender_jid+' is typing'); // Insert <composing> message, to also check that
expect($(view.el).find('.chat-event').length).toBe(1); // text messages are inserted correctly with
// temporary chat events in the chat contents.
var msg = $msg({ var msg = $msg({
'to': _converse.bare_jid,
'xmlns': 'jabber:client',
'from': sender_jid,
'type': 'chat'})
.c('composing', {'xmlns': Strophe.NS.CHATSTATES}).up()
.tree();
_converse.chatboxes.onMessage(msg);
expect(view.el.querySelectorAll('.chat-state-notification').length).toBe(1);
msg = $msg({
from: sender_jid, from: sender_jid,
to: _converse.connection.jid, to: _converse.connection.jid,
type: 'chat', type: 'chat',
...@@ -2130,7 +2153,7 @@ ...@@ -2130,7 +2153,7 @@
}).c('body').c('inactive', {'xmlns': Strophe.NS.CHATSTATES}).tree(); }).c('body').c('inactive', {'xmlns': Strophe.NS.CHATSTATES}).tree();
_converse.chatboxes.onMessage(msg); _converse.chatboxes.onMessage(msg);
expect(_converse.emit).toHaveBeenCalledWith('message', jasmine.any(Object)); expect(_converse.emit).toHaveBeenCalledWith('message', jasmine.any(Object));
expect($(view.el).find('.chat-event').length).toBe(0); expect($(view.el).find('.chat-state-notification').length).toBe(0);
done(); done();
})); }));
}); });
...@@ -2157,7 +2180,7 @@ ...@@ -2157,7 +2180,7 @@
_converse.chatboxes.onMessage(msg); _converse.chatboxes.onMessage(msg);
expect(_converse.emit).toHaveBeenCalledWith('message', jasmine.any(Object)); expect(_converse.emit).toHaveBeenCalledWith('message', jasmine.any(Object));
var chatboxview = _converse.chatboxviews.get(sender_jid); var chatboxview = _converse.chatboxviews.get(sender_jid);
var $events = $(chatboxview.el).find('.chat-event'); var $events = $(chatboxview.el).find('.chat-state-notification');
expect($events.text()).toEqual(mock.cur_names[1] + ' has gone away'); expect($events.text()).toEqual(mock.cur_names[1] + ' has gone away');
done(); done();
})); }));
......
This diff is collapsed.
...@@ -167,6 +167,7 @@ ...@@ -167,6 +167,7 @@
const spoiler = message.querySelector(`spoiler[xmlns="${Strophe.NS.SPOILER}"]`); const spoiler = message.querySelector(`spoiler[xmlns="${Strophe.NS.SPOILER}"]`);
const attrs = { const attrs = {
'type': type, 'type': type,
'from': from,
'chat_state': chat_state, 'chat_state': chat_state,
'delayed': delayed, 'delayed': delayed,
'fullname': fullname, 'fullname': fullname,
......
This diff is collapsed.
...@@ -497,25 +497,16 @@ ...@@ -497,25 +497,16 @@
informOfOccupantsRoleChange (occupant, changed) { informOfOccupantsRoleChange (occupant, changed) {
const previous_role = occupant._previousAttributes.role; const previous_role = occupant._previousAttributes.role;
if (previous_role === 'moderator') { if (previous_role === 'moderator') {
this.showStatusNotification( this.showChatEvent(__("%1$s is no longer a moderator", occupant.get('nick')))
__("%1$s is no longer a moderator.", occupant.get('nick')),
false, true)
} }
if (previous_role === 'visitor') { if (previous_role === 'visitor') {
this.showStatusNotification( this.showChatEvent(__("%1$s has been given a voice again", occupant.get('nick')))
__("%1$s has been given a voice again.", occupant.get('nick')),
false, true)
} }
if (occupant.get('role') === 'visitor') { if (occupant.get('role') === 'visitor') {
this.showStatusNotification( this.showChatEvent(__("%1$s has been muted", occupant.get('nick')))
__("%1$s has been muted.", occupant.get('nick')),
false, true)
} }
if (occupant.get('role') === 'moderator') { if (occupant.get('role') === 'moderator') {
this.showStatusNotification( this.showChatEvent(__("%1$s is now a moderator", occupant.get('nick')))
__("%1$s is now a moderator.", occupant.get('nick')),
false, true)
} }
}, },
...@@ -629,7 +620,7 @@ ...@@ -629,7 +620,7 @@
this.insertIntoTextArea(ev.target.textContent); this.insertIntoTextArea(ev.target.textContent);
}, },
handleChatStateMessage (message) { handleChatStateNotification (message) {
/* Override the method on the ChatBoxView base class to /* Override the method on the ChatBoxView base class to
* ignore <gone/> notifications in groupchats. * ignore <gone/> notifications in groupchats.
* *
...@@ -643,7 +634,7 @@ ...@@ -643,7 +634,7 @@
return; return;
} }
if (message.get('chat_state') !== _converse.GONE) { if (message.get('chat_state') !== _converse.GONE) {
_converse.ChatBoxView.prototype.handleChatStateMessage.apply(this, arguments); _converse.ChatBoxView.prototype.handleChatStateNotification.apply(this, arguments);
} }
}, },
...@@ -707,7 +698,7 @@ ...@@ -707,7 +698,7 @@
*/ */
// TODO check if first argument is valid // TODO check if first argument is valid
if (args.length < 1 || args.length > 2) { if (args.length < 1 || args.length > 2) {
this.showStatusNotification( this.showErrorMessage(
__('Error: the "%1$s" command takes two arguments, the user\'s nickname and optionally a reason.', __('Error: the "%1$s" command takes two arguments, the user\'s nickname and optionally a reason.',
command), command),
true true
...@@ -729,7 +720,7 @@ ...@@ -729,7 +720,7 @@
}, },
onCommandError () { onCommandError () {
this.showStatusNotification(__("Error: could not execute the command"), true); this.showErrorMessage(__("Error: could not execute the command"), true);
}, },
onMessageSubmitted (text) { onMessageSubmitted (text) {
...@@ -1446,7 +1437,7 @@ ...@@ -1446,7 +1437,7 @@
})); }));
}); });
if (notification.reason) { if (notification.reason) {
this.showStatusNotification(__('The reason given is: "%1$s".', notification.reason), true); this.showChatEvent(__('The reason given is: "%1$s".', notification.reason));
} }
if (_.get(notification.messages, 'length')) { if (_.get(notification.messages, 'length')) {
this.scrollDown(); this.scrollDown();
...@@ -1577,7 +1568,7 @@ ...@@ -1577,7 +1568,7 @@
return stanza; return stanza;
}, },
showErrorMessage (presence) { showErrorMessageFromPresence (presence) {
// We didn't enter the room, so we must remove it from the MUC add-on // We didn't enter the room, so we must remove it from the MUC add-on
const error = presence.querySelector('error'); const error = presence.querySelector('error');
if (error.getAttribute('type') === 'auth') { if (error.getAttribute('type') === 'auth') {
...@@ -1698,7 +1689,7 @@ ...@@ -1698,7 +1689,7 @@
*/ */
if (pres.getAttribute('type') === 'error') { if (pres.getAttribute('type') === 'error') {
this.model.save('connection_status', converse.ROOMSTATUS.DISCONNECTED); this.model.save('connection_status', converse.ROOMSTATUS.DISCONNECTED);
this.showErrorMessage(pres); this.showErrorMessageFromPresence(pres);
return true; return true;
} }
const is_self = pres.querySelector("status[code='110']"); const is_self = pres.querySelector("status[code='110']");
......
<div class="message chat-info chat-state-notification"
data-isodate="{{{o.isodate}}}"
data-csn="{{{o.from}}}">{{{o.message}}}</div>
<div class="message chat-info chat-error" data-isodate="{{{o.isodate}}}">{{{o.message}}}</div>
<div class="message chat-info {{{o.extra_classes}}}" data-isodate="{{{o.isodate}}}" {{{o.data}}}>{{{o.message}}}</div> <div class="message chat-info {{{o.extra_classes}}}"
data-isodate="{{{o.isodate}}}"
{{{o.data}}}>{{{o.message}}}</div>
<div class="message chat-info chat-status"
data-isodate="{{{o.isodate}}}"
data-status="{{{o.from}}}">{{{o.message}}}</div>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment