From 709d2e8a7a7dbbc959b8a4f6b29a13fee1c07196 Mon Sep 17 00:00:00 2001
From: Julien Mougenot <jum@odoo.com>
Date: Tue, 26 May 2020 09:45:25 +0000
Subject: [PATCH] [IMP] account,web: SearchPanel: improve folding behaviour

1) Clicks on a section value are now bound to the entire header, meaning
that the sole purpose of the carets is purely esthetic.
2) Search panel categories will now only be folded if they are being
clicked on while already selected.

Classes that were no longer useful have been removed and tests have been
updated accordingly.

Task 2264700

closes odoo/odoo#51555

Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
---
 .../static/src/scss/account_searchpanel.scss  |  2 +-
 .../web/static/src/js/views/search_panel.js   | 19 ++-------
 addons/web/static/src/scss/search_panel.scss  |  2 +-
 addons/web/static/src/xml/kanban.xml          |  6 +--
 .../static/tests/views/search_panel_tests.js  | 42 ++++++++++++-------
 5 files changed, 34 insertions(+), 37 deletions(-)

diff --git a/addons/account/static/src/scss/account_searchpanel.scss b/addons/account/static/src/scss/account_searchpanel.scss
index b8b04a11a9e..621a283a581 100644
--- a/addons/account/static/src/scss/account_searchpanel.scss
+++ b/addons/account/static/src/scss/account_searchpanel.scss
@@ -13,7 +13,7 @@
             margin-left: 0;
             padding-left: 0;
         }
-        .o_search_panel_category_value .o_toggle_fold_container {
+        .o_search_panel_category_value .o_toggle_fold {
             width: 0.3rem;
         }
     }
diff --git a/addons/web/static/src/js/views/search_panel.js b/addons/web/static/src/js/views/search_panel.js
index eba680627f2..38019fc01cd 100644
--- a/addons/web/static/src/js/views/search_panel.js
+++ b/addons/web/static/src/js/views/search_panel.js
@@ -67,7 +67,6 @@ const SearchPanel = Widget.extend({
     className: 'o_search_panel',
     events: {
         'click .o_search_panel_category_value header': '_onCategoryValueClicked',
-        'click .o_search_panel_category_value .o_toggle_fold_container': '_onToggleFoldCategory',
         'change .o_search_panel_filter_value > header > div > input': '_onFilterValueChanged',
         'change .o_search_panel_filter_group > header > div > input': '_onFilterGroupChanged',
     },
@@ -726,8 +725,9 @@ const SearchPanel = Widget.extend({
             item.dataset.id || false;
         const hasChanged = category.activeValueId !== valueId;
         category.activeValueId = valueId;
-        if (category.values[valueId]) {
-            category.values[valueId].folded = !category.values[valueId].folded;
+        const value = category.values[category.activeValueId];
+        if (value) {
+            value.folded = value.folded ? false : !hasChanged;
         }
         if (hasChanged) {
             const storageKey = this._getLocalStorageKey(category);
@@ -769,19 +769,6 @@ const SearchPanel = Widget.extend({
         }
         this._notifyDomainUpdated();
     },
-    /**
-     * @private
-     * @param {MouseEvent} ev
-     */
-    _onToggleFoldCategory(ev) {
-        ev.preventDefault();
-        ev.stopPropagation();
-        const item = ev.currentTarget.closest('.o_search_panel_category_value');
-        const category = this.categories[item.dataset.categoryId];
-        const valueId = item.dataset.id;
-        category.values[valueId].folded = !category.values[valueId].folded;
-        this._render();
-    },
 });
 
 return SearchPanel;
diff --git a/addons/web/static/src/scss/search_panel.scss b/addons/web/static/src/scss/search_panel.scss
index a3cf4b319c3..c40686804ea 100644
--- a/addons/web/static/src/scss/search_panel.scss
+++ b/addons/web/static/src/scss/search_panel.scss
@@ -32,7 +32,7 @@ $o-searchpanel-filter-default-color: #D59244;
             color: $o-searchpanel-filter-default-color;
         }
 
-        .o_toggle_fold_container {
+        .o_toggle_fold {
             text-align: center;
             width: 1.5rem;
         }
diff --git a/addons/web/static/src/xml/kanban.xml b/addons/web/static/src/xml/kanban.xml
index 980decc1a8e..374ddf73353 100644
--- a/addons/web/static/src/xml/kanban.xml
+++ b/addons/web/static/src/xml/kanban.xml
@@ -226,7 +226,7 @@
             <li class="o_search_panel_category_value border-0 list-group-item" t-att-data-category-id="category.id">
                 <header t-att-class="'list-group-item-action' + (!category.activeValueId ? ' active' : '')">
                     <label class="o_search_panel_label mb0">
-                        <div class="o_toggle_fold_container"/>
+                        <div class="o_toggle_fold"/>
                         <span class="o_search_panel_label_title"><b>All</b></span>
                     </label>
                 </header>
@@ -247,8 +247,8 @@
         <li class="o_search_panel_category_value border-0 list-group-item" t-att-data-id="value.id" t-att-data-category-id="category.id">
             <header t-att-class="'list-group-item-action' + (value.id === category.activeValueId ? ' active' : '')">
                 <label t-attf-class="o_search_panel_label mb0{{ category.disableCounters ? '' : ' o_with_counters' }}">
-                    <div class="o_toggle_fold_container">
-                        <i t-if="value.childrenIds.length" t-attf-class="o_toggle_fold fa fa-caret-{{ value.folded ? 'right' : 'down' }}"/>
+                    <div class="o_toggle_fold">
+                        <i t-if="value.childrenIds.length" t-attf-class="fa fa-caret-{{ value.folded ? 'right' : 'down' }}"/>
                     </div>
                     <span class="o_search_panel_label_title"><t t-esc="value.display_name"/></span>
                 </label>
diff --git a/addons/web/static/tests/views/search_panel_tests.js b/addons/web/static/tests/views/search_panel_tests.js
index d445d9b6a96..62fbbfbad84 100644
--- a/addons/web/static/tests/views/search_panel_tests.js
+++ b/addons/web/static/tests/views/search_panel_tests.js
@@ -625,7 +625,7 @@ QUnit.module('Views', {
         assert.containsN(kanban, '.o_search_panel_category_value', 2,
             'The number of categories should be 2: All and Company 5');
 
-        assert.containsNone(kanban, '.o_toggle_fold',
+        assert.containsNone(kanban, '.o_toggle_fold > i',
             'None of the categories should have children');
 
         kanban.destroy();
@@ -703,7 +703,7 @@ QUnit.module('Views', {
     });
 
     QUnit.test('category with parent_field', async function (assert) {
-        assert.expect(28);
+        assert.expect(33);
 
         this.data.company.records.push({id: 40, name: 'child company 1', parent_id: 5});
         this.data.company.records.push({id: 41, name: 'child company 2', parent_id: 5});
@@ -740,10 +740,11 @@ QUnit.module('Views', {
         assert.containsOnce(kanban, '.o_search_panel_category_value:first .active');
         assert.containsN(kanban, '.o_kanban_record:not(.o_kanban_ghost)', 4);
         assert.containsN(kanban, '.o_search_panel_category_value', 3);
-        assert.containsOnce(kanban, '.o_search_panel_category_value .o_toggle_fold');
+        assert.containsOnce(kanban, '.o_search_panel_category_value .o_toggle_fold > i');
 
-        // unfold parent category
-        await testUtils.dom.click(kanban.$('.o_search_panel_category_value .o_toggle_fold'));
+        // unfold parent category and select 'All' again
+        await testUtils.dom.click(kanban.$('.o_search_panel_category_value:nth(2) > header'));
+        await testUtils.dom.click(kanban.$('.o_search_panel_category_value:first > header'));
 
         assert.containsOnce(kanban, '.o_search_panel_category_value .active');
         assert.containsOnce(kanban, '.o_search_panel_category_value:first .active');
@@ -761,6 +762,13 @@ QUnit.module('Views', {
         // click on parent company
         await testUtils.dom.click(kanban.$('.o_search_panel_category_value:nth(2) > header'));
 
+        assert.containsOnce(kanban, '.o_search_panel_category_value .active');
+        assert.containsOnce(kanban, '.o_search_panel_category_value:nth(2) .active');
+        assert.containsOnce(kanban, '.o_kanban_record:not(.o_kanban_ghost)');
+
+        // fold parent company by clicking on it
+        await testUtils.dom.click(kanban.$('.o_search_panel_category_value:nth(2) > header'));
+
         assert.containsOnce(kanban, '.o_search_panel_category_value .active');
         assert.containsOnce(kanban, '.o_search_panel_category_value:nth(2) .active');
         assert.containsN(kanban, '.o_kanban_record:not(.o_kanban_ghost)', 1);
@@ -772,8 +780,8 @@ QUnit.module('Views', {
         assert.containsN(kanban, '.o_kanban_record:not(.o_kanban_ghost)', 1);
 
         // fold category with children
-        await testUtils.dom.click(kanban.$('.o_search_panel_category_value .o_toggle_fold'));
-        await testUtils.dom.click(kanban.$('.o_search_panel_category_value .o_toggle_fold'));
+        await testUtils.dom.click(kanban.$('.o_search_panel_category_value:nth(2) > header'));
+        await testUtils.dom.click(kanban.$('.o_search_panel_category_value:nth(2) > header'));
 
         assert.containsOnce(kanban, '.o_search_panel_category_value .active');
         assert.containsOnce(kanban, '.o_search_panel_category_value:nth(2) .active');
@@ -781,6 +789,8 @@ QUnit.module('Views', {
         assert.containsN(kanban, '.o_kanban_record:not(.o_kanban_ghost)', 1);
 
         assert.verifySteps([
+            '[]',
+            '[["company_id","child_of",5]]',
             '[]',
             '[["company_id","child_of",40]]',
             '[["company_id","child_of",5]]',
@@ -866,21 +876,21 @@ QUnit.module('Views', {
             },
         });
 
-        assert.strictEqual(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold').length, 1,
+        assert.strictEqual(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold > i').length, 1,
             "'agrolait' should be displayed as a parent category value");
-        assert.hasClass(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold'), 'fa-caret-right',
+        assert.hasClass(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold > i'), 'fa-caret-right',
             "'agrolait' should be folded");
         assert.containsN(kanban, '.o_search_panel_category_value', 3);
 
         // unfold agrolait
-        await testUtils.dom.click(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold'));
-        assert.hasClass(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold'), 'fa-caret-down',
+        await testUtils.dom.click(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold > i'));
+        assert.hasClass(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold > i'), 'fa-caret-down',
             "'agrolait' should be open");
         assert.containsN(kanban, '.o_search_panel_category_value', 5);
 
         // fold agrolait
-        await testUtils.dom.click(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold'));
-        assert.hasClass(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold'), 'fa-caret-right',
+        await testUtils.dom.click(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold > i'));
+        assert.hasClass(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold > i'), 'fa-caret-right',
             "'agrolait' should be folded");
         assert.containsN(kanban, '.o_search_panel_category_value', 3);
 
@@ -915,14 +925,14 @@ QUnit.module('Views', {
         });
 
         // unfold agrolait
-        await testUtils.dom.click(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold'));
-        assert.hasClass(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold'), 'fa-caret-down',
+        await testUtils.dom.click(kanban.$('.o_search_panel_category_value:contains(agrolait) > header'));
+        assert.hasClass(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold > i'), 'fa-caret-down',
             "'agrolait' should be open");
         assert.containsN(kanban, '.o_search_panel_category_value', 5);
 
         await kanban.reload({});
 
-        assert.hasClass(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold'), 'fa-caret-down',
+        assert.hasClass(kanban.$('.o_search_panel_category_value:contains(agrolait) .o_toggle_fold > i'), 'fa-caret-down',
             "'agrolait' should be open");
         assert.containsN(kanban, '.o_search_panel_category_value', 5);