From 6872f0a5470c2e2cbdbbe732bc03844052769c3a Mon Sep 17 00:00:00 2001 From: luvi Date: Mon, 4 Dec 2023 21:51:25 +0000 Subject: [PATCH] [FIX] web: prevent annoying focus on touch devices This commit fixes an annoying issue when using Odoo on a tablet or a PC when using the touch screen primarly. The virtual keyboard popped way too much when navigating in between views and screens, since the focus is often set on inputs (mostly the search bar with useAutofocus). Steps to reproduce: - on a Windows laptop or tablet with a touch screen, open any view - the virtual keyboard appears - you must touch out of the keyboard to use Odoo To fix the useAutofocus hook, hasTouch is now being used instead of relying on the size of the screen. Autofocus test with isSmall has been adapted to specify that touch isn't modified, and that the autofocus is still present. And another test has been added asserting the behavior on a touch device. In the form renderer, the autofocus set manually on the first element is now prevented as well, reducing the popping effect of the keyboard when opening a view. A test has been added as well. task-3627697 closes odoo/odoo#148925 X-original-commit: 1f2ab6d5dda61a6aad6e0e9bd282195cdd990f15 Related: odoo/enterprise#54074 Signed-off-by: Romain Estievenart (res) Signed-off-by: Luca Vitali (luvi) --- addons/web/static/src/core/utils/hooks.js | 9 +- .../static/src/views/form/form_renderer.js | 3 +- .../static/tests/core/utils/hooks_tests.js | 192 +++++++++--------- .../tests/views/form/form_view_tests.js | 24 +++ 4 files changed, 127 insertions(+), 101 deletions(-) diff --git a/addons/web/static/src/core/utils/hooks.js b/addons/web/static/src/core/utils/hooks.js index 5ae0be99334..b8f007ce54a 100644 --- a/addons/web/static/src/core/utils/hooks.js +++ b/addons/web/static/src/core/utils/hooks.js @@ -1,7 +1,7 @@ /** @odoo-module **/ import { SERVICES_METADATA } from "@web/env"; -import { isMobileOS } from "@web/core/browser/feature_detection"; +import { hasTouch, isMobileOS } from "@web/core/browser/feature_detection"; import { status, useComponent, useEffect, useRef, onWillUnmount } from "@odoo/owl"; @@ -35,16 +35,15 @@ import { status, useComponent, useEffect, useRef, onWillUnmount } from "@odoo/ow * @param {Object} [params] * @param {string} [params.refName] override the ref name "autofocus" * @param {boolean} [params.selectAll] if true, will select the entire text value. - * @param {boolean} [params.mobile] if true, will autofocus on mobile devices. + * @param {boolean} [params.mobile] if true, will force autofocus on touch devices. * @returns {Ref} the element reference */ export function useAutofocus({ refName, selectAll, mobile } = {}) { - const comp = useComponent(); const ref = useRef(refName || "autofocus"); const uiService = useService("ui"); - // Prevent autofocus in mobile - if (!mobile && comp.env.isSmall) { + // Prevent autofocus on touch devices to avoid the virtual keyboard from popping up unexpectedly + if (!mobile && hasTouch()) { return ref; } // LEGACY diff --git a/addons/web/static/src/views/form/form_renderer.js b/addons/web/static/src/views/form/form_renderer.js index 94fbc075730..06bf2f77e1a 100644 --- a/addons/web/static/src/views/form/form_renderer.js +++ b/addons/web/static/src/views/form/form_renderer.js @@ -5,6 +5,7 @@ import { Notebook } from "@web/core/notebook/notebook"; import { Setting } from "./setting/setting"; import { Field } from "@web/views/fields/field"; import { browser } from "@web/core/browser/browser"; +import { hasTouch } from "@web/core/browser/feature_detection"; import { useService } from "@web/core/utils/hooks"; import { useDebounced } from "@web/core/utils/timing"; import { ButtonBox } from "@web/views/form/button_box/button_box"; @@ -77,7 +78,7 @@ export class FormRenderer extends Component { } get shouldAutoFocus() { - return !this.props.archInfo.disableAutofocus; + return !hasTouch() && !this.props.archInfo.disableAutofocus; } } diff --git a/addons/web/static/tests/core/utils/hooks_tests.js b/addons/web/static/tests/core/utils/hooks_tests.js index 1ab1b37ad81..530b54af3f6 100644 --- a/addons/web/static/tests/core/utils/hooks_tests.js +++ b/addons/web/static/tests/core/utils/hooks_tests.js @@ -1,5 +1,6 @@ /** @odoo-module **/ +import { browser } from "@web/core/browser/browser"; import { uiService } from "@web/core/ui/ui_service"; import { useAutofocus, @@ -11,7 +12,14 @@ import { } from "@web/core/utils/hooks"; import { registry } from "@web/core/registry"; import { makeTestEnv } from "@web/../tests/helpers/mock_env"; -import { destroy, getFixture, makeDeferred, mount, nextTick } from "@web/../tests/helpers/utils"; +import { + destroy, + getFixture, + makeDeferred, + mount, + nextTick, + patchWithCleanup, +} from "@web/../tests/helpers/utils"; import { Component, onMounted, useState, xml } from "@odoo/owl"; import { dialogService } from "@web/core/dialog/dialog_service"; @@ -110,45 +118,44 @@ QUnit.module("utils", () => { assert.strictEqual(document.activeElement, comp.inputRef.el); }); - QUnit.test("useAutofocus returns also a ref when isSmall is true", async function (assert) { - assert.expect(2); - class MyComponent extends Component { - setup() { - this.inputRef = useAutofocus(); - assert.ok(this.env.isSmall); - onMounted(() => { - assert.ok(this.inputRef.el); - }); + QUnit.test( + "useAutofocus returns also a ref when screen has touch", + async function (assert) { + assert.expect(1); + class MyComponent extends Component { + setup() { + this.inputRef = useAutofocus(); + onMounted(() => { + assert.ok(this.inputRef.el); + }); + } } - } - MyComponent.template = xml` + MyComponent.template = xml` `; - const fakeUIService = { - start(env) { - const ui = {}; - Object.defineProperty(env, "isSmall", { - get() { - return true; - }, - }); + registry.category("services").add("ui", uiService); - return ui; - }, - }; + // patch matchMedia to alter hasTouch value + patchWithCleanup(browser, { + matchMedia: (media) => { + if (media === "(pointer:coarse)") { + return { matches: true }; + } + this._super(); + }, + }); - registry.category("services").add("ui", fakeUIService); - - const env = await makeTestEnv(); - const target = getFixture(); - await mount(MyComponent, target, { env }); - }); + const env = await makeTestEnv(); + const target = getFixture(); + await mount(MyComponent, target, { env }); + } + ); QUnit.test( - "useAutofocus works when isSmall and you provide mobile param", + "useAutofocus works when screen has touch and you provide mobile param", async function (assert) { class MyComponent extends Component { setup() { @@ -161,20 +168,17 @@ QUnit.module("utils", () => { `; - const fakeUIService = { - start(env) { - const ui = {}; - Object.defineProperty(env, "isSmall", { - get() { - return true; - }, - }); + registry.category("services").add("ui", uiService); - return ui; + // patch matchMedia to alter hasTouch value + patchWithCleanup(browser, { + matchMedia: (media) => { + if (media === "(pointer:coarse)") { + return { matches: true }; + } + this._super(); }, - }; - - registry.category("services").add("ui", fakeUIService); + }); const env = await makeTestEnv(); const target = getFixture(); @@ -182,41 +186,36 @@ QUnit.module("utils", () => { assert.strictEqual(document.activeElement, comp.inputRef.el); } ); - QUnit.test( - "useAutofocus does not focus when isSmall and you don't provide mobile param", - async function (assert) { - class MyComponent extends Component { - setup() { - this.inputRef = useAutofocus(); - } + + QUnit.test("useAutofocus does not focus when screen has touch", async function (assert) { + class MyComponent extends Component { + setup() { + this.inputRef = useAutofocus(); } - MyComponent.template = xml` + } + MyComponent.template = xml` `; - const fakeUIService = { - start(env) { - const ui = {}; - Object.defineProperty(env, "isSmall", { - get() { - return true; - }, - }); + registry.category("services").add("ui", uiService); - return ui; - }, - }; + // patch matchMedia to alter hasTouch value + patchWithCleanup(browser, { + matchMedia: (media) => { + if (media === "(pointer:coarse)") { + return { matches: true }; + } + this._super(); + }, + }); - registry.category("services").add("ui", fakeUIService); - - const env = await makeTestEnv(); - const target = getFixture(); - const comp = await mount(MyComponent, target, { env }); - assert.notEqual(document.activeElement, comp.inputRef.el); - } - ); + const env = await makeTestEnv(); + const target = getFixture(); + const comp = await mount(MyComponent, target, { env }); + assert.notEqual(document.activeElement, comp.inputRef.el); + }); QUnit.test("supports different ref names", async (assert) => { class MyComponent extends Component { @@ -276,16 +275,18 @@ QUnit.module("utils", () => { assert.strictEqual(comp.inputRef.el.selectionEnd, 10); }); - QUnit.test("useAutofocus: autofocus outside of active element doesn't work (CommandPalette)", async function (assert) { - class MyComponent extends Component { - setup() { - this.inputRef = useAutofocus(); + QUnit.test( + "useAutofocus: autofocus outside of active element doesn't work (CommandPalette)", + async function (assert) { + class MyComponent extends Component { + setup() { + this.inputRef = useAutofocus(); + } + get OverlayContainer() { + return registry.category("main_components").get("OverlayContainer"); + } } - get OverlayContainer() { - return registry.category("main_components").get("OverlayContainer"); - } - } - MyComponent.template = xml` + MyComponent.template = xml`
@@ -293,27 +294,28 @@ QUnit.module("utils", () => {
`; - registry.category("services").add("ui", uiService); - registry.category("services").add("dialog", dialogService); - registry.category("services").add("hotkey", hotkeyService); + registry.category("services").add("ui", uiService); + registry.category("services").add("dialog", dialogService); + registry.category("services").add("hotkey", hotkeyService); - const config = { providers: [] }; - const env = await makeTestEnv(); - const target = getFixture(); - const comp = await mount(MyComponent, target , { env }); - await nextTick(); + const config = { providers: [] }; + const env = await makeTestEnv(); + const target = getFixture(); + const comp = await mount(MyComponent, target, { env }); + await nextTick(); - assert.strictEqual(document.activeElement, comp.inputRef.el); + assert.strictEqual(document.activeElement, comp.inputRef.el); - env.services.dialog.add(CommandPalette, { config }); - await nextTick(); - assert.containsOnce(target, ".o_command_palette"); - assert.notStrictEqual(document.activeElement, comp.inputRef.el); + env.services.dialog.add(CommandPalette, { config }); + await nextTick(); + assert.containsOnce(target, ".o_command_palette"); + assert.notStrictEqual(document.activeElement, comp.inputRef.el); - comp.render(); - await nextTick(); - assert.notStrictEqual(document.activeElement, comp.inputRef.el); - }); + comp.render(); + await nextTick(); + assert.notStrictEqual(document.activeElement, comp.inputRef.el); + } + ); QUnit.module("useBus"); diff --git a/addons/web/static/tests/views/form/form_view_tests.js b/addons/web/static/tests/views/form/form_view_tests.js index 6016d7cf449..7ceb3854bb2 100644 --- a/addons/web/static/tests/views/form/form_view_tests.js +++ b/addons/web/static/tests/views/form/form_view_tests.js @@ -8208,6 +8208,30 @@ QUnit.module("Views", (hooks) => { ); }); + QUnit.test("on a touch screen, fields are not focused", async function (assert) { + // patch matchMedia to alter hasTouch value + patchWithCleanup(browser, { + matchMedia: (media) => { + if (media === "(pointer:coarse)") { + return { matches: true }; + } + this._super(); + }, + }); + + await makeView({ + type: "form", + resModel: "partner", + serverData, + arch: '
', + }); + + assert.notEqual( + document.activeElement, + target.querySelector('.o_field_widget[name="foo"] input') + ); + }); + QUnit.test( "no autofocus with disable_autofocus option [REQUIRE FOCUS]", async function (assert) {