From 652d835f77da3e19eeb70a4a2fec46fd9bff8b46 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Theys?= <seb@odoo.com>
Date: Tue, 23 Jun 2020 17:03:08 +0000
Subject: [PATCH] [FIX] mail: restore close of other menus on click of
 messaging menu
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

task-2278830

closes odoo/odoo#53566

X-original-commit: 0407151b34da16ca54066bb15e578dc1db85f365
Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>
---
 .../messaging_menu/messaging_menu.js          | 10 ++--
 .../messaging_menu/messaging_menu.scss        |  2 +-
 .../messaging_menu/messaging_menu.xml         |  4 +-
 .../messaging_menu/messaging_menu_tests.js    | 18 +++---
 .../mail/static/src/models/thread/thread.js   | 12 ----
 .../systray/systray_activity_menu_tests.js    | 56 ++++++++++++++++---
 6 files changed, 67 insertions(+), 35 deletions(-)

diff --git a/addons/mail/static/src/components/messaging_menu/messaging_menu.js b/addons/mail/static/src/components/messaging_menu/messaging_menu.js
index 864c7df97e1..b4647d698cc 100644
--- a/addons/mail/static/src/components/messaging_menu/messaging_menu.js
+++ b/addons/mail/static/src/components/messaging_menu/messaging_menu.js
@@ -113,6 +113,8 @@ class MessagingMenu extends Component {
     //--------------------------------------------------------------------------
 
     /**
+     * Closes the menu when clicking outside, if appropriate.
+     *
      * @private
      * @param {MouseEvent} ev
      */
@@ -126,7 +128,7 @@ class MessagingMenu extends Component {
         ) {
             return;
         }
-        // closes the menu when clicking outside
+        // ignore click inside the menu
         if (this.el.contains(ev.target)) {
             return;
         }
@@ -142,7 +144,6 @@ class MessagingMenu extends Component {
      * @param {MouseEvent} ev
      */
     _onClickDesktopTabButton(ev) {
-        ev.stopPropagation();
         this.messagingMenu.update({ activeTabId: ev.currentTarget.dataset.tabId });
     }
 
@@ -151,9 +152,8 @@ class MessagingMenu extends Component {
      * @param {MouseEvent} ev
      */
     _onClickNewMessage(ev) {
-        ev.stopPropagation();
         if (!this.env.messaging.device.isMobile) {
-            this.env.models['mail.thread'].openNewMessage();
+            this.env.messaging.chatWindowManager.openNewMessage();
             this.messagingMenu.close();
         } else {
             this.messagingMenu.toggleMobileNewMessage();
@@ -165,7 +165,7 @@ class MessagingMenu extends Component {
      * @param {MouseEvent} ev
      */
     _onClickToggler(ev) {
-        ev.stopPropagation();
+        // avoid following dummy href
         ev.preventDefault();
         this.messagingMenu.toggleOpen();
     }
diff --git a/addons/mail/static/src/components/messaging_menu/messaging_menu.scss b/addons/mail/static/src/components/messaging_menu/messaging_menu.scss
index 54855c15c13..1aaea9db5fb 100644
--- a/addons/mail/static/src/components/messaging_menu/messaging_menu.scss
+++ b/addons/mail/static/src/components/messaging_menu/messaging_menu.scss
@@ -99,7 +99,7 @@
 // ------------------------------------------------------------------
 
 // Make hightlight more consistent, due to messaging menu looking quite similar to discuss app in mobile
-.o_MessagingMenu.show {
+.o_MessagingMenu.o-is-open {
     background-color: rgba(black, 0.1);
 }
 
diff --git a/addons/mail/static/src/components/messaging_menu/messaging_menu.xml b/addons/mail/static/src/components/messaging_menu/messaging_menu.xml
index db1c211f858..0a6096ee984 100644
--- a/addons/mail/static/src/components/messaging_menu/messaging_menu.xml
+++ b/addons/mail/static/src/components/messaging_menu/messaging_menu.xml
@@ -2,10 +2,10 @@
 <templates xml:space="preserve">
 
     <t t-name="mail.MessagingMenu" owl="1">
-        <li class="o_MessagingMenu" t-att-class="{ show: messagingMenu.isOpen, 'o-mobile': env.messaging.device.isMobile }">
+        <li class="o_MessagingMenu" t-att-class="{ 'o-is-open': messagingMenu.isOpen, 'o-mobile': env.messaging.device.isMobile }">
             <!-- TODO FIXME restore mobile stuff here (activeMobileThread) -->
             <t t-if="!env.messaging.device.isMobile or true">
-                <a class="o_MessagingMenu_toggler o-no-caret" t-att-class="{ show: messagingMenu.isOpen, 'o-no-notification': !messagingMenu.counter }" href="#" t-on-click="_onClickToggler" data-toggle="dropdown" data-display="static" title="Conversations" role="button" aria-expanded="false">
+                <a class="o_MessagingMenu_toggler" t-att-class="{ 'o-no-notification': !messagingMenu.counter }" href="#" title="Conversations" role="button" t-att-aria-expanded="messagingMenu.isOpen ? 'true' : 'false'" aria-haspopup="true" t-on-click="_onClickToggler">
                     <i class="o_MessagingMenu_icon fa fa-comments" role="img" aria-label="Messages"/>
                     <t t-if="!env.messaging.isInitialized">
                         <i class="o_MessagingMenu_loading fa fa-spinner fa-spin"/>
diff --git a/addons/mail/static/src/components/messaging_menu/messaging_menu_tests.js b/addons/mail/static/src/components/messaging_menu/messaging_menu_tests.js
index e2ccaab78ba..4f070d7881c 100644
--- a/addons/mail/static/src/components/messaging_menu/messaging_menu_tests.js
+++ b/addons/mail/static/src/components/messaging_menu/messaging_menu_tests.js
@@ -140,13 +140,10 @@ QUnit.test('basic rendering', async function (assert) {
     );
 
     await afterNextRender(() => document.querySelector(`.o_MessagingMenu_toggler`).click());
-    assert.ok(
-        document.querySelector('.o_MessagingMenu').classList.contains('show'),
-        "should mark messaging menu item as shown"
-    );
-    assert.ok(
-        document.querySelector(`.o_MessagingMenu_toggler`).classList.contains('show'),
-        "should mark messaging menu clickable item as shown"
+    assert.hasClass(
+        document.querySelector('.o_MessagingMenu'),
+        "o-is-open",
+        "should mark messaging menu as opened"
     );
     assert.strictEqual(
         document.querySelectorAll(`.o_MessagingMenu_dropdownMenu`).length,
@@ -220,6 +217,13 @@ QUnit.test('basic rendering', async function (assert) {
         1,
         "should display no conversation in thread preview list"
     );
+
+    await afterNextRender(() => document.querySelector(`.o_MessagingMenu_toggler`).click());
+    assert.doesNotHaveClass(
+        document.querySelector('.o_MessagingMenu'),
+        "o-is-open",
+        "should mark messaging menu as closed"
+    );
 });
 
 QUnit.test('counter is taking into account failure notification', async function (assert) {
diff --git a/addons/mail/static/src/models/thread/thread.js b/addons/mail/static/src/models/thread/thread.js
index 210fa0cafd1..06c41652ff8 100644
--- a/addons/mail/static/src/models/thread/thread.js
+++ b/addons/mail/static/src/models/thread/thread.js
@@ -366,18 +366,6 @@ function factory(dependencies) {
             }
         }
 
-        /**
-         * @static
-         */
-        static openNewMessage() {
-            const discuss = this.env.messaging.discuss;
-            if (discuss.isOpen) {
-                discuss.openNewMessage();
-            } else {
-                this.env.messaging.chatWindowManager.openNewMessage();
-            }
-        }
-
         /**
          * @param {string} [stringifiedDomain='[]']
          * @returns {mail.thread_cache}
diff --git a/addons/mail/static/tests/systray/systray_activity_menu_tests.js b/addons/mail/static/tests/systray/systray_activity_menu_tests.js
index 68602e237e7..0258b35652f 100644
--- a/addons/mail/static/tests/systray/systray_activity_menu_tests.js
+++ b/addons/mail/static/tests/systray/systray_activity_menu_tests.js
@@ -1,7 +1,10 @@
 odoo.define('mail.systray.ActivityMenuTests', function (require) {
 "use strict";
 
-const { start } = require('mail/static/src/utils/test_utils.js');
+const {
+    afterNextRender,
+    start,
+} = require('mail/static/src/utils/test_utils.js');
 var ActivityMenu = require('mail.systray.ActivityMenu');
 
 var testUtils = require('web.test_utils');
@@ -15,10 +18,10 @@ QUnit.module('ActivityMenu', {
                     name: { type: "char" },
                     model: { type: "char" },
                     type: { type: "char" },
-                    planned_count: { type: "integer"},
-                    today_count: { type: "integer"},
-                    overdue_count: { type: "integer"},
-                    total_count: { type: "integer"},
+                    planned_count: { type: "integer" },
+                    today_count: { type: "integer" },
+                    overdue_count: { type: "integer" },
+                    total_count: { type: "integer" },
                     actions: [{
                         icon: { type: "char" },
                         name: { type: "char" },
@@ -181,17 +184,17 @@ QUnit.test('activity menu widget: activity view icon', async function (assert) {
     var $second = activityMenu.$('.o_mail_activity_action').eq(1);
     assert.strictEqual($first.data('model_name'), "Issue",
                        "first activity action should link to 'Issue'");
-    assert.hasClass($first,'fa-clock-o', "should display the activity action icon");
+    assert.hasClass($first, 'fa-clock-o', "should display the activity action icon");
 
     assert.strictEqual($second.data('model_name'), "Note",
                        "Second activity action should link to 'Note'");
-    assert.hasClass($second,'fa-clock-o', "should display the activity action icon");
+    assert.hasClass($second, 'fa-clock-o', "should display the activity action icon");
 
     testUtils.mock.intercept(activityMenu, 'do_action', function (ev) {
         if (ev.data.action.name) {
             assert.ok(ev.data.action.domain, "should define a domain on the action");
             assert.deepEqual(ev.data.action.domain, [["activity_ids.user_id", "=", 10]],
-                "should set domain to user's activity only")
+                "should set domain to user's activity only");
             assert.step('do_action:' + ev.data.action.name);
         } else {
             assert.step('do_action:' + ev.data.action);
@@ -218,5 +221,42 @@ QUnit.test('activity menu widget: activity view icon', async function (assert) {
 
     widget.destroy();
 });
+
+QUnit.test('activity menu widget: close on messaging menu click', async function (assert) {
+    assert.expect(2);
+
+    const { widget } = await start({
+        hasMessagingMenu: true,
+        async mockRPC(route, args) {
+            if (args.method === 'systray_get_activities') {
+                return [];
+            }
+            return this._super(route, args);
+        },
+    });
+    const activityMenu = new ActivityMenu(widget);
+    await activityMenu.appendTo($('#qunit-fixture'));
+    await testUtils.nextTick();
+
+    await testUtils.dom.click(activityMenu.$('.dropdown-toggle'));
+    assert.hasClass(
+        activityMenu.el.querySelector('.o_mail_systray_dropdown'),
+        'show',
+        "activity menu should be shown after click on itself"
+    );
+
+    await afterNextRender(() =>
+        document.querySelector(`.o_MessagingMenu_toggler`).click()
+    );
+    assert.doesNotHaveClass(
+        activityMenu.el.querySelector('.o_mail_systray_dropdown'),
+        'show',
+        "activity menu should be hidden after click on messaging menu"
+    );
+
+    widget.destroy();
 });
+
+});
+
 });