Commit f8efd942 authored by JC Brand's avatar JC Brand

Don't smooth-scroll upon first appearance of the chat

it's annoying in overlayed view-mode where the user might be navigating
around the site.

Fixes #2091
parent 0c5f0e24
...@@ -100,10 +100,6 @@ body.converse-fullscreen { ...@@ -100,10 +100,6 @@ body.converse-fullscreen {
background-color: #EEE !important; background-color: #EEE !important;
} }
.smooth-scroll {
scroll-behavior: smooth !important;
}
.nopadding { .nopadding {
padding: 0 !important; padding: 0 !important;
} }
......
...@@ -1322,7 +1322,7 @@ describe("Chatboxes", function () { ...@@ -1322,7 +1322,7 @@ describe("Chatboxes", function () {
['rosterGroupsFetched', 'chatBoxesFetched'], {}, ['rosterGroupsFetched', 'chatBoxesFetched'], {},
async function (done, _converse) { async function (done, _converse) {
await mock.waitForRoster(_converse, 'current'); await mock.waitForRoster(_converse, 'current', 1);
const sender_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit', const sender_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit',
msg = mock.createChatMessage(_converse, sender_jid, 'This message will be unread'); msg = mock.createChatMessage(_converse, sender_jid, 'This message will be unread');
...@@ -1347,10 +1347,9 @@ describe("Chatboxes", function () { ...@@ -1347,10 +1347,9 @@ describe("Chatboxes", function () {
['rosterGroupsFetched', 'chatBoxesFetched'], {}, ['rosterGroupsFetched', 'chatBoxesFetched'], {},
async function (done, _converse) { async function (done, _converse) {
await mock.waitForRoster(_converse, 'current'); await mock.waitForRoster(_converse, 'current', 1);
const sender_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit';
const sender_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit', const msg = mock.createChatMessage(_converse, sender_jid, 'This message will be read');
msg = mock.createChatMessage(_converse, sender_jid, 'This message will be read');
const sent_stanzas = []; const sent_stanzas = [];
spyOn(_converse.connection, 'send').and.callFake(s => sent_stanzas.push(s)); spyOn(_converse.connection, 'send').and.callFake(s => sent_stanzas.push(s));
await mock.openChatBoxFor(_converse, sender_jid); await mock.openChatBoxFor(_converse, sender_jid);
...@@ -1470,7 +1469,7 @@ describe("Chatboxes", function () { ...@@ -1470,7 +1469,7 @@ describe("Chatboxes", function () {
await u.waitUntil(() => chatbox.sendMarker.calls.count() === 1); await u.waitUntil(() => chatbox.sendMarker.calls.count() === 1);
expect(sent_stanzas[0].nodeTree.querySelector('received')).toBeDefined(); expect(sent_stanzas[0].nodeTree.querySelector('received')).toBeDefined();
_converse.saveWindowState(null, 'focus'); _converse.saveWindowState(null, 'focus');
expect(chatbox.get('num_unread')).toBe(1); await u.waitUntil(() => chatbox.get('num_unread') === 1);
expect(chatbox.get('first_unread_id')).toBe(msgid); expect(chatbox.get('first_unread_id')).toBe(msgid);
await u.waitUntil(() => chatbox.sendMarker.calls.count() === 1); await u.waitUntil(() => chatbox.sendMarker.calls.count() === 1);
expect(sent_stanzas[0].nodeTree.querySelector('received')).toBeDefined(); expect(sent_stanzas[0].nodeTree.querySelector('received')).toBeDefined();
......
...@@ -1743,6 +1743,9 @@ describe("A Chat Message", function () { ...@@ -1743,6 +1743,9 @@ describe("A Chat Message", function () {
const view = _converse.api.chatviews.get(sender_jid); const view = _converse.api.chatviews.get(sender_jid);
// Create enough messages so that there's a scrollbar. // Create enough messages so that there's a scrollbar.
const promises = []; const promises = [];
view.content.scrollTop = 0;
view.model.set('scrolled', true);
for (let i=0; i<20; i++) { for (let i=0; i<20; i++) {
_converse.handleMessageStanza($msg({ _converse.handleMessageStanza($msg({
from: sender_jid, from: sender_jid,
...@@ -1754,35 +1757,15 @@ describe("A Chat Message", function () { ...@@ -1754,35 +1757,15 @@ describe("A Chat Message", function () {
promises.push(new Promise(resolve => view.model.messages.once('rendered', resolve))); promises.push(new Promise(resolve => view.model.messages.once('rendered', resolve)));
} }
await Promise.all(promises); await Promise.all(promises);
// XXX Fails on Travis
// await u.waitUntil(() => view.content.scrollTop, 1000)
await u.waitUntil(() => !view.model.get('auto_scrolled'), 500);
view.content.scrollTop = 0;
// XXX Fails on Travis
// await u.waitUntil(() => view.model.get('scrolled'), 900);
view.model.set('scrolled', true);
const message = 'This message is received while the chat area is scrolled up'; const indicator_el = view.el.querySelector('.new-msgs-indicator');
_converse.handleMessageStanza($msg({ expect(u.isVisible(indicator_el)).toBeTruthy();
from: sender_jid,
to: _converse.connection.jid,
type: 'chat',
id: u.getUniqueId()
}).c('body').t(message).up()
.c('active', {'xmlns': 'http://jabber.org/protocol/chatstates'}).tree());
await new Promise(resolve => view.model.messages.once('rendered', resolve));
await u.waitUntil(() => view.model.messages.length > 20, 1000);
// Now check that the message appears inside the chatbox in the DOM
const msg_txt = sizzle('.chat-content .chat-msg:last .chat-msg__text', view.el).pop().textContent;
expect(msg_txt).toEqual(message);
await u.waitUntil(() => u.isVisible(view.el.querySelector('.new-msgs-indicator')), 900);
expect(view.model.get('scrolled')).toBe(true); expect(view.model.get('scrolled')).toBe(true);
expect(view.content.scrollTop).toBe(0); expect(view.content.scrollTop).toBe(0);
expect(u.isVisible(view.el.querySelector('.new-msgs-indicator'))).toBeTruthy(); indicator_el.click();
// Scroll down again expect(u.isVisible(indicator_el)).toBeFalsy();
view.content.scrollTop = view.content.scrollHeight; expect(view.model.get('scrolled')).toBe(false);
// XXX Fails on Travis
// await u.waitUntil(() => !u.isVisible(view.el.querySelector('.new-msgs-indicator')), 900);
done(); done();
})); }));
......
...@@ -25,16 +25,6 @@ class ChatContent extends CustomElement { ...@@ -25,16 +25,6 @@ class ChatContent extends CustomElement {
<div class="chat-content__notifications">${unsafeHTML(notifications)}</div> <div class="chat-content__notifications">${unsafeHTML(notifications)}</div>
`; `;
} }
scrollDown () {
if (!this.chatview.model.get('scrolled')) {
this.parentElement.scrollTop = this.parentElement.scrollHeight;
}
}
updated () {
this.scrollDown();
}
} }
customElements.define('converse-chat-content', ChatContent); customElements.define('converse-chat-content', ChatContent);
...@@ -211,6 +211,7 @@ converse.plugins.add('converse-chatview', { ...@@ -211,6 +211,7 @@ converse.plugins.add('converse-chatview', {
// Need to be registered after render has been called. // Need to be registered after render has been called.
this.listenTo(this.model.messages, 'add', this.onMessageAdded); this.listenTo(this.model.messages, 'add', this.onMessageAdded);
this.listenTo(this.model.messages, 'change', this.renderChatHistory); this.listenTo(this.model.messages, 'change', this.renderChatHistory);
this.listenTo(this.model.messages, 'rendered', this.maybeScrollDownOnMessage);
this.listenTo(this.model.messages, 'reset', this.renderChatHistory); this.listenTo(this.model.messages, 'reset', this.renderChatHistory);
this.listenTo(this.model.notifications, 'change', this.renderNotifications); this.listenTo(this.model.notifications, 'change', this.renderNotifications);
this.listenTo(this.model, 'change:show_help_messages', this.renderHelpMessages); this.listenTo(this.model, 'change:show_help_messages', this.renderHelpMessages);
...@@ -228,6 +229,8 @@ converse.plugins.add('converse-chatview', { ...@@ -228,6 +229,8 @@ converse.plugins.add('converse-chatview', {
initDebounced () { initDebounced () {
this.markScrolled = debounce(this._markScrolled, 100); this.markScrolled = debounce(this._markScrolled, 100);
this.debouncedScrollDown = debounce(this.scrollDown, 100);
// For tests that use Jasmine.Clock we want to turn of // For tests that use Jasmine.Clock we want to turn of
// debouncing, since setTimeout breaks. // debouncing, since setTimeout breaks.
if (api.settings.get('debounced_content_rendering')) { if (api.settings.get('debounced_content_rendering')) {
...@@ -552,9 +555,28 @@ converse.plugins.add('converse-chatview', { ...@@ -552,9 +555,28 @@ converse.plugins.add('converse-chatview', {
api.trigger('afterMessagesFetched', this.model); api.trigger('afterMessagesFetched', this.model);
}, },
scrollDown () { maybeScrollDownOnMessage (message) {
const el = this.msgs_container.firstElementChild; if (message.get('sender') !== 'me' && !this.model.get('scrolled')) {
el && el.scrollDown(); this.debouncedScrollDown();
}
},
scrollDown (ev) {
ev?.preventDefault?.();
ev?.stopPropagation?.();
if (this.model.get('scrolled')) {
u.safeSave(this.model, {
'scrolled': false,
'top_visible_message': null,
});
}
if (this.msgs_container.scrollTo) {
const behavior = this.msgs_container.scrollTop ? 'smooth' : 'auto';
this.msgs_container.scrollTo({'top': this.msgs_container.scrollHeight, behavior});
} else {
this.msgs_container.scrollTop = this.msgs_container.scrollHeight;
}
this.onScrolledDown();
}, },
insertIntoDOM () { insertIntoDOM () {
...@@ -597,7 +619,6 @@ converse.plugins.add('converse-chatview', { ...@@ -597,7 +619,6 @@ converse.plugins.add('converse-chatview', {
// should maintain our current scroll position. // should maintain our current scroll position.
if (this.content.scrollTop === 0 || this.model.get('top_visible_message')) { if (this.content.scrollTop === 0 || this.model.get('top_visible_message')) {
const top_visible_message = this.model.get('top_visible_message') || next_msg_el; const top_visible_message = this.model.get('top_visible_message') || next_msg_el;
this.model.set('top_visible_message', top_visible_message); this.model.set('top_visible_message', top_visible_message);
this.content.scrollTop = top_visible_message.offsetTop - 30; this.content.scrollTop = top_visible_message.offsetTop - 30;
} }
......
...@@ -466,7 +466,7 @@ converse.plugins.add('converse-muc-views', { ...@@ -466,7 +466,7 @@ converse.plugins.add('converse-muc-views', {
this.listenTo(this.model, 'show', this.show); this.listenTo(this.model, 'show', this.show);
this.listenTo(this.model.features, 'change:moderated', this.renderBottomPanel); this.listenTo(this.model.features, 'change:moderated', this.renderBottomPanel);
this.listenTo(this.model.features, 'change:open', this.renderHeading); this.listenTo(this.model.features, 'change:open', this.renderHeading);
this.listenTo(this.model.messages, 'rendered', this.scrollDown); this.listenTo(this.model.messages, 'rendered', this.maybeScrollDownOnMessage);
this.listenTo(this.model.session, 'change:connection_status', this.onConnectionStatusChanged); this.listenTo(this.model.session, 'change:connection_status', this.onConnectionStatusChanged);
// Bind so that we can pass it to addEventListener and removeEventListener // Bind so that we can pass it to addEventListener and removeEventListener
......
...@@ -5,7 +5,7 @@ export default (o) => html` ...@@ -5,7 +5,7 @@ export default (o) => html`
<div class="chat-head chat-head-chatbox row no-gutters"></div> <div class="chat-head chat-head-chatbox row no-gutters"></div>
<div class="chat-body"> <div class="chat-body">
<div class="chat-content ${ o.show_send_button ? 'chat-content-sendbutton' : '' }" aria-live="polite"> <div class="chat-content ${ o.show_send_button ? 'chat-content-sendbutton' : '' }" aria-live="polite">
<div class="chat-content__messages smooth-scroll" @scroll=${o.markScrolled}></div> <div class="chat-content__messages" @scroll=${o.markScrolled}></div>
<div class="chat-content__help"></div> <div class="chat-content__help"></div>
</div> </div>
<div class="bottom-panel"> <div class="bottom-panel">
......
...@@ -6,7 +6,7 @@ export default (o) => html` ...@@ -6,7 +6,7 @@ export default (o) => html`
<div class="chat-body chatroom-body row no-gutters"> <div class="chat-body chatroom-body row no-gutters">
<div class="chat-area col"> <div class="chat-area col">
<div class="chat-content ${ o.show_send_button ? 'chat-content-sendbutton' : '' }" aria-live="polite"> <div class="chat-content ${ o.show_send_button ? 'chat-content-sendbutton' : '' }" aria-live="polite">
<div class="chat-content__messages smooth-scroll" @scroll=${o.markScrolled}></div> <div class="chat-content__messages" @scroll=${o.markScrolled}></div>
<div class="chat-content__help"></div> <div class="chat-content__help"></div>
</div> </div>
<div class="bottom-panel"></div> <div class="bottom-panel"></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