From a35c71690ad6fa6d2ca06827b43ce04ce158b560 Mon Sep 17 00:00:00 2001 From: Hiral Bhavsar Date: Thu, 5 May 2022 06:51:27 +0000 Subject: [PATCH] [IMP] mail: chatter file drag and drop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PURPOSE The attachment icon in the chatter is too discrete, a lot of users easily miss it. SPECIFICATIONS - Allow for file upload through drag and drop and the whole chatter. It doesn't matter whether the file tray is unfolded. Hence, removed DropZone on attachmentBox as now have DropZone on the whole chatter. - On the file tray, rename "Attachments" to "Files" and "Add attachments" to "Attach files". - When there are files attached to the record, display "X files". - when there is no file yet, display 'Attach files'. Although clicking on it opens the file explorer, but doesn't unfold the tray yet. Once the file is submitted, the file tray unfolds to show confirmed upload to the user. Task-2413814 closes odoo/odoo#91335 Related: odoo/enterprise#27763 Signed-off-by: Sébastien Theys (seb) --- .../attachment_box/attachment_box.xml | 10 +-- .../static/src/components/chatter/chatter.xml | 6 ++ .../chatter_topbar/chatter_topbar.xml | 2 +- .../static/src/models/attachment_box_view.js | 22 ----- addons/mail/static/src/models/chatter.js | 43 ++++++++- .../mail/static/src/models/chatter_topbar.js | 35 +++++++- .../mail/static/src/models/drop_zone_view.js | 8 +- .../mail/static/src/models/file_uploader.js | 14 ++- .../src/models/use_drag_visible_drop_zone.js | 4 +- .../components/attachment_box_tests.js | 88 +------------------ .../components/chatter_tests.js | 75 +++++++++++++++- .../components/chatter_topbar_tests.js | 12 +-- .../models/file_uploader_tests.js | 8 +- .../widgets/form_renderer_tests.js | 4 +- .../tours/mail_full_composer_test_tour.js | 4 +- 15 files changed, 190 insertions(+), 145 deletions(-) diff --git a/addons/mail/static/src/components/attachment_box/attachment_box.xml b/addons/mail/static/src/components/attachment_box/attachment_box.xml index 2a03d3fabc7..3fbe6188ec0 100644 --- a/addons/mail/static/src/components/attachment_box/attachment_box.xml +++ b/addons/mail/static/src/components/attachment_box/attachment_box.xml @@ -7,17 +7,11 @@
- Attachments + Files
- - -
diff --git a/addons/mail/static/src/components/chatter/chatter.xml b/addons/mail/static/src/components/chatter/chatter.xml index bc6726d782e..4f87bc78b66 100644 --- a/addons/mail/static/src/components/chatter/chatter.xml +++ b/addons/mail/static/src/components/chatter/chatter.xml @@ -4,6 +4,12 @@
+ + +
- + diff --git a/addons/mail/static/src/models/attachment_box_view.js b/addons/mail/static/src/models/attachment_box_view.js index 91fcc6f6c22..57c2dc3275c 100644 --- a/addons/mail/static/src/models/attachment_box_view.js +++ b/addons/mail/static/src/models/attachment_box_view.js @@ -23,16 +23,6 @@ registerModel({ ? insertAndReplace() : clear(); }, - /** - * @private - * @returns {FieldCommand} - */ - _computeDropZoneView() { - if (this.useDragVisibleDropZone.isVisible) { - return insertAndReplace(); - } - return clear(); - }, }, fields: { /** @@ -53,11 +43,6 @@ registerModel({ * States the OWL component displaying this attachment box. */ component: attr(), - dropZoneView: one('DropZoneView', { - compute: '_computeDropZoneView', - inverse: 'attachmentBoxViewOwner', - isCausal: true, - }), fileUploader: one('FileUploader', { default: insertAndReplace(), inverse: 'attachmentBoxView', @@ -65,12 +50,5 @@ registerModel({ readonly: true, required: true, }), - useDragVisibleDropZone: one('UseDragVisibleDropZone', { - default: insertAndReplace(), - inverse: 'attachmentBoxViewOwner', - isCausal: true, - readonly: true, - required: true, - }), }, }); diff --git a/addons/mail/static/src/models/chatter.js b/addons/mail/static/src/models/chatter.js index 84529f8ad2a..e694b056214 100644 --- a/addons/mail/static/src/models/chatter.js +++ b/addons/mail/static/src/models/chatter.js @@ -41,7 +41,11 @@ registerModel({ * @param {MouseEvent} ev */ onClickButtonAttachments(ev) { - this.update({ attachmentBoxView: this.attachmentBoxView ? clear() : insertAndReplace() }); + if (this.thread && this.thread.allAttachments.length === 0) { + this.fileUploader.openBrowserFileUploader(); + } else { + this.update({ attachmentBoxView: this.attachmentBoxView ? clear() : insertAndReplace() }); + } }, /** * Handles click on top bar close button. @@ -112,6 +116,9 @@ registerModel({ } this.threadView.messageListView.component.onScroll(ev); }, + openAttachmentBoxView() { + this.update({ attachmentBoxView: insertAndReplace() }); + }, async refresh() { const requestData = ['activities', 'followers', 'suggestedRecipients']; if (this.hasMessageList) { @@ -152,6 +159,23 @@ registerModel({ } return clear(); }, + /** + * @private + * @returns {FieldCommand} + */ + _computeDropZoneView() { + if (this.useDragVisibleDropZone.isVisible) { + return insertAndReplace(); + } + return clear(); + }, + /** + * @private + * @returns {FieldCommand} + */ + _computeFileUploader() { + return this.thread ? insertAndReplace() : clear(); + }, /** * @private * @returns {FieldCommand} @@ -310,6 +334,16 @@ registerModel({ context: attr({ default: {}, }), + dropZoneView: one('DropZoneView', { + compute: '_computeDropZoneView', + inverse: 'chatterOwner', + isCausal: true, + }), + fileUploader: one('FileUploader', { + compute: '_computeFileUploader', + inverse: 'chatterOwner', + isCausal: true, + }), followButtonView: one('FollowButtonView', { compute: '_computeFollowButtonView', inverse: 'chatterOwner', @@ -423,6 +457,13 @@ registerModel({ inverse: 'chatter', isCausal: true, }), + useDragVisibleDropZone: one('UseDragVisibleDropZone', { + default: insertAndReplace(), + inverse: 'chatterOwner', + isCausal: true, + readonly: true, + required: true, + }), }, onChanges: [ new OnChange({ diff --git a/addons/mail/static/src/models/chatter_topbar.js b/addons/mail/static/src/models/chatter_topbar.js index 9b64dee1bc7..4ff0c0205b5 100644 --- a/addons/mail/static/src/models/chatter_topbar.js +++ b/addons/mail/static/src/models/chatter_topbar.js @@ -1,12 +1,45 @@ /** @odoo-module **/ import { registerModel } from '@mail/model/model_core'; -import { one } from '@mail/model/model_field'; +import { attr, one } from '@mail/model/model_field'; + +import { sprintf } from '@web/core/utils/strings'; registerModel({ name: 'ChatterTopbar', identifyingFields: ['chatter'], + recordMethods: { + /** + * @private + * @returns {string} + */ + _computeAttachmentButtonText() { + if (!this.chatter || !this.chatter.thread) { + return; + } + const attachments = this.chatter.thread.allAttachments; + switch (attachments.length) { + case 0: + return this.env._t("Attach files"); + case 1: + return this.env._t("1 file"); + case 2: + return this.env._t("2 files"); + default: + return sprintf( + this.env._t("%(attachmentCount)s files"), + { attachmentCount: attachments.length } + ); + } + }, + }, fields: { + /** + * Determines the label on the attachment button of the topbar. + */ + attachmentButtonText: attr({ + compute: '_computeAttachmentButtonText', + }), chatter: one('Chatter', { inverse: 'topbar', readonly: true, diff --git a/addons/mail/static/src/models/drop_zone_view.js b/addons/mail/static/src/models/drop_zone_view.js index 8f4d2e2da01..b15607bb8f5 100644 --- a/addons/mail/static/src/models/drop_zone_view.js +++ b/addons/mail/static/src/models/drop_zone_view.js @@ -6,7 +6,7 @@ import { decrement, increment } from '@mail/model/model_field_command'; registerModel({ name: 'DropZoneView', - identifyingFields: [['attachmentBoxViewOwner', 'composerViewOwner']], + identifyingFields: [['chatterOwner', 'composerViewOwner']], recordMethods: { /** * Shows a visual drop effect when dragging inside the dropzone. @@ -63,8 +63,8 @@ registerModel({ ev.preventDefault(); this.update({ isDraggingInside: false }); if (this._isDragSourceExternalFile(ev.dataTransfer)) { - if (this.attachmentBoxViewOwner) { - await this.attachmentBoxViewOwner.fileUploader.uploadFiles(ev.dataTransfer.files); + if (this.chatterOwner) { + await this.chatterOwner.fileUploader.uploadFiles(ev.dataTransfer.files); } if (this.composerViewOwner) { await this.composerViewOwner.fileUploader.uploadFiles(ev.dataTransfer.files); @@ -91,7 +91,7 @@ registerModel({ }, }, fields: { - attachmentBoxViewOwner: one('AttachmentBoxView', { + chatterOwner: one('Chatter', { inverse: 'dropZoneView', readonly: true, }), diff --git a/addons/mail/static/src/models/file_uploader.js b/addons/mail/static/src/models/file_uploader.js index 792599958a8..7412993855d 100644 --- a/addons/mail/static/src/models/file_uploader.js +++ b/addons/mail/static/src/models/file_uploader.js @@ -16,7 +16,7 @@ const getAttachmentNextTemporaryId = (function () { registerModel({ name: 'FileUploader', - identifyingFields: [['activityView', 'attachmentBoxView', 'composerView']], + identifyingFields: [['activityView', 'attachmentBoxView', 'chatterOwner', 'composerView']], recordMethods: { openBrowserFileUploader() { this.fileInput.click(); @@ -40,6 +40,9 @@ registerModel({ if (this.fileInput && this.fileInput.el) { this.fileInput.el.value = ''; } + if (this.chatterOwner && !this.chatterOwner.attachmentBoxView) { + this.chatterOwner.openAttachmentBoxView(); + } }, /** * Create an HTML element that will serve as file input. @@ -67,6 +70,9 @@ registerModel({ if (this.attachmentBoxView) { return replace(this.attachmentBoxView.chatter.thread); } + if (this.chatterOwner) { + return replace(this.chatterOwner.thread); + } if (this.composerView) { return replace(this.composerView.composer.activeThread); } @@ -120,7 +126,7 @@ registerModel({ async _performUpload({ files }) { const composer = this.composerView && this.composerView.composer; // save before async const thread = this.thread; // save before async - const chatter = this.attachmentBoxView && this.attachmentBoxView.chatter; // save before async + const chatter = this.chatterOwner; // save before async const activity = this.activityView && this.activityView.activity; // save before async const uploadingAttachments = new Map(); for (const file of files) { @@ -182,6 +188,10 @@ registerModel({ inverse: 'fileUploader', readonly: true, }), + chatterOwner: one('Chatter', { + inverse: 'fileUploader', + readonly: true, + }), composerView: one('ComposerView', { inverse: 'fileUploader', readonly: true, diff --git a/addons/mail/static/src/models/use_drag_visible_drop_zone.js b/addons/mail/static/src/models/use_drag_visible_drop_zone.js index 9d927dcd2d4..60404c8e22e 100644 --- a/addons/mail/static/src/models/use_drag_visible_drop_zone.js +++ b/addons/mail/static/src/models/use_drag_visible_drop_zone.js @@ -6,7 +6,7 @@ import { decrement, increment } from '@mail/model/model_field_command'; registerModel({ name: 'UseDragVisibleDropZone', - identifyingFields: [['attachmentBoxViewOwner', 'composerViewOwner']], + identifyingFields: [['chatterOwner', 'composerViewOwner']], lifecycleHooks: { _created() { document.addEventListener('dragenter', this._onDragenterListener, true); @@ -68,7 +68,7 @@ registerModel({ }, }, fields: { - attachmentBoxViewOwner: one('AttachmentBoxView', { + chatterOwner: one('Chatter', { inverse: 'useDragVisibleDropZone', readonly: true, }), diff --git a/addons/mail/static/tests/qunit_suite_tests/components/attachment_box_tests.js b/addons/mail/static/tests/qunit_suite_tests/components/attachment_box_tests.js index afbcc8fb8c0..a677f752f28 100644 --- a/addons/mail/static/tests/qunit_suite_tests/components/attachment_box_tests.js +++ b/addons/mail/static/tests/qunit_suite_tests/components/attachment_box_tests.js @@ -1,16 +1,6 @@ /** @odoo-module **/ -import { - afterNextRender, - dragenterFiles, - dropFiles, - start, - startServer, -} from '@mail/../tests/helpers/test_utils'; - -import { file } from 'web.test_utils'; - -const { createFile } = file; +import { start, startServer } from '@mail/../tests/helpers/test_utils'; QUnit.module('mail', {}, function () { QUnit.module('components', {}, function () { @@ -94,82 +84,6 @@ QUnit.test('base non-empty rendering', async function (assert) { ); }); -QUnit.test('attachment box: drop attachments', async function (assert) { - assert.expect(5); - - const pyEnv = await startServer(); - const resPartnerId1 = pyEnv['res.partner'].create(); - const { createChatterContainerComponent } = await start(); - await createChatterContainerComponent({ - isAttachmentBoxVisibleInitially: true, - threadId: resPartnerId1, - threadModel: 'res.partner', - }); - const files = [ - await createFile({ - content: 'hello, world', - contentType: 'text/plain', - name: 'text.txt', - }), - ]; - assert.strictEqual( - document.querySelectorAll('.o_AttachmentBox').length, - 1, - "should have an attachment box" - ); - - await afterNextRender(() => - dragenterFiles(document.querySelector('.o_AttachmentBox')) - ); - assert.ok( - document.querySelector('.o_AttachmentBox_dropZone'), - "should have a drop zone" - ); - assert.strictEqual( - document.querySelectorAll(`.o_AttachmentBox .o_AttachmentCard`).length, - 0, - "should have no attachment before files are dropped" - ); - - await afterNextRender(() => - dropFiles( - document.querySelector('.o_AttachmentBox_dropZone'), - files - ) - ); - - assert.strictEqual( - document.querySelectorAll(`.o_AttachmentBox .o_AttachmentCard`).length, - 1, - "should have 1 attachment in the box after files dropped" - ); - - await afterNextRender(() => - dragenterFiles(document.querySelector('.o_AttachmentBox')) - ); - const file1 = await createFile({ - content: 'hello, world', - contentType: 'text/plain', - name: 'text2.txt', - }); - const file2 = await createFile({ - content: 'hello, world', - contentType: 'text/plain', - name: 'text3.txt', - }); - await afterNextRender(() => - dropFiles( - document.querySelector('.o_AttachmentBox_dropZone'), - [file1, file2] - ) - ); - assert.strictEqual( - document.querySelectorAll(`.o_AttachmentBox .o_AttachmentCard`).length, - 3, - "should have 3 attachments in the box after files dropped" - ); -}); - QUnit.test('view attachments', async function (assert) { assert.expect(7); diff --git a/addons/mail/static/tests/qunit_suite_tests/components/chatter_tests.js b/addons/mail/static/tests/qunit_suite_tests/components/chatter_tests.js index 57dcb368ed9..a41c347f2fd 100644 --- a/addons/mail/static/tests/qunit_suite_tests/components/chatter_tests.js +++ b/addons/mail/static/tests/qunit_suite_tests/components/chatter_tests.js @@ -1,6 +1,17 @@ /** @odoo-module **/ -import { afterNextRender, nextAnimationFrame, start, startServer } from '@mail/../tests/helpers/test_utils'; +import { + afterNextRender, + dragenterFiles, + dropFiles, + nextAnimationFrame, + start, + startServer +} from '@mail/../tests/helpers/test_utils'; + +import { file } from 'web.test_utils'; + +const { createFile } = file; QUnit.module('mail', {}, function () { QUnit.module('components', {}, function () { @@ -218,6 +229,68 @@ QUnit.test('show attachment box', async function (assert) { ); }); +QUnit.test('chatter: drop attachments', async function (assert) { + assert.expect(4); + + const pyEnv = await startServer(); + const resPartnerId1 = pyEnv['res.partner'].create(); + const { createChatterContainerComponent } = await start(); + await createChatterContainerComponent({ + threadId: resPartnerId1, + threadModel: 'res.partner', + }); + const files = [ + await createFile({ + content: 'hello, world', + contentType: 'text/plain', + name: 'text.txt', + }), + await createFile({ + content: 'hello, worlduh', + contentType: 'text/plain', + name: 'text2.txt', + }), + ]; + await afterNextRender(() => dragenterFiles(document.querySelector('.o_Chatter'))); + assert.ok( + document.querySelector('.o_Chatter_dropZone'), + "should have a drop zone" + ); + assert.strictEqual( + document.querySelectorAll(`.o_AttachmentBox`).length, + 0, + "should have no attachment before files are dropped" + ); + + await afterNextRender(() => + dropFiles(document.querySelector('.o_Chatter_dropZone'), files) + ); + assert.strictEqual( + document.querySelectorAll(`.o_AttachmentBox .o_AttachmentCard`).length, + 2, + "should have 2 attachments in the attachment box after files dropped" + ); + + await afterNextRender(() => dragenterFiles(document.querySelector('.o_Chatter'))); + await afterNextRender(async () => + dropFiles( + document.querySelector('.o_Chatter_dropZone'), + [ + await createFile({ + content: 'hello, world', + contentType: 'text/plain', + name: 'text3.txt', + }) + ] + ) + ); + assert.strictEqual( + document.querySelectorAll(`.o_AttachmentBox .o_AttachmentCard`).length, + 3, + "should have 3 attachments in the attachment box after files dropped" + ); +}); + QUnit.test('composer show/hide on log note/send message [REQUIRE FOCUS]', async function (assert) { assert.expect(10); diff --git a/addons/mail/static/tests/qunit_suite_tests/components/chatter_topbar_tests.js b/addons/mail/static/tests/qunit_suite_tests/components/chatter_topbar_tests.js index c3682aa0731..7dbec6b83ae 100644 --- a/addons/mail/static/tests/qunit_suite_tests/components/chatter_topbar_tests.js +++ b/addons/mail/static/tests/qunit_suite_tests/components/chatter_topbar_tests.js @@ -101,8 +101,8 @@ QUnit.test('base disabled rendering', async function (assert) { ); assert.strictEqual( document.querySelector(`.o_ChatterTopbar_buttonAttachmentsCount`).textContent, - '0', - "attachments button counter should be 0" + 'Attach files', + "attachments button counter content should be 'Attach files'" ); }); @@ -273,8 +273,8 @@ QUnit.test('attachment counter without attachments', async function (assert) { ); assert.strictEqual( document.querySelector(`.o_ChatterTopbar_buttonAttachmentsCount`).textContent, - '0', - 'attachment counter should contain "0"' + 'Attach files', + 'attachment counter content should contain "Attach files"' ); }); @@ -320,8 +320,8 @@ QUnit.test('attachment counter with attachments', async function (assert) { ); assert.strictEqual( document.querySelector(`.o_ChatterTopbar_buttonAttachmentsCount`).textContent, - '2', - 'attachment counter should contain "2"' + '2 files', + 'attachment counter content should contain "2 files"' ); }); diff --git a/addons/mail/static/tests/qunit_suite_tests/models/file_uploader_tests.js b/addons/mail/static/tests/qunit_suite_tests/models/file_uploader_tests.js index f4464ce4d16..8f5408219a2 100644 --- a/addons/mail/static/tests/qunit_suite_tests/models/file_uploader_tests.js +++ b/addons/mail/static/tests/qunit_suite_tests/models/file_uploader_tests.js @@ -24,10 +24,6 @@ QUnit.test('no conflicts between file uploaders', async function (assert) { threadId: resPartnerId2, threadModel: 'res.partner', }); - await afterNextRender(() => { - document.querySelectorAll('.o_ChatterTopbar_buttonAttachments')[0].click(); - document.querySelectorAll('.o_ChatterTopbar_buttonAttachments')[1].click(); - }); const file1 = await createFile({ name: 'text1.txt', @@ -35,7 +31,7 @@ QUnit.test('no conflicts between file uploaders', async function (assert) { contentType: 'text/plain', }); await afterNextRender(() => inputFiles( - firstChatterContainerComponent.chatter.attachmentBoxView.fileUploader.fileInput, + firstChatterContainerComponent.chatter.fileUploader.fileInput, [file1] )); assert.strictEqual( @@ -50,7 +46,7 @@ QUnit.test('no conflicts between file uploaders', async function (assert) { contentType: 'text/plain', }); await afterNextRender(() => inputFiles( - secondChatterContainerComponent.chatter.attachmentBoxView.fileUploader.fileInput, + secondChatterContainerComponent.chatter.fileUploader.fileInput, [file2] )); assert.strictEqual( diff --git a/addons/mail/static/tests/qunit_suite_tests/widgets/form_renderer_tests.js b/addons/mail/static/tests/qunit_suite_tests/widgets/form_renderer_tests.js index 848a07a0e8a..2c132183e21 100644 --- a/addons/mail/static/tests/qunit_suite_tests/widgets/form_renderer_tests.js +++ b/addons/mail/static/tests/qunit_suite_tests/widgets/form_renderer_tests.js @@ -879,7 +879,7 @@ QUnit.test('Attachments that have been unlinked from server should be visually u ); assert.strictEqual( document.querySelector('.o_ChatterTopbar_buttonCount').textContent, - '2', + '2 files', "Partner1 should have 2 attachments initially" ); @@ -891,7 +891,7 @@ QUnit.test('Attachments that have been unlinked from server should be visually u await click('.o_pager_previous'); assert.strictEqual( document.querySelector('.o_ChatterTopbar_buttonCount').textContent, - '1', + '1 file', "Partner1 should now have 1 attachment after it has been unlinked from server" ); }); diff --git a/addons/mail/static/tests/tours/mail_full_composer_test_tour.js b/addons/mail/static/tests/tours/mail_full_composer_test_tour.js index 7af58fa351c..f4cf606e706 100644 --- a/addons/mail/static/tests/tours/mail_full_composer_test_tour.js +++ b/addons/mail/static/tests/tours/mail_full_composer_test_tour.js @@ -32,9 +32,9 @@ tour.register('mail/static/tests/tours/mail_full_composer_test_tour.js', { name: 'text.txt', }); const messaging = await odoo.__DEBUG__.messaging; - const uploaders = messaging.models['FileUploader'].all(); + const uploader = messaging.models['ComposerView'].all()[0].fileUploader; inputFiles( - uploaders[0].fileInput, + uploader.fileInput, [file] ); },