pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] mail: mark message as seen only when thread composer is focused

Browse Source 
task-2272510

closes odoo/odoo#53088

X-original-commit: edf356c2022b3039e53e82a092fc64533f59df27
Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
This commit is contained in:
Xavier Dubuc
2020-06-16 15:52:34 +00:00
parent 57a32316af
commit 4ca549253b
11 changed files with 228 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mail/static/src/components/composer/composer.js
-24
View File
@@ -48,35 +48,13 @@ class Composer extends Component {
* Reference of the text input component.
*/
this._textInputRef = useRef('textInput');
/**
* Tracked focus counter from props. Useful to determine whether it
* should auto focus this composer when patched.
*/
this._focusCount = 0;
// to focus if the prop changes
this._lastComposer = undefined;
this._onClickCaptureGlobal = this._onClickCaptureGlobal.bind(this);
}
mounted() {
this._lastComposer = this.composer;
if (this.props.isFocusOnMount) {
this.focus();
}
document.addEventListener('click', this._onClickCaptureGlobal, true);
}
patched() {
// focus when changing composer
if (
this.props.isFocusOnMount &&
this._lastComposer !== this.composer
) {
this.focus();
}
this._lastComposer = this.composer;
}
willUnmount() {
document.removeEventListener('click', this._onClickCaptureGlobal, true);
}
@@ -338,7 +316,6 @@ Object.assign(Composer, {
isCompact: true,
isDiscardOnClickAway: false,
isExpandable: false,
isFocusOnMount: false,
isLog: false,
},
props: {
@@ -382,7 +359,6 @@ Object.assign(Composer, {
isCompact: Boolean,
isDiscardOnClickAway: Boolean,
isExpandable: Boolean,
isFocusOnMount: Boolean,
isLog: Boolean,
},
template: 'mail.Composer',
addons/mail/static/src/components/composer_text_input/composer_text_input.js
+14
View File
@@ -158,6 +158,20 @@ class ComposerTextInput extends Component {
// Handlers
//--------------------------------------------------------------------------
/**
* @private
*/
_onFocusinTextarea() {
this.composer.update({ hasFocus: true });
}
/**
* @private
*/
_onFocusoutTextarea() {
this.composer.update({ hasFocus: false });
}
/**
* @private
*/
addons/mail/static/src/components/composer_text_input/composer_text_input.xml
+1 -1
View File
@@ -36,7 +36,7 @@
</div>
</div>
</t>
<textarea class="o_ComposerTextInput_textarea" t-esc="composer.textInputContent" placeholder="Write something..." t-on-keydown="_onKeydownTextarea" t-on-keyup="_onKeyupTextarea" t-on-input="_onInputTextarea" t-ref="textarea"/>
<textarea class="o_ComposerTextInput_textarea" t-esc="composer.textInputContent" placeholder="Write something..." t-on-focusin="_onFocusinTextarea" t-on-focusout="_onFocusoutTextarea" t-on-keydown="_onKeydownTextarea" t-on-keyup="_onKeyupTextarea" t-on-input="_onInputTextarea" t-ref="textarea"/>
</t>
</div>
</t>
addons/mail/static/src/components/discuss/discuss.xml
-1
View File
@@ -97,7 +97,6 @@
hasDiscardButton="true"
hasThreadName="true"
isDiscardOnClickAway="true"
isFocusOnMount="true"
t-on-o-message-posted="_onReplyingToMessageMessagePosted"
/>
</t>
addons/mail/static/src/components/discuss/discuss_tests.js
+44 -41
View File
@@ -1951,7 +1951,7 @@ QUnit.test('new messages separator', async function (assert) {
// visible. This is necessary in order to display 'new messages' and not
// remove from DOM right away from seeing last message.
// AKU TODO: thread specific test
assert.expect(5);
assert.expect(6);
let step = 0;
Object.assign(this.data.initMessaging, {
@@ -2000,26 +2000,26 @@ QUnit.test('new messages separator', async function (assert) {
return this._super(...arguments);
},
});
assert.strictEqual(
document.querySelectorAll(`
.o_Discuss_thread .o_ThreadViewer_messageList .o_MessageList_message
`).length,
assert.containsN(
document.body,
'.o_MessageList_message',
25,
"should have 25 messages"
);
assert.strictEqual(
document.querySelectorAll(`
.o_Discuss_thread .o_ThreadViewer_messageList .o_MessageList_separatorNewMessages
`).length,
0,
assert.containsNone(
document.body,
'.o_MessageList_separatorNewMessages',
"should not display 'new messages' separator"
);
// scroll to top
document.querySelector(`.o_Discuss_thread .o_ThreadViewer_messageList`).scrollTop = 0;
// composer is focused by default, we remove that focus
document.querySelector('.o_ComposerTextInput_textarea').blur();
// simulate receiving a new message
const data = {
author_id: [36, "User26"],
body: "<p>boddy26</p>",
body: "<p>body26</p>",
channel_ids: [20],
date: "2019-04-20 10:00:00",
id: 126,
@@ -2030,33 +2030,36 @@ QUnit.test('new messages separator', async function (assert) {
};
await afterNextRender(() => {
const notifications = [[['my-db', 'mail.channel', 20], data]];
this.widget.call('bus_service', 'trigger', 'notification', notifications);
this.widget.call('bus_service', 'trigger', 'notification', notifications)
});
assert.strictEqual(
document.querySelectorAll(`
.o_Discuss_thread .o_ThreadViewer_messageList .o_MessageList_message
`).length,
assert.containsN(
document.body,
'.o_MessageList_message',
26,
"should have 26 messages"
);
assert.strictEqual(
document.querySelectorAll(`
.o_Discuss_thread .o_ThreadViewer_messageList .o_MessageList_separatorNewMessages
`).length,
1,
assert.containsOnce(
document.body,
'.o_MessageList_separatorNewMessages',
"should display 'new messages' separator"
);
// scroll to bottom
await afterNextRender(() => {
document.querySelector(`.o_Discuss_thread .o_ThreadViewer_messageList`).scrollTop =
document.querySelector(`.o_Discuss_thread .o_ThreadViewer_messageList`).scrollHeight;
});
assert.strictEqual(
document.querySelectorAll(`
.o_Discuss_thread .o_ThreadViewer_messageList .o_MessageList_separatorNewMessages
`).length,
0,
assert.containsOnce(
document.body,
'.o_MessageList_separatorNewMessages',
"should still display 'new messages' separator as composer is not focused"
);
await afterNextRender(() =>
document.querySelector('.o_ComposerTextInput_textarea').focus()
);
assert.containsNone(
document.body,
'.o_MessageList_separatorNewMessages',
"should no longer display 'new messages' separator (message seen)"
);
});
@@ -3491,19 +3494,17 @@ QUnit.test('mark channel as seen on last message visible', async function (asser
return this._super(...arguments);
},
});
assert.strictEqual(
document.querySelectorAll(`
.o_DiscussSidebar_item[data-thread-local-id="${
this.env.models['mail.thread'].find(thread =>
thread.id === 10 &&
thread.model === 'mail.channel'
).localId
}"]
`).length,
1,
assert.containsOnce(
document.body,
`.o_DiscussSidebar_item[data-thread-local-id="${
this.env.models['mail.thread'].find(thread =>
thread.id === 10 &&
thread.model === 'mail.channel'
).localId
}"]`,
"should have discuss sidebar item with the channel"
);
assert.ok(
assert.hasClass(
document.querySelector(`
.o_DiscussSidebar_item[data-thread-local-id="${
this.env.models['mail.thread'].find(thread =>
@@ -3511,7 +3512,8 @@ QUnit.test('mark channel as seen on last message visible', async function (asser
thread.model === 'mail.channel'
).localId
}"]
`).classList.contains('o-unread'),
`),
'o-unread',
"sidebar item of channel ID 10 should be unread"
);
@@ -3525,7 +3527,7 @@ QUnit.test('mark channel as seen on last message visible', async function (asser
}"]
`).click()
);
assert.notOk(
assert.doesNotHaveClass(
document.querySelector(`
.o_DiscussSidebar_item[data-thread-local-id="${
this.env.models['mail.thread'].find(thread =>
@@ -3533,7 +3535,8 @@ QUnit.test('mark channel as seen on last message visible', async function (asser
thread.model === 'mail.channel'
).localId
}"]
`).classList.contains('o-unread'),
`),
'o-unread',
"sidebar item of channel ID 10 should not longer be unread"
);
});
addons/mail/static/src/components/message_list/message_list.js
+10 -16
View File
@@ -374,23 +374,17 @@ class MessageList extends Component {
/**
* @private
*/
_checkThreadMarkAsRead() {
_checkMostRecentMessageIsVisible() {
const thread = this.threadViewer.thread;
const threadCache = this.threadViewer.threadCache;
if (!threadCache) {
return;
}
if (threadCache.messages.length === 0) {
return;
}
if (!this.mostRecentMessageRef) {
return;
}
if (
const lastMessageIsVisible =
threadCache &&
threadCache.messages.length > 0 &&
this.mostRecentMessageRef &&
threadCache === thread.mainCache &&
this.mostRecentMessageRef.isPartiallyVisible()
) {
thread.markAsSeen();
this.mostRecentMessageRef.isPartiallyVisible();
if (lastMessageIsVisible) {
this.threadViewer.handleVisibleMessage(this.mostRecentMessageRef.message);
}
}
@@ -460,7 +454,7 @@ class MessageList extends Component {
* @private
*/
_update() {
this._checkThreadMarkAsRead();
this._checkMostRecentMessageIsVisible();
}
//--------------------------------------------------------------------------
@@ -492,7 +486,7 @@ class MessageList extends Component {
if (this._isLoadMoreVisible()) {
this._loadMore();
}
this._checkThreadMarkAsRead();
this._checkMostRecentMessageIsVisible();
}
}
addons/mail/static/src/components/thread_viewer/thread_viewer.xml
-1
View File
@@ -31,7 +31,6 @@
hasCurrentPartnerAvatar="props.hasComposerCurrentPartnerAvatar"
hasSendButton="props.hasComposerSendButton"
hasThreadTyping="props.hasComposerThreadTyping"
isFocusOnMount="!env.messaging.device.isMobile"
showAttachmentsExtensions="props.showComposerAttachmentsExtensions"
showAttachmentsFilenames="props.showComposerAttachmentsFilenames"
t-ref="composer"
addons/mail/static/src/components/thread_viewer/thread_viewer_tests.js
+84 -11
View File
@@ -286,8 +286,8 @@ QUnit.test('message list asc order', async function (assert) {
assert.verifySteps(['message_fetch_1', 'message_fetch_2']);
});
QUnit.test('mark channel as fetched when a new message is loaded and as seen when message is visible', async function (assert) {
assert.expect(7);
QUnit.test('mark channel as fetched when a new message is loaded and as seen when focusing composer', async function (assert) {
assert.expect(8);
await this.start({
mockRPC(route, args) {
@@ -302,7 +302,7 @@ QUnit.test('mark channel as fetched when a new message is loaded and as seen whe
'mail.channel',
'channel_fetched is called on the right channel model'
);
assert.step('rpc_channel_fetched:called');
assert.step('rpc:channel_fetch');
} else if (args.method === 'channel_seen') {
assert.strictEqual(
args.args[0][0],
@@ -314,7 +314,79 @@ QUnit.test('mark channel as fetched when a new message is loaded and as seen whe
'mail.channel',
'channel_seeb is called on the right channel model'
);
assert.step('rpc_channel_seen:called');
assert.step('rpc:channel_seen');
}
return this._super(...arguments);
}
});
const thread = this.env.models['mail.thread'].create({
// FIXME should not be needed, see task-2277537
composer: [['create']], // avoid initial focus
id: 100,
isPinned: true, // just to avoid joinChannel to be called
members: [['insert', [
{
email: "john@example.com",
id: this.env.session.partner_id,
name: "John",
},
{
email: "fred@example.com",
id: 10,
name: "Fred",
},
]]],
message_unread_counter: 1, // seen would not be called if not > 0
model: 'mail.channel',
});
const threadViewer = this.env.models['mail.thread_viewer'].create({ thread: [['link', thread]] });
await this.createThreadViewerComponent(threadViewer, { hasComposer: true });
const notifications = [
[['myDB', 'mail.channel', 100], {
channelId: 100,
id: 1,
body: "<p>fdsfsd</p>",
author_id: [10, "Fred"],
model: "mail.channel",
channel_ids: [100],
}]
];
await afterNextRender(() =>
this.widget.call('bus_service', 'trigger', 'notification', notifications)
);
assert.verifySteps(
['rpc:channel_fetch'],
"Channel should have been fetched but not seen yet"
);
await afterNextRender(() => thread.composer.focus());
assert.verifySteps(
['rpc:channel_seen'],
"Channel should have been marked as seen after threadViewer got the focus"
);
});
QUnit.test('mark channel as fetched and seen when a new message is loaded if composer is focused', async function (assert) {
assert.expect(4);
await this.start({
mockRPC(route, args) {
if (args.method === 'channel_fetched' && args.args[0] === 100) {
throw new Error("'channel_fetched' RPC must not be called for created channel as message is directly seen");
} else if (args.method === 'channel_seen') {
assert.strictEqual(
args.args[0][0],
100,
'channel_seen is called on the right channel id'
);
assert.strictEqual(
args.model,
'mail.channel',
'channel_seen is called on the right channel model'
);
assert.step('rpc:channel_seen');
}
return this._super(...arguments);
}
@@ -334,10 +406,12 @@ QUnit.test('mark channel as fetched when a new message is loaded and as seen whe
name: "Fred",
},
]]],
message_unread_counter: 1, // seen would not be called if not > 0
model: 'mail.channel',
});
const threadViewer = this.env.models['mail.thread_viewer'].create({ thread: [['link', thread]] });
await this.createThreadViewerComponent(threadViewer);
await this.createThreadViewerComponent(threadViewer, { hasComposer: true });
const notifications = [
[['myDB', 'mail.channel', 100], {
channelId: 100,
@@ -348,15 +422,14 @@ QUnit.test('mark channel as fetched when a new message is loaded and as seen whe
channel_ids: [100],
}]
];
await afterNextRender(() => {
this.widget.call('bus_service', 'trigger', 'notification', notifications);
});
await afterNextRender(() =>
this.widget.call('bus_service', 'trigger', 'notification', notifications)
);
assert.verifySteps(
['rpc_channel_fetched:called', 'rpc_channel_seen:called'],
"Channel should have been fetched before having been seen"
['rpc:channel_seen'],
"Channel should have been mark as seen directly"
);
});
});
});
});
addons/mail/static/src/models/composer/composer.js
+3
View File
@@ -554,6 +554,9 @@ function factory(dependencies) {
'mainSuggestedPartners',
],
}),
hasFocus: attr({
default: false,
}),
hasSuggestedPartners: attr({
compute: '_computeHasSuggestedPartners',
dependencies: [
addons/mail/static/src/models/thread/thread.js
+17 -1
View File
@@ -10,6 +10,23 @@ function factory(dependencies) {
class Thread extends dependencies['mail.model'] {
/**
* FIXME With this, whenever client is aware of new thread, this will
* (almost) always focus its composer when displayed. This shouldn't be
* the case, instead auto-focus of composer is flow-specific.
* See task-2277537
*
* @override
*/
static create(data) {
if (!data.composer) {
data.composer = [['create', {
isDoFocus: true,
}]];
}
return super.create(data);
}
/**
* @override
*/
@@ -1140,7 +1157,6 @@ function factory(dependencies) {
dependencies: ['viewersChatWindow'],
}),
composer: one2one('mail.composer', {
autocreate: true,
inverse: 'thread',
isCausal: true,
}),
addons/mail/static/src/models/thread_viewer/thread_viewer.js
+55 -1
View File
@@ -67,6 +67,15 @@ function factory(dependencies) {
});
}
/**
* @param {mail.message} message
*/
handleVisibleMessage(message) {
if (!this.lastVisibleMessage || this.lastVisibleMessage.id < message.id) {
this.update({ lastVisibleMessage: [['replace', message]] });
}
}
/**
* @param {string} scrollTop
*/
@@ -96,6 +105,24 @@ function factory(dependencies) {
return [['replace', this.thread.cache(this.stringifiedDomain)]];
}
/**
* Not a real field, used to trigger `thread.markAsSeen` when one of
* the dependencies changes.
*
* @private
* @returns {boolean}
*/
_computeThreadShouldBeSetAsSeen() {
// FIXME condition should not be on "composer is focused" but "thread viewer is active"
// See task-2277543
const lastMessageIsVisible = this.lastVisibleMessage &&
this.lastVisibleMessage === this.lastMessage;
if (lastMessageIsVisible && this.hasComposerFocus && this.thread) {
this.thread.markAsSeen();
}
return true;
}
/**
* @private
* @returns {integer|undefined}
@@ -171,7 +198,7 @@ function factory(dependencies) {
}
if (
this.thread && this.threadCache.isLoading &&
this.thread && this.threadCache && this.threadCache.isLoading &&
!this.isShowingLoading && !this._isPreparingLoading
) {
this._prepareLoading();
@@ -218,6 +245,20 @@ function factory(dependencies) {
componentHintList: attr({
default: [],
}),
composer: many2one('mail.composer', {
related: 'thread.composer',
}),
hasComposerFocus: attr({
related: 'composer.hasFocus',
}),
lastMessage: many2one('mail.message', {
related: 'threadCache.lastMessage',
}),
/**
* Most recent message in the current thread viewer that has been shown
* to the current partner.
*/
lastVisibleMessage: many2one('mail.message'),
messages: many2many('mail.message', {
related: 'threadCache.messages',
}),
@@ -255,6 +296,19 @@ function factory(dependencies) {
threadCaches: many2many('mail.thread_cache', {
related: 'thread.caches',
}),
/**
* Not a real field, used to trigger `thread.markAsSeen` when one of
* the dependencies changes.
*/
threadShouldBeSetAsSeen: attr({
compute: '_computeThreadShouldBeSetAsSeen',
dependencies: [
'hasComposerFocus',
'lastMessage',
'lastVisibleMessage',
'threadCache',
],
}),
uncheckedMessages: many2many('mail.message', {
related: 'threadCache.uncheckedMessages',
}),