From cf31cd7946cad77fa3147a264b355dadc936c035 Mon Sep 17 00:00:00 2001
From: Xavier Dubuc <xdu@odoo.com>
Date: Wed, 2 Dec 2020 09:32:47 +0000
Subject: [PATCH] [FIX] mail: split attachments list to improve style
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There is now two lists of attachments : first one with images and second
one with non-images. Attachment viewer allows to navigate through
previews of all viewable attachments in the order they're shown.

task-2272587
task-2401325

closes odoo/odoo#62719

Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
---
 .../attachment_box/attachment_box_tests.js    |  4 +-
 .../attachment_list/attachment_list.js        | 55 ++++++++++++++++++-
 .../attachment_list/attachment_list.scss      | 14 ++++-
 .../attachment_list/attachment_list.xml       | 43 ++++++++++-----
 .../src/components/composer/composer.xml      |  1 +
 .../static/src/components/message/message.js  | 15 -----
 .../static/src/components/message/message.xml | 26 +++------
 7 files changed, 106 insertions(+), 52 deletions(-)

diff --git a/addons/mail/static/src/components/attachment_box/attachment_box_tests.js b/addons/mail/static/src/components/attachment_box/attachment_box_tests.js
index 6f217c2b5d7..885742b72cc 100644
--- a/addons/mail/static/src/components/attachment_box/attachment_box_tests.js
+++ b/addons/mail/static/src/components/attachment_box/attachment_box_tests.js
@@ -307,11 +307,11 @@ QUnit.test('remove attachment should ask for confirmation', async function (asse
     );
     assert.containsOnce(
         document.body,
-        '.o_Attachment_actionUnlink',
+        '.o_Attachment_asideItemUnlink',
         "attachment should have a delete button"
     );
 
-    await afterNextRender(() => document.querySelector('.o_Attachment_actionUnlink').click());
+    await afterNextRender(() => document.querySelector('.o_Attachment_asideItemUnlink').click());
     assert.containsOnce(
         document.body,
         '.o_AttachmentDeleteConfirmDialog',
diff --git a/addons/mail/static/src/components/attachment_list/attachment_list.js b/addons/mail/static/src/components/attachment_list/attachment_list.js
index d097e5d705d..ecd7e2f47c8 100644
--- a/addons/mail/static/src/components/attachment_list/attachment_list.js
+++ b/addons/mail/static/src/components/attachment_list/attachment_list.js
@@ -4,10 +4,63 @@ odoo.define('mail/static/src/components/attachment_list/attachment_list.js', fun
 const components = {
     Attachment: require('mail/static/src/components/attachment/attachment.js'),
 };
+const useStore = require('mail/static/src/component_hooks/use_store/use_store.js');
 
 const { Component } = owl;
 
-class AttachmentList extends Component {}
+class AttachmentList extends Component {
+    /**
+     * @override
+     */
+    constructor(...args) {
+        super(...args);
+        useStore(props => {
+            const attachments = this.env.models['mail.attachment'].all().filter(attachment =>
+                props.attachmentLocalIds.includes(attachment.localId)
+            );
+            return {
+                attachments: attachments
+                    ? attachments.map(attachment => attachment.__state)
+                    : undefined,
+            };
+        });
+    }
+
+    //--------------------------------------------------------------------------
+    // Public
+    //--------------------------------------------------------------------------
+
+    /**
+     * @returns {mail.attachment[]}
+     */
+    get attachments() {
+        return this.env.models['mail.attachment'].all().filter(attachment =>
+            this.props.attachmentLocalIds.includes(attachment.localId)
+        );
+    }
+
+    /**
+     * @returns {mail.attachment[]}
+     */
+    get imageAttachments() {
+        return this.attachments.filter(attachment => attachment.fileType === 'image');
+    }
+
+    /**
+     * @returns {mail.attachment[]}
+     */
+    get nonImageAttachments() {
+        return this.attachments.filter(attachment => attachment.fileType !== 'image');
+    }
+
+    /**
+     * @returns {mail.attachment[]}
+     */
+    get viewableAttachments() {
+        return this.attachments.filter(attachment => attachment.isViewable);
+    }
+
+}
 
 Object.assign(AttachmentList, {
     components,
diff --git a/addons/mail/static/src/components/attachment_list/attachment_list.scss b/addons/mail/static/src/components/attachment_list/attachment_list.scss
index 710e48e75ff..dfe281aed2d 100644
--- a/addons/mail/static/src/components/attachment_list/attachment_list.scss
+++ b/addons/mail/static/src/components/attachment_list/attachment_list.scss
@@ -4,9 +4,8 @@
 
 .o_AttachmentList {
     display: flex;
-    flex-flow: wrap;
+    flex-flow: column;
     justify-content: flex-start;
-    margin: map-get($spacers, 1);
 }
 
 /* Avoid overflow of long attachment text */
@@ -17,3 +16,14 @@
     margin-inline-start: map-get($spacers, 0);
     max-width: 100%;
 }
+
+.o_AttachmentList_partialList {
+    display: flex;
+    flex: 1;
+    flex-flow: wrap;
+}
+
+.o_AttachmentList_partialListNonImages {
+    margin: map-get($spacers, 1);
+    justify-content: flex-start;
+}
diff --git a/addons/mail/static/src/components/attachment_list/attachment_list.xml b/addons/mail/static/src/components/attachment_list/attachment_list.xml
index 452419105ff..394992852fd 100644
--- a/addons/mail/static/src/components/attachment_list/attachment_list.xml
+++ b/addons/mail/static/src/components/attachment_list/attachment_list.xml
@@ -3,19 +3,36 @@
 
     <t t-name="mail.AttachmentList" owl="1">
         <div class="o_AttachmentList">
-            <t t-foreach="props.attachmentLocalIds" t-as="attachmentLocalId" t-key="attachmentLocalId">
-                <Attachment
-                    class="o_AttachmentList_attachment"
-                    attachmentLocalId="attachmentLocalId"
-                    attachmentLocalIds="props.attachmentLocalIds"
-                    detailsMode="props.attachmentsDetailsMode"
-                    imageSize="props.attachmentsImageSize"
-                    isDownloadable="props.areAttachmentsDownloadable"
-                    isEditable="props.areAttachmentsEditable"
-                    showExtension="props.showAttachmentsExtensions"
-                    showFilename="props.showAttachmentsFilenames"
-                />
-            </t>
+            <div class="o_AttachmentList_partialList o_AttachmentList_partialListImages">
+                <t t-foreach="imageAttachments" t-as="attachment" t-key="attachment.localId">
+                    <Attachment
+                        class="o_AttachmentList_attachment o_AttachmentList_imageAttachment"
+                        attachmentLocalId="attachment.localId"
+                        attachmentLocalIds="viewableAttachments.map(attachment => attachment.localId)"
+                        detailsMode="props.attachmentsDetailsMode"
+                        imageSize="props.attachmentsImageSize"
+                        isDownloadable="props.areAttachmentsDownloadable"
+                        isEditable="props.areAttachmentsEditable"
+                        showExtension="props.showAttachmentsExtensions"
+                        showFilename="props.showAttachmentsFilenames"
+                    />
+                </t>
+            </div>
+            <div class="o_AttachmentList_partialList o_AttachmentList_partialListNonImages">
+                <t t-foreach="nonImageAttachments" t-as="attachment" t-key="attachment.localId">
+                    <Attachment
+                        class="o_AttachmentList_attachment o_AttachmentList_nonImageAttachment"
+                        attachmentLocalId="attachment.localId"
+                        attachmentLocalIds="viewableAttachments.map(attachment => attachment.localId)"
+                        detailsMode="'card'"
+                        imageSize="props.attachmentsImageSize"
+                        isDownloadable="props.areAttachmentsDownloadable"
+                        isEditable="props.areAttachmentsEditable"
+                        showExtension="props.showAttachmentsExtensions"
+                        showFilename="props.showAttachmentsFilenames"
+                    />
+                </t>
+            </div>
         </div>
     </t>
 
diff --git a/addons/mail/static/src/components/composer/composer.xml b/addons/mail/static/src/components/composer/composer.xml
index c85b5802863..cc7038d3a92 100644
--- a/addons/mail/static/src/components/composer/composer.xml
+++ b/addons/mail/static/src/components/composer/composer.xml
@@ -135,6 +135,7 @@
                                 t-att-class="{ 'o-composer-is-compact': props.isCompact }"
                                 areAttachmentsEditable="true"
                                 attachmentsDetailsMode="props.attachmentsDetailsMode"
+                                attachmentsImageSize="'small'"
                                 attachmentLocalIds="composer.attachments.map(attachment => attachment.localId)"
                                 showAttachmentsExtensions="props.showAttachmentsExtensions"
                                 showAttachmentsFilenames="props.showAttachmentsFilenames"
diff --git a/addons/mail/static/src/components/message/message.js b/addons/mail/static/src/components/message/message.js
index 28c36f4c9fc..256a713289c 100644
--- a/addons/mail/static/src/components/message/message.js
+++ b/addons/mail/static/src/components/message/message.js
@@ -155,13 +155,6 @@ class Message extends Component {
         return true;
     }
 
-    /**
-     * @returns {mail.attachment[]}
-     */
-    get imageAttachments() {
-        return this.message.attachments.filter(attachment => attachment.fileType === 'image');
-    }
-
     /**
      * Tell whether the bottom of this message is visible or not.
      *
@@ -222,14 +215,6 @@ class Message extends Component {
     get message() {
         return this.env.models['mail.message'].get(this.props.messageLocalId);
     }
-
-    /**
-     * @returns {mail.attachment[]}
-     */
-    get nonImageAttachments() {
-        return this.message.attachments.filter(attachment => attachment.fileType !== 'image');
-    }
-
     /**
      * @returns {string}
      */
diff --git a/addons/mail/static/src/components/message/message.xml b/addons/mail/static/src/components/message/message.xml
index e43adc413b4..f60eab136f8 100644
--- a/addons/mail/static/src/components/message/message.xml
+++ b/addons/mail/static/src/components/message/message.xml
@@ -183,25 +183,13 @@
                     </t>
                     <t t-if="message.attachments and message.attachments.length > 0">
                         <div class="o_Message_footer">
-                            <!-- TODO XDU This does not allow to preview all attachments at once, find somethind else -->
-                            <t t-if="imageAttachments and imageAttachments.length > 0">
-                                <AttachmentList
-                                    class="o_Message_attachmentList"
-                                    areAttachmentsDownloadable="true"
-                                    areAttachmentsEditable="message.author === env.messaging.currentPartner"
-                                    attachmentLocalIds="imageAttachments.map(attachment => attachment.localId)"
-                                    attachmentsDetailsMode="props.attachmentsDetailsMode"
-                                />
-                            </t>
-                            <t t-if="nonImageAttachments and nonImageAttachments.length > 0">
-                                <AttachmentList
-                                    class="o_Message_attachmentList"
-                                    areAttachmentsDownloadable="true"
-                                    areAttachmentsEditable="message.author === env.messaging.currentPartner"
-                                    attachmentLocalIds="nonImageAttachments.map(attachment => attachment.localId)"
-                                    attachmentsDetailsMode="props.attachmentsDetailsMode"
-                                />
-                            </t>
+                            <AttachmentList
+                                class="o_Message_attachmentList"
+                                areAttachmentsDownloadable="true"
+                                areAttachmentsEditable="message.author === env.messaging.currentPartner"
+                                attachmentLocalIds="message.attachments.map(attachment => attachment.localId)"
+                                attachmentsDetailsMode="props.attachmentsDetailsMode"
+                            />
                         </div>
                     </t>
                 </div>