From 4308de6aab93630c2c24a2d1d2ffd2db4e1d4b6b Mon Sep 17 00:00:00 2001
From: Mathieu Duckerts-Antoine <dam@odoo.com>
Date: Wed, 9 Feb 2022 14:35:04 +0000
Subject: [PATCH] [REF] web: useAutofocus simplified API

We bring two changes to the API of useAutofocus:
    - it takes no parameter, the element focused is the one that is the
      target of a t-ref="autofocus".
    - it returns the element reference instead of a function to call if
      one wants to force the focus on a next patch.

Indeed, having access to the element reference makes it easy to focus
that element but also allow to remove an eventual useRef that would
target the same element. This in turn permits the use of t-ref="autofocus"
alone.

closes odoo/odoo#84306

Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
---
 addons/web/static/src/core/pager/pager.js     | 11 +++++-----
 addons/web/static/src/core/pager/pager.xml    |  2 +-
 addons/web/static/src/core/utils/hooks.js     | 20 ++++++++-----------
 .../src/legacy/js/components/datepicker.js    |  5 +----
 .../js/control_panel/custom_favorite_item.js  |  9 +++------
 .../src/legacy/js/control_panel/search_bar.js | 11 +++++-----
 addons/web/static/src/legacy/xml/base.xml     |  4 ++--
 .../favorite_menu/custom_favorite_item.js     |  5 ++---
 .../favorite_menu/custom_favorite_item.xml    |  2 +-
 .../src/search/search_bar/search_bar.js       | 10 ++++------
 .../src/search/search_bar/search_bar.xml      |  2 +-
 .../static/tests/core/utils/hooks_tests.js    | 16 ++++++---------
 12 files changed, 40 insertions(+), 57 deletions(-)

diff --git a/addons/web/static/src/core/pager/pager.js b/addons/web/static/src/core/pager/pager.js
index d562af39d64..108c347332b 100644
--- a/addons/web/static/src/core/pager/pager.js
+++ b/addons/web/static/src/core/pager/pager.js
@@ -2,7 +2,7 @@
 
 import { useAutofocus } from "../utils/hooks";
 
-const { Component, useExternalListener, useRef, useState } = owl;
+const { Component, useExternalListener, useState } = owl;
 
 /**
  * Pager
@@ -23,11 +23,8 @@ export class Pager extends Component {
             isEditing: false,
             isDisabled: false,
         });
-
-        this.inputRef = useRef("input");
+        this.inputRef = useAutofocus();
         useExternalListener(document, "mousedown", this.onClickAway, { capture: true });
-
-        this.forceFocus = useAutofocus("input");
     }
 
     /**
@@ -146,7 +143,9 @@ export class Pager extends Component {
     }
     onValueClick() {
         if (this.props.isEditable && !this.state.isEditing && !this.state.isDisabled) {
-            this.forceFocus();
+            if (this.inputRef.el) {
+                this.inputRef.el.focus();
+            }
             this.state.isEditing = true;
         }
     }
diff --git a/addons/web/static/src/core/pager/pager.xml b/addons/web/static/src/core/pager/pager.xml
index 9745a3d04bd..a11dea6423b 100644
--- a/addons/web/static/src/core/pager/pager.xml
+++ b/addons/web/static/src/core/pager/pager.xml
@@ -5,7 +5,7 @@
         <nav class="o_pager" aria-label="Pager">
             <span class="o_pager_counter mr-2" t-on-click.stop="">
                 <t t-if="state.isEditing">
-                    <input type="text" class="o_pager_value o_input" t-ref="input" t-att-value="value" t-on-blur="onInputBlur" t-on-change="onInputChange" t-on-keydown.stop="onInputKeydown" />
+                    <input type="text" class="o_pager_value o_input" t-ref="autofocus" t-att-value="value" t-on-blur="onInputBlur" t-on-change="onInputChange" t-on-keydown.stop="onInputKeydown" />
                 </t>
                 <t t-else="">
                     <span class="o_pager_value" t-esc="value" t-on-click="onValueClick" />
diff --git a/addons/web/static/src/core/utils/hooks.js b/addons/web/static/src/core/utils/hooks.js
index 2e51adb1f14..f2dd5c0a6f8 100644
--- a/addons/web/static/src/core/utils/hooks.js
+++ b/addons/web/static/src/core/utils/hooks.js
@@ -24,13 +24,12 @@ const { status, useComponent, useEffect, useRef } = owl;
 // -----------------------------------------------------------------------------
 
 /**
- * Focus a dom element with provided ref as soon as it appears in the DOM and if it was not
- * displayed before. If it is an input|textarea, set the selection
- * at the end.
- * @param {string} [refName="autofocus"]
- * @returns {Function} function that forces the focus on the next update if visible.
+ * Focus an element referenced by a t-ref="autofocus" in the current component
+ * as soon as it appears in the DOM and if it was not displayed before.
+ * If it is an input/textarea, set the selection at the end.
+ * @returns {Object} the element reference
  */
-export function useAutofocus(refName = "autofocus") {
+export function useAutofocus() {
     const comp = useComponent();
     // Prevent autofocus in mobile
     if (comp.env.isSmall) {
@@ -41,8 +40,7 @@ export function useAutofocus(refName = "autofocus") {
         return () => {};
     }
     // LEGACY
-    let ref = useRef(refName);
-    let forceFocusCount = 0;
+    let ref = useRef("autofocus");
     useEffect(
         (el) => {
             if (el) {
@@ -52,11 +50,9 @@ export function useAutofocus(refName = "autofocus") {
                 }
             }
         },
-        () => [ref.el, forceFocusCount]
+        () => [ref.el]
     );
-    return function focusOnUpdate() {
-        forceFocusCount++; // force the effect to rerun on next patch
-    };
+    return ref;
 }
 
 // -----------------------------------------------------------------------------
diff --git a/addons/web/static/src/legacy/js/components/datepicker.js b/addons/web/static/src/legacy/js/components/datepicker.js
index cbb63db8098..5dd74f6b518 100644
--- a/addons/web/static/src/legacy/js/components/datepicker.js
+++ b/addons/web/static/src/legacy/js/components/datepicker.js
@@ -12,7 +12,6 @@ odoo.define('web.DatePickerOwl', function (require) {
         onWillUnmount,
         onWillUpdateProps,
         useExternalListener,
-        useRef,
         useState,
     } = owl;
 
@@ -38,11 +37,9 @@ odoo.define('web.DatePickerOwl', function (require) {
             this.datePickerId = `o_datepicker_${datePickerId++}`;
             this.typeOfDate = 'date';
 
-            useAutofocus("input");
+            this.inputRef = useAutofocus();
             useExternalListener(window, 'scroll', this._onWindowScroll);
 
-            this.inputRef = useRef('input');
-
             onMounted(() => {
                 $(this.el).on('show.datetimepicker', this._onDateTimePickerShow.bind(this));
                 $(this.el).on('hide.datetimepicker', this._onDateTimePickerHide.bind(this));
diff --git a/addons/web/static/src/legacy/js/control_panel/custom_favorite_item.js b/addons/web/static/src/legacy/js/control_panel/custom_favorite_item.js
index 42ecc52bd1b..2a7edd30e41 100644
--- a/addons/web/static/src/legacy/js/control_panel/custom_favorite_item.js
+++ b/addons/web/static/src/legacy/js/control_panel/custom_favorite_item.js
@@ -7,7 +7,7 @@ odoo.define('web.CustomFavoriteItem', function (require) {
     const { useAutofocus } = require("@web/core/utils/hooks");
     const { useModel } = require('web.Model');
 
-    const { Component, useRef, useState } = owl;
+    const { Component, useState } = owl;
 
     let favoriteId = 0;
 
@@ -35,17 +35,14 @@ odoo.define('web.CustomFavoriteItem', function (require) {
             const favId = favoriteId++;
             this.useByDefaultId = `o_favorite_use_by_default_${favId}`;
             this.shareAllUsersId = `o_favorite_share_all_users_${favId}`;
-
-            this.descriptionRef = useRef('description');
-            this.model = useModel('searchModel');
+            this.descriptionRef = useAutofocus();
+            this.model = useModel("searchModel");
             this.interactive = true;
             this.state = useState({
                 description: this.env.action.name || "",
                 isDefault: false,
                 isShared: false,
             });
-
-            useAutofocus("description");
         }
 
         //---------------------------------------------------------------------
diff --git a/addons/web/static/src/legacy/js/control_panel/search_bar.js b/addons/web/static/src/legacy/js/control_panel/search_bar.js
index baf851d85a7..ac06f92852b 100644
--- a/addons/web/static/src/legacy/js/control_panel/search_bar.js
+++ b/addons/web/static/src/legacy/js/control_panel/search_bar.js
@@ -7,7 +7,7 @@ odoo.define('web.SearchBar', function (require) {
     const { useModel } = require('web.Model');
     const { fuzzyTest } = require('@web/core/utils/search');
 
-    const { Component, onMounted, onWillUnmount, useExternalListener, useRef, useState } = owl;
+    const { Component, onMounted, onWillUnmount, useExternalListener, useState } = owl;
     const CHAR_FIELDS = ['char', 'html', 'many2many', 'many2one', 'one2many', 'text'];
 
     let sourceId = 0;
@@ -38,8 +38,7 @@ odoo.define('web.SearchBar', function (require) {
      */
     class SearchBar extends Component {
         setup() {
-            this.focusOnUpdate = useAutofocus('search-input');
-            this.inputRef = useRef('search-input');
+            this.inputRef = useAutofocus();
             this.model = useModel('searchModel');
             this.state = useState({
                 sources: [],
@@ -57,7 +56,9 @@ odoo.define('web.SearchBar', function (require) {
 
             onMounted(() => {
                 // 'search' will always patch the search bar, 'focus' will never.
-                this.model.on('search', this, this.focusOnUpdate);
+                this.model.on('search', this, () => {
+                    this.inputRef.el.focus();
+                });
                 this.model.on('focus-control-panel', this, () => {
                     this.inputRef.el.focus();
                 });
@@ -81,7 +82,7 @@ odoo.define('web.SearchBar', function (require) {
             this.state.focusedItem = 0;
             this.state.inputValue = "";
             this.inputRef.el.value = "";
-            this.focusOnUpdate();
+            this.inputRef.el.focus();
         }
 
         /**
diff --git a/addons/web/static/src/legacy/xml/base.xml b/addons/web/static/src/legacy/xml/base.xml
index 7ecc5595305..17148d78fc3 100644
--- a/addons/web/static/src/legacy/xml/base.xml
+++ b/addons/web/static/src/legacy/xml/base.xml
@@ -129,7 +129,7 @@
             t-att-placeholder="props.placeholder"
             t-attf-data-target="#{{ datePickerId }}"
             t-att-readonly="props.readonly"
-            t-ref="input"
+            t-ref="autofocus"
             t-on-change="_onInputChange"
             t-on-click="_onInputClick"
         />
@@ -362,7 +362,7 @@
             placeholder="Search..."
             role="searchbox"
             title="Search for records"
-            t-ref="search-input"
+            t-ref="autofocus"
             t-on-keydown="_onSearchKeydown"
             t-on-input="_onSearchInput"
         />
diff --git a/addons/web/static/src/search/favorite_menu/custom_favorite_item.js b/addons/web/static/src/search/favorite_menu/custom_favorite_item.js
index defd55a9c46..ff8e4fcabfc 100644
--- a/addons/web/static/src/search/favorite_menu/custom_favorite_item.js
+++ b/addons/web/static/src/search/favorite_menu/custom_favorite_item.js
@@ -5,15 +5,14 @@ import { CheckBox } from "@web/core/checkbox/checkbox";
 import { registry } from "@web/core/registry";
 import { useAutofocus, useService } from "@web/core/utils/hooks";
 
-const { Component, useRef, useState } = owl;
+const { Component, useState } = owl;
 
 const favoriteMenuRegistry = registry.category("favoriteMenu");
 
 export class CustomFavoriteItem extends Component {
     setup() {
         this.notificationService = useService("notification");
-        this.descriptionRef = useRef("description");
-        useAutofocus("description");
+        this.descriptionRef = useAutofocus();
         this.state = useState({
             description: this.env.config.getDisplayName(),
             isDefault: false,
diff --git a/addons/web/static/src/search/favorite_menu/custom_favorite_item.xml b/addons/web/static/src/search/favorite_menu/custom_favorite_item.xml
index 04b53015f30..46d40a1bb8d 100644
--- a/addons/web/static/src/search/favorite_menu/custom_favorite_item.xml
+++ b/addons/web/static/src/search/favorite_menu/custom_favorite_item.xml
@@ -9,7 +9,7 @@
             <div class="px-3 py-2">
                 <input type="text"
                     class="o_input"
-                    t-ref="description"
+                    t-ref="autofocus"
                     t-model.trim="state.description"
                     t-on-keydown="onInputKeydown"
                     />
diff --git a/addons/web/static/src/search/search_bar/search_bar.js b/addons/web/static/src/search/search_bar/search_bar.js
index a0af07ca743..7b9a0d473e9 100644
--- a/addons/web/static/src/search/search_bar/search_bar.js
+++ b/addons/web/static/src/search/search_bar/search_bar.js
@@ -7,7 +7,7 @@ import { KeepLast } from "@web/core/utils/concurrency";
 import { useAutofocus, useBus, useService } from "@web/core/utils/hooks";
 import { fuzzyTest } from "@web/core/utils/search";
 
-const { Component, useExternalListener, useRef, useState } = owl;
+const { Component, useExternalListener, useState } = owl;
 const parsers = registry.category("parsers");
 
 const CHAR_FIELDS = ["char", "html", "many2many", "many2one", "one2many", "text"];
@@ -34,9 +34,7 @@ export class SearchBar extends Component {
 
         this.keepLast = new KeepLast();
 
-        this.focusOnUpdate = useAutofocus("search-input");
-
-        this.inputRef = useRef("search-input");
+        this.inputRef = useAutofocus();
 
         useBus(this.env.bus, "FOCUS-SEARCH-BAR", () => {
             this.inputRef.el.focus();
@@ -237,12 +235,12 @@ export class SearchBar extends Component {
      */
     removeFacet(facet) {
         this.env.searchModel.deactivateGroup(facet.groupId);
-        this.focusOnUpdate();
+        this.inputRef.el.focus();
     }
 
     resetState() {
         this.computeState({ expanded: [], focusedIndex: 0, query: "", subItems: [] });
-        this.focusOnUpdate();
+        this.inputRef.el.focus();
     }
 
     /**
diff --git a/addons/web/static/src/search/search_bar/search_bar.xml b/addons/web/static/src/search/search_bar/search_bar.xml
index 173059506df..db1c9500dde 100644
--- a/addons/web/static/src/search/search_bar/search_bar.xml
+++ b/addons/web/static/src/search/search_bar/search_bar.xml
@@ -39,7 +39,7 @@
             accesskey="Q"
             placeholder="Search..."
             role="searchbox"
-            t-ref="search-input"
+            t-ref="autofocus"
             t-on-keydown="onSearchKeydown"
             t-on-input="onSearchInput"
         />
diff --git a/addons/web/static/tests/core/utils/hooks_tests.js b/addons/web/static/tests/core/utils/hooks_tests.js
index e41de8f9c10..3ca312908c9 100644
--- a/addons/web/static/tests/core/utils/hooks_tests.js
+++ b/addons/web/static/tests/core/utils/hooks_tests.js
@@ -3,10 +3,10 @@
 import { uiService } from "@web/core/ui/ui_service";
 import { useAutofocus, useBus, useListener, useService } from "@web/core/utils/hooks";
 import { registry } from "@web/core/registry";
-import { makeTestEnv } from "../../helpers/mock_env";
-import { click, destroy, getFixture, mount, nextTick } from "../../helpers/utils";
+import { makeTestEnv } from "@web/../tests/helpers/mock_env";
+import { click, destroy, getFixture, mount, nextTick } from "@web/../tests/helpers/utils";
 
-const { Component, xml, useRef } = owl;
+const { Component, xml } = owl;
 const serviceRegistry = registry.category("services");
 
 QUnit.module("utils", () => {
@@ -16,8 +16,7 @@ QUnit.module("utils", () => {
         QUnit.test("useAutofocus: simple usecase", async function (assert) {
             class MyComponent extends Component {
                 setup() {
-                    useAutofocus();
-                    this.inputRef = useRef("autofocus");
+                    this.inputRef = useAutofocus();
                 }
             }
             MyComponent.template = xml`
@@ -43,14 +42,13 @@ QUnit.module("utils", () => {
         QUnit.test("useAutofocus: conditional autofocus", async function (assert) {
             class MyComponent extends Component {
                 setup() {
-                    this.forceFocus = useAutofocus("input");
-                    this.inputRef = useRef("input");
+                    this.inputRef = useAutofocus();
                     this.showInput = true;
                 }
             }
             MyComponent.template = xml`
                 <span>
-                    <input t-if="showInput" type="text" t-ref="input" />
+                    <input t-if="showInput" type="text" t-ref="autofocus" />
                 </span>
             `;
 
@@ -64,13 +62,11 @@ QUnit.module("utils", () => {
             assert.strictEqual(document.activeElement, comp.inputRef.el);
 
             comp.showInput = false;
-            comp.forceFocus();
             comp.render();
             await nextTick();
             assert.notStrictEqual(document.activeElement, comp.inputRef.el);
 
             comp.showInput = true;
-            comp.forceFocus();
             comp.render();
             await nextTick();
             assert.strictEqual(document.activeElement, comp.inputRef.el);