From 130a7d9ee118563b7bf792634e220fbb06466dcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Thu, 28 Jul 2022 13:43:38 +0000 Subject: [PATCH] [IMP] web, web_tour: improve isVisible helper Before this commit, the isVisible helper would most of the time work as expected, except if an element has display: contents. In that case, it has no bounding box, but it may still be visible if one of its child is visible. This is particularly important since we want to set the display property of all field components to "contents". Part-of: odoo/odoo#96865 --- addons/web/static/src/core/macro.js | 7 +- addons/web/static/src/core/utils/ui.js | 71 ++++++++++++++++++- .../web/static/tests/core/colorlist_tests.js | 10 +-- addons/web/static/tests/helpers/utils.js | 8 +-- addons/web/static/tests/main.js | 2 + addons/web/static/tests/qunit.js | 61 ++++++++-------- .../fields/boolean_toggle_field_tests.js | 30 ++++---- .../static/tests/views/kanban_view_tests.js | 2 +- .../web/static/tests/views/list_view_tests.js | 8 +-- .../tests/webclient/actions/target_tests.js | 5 +- addons/web_tour/static/src/js/tour_utils.js | 4 +- .../tours/website_event_booth_exhibitor.js | 2 +- 12 files changed, 133 insertions(+), 77 deletions(-) diff --git a/addons/web/static/src/core/macro.js b/addons/web/static/src/core/macro.js index 7511e564dab..b4499341222 100644 --- a/addons/web/static/src/core/macro.js +++ b/addons/web/static/src/core/macro.js @@ -1,12 +1,7 @@ /** @odoo-module **/ import { browser } from "@web/core/browser/browser"; - -// rough approximation of a visible element. not perfect (does not take into -// account opacity = 0 for example), but good enough for our purpose -function isVisible(e) { - return e.offsetWidth > 0 || e.offsetHeight > 0; -} +import { isVisible } from "@web/core/utils/ui"; export const ACTION_HELPERS = { click(el, _step) { diff --git a/addons/web/static/src/core/utils/ui.js b/addons/web/static/src/core/utils/ui.js index 09740b0f4da..0126c5ee641 100644 --- a/addons/web/static/src/core/utils/ui.js +++ b/addons/web/static/src/core/utils/ui.js @@ -1,5 +1,73 @@ /** @odoo-module **/ +/** + * rough approximation of a visible element. not perfect (does not take into + * account opacity = 0 for example), but good enough for our purpose + * + * @param {Element} el + * @returns {boolean} + */ +export function isVisible(el) { + if (el === document || el === window) { + return true; + } + if (!el) { + return false; + } + let _isVisible = false; + if ("offsetWidth" in el && "offsetHeight" in el) { + _isVisible = el.offsetWidth > 0 && el.offsetHeight > 0; + } else if ("getBoundingClientRect" in el) { + // for example, svgelements + const rect = el.getBoundingClientRect(); + _isVisible = rect.width > 0 && rect.height > 0; + } + if (!_isVisible && getComputedStyle(el).display === "contents") { + for (const child of el.children) { + if (isVisible(child)) { + return true; + } + } + } + return _isVisible; +} + +/** + * This function only exists because some tours currently rely on the fact that + * we can click on elements with a non null width *xor* height (not both). However, + * if one of these is 0, the element is not visible. We thus keep this function + * to ease the transition to the more robust "isVisible" helper, which requires + * both a non null width *and* height. + * + * @deprecated use isVisible instead + * @param {Element} el + * @returns {boolean} + */ +export function _legacyIsVisible(el) { + if (el === document || el === window) { + return true; + } + if (!el) { + return false; + } + let _isVisible = false; + if ("offsetWidth" in el && "offsetHeight" in el) { + _isVisible = el.offsetWidth > 0 || el.offsetHeight > 0; + } else if ("getBoundingClientRect" in el) { + // for example, svgelements + const rect = el.getBoundingClientRect(); + _isVisible = rect.width > 0 || rect.height > 0; + } + if (!_isVisible && getComputedStyle(el).display === "contents") { + for (const child of el.children) { + if (isVisible(child)) { + return true; + } + } + } + return _isVisible; +} + /** * @param {Element} activeElement * @param {String} selector @@ -10,8 +78,7 @@ export function getVisibleElements(activeElement, selector) { /** @type {NodeListOf} */ const elements = activeElement.querySelectorAll(selector); for (const el of elements) { - const isVisible = el.offsetWidth > 0 && el.offsetHeight > 0; - if (isVisible) { + if (isVisible(el)) { visibleElements.push(el); } } diff --git a/addons/web/static/tests/core/colorlist_tests.js b/addons/web/static/tests/core/colorlist_tests.js index 3d8d2ae85aa..f42edb411d3 100644 --- a/addons/web/static/tests/core/colorlist_tests.js +++ b/addons/web/static/tests/core/colorlist_tests.js @@ -23,7 +23,7 @@ async function mountComponent(Picker, props) { class Parent extends Component {} Parent.template = xml/* xml */ ` -
+
Outside div
`; const env = await makeTestEnv(); @@ -38,8 +38,6 @@ QUnit.module("Components", () => { QUnit.module("ColorList"); QUnit.test("basic rendering", async function (assert) { - assert.expect(4); - await mountComponent(ColorList, { colors: [0, 9], }); @@ -60,8 +58,6 @@ QUnit.module("Components", () => { }); QUnit.test("toggler is available if togglerColor props is given", async function (assert) { - assert.expect(9); - const togglerColorId = 0; await mountComponent(ColorList, { colors: [4, 5, 6], @@ -88,10 +84,10 @@ QUnit.module("Components", () => { "button.o_colorlist_toggler", "toggler button is no longer visible" ); - assert.containsN(target, "button", 3, "three buttons are available"); + assert.containsN(target, ".o_colorlist button", 3, "three buttons are available"); await click(target.querySelector(".outsideDiv")); - assert.containsN(target, "button", 1, "only one button is available"); + assert.containsOnce(target, ".o_colorlist button", "only one button is available"); assert.containsOnce( target, "button.o_colorlist_toggler", diff --git a/addons/web/static/tests/helpers/utils.js b/addons/web/static/tests/helpers/utils.js index 7deb9242e2e..f1c0987ce9a 100644 --- a/addons/web/static/tests/helpers/utils.js +++ b/addons/web/static/tests/helpers/utils.js @@ -5,6 +5,7 @@ import { isMacOS } from "@web/core/browser/feature_detection"; import { download } from "@web/core/network/download"; import { Deferred } from "@web/core/utils/concurrency"; import { patch, unpatch } from "@web/core/utils/patch"; +import { isVisible } from "@web/core/utils/ui"; import { registerCleanup } from "./cleanup"; const { App, onMounted, onPatched, useComponent } = owl; @@ -285,12 +286,7 @@ export function triggerEvent(el, selector, eventType, eventAttrs = {}, options = throw new Error(`Can't find a target to trigger ${eventType} event`); } if (!options.skipVisibilityCheck) { - const isVisible = - target === document || - target === window || - target.offsetWidth > 0 || - target.offsetHeight > 0; - if (!isVisible) { + if (!isVisible(target)) { throw new Error(`Called triggerEvent ${eventType} on invisible target`); } } diff --git a/addons/web/static/tests/main.js b/addons/web/static/tests/main.js index 8ed491aba20..3eb55113dc5 100644 --- a/addons/web/static/tests/main.js +++ b/addons/web/static/tests/main.js @@ -1,8 +1,10 @@ /** @odoo-module **/ +import { setupQUnit } from "./qunit"; import { setupTests } from "./setup"; (async () => { + setupQUnit(); await setupTests(); QUnit.start(); })(); diff --git a/addons/web/static/tests/qunit.js b/addons/web/static/tests/qunit.js index 2cb5ccdf026..6ba6382a5b9 100644 --- a/addons/web/static/tests/qunit.js +++ b/addons/web/static/tests/qunit.js @@ -1,14 +1,37 @@ /** @odoo-module */ -(function () { - "use strict"; +import { isVisible as isElemVisible } from "@web/core/utils/ui"; +function setQUnitDebugMode() { + owl.whenReady(() => document.body.classList.add("debug")); // make the test visible to the naked eye + QUnit.config.debug = true; // allows for helper functions to behave differently (logging, the HTML element in which the test occurs etc...) + QUnit.config.testTimeout = 60 * 60 * 1000; + // Allows for interacting with the test when it is over + // In fact, this will pause QUnit. + // Also, logs useful info in the console. + QUnit.testDone(async (...args) => { + console.groupCollapsed("Debug Test output"); + console.log(...args); + console.groupEnd(); + await new Promise(() => {}); + }); +} + +// need to do this outside of the setup function so the QUnit.debug is defined when we need it +QUnit.debug = (name, cb) => { + setQUnitDebugMode(); + QUnit.only(name, cb); +}; + +// need to do this outside of the setup function so it is executed quickly +QUnit.config.autostart = false; + +export function setupQUnit() { const { Component } = owl; // ----------------------------------------------------------------------------- // QUnit config // ----------------------------------------------------------------------------- - QUnit.config.autostart = false; QUnit.config.testTimeout = 1 * 60 * 1000; QUnit.config.hidepassed = window.location.href.match(/[?&]testId=/) === null; @@ -166,14 +189,8 @@ } } msg = msg || `target should ${shouldBeVisible ? "" : "not"} be visible`; - let isVisible = el && el.offsetWidth && el.offsetHeight; - if (isVisible) { - // This computation is a little more heavy and we only want to perform it - // if the above assertion has failed. - const rect = el.getBoundingClientRect(); - isVisible = rect.width + rect.height; - } - const condition = shouldBeVisible ? isVisible : !isVisible; + const _isVisible = isElemVisible(el); + const condition = shouldBeVisible ? _isVisible : !_isVisible; QUnit.assert.ok(condition, msg); } function isVisible(el, msg) { @@ -330,26 +347,6 @@ reRun.parentElement.insertBefore(reRunDebug, reRun.nextSibling); }); - function setQUnitDebugMode() { - owl.whenReady(() => document.body.classList.add("debug")); // make the test visible to the naked eye - QUnit.config.debug = true; // allows for helper functions to behave differently (logging, the HTML element in which the test occurs etc...) - QUnit.config.testTimeout = 60 * 60 * 1000; - // Allows for interacting with the test when it is over - // In fact, this will pause QUnit. - // Also, logs useful info in the console. - QUnit.testDone(async (...args) => { - console.groupCollapsed("Debug Test output"); - console.log(...args); - console.groupEnd(); - await new Promise(() => {}); - }); - } - - QUnit.debug = (name, cb) => { - setQUnitDebugMode(); - QUnit.only(name, cb); - }; - const debugTestId = new URLSearchParams(location.search).get("debugTestId"); if (debugTestId) { QUnit.config.testId = [debugTestId]; @@ -513,4 +510,4 @@ el.innerText = "details:not([open]) > :not(summary) { display: none; }"; document.head.appendChild(el); }); -})(); +} diff --git a/addons/web/static/tests/views/fields/boolean_toggle_field_tests.js b/addons/web/static/tests/views/fields/boolean_toggle_field_tests.js index b7acbdac836..1f28dab830b 100644 --- a/addons/web/static/tests/views/fields/boolean_toggle_field_tests.js +++ b/addons/web/static/tests/views/fields/boolean_toggle_field_tests.js @@ -55,7 +55,7 @@ QUnit.module("Fields", (hooks) => { ); }); - QUnit.test("readonly BooleanToggleField is not disabled in edit mode", async function (assert) { + QUnit.test("readonly BooleanToggleField is disabled in edit mode", async function (assert) { await makeView({ type: "form", resModel: "partner", @@ -66,11 +66,8 @@ QUnit.module("Fields", (hooks) => { `, }); - assert.containsOnce(target, ".o_boolean_toggle input:disabled:checked"); - - await click(target, ".o_field_widget[name='bar'] label"); - - assert.containsOnce(target, ".o_boolean_toggle input:disabled:checked"); + assert.containsOnce(target, ".o_form_editable"); + assert.ok(target.querySelector(".o_boolean_toggle input").disabled); }); QUnit.test("BooleanToggleField is not disabled in readonly mode", async function (assert) { @@ -82,9 +79,11 @@ QUnit.module("Fields", (hooks) => { resId: 1, }); + assert.containsOnce(target, ".o_form_readonly"); assert.containsOnce(target, ".form-check.o_boolean_toggle"); + assert.notOk(target.querySelector(".o_boolean_toggle input").disabled); assert.notOk(target.querySelector(".o_boolean_toggle input").checked); - await click(target, ".o_field_widget[name='bar'] label"); + await click(target, ".o_field_widget[name='bar'] input"); assert.ok(target.querySelector(".o_boolean_toggle input").checked); }); @@ -98,10 +97,9 @@ QUnit.module("Fields", (hooks) => { }); assert.containsOnce(target, ".form-check.o_boolean_toggle"); + assert.ok(target.querySelector(".o_boolean_toggle input").disabled); await click(target.querySelector(".o_form_button_edit")); - assert.notOk(target.querySelector(".o_boolean_toggle input").checked); - await click(target, ".o_field_widget[name='bar'] label"); - assert.notOk(target.querySelector(".o_boolean_toggle input").checked); + assert.ok(target.querySelector(".o_boolean_toggle input").disabled); }); QUnit.test("BooleanToggleField is enabled in edit mode", async function (assert) { @@ -116,23 +114,23 @@ QUnit.module("Fields", (hooks) => { assert.containsOnce(target, ".form-check.o_boolean_toggle"); await click(target.querySelector(".o_form_button_edit")); + assert.notOk(target.querySelector(".o_boolean_toggle input").disabled); assert.notOk(target.querySelector(".o_boolean_toggle input").checked); - await click(target, ".o_field_widget[name='bar'] label"); + await click(target, ".o_field_widget[name='bar'] input"); + assert.notOk(target.querySelector(".o_boolean_toggle input").disabled); assert.ok(target.querySelector(".o_boolean_toggle input").checked); }); QUnit.test("boolean toggle widget is not disabled in readonly mode", async function (assert) { - assert.expect(3); - await makeView({ type: "form", serverData, resModel: "partner", resId: 1, arch: ` -
- - `, +
+ + `, }); assert.containsOnce( diff --git a/addons/web/static/tests/views/kanban_view_tests.js b/addons/web/static/tests/views/kanban_view_tests.js index d64a4c33b6f..3164299bc0d 100644 --- a/addons/web/static/tests/views/kanban_view_tests.js +++ b/addons/web/static/tests/views/kanban_view_tests.js @@ -9204,7 +9204,7 @@ QUnit.module("Views", (hooks) => { assert.ok(getCard(0).querySelector("[name='bar'] input").checked); assert.ok(getCard(1).querySelector("[name='bar'] input").checked); - await click(getCard(1), "[name='bar'] label"); + await click(getCard(1), "[name='bar'] input"); assert.ok(getCard(0).querySelector("[name='bar'] input").checked); assert.notOk(getCard(1).querySelector("[name='bar'] input").checked); }); diff --git a/addons/web/static/tests/views/list_view_tests.js b/addons/web/static/tests/views/list_view_tests.js index e9dad2c078e..7165ad23623 100644 --- a/addons/web/static/tests/views/list_view_tests.js +++ b/addons/web/static/tests/views/list_view_tests.js @@ -3129,7 +3129,7 @@ QUnit.module("Views", (hooks) => { `, }); - assert.isNotVisible(target.querySelector(".o_field_one2many")); + assert.containsNone(target, ".o_field_one2many"); await click(target.querySelector(".nav-item:last-child .nav-link")); @@ -5079,7 +5079,7 @@ QUnit.module("Views", (hooks) => { await toggleMenuItem(target, "Empty List"); assert.containsOnce(target, ".o_view_nocontent"); - await click(target, ".o_view_nocontent"); + await click(target, ".o_nocontent_help"); assert.hasClass(target.querySelector(".o_list_button_add"), "o_catch_attention"); } ); @@ -8649,7 +8649,7 @@ QUnit.module("Views", (hooks) => { await click(target.querySelectorAll(".o_data_row .o_list_record_selector input")[0]); await click(target.querySelectorAll(".o_data_row .o_list_record_selector input")[1]); await click(target.querySelectorAll(".o_data_row .o_list_record_selector input")[2]); - await click(target.querySelector(".o_data_row .o_field_boolean label")); + await click(target.querySelector(".o_data_row .o_field_boolean input")); assert.containsOnce(target, ".modal"); await click(target, ".modal .modal-footer .btn-primary"); @@ -9471,7 +9471,7 @@ QUnit.module("Views", (hooks) => { const rows = target.querySelectorAll(".o_data_row"); await click(rows[0], ".o_list_record_selector input"); await click(rows[2], ".o_list_record_selector input"); - await click(rows[0].querySelector(".o_boolean_toggle label")); + await click(rows[0].querySelector(".o_boolean_toggle input")); assert.ok( $(".modal").text().includes("Confirmation"), diff --git a/addons/web/static/tests/webclient/actions/target_tests.js b/addons/web/static/tests/webclient/actions/target_tests.js index 86ce2343713..5c5a6c4b5fa 100644 --- a/addons/web/static/tests/webclient/actions/target_tests.js +++ b/addons/web/static/tests/webclient/actions/target_tests.js @@ -358,7 +358,10 @@ QUnit.module("ActionManager", (hooks) => { } } } - ClientAction.template = xml`
`; + ClientAction.template = xml` +
+ My Action +
`; registry.category("actions").add("clientAction", ClientAction); const errorDialogOpened = makeDeferred(); diff --git a/addons/web_tour/static/src/js/tour_utils.js b/addons/web_tour/static/src/js/tour_utils.js index ff0d870ff15..18570540aba 100644 --- a/addons/web_tour/static/src/js/tour_utils.js +++ b/addons/web_tour/static/src/js/tour_utils.js @@ -1,6 +1,8 @@ odoo.define('web_tour.utils', function(require) { "use strict"; +const { _legacyIsVisible } = require("@web/core/utils/ui"); + function get_step_key(name) { return 'tour_' + name + '_step'; } @@ -20,7 +22,7 @@ function get_running_delay_key() { function get_first_visible_element($elements) { for (var i = 0 ; i < $elements.length ; i++) { var $i = $elements.eq(i); - if ($i.is(':visible:hasVisibility')) { + if (_legacyIsVisible($i[0])) { return $i; } } diff --git a/addons/website_event_booth_exhibitor/static/tests/tours/website_event_booth_exhibitor.js b/addons/website_event_booth_exhibitor/static/tests/tours/website_event_booth_exhibitor.js index 3262c207968..4b49211d51a 100644 --- a/addons/website_event_booth_exhibitor/static/tests/tours/website_event_booth_exhibitor.js +++ b/addons/website_event_booth_exhibitor/static/tests/tours/website_event_booth_exhibitor.js @@ -38,7 +38,7 @@ odoo.define("website_event_booth_exhibitor.tour", function (require) { trigger: 'img[alt="Premium Booth"]', }, { content: 'Choose Booth', - trigger: 'div:contains("OpenWood Demonstrator 2") input', + trigger: '.o_wbooth_booths div:contains("OpenWood Demonstrator 2") input', }, { content: "Validate attendees details", trigger: 'button:contains("Book my Booths")',