Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Master doc chat window imp ppr #30957

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants

@robodoo robodoo added the seen 🙂 label Feb 8, 2019

@ppr-odoo ppr-odoo force-pushed the odoo-dev:master-doc-chat-window-imp-ppr branch Feb 8, 2019

@C3POdoo C3POdoo added the RD label Feb 8, 2019

Show resolved Hide resolved addons/mail/static/src/js/models/messages/message.js Outdated
Show resolved Hide resolved addons/mail/static/tests/systray/systray_messaging_menu_tests.js Outdated
Show resolved Hide resolved addons/mail/static/src/xml/discuss.xml Outdated
Show resolved Hide resolved addons/mail/static/src/js/systray/systray_messaging_menu.js Outdated

@robodoo robodoo added the CI 🤖 label Feb 8, 2019

@ppr-odoo ppr-odoo force-pushed the odoo-dev:master-doc-chat-window-imp-ppr branch Feb 11, 2019

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Feb 11, 2019

@ppr-odoo ppr-odoo force-pushed the odoo-dev:master-doc-chat-window-imp-ppr branch Feb 21, 2019

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Feb 21, 2019

@mba-odoo mba-odoo force-pushed the odoo-dev:master-doc-chat-window-imp-ppr branch Feb 22, 2019

@alexkuhn
Copy link
Contributor

alexkuhn left a comment

This is not the correct way to do it. In the model Message, the document name never changes, so it makes no sense to provide empty string in the model.

The correct way would be to keep the true information in the Message model (so no diff at all in this file) and to put rendering options to not show the document link. In fact, there's already an option for that if you look at the template:
https://github.com/odoo/odoo/blob/master/addons/mail/static/src/xml/thread.xml#L310

This is used by the field widget mail_thread in order to not not display document links on each message:
https://github.com/odoo/odoo/blob/master/addons/mail/static/src/js/thread_field.js#L44

So if you want to do the same with document chat window, this options displayDocumentLink should be set for document threads. Something like here:
https://github.com/odoo/odoo/blob/master/addons/mail/static/src/js/thread_windows/abstract_thread_window.js#L81

And it should look like this:

var options = {
   displayMarkAsRead: false,
   displayStars: this.options.displayStars,
};
if (this._thread && this._thread.getType() === 'document_thread') {
   options.displayDocumentLinks = false;
}
this._threadWidget = new ThreadWidget(this, options);
Show resolved Hide resolved addons/mail/static/tests/systray/systray_messaging_menu_tests.js Outdated

@robodoo robodoo added the CI 🤖 label Feb 22, 2019

@ppr-odoo ppr-odoo force-pushed the odoo-dev:master-doc-chat-window-imp-ppr branch Feb 22, 2019

@robodoo robodoo removed the CI 🤖 label Feb 22, 2019

@mba-odoo mba-odoo force-pushed the odoo-dev:master-doc-chat-window-imp-ppr branch Feb 22, 2019

@mba-odoo mba-odoo force-pushed the odoo-dev:master-doc-chat-window-imp-ppr branch to 9c939c7 Feb 22, 2019

mba-odoo added a commit to odoo-dev/odoo that referenced this pull request Feb 22, 2019

[IMP] mail: Improve the document chat window
On Hover the systray, add 'expand' icon, it directly open the form view.

Delete the name of the task on all messages: only keep it in the header of the chat window.
For messages on praticular document do not diplay the Document Links in chatter window.

Task:1929169

closes odoo#30957

@robodoo robodoo added the CI 🤖 label Feb 22, 2019

Show resolved Hide resolved addons/mail/static/src/xml/discuss.xml Outdated
Show resolved Hide resolved addons/mail/static/src/xml/discuss.xml Outdated
Show resolved Hide resolved addons/mail/static/tests/systray/systray_messaging_menu_tests.js Outdated
Show resolved Hide resolved addons/mail/static/tests/systray/systray_messaging_menu_tests.js Outdated
Show resolved Hide resolved addons/mail/static/tests/systray/systray_messaging_menu_tests.js Outdated
var $preview = messagingMenu.$('.o_mail_preview');
assert.containsOnce(messagingMenu, '.o_thread_window_expand', "should display one preview");
assert.strictEqual($preview.data('document-model'), "some.res.model", "preview should be from some.res.model");
$preview.find('.o_thread_window_expand').click();

This comment has been minimized.

@alexkuhn

alexkuhn Mar 18, 2019

Contributor

await testUtils.dom.click(/* ... */)

This comment has been minimized.

@ppr-odoo

ppr-odoo Mar 18, 2019

Author Contributor

changes done

This comment has been minimized.

@alexkuhn

alexkuhn Mar 18, 2019

Contributor

click should be mocked:

await testUtils.dom.click($preview.find('.o_thread_window_expand'));
Show resolved Hide resolved addons/mail/static/tests/systray/systray_messaging_menu_tests.js Outdated
},
intercepts: {
do_action: function (ev) {
assert.step('do_action');

This comment has been minimized.

@alexkuhn

alexkuhn Mar 18, 2019

Contributor

I would add more assertion about this do_action, to ensure it opens the related document.

This comment has been minimized.

@ppr-odoo

ppr-odoo Mar 18, 2019

Author Contributor

changes done

@ppr-odoo ppr-odoo force-pushed the odoo-dev:master-doc-chat-window-imp-ppr branch from 9c939c7 to b8ab068 Mar 18, 2019

ppr-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 18, 2019

[IMP] mail: Improve the document chat window
On Hover the systray, add 'expand' icon, it directly open the form view.

Delete the name of the task on all messages: only keep it in the header of the chat window.
For messages on praticular document do not diplay the Document Links in chatter window.

Task:1929169

closes odoo#30957

@robodoo robodoo removed the CI 🤖 label Mar 18, 2019

@@ -316,6 +316,7 @@
<span class="o_preview_counter">
<t t-if="preview.unreadCounter &gt; 0">&amp;nbsp;(<t t-esc="preview.unreadCounter"/>)</t>
</span>
<span class="o_preview_expand o_discuss_icon o_thread_window_expand fa fa-expand p-2 align-self-center" t-if="preview.isLinkedToDocumentThread"/>

This comment has been minimized.

@alexkuhn

alexkuhn Mar 18, 2019

Contributor

I would remove p-2 (too much padding) and align-self-center (no visual changes).
I would instead add the following style on this element in SCSS:

.o_preview_title {
   align-items: center;
}

.o_preview_expand {
   margin: 0px 6px;
}
var $preview = messagingMenu.$('.o_mail_preview');
assert.containsOnce(messagingMenu, '.o_thread_window_expand', "should display one preview");
assert.strictEqual($preview.data('document-model'), "some.res.model", "preview should be from some.res.model");
$preview.find('.o_thread_window_expand').click();

This comment has been minimized.

@alexkuhn

alexkuhn Mar 18, 2019

Contributor

click should be mocked:

await testUtils.dom.click($preview.find('.o_thread_window_expand'));
if (ev.data.action.res_model === 'some.res.model' &&
ev.data.action.res_id === 123
) {
assert.step('do_action');

This comment has been minimized.

@alexkuhn

alexkuhn Mar 18, 2019

Contributor

do_action:open_document

assert.containsOnce(messagingMenu, '.o_thread_window_expand', "should display one preview");
assert.strictEqual($preview.data('document-model'), "some.res.model", "preview should be from some.res.model");
$preview.find('.o_thread_window_expand').click();
assert.verifySteps(['do_action']);

This comment has been minimized.

@alexkuhn

alexkuhn Mar 18, 2019

Contributor

do_action:open_document

This comment has been minimized.

@ppr-odoo

ppr-odoo Mar 18, 2019

Author Contributor

Changes done

@robodoo robodoo added the CI 🤖 label Mar 18, 2019

[IMP] mail: Improve the document chat window
On Hover the systray, add 'expand' icon, it directly open the form view.

Delete the name of the task on all messages: only keep it in the header of the chat window.
For messages on praticular document do not diplay the Document Links in chatter window.

Task:1929169

closes #30957

@ppr-odoo ppr-odoo force-pushed the odoo-dev:master-doc-chat-window-imp-ppr branch from b8ab068 to 3e353eb Mar 18, 2019

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Mar 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.