From 2a3bd05bacb18f59db8125aae5d13b7cefd93407 Mon Sep 17 00:00:00 2001
From: Lucas Perais <lpe@odoo.com>
Date: Mon, 17 Oct 2022 13:22:48 +0000
Subject: [PATCH] [FIX] web: form settings: keep scroll position per module app

Have more than one Odoo module installed. Go to the Settings.
Scroll the first page.
Change the app in the sidebar.

Before this commit, the scroll position was the same for all modules (or apps, same thing).
It is not wrong per se, but when changing app, there is no reason that the scroll position of the former
is somehow correct business-wise.

After this commit, the scroll position is stored and restored on a per-app basis.

closes odoo/odoo#103369

X-original-commit: 01b16c392b888ceaccc8b0c95da142dbb90517ef
Signed-off-by: Lucas Perais (lpe) <lpe@odoo.com>
---
 .../settings/settings_page.js                 | 20 +++++-
 .../settings/settings_page.xml                |  2 +-
 .../settings_form_view_tests.js               | 61 +++++++++++++++++++
 3 files changed, 81 insertions(+), 2 deletions(-)

diff --git a/addons/web/static/src/webclient/settings_form_view/settings/settings_page.js b/addons/web/static/src/webclient/settings_form_view/settings/settings_page.js
index b69069473b0..eb1cf876bf0 100644
--- a/addons/web/static/src/webclient/settings_form_view/settings/settings_page.js
+++ b/addons/web/static/src/webclient/settings_form_view/settings/settings_page.js
@@ -1,7 +1,7 @@
 /** @odoo-module **/
 import { ActionSwiper } from "@web/core/action_swiper/action_swiper";
 
-const { Component, useState } = owl;
+const { Component, useState, useRef, useEffect } = owl;
 
 export class SettingsPage extends Component {
     setup() {
@@ -13,6 +13,20 @@ export class SettingsPage extends Component {
         if (this.props.modules) {
             this.state.selectedTab = this.props.initialTab || this.props.modules[0].key;
         }
+
+        this.settingsRef = useRef("settings");
+        this.scrollMap = Object.create(null);
+        useEffect(
+            (settingsEl, currentTab) => {
+                if (!settingsEl) {
+                    return;
+                }
+
+                const { scrollTop } = this.scrollMap[currentTab] || 0;
+                settingsEl.scrollTop = scrollTop;
+            },
+            () => [this.settingsRef.el, this.state.selectedTab]
+        );
     }
 
     getCurrentIndex() {
@@ -41,6 +55,10 @@ export class SettingsPage extends Component {
     }
 
     onSettingTabClick(key) {
+        if (this.settingsRef.el) {
+            const { scrollTop } = this.settingsRef.el;
+            this.scrollMap[this.state.selectedTab] = { scrollTop };
+        }
         this.state.selectedTab = key;
         this.env.searchState.value = "";
     }
diff --git a/addons/web/static/src/webclient/settings_form_view/settings/settings_page.xml b/addons/web/static/src/webclient/settings_form_view/settings/settings_page.xml
index 26584c4ef0c..9336eb218d0 100644
--- a/addons/web/static/src/webclient/settings_form_view/settings/settings_page.xml
+++ b/addons/web/static/src/webclient/settings_form_view/settings/settings_page.xml
@@ -18,7 +18,7 @@
                 animationOnMove="false"
                 animationType="'forwards'"
                 swipeDistanceRatio="6">
-            <div class="settings">
+            <div class="settings" t-ref="settings">
                 <t t-slot="NoContentHelper" t-if="props.slots['NoContentHelper'].isVisible"/>
                 <t t-slot="default" selectedTab="state.selectedTab"/>
             </div>
diff --git a/addons/web/static/tests/webclient/settings_form_view/settings_form_view_tests.js b/addons/web/static/tests/webclient/settings_form_view/settings_form_view_tests.js
index 1c4f3d38142..d715da92c40 100644
--- a/addons/web/static/tests/webclient/settings_form_view/settings_form_view_tests.js
+++ b/addons/web/static/tests/webclient/settings_form_view/settings_form_view_tests.js
@@ -1526,4 +1526,65 @@ QUnit.module("SettingsFormView", (hooks) => {
             `focusin: <input type="text" class="o_searchview_input" accesskey="Q" placeholder="Search..." role="searchbox">`,
         ]);
     });
+
+    QUnit.test("settings form keeps scrolling by app", async (assert) => {
+        const oldHeight = target.style.getPropertyValue("height");
+        target.style.setProperty("height", "200px");
+        registerCleanup(() => {
+            target.style.setProperty("height", oldHeight);
+        });
+
+        await makeView({
+            type: "form",
+            resModel: "res.config.settings",
+            serverData,
+            arch: `
+            <form string="Settings" class="oe_form_configuration o_base_settings" js_class="base_settings">
+            <div class="o_setting_container">
+                <div class="settings">
+                    <div class="app_settings_block" string="CRM" data-key="crm">
+                        <h2>Title of group Bar</h2>
+                        <div class="row mt16 o_settings_container">
+                                <br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br />
+                                <br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br />
+                                <br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br />
+                                <div id="deepDivCrm" />
+                        </div>
+                    </div>
+
+                    <div class="app_settings_block" string="OtherApp" data-key="otherapp">
+                        <h2>Title of group Other</h2>
+                        <div class="row mt16 o_settings_container">
+                            <div class="col-12 col-lg-6 o_setting_box">
+                                <br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br />
+                                <br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br />
+                                <br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br />
+                                <div id="deepDivOther" />
+                            </div>
+                        </div>
+                    </div>
+                </div>
+            </div>
+        </form>`,
+        });
+
+        // constrain o_content to have height for its children to be scrollable
+        target.querySelector(".o_content").style.setProperty("height", "200px");
+
+        const scrollingEl = target.querySelector(".settings");
+        assert.strictEqual(scrollingEl.scrollTop, 0);
+
+        await click(target.querySelector(".settings_tab [data-key='otherapp']"));
+        assert.strictEqual(scrollingEl.scrollTop, 0);
+        target.querySelector("#deepDivOther").scrollIntoView();
+
+        const scrollTop = scrollingEl.scrollTop;
+        assert.ok(scrollTop > 0);
+
+        await click(target.querySelector(".settings_tab [data-key='crm']"));
+        assert.strictEqual(scrollingEl.scrollTop, 0);
+
+        await click(target.querySelector(".settings_tab [data-key='otherapp']"));
+        assert.strictEqual(scrollingEl.scrollTop, scrollTop);
+    });
 });