From d30993adbbe108f9e39f456e05da12ed0f394b14 Mon Sep 17 00:00:00 2001
From: Lucas Perais <lpe@odoo.com>
Date: Mon, 8 Aug 2022 16:03:15 +0000
Subject: [PATCH] [FIX] web: Notebook: compute defaultPage when isVisible
 dynamic

Have a form view with a notebook.
In the notebook a page has autofocus, and is also invisible in some
circumstances.

Before this commit, the default page was always displayed, even if invisible,
its anchor was not selected though.

After this commit:
- if the default page is visible, display it.
- if the default page is not visible, select the first visible page
- if the default page was visible and the page has changed, don't select the default page at next render
- if the default page was not visible and becomes visible, select it at next render

closes odoo/odoo#97742

Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
---
 .../web/static/src/core/notebook/notebook.js  | 28 +++++++---
 .../web/static/tests/core/notebook_tests.js   | 55 +++++++++++++++++--
 2 files changed, 70 insertions(+), 13 deletions(-)

diff --git a/addons/web/static/src/core/notebook/notebook.js b/addons/web/static/src/core/notebook/notebook.js
index 1da99e393b7..3445022d3e7 100644
--- a/addons/web/static/src/core/notebook/notebook.js
+++ b/addons/web/static/src/core/notebook/notebook.js
@@ -59,7 +59,7 @@ export class Notebook extends Component {
         this.anchorTarget = null;
         this.pages = this.computePages(this.props);
         this.state = useState({ currentPage: null });
-        this.state.currentPage = this.props.defaultPage || this.computeActivePage();
+        this.state.currentPage = this.computeActivePage(this.props.defaultPage, true);
         const onAnchorClicked = this.onAnchorClicked.bind(this);
         this.env.bus.addEventListener("SCROLLER:ANCHOR_LINK_CLICKED", onAnchorClicked);
         useEffect(
@@ -74,8 +74,10 @@ export class Notebook extends Component {
             () => [this.state.currentPage]
         );
         onWillUpdateProps((nextProps) => {
+            const activateDefault =
+                this.props.defaultPage !== nextProps.defaultPage || !this.defaultVisible;
             this.pages = this.computePages(nextProps);
-            this.state.currentPage = this.computeActivePage();
+            this.state.currentPage = this.computeActivePage(nextProps.defaultPage, activateDefault);
         });
         onWillDestroy(() => {
             this.env.bus.removeEventListener("SCROLLER:ANCHOR_LINK_CLICKED", onAnchorClicked);
@@ -131,15 +133,27 @@ export class Notebook extends Component {
         return pages;
     }
 
-    computeActivePage() {
+    computeActivePage(defaultPage, activateDefault) {
         if (!this.pages.length) {
             return null;
         }
-        const current = this.state.currentPage;
-        if (!current || (current && !this.pages.find((e) => e[0] === current)[1].isVisible)) {
-            const candidate = this.pages.find((v) => v[1].isVisible);
-            return candidate ? candidate[0] : null;
+        const pages = this.pages.filter((e) => e[1].isVisible).map((e) => e[0]);
+
+        if (defaultPage) {
+            if (!pages.includes(defaultPage)) {
+                this.defaultVisible = false;
+            } else {
+                this.defaultVisible = true;
+                if (activateDefault) {
+                    return defaultPage;
+                }
+            }
         }
+        const current = this.state.currentPage;
+        if (!current || (current && !pages.includes(current))) {
+            return pages[0];
+        }
+
         return current;
     }
 }
diff --git a/addons/web/static/tests/core/notebook_tests.js b/addons/web/static/tests/core/notebook_tests.js
index b71c3058ec3..ab3043f8f78 100644
--- a/addons/web/static/tests/core/notebook_tests.js
+++ b/addons/web/static/tests/core/notebook_tests.js
@@ -2,7 +2,7 @@
 
 import { Notebook } from "@web/core/notebook/notebook";
 import { makeTestEnv } from "@web/../tests/helpers/mock_env";
-import { click, getFixture, mount } from "@web/../tests/helpers/utils";
+import { click, getFixture, mount, nextTick } from "@web/../tests/helpers/utils";
 
 const { Component, xml } = owl;
 let target;
@@ -139,11 +139,12 @@ QUnit.module("Components", (hooks) => {
 
         await mount(Parent, target, { env });
         assert.containsOnce(target, "div.o_notebook");
-        assert.containsNone(
-            target,
-            ".o_notebook_headers .nav-item a.active",
-            "no tab is selected as the default page is not visible"
+        assert.strictEqual(
+            target.querySelector(".o_notebook_headers .nav-item a.active").textContent,
+            "About",
+            "The first page is selected"
         );
+
         assert.containsN(
             target,
             ".o_notebook_headers a.nav-link",
@@ -152,7 +153,7 @@ QUnit.module("Components", (hooks) => {
         );
         assert.strictEqual(
             target.querySelector(".tab-pane.active").firstElementChild.textContent,
-            "Oooops",
+            "About the bird",
             "third page content is displayed by the notebook"
         );
     });
@@ -291,4 +292,46 @@ QUnit.module("Components", (hooks) => {
 
         assert.notEqual(firstPage, secondPage);
     });
+
+    QUnit.test("defaultPage recomputed when isVisible is dynamic", async (assert) => {
+        let defaultPageVisible = false;
+        class Parent extends Component {
+            get defaultPageVisible() {
+                return defaultPageVisible;
+            }
+        }
+        Parent.components = { Notebook };
+        Parent.template = xml`
+        <Notebook defaultPage="'3'">
+            <t t-set-slot="1" title="'page1'" isVisible="true">
+                <div class="page1" />
+            </t>
+             <t t-set-slot="2" title="'page2'" isVisible="true">
+                <div class="page2" />
+            </t>
+             <t t-set-slot="3" title="'page3'" isVisible="defaultPageVisible">
+                <div class="page3" />
+            </t>
+        </Notebook>`;
+
+        const env = await makeTestEnv();
+        const parent = await mount(Parent, target, { env });
+        assert.containsOnce(target, ".page1");
+        assert.strictEqual(target.querySelector(".nav-link.active").textContent, "page1");
+
+        defaultPageVisible = true;
+        parent.render(true);
+        await nextTick();
+        assert.containsOnce(target, ".page3");
+        assert.strictEqual(target.querySelector(".nav-link.active").textContent, "page3");
+
+        await click(target.querySelectorAll(".nav-link")[1]);
+        assert.containsOnce(target, ".page2");
+        assert.strictEqual(target.querySelector(".nav-link.active").textContent, "page2");
+
+        parent.render(true);
+        await nextTick();
+        assert.containsOnce(target, ".page2");
+        assert.strictEqual(target.querySelector(".nav-link.active").textContent, "page2");
+    });
 });